diff --git a/apps/files/composer/composer/autoload_classmap.php b/apps/files/composer/composer/autoload_classmap.php index a005578aa2428..3fd82c774c240 100644 --- a/apps/files/composer/composer/autoload_classmap.php +++ b/apps/files/composer/composer/autoload_classmap.php @@ -108,4 +108,5 @@ 'OCA\\Files\\Sharing\\Property\\NodeGridViewSharePropertyType' => $baseDir . '/../lib/Sharing/Property/NodeGridViewSharePropertyType.php', 'OCA\\Files\\Sharing\\Source\\NodeShareSourceMetadata' => $baseDir . '/../lib/Sharing/Source/NodeShareSourceMetadata.php', 'OCA\\Files\\Sharing\\Source\\NodeShareSourceType' => $baseDir . '/../lib/Sharing/Source/NodeShareSourceType.php', + 'OCA\\Files\\TokenScopes' => $baseDir . '/../lib/TokenScopes.php', ); diff --git a/apps/files/composer/composer/autoload_static.php b/apps/files/composer/composer/autoload_static.php index 6f0f0d4cc602f..5c8eb91c4c2e5 100644 --- a/apps/files/composer/composer/autoload_static.php +++ b/apps/files/composer/composer/autoload_static.php @@ -123,6 +123,7 @@ class ComposerStaticInitFiles 'OCA\\Files\\Sharing\\Property\\NodeGridViewSharePropertyType' => __DIR__ . '/..' . '/../lib/Sharing/Property/NodeGridViewSharePropertyType.php', 'OCA\\Files\\Sharing\\Source\\NodeShareSourceMetadata' => __DIR__ . '/..' . '/../lib/Sharing/Source/NodeShareSourceMetadata.php', 'OCA\\Files\\Sharing\\Source\\NodeShareSourceType' => __DIR__ . '/..' . '/../lib/Sharing/Source/NodeShareSourceType.php', + 'OCA\\Files\\TokenScopes' => __DIR__ . '/..' . '/../lib/TokenScopes.php', ); public static function getInitializer(ClassLoader $loader) diff --git a/apps/files/lib/TokenScopes.php b/apps/files/lib/TokenScopes.php new file mode 100644 index 0000000000000..47b1174d37ba5 --- /dev/null +++ b/apps/files/lib/TokenScopes.php @@ -0,0 +1,18 @@ + $baseDir . '/../lib/SharedMount.php', 'OCA\\Files_Sharing\\SharedStorage' => $baseDir . '/../lib/SharedStorage.php', 'OCA\\Files_Sharing\\SharesReminderJob' => $baseDir . '/../lib/SharesReminderJob.php', + 'OCA\\Files_Sharing\\TokenScopes' => $baseDir . '/../lib/TokenScopes.php', 'OCA\\Files_Sharing\\Updater' => $baseDir . '/../lib/Updater.php', 'OCA\\Files_Sharing\\ViewOnly' => $baseDir . '/../lib/ViewOnly.php', ); diff --git a/apps/files_sharing/composer/composer/autoload_static.php b/apps/files_sharing/composer/composer/autoload_static.php index 0550359c85807..c630ad855bd0e 100644 --- a/apps/files_sharing/composer/composer/autoload_static.php +++ b/apps/files_sharing/composer/composer/autoload_static.php @@ -122,6 +122,7 @@ class ComposerStaticInitFiles_Sharing 'OCA\\Files_Sharing\\SharedMount' => __DIR__ . '/..' . '/../lib/SharedMount.php', 'OCA\\Files_Sharing\\SharedStorage' => __DIR__ . '/..' . '/../lib/SharedStorage.php', 'OCA\\Files_Sharing\\SharesReminderJob' => __DIR__ . '/..' . '/../lib/SharesReminderJob.php', + 'OCA\\Files_Sharing\\TokenScopes' => __DIR__ . '/..' . '/../lib/TokenScopes.php', 'OCA\\Files_Sharing\\Updater' => __DIR__ . '/..' . '/../lib/Updater.php', 'OCA\\Files_Sharing\\ViewOnly' => __DIR__ . '/..' . '/../lib/ViewOnly.php', ); diff --git a/apps/files_sharing/lib/TokenScopes.php b/apps/files_sharing/lib/TokenScopes.php new file mode 100644 index 0000000000000..d56cc459a5dda --- /dev/null +++ b/apps/files_sharing/lib/TokenScopes.php @@ -0,0 +1,17 @@ +l->t('You granted filesystem access to app password "{token}"'); } elseif ($event->getSubject() === self::APP_TOKEN_FILESYSTEM_REVOKED) { $subject = $this->l->t('You revoked filesystem access from app password "{token}"'); + } elseif ($event->getSubject() === self::APP_TOKEN_SCOPES_CHANGED) { + $subject = $this->l->t('You changed the access of app password "{token}"'); } else { throw new UnknownActivityException('Unknown subject'); } @@ -141,6 +144,7 @@ protected function getParameters(IEvent $event): array { case self::APP_TOKEN_DELETED_WIPE_CANCELLED: case self::APP_TOKEN_FILESYSTEM_GRANTED: case self::APP_TOKEN_FILESYSTEM_REVOKED: + case self::APP_TOKEN_SCOPES_CHANGED: return [ 'token' => [ 'type' => 'highlight', diff --git a/apps/settings/lib/Controller/AuthSettingsController.php b/apps/settings/lib/Controller/AuthSettingsController.php index 508ef9b7f5e49..ae2f2e3e2056f 100644 --- a/apps/settings/lib/Controller/AuthSettingsController.php +++ b/apps/settings/lib/Controller/AuthSettingsController.php @@ -14,6 +14,7 @@ use OC\Authentication\Token\INamedToken; use OC\Authentication\Token\IProvider; use OC\Authentication\Token\RemoteWipe; +use OC\Authentication\Token\TokenScopes; use OCA\Settings\Activity\Provider; use OCA\Settings\ConfigLexicon; use OCP\Activity\IManager; @@ -52,6 +53,7 @@ public function __construct( private LoggerInterface $logger, private IConfig $serverConfig, private IL10N $l, + private TokenScopes $tokenScopes, ) { parent::__construct($appName, $request); } @@ -245,10 +247,22 @@ public function update(int $id, array $scope, string $name): JSONResponse { } $currentName = $token->getName(); + $currentScope = $token->getScopeAsArray(); - if ($scope !== $token->getScopeAsArray()) { - $token->setScope([IToken::SCOPE_FILESYSTEM => $scope[IToken::SCOPE_FILESYSTEM]]); - $this->publishActivity($scope[IToken::SCOPE_FILESYSTEM] ? Provider::APP_TOKEN_FILESYSTEM_GRANTED : Provider::APP_TOKEN_FILESYSTEM_REVOKED, $token->getId(), ['name' => $currentName]); + try { + $newScope = $this->tokenScopes->updateScope($currentScope, $scope); + } catch (\InvalidArgumentException) { + return new JSONResponse([], Http::STATUS_BAD_REQUEST); + } + + if ($newScope !== $currentScope) { + $token->setScope($newScope); + if ($this->tokenScopes->fromBlob($newScope) !== $this->tokenScopes->fromBlob($currentScope)) { + $this->publishActivity(Provider::APP_TOKEN_SCOPES_CHANGED, $token->getId(), ['name' => $currentName]); + } elseif ($newScope[IToken::SCOPE_FILESYSTEM] !== ($currentScope[IToken::SCOPE_FILESYSTEM] ?? false)) { + // Only legacy tokens without scopes flip filesystem on its own + $this->publishActivity($newScope[IToken::SCOPE_FILESYSTEM] ? Provider::APP_TOKEN_FILESYSTEM_GRANTED : Provider::APP_TOKEN_FILESYSTEM_REVOKED, $token->getId(), ['name' => $currentName]); + } } if (mb_strlen($name) > 128) { diff --git a/apps/settings/tests/Activity/ProviderTest.php b/apps/settings/tests/Activity/ProviderTest.php index bf13ec5f97572..d564d18d62949 100644 --- a/apps/settings/tests/Activity/ProviderTest.php +++ b/apps/settings/tests/Activity/ProviderTest.php @@ -95,6 +95,25 @@ public function testParseRevokedAllUsesPluralForm(int $count): void { $this->provider->parse('en', $event); } + public function testParseScopesChanged(): void { + $event = $this->createMock(IEvent::class); + $event->method('getApp')->willReturn('settings'); + $event->method('getSubject')->willReturn(Provider::APP_TOKEN_SCOPES_CHANGED); + $event->method('getSubjectParameters')->willReturn(['name' => 'Thunderbird']); + $event->method('getObjectId')->willReturn(42); + + $this->l->expects($this->once()) + ->method('t') + ->with('You changed the access of app password "{token}"') + ->willReturn('parsed subject'); + + $event->expects($this->once()) + ->method('setRichSubject') + ->with('parsed subject', ['token' => ['type' => 'highlight', 'id' => '42', 'name' => 'Thunderbird']]); + + $this->provider->parse('en', $event); + } + public function testParseRevokedAllWithoutCountParameter(): void { $event = $this->createMock(IEvent::class); $event->method('getApp')->willReturn('settings'); diff --git a/apps/settings/tests/Controller/AuthSettingsControllerTest.php b/apps/settings/tests/Controller/AuthSettingsControllerTest.php index 1a6a50a08cc4f..b4b2201e7a78e 100644 --- a/apps/settings/tests/Controller/AuthSettingsControllerTest.php +++ b/apps/settings/tests/Controller/AuthSettingsControllerTest.php @@ -16,10 +16,13 @@ use OC\Authentication\Token\IWipeableToken; use OC\Authentication\Token\PublicKeyToken; use OC\Authentication\Token\RemoteWipe; +use OC\Authentication\Token\TokenScopes; +use OCA\Files\TokenScopes as FilesTokenScopes; use OCA\Settings\Activity\Provider; use OCA\Settings\Controller\AuthSettingsController; use OCP\Activity\IEvent; use OCP\Activity\IManager; +use OCP\AppFramework\Http\Attribute\PasswordConfirmationRequired; use OCP\AppFramework\Http\JSONResponse; use OCP\AppFramework\Services\IAppConfig; use OCP\Authentication\Exceptions\WipeTokenException; @@ -78,6 +81,7 @@ protected function setUp(): void { $logger, $this->serverConfig, $this->l, + new TokenScopes(), ); } @@ -528,6 +532,100 @@ public function testUpdateNoChange(): void { $this->assertSame([], $this->controller->update($tokenId, [IToken::SCOPE_FILESYSTEM => true], 'App password')->getData()); } + public static function dataUpdateScopedToken(): array { + $stored = [IToken::SCOPE_FILESYSTEM => false, TokenScopes::KEY => ['dav:calendar:read']]; + return [ + 'scopes omitted' => [$stored, [IToken::SCOPE_FILESYSTEM => false]], + 'scopes sent back unchanged' => [$stored, $stored], + 'legacy filesystem toggle' => [$stored, [IToken::SCOPE_FILESYSTEM => true, TokenScopes::KEY => ['dav:calendar:read']]], + ]; + } + + #[\PHPUnit\Framework\Attributes\DataProvider(methodName: 'dataUpdateScopedToken')] + public function testUpdateScopedTokenKeepsScopes(array $stored, array $scope): void { + $tokenId = 42; + $token = $this->createMock(PublicKeyToken::class); + + $this->mockGetTokenById($tokenId, $token); + $this->mockActivityManager(); + + $token->method('getUID')->willReturn('jane'); + $token->method('getName')->willReturn('App password'); + $token->method('getScopeAsArray')->willReturn($stored); + + $token->expects($this->never()) + ->method('setScope'); + $token->expects($this->once()) + ->method('setName') + ->with('Renamed'); + + $this->tokenProvider->expects($this->once()) + ->method('updateToken') + ->with($token); + + $this->assertSame([], $this->controller->update($tokenId, $scope, 'Renamed')->getData()); + } + + public static function dataUpdateScopes(): array { + $calendar = [IToken::SCOPE_FILESYSTEM => false, TokenScopes::KEY => ['dav:calendar:read']]; + return [ + 'scopes set, filesystem flips too' => [[IToken::SCOPE_FILESYSTEM => true], [TokenScopes::KEY => ['dav:calendar:read']], $calendar], + 'scopes cleared' => [$calendar, [IToken::SCOPE_FILESYSTEM => false, TokenScopes::KEY => null], [IToken::SCOPE_FILESYSTEM => true]], + ]; + } + + #[\PHPUnit\Framework\Attributes\DataProvider(methodName: 'dataUpdateScopes')] + public function testUpdateScopesPublishOneActivity(array $stored, array $scope, array $expected): void { + $tokenId = 42; + $token = $this->createMock(PublicKeyToken::class); + + $this->mockGetTokenById($tokenId, $token); + $this->mockActivityManager(Provider::APP_TOKEN_SCOPES_CHANGED); + + $token->method('getUID')->willReturn('jane'); + $token->method('getName')->willReturn('App password'); + $token->method('getScopeAsArray')->willReturn($stored); + + $token->expects($this->once()) + ->method('setScope') + ->with($expected); + + $this->tokenProvider->expects($this->once()) + ->method('updateToken') + ->with($token); + + $this->assertSame([], $this->controller->update($tokenId, $scope, 'App password')->getData()); + } + + public function testUpdateInvalidScopes(): void { + $tokenId = 42; + $token = $this->createMock(PublicKeyToken::class); + + $this->mockGetTokenById($tokenId, $token); + + $token->method('getUID')->willReturn('jane'); + $token->method('getName')->willReturn('App password'); + $token->method('getScopeAsArray')->willReturn([IToken::SCOPE_FILESYSTEM => true]); + + $token->expects($this->never()) + ->method('setScope'); + $token->expects($this->never()) + ->method('setName'); + $this->tokenProvider->expects($this->never()) + ->method('updateToken'); + + $response = $this->controller->update($tokenId, [TokenScopes::KEY => [FilesTokenScopes::FILES_READ]], 'Renamed'); + $this->assertSame(Http::STATUS_BAD_REQUEST, $response->getStatus()); + } + + public function testUpdateRequiresStrictPasswordConfirmation(): void { + $attributes = (new \ReflectionMethod(AuthSettingsController::class, 'update')) + ->getAttributes(PasswordConfirmationRequired::class); + + $this->assertCount(1, $attributes); + $this->assertTrue($attributes[0]->newInstance()->getStrict()); + } + public function testUpdateExpired(): void { $tokenId = 42; $token = $this->createMock(PublicKeyToken::class); @@ -589,10 +687,21 @@ private function mockAuthToken(int $id, int $type = IToken::PERMANENT_TOKEN): Pu return $token; } - private function mockActivityManager(): void { + private function mockActivityManager(?string $subject = null): void { + $event = $this->createMock(IEvent::class); + if ($subject !== null) { + foreach (['setApp', 'setType', 'setAffectedUser', 'setAuthor', 'setObject'] as $method) { + $event->method($method)->willReturnSelf(); + } + $event->expects($this->once()) + ->method('setSubject') + ->with($subject, $this->anything()) + ->willReturnSelf(); + } + $this->activityManager->expects($this->once()) ->method('generateEvent') - ->willReturn($this->createMock(IEvent::class)); + ->willReturn($event); $this->activityManager->expects($this->once()) ->method('publish'); } diff --git a/core/Command/User/AuthTokens/ListCommand.php b/core/Command/User/AuthTokens/ListCommand.php index 73a07b34f51ec..9b022a605b82b 100644 --- a/core/Command/User/AuthTokens/ListCommand.php +++ b/core/Command/User/AuthTokens/ListCommand.php @@ -9,6 +9,7 @@ use OC\Authentication\Token\IProvider; use OC\Authentication\Token\IToken; +use OC\Authentication\Token\TokenScopes; use OC\Core\Command\Base; use OCP\IUserManager; use Symfony\Component\Console\Input\InputArgument; @@ -19,6 +20,7 @@ class ListCommand extends Base { public function __construct( protected IUserManager $userManager, protected IProvider $tokenProvider, + private TokenScopes $tokenScopes, ) { parent::__construct(); } @@ -71,7 +73,14 @@ protected function execute(InputInterface $input, OutputInterface $output): int } public function formatTokenForPlainOutput(array $token): array { - $token['scope'] = implode(', ', array_keys(array_filter($token['scope'] ?? []))); + $scope = $token['scope'] ?? []; + $scopes = $this->tokenScopes->fromBlob($scope); + unset($scope[TokenScopes::KEY]); + $parts = array_keys(array_filter($scope)); + if ($scopes !== null) { + $parts[] = 'scopes: ' . ($scopes === [] ? 'none' : implode(' ', $scopes)); + } + $token['scope'] = implode(', ', $parts); $token['lastActivity'] = date(DATE_ATOM, $token['lastActivity']); diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 3e97fa37854aa..43568b18a9e8d 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1353,6 +1353,7 @@ 'OC\\Authentication\\Token\\PublicKeyTokenProvider' => $baseDir . '/lib/private/Authentication/Token/PublicKeyTokenProvider.php', 'OC\\Authentication\\Token\\RemoteWipe' => $baseDir . '/lib/private/Authentication/Token/RemoteWipe.php', 'OC\\Authentication\\Token\\TokenCleanupJob' => $baseDir . '/lib/private/Authentication/Token/TokenCleanupJob.php', + 'OC\\Authentication\\Token\\TokenScopes' => $baseDir . '/lib/private/Authentication/Token/TokenScopes.php', 'OC\\Authentication\\TwoFactorAuth\\Db\\ProviderUserAssignmentDao' => $baseDir . '/lib/private/Authentication/TwoFactorAuth/Db/ProviderUserAssignmentDao.php', 'OC\\Authentication\\TwoFactorAuth\\EnforcementState' => $baseDir . '/lib/private/Authentication/TwoFactorAuth/EnforcementState.php', 'OC\\Authentication\\TwoFactorAuth\\Manager' => $baseDir . '/lib/private/Authentication/TwoFactorAuth/Manager.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index d728162291c78..4d239a24aae53 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1399,6 +1399,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Authentication\\Token\\PublicKeyTokenProvider' => __DIR__ . '/../../..' . '/lib/private/Authentication/Token/PublicKeyTokenProvider.php', 'OC\\Authentication\\Token\\RemoteWipe' => __DIR__ . '/../../..' . '/lib/private/Authentication/Token/RemoteWipe.php', 'OC\\Authentication\\Token\\TokenCleanupJob' => __DIR__ . '/../../..' . '/lib/private/Authentication/Token/TokenCleanupJob.php', + 'OC\\Authentication\\Token\\TokenScopes' => __DIR__ . '/../../..' . '/lib/private/Authentication/Token/TokenScopes.php', 'OC\\Authentication\\TwoFactorAuth\\Db\\ProviderUserAssignmentDao' => __DIR__ . '/../../..' . '/lib/private/Authentication/TwoFactorAuth/Db/ProviderUserAssignmentDao.php', 'OC\\Authentication\\TwoFactorAuth\\EnforcementState' => __DIR__ . '/../../..' . '/lib/private/Authentication/TwoFactorAuth/EnforcementState.php', 'OC\\Authentication\\TwoFactorAuth\\Manager' => __DIR__ . '/../../..' . '/lib/private/Authentication/TwoFactorAuth/Manager.php', diff --git a/lib/private/Authentication/Token/TokenScopes.php b/lib/private/Authentication/Token/TokenScopes.php new file mode 100644 index 0000000000000..dc363be50e208 --- /dev/null +++ b/lib/private/Authentication/Token/TokenScopes.php @@ -0,0 +1,104 @@ +|null null for full access, an empty list if the stored scopes are malformed + */ + public function fromBlob(array $blob): ?array { + if (!array_key_exists(self::KEY, $blob)) { + return null; + } + $scopes = $blob[self::KEY]; + return is_array($scopes) && $scopes === array_filter($scopes, is_string(...)) ? array_values($scopes) : []; + } + + /** + * Only `filesystem` and `scopes` are taken from `$requested`. Omitted `scopes` stay as stored, + * `null` means full access, and scoped tokens get `filesystem` derived from their scopes. + * + * @throws \InvalidArgumentException + */ + public function updateScope(array $stored, array $requested): array { + $blob = $stored; + + if (array_key_exists(self::KEY, $requested)) { + $requestedScopes = $requested[self::KEY]; + if ($requestedScopes === null) { + unset($blob[self::KEY]); + $blob[IToken::SCOPE_FILESYSTEM] = true; + return $blob; + } + if ($requestedScopes !== ($stored[self::KEY] ?? null)) { + if (!is_array($requestedScopes)) { + throw new \InvalidArgumentException('scopes must be an array'); + } + $blob[self::KEY] = $this->validate($requestedScopes); + } + } + + $scopes = $this->fromBlob($blob); + if ($scopes !== null) { + $blob[IToken::SCOPE_FILESYSTEM] = in_array(FilesTokenScopes::FILES_READ, $scopes, true); + } elseif (array_key_exists(IToken::SCOPE_FILESYSTEM, $requested)) { + if (!is_bool($requested[IToken::SCOPE_FILESYSTEM])) { + throw new \InvalidArgumentException('filesystem must be a boolean'); + } + $blob[IToken::SCOPE_FILESYSTEM] = $requested[IToken::SCOPE_FILESYSTEM]; + } else { + $blob[IToken::SCOPE_FILESYSTEM] ??= true; + } + + return $blob; + } + + /** + * @return list + * @throws \InvalidArgumentException + */ + private function validate(array $scopes): array { + if (count($scopes) > self::MAX_SCOPES) { + throw new \InvalidArgumentException('Too many scopes'); + } + foreach ($scopes as $scope) { + if (!is_string($scope) || $scope === '' || strlen($scope) > self::MAX_SCOPE_LENGTH) { + throw new \InvalidArgumentException('Invalid scope'); + } + } + // Matched by prefix: files_sharing can be disabled, and its classes don't load then + $files = array_filter($scopes, static fn (string $scope) => str_starts_with($scope, 'files:') || str_starts_with($scope, 'files_sharing:')); + // Files endpoints look nodes up by ID or path, so every files scope reads file data + if ($files !== [] && !in_array(FilesTokenScopes::FILES_READ, $files, true)) { + throw new \InvalidArgumentException('Files scopes require ' . FilesTokenScopes::FILES_READ); + } + // Read-only files need a permissions mask on the mounts, which does not exist yet + if ($files !== [] && !in_array(FilesTokenScopes::FILES_WRITE, $files, true)) { + throw new \InvalidArgumentException(FilesTokenScopes::FILES_READ . ' requires ' . FilesTokenScopes::FILES_WRITE); + } + + $scopes = array_unique($scopes); + sort($scopes); + return $scopes; + } +} diff --git a/lib/private/Lockdown/LockdownManager.php b/lib/private/Lockdown/LockdownManager.php index d5315c8963385..ff4320f951f32 100644 --- a/lib/private/Lockdown/LockdownManager.php +++ b/lib/private/Lockdown/LockdownManager.php @@ -7,6 +7,8 @@ namespace OC\Lockdown; +use OC\Authentication\Token\TokenScopes; +use OCA\Files\TokenScopes as FilesTokenScopes; use OCP\Authentication\Token\IToken; use OCP\ISession; use OCP\Lockdown\ILockdownManager; @@ -25,7 +27,10 @@ class LockdownManager implements ILockdownManager { * * @param callable $sessionCallback we need to inject the session lazily to avoid dependency loops */ - public function __construct(callable $sessionCallback) { + public function __construct( + callable $sessionCallback, + private TokenScopes $tokenScopes, + ) { $this->sessionCallback = $sessionCallback; } @@ -63,7 +68,26 @@ public function setToken(IToken $token) { #[\Override] public function canAccessFilesystem() { + $scopes = $this->scopes(); + if ($scopes !== null) { + return in_array(FilesTokenScopes::FILES_READ, $scopes, true); + } $scope = $this->getScopeAsArray(); - return !$scope || $scope[IToken::SCOPE_FILESYSTEM]; + return !$scope || ($scope[IToken::SCOPE_FILESYSTEM] ?? false); + } + + /** + * True for every scope when there is no token or it has no scopes (full access) + */ + public function hasScope(string $scope): bool { + $scopes = $this->scopes(); + return $scopes === null || in_array($scope, $scopes, true); + } + + /** + * @return list|null + */ + private function scopes(): ?array { + return $this->tokenScopes->fromBlob($this->getScopeAsArray() ?? []); } } diff --git a/lib/private/Server.php b/lib/private/Server.php index 1ca6e3cf341c9..771a9ae35bf70 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -23,6 +23,7 @@ use OC\Authentication\Listeners\UserLoggedInListener; use OC\Authentication\LoginCredentials\Store; use OC\Authentication\Token\IProvider; +use OC\Authentication\Token\TokenScopes; use OC\Authentication\TwoFactorAuth\Registry; use OC\Avatar\AvatarManager; use OC\BackgroundJob\JobList; @@ -1021,7 +1022,7 @@ public function __construct( /** @deprecated 35.0.0 */ $this->registerDeprecatedAlias('LockdownManager', ILockdownManager::class); $this->registerService(LockdownManager::class, static function (ContainerInterface $c): LockdownManager { - return new LockdownManager(static fn () => $c->get(ISession::class)); + return new LockdownManager(static fn () => $c->get(ISession::class), $c->get(TokenScopes::class)); }); $this->registerAlias(ILockdownManager::class, LockdownManager::class); $this->registerAlias(IDiscoveryService::class, DiscoveryService::class); diff --git a/tests/Core/Command/User/AuthTokens/ListCommandTest.php b/tests/Core/Command/User/AuthTokens/ListCommandTest.php new file mode 100644 index 0000000000000..abe72492e6d12 --- /dev/null +++ b/tests/Core/Command/User/AuthTokens/ListCommandTest.php @@ -0,0 +1,50 @@ + [ + [IToken::SCOPE_FILESYSTEM => true, IToken::SCOPE_SKIP_PASSWORD_VALIDATION => true], + 'filesystem, password-unconfirmable', + ], + 'scoped' => [ + [IToken::SCOPE_FILESYSTEM => true, TokenScopes::KEY => [FilesTokenScopes::FILES_READ, FilesTokenScopes::FILES_WRITE]], + 'filesystem, scopes: files:files:read files:files:write', + ], + 'empty scopes' => [ + [IToken::SCOPE_FILESYSTEM => false, TokenScopes::KEY => []], + 'scopes: none', + ], + ]; + } + + #[DataProvider('plainScopes')] + public function testPlainOutputShowsScopes(array $scope, string $expected): void { + $command = new ListCommand($this->createMock(IUserManager::class), $this->createMock(IProvider::class), new TokenScopes()); + + $token = $command->formatTokenForPlainOutput([ + 'scope' => $scope, + 'lastActivity' => 0, + 'type' => IToken::PERMANENT_TOKEN, + ]); + + $this->assertSame($expected, $token['scope']); + } +} diff --git a/tests/lib/Authentication/Token/TokenScopesTest.php b/tests/lib/Authentication/Token/TokenScopesTest.php new file mode 100644 index 0000000000000..f1ed8e3f167a2 --- /dev/null +++ b/tests/lib/Authentication/Token/TokenScopesTest.php @@ -0,0 +1,102 @@ +tokenScopes = new TokenScopes(); + } + + public static function merges(): array { + $fs = IToken::SCOPE_FILESYSTEM; + $calendar = [$fs => false, TokenScopes::KEY => ['dav:calendar:read']]; + $deck = [$fs => false, TokenScopes::KEY => ['app:deck']]; + return [ + 'legacy token takes requested filesystem' => [[$fs => true], [$fs => false], [$fs => false]], + 'legacy token keeps filesystem when omitted' => [[$fs => false], [], [$fs => false]], + 'scoped token keeps scopes, ignores requested filesystem' => [$calendar, [$fs => true], $calendar], + 'null returns to full access' => [$calendar, [$fs => false, TokenScopes::KEY => null], [$fs => true]], + 'new scopes normalised, filesystem derived' => [ + [$fs => true], + [TokenScopes::KEY => ['a' => FilesTokenScopes::FILES_WRITE, 3 => FilesTokenScopes::FILES_READ, 7 => FilesTokenScopes::FILES_WRITE]], + [$fs => true, TokenScopes::KEY => [FilesTokenScopes::FILES_READ, FilesTokenScopes::FILES_WRITE]], + ], + 'files with sharing' => [ + [$fs => true], + [TokenScopes::KEY => [FilesTokenScopes::FILES_READ, FilesTokenScopes::FILES_WRITE, SharingTokenScopes::SHARES_WRITE]], + [$fs => true, TokenScopes::KEY => [FilesTokenScopes::FILES_READ, FilesTokenScopes::FILES_WRITE, SharingTokenScopes::SHARES_WRITE]], + ], + 'empty scopes deny filesystem' => [[$fs => true], [TokenScopes::KEY => []], [$fs => false, TokenScopes::KEY => []]], + 'unchanged scopes skip validation' => [$deck, $deck, $deck], + 'scopes core does not know are stored' => [[$fs => true], [TokenScopes::KEY => ['app:deck']], $deck], + 'write without read is stored' => [[$fs => true], [TokenScopes::KEY => ['dav:calendar:write']], [$fs => false, TokenScopes::KEY => ['dav:calendar:write']]], + 'stored server keys are kept' => [ + [IToken::SCOPE_SKIP_PASSWORD_VALIDATION => true, $fs => true], + [IToken::SCOPE_SKIP_PASSWORD_VALIDATION => false], + [IToken::SCOPE_SKIP_PASSWORD_VALIDATION => true, $fs => true], + ], + 'requested server keys are ignored' => [[$fs => true], [IToken::SCOPE_SKIP_PASSWORD_VALIDATION => true], [$fs => true]], + ]; + } + + #[DataProvider('merges')] + public function testUpdateScope(array $stored, array $requested, array $expected): void { + $this->assertSame($expected, $this->tokenScopes->updateScope($stored, $requested)); + } + + public static function invalidUpdates(): array { + return [ + 'files write without read' => [[TokenScopes::KEY => [FilesTokenScopes::FILES_WRITE]]], + 'files share without read' => [[TokenScopes::KEY => [SharingTokenScopes::SHARES_WRITE]]], + 'files write and share without read' => [[TokenScopes::KEY => [FilesTokenScopes::FILES_WRITE, SharingTokenScopes::SHARES_WRITE]]], + 'read-only files' => [[TokenScopes::KEY => [FilesTokenScopes::FILES_READ]]], + 'read and share without write' => [[TokenScopes::KEY => [FilesTokenScopes::FILES_READ, SharingTokenScopes::SHARES_WRITE]]], + 'not an array' => [[TokenScopes::KEY => FilesTokenScopes::FILES_READ]], + 'nested array entry' => [[TokenScopes::KEY => ['dav:calendar:read', ['x']]]], + 'empty scope' => [[TokenScopes::KEY => ['']]], + 'scope too long' => [[TokenScopes::KEY => [str_repeat('a', 65)]]], + 'too many scopes' => [[TokenScopes::KEY => array_map(static fn (int $i) => 'app:' . $i, range(1, 101))]], + 'non-bool filesystem' => [[IToken::SCOPE_FILESYSTEM => 'false']], + ]; + } + + #[DataProvider('invalidUpdates')] + public function testInvalidUpdateIsRejected(array $requested): void { + $this->expectException(\InvalidArgumentException::class); + $this->tokenScopes->updateScope([IToken::SCOPE_FILESYSTEM => true], $requested); + } + + public static function blobs(): array { + return [ + 'legacy' => [[IToken::SCOPE_FILESYSTEM => true], null], + 'scoped' => [[TokenScopes::KEY => [FilesTokenScopes::FILES_READ]], [FilesTokenScopes::FILES_READ]], + 'empty' => [[TokenScopes::KEY => []], []], + 'string' => [[TokenScopes::KEY => FilesTokenScopes::FILES_READ], []], + 'null' => [[TokenScopes::KEY => null], []], + 'map' => [[TokenScopes::KEY => ['a' => FilesTokenScopes::FILES_READ]], [FilesTokenScopes::FILES_READ]], + 'int entry' => [[TokenScopes::KEY => [FilesTokenScopes::FILES_READ, 1]], []], + ]; + } + + #[DataProvider('blobs')] + public function testFromBlob(array $blob, ?array $expected): void { + $this->assertSame($expected, $this->tokenScopes->fromBlob($blob)); + } +} diff --git a/tests/lib/Lockdown/LockdownManagerTest.php b/tests/lib/Lockdown/LockdownManagerTest.php index ae7e8b5b13468..f3f5dd2a3c07e 100644 --- a/tests/lib/Lockdown/LockdownManagerTest.php +++ b/tests/lib/Lockdown/LockdownManagerTest.php @@ -8,9 +8,12 @@ namespace Test\Lockdown; use OC\Authentication\Token\PublicKeyToken; +use OC\Authentication\Token\TokenScopes; use OC\Lockdown\LockdownManager; +use OCA\Files\TokenScopes as FilesTokenScopes; use OCP\Authentication\Token\IToken; use OCP\ISession; +use PHPUnit\Framework\Attributes\DataProvider; use Test\TestCase; class LockdownManagerTest extends TestCase { @@ -26,14 +29,14 @@ protected function setUp(): void { } public function testCanAccessFilesystemDisabled(): void { - $manager = new LockdownManager($this->sessionCallback); + $manager = new LockdownManager($this->sessionCallback, new TokenScopes()); $this->assertTrue($manager->canAccessFilesystem()); } public function testCanAccessFilesystemAllowed(): void { $token = new PublicKeyToken(); $token->setScope([IToken::SCOPE_FILESYSTEM => true]); - $manager = new LockdownManager($this->sessionCallback); + $manager = new LockdownManager($this->sessionCallback, new TokenScopes()); $manager->setToken($token); $this->assertTrue($manager->canAccessFilesystem()); } @@ -41,8 +44,49 @@ public function testCanAccessFilesystemAllowed(): void { public function testCanAccessFilesystemNotAllowed(): void { $token = new PublicKeyToken(); $token->setScope([IToken::SCOPE_FILESYSTEM => false]); - $manager = new LockdownManager($this->sessionCallback); + $manager = new LockdownManager($this->sessionCallback, new TokenScopes()); $manager->setToken($token); $this->assertFalse($manager->canAccessFilesystem()); } + + public static function filesystemAccess(): array { + return [ + 'legacy without filesystem key' => [[IToken::SCOPE_SKIP_PASSWORD_VALIDATION => true], false], + 'files' => [[IToken::SCOPE_FILESYSTEM => false, TokenScopes::KEY => [FilesTokenScopes::FILES_READ]], true], + 'calendar only' => [[IToken::SCOPE_FILESYSTEM => true, TokenScopes::KEY => ['dav:calendar:read']], false], + 'malformed' => [[IToken::SCOPE_FILESYSTEM => true, TokenScopes::KEY => FilesTokenScopes::FILES_READ], false], + ]; + } + + #[DataProvider('filesystemAccess')] + public function testCanAccessFilesystemWithScope(array $scope, bool $expected): void { + $this->assertSame($expected, $this->managerWithScope($scope)->canAccessFilesystem()); + } + + public function testHasScopeWithoutToken(): void { + $manager = new LockdownManager($this->sessionCallback, new TokenScopes()); + $this->assertTrue($manager->hasScope('dav:calendar:read')); + } + + public static function hasScope(): array { + return [ + 'legacy' => [[IToken::SCOPE_FILESYSTEM => true], 'dav:calendar:read', true], + 'granted' => [[TokenScopes::KEY => ['dav:calendar:read']], 'dav:calendar:read', true], + 'not granted' => [[TokenScopes::KEY => ['dav:calendar:read']], FilesTokenScopes::FILES_READ, false], + 'malformed' => [[TokenScopes::KEY => 'dav:calendar:read'], 'dav:calendar:read', false], + ]; + } + + #[DataProvider('hasScope')] + public function testHasScope(array $scope, string $requested, bool $expected): void { + $this->assertSame($expected, $this->managerWithScope($scope)->hasScope($requested)); + } + + private function managerWithScope(array $scope): LockdownManager { + $token = new PublicKeyToken(); + $token->setScope($scope); + $manager = new LockdownManager($this->sessionCallback, new TokenScopes()); + $manager->setToken($token); + return $manager; + } }