diff options
31 files changed, 448 insertions, 163 deletions
diff --git a/3rdparty b/3rdparty -Subproject 3558cc253049d15a89760af2ecfa8bcfac4f480 +Subproject 9e360791930a82b6770ba72ab0456f5ba27e543 diff --git a/apps/dav/lib/Connector/Sabre/ServerFactory.php b/apps/dav/lib/Connector/Sabre/ServerFactory.php index 4235a02831a..617ba60f80d 100644 --- a/apps/dav/lib/Connector/Sabre/ServerFactory.php +++ b/apps/dav/lib/Connector/Sabre/ServerFactory.php @@ -28,39 +28,19 @@ use Psr\Log\LoggerInterface; use Sabre\DAV\Auth\Plugin; class ServerFactory { - private IConfig $config; - private LoggerInterface $logger; - private IDBConnection $databaseConnection; - private IUserSession $userSession; - private IMountManager $mountManager; - private ITagManager $tagManager; - private IRequest $request; - private IPreview $previewManager; - private IEventDispatcher $eventDispatcher; - private IL10N $l10n; public function __construct( - IConfig $config, - LoggerInterface $logger, - IDBConnection $databaseConnection, - IUserSession $userSession, - IMountManager $mountManager, - ITagManager $tagManager, - IRequest $request, - IPreview $previewManager, - IEventDispatcher $eventDispatcher, - IL10N $l10n, + private IConfig $config, + private LoggerInterface $logger, + private IDBConnection $databaseConnection, + private IUserSession $userSession, + private IMountManager $mountManager, + private ITagManager $tagManager, + private IRequest $request, + private IPreview $previewManager, + private IEventDispatcher $eventDispatcher, + private IL10N $l10n, ) { - $this->config = $config; - $this->logger = $logger; - $this->databaseConnection = $databaseConnection; - $this->userSession = $userSession; - $this->mountManager = $mountManager; - $this->tagManager = $tagManager; - $this->request = $request; - $this->previewManager = $previewManager; - $this->eventDispatcher = $eventDispatcher; - $this->l10n = $l10n; } /** @@ -80,7 +60,7 @@ class ServerFactory { // Load plugins $server->addPlugin(new \OCA\DAV\Connector\Sabre\MaintenancePlugin($this->config, $this->l10n)); $server->addPlugin(new BlockLegacyClientPlugin( - \OCP\Server::get(IConfig::class), + $this->config, \OCP\Server::get(ThemingDefaults::class), )); $server->addPlugin(new \OCA\DAV\Connector\Sabre\AnonymousOptionsPlugin()); @@ -90,11 +70,12 @@ class ServerFactory { $server->addPlugin(new \OCA\DAV\Connector\Sabre\ExceptionLoggerPlugin('webdav', $this->logger)); $server->addPlugin(new \OCA\DAV\Connector\Sabre\LockPlugin()); - $server->addPlugin(new RequestIdHeaderPlugin(\OC::$server->get(IRequest::class))); + $server->addPlugin(new RequestIdHeaderPlugin($this->request)); $server->addPlugin(new ZipFolderPlugin( $objectTree, $this->logger, + $this->eventDispatcher, )); // Some WebDAV clients do require Class 2 WebDAV support (locking), since diff --git a/apps/dav/lib/Connector/Sabre/ZipFolderPlugin.php b/apps/dav/lib/Connector/Sabre/ZipFolderPlugin.php index a5820f80538..8484dfa1935 100644 --- a/apps/dav/lib/Connector/Sabre/ZipFolderPlugin.php +++ b/apps/dav/lib/Connector/Sabre/ZipFolderPlugin.php @@ -9,6 +9,9 @@ declare(strict_types=1); namespace OCA\DAV\Connector\Sabre; use OC\Streamer; +use OCA\DAV\Connector\Sabre\Exception\Forbidden; +use OCP\EventDispatcher\IEventDispatcher; +use OCP\Files\Events\BeforeZipCreatedEvent; use OCP\Files\File as NcFile; use OCP\Files\Folder as NcFolder; use OCP\Files\Node as NcNode; @@ -37,6 +40,7 @@ class ZipFolderPlugin extends ServerPlugin { public function __construct( private Tree $tree, private LoggerInterface $logger, + private IEventDispatcher $eventDispatcher, ) { } @@ -114,17 +118,33 @@ class ZipFolderPlugin extends ServerPlugin { if ($filesParam !== '') { $files = json_decode($filesParam); if (!is_array($files)) { - if (!is_string($files)) { + $files = [$files]; + } + + foreach ($files as $file) { + if (!is_string($file)) { + // we log this as this means either we - or an app - have a bug somewhere or a user is trying invalid things + $this->logger->notice('Invalid files filter parameter for ZipFolderPlugin', ['filter' => $filesParam]); // no valid parameter so continue with Sabre behavior - $this->logger->debug('Invalid files filter parameter for ZipFolderPlugin', ['filter' => $filesParam]); return null; } - - $files = [$files]; } } $folder = $node->getNode(); + $event = new BeforeZipCreatedEvent($folder, $files); + $this->eventDispatcher->dispatchTyped($event); + if ((!$event->isSuccessful()) || $event->getErrorMessage() !== null) { + $errorMessage = $event->getErrorMessage(); + if ($errorMessage === null) { + // Not allowed to download but also no explaining error + // so we abort the ZIP creation and fall back to Sabre default behavior. + return null; + } + // Downloading was denied by an app + throw new Forbidden($errorMessage); + } + $content = empty($files) ? $folder->getDirectoryListing() : []; foreach ($files as $path) { $child = $node->getChild($path); diff --git a/apps/dav/lib/Server.php b/apps/dav/lib/Server.php index 69f05395cb5..4bfc8019218 100644 --- a/apps/dav/lib/Server.php +++ b/apps/dav/lib/Server.php @@ -86,9 +86,9 @@ class Server { $this->request = $request; $this->baseUri = $baseUri; - $logger = \OC::$server->get(LoggerInterface::class); - /** @var IEventDispatcher $dispatcher */ - $dispatcher = \OC::$server->get(IEventDispatcher::class); + + $logger = \OCP\Server::get(LoggerInterface::class); + $eventDispatcher = \OCP\Server::get(IEventDispatcher::class); $root = new RootCollection(); $this->server = new \OCA\DAV\Connector\Sabre\Server(new CachingTree($root)); @@ -123,10 +123,10 @@ class Server { // allow setup of additional auth backends $event = new SabrePluginEvent($this->server); - $dispatcher->dispatch('OCA\DAV\Connector\Sabre::authInit', $event); + $eventDispatcher->dispatch('OCA\DAV\Connector\Sabre::authInit', $event); $newAuthEvent = new SabrePluginAuthInitEvent($this->server); - $dispatcher->dispatchTyped($newAuthEvent); + $eventDispatcher->dispatchTyped($newAuthEvent); $bearerAuthBackend = new BearerAuth( \OC::$server->getUserSession(), @@ -213,12 +213,13 @@ class Server { $this->server->addPlugin(new ZipFolderPlugin( $this->server->tree, $logger, + $eventDispatcher, )); // allow setup of additional plugins - $dispatcher->dispatch('OCA\DAV\Connector\Sabre::addPlugin', $event); + $eventDispatcher->dispatch('OCA\DAV\Connector\Sabre::addPlugin', $event); $typedEvent = new SabrePluginAddEvent($this->server); - $dispatcher->dispatchTyped($typedEvent); + $eventDispatcher->dispatchTyped($typedEvent); // Some WebDAV clients do require Class 2 WebDAV support (locking), since // we do not provide locking we emulate it using a fake locking plugin. diff --git a/apps/encryption/tests/Crypto/CryptTest.php b/apps/encryption/tests/Crypto/CryptTest.php index a9869af99d9..9b6c8227083 100644 --- a/apps/encryption/tests/Crypto/CryptTest.php +++ b/apps/encryption/tests/Crypto/CryptTest.php @@ -37,8 +37,7 @@ class CryptTest extends TestCase { ->disableOriginalConstructor() ->getMock(); $this->logger->expects($this->any()) - ->method('warning') - ->willReturn(true); + ->method('warning'); $this->userSession = $this->getMockBuilder(IUserSession::class) ->disableOriginalConstructor() ->getMock(); diff --git a/apps/files_external/lib/Command/Config.php b/apps/files_external/lib/Command/Config.php index f677e51d604..0736749f6d9 100644 --- a/apps/files_external/lib/Command/Config.php +++ b/apps/files_external/lib/Command/Config.php @@ -73,7 +73,7 @@ class Config extends Base { if (!is_string($value) && json_decode(json_encode($value)) === $value) { // show bools and objects correctly $value = json_encode($value); } - $output->writeln($value); + $output->writeln((string)$value); } /** diff --git a/apps/files_external/lib/Command/Delete.php b/apps/files_external/lib/Command/Delete.php index 61e974ff589..3e6ccf751a8 100644 --- a/apps/files_external/lib/Command/Delete.php +++ b/apps/files_external/lib/Command/Delete.php @@ -13,6 +13,7 @@ use OCA\Files_External\Service\UserStoragesService; use OCP\AppFramework\Http; use OCP\IUserManager; use OCP\IUserSession; +use Symfony\Component\Console\Helper\QuestionHelper; use Symfony\Component\Console\Input\ArrayInput; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; @@ -26,6 +27,7 @@ class Delete extends Base { protected UserStoragesService $userService, protected IUserSession $userSession, protected IUserManager $userManager, + protected QuestionHelper $questionHelper, ) { parent::__construct(); } @@ -64,6 +66,7 @@ class Delete extends Base { $listInput->setOption('output', $input->getOption('output')); $listCommand->listMounts(null, [$mount], $listInput, $output); + /** @var QuestionHelper $questionHelper */ $questionHelper = $this->getHelper('question'); $question = new ConfirmationQuestion('Delete this mount? [y/N] ', false); diff --git a/apps/files_external/lib/Command/Option.php b/apps/files_external/lib/Command/Option.php index 6b679f1d6f7..6051c9c5fbd 100644 --- a/apps/files_external/lib/Command/Option.php +++ b/apps/files_external/lib/Command/Option.php @@ -38,7 +38,7 @@ class Option extends Config { if (!is_string($value)) { // show bools and objects correctly $value = json_encode($value); } - $output->writeln($value); + $output->writeln((string)$value); } /** diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index a143c6188a1..16fe161a9ba 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -44,11 +44,6 @@ <code><![CDATA[array]]></code> </LessSpecificImplementedReturnType> </file> - <file src="3rdparty/symfony/routing/Route.php"> - <MethodSignatureMustProvideReturnType> - <code><![CDATA[unserialize]]></code> - </MethodSignatureMustProvideReturnType> - </file> <file src="apps/dav/appinfo/v1/caldav.php"> <UndefinedGlobalVariable> <code><![CDATA[$baseuri]]></code> diff --git a/core/Command/Background/Delete.php b/core/Command/Background/Delete.php index 41efaf84665..50ae309065b 100644 --- a/core/Command/Background/Delete.php +++ b/core/Command/Background/Delete.php @@ -10,6 +10,7 @@ namespace OC\Core\Command\Background; use OC\Core\Command\Base; use OCP\BackgroundJob\IJobList; +use Symfony\Component\Console\Helper\QuestionHelper; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; @@ -51,6 +52,7 @@ class Delete extends Base { '/^(y|Y)/i' ); + /** @var QuestionHelper $helper */ $helper = $this->getHelper('question'); if (!$helper->ask($input, $output, $question)) { $output->writeln('aborted.'); diff --git a/core/Command/Base.php b/core/Command/Base.php index 0af5981e942..b915ae2ae4a 100644 --- a/core/Command/Base.php +++ b/core/Command/Base.php @@ -149,7 +149,7 @@ class Base extends Command implements CompletionAwareInterface { $this->interrupted = true; } - public function run(InputInterface $input, OutputInterface $output) { + public function run(InputInterface $input, OutputInterface $output): int { // check if the php pcntl_signal functions are accessible $this->php_pcntl_signal = function_exists('pcntl_signal'); if ($this->php_pcntl_signal) { diff --git a/core/Command/Config/App/SetConfig.php b/core/Command/Config/App/SetConfig.php index 461c024d038..b1d1632ff1a 100644 --- a/core/Command/Config/App/SetConfig.php +++ b/core/Command/Config/App/SetConfig.php @@ -12,6 +12,7 @@ use OC\AppConfig; use OCP\Exceptions\AppConfigIncorrectTypeException; use OCP\Exceptions\AppConfigUnknownKeyException; use OCP\IAppConfig; +use Symfony\Component\Console\Helper\QuestionHelper; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; @@ -229,6 +230,7 @@ class SetConfig extends Base { } private function ask(InputInterface $input, OutputInterface $output, string $request): bool { + /** @var QuestionHelper $helper */ $helper = $this->getHelper('question'); if ($input->getOption('no-interaction')) { return true; diff --git a/core/Command/Db/ConvertFilecacheBigInt.php b/core/Command/Db/ConvertFilecacheBigInt.php index d16e6d30231..f5028aacaef 100644 --- a/core/Command/Db/ConvertFilecacheBigInt.php +++ b/core/Command/Db/ConvertFilecacheBigInt.php @@ -11,6 +11,7 @@ use OC\DB\SchemaWrapper; use OCP\DB\Types; use OCP\IDBConnection; use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Helper\QuestionHelper; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Question\ConfirmationQuestion; @@ -89,6 +90,7 @@ class ConvertFilecacheBigInt extends Command { $output->writeln('<comment>This can take up to hours, depending on the number of files in your instance!</comment>'); if ($input->isInteractive()) { + /** @var QuestionHelper $helper */ $helper = $this->getHelper('question'); $question = new ConfirmationQuestion('Continue with the conversion (y/n)? [n] ', false); diff --git a/core/Command/Db/Migrations/GenerateCommand.php b/core/Command/Db/Migrations/GenerateCommand.php index cd92dc5acd6..ed29412f00b 100644 --- a/core/Command/Db/Migrations/GenerateCommand.php +++ b/core/Command/Db/Migrations/GenerateCommand.php @@ -15,6 +15,7 @@ use Stecman\Component\Symfony\Console\BashCompletion\Completion\CompletionAwareI use Stecman\Component\Symfony\Console\BashCompletion\CompletionContext; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Exception\RuntimeException; +use Symfony\Component\Console\Helper\QuestionHelper; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; @@ -117,6 +118,7 @@ class {{classname}} extends SimpleMigrationStep { $output->writeln('<comment> - Actual: ' . $version . '</comment>'); if ($input->isInteractive()) { + /** @var QuestionHelper $helper */ $helper = $this->getHelper('question'); $question = new ConfirmationQuestion('Continue with your given version? (y/n) [n] ', false); diff --git a/core/Command/Maintenance/RepairShareOwnership.php b/core/Command/Maintenance/RepairShareOwnership.php index a24be53b00e..16675545afe 100644 --- a/core/Command/Maintenance/RepairShareOwnership.php +++ b/core/Command/Maintenance/RepairShareOwnership.php @@ -14,6 +14,7 @@ use OCP\IDBConnection; use OCP\IUser; use OCP\IUserManager; use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Helper\QuestionHelper; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; @@ -60,6 +61,7 @@ class RepairShareOwnership extends Command { $output->writeln(''); if (!$noConfirm) { + /** @var QuestionHelper $helper */ $helper = $this->getHelper('question'); $question = new ConfirmationQuestion('Repair these shares? [y/N]', false); diff --git a/core/Command/Preview/Repair.php b/core/Command/Preview/Repair.php index 641e204e050..3ccd8231300 100644 --- a/core/Command/Preview/Repair.php +++ b/core/Command/Preview/Repair.php @@ -19,6 +19,7 @@ use OCP\Lock\LockedException; use Psr\Log\LoggerInterface; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Helper\ProgressBar; +use Symfony\Component\Console\Helper\QuestionHelper; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; @@ -133,6 +134,7 @@ class Repair extends Command { if ($input->getOption('batch')) { $output->writeln('Batch mode active: migration is started right away.'); } else { + /** @var QuestionHelper $helper */ $helper = $this->getHelper('question'); $question = new ConfirmationQuestion('<info>Should the migration be started? (y/[n]) </info>', false); diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 603e0a1455c..35416cc1303 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1797,6 +1797,7 @@ return array( 'OC\\RepairException' => $baseDir . '/lib/private/RepairException.php', 'OC\\Repair\\AddAppConfigLazyMigration' => $baseDir . '/lib/private/Repair/AddAppConfigLazyMigration.php', 'OC\\Repair\\AddBruteForceCleanupJob' => $baseDir . '/lib/private/Repair/AddBruteForceCleanupJob.php', + 'OC\\Repair\\AddCleanupDeletedUsersBackgroundJob' => $baseDir . '/lib/private/Repair/AddCleanupDeletedUsersBackgroundJob.php', 'OC\\Repair\\AddCleanupUpdaterBackupsJob' => $baseDir . '/lib/private/Repair/AddCleanupUpdaterBackupsJob.php', 'OC\\Repair\\AddMetadataGenerationJob' => $baseDir . '/lib/private/Repair/AddMetadataGenerationJob.php', 'OC\\Repair\\AddRemoveOldTasksBackgroundJob' => $baseDir . '/lib/private/Repair/AddRemoveOldTasksBackgroundJob.php', @@ -1996,6 +1997,7 @@ return array( 'OC\\UserStatus\\Manager' => $baseDir . '/lib/private/UserStatus/Manager.php', 'OC\\User\\AvailabilityCoordinator' => $baseDir . '/lib/private/User/AvailabilityCoordinator.php', 'OC\\User\\Backend' => $baseDir . '/lib/private/User/Backend.php', + 'OC\\User\\BackgroundJobs\\CleanupDeletedUsers' => $baseDir . '/lib/private/User/BackgroundJobs/CleanupDeletedUsers.php', 'OC\\User\\Database' => $baseDir . '/lib/private/User/Database.php', 'OC\\User\\DisplayNameCache' => $baseDir . '/lib/private/User/DisplayNameCache.php', 'OC\\User\\LazyUser' => $baseDir . '/lib/private/User/LazyUser.php', @@ -2005,6 +2007,7 @@ return array( 'OC\\User\\Manager' => $baseDir . '/lib/private/User/Manager.php', 'OC\\User\\NoUserException' => $baseDir . '/lib/private/User/NoUserException.php', 'OC\\User\\OutOfOfficeData' => $baseDir . '/lib/private/User/OutOfOfficeData.php', + 'OC\\User\\PartiallyDeletedUsersBackend' => $baseDir . '/lib/private/User/PartiallyDeletedUsersBackend.php', 'OC\\User\\Session' => $baseDir . '/lib/private/User/Session.php', 'OC\\User\\User' => $baseDir . '/lib/private/User/User.php', 'OC_App' => $baseDir . '/lib/private/legacy/OC_App.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 17bbd9738dd..71eb43cddf8 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1830,6 +1830,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\RepairException' => __DIR__ . '/../../..' . '/lib/private/RepairException.php', 'OC\\Repair\\AddAppConfigLazyMigration' => __DIR__ . '/../../..' . '/lib/private/Repair/AddAppConfigLazyMigration.php', 'OC\\Repair\\AddBruteForceCleanupJob' => __DIR__ . '/../../..' . '/lib/private/Repair/AddBruteForceCleanupJob.php', + 'OC\\Repair\\AddCleanupDeletedUsersBackgroundJob' => __DIR__ . '/../../..' . '/lib/private/Repair/AddCleanupDeletedUsersBackgroundJob.php', 'OC\\Repair\\AddCleanupUpdaterBackupsJob' => __DIR__ . '/../../..' . '/lib/private/Repair/AddCleanupUpdaterBackupsJob.php', 'OC\\Repair\\AddMetadataGenerationJob' => __DIR__ . '/../../..' . '/lib/private/Repair/AddMetadataGenerationJob.php', 'OC\\Repair\\AddRemoveOldTasksBackgroundJob' => __DIR__ . '/../../..' . '/lib/private/Repair/AddRemoveOldTasksBackgroundJob.php', @@ -2029,6 +2030,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\UserStatus\\Manager' => __DIR__ . '/../../..' . '/lib/private/UserStatus/Manager.php', 'OC\\User\\AvailabilityCoordinator' => __DIR__ . '/../../..' . '/lib/private/User/AvailabilityCoordinator.php', 'OC\\User\\Backend' => __DIR__ . '/../../..' . '/lib/private/User/Backend.php', + 'OC\\User\\BackgroundJobs\\CleanupDeletedUsers' => __DIR__ . '/../../..' . '/lib/private/User/BackgroundJobs/CleanupDeletedUsers.php', 'OC\\User\\Database' => __DIR__ . '/../../..' . '/lib/private/User/Database.php', 'OC\\User\\DisplayNameCache' => __DIR__ . '/../../..' . '/lib/private/User/DisplayNameCache.php', 'OC\\User\\LazyUser' => __DIR__ . '/../../..' . '/lib/private/User/LazyUser.php', @@ -2038,6 +2040,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\User\\Manager' => __DIR__ . '/../../..' . '/lib/private/User/Manager.php', 'OC\\User\\NoUserException' => __DIR__ . '/../../..' . '/lib/private/User/NoUserException.php', 'OC\\User\\OutOfOfficeData' => __DIR__ . '/../../..' . '/lib/private/User/OutOfOfficeData.php', + 'OC\\User\\PartiallyDeletedUsersBackend' => __DIR__ . '/../../..' . '/lib/private/User/PartiallyDeletedUsersBackend.php', 'OC\\User\\Session' => __DIR__ . '/../../..' . '/lib/private/User/Session.php', 'OC\\User\\User' => __DIR__ . '/../../..' . '/lib/private/User/User.php', 'OC_App' => __DIR__ . '/../../..' . '/lib/private/legacy/OC_App.php', diff --git a/lib/private/Authentication/Listeners/UserDeletedFilesCleanupListener.php b/lib/private/Authentication/Listeners/UserDeletedFilesCleanupListener.php index 8523fb6abc7..a619021d192 100644 --- a/lib/private/Authentication/Listeners/UserDeletedFilesCleanupListener.php +++ b/lib/private/Authentication/Listeners/UserDeletedFilesCleanupListener.php @@ -16,27 +16,31 @@ use OCP\Files\Config\IMountProviderCollection; use OCP\Files\Storage\IStorage; use OCP\User\Events\BeforeUserDeletedEvent; use OCP\User\Events\UserDeletedEvent; +use Psr\Log\LoggerInterface; /** @template-implements IEventListener<BeforeUserDeletedEvent|UserDeletedEvent> */ class UserDeletedFilesCleanupListener implements IEventListener { /** @var array<string,IStorage> */ private $homeStorageCache = []; - /** @var IMountProviderCollection */ - private $mountProviderCollection; - - public function __construct(IMountProviderCollection $mountProviderCollection) { - $this->mountProviderCollection = $mountProviderCollection; + public function __construct( + private IMountProviderCollection $mountProviderCollection, + private LoggerInterface $logger, + ) { } public function handle(Event $event): void { + $user = $event->getUser(); + // since we can't reliably get the user home storage after the user is deleted // but the user deletion might get canceled during the before event // we only cache the user home storage during the before event and then do the // action deletion during the after event if ($event instanceof BeforeUserDeletedEvent) { - $userHome = $this->mountProviderCollection->getHomeMountForUser($event->getUser()); + $this->logger->debug('Prepare deleting storage for user {userId}', ['userId' => $user->getUID()]); + + $userHome = $this->mountProviderCollection->getHomeMountForUser($user); $storage = $userHome->getStorage(); if (!$storage) { throw new \Exception('Account has no home storage'); @@ -51,12 +55,14 @@ class UserDeletedFilesCleanupListener implements IEventListener { $this->homeStorageCache[$event->getUser()->getUID()] = $storage; } if ($event instanceof UserDeletedEvent) { - if (!isset($this->homeStorageCache[$event->getUser()->getUID()])) { + if (!isset($this->homeStorageCache[$user->getUID()])) { throw new \Exception('UserDeletedEvent fired without matching BeforeUserDeletedEvent'); } - $storage = $this->homeStorageCache[$event->getUser()->getUID()]; + $storage = $this->homeStorageCache[$user->getUID()]; $cache = $storage->getCache(); $storage->rmdir(''); + $this->logger->debug('Deleted storage for user {userId}', ['userId' => $user->getUID()]); + if ($cache instanceof Cache) { $cache->clear(); } else { diff --git a/lib/private/Console/TimestampFormatter.php b/lib/private/Console/TimestampFormatter.php index da1b7ba48dd..e8d9eef6b16 100644 --- a/lib/private/Console/TimestampFormatter.php +++ b/lib/private/Console/TimestampFormatter.php @@ -32,7 +32,7 @@ class TimestampFormatter implements OutputFormatterInterface { * * @param bool $decorated Whether to decorate the messages or not */ - public function setDecorated($decorated) { + public function setDecorated(bool $decorated) { $this->formatter->setDecorated($decorated); } @@ -41,7 +41,7 @@ class TimestampFormatter implements OutputFormatterInterface { * * @return bool true if the output will decorate messages, false otherwise */ - public function isDecorated() { + public function isDecorated(): bool { return $this->formatter->isDecorated(); } @@ -51,7 +51,7 @@ class TimestampFormatter implements OutputFormatterInterface { * @param string $name The style name * @param OutputFormatterStyleInterface $style The style instance */ - public function setStyle($name, OutputFormatterStyleInterface $style) { + public function setStyle(string $name, OutputFormatterStyleInterface $style) { $this->formatter->setStyle($name, $style); } @@ -61,7 +61,7 @@ class TimestampFormatter implements OutputFormatterInterface { * @param string $name * @return bool */ - public function hasStyle($name) { + public function hasStyle(string $name): bool { return $this->formatter->hasStyle($name); } @@ -72,7 +72,7 @@ class TimestampFormatter implements OutputFormatterInterface { * @return OutputFormatterStyleInterface * @throws \InvalidArgumentException When style isn't defined */ - public function getStyle($name) { + public function getStyle(string $name): OutputFormatterStyleInterface { return $this->formatter->getStyle($name); } diff --git a/lib/private/Repair.php b/lib/private/Repair.php index 42d3cfb4910..b1a824ba5e3 100644 --- a/lib/private/Repair.php +++ b/lib/private/Repair.php @@ -11,6 +11,7 @@ use OC\DB\Connection; use OC\DB\ConnectionAdapter; use OC\Repair\AddAppConfigLazyMigration; use OC\Repair\AddBruteForceCleanupJob; +use OC\Repair\AddCleanupDeletedUsersBackgroundJob; use OC\Repair\AddCleanupUpdaterBackupsJob; use OC\Repair\AddMetadataGenerationJob; use OC\Repair\AddRemoveOldTasksBackgroundJob; @@ -192,6 +193,7 @@ class Repair implements IOutput { \OCP\Server::get(AddAppConfigLazyMigration::class), \OCP\Server::get(RepairLogoDimension::class), \OCP\Server::get(RemoveLegacyDatadirFile::class), + \OCP\Server::get(AddCleanupDeletedUsersBackgroundJob::class), ]; } diff --git a/lib/private/Repair/AddCleanupDeletedUsersBackgroundJob.php b/lib/private/Repair/AddCleanupDeletedUsersBackgroundJob.php new file mode 100644 index 00000000000..9713d8595e7 --- /dev/null +++ b/lib/private/Repair/AddCleanupDeletedUsersBackgroundJob.php @@ -0,0 +1,30 @@ +<?php + +declare(strict_types=1); + +/** + * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ +namespace OC\Repair; + +use OC\User\BackgroundJobs\CleanupDeletedUsers; +use OCP\BackgroundJob\IJobList; +use OCP\Migration\IOutput; +use OCP\Migration\IRepairStep; + +class AddCleanupDeletedUsersBackgroundJob implements IRepairStep { + private IJobList $jobList; + + public function __construct(IJobList $jobList) { + $this->jobList = $jobList; + } + + public function getName(): string { + return 'Add cleanup-deleted-users background job'; + } + + public function run(IOutput $output) { + $this->jobList->add(CleanupDeletedUsers::class); + } +} diff --git a/lib/private/Setup.php b/lib/private/Setup.php index e64f6806b87..f286c3698cf 100644 --- a/lib/private/Setup.php +++ b/lib/private/Setup.php @@ -17,6 +17,7 @@ use OC\Authentication\Token\TokenCleanupJob; use OC\Log\Rotate; use OC\Preview\BackgroundCleanupJob; use OC\TextProcessing\RemoveOldTasksBackgroundJob; +use OC\User\BackgroundJobs\CleanupDeletedUsers; use OCP\AppFramework\Utility\ITimeFactory; use OCP\BackgroundJob\IJobList; use OCP\Defaults; @@ -415,6 +416,7 @@ class Setup { $jobList->add(Rotate::class); $jobList->add(BackgroundCleanupJob::class); $jobList->add(RemoveOldTasksBackgroundJob::class); + $jobList->add(CleanupDeletedUsers::class); } /** diff --git a/lib/private/User/Backend.php b/lib/private/User/Backend.php index 98b291e5dee..9b6a9a890ef 100644 --- a/lib/private/User/Backend.php +++ b/lib/private/User/Backend.php @@ -107,9 +107,9 @@ abstract class Backend implements UserInterface { /** * get the user's home directory * @param string $uid the username - * @return boolean + * @return string|boolean */ - public function getHome($uid) { + public function getHome(string $uid) { return false; } diff --git a/lib/private/User/BackgroundJobs/CleanupDeletedUsers.php b/lib/private/User/BackgroundJobs/CleanupDeletedUsers.php new file mode 100644 index 00000000000..3c1b73637ac --- /dev/null +++ b/lib/private/User/BackgroundJobs/CleanupDeletedUsers.php @@ -0,0 +1,64 @@ +<?php + +declare(strict_types=1); + +/** + * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ +namespace OC\User\BackgroundJobs; + +use OC\User\Manager; +use OC\User\PartiallyDeletedUsersBackend; +use OC\User\User; +use OCP\AppFramework\Utility\ITimeFactory; +use OCP\BackgroundJob\IJob; +use OCP\BackgroundJob\TimedJob; +use OCP\EventDispatcher\IEventDispatcher; +use OCP\IConfig; +use Psr\Log\LoggerInterface; + +class CleanupDeletedUsers extends TimedJob { + public function __construct( + ITimeFactory $time, + private Manager $userManager, + private IConfig $config, + private LoggerInterface $logger, + ) { + parent::__construct($time); + $this->setTimeSensitivity(IJob::TIME_INSENSITIVE); + $this->setInterval(24 * 3600); + } + + protected function run($argument): void { + $backend = new PartiallyDeletedUsersBackend($this->config); + $users = $backend->getUsers(); + + if (empty($users)) { + $this->logger->debug('No failed deleted users found.'); + return; + } + + foreach ($users as $userId) { + if ($this->userManager->userExists($userId)) { + $this->logger->info('Skipping user {userId}, marked as deleted, as they still exists in user backend.', ['userId' => $userId]); + $backend->unmarkUser($userId); + continue; + } + + try { + $user = new User( + $userId, + $backend, + \OCP\Server::get(IEventDispatcher::class), + $this->userManager, + $this->config, + ); + $user->delete(); + $this->logger->info('Cleaned up deleted user {userId}', ['userId' => $userId]); + } catch (\Throwable $error) { + $this->logger->warning('Could not cleanup deleted user {userId}', ['userId' => $userId, 'exception' => $error]); + } + } + } +} diff --git a/lib/private/User/PartiallyDeletedUsersBackend.php b/lib/private/User/PartiallyDeletedUsersBackend.php new file mode 100644 index 00000000000..298ddaff6c6 --- /dev/null +++ b/lib/private/User/PartiallyDeletedUsersBackend.php @@ -0,0 +1,56 @@ +<?php + +/** + * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ +namespace OC\User; + +use OCP\IConfig; +use OCP\IUserBackend; +use OCP\User\Backend\IGetHomeBackend; + +/** + * This is a "fake" backend for users that were deleted, + * but not properly removed from Nextcloud (e.g. an exception occurred). + * This backend is only needed because some APIs in user-deleted-events require a "real" user with backend. + */ +class PartiallyDeletedUsersBackend extends Backend implements IGetHomeBackend, IUserBackend { + + public function __construct( + private IConfig $config, + ) { + } + + public function deleteUser($uid): bool { + // fake true, deleting failed users is automatically handled by User::delete() + return true; + } + + public function getBackendName(): string { + return 'deleted users'; + } + + public function userExists($uid) { + return $this->config->getUserValue($uid, 'core', 'deleted') === 'true'; + } + + public function getHome(string $uid): string|false { + return $this->config->getUserValue($uid, 'core', 'deleted.home-path') ?: false; + } + + public function getUsers($search = '', $limit = null, $offset = null) { + return $this->config->getUsersForUserValue('core', 'deleted', 'true'); + } + + /** + * Unmark a user as deleted. + * This typically the case if the user deletion failed in the backend but before the backend deleted the user, + * meaning the user still exists so we unmark them as it still can be accessed (and deleted) normally. + */ + public function unmarkUser(string $userId): void { + $this->config->deleteUserValue($userId, 'core', 'deleted'); + $this->config->deleteUserValue($userId, 'core', 'deleted.home-path'); + } + +} diff --git a/lib/private/User/User.php b/lib/private/User/User.php index f2c38825338..6e1085b4fe3 100644 --- a/lib/private/User/User.php +++ b/lib/private/User/User.php @@ -19,6 +19,8 @@ use OCP\Group\Events\BeforeUserRemovedEvent; use OCP\Group\Events\UserRemovedEvent; use OCP\IAvatarManager; use OCP\IConfig; +use OCP\IDBConnection; +use OCP\IGroupManager; use OCP\IImage; use OCP\IURLGenerator; use OCP\IUser; @@ -37,30 +39,27 @@ use OCP\User\Events\UserChangedEvent; use OCP\User\Events\UserDeletedEvent; use OCP\User\GetQuotaEvent; use OCP\UserInterface; +use Psr\Log\LoggerInterface; + use function json_decode; use function json_encode; class User implements IUser { private const CONFIG_KEY_MANAGERS = 'manager'; + private IConfig $config; + private IURLGenerator $urlGenerator; + /** @var IAccountManager */ protected $accountManager; - /** @var string */ - private $uid; /** @var string|null */ private $displayName; - /** @var UserInterface|null */ - private $backend; - - /** @var IEventDispatcher */ - private $dispatcher; - /** @var bool|null */ private $enabled; - /** @var Emitter|Manager */ + /** @var Emitter|Manager|null */ private $emitter; /** @var string */ @@ -69,28 +68,20 @@ class User implements IUser { /** @var int|null */ private $lastLogin; - /** @var \OCP\IConfig */ - private $config; - /** @var IAvatarManager */ private $avatarManager; - /** @var IURLGenerator */ - private $urlGenerator; - - public function __construct(string $uid, ?UserInterface $backend, IEventDispatcher $dispatcher, $emitter = null, ?IConfig $config = null, $urlGenerator = null) { - $this->uid = $uid; - $this->backend = $backend; + public function __construct( + private string $uid, + private ?UserInterface $backend, + private IEventDispatcher $dispatcher, + $emitter = null, + ?IConfig $config = null, + $urlGenerator = null, + ) { $this->emitter = $emitter; - if (is_null($config)) { - $config = \OC::$server->getConfig(); - } - $this->config = $config; - $this->urlGenerator = $urlGenerator; - if (is_null($this->urlGenerator)) { - $this->urlGenerator = \OC::$server->getURLGenerator(); - } - $this->dispatcher = $dispatcher; + $this->config = $config ?? \OCP\Server::get(IConfig::class); + $this->urlGenerator = $urlGenerator ?? \OCP\Server::get(IURLGenerator::class); } /** @@ -244,50 +235,85 @@ class User implements IUser { * @return bool */ public function delete() { + if ($this->backend === null) { + \OCP\Server::get(LoggerInterface::class)->error('Cannot delete user: No backend set'); + return false; + } + if ($this->emitter) { /** @deprecated 21.0.0 use BeforeUserDeletedEvent event with the IEventDispatcher instead */ $this->emitter->emit('\OC\User', 'preDelete', [$this]); } $this->dispatcher->dispatchTyped(new BeforeUserDeletedEvent($this)); + + // Set delete flag on the user - this is needed to ensure that the user data is removed if there happen any exception in the backend + // because we can not restore the user meaning we could not rollback to any stable state otherwise. + $this->config->setUserValue($this->uid, 'core', 'deleted', 'true'); + // We also need to backup the home path as this can not be reconstructed later if the original backend uses custom home paths + $this->config->setUserValue($this->uid, 'core', 'deleted.home-path', $this->getHome()); + + // Try to delete the user on the backend $result = $this->backend->deleteUser($this->uid); - if ($result) { - // FIXME: Feels like an hack - suggestions? - - $groupManager = \OC::$server->getGroupManager(); - // We have to delete the user from all groups - foreach ($groupManager->getUserGroupIds($this) as $groupId) { - $group = $groupManager->get($groupId); - if ($group) { - $this->dispatcher->dispatchTyped(new BeforeUserRemovedEvent($group, $this)); - $group->removeUser($this); - $this->dispatcher->dispatchTyped(new UserRemovedEvent($group, $this)); - } + if ($result === false) { + // The deletion was aborted or something else happened, we are in a defined state, so remove the delete flag + $this->config->deleteUserValue($this->uid, 'core', 'deleted'); + return false; + } + + // We have to delete the user from all groups + $groupManager = \OCP\Server::get(IGroupManager::class); + foreach ($groupManager->getUserGroupIds($this) as $groupId) { + $group = $groupManager->get($groupId); + if ($group) { + $this->dispatcher->dispatchTyped(new BeforeUserRemovedEvent($group, $this)); + $group->removeUser($this); + $this->dispatcher->dispatchTyped(new UserRemovedEvent($group, $this)); } - // Delete the user's keys in preferences - \OC::$server->getConfig()->deleteAllUserValues($this->uid); + } - \OC::$server->get(ICommentsManager::class)->deleteReferencesOfActor('users', $this->uid); - \OC::$server->get(ICommentsManager::class)->deleteReadMarksFromUser($this); + $commentsManager = \OCP\Server::get(ICommentsManager::class); + $commentsManager->deleteReferencesOfActor('users', $this->uid); + $commentsManager->deleteReadMarksFromUser($this); - /** @var AvatarManager $avatarManager */ - $avatarManager = \OCP\Server::get(AvatarManager::class); - $avatarManager->deleteUserAvatar($this->uid); + $avatarManager = \OCP\Server::get(AvatarManager::class); + $avatarManager->deleteUserAvatar($this->uid); - $notification = \OC::$server->get(INotificationManager::class)->createNotification(); - $notification->setUser($this->uid); - \OC::$server->get(INotificationManager::class)->markProcessed($notification); + $notificationManager = \OCP\Server::get(INotificationManager::class); + $notification = $notificationManager->createNotification(); + $notification->setUser($this->uid); + $notificationManager->markProcessed($notification); - /** @var AccountManager $accountManager */ - $accountManager = \OCP\Server::get(AccountManager::class); - $accountManager->deleteUser($this); + $accountManager = \OCP\Server::get(AccountManager::class); + $accountManager->deleteUser($this); - if ($this->emitter) { - /** @deprecated 21.0.0 use UserDeletedEvent event with the IEventDispatcher instead */ - $this->emitter->emit('\OC\User', 'postDelete', [$this]); - } - $this->dispatcher->dispatchTyped(new UserDeletedEvent($this)); + $database = \OCP\Server::get(IDBConnection::class); + try { + // We need to create a transaction to make sure we are in a defined state + // because if all user values are removed also the flag is gone, but if an exception happens (e.g. database lost connection on the set operation) + // exactly here we are in an undefined state as the data is still present but the user does not exist on the system anymore. + $database->beginTransaction(); + // Remove all user settings + $this->config->deleteAllUserValues($this->uid); + // But again set flag that this user is about to be deleted + $this->config->setUserValue($this->uid, 'core', 'deleted', 'true'); + $this->config->setUserValue($this->uid, 'core', 'deleted.home-path', $this->getHome()); + // Commit the transaction so we are in a defined state: either the preferences are removed or an exception occurred but the delete flag is still present + $database->commit(); + } catch (\Throwable $e) { + $database->rollback(); + throw $e; } - return !($result === false); + + if ($this->emitter !== null) { + /** @deprecated 21.0.0 use UserDeletedEvent event with the IEventDispatcher instead */ + $this->emitter->emit('\OC\User', 'postDelete', [$this]); + } + $this->dispatcher->dispatchTyped(new UserDeletedEvent($this)); + + // Finally we can unset the delete flag and all other states + $this->config->deleteAllUserValues($this->uid); + + return true; } /** @@ -344,10 +370,8 @@ class User implements IUser { /** @psalm-suppress UndefinedInterfaceMethod Once we get rid of the legacy implementsActions, psalm won't complain anymore */ if (($this->backend instanceof IGetHomeBackend || $this->backend->implementsActions(Backend::GET_HOME)) && $home = $this->backend->getHome($this->uid)) { $this->home = $home; - } elseif ($this->config) { - $this->home = $this->config->getSystemValueString('datadirectory', \OC::$SERVERROOT . '/data') . '/' . $this->uid; } else { - $this->home = \OC::$SERVERROOT . '/data/' . $this->uid; + $this->home = $this->config->getSystemValueString('datadirectory', \OC::$SERVERROOT . '/data') . '/' . $this->uid; } } return $this->home; diff --git a/lib/public/Files/Events/BeforeZipCreatedEvent.php b/lib/public/Files/Events/BeforeZipCreatedEvent.php index b55b36d3968..0363d385d36 100644 --- a/lib/public/Files/Events/BeforeZipCreatedEvent.php +++ b/lib/public/Files/Events/BeforeZipCreatedEvent.php @@ -10,23 +10,45 @@ declare(strict_types=1); namespace OCP\Files\Events; use OCP\EventDispatcher\Event; +use OCP\Files\Folder; /** + * This event is triggered before a archive is created when a user requested + * downloading a folder or multiple files. + * + * By setting `successful` to false the tar creation can be aborted and the download denied. + * * @since 25.0.0 */ class BeforeZipCreatedEvent extends Event { private string $directory; - private array $files; private bool $successful = true; private ?string $errorMessage = null; + private ?Folder $folder = null; /** + * @param list<string> $files * @since 25.0.0 + * @since 31.0.0 support `OCP\Files\Folder` as `$directory` parameter - passing a string is deprecated now */ - public function __construct(string $directory, array $files) { + public function __construct( + string|Folder $directory, + private array $files, + ) { parent::__construct(); - $this->directory = $directory; - $this->files = $files; + if ($directory instanceof Folder) { + $this->directory = $directory->getPath(); + $this->folder = $directory; + } else { + $this->directory = $directory; + } + } + + /** + * @since 31.0.0 + */ + public function getFolder(): ?Folder { + return $this->folder; } /** diff --git a/tests/Core/Command/User/SettingTest.php b/tests/Core/Command/User/SettingTest.php index 6848b0bb350..62b75191d36 100644 --- a/tests/Core/Command/User/SettingTest.php +++ b/tests/Core/Command/User/SettingTest.php @@ -49,15 +49,14 @@ class SettingTest extends TestCase { public function getCommand(array $methods = []) { if (empty($methods)) { - return new Setting($this->userManager, $this->config, $this->connection); + return new Setting($this->userManager, $this->config); } else { - $mock = $this->getMockBuilder('OC\Core\Command\User\Setting') + $mock = $this->getMockBuilder(Setting::class) ->setConstructorArgs([ $this->userManager, $this->config, - $this->connection, ]) - ->setMethods($methods) + ->onlyMethods($methods) ->getMock(); return $mock; } @@ -194,7 +193,16 @@ class SettingTest extends TestCase { ->willReturnMap($options); $this->consoleInput->expects($this->any()) ->method('hasParameterOption') - ->willReturnMap($parameterOptions); + ->willReturnCallback(function (string|array $config, bool $default = false) use ($parameterOptions): bool { + foreach ($parameterOptions as $parameterOption) { + if ($config === $parameterOption[0] + // Check the default value if the maps has 3 entries + && (!isset($parameterOption[2]) || $default === $parameterOption[1])) { + return end($parameterOption); + } + } + return false; + }); if ($user !== false) { $this->userManager->expects($this->once()) @@ -402,15 +410,16 @@ class SettingTest extends TestCase { if ($defaultValue === null) { $this->consoleInput->expects($this->atLeastOnce()) ->method('hasParameterOption') - ->willReturnMap([ - ['--default-value', false], - ]); + ->willReturn(false); } else { $this->consoleInput->expects($this->atLeastOnce()) ->method('hasParameterOption') - ->willReturnMap([ - ['--default-value', false, true], - ]); + ->willReturnCallback(function (string|array $config, bool $default = false): bool { + if ($config === '--default-value' && $default === false) { + return true; + } + return false; + }); $this->consoleInput->expects($this->once()) ->method('getOption') ->with('default-value') diff --git a/tests/lib/Mail/MessageTest.php b/tests/lib/Mail/MessageTest.php index 681ec033380..260d3aaff76 100644 --- a/tests/lib/Mail/MessageTest.php +++ b/tests/lib/Mail/MessageTest.php @@ -91,23 +91,23 @@ class MessageTest extends TestCase { $this->symfonyEmail ->expects($this->once()) ->method('from') - ->willReturn(new Address('pierres-general-store@stardewvalley.com', 'Pierres General Store')); + ->with(new Address('pierres-general-store@stardewvalley.com', 'Pierres General Store')); $this->symfonyEmail ->expects($this->once()) ->method('to') - ->willReturn(new Address('lewis-tent@stardewvalley.com', "Lewis' Tent Life")); + ->with(new Address('lewis-tent@stardewvalley.com', "Lewis' Tent Life")); $this->symfonyEmail ->expects($this->once()) ->method('replyTo') - ->willReturn(new Address('penny@stardewvalley-library.co.edu', 'Penny')); + ->with(new Address('penny@stardewvalley-library.co.edu', 'Penny')); $this->symfonyEmail ->expects($this->once()) ->method('cc') - ->willReturn(new Address('gunther@stardewvalley-library.co.edu', 'Gunther')); + ->with(new Address('gunther@stardewvalley-library.co.edu', 'Gunther')); $this->symfonyEmail ->expects($this->once()) ->method('bcc') - ->willReturn(new Address('pam@stardewvalley-bus.com', 'Pam')); + ->with(new Address('pam@stardewvalley-bus.com', 'Pam')); $this->message->setRecipients(); } diff --git a/tests/lib/User/UserTest.php b/tests/lib/User/UserTest.php index 55230b83667..e79aaadd593 100644 --- a/tests/lib/User/UserTest.php +++ b/tests/lib/User/UserTest.php @@ -513,14 +513,25 @@ class UserTest extends TestCase { $test = $this; /** - * @var Backend | MockObject $backend + * @var UserInterface&MockObject $backend */ $backend = $this->createMock(\Test\Util\User\Dummy::class); $backend->expects($this->once()) ->method('deleteUser') ->willReturn($result); + + $config = $this->createMock(IConfig::class); + $config->method('getSystemValue') + ->willReturnArgument(1); + $config->method('getSystemValueString') + ->willReturnArgument(1); + $config->method('getSystemValueBool') + ->willReturnArgument(1); + $config->method('getSystemValueInt') + ->willReturnArgument(1); + $emitter = new PublicEmitter(); - $user = new User('foo', $backend, $this->dispatcher, $emitter); + $user = new User('foo', $backend, $this->dispatcher, $emitter, $config); /** * @param User $user @@ -533,21 +544,11 @@ class UserTest extends TestCase { $emitter->listen('\OC\User', 'preDelete', $hook); $emitter->listen('\OC\User', 'postDelete', $hook); - $config = $this->createMock(IConfig::class); $commentsManager = $this->createMock(ICommentsManager::class); $notificationManager = $this->createMock(INotificationManager::class); - $config->method('getSystemValue') - ->willReturnArgument(1); - $config->method('getSystemValueString') - ->willReturnArgument(1); - $config->method('getSystemValueBool') - ->willReturnArgument(1); - $config->method('getSystemValueInt') - ->willReturnArgument(1); - if ($result) { - $config->expects($this->once()) + $config->expects($this->atLeastOnce()) ->method('deleteAllUserValues') ->with('foo'); @@ -586,7 +587,6 @@ class UserTest extends TestCase { $this->overwriteService(\OCP\Notification\IManager::class, $notificationManager); $this->overwriteService(\OCP\Comments\ICommentsManager::class, $commentsManager); - $this->overwriteService(AllConfig::class, $config); $this->assertSame($result, $user->delete()); @@ -597,6 +597,59 @@ class UserTest extends TestCase { $this->assertEquals($expectedHooks, $hooksCalled); } + public function testDeleteRecoverState() { + $backend = $this->createMock(\Test\Util\User\Dummy::class); + $backend->expects($this->once()) + ->method('deleteUser') + ->willReturn(true); + + $config = $this->createMock(IConfig::class); + $config->method('getSystemValue') + ->willReturnArgument(1); + $config->method('getSystemValueString') + ->willReturnArgument(1); + $config->method('getSystemValueBool') + ->willReturnArgument(1); + $config->method('getSystemValueInt') + ->willReturnArgument(1); + + $userConfig = []; + $config->expects(self::atLeast(2)) + ->method('setUserValue') + ->willReturnCallback(function () { + $userConfig[] = func_get_args(); + }); + + $commentsManager = $this->createMock(ICommentsManager::class); + $commentsManager->expects($this->once()) + ->method('deleteReferencesOfActor') + ->willThrowException(new \Error('Test exception')); + + $this->overwriteService(\OCP\Comments\ICommentsManager::class, $commentsManager); + $this->expectException(\Error::class); + + $user = $this->getMockBuilder(User::class) + ->onlyMethods(['getHome']) + ->setConstructorArgs(['foo', $backend, $this->dispatcher, null, $config]) + ->getMock(); + + $user->expects(self::atLeastOnce()) + ->method('getHome') + ->willReturn('/home/path'); + + $user->delete(); + + $this->assertEqualsCanonicalizing( + [ + ['foo', 'core', 'deleted', 'true', null], + ['foo', 'core', 'deleted.backup-home', '/home/path', null], + ], + $userConfig, + ); + + $this->restoreService(\OCP\Comments\ICommentsManager::class); + } + public function dataGetCloudId(): array { return [ ['https://localhost:8888/nextcloud', 'foo@localhost:8888/nextcloud'], |