From 358b8bebc6c65f40250ca6ff796a2fe23085499a Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Thu, 22 Aug 2019 12:19:51 +0200 Subject: [PATCH] set a storage availability delay on auth issues to avoid lock out Signed-off-by: Arthur Schiwon --- apps/files_external/lib/Lib/Storage/SMB.php | 18 +- config/config.sample.php | 11 + lib/private/Files/Cache/Storage.php | 5 +- .../Files/Storage/Wrapper/Availability.php | 206 +++++++++--------- 4 files changed, 126 insertions(+), 114 deletions(-) diff --git a/apps/files_external/lib/Lib/Storage/SMB.php b/apps/files_external/lib/Lib/Storage/SMB.php index 5c8804695b..85b4e62075 100644 --- a/apps/files_external/lib/Lib/Storage/SMB.php +++ b/apps/files_external/lib/Lib/Storage/SMB.php @@ -55,6 +55,7 @@ use OCA\Files_External\Lib\Notify\SMBNotifyHandler; use OCP\Files\Notify\IChange; use OCP\Files\Notify\IRenameChange; use OCP\Files\Storage\INotifyStorage; +use OCP\Files\StorageAuthException; use OCP\Files\StorageNotAvailableException; use OCP\ILogger; @@ -160,7 +161,7 @@ class SMB extends Common implements INotifyStorage { /** * @param string $path * @return \Icewind\SMB\IFileInfo - * @throws StorageNotAvailableException + * @throws StorageAuthException */ protected function getFileInfo($path) { try { @@ -170,19 +171,26 @@ class SMB extends Common implements INotifyStorage { } return $this->statCache[$path]; } catch (ConnectException $e) { - $this->logger->logException($e, ['message' => 'Error while getting file info']); - throw new StorageNotAvailableException($e->getMessage(), $e->getCode(), $e); + $this->throwUnavailable($e); } catch (ForbiddenException $e) { // with php-smbclient, this exceptions is thrown when the provided password is invalid. // Possible is also ForbiddenException with a different error code, so we check it. if($e->getCode() === 1) { - $this->logger->logException($e, ['message' => 'Error while getting file info']); - throw new StorageNotAvailableException($e->getMessage(), $e->getCode(), $e); + $this->throwUnavailable($e); } throw $e; } } + /** + * @param \Exception $e + * @throws StorageAuthException + */ + protected function throwUnavailable(\Exception $e) { + $this->logger->logException($e, ['message' => 'Error while getting file info']); + throw new StorageAuthException($e->getMessage(), $e); + } + /** * @param string $path * @return \Icewind\SMB\IFileInfo[] diff --git a/config/config.sample.php b/config/config.sample.php index f939fc371b..e6ef5d9079 100644 --- a/config/config.sample.php +++ b/config/config.sample.php @@ -1480,6 +1480,17 @@ $CONFIG = array( */ 'quota_include_external_storage' => false, +/** + * When an external storage is unavailable for some reasons, it will be flagged + * as such for 10 minutes. When the trigger is a failed authentication attempt + * the delay is higher and can be controlled with this option. The motivation + * is to make account lock outs at Active Directories (and compatible) more + * unlikely. + * + * Defaults to ``1800`` (seconds) + */ +'external_storage.auth_availability_delay' => 1800, + /** * Specifies how often the local filesystem (the Nextcloud data/ directory, and * NFS mounts in data/) is checked for changes made outside Nextcloud. This diff --git a/lib/private/Files/Cache/Storage.php b/lib/private/Files/Cache/Storage.php index 5b37c1f43f..72aaac11c5 100644 --- a/lib/private/Files/Cache/Storage.php +++ b/lib/private/Files/Cache/Storage.php @@ -166,11 +166,12 @@ class Storage { /** * @param bool $isAvailable + * @param int $delay amount of seconds to delay reconsidering that storage further */ - public function setAvailability($isAvailable) { + public function setAvailability($isAvailable, int $delay = 0) { $sql = 'UPDATE `*PREFIX*storages` SET `available` = ?, `last_checked` = ? WHERE `id` = ?'; $available = $isAvailable ? 1 : 0; - \OC_DB::executeAudited($sql, array($available, time(), $this->storageId)); + \OC_DB::executeAudited($sql, [$available, time() + $delay, $this->storageId]); } /** diff --git a/lib/private/Files/Storage/Wrapper/Availability.php b/lib/private/Files/Storage/Wrapper/Availability.php index 5b957ae036..77a23d26fb 100644 --- a/lib/private/Files/Storage/Wrapper/Availability.php +++ b/lib/private/Files/Storage/Wrapper/Availability.php @@ -24,6 +24,9 @@ namespace OC\Files\Storage\Wrapper; use OCP\Files\Storage\IStorage; +use OCP\Files\StorageAuthException; +use OCP\Files\StorageNotAvailableException; +use OCP\IConfig; /** * Availability checker for storages @@ -33,6 +36,14 @@ use OCP\Files\Storage\IStorage; class Availability extends Wrapper { const RECHECK_TTL_SEC = 600; // 10 minutes + /** @var IConfig */ + protected $config; + + public function __construct($parameters) { + $this->config = $parameters['config'] ?? \OC::$server->getConfig(); + parent::__construct($parameters); + } + public static function shouldRecheck($availability) { if (!$availability['available']) { // trigger a recheck if TTL reached @@ -72,11 +83,11 @@ class Availability extends Wrapper { } /** - * @throws \OCP\Files\StorageNotAvailableException + * @throws StorageNotAvailableException */ private function checkAvailability() { if (!$this->isAvailable()) { - throw new \OCP\Files\StorageNotAvailableException(); + throw new StorageNotAvailableException(); } } @@ -85,9 +96,8 @@ class Availability extends Wrapper { $this->checkAvailability(); try { return parent::mkdir($path); - } catch (\OCP\Files\StorageNotAvailableException $e) { - $this->setAvailability(false); - throw $e; + } catch (StorageNotAvailableException $e) { + $this->setUnavailable($e); } } @@ -96,9 +106,8 @@ class Availability extends Wrapper { $this->checkAvailability(); try { return parent::rmdir($path); - } catch (\OCP\Files\StorageNotAvailableException $e) { - $this->setAvailability(false); - throw $e; + } catch (StorageNotAvailableException $e) { + $this->setUnavailable($e); } } @@ -107,9 +116,8 @@ class Availability extends Wrapper { $this->checkAvailability(); try { return parent::opendir($path); - } catch (\OCP\Files\StorageNotAvailableException $e) { - $this->setAvailability(false); - throw $e; + } catch (StorageNotAvailableException $e) { + $this->setUnavailable($e); } } @@ -118,9 +126,8 @@ class Availability extends Wrapper { $this->checkAvailability(); try { return parent::is_dir($path); - } catch (\OCP\Files\StorageNotAvailableException $e) { - $this->setAvailability(false); - throw $e; + } catch (StorageNotAvailableException $e) { + $this->setUnavailable($e); } } @@ -129,9 +136,8 @@ class Availability extends Wrapper { $this->checkAvailability(); try { return parent::is_file($path); - } catch (\OCP\Files\StorageNotAvailableException $e) { - $this->setAvailability(false); - throw $e; + } catch (StorageNotAvailableException $e) { + $this->setUnavailable($e); } } @@ -140,9 +146,8 @@ class Availability extends Wrapper { $this->checkAvailability(); try { return parent::stat($path); - } catch (\OCP\Files\StorageNotAvailableException $e) { - $this->setAvailability(false); - throw $e; + } catch (StorageNotAvailableException $e) { + $this->setUnavailable($e); } } @@ -151,9 +156,8 @@ class Availability extends Wrapper { $this->checkAvailability(); try { return parent::filetype($path); - } catch (\OCP\Files\StorageNotAvailableException $e) { - $this->setAvailability(false); - throw $e; + } catch (StorageNotAvailableException $e) { + $this->setUnavailable($e); } } @@ -162,9 +166,8 @@ class Availability extends Wrapper { $this->checkAvailability(); try { return parent::filesize($path); - } catch (\OCP\Files\StorageNotAvailableException $e) { - $this->setAvailability(false); - throw $e; + } catch (StorageNotAvailableException $e) { + $this->setUnavailable($e); } } @@ -173,9 +176,8 @@ class Availability extends Wrapper { $this->checkAvailability(); try { return parent::isCreatable($path); - } catch (\OCP\Files\StorageNotAvailableException $e) { - $this->setAvailability(false); - throw $e; + } catch (StorageNotAvailableException $e) { + $this->setUnavailable($e); } } @@ -184,9 +186,8 @@ class Availability extends Wrapper { $this->checkAvailability(); try { return parent::isReadable($path); - } catch (\OCP\Files\StorageNotAvailableException $e) { - $this->setAvailability(false); - throw $e; + } catch (StorageNotAvailableException $e) { + $this->setUnavailable($e); } } @@ -195,9 +196,8 @@ class Availability extends Wrapper { $this->checkAvailability(); try { return parent::isUpdatable($path); - } catch (\OCP\Files\StorageNotAvailableException $e) { - $this->setAvailability(false); - throw $e; + } catch (StorageNotAvailableException $e) { + $this->setUnavailable($e); } } @@ -206,9 +206,8 @@ class Availability extends Wrapper { $this->checkAvailability(); try { return parent::isDeletable($path); - } catch (\OCP\Files\StorageNotAvailableException $e) { - $this->setAvailability(false); - throw $e; + } catch (StorageNotAvailableException $e) { + $this->setUnavailable($e); } } @@ -217,9 +216,8 @@ class Availability extends Wrapper { $this->checkAvailability(); try { return parent::isSharable($path); - } catch (\OCP\Files\StorageNotAvailableException $e) { - $this->setAvailability(false); - throw $e; + } catch (StorageNotAvailableException $e) { + $this->setUnavailable($e); } } @@ -228,9 +226,8 @@ class Availability extends Wrapper { $this->checkAvailability(); try { return parent::getPermissions($path); - } catch (\OCP\Files\StorageNotAvailableException $e) { - $this->setAvailability(false); - throw $e; + } catch (StorageNotAvailableException $e) { + $this->setUnavailable($e); } } @@ -242,9 +239,8 @@ class Availability extends Wrapper { $this->checkAvailability(); try { return parent::file_exists($path); - } catch (\OCP\Files\StorageNotAvailableException $e) { - $this->setAvailability(false); - throw $e; + } catch (StorageNotAvailableException $e) { + $this->setUnavailable($e); } } @@ -253,9 +249,8 @@ class Availability extends Wrapper { $this->checkAvailability(); try { return parent::filemtime($path); - } catch (\OCP\Files\StorageNotAvailableException $e) { - $this->setAvailability(false); - throw $e; + } catch (StorageNotAvailableException $e) { + $this->setUnavailable($e); } } @@ -264,9 +259,8 @@ class Availability extends Wrapper { $this->checkAvailability(); try { return parent::file_get_contents($path); - } catch (\OCP\Files\StorageNotAvailableException $e) { - $this->setAvailability(false); - throw $e; + } catch (StorageNotAvailableException $e) { + $this->setUnavailable($e); } } @@ -275,9 +269,8 @@ class Availability extends Wrapper { $this->checkAvailability(); try { return parent::file_put_contents($path, $data); - } catch (\OCP\Files\StorageNotAvailableException $e) { - $this->setAvailability(false); - throw $e; + } catch (StorageNotAvailableException $e) { + $this->setUnavailable($e); } } @@ -286,9 +279,8 @@ class Availability extends Wrapper { $this->checkAvailability(); try { return parent::unlink($path); - } catch (\OCP\Files\StorageNotAvailableException $e) { - $this->setAvailability(false); - throw $e; + } catch (StorageNotAvailableException $e) { + $this->setUnavailable($e); } } @@ -297,9 +289,8 @@ class Availability extends Wrapper { $this->checkAvailability(); try { return parent::rename($path1, $path2); - } catch (\OCP\Files\StorageNotAvailableException $e) { - $this->setAvailability(false); - throw $e; + } catch (StorageNotAvailableException $e) { + $this->setUnavailable($e); } } @@ -308,9 +299,8 @@ class Availability extends Wrapper { $this->checkAvailability(); try { return parent::copy($path1, $path2); - } catch (\OCP\Files\StorageNotAvailableException $e) { - $this->setAvailability(false); - throw $e; + } catch (StorageNotAvailableException $e) { + $this->setUnavailable($e); } } @@ -319,9 +309,8 @@ class Availability extends Wrapper { $this->checkAvailability(); try { return parent::fopen($path, $mode); - } catch (\OCP\Files\StorageNotAvailableException $e) { - $this->setAvailability(false); - throw $e; + } catch (StorageNotAvailableException $e) { + $this->setUnavailable($e); } } @@ -330,9 +319,8 @@ class Availability extends Wrapper { $this->checkAvailability(); try { return parent::getMimeType($path); - } catch (\OCP\Files\StorageNotAvailableException $e) { - $this->setAvailability(false); - throw $e; + } catch (StorageNotAvailableException $e) { + $this->setUnavailable($e); } } @@ -341,9 +329,8 @@ class Availability extends Wrapper { $this->checkAvailability(); try { return parent::hash($type, $path, $raw); - } catch (\OCP\Files\StorageNotAvailableException $e) { - $this->setAvailability(false); - throw $e; + } catch (StorageNotAvailableException $e) { + $this->setUnavailable($e); } } @@ -352,9 +339,8 @@ class Availability extends Wrapper { $this->checkAvailability(); try { return parent::free_space($path); - } catch (\OCP\Files\StorageNotAvailableException $e) { - $this->setAvailability(false); - throw $e; + } catch (StorageNotAvailableException $e) { + $this->setUnavailable($e); } } @@ -363,9 +349,8 @@ class Availability extends Wrapper { $this->checkAvailability(); try { return parent::search($query); - } catch (\OCP\Files\StorageNotAvailableException $e) { - $this->setAvailability(false); - throw $e; + } catch (StorageNotAvailableException $e) { + $this->setUnavailable($e); } } @@ -374,9 +359,8 @@ class Availability extends Wrapper { $this->checkAvailability(); try { return parent::touch($path, $mtime); - } catch (\OCP\Files\StorageNotAvailableException $e) { - $this->setAvailability(false); - throw $e; + } catch (StorageNotAvailableException $e) { + $this->setUnavailable($e); } } @@ -385,9 +369,8 @@ class Availability extends Wrapper { $this->checkAvailability(); try { return parent::getLocalFile($path); - } catch (\OCP\Files\StorageNotAvailableException $e) { - $this->setAvailability(false); - throw $e; + } catch (StorageNotAvailableException $e) { + $this->setUnavailable($e); } } @@ -396,9 +379,8 @@ class Availability extends Wrapper { $this->checkAvailability(); try { return parent::hasUpdated($path, $time); - } catch (\OCP\Files\StorageNotAvailableException $e) { - $this->setAvailability(false); - throw $e; + } catch (StorageNotAvailableException $e) { + $this->setUnavailable($e); } } @@ -406,9 +388,8 @@ class Availability extends Wrapper { public function getOwner($path) { try { return parent::getOwner($path); - } catch (\OCP\Files\StorageNotAvailableException $e) { - $this->setAvailability(false); - throw $e; + } catch (StorageNotAvailableException $e) { + $this->setUnavailable($e); } } @@ -417,9 +398,8 @@ class Availability extends Wrapper { $this->checkAvailability(); try { return parent::getETag($path); - } catch (\OCP\Files\StorageNotAvailableException $e) { - $this->setAvailability(false); - throw $e; + } catch (StorageNotAvailableException $e) { + $this->setUnavailable($e); } } @@ -428,9 +408,8 @@ class Availability extends Wrapper { $this->checkAvailability(); try { return parent::getDirectDownload($path); - } catch (\OCP\Files\StorageNotAvailableException $e) { - $this->setAvailability(false); - throw $e; + } catch (StorageNotAvailableException $e) { + $this->setUnavailable($e); } } @@ -439,9 +418,8 @@ class Availability extends Wrapper { $this->checkAvailability(); try { return parent::copyFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath); - } catch (\OCP\Files\StorageNotAvailableException $e) { - $this->setAvailability(false); - throw $e; + } catch (StorageNotAvailableException $e) { + $this->setUnavailable($e); } } @@ -450,9 +428,8 @@ class Availability extends Wrapper { $this->checkAvailability(); try { return parent::moveFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath); - } catch (\OCP\Files\StorageNotAvailableException $e) { - $this->setAvailability(false); - throw $e; + } catch (StorageNotAvailableException $e) { + $this->setUnavailable($e); } } @@ -461,9 +438,24 @@ class Availability extends Wrapper { $this->checkAvailability(); try { return parent::getMetaData($path); - } catch (\OCP\Files\StorageNotAvailableException $e) { - $this->setAvailability(false); - throw $e; + } catch (StorageNotAvailableException $e) { + $this->setUnavailable($e); } } + + /** + * @throws StorageNotAvailableException + */ + protected function setUnavailable(StorageNotAvailableException $e) { + $delay = self::RECHECK_TTL_SEC; + if($e instanceof StorageAuthException) { + $delay = max( + // 30min + $this->config->getSystemValueInt('external_storage.auth_availability_delay', 1800), + self::RECHECK_TTL_SEC + ); + } + $this->getStorageCache()->setAvailability(false, $delay); + throw $e; + } }