From 3748c9c9b93a1a682a351ee0d7870a85ecfb71e3 Mon Sep 17 00:00:00 2001 From: Peter Ringelmann Date: Wed, 7 Oct 2026 14:29:11 +0200 Subject: [PATCH 1/9] feat(auth): limit scoped app passwords to the routes they open Signed-off-by: Peter Ringelmann --- .../composer/composer/autoload_classmap.php | 1 + .../dav/composer/composer/autoload_static.php | 1 + .../Controller/CalendarExportController.php | 3 + .../Controller/CalendarImportController.php | 3 + .../Controller/ContactsImportController.php | 3 + apps/dav/lib/Controller/DirectController.php | 3 + .../FederatedCalendarController.php | 5 + .../lib/Controller/OutOfOfficeController.php | 6 + .../Controller/UpcomingEventsController.php | 3 + apps/dav/lib/TokenScopes.php | 20 ++ apps/files/lib/Controller/ApiController.php | 3 + .../Controller/OpenLocalEditorController.php | 3 + .../TransferOwnershipController.php | 3 + apps/files/lib/Controller/ViewController.php | 3 + .../lib/Controller/AcceptController.php | 3 + .../Controller/DeletedShareAPIController.php | 3 + .../Controller/ExternalSharesController.php | 3 + .../Controller/PublicPreviewController.php | 3 + .../lib/Controller/RemoteController.php | 3 + .../lib/Controller/SettingsController.php | 3 + .../lib/Controller/ShareAPIController.php | 7 + .../lib/Controller/ShareController.php | 3 + .../lib/Controller/ShareInfoController.php | 3 + .../lib/Controller/ShareesAPIController.php | 3 + .../lib/Controller/PreviewController.php | 3 + .../lib/Controller/PreviewController.php | 3 + .../lib/Controller/OauthApiController.php | 2 + .../lib/Controller/UsersController.php | 2 + .../theming/lib/Controller/IconController.php | 4 + .../lib/Controller/ThemingController.php | 4 + .../lib/Controller/UserThemeController.php | 2 + core/Controller/AppPasswordController.php | 3 + core/Controller/AvatarController.php | 3 + core/Controller/GuestAvatarController.php | 3 + core/Controller/OCSController.php | 2 + core/Controller/PreviewController.php | 5 + core/Controller/WipeController.php | 3 + lib/composer/composer/autoload_classmap.php | 3 + lib/composer/composer/autoload_static.php | 3 + .../DependencyInjection/DIContainer.php | 3 + .../Http/Attributes/TokenScope.php | 29 +++ .../Exceptions/TokenScopeException.php | 20 ++ .../Security/TokenScopeMiddleware.php | 70 +++++++ lib/private/Lockdown/LockdownManager.php | 7 + .../DependencyInjection/DIContainerTest.php | 18 ++ .../Security/TokenScopeMiddlewareTest.php | 183 ++++++++++++++++++ tests/lib/Lockdown/LockdownManagerTest.php | 19 ++ 47 files changed, 488 insertions(+) create mode 100644 apps/dav/lib/TokenScopes.php create mode 100644 lib/private/AppFramework/Http/Attributes/TokenScope.php create mode 100644 lib/private/AppFramework/Middleware/Security/Exceptions/TokenScopeException.php create mode 100644 lib/private/AppFramework/Middleware/Security/TokenScopeMiddleware.php create mode 100644 tests/lib/AppFramework/Middleware/Security/TokenScopeMiddlewareTest.php diff --git a/apps/dav/composer/composer/autoload_classmap.php b/apps/dav/composer/composer/autoload_classmap.php index a15bb5d49e352..36779c3833f85 100644 --- a/apps/dav/composer/composer/autoload_classmap.php +++ b/apps/dav/composer/composer/autoload_classmap.php @@ -460,6 +460,7 @@ 'OCA\\DAV\\SystemTag\\SystemTagsObjectMappingCollection' => $baseDir . '/../lib/SystemTag/SystemTagsObjectMappingCollection.php', 'OCA\\DAV\\SystemTag\\SystemTagsObjectTypeCollection' => $baseDir . '/../lib/SystemTag/SystemTagsObjectTypeCollection.php', 'OCA\\DAV\\SystemTag\\SystemTagsRelationsCollection' => $baseDir . '/../lib/SystemTag/SystemTagsRelationsCollection.php', + 'OCA\\DAV\\TokenScopes' => $baseDir . '/../lib/TokenScopes.php', 'OCA\\DAV\\Traits\\PrincipalProxyTrait' => $baseDir . '/../lib/Traits/PrincipalProxyTrait.php', 'OCA\\DAV\\Upload\\AssemblyStream' => $baseDir . '/../lib/Upload/AssemblyStream.php', 'OCA\\DAV\\Upload\\ChunkingPlugin' => $baseDir . '/../lib/Upload/ChunkingPlugin.php', diff --git a/apps/dav/composer/composer/autoload_static.php b/apps/dav/composer/composer/autoload_static.php index 16788a2db444a..13397cb409415 100644 --- a/apps/dav/composer/composer/autoload_static.php +++ b/apps/dav/composer/composer/autoload_static.php @@ -475,6 +475,7 @@ class ComposerStaticInitDAV 'OCA\\DAV\\SystemTag\\SystemTagsObjectMappingCollection' => __DIR__ . '/..' . '/../lib/SystemTag/SystemTagsObjectMappingCollection.php', 'OCA\\DAV\\SystemTag\\SystemTagsObjectTypeCollection' => __DIR__ . '/..' . '/../lib/SystemTag/SystemTagsObjectTypeCollection.php', 'OCA\\DAV\\SystemTag\\SystemTagsRelationsCollection' => __DIR__ . '/..' . '/../lib/SystemTag/SystemTagsRelationsCollection.php', + 'OCA\\DAV\\TokenScopes' => __DIR__ . '/..' . '/../lib/TokenScopes.php', 'OCA\\DAV\\Traits\\PrincipalProxyTrait' => __DIR__ . '/..' . '/../lib/Traits/PrincipalProxyTrait.php', 'OCA\\DAV\\Upload\\AssemblyStream' => __DIR__ . '/..' . '/../lib/Upload/AssemblyStream.php', 'OCA\\DAV\\Upload\\ChunkingPlugin' => __DIR__ . '/..' . '/../lib/Upload/ChunkingPlugin.php', diff --git a/apps/dav/lib/Controller/CalendarExportController.php b/apps/dav/lib/Controller/CalendarExportController.php index cd66c139f89ed..344f9387cb721 100644 --- a/apps/dav/lib/Controller/CalendarExportController.php +++ b/apps/dav/lib/Controller/CalendarExportController.php @@ -8,8 +8,10 @@ namespace OCA\DAV\Controller; +use OC\AppFramework\Http\Attributes\TokenScope; use OCA\DAV\AppInfo\Application; use OCA\DAV\CalDAV\Export\ExportService; +use OCA\DAV\TokenScopes; use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\ApiRoute; use OCP\AppFramework\Http\Attribute\NoAdminRequired; @@ -55,6 +57,7 @@ public function __construct( #[ApiRoute(verb: 'POST', url: '/export', root: '/calendar')] #[UserRateLimit(limit: 1, period: 60)] #[NoAdminRequired] + #[TokenScope(scope: TokenScopes::CALENDAR_READ)] public function export(string $target, ?string $type = null, ?array $options = null, ?string $user = null) { $calendarId = $target; $format = $type ?? 'ical'; diff --git a/apps/dav/lib/Controller/CalendarImportController.php b/apps/dav/lib/Controller/CalendarImportController.php index 83c40fa5651b4..446b9d4f313e1 100644 --- a/apps/dav/lib/Controller/CalendarImportController.php +++ b/apps/dav/lib/Controller/CalendarImportController.php @@ -9,12 +9,14 @@ namespace OCA\DAV\Controller; use InvalidArgumentException; +use OC\AppFramework\Http\Attributes\TokenScope; /** * @psalm-type CalendarImportResult = array{items: list, total: non-negative-int} */ use OCA\DAV\AppInfo\Application; use OCA\DAV\CalDAV\CalendarImpl; use OCA\DAV\CalDAV\Import\ImportService; +use OCA\DAV\TokenScopes; use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\ApiRoute; use OCP\AppFramework\Http\Attribute\NoAdminRequired; @@ -62,6 +64,7 @@ public function __construct( #[ApiRoute(verb: 'POST', url: '/import', root: '/calendar')] #[UserRateLimit(limit: 10, period: 3600)] #[NoAdminRequired] + #[TokenScope(scope: TokenScopes::CALENDAR_WRITE)] public function import(string $transaction, string $target, array $options, string $data, ?string $user = null): DataResponse|StreamTraversableResponse { $calendarId = $target; $format = isset($options['format']) ? $options['format'] : null; diff --git a/apps/dav/lib/Controller/ContactsImportController.php b/apps/dav/lib/Controller/ContactsImportController.php index fe1264a5f3b1d..6d05260e04aaa 100644 --- a/apps/dav/lib/Controller/ContactsImportController.php +++ b/apps/dav/lib/Controller/ContactsImportController.php @@ -9,11 +9,13 @@ namespace OCA\DAV\Controller; use InvalidArgumentException; +use OC\AppFramework\Http\Attributes\TokenScope; /** * @psalm-type ContactsImportResult = array{items: list, total: non-negative-int} */ use OCA\DAV\AppInfo\Application; use OCA\DAV\CardDAV\Import\ImportService; +use OCA\DAV\TokenScopes; use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\ApiRoute; use OCP\AppFramework\Http\Attribute\NoAdminRequired; @@ -62,6 +64,7 @@ public function __construct( #[ApiRoute(verb: 'POST', url: '/import', root: '/contacts')] #[UserRateLimit(limit: 10, period: 3600)] #[NoAdminRequired] + #[TokenScope(scope: TokenScopes::CONTACTS_WRITE)] public function import(string $transaction, string $target, array $options, string $data, ?string $user = null): DataResponse|StreamTraversableResponse { $addressBookId = $target; $format = isset($options['format']) ? $options['format'] : null; diff --git a/apps/dav/lib/Controller/DirectController.php b/apps/dav/lib/Controller/DirectController.php index 7ccdcdcc34dd9..dd587908d4366 100644 --- a/apps/dav/lib/Controller/DirectController.php +++ b/apps/dav/lib/Controller/DirectController.php @@ -9,8 +9,10 @@ namespace OCA\DAV\Controller; +use OC\AppFramework\Http\Attributes\TokenScope; use OCA\DAV\Db\Direct; use OCA\DAV\Db\DirectMapper; +use OCA\Files\TokenScopes; use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\NoAdminRequired; use OCP\AppFramework\Http\DataResponse; @@ -58,6 +60,7 @@ public function __construct( * 200: Direct link returned */ #[NoAdminRequired] + #[TokenScope(scope: TokenScopes::FILES_SHARE)] public function getUrl(int $fileId, int $expirationTime = 60 * 60 * 8): DataResponse { if (!$this->shareManager->shareApiAllowLinks()) { throw new OCSForbiddenException('Creating direct links is disabled'); diff --git a/apps/dav/lib/Controller/FederatedCalendarController.php b/apps/dav/lib/Controller/FederatedCalendarController.php index cd674f6006a08..4f7e6f4b84218 100644 --- a/apps/dav/lib/Controller/FederatedCalendarController.php +++ b/apps/dav/lib/Controller/FederatedCalendarController.php @@ -9,11 +9,13 @@ namespace OCA\DAV\Controller; +use OC\AppFramework\Http\Attributes\TokenScope; use OCA\DAV\AppInfo\Application; use OCA\DAV\CalDAV\Federation\FederatedCalendarEntity; use OCA\DAV\CalDAV\Federation\FederatedCalendarInvitationService; use OCA\DAV\CalDAV\Federation\FederatedCalendarMapper; use OCA\DAV\ResponseDefinitions; +use OCA\DAV\TokenScopes; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\NoAdminRequired; @@ -44,6 +46,7 @@ public function __construct( * 401: When the user is not logged in */ #[NoAdminRequired] + #[TokenScope(scope: TokenScopes::CALENDAR_READ)] public function getPending(): DataResponse { if ($this->userId === null) { return new DataResponse(null, Http::STATUS_UNAUTHORIZED); @@ -76,6 +79,7 @@ public function getPending(): DataResponse { * 200: Invitation accepted successfully */ #[NoAdminRequired] + #[TokenScope(scope: TokenScopes::CALENDAR_WRITE)] public function accept(int $id): DataResponse { $this->invitationService->accept($this->getPendingCalendar($id)); return new DataResponse(null); @@ -91,6 +95,7 @@ public function accept(int $id): DataResponse { * 200: Invitation declined successfully */ #[NoAdminRequired] + #[TokenScope(scope: TokenScopes::CALENDAR_WRITE)] public function decline(int $id): DataResponse { $this->invitationService->decline($this->getPendingCalendar($id)); return new DataResponse(null); diff --git a/apps/dav/lib/Controller/OutOfOfficeController.php b/apps/dav/lib/Controller/OutOfOfficeController.php index d3516d092e8cd..7928112e23f6c 100644 --- a/apps/dav/lib/Controller/OutOfOfficeController.php +++ b/apps/dav/lib/Controller/OutOfOfficeController.php @@ -10,8 +10,10 @@ namespace OCA\DAV\Controller; use DateTimeImmutable; +use OC\AppFramework\Http\Attributes\TokenScope; use OCA\DAV\ResponseDefinitions; use OCA\DAV\Service\AbsenceService; +use OCA\DAV\TokenScopes; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\NoAdminRequired; @@ -50,6 +52,7 @@ public function __construct( * 404: No out-of-office data was found */ #[NoAdminRequired] + #[TokenScope(scope: TokenScopes::CALENDAR_READ)] public function getCurrentOutOfOfficeData(string $userId): DataResponse { $user = $this->userManager->get($userId); if ($user === null) { @@ -77,6 +80,7 @@ public function getCurrentOutOfOfficeData(string $userId): DataResponse { * 404: No out-of-office data was found */ #[NoAdminRequired] + #[TokenScope(scope: TokenScopes::CALENDAR_READ)] public function getOutOfOffice(string $userId): DataResponse { try { $data = $this->absenceService->getAbsence($userId); @@ -115,6 +119,7 @@ public function getOutOfOffice(string $userId): DataResponse { * 404: When the replacementUserId was provided but replacement user was not found */ #[NoAdminRequired] + #[TokenScope(scope: TokenScopes::CALENDAR_WRITE)] public function setOutOfOffice( string $firstDay, string $lastDay, @@ -176,6 +181,7 @@ public function setOutOfOffice( * 401: When the user is not logged in */ #[NoAdminRequired] + #[TokenScope(scope: TokenScopes::CALENDAR_WRITE)] public function clearOutOfOffice(): DataResponse { $user = $this->userSession?->getUser(); if ($user === null) { diff --git a/apps/dav/lib/Controller/UpcomingEventsController.php b/apps/dav/lib/Controller/UpcomingEventsController.php index a5d54f4475445..bedbe596b4587 100644 --- a/apps/dav/lib/Controller/UpcomingEventsController.php +++ b/apps/dav/lib/Controller/UpcomingEventsController.php @@ -9,10 +9,12 @@ namespace OCA\DAV\Controller; +use OC\AppFramework\Http\Attributes\TokenScope; use OCA\DAV\AppInfo\Application; use OCA\DAV\CalDAV\UpcomingEvent; use OCA\DAV\CalDAV\UpcomingEventsService; use OCA\DAV\ResponseDefinitions; +use OCA\DAV\TokenScopes; use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\NoAdminRequired; use OCP\AppFramework\Http\DataResponse; @@ -41,6 +43,7 @@ public function __construct( * 401: When not authenticated */ #[NoAdminRequired] + #[TokenScope(scope: TokenScopes::CALENDAR_READ)] public function getEvents(?string $location = null): DataResponse { if ($this->userId === null) { return new DataResponse(null, Http::STATUS_UNAUTHORIZED); diff --git a/apps/dav/lib/TokenScopes.php b/apps/dav/lib/TokenScopes.php new file mode 100644 index 0000000000000..1e2ebec9c03ee --- /dev/null +++ b/apps/dav/lib/TokenScopes.php @@ -0,0 +1,20 @@ +getShareById($id); @@ -611,6 +615,7 @@ public function deleteShare(string $id): DataResponse { */ #[NoAdminRequired] #[UserRateLimit(limit: 60, period: 600)] + #[TokenScope(scope: TokenScopes::FILES_SHARE)] public function createShare( ?string $path = null, ?int $permissions = null, @@ -1247,6 +1252,7 @@ private function hasPermission(int $permissionsSet, int $permissionsToCheck): bo * 200: Share updated successfully */ #[NoAdminRequired] + #[TokenScope(scope: TokenScopes::FILES_SHARE)] public function updateShare( string $id, ?int $permissions = null, @@ -2160,6 +2166,7 @@ private function checkInheritedAttributes(IShare $share): void { */ #[NoAdminRequired] #[UserRateLimit(limit: 10, period: 600)] + #[TokenScope(scope: TokenScopes::FILES_SHARE)] public function sendShareEmail(string $id, $password = ''): DataResponse { try { $share = $this->getShareById($id); diff --git a/apps/files_sharing/lib/Controller/ShareController.php b/apps/files_sharing/lib/Controller/ShareController.php index 38a355bda5d39..e0668dcc8f1ed 100644 --- a/apps/files_sharing/lib/Controller/ShareController.php +++ b/apps/files_sharing/lib/Controller/ShareController.php @@ -8,9 +8,11 @@ namespace OCA\Files_Sharing\Controller; +use OC\AppFramework\Http\Attributes\TokenScope; use OC\ServerNotAvailableException; use OCA\DAV\Connector\Sabre\PublicAuth; use OCA\FederatedFileSharing\FederatedShareProvider; +use OCA\Files\TokenScopes; use OCA\Files_Sharing\Event\BeforeTemplateRenderedEvent; use OCA\Files_Sharing\Event\ShareLinkAccessedEvent; use OCP\Accounts\IAccountManager; @@ -54,6 +56,7 @@ * @package OCA\Files_Sharing\Controllers */ #[OpenAPI(scope: OpenAPI::SCOPE_IGNORE)] +#[TokenScope(scope: TokenScopes::FILES_WRITE)] class ShareController extends AuthPublicShareController { protected ?IShare $share = null; diff --git a/apps/files_sharing/lib/Controller/ShareInfoController.php b/apps/files_sharing/lib/Controller/ShareInfoController.php index 1265256d7a6c2..912a223b6646f 100644 --- a/apps/files_sharing/lib/Controller/ShareInfoController.php +++ b/apps/files_sharing/lib/Controller/ShareInfoController.php @@ -7,6 +7,8 @@ namespace OCA\Files_Sharing\Controller; +use OC\AppFramework\Http\Attributes\TokenScope; +use OCA\Files\TokenScopes; use OCA\Files_Sharing\ResponseDefinitions; use OCP\AppFramework\ApiController; use OCP\AppFramework\Http; @@ -27,6 +29,7 @@ /** * @psalm-import-type Files_SharingShareInfo from ResponseDefinitions */ +#[TokenScope(scope: TokenScopes::FILES_READ)] class ShareInfoController extends ApiController { /** diff --git a/apps/files_sharing/lib/Controller/ShareesAPIController.php b/apps/files_sharing/lib/Controller/ShareesAPIController.php index 666ee52e2bd2e..093589b3bfd36 100644 --- a/apps/files_sharing/lib/Controller/ShareesAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareesAPIController.php @@ -10,8 +10,10 @@ namespace OCA\Files_Sharing\Controller; use Generator; +use OC\AppFramework\Http\Attributes\TokenScope; use OC\Collaboration\Collaborators\SearchResult; use OCA\FederatedFileSharing\FederatedShareProvider; +use OCA\Files\TokenScopes; use OCA\Files_Sharing\ResponseDefinitions; use OCP\App\IAppManager; use OCP\AppFramework\Http; @@ -38,6 +40,7 @@ * @psalm-import-type Files_SharingShareesSearchResult from ResponseDefinitions * @psalm-import-type Files_SharingShareesRecommendedResult from ResponseDefinitions */ +#[TokenScope(scope: TokenScopes::FILES_SHARE)] class ShareesAPIController extends OCSController { protected int $offset = 0; diff --git a/apps/files_trashbin/lib/Controller/PreviewController.php b/apps/files_trashbin/lib/Controller/PreviewController.php index 3ca66ec3047b8..ef6b7d3c22d8a 100644 --- a/apps/files_trashbin/lib/Controller/PreviewController.php +++ b/apps/files_trashbin/lib/Controller/PreviewController.php @@ -9,6 +9,8 @@ namespace OCA\Files_Trashbin\Controller; +use OC\AppFramework\Http\Attributes\TokenScope; +use OCA\Files\TokenScopes; use OCA\Files_Trashbin\Trash\ITrashManager; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; @@ -27,6 +29,7 @@ use OCP\IUserSession; #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] +#[TokenScope(scope: TokenScopes::FILES_READ)] class PreviewController extends Controller { public function __construct( string $appName, diff --git a/apps/files_versions/lib/Controller/PreviewController.php b/apps/files_versions/lib/Controller/PreviewController.php index 126506dbd5818..82af152f21ce0 100644 --- a/apps/files_versions/lib/Controller/PreviewController.php +++ b/apps/files_versions/lib/Controller/PreviewController.php @@ -7,6 +7,8 @@ namespace OCA\Files_Versions\Controller; +use OC\AppFramework\Http\Attributes\TokenScope; +use OCA\Files\TokenScopes; use OCA\Files_Versions\Versions\IVersionManager; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; @@ -24,6 +26,7 @@ use OCP\Preview\IMimeIconProvider; #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] +#[TokenScope(scope: TokenScopes::FILES_READ)] class PreviewController extends Controller { public function __construct( diff --git a/apps/oauth2/lib/Controller/OauthApiController.php b/apps/oauth2/lib/Controller/OauthApiController.php index 8dc05851cf801..4c23bda6f3290 100644 --- a/apps/oauth2/lib/Controller/OauthApiController.php +++ b/apps/oauth2/lib/Controller/OauthApiController.php @@ -9,6 +9,7 @@ namespace OCA\OAuth2\Controller; +use OC\AppFramework\Http\Attributes\TokenScope; use OC\Authentication\Token\IProvider as TokenProvider; use OCA\OAuth2\Db\AccessTokenMapper; use OCA\OAuth2\Db\ClientMapper; @@ -83,6 +84,7 @@ public function __construct( #[PublicPage] #[NoCSRFRequired] #[BruteForceProtection(action: 'oauth2GetToken')] + #[TokenScope] public function getToken( string $grant_type, ?string $code, ?string $refresh_token, ?string $client_id, ?string $client_secret, diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index 953326fa130b5..d30eb9a882868 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -11,6 +11,7 @@ namespace OCA\Provisioning_API\Controller; use InvalidArgumentException; +use OC\AppFramework\Http\Attributes\TokenScope; use OC\AppFramework\Http\PaginationTrait; use OC\Authentication\Token\RemoteWipe; use OC\Group\DisplayNameCache as GroupDisplayNameCache; @@ -705,6 +706,7 @@ public function getUser(string $userId): DataResponse { */ #[NoAdminRequired] #[NoSubAdminRequired] + #[TokenScope] public function getCurrentUser(): DataResponse { $user = $this->userSession->getUser(); if ($user) { diff --git a/apps/theming/lib/Controller/IconController.php b/apps/theming/lib/Controller/IconController.php index 69ef91bf5b088..38ea0d93a3adc 100644 --- a/apps/theming/lib/Controller/IconController.php +++ b/apps/theming/lib/Controller/IconController.php @@ -7,6 +7,7 @@ namespace OCA\Theming\Controller; +use OC\AppFramework\Http\Attributes\TokenScope; use OC\IntegrityCheck\Helpers\FileAccessHelper; use OCA\Theming\IconBuilder; use OCA\Theming\ImageManager; @@ -53,6 +54,7 @@ public function __construct( #[PublicPage] #[NoCSRFRequired] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] + #[TokenScope] public function getThemedIcon(string $app, string $image): Response { if ($app !== 'core' && !$this->appManager->isEnabledForUser($app)) { $app = 'core'; @@ -87,6 +89,7 @@ public function getThemedIcon(string $app, string $image): Response { #[PublicPage] #[NoCSRFRequired] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] + #[TokenScope] public function getFavicon(string $app = 'core'): Response { if ($app !== 'core' && !$this->appManager->isEnabledForUser($app)) { $app = 'core'; @@ -136,6 +139,7 @@ public function getFavicon(string $app = 'core'): Response { #[PublicPage] #[NoCSRFRequired] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] + #[TokenScope] public function getTouchIcon(string $app = 'core'): Response { if ($app !== 'core' && !$this->appManager->isEnabledForUser($app)) { $app = 'core'; diff --git a/apps/theming/lib/Controller/ThemingController.php b/apps/theming/lib/Controller/ThemingController.php index 76c1129366701..daa12b94d5977 100644 --- a/apps/theming/lib/Controller/ThemingController.php +++ b/apps/theming/lib/Controller/ThemingController.php @@ -8,6 +8,7 @@ namespace OCA\Theming\Controller; use InvalidArgumentException; +use OC\AppFramework\Http\Attributes\TokenScope; use OCA\Theming\ImageManager; use OCA\Theming\Service\ThemesService; use OCA\Theming\Settings\Admin; @@ -360,6 +361,7 @@ public function undoAll(): DataResponse { #[PublicPage] #[NoCSRFRequired] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] + #[TokenScope] public function getImage(string $key, bool $useSvg = true) { try { $useSvg = $useSvg && $this->imageManager->canConvert('SVG'); @@ -400,6 +402,7 @@ public function getImage(string $key, bool $useSvg = true) { #[NoTwoFactorRequired] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] #[NoSameSiteCookieRequired] + #[TokenScope] public function getThemeStylesheet(string $themeId, bool $plain = false, bool $withCustomCss = false) { $themes = $this->themesService->getThemes(); if (!in_array($themeId, array_keys($themes), true)) { @@ -456,6 +459,7 @@ public function getThemeStylesheet(string $themeId, bool $plain = false, bool $w #[NoCSRFRequired] #[BruteForceProtection(action: 'manifest')] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] + #[TokenScope] public function getManifest(string $app): JSONResponse { $cacheBusterValue = (string)$this->appConfig->getAppValueInt('cachebuster'); if ($app === 'core' || $app === 'settings') { diff --git a/apps/theming/lib/Controller/UserThemeController.php b/apps/theming/lib/Controller/UserThemeController.php index fa3dc4219b1da..ef4f7c9b65a29 100644 --- a/apps/theming/lib/Controller/UserThemeController.php +++ b/apps/theming/lib/Controller/UserThemeController.php @@ -9,6 +9,7 @@ namespace OCA\Theming\Controller; +use OC\AppFramework\Http\Attributes\TokenScope; use OCA\Theming\AppInfo\Application; use OCA\Theming\ITheme; use OCA\Theming\ResponseDefinitions; @@ -133,6 +134,7 @@ private function validateTheme(string $themeId): ITheme { #[NoAdminRequired] #[NoCSRFRequired] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] + #[TokenScope] public function getBackground(): Response { $file = $this->backgroundService->getBackground(); if ($file !== null) { diff --git a/core/Controller/AppPasswordController.php b/core/Controller/AppPasswordController.php index faa26a31913f7..a641a3488f377 100644 --- a/core/Controller/AppPasswordController.php +++ b/core/Controller/AppPasswordController.php @@ -9,6 +9,7 @@ namespace OC\Core\Controller; +use OC\AppFramework\Http\Attributes\TokenScope; use OC\Authentication\Events\AppPasswordCreatedEvent; use OC\Authentication\Token\IProvider; use OC\Authentication\Token\IToken; @@ -120,6 +121,7 @@ public function getAppPassword(): DataResponse { */ #[NoAdminRequired] #[ApiRoute(verb: 'DELETE', url: '/apppassword', root: '/core')] + #[TokenScope] public function deleteAppPassword(): DataResponse { if (!$this->session->exists('app_password')) { throw new OCSForbiddenException('no app password in use'); @@ -147,6 +149,7 @@ public function deleteAppPassword(): DataResponse { */ #[NoAdminRequired] #[ApiRoute(verb: 'POST', url: '/apppassword/rotate', root: '/core')] + #[TokenScope] public function rotateAppPassword(): DataResponse { if (!$this->session->exists('app_password')) { throw new OCSForbiddenException('no app password in use'); diff --git a/core/Controller/AvatarController.php b/core/Controller/AvatarController.php index 997c082cd5b33..36c62b6d7a644 100644 --- a/core/Controller/AvatarController.php +++ b/core/Controller/AvatarController.php @@ -8,6 +8,7 @@ namespace OC\Core\Controller; +use OC\AppFramework\Http\Attributes\TokenScope; use OC\AppFramework\Utility\TimeFactory; use OC\Avatar\AvatarManager; use OCP\AppFramework\Controller; @@ -85,6 +86,7 @@ public function __construct( #[FrontpageRoute(verb: 'GET', url: '/avatar/{userId}/{size}/dark')] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] #[NoSameSiteCookieRequired] + #[TokenScope] public function getAvatarDark(string $userId, int $size, bool $guestFallback = false, string $v = '') { if ($size <= 64) { if ($size !== 64) { @@ -135,6 +137,7 @@ public function getAvatarDark(string $userId, int $size, bool $guestFallback = f #[FrontpageRoute(verb: 'GET', url: '/avatar/{userId}/{size}')] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] #[NoSameSiteCookieRequired] + #[TokenScope] public function getAvatar(string $userId, int $size, bool $guestFallback = false, string $v = '') { if ($size <= 64) { if ($size !== 64) { diff --git a/core/Controller/GuestAvatarController.php b/core/Controller/GuestAvatarController.php index c78350cdf00d8..8115ad881b90e 100644 --- a/core/Controller/GuestAvatarController.php +++ b/core/Controller/GuestAvatarController.php @@ -7,6 +7,7 @@ namespace OC\Core\Controller; +use OC\AppFramework\Http\Attributes\TokenScope; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\FrontpageRoute; @@ -50,6 +51,7 @@ public function __construct( #[NoCSRFRequired] #[FrontpageRoute(verb: 'GET', url: '/avatar/guest/{guestName}/{size}')] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] + #[TokenScope] public function getAvatar(string $guestName, int $size, ?bool $darkTheme = false) { $darkTheme = $darkTheme ?? false; @@ -102,6 +104,7 @@ public function getAvatar(string $guestName, int $size, ?bool $darkTheme = false #[NoCSRFRequired] #[FrontpageRoute(verb: 'GET', url: '/avatar/guest/{guestName}/{size}/dark')] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] + #[TokenScope] public function getAvatarDark(string $guestName, int $size) { return $this->getAvatar($guestName, $size, true); } diff --git a/core/Controller/OCSController.php b/core/Controller/OCSController.php index 144012ac35f86..e03d05e851380 100644 --- a/core/Controller/OCSController.php +++ b/core/Controller/OCSController.php @@ -7,6 +7,7 @@ namespace OC\Core\Controller; +use OC\AppFramework\Http\Attributes\TokenScope; use OC\CapabilitiesManager; use OC\Security\IdentityProof\Manager; use OCP\AppFramework\Http; @@ -62,6 +63,7 @@ public function getConfig(): DataResponse { */ #[PublicPage] #[ApiRoute(verb: 'GET', url: '/capabilities', root: '/cloud')] + #[TokenScope] public function getCapabilities(): DataResponse { $result = []; $result['version'] = [ diff --git a/core/Controller/PreviewController.php b/core/Controller/PreviewController.php index d4bb3e5f0955f..c2d86d3d1858f 100644 --- a/core/Controller/PreviewController.php +++ b/core/Controller/PreviewController.php @@ -9,6 +9,8 @@ namespace OC\Core\Controller; +use OC\AppFramework\Http\Attributes\TokenScope; +use OCA\Files\TokenScopes; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\FrontpageRoute; @@ -62,6 +64,7 @@ public function __construct( #[NoCSRFRequired] #[FrontpageRoute(verb: 'GET', url: '/core/preview.png')] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] + #[TokenScope(scope: TokenScopes::FILES_READ)] public function getPreview( string $file = '', int $x = 32, @@ -105,6 +108,7 @@ public function getPreview( #[NoCSRFRequired] #[FrontpageRoute(verb: 'GET', url: '/core/preview')] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] + #[TokenScope(scope: TokenScopes::FILES_READ)] public function getPreviewByFileId( int $fileId, int $x = 32, @@ -193,6 +197,7 @@ private function fetchPreview( #[PublicPage] #[FrontpageRoute(verb: 'GET', url: '/core/mimeicon')] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] + #[TokenScope] public function getMimeIconUrl(string $mime = 'application/octet-stream') { $url = $this->mimeIconProvider->getMimeIconUrl($mime); if ($url === null) { diff --git a/core/Controller/WipeController.php b/core/Controller/WipeController.php index f495ba5a692a4..bacc4897a4d6b 100644 --- a/core/Controller/WipeController.php +++ b/core/Controller/WipeController.php @@ -9,6 +9,7 @@ namespace OC\Core\Controller; +use OC\AppFramework\Http\Attributes\TokenScope; use OC\Authentication\Token\RemoteWipe; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; @@ -45,6 +46,7 @@ public function __construct( #[NoCSRFRequired] #[AnonRateLimit(limit: 10, period: 300)] #[FrontpageRoute(verb: 'POST', url: '/core/wipe/check')] + #[TokenScope] public function checkWipe(string $token): JSONResponse { try { if ($this->remoteWipe->start($token)) { @@ -73,6 +75,7 @@ public function checkWipe(string $token): JSONResponse { #[NoCSRFRequired] #[AnonRateLimit(limit: 10, period: 300)] #[FrontpageRoute(verb: 'POST', url: '/core/wipe/success')] + #[TokenScope] public function wipeDone(string $token): JSONResponse { try { if ($this->remoteWipe->finish($token)) { diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index d9349026b8639..1745eff0ff8a5 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1221,6 +1221,7 @@ 'OC\\AppFramework\\DependencyInjection\\DIContainer' => $baseDir . '/lib/private/AppFramework/DependencyInjection/DIContainer.php', 'OC\\AppFramework\\Http' => $baseDir . '/lib/private/AppFramework/Http.php', 'OC\\AppFramework\\Http\\Attributes\\FederationRateLimit' => $baseDir . '/lib/private/AppFramework/Http/Attributes/FederationRateLimit.php', + 'OC\\AppFramework\\Http\\Attributes\\TokenScope' => $baseDir . '/lib/private/AppFramework/Http/Attributes/TokenScope.php', 'OC\\AppFramework\\Http\\Attributes\\TwoFactorSetUpDoneRequired' => $baseDir . '/lib/private/AppFramework/Http/Attributes/TwoFactorSetUpDoneRequired.php', 'OC\\AppFramework\\Http\\Dispatcher' => $baseDir . '/lib/private/AppFramework/Http/Dispatcher.php', 'OC\\AppFramework\\Http\\Output' => $baseDir . '/lib/private/AppFramework/Http/Output.php', @@ -1248,11 +1249,13 @@ 'OC\\AppFramework\\Middleware\\Security\\Exceptions\\NotLoggedInException' => $baseDir . '/lib/private/AppFramework/Middleware/Security/Exceptions/NotLoggedInException.php', 'OC\\AppFramework\\Middleware\\Security\\Exceptions\\SecurityException' => $baseDir . '/lib/private/AppFramework/Middleware/Security/Exceptions/SecurityException.php', 'OC\\AppFramework\\Middleware\\Security\\Exceptions\\StrictCookieMissingException' => $baseDir . '/lib/private/AppFramework/Middleware/Security/Exceptions/StrictCookieMissingException.php', + 'OC\\AppFramework\\Middleware\\Security\\Exceptions\\TokenScopeException' => $baseDir . '/lib/private/AppFramework/Middleware/Security/Exceptions/TokenScopeException.php', 'OC\\AppFramework\\Middleware\\Security\\FeaturePolicyMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/Security/FeaturePolicyMiddleware.php', 'OC\\AppFramework\\Middleware\\Security\\PasswordConfirmationMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php', 'OC\\AppFramework\\Middleware\\Security\\RateLimitingMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/Security/RateLimitingMiddleware.php', 'OC\\AppFramework\\Middleware\\Security\\SameSiteCookieMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/Security/SameSiteCookieMiddleware.php', 'OC\\AppFramework\\Middleware\\Security\\SecurityMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php', + 'OC\\AppFramework\\Middleware\\Security\\TokenScopeMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/Security/TokenScopeMiddleware.php', 'OC\\AppFramework\\Middleware\\SessionMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/SessionMiddleware.php', 'OC\\AppFramework\\OCS\\BaseResponse' => $baseDir . '/lib/private/AppFramework/OCS/BaseResponse.php', 'OC\\AppFramework\\OCS\\V1Response' => $baseDir . '/lib/private/AppFramework/OCS/V1Response.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index fb689a3d7cad6..634ae9e58e7ac 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1262,6 +1262,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\AppFramework\\DependencyInjection\\DIContainer' => __DIR__ . '/../../..' . '/lib/private/AppFramework/DependencyInjection/DIContainer.php', 'OC\\AppFramework\\Http' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Http.php', 'OC\\AppFramework\\Http\\Attributes\\FederationRateLimit' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Http/Attributes/FederationRateLimit.php', + 'OC\\AppFramework\\Http\\Attributes\\TokenScope' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Http/Attributes/TokenScope.php', 'OC\\AppFramework\\Http\\Attributes\\TwoFactorSetUpDoneRequired' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Http/Attributes/TwoFactorSetUpDoneRequired.php', 'OC\\AppFramework\\Http\\Dispatcher' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Http/Dispatcher.php', 'OC\\AppFramework\\Http\\Output' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Http/Output.php', @@ -1289,11 +1290,13 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\AppFramework\\Middleware\\Security\\Exceptions\\NotLoggedInException' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/Exceptions/NotLoggedInException.php', 'OC\\AppFramework\\Middleware\\Security\\Exceptions\\SecurityException' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/Exceptions/SecurityException.php', 'OC\\AppFramework\\Middleware\\Security\\Exceptions\\StrictCookieMissingException' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/Exceptions/StrictCookieMissingException.php', + 'OC\\AppFramework\\Middleware\\Security\\Exceptions\\TokenScopeException' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/Exceptions/TokenScopeException.php', 'OC\\AppFramework\\Middleware\\Security\\FeaturePolicyMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/FeaturePolicyMiddleware.php', 'OC\\AppFramework\\Middleware\\Security\\PasswordConfirmationMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php', 'OC\\AppFramework\\Middleware\\Security\\RateLimitingMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/RateLimitingMiddleware.php', 'OC\\AppFramework\\Middleware\\Security\\SameSiteCookieMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/SameSiteCookieMiddleware.php', 'OC\\AppFramework\\Middleware\\Security\\SecurityMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php', + 'OC\\AppFramework\\Middleware\\Security\\TokenScopeMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/TokenScopeMiddleware.php', 'OC\\AppFramework\\Middleware\\SessionMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/SessionMiddleware.php', 'OC\\AppFramework\\OCS\\BaseResponse' => __DIR__ . '/../../..' . '/lib/private/AppFramework/OCS/BaseResponse.php', 'OC\\AppFramework\\OCS\\V1Response' => __DIR__ . '/../../..' . '/lib/private/AppFramework/OCS/V1Response.php', diff --git a/lib/private/AppFramework/DependencyInjection/DIContainer.php b/lib/private/AppFramework/DependencyInjection/DIContainer.php index 491b1c1332f9e..9afbc007bb10d 100644 --- a/lib/private/AppFramework/DependencyInjection/DIContainer.php +++ b/lib/private/AppFramework/DependencyInjection/DIContainer.php @@ -29,6 +29,7 @@ use OC\AppFramework\Middleware\Security\RateLimitingMiddleware; use OC\AppFramework\Middleware\Security\SameSiteCookieMiddleware; use OC\AppFramework\Middleware\Security\SecurityMiddleware; +use OC\AppFramework\Middleware\Security\TokenScopeMiddleware; use OC\AppFramework\Middleware\SessionMiddleware; use OC\AppFramework\ScopedPsrLogger; use OC\AppFramework\Services\AppConfig; @@ -214,6 +215,8 @@ public function __construct( $dispatcher->registerMiddleware($c->get(RateLimitingMiddleware::class)); $dispatcher->registerMiddleware($c->get(PublicShareMiddleware::class)); $dispatcher->registerMiddleware($c->get(AdditionalScriptsMiddleware::class)); + // After AdditionalScriptsMiddleware, so pages are refused before their listeners run + $dispatcher->registerMiddleware($c->get(TokenScopeMiddleware::class)); $coordinator = $c->get(Coordinator::class); $registrationContext = $coordinator->getRegistrationContext(); diff --git a/lib/private/AppFramework/Http/Attributes/TokenScope.php b/lib/private/AppFramework/Http/Attributes/TokenScope.php new file mode 100644 index 0000000000000..093689d9d7459 --- /dev/null +++ b/lib/private/AppFramework/Http/Attributes/TokenScope.php @@ -0,0 +1,29 @@ +lockdownManager->isScoped()) { + return; + } + + if (!$this->reflector->hasAnnotationOrAttribute('PublicPage', PublicPage::class) + && !$this->reflector->hasAnnotationOrAttribute('NoAdminRequired', NoAdminRequired::class) + && !$this->reflector->hasAnnotationOrAttribute('SubAdminRequired', SubAdminRequired::class)) { + throw new TokenScopeException(); + } + + $attribute = $this->tokenScope($controller); + if ($attribute === null + || ($attribute->scope !== null && !$this->lockdownManager->hasScope($attribute->scope))) { + throw new TokenScopeException(); + } + } + + /** + * The method's attribute, else its class's + */ + private function tokenScope(Controller $controller): ?TokenScope { + $attribute = $this->reflector->getAttribute(TokenScope::class) + ?? (new ReflectionClass($controller))->getAttributes(TokenScope::class)[0] ?? null; + return $attribute?->newInstance(); + } + + /** + * Page renders run every app's listeners and initial state, so none reach a token with scopes + */ + #[\Override] + public function afterController(Controller $controller, string $methodName, Response $response): Response { + if ($response instanceof TemplateResponse && $this->lockdownManager->isScoped()) { + return new Response(Http::STATUS_FORBIDDEN); + } + return $response; + } +} diff --git a/lib/private/Lockdown/LockdownManager.php b/lib/private/Lockdown/LockdownManager.php index ff4320f951f32..3ee8d121ff46f 100644 --- a/lib/private/Lockdown/LockdownManager.php +++ b/lib/private/Lockdown/LockdownManager.php @@ -76,6 +76,13 @@ public function canAccessFilesystem() { return !$scope || ($scope[IToken::SCOPE_FILESYSTEM] ?? false); } + /** + * Malformed scopes count as scoped, so they fail closed + */ + public function isScoped(): bool { + return $this->scopes() !== null; + } + /** * True for every scope when there is no token or it has no scopes (full access) */ diff --git a/tests/lib/AppFramework/DependencyInjection/DIContainerTest.php b/tests/lib/AppFramework/DependencyInjection/DIContainerTest.php index 8a9e2729b944d..f9d81d264be45 100644 --- a/tests/lib/AppFramework/DependencyInjection/DIContainerTest.php +++ b/tests/lib/AppFramework/DependencyInjection/DIContainerTest.php @@ -15,7 +15,9 @@ use OC\AppFramework\Bootstrap\RegistrationContext; use OC\AppFramework\DependencyInjection\DIContainer; use OC\AppFramework\Http\Request; +use OC\AppFramework\Middleware\AdditionalScriptsMiddleware; use OC\AppFramework\Middleware\Security\SecurityMiddleware; +use OC\AppFramework\Middleware\Security\TokenScopeMiddleware; use OCP\AppFramework\Middleware; use OCP\AppFramework\QueryException; use OCP\IConfig; @@ -72,6 +74,22 @@ public function testMiddlewareDispatcherIncludesSecurityMiddleware(): void { $this->assertTrue($found); } + public function testTokenScopeMiddlewareRefusesPagesBeforeTheirListenersRun(): void { + $this->container->registerService('Request', fn () => new Request( + ['method' => 'GET'], + $this->createMock(IRequestId::class), + $this->createMock(IConfig::class) + )); + $middlewares = $this->container->get('MiddlewareDispatcher')->getMiddlewares(); + $classes = array_map(get_class(...), $middlewares); + + // After-hooks run in reverse, so registering later means checking the page earlier + $this->assertGreaterThan( + array_search(AdditionalScriptsMiddleware::class, $classes, true), + array_search(TokenScopeMiddleware::class, $classes, true), + ); + } + public function testMiddlewareDispatcherIncludesBootstrapMiddlewares(): void { $coordinator = $this->createMock(Coordinator::class); $this->container->registerService(Coordinator::class, fn () => $coordinator); diff --git a/tests/lib/AppFramework/Middleware/Security/TokenScopeMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/TokenScopeMiddlewareTest.php new file mode 100644 index 0000000000000..61ee71d49d5ff --- /dev/null +++ b/tests/lib/AppFramework/Middleware/Security/TokenScopeMiddlewareTest.php @@ -0,0 +1,183 @@ +request = $this->createMock(IRequest::class); + $this->reflector = new ControllerMethodReflector(Server::get(LoggerInterface::class)); + } + + public static function requests(): array { + return [ + 'any token' => [[], TokenScopeTestController::class, 'anyToken', true], + 'scope missing' => [self::FILES, TokenScopeTestController::class, 'share', false], + 'scope held' => [[...self::FILES, FilesTokenScopes::FILES_SHARE], TokenScopeTestController::class, 'share', true], + 'unannotated' => [self::FILES, TokenScopeTestController::class, 'unannotated', false], + 'unannotated, legacy token' => [null, TokenScopeTestController::class, 'unannotated', true], + 'admin only' => [self::FILES, TokenScopeTestController::class, 'adminOnly', false], + 'class attribute' => [self::FILES, TokenScopeTestClassController::class, 'files', true], + 'class attribute, other scope' => [self::CALENDAR, TokenScopeTestClassController::class, 'files', false], + 'method overrides class' => [self::FILES, TokenScopeTestClassController::class, 'share', false], + ]; + } + + #[DataProvider('requests')] + public function testBeforeController(?array $scopes, string $class, string $method, bool $allowed): void { + $controller = new $class('test', $this->request); + $this->reflector->reflect($controller, $method); + + if (!$allowed) { + $this->expectException(TokenScopeException::class); + } + $this->middleware($scopes)->beforeController($controller, $method); + $this->addToAssertionCount(1); + } + + public function testPageRenderIsRefused(): void { + $response = $this->middleware(self::FILES) + ->afterController(new TokenScopeTestController('test', $this->request), 'anyToken', new TemplateResponse('files', 'index')); + + $this->assertNotInstanceOf(TemplateResponse::class, $response); + $this->assertSame(Http::STATUS_FORBIDDEN, $response->getStatus()); + } + + public function testOtherResponsesPass(): void { + $controller = new TokenScopeTestController('test', $this->request); + $json = new JSONResponse([]); + $this->assertSame($json, $this->middleware(self::FILES)->afterController($controller, 'anyToken', $json)); + + $page = new TemplateResponse('files', 'index'); + $this->assertSame($page, $this->middleware(null)->afterController($controller, 'anyToken', $page)); + } + + /** + * Without their own attribute these would fall back to their class's wider scope + */ + public static function pinnedScopes(): array { + return [ + 'create share' => [ShareAPIController::class, 'createShare', FilesTokenScopes::FILES_SHARE], + 'update share' => [ShareAPIController::class, 'updateShare', FilesTokenScopes::FILES_SHARE], + 'delete share' => [ShareAPIController::class, 'deleteShare', FilesTokenScopes::FILES_SHARE], + 'send share email' => [ShareAPIController::class, 'sendShareEmail', FilesTokenScopes::FILES_SHARE], + 'sharee search' => [ShareesAPIController::class, 'search', FilesTokenScopes::FILES_SHARE], + 'recommended sharees' => [ShareesAPIController::class, 'findRecommended', FilesTokenScopes::FILES_SHARE], + 'direct link' => [DirectController::class, 'getUrl', FilesTokenScopes::FILES_SHARE], + 'preview' => [PreviewController::class, 'getPreview', FilesTokenScopes::FILES_READ], + 'preview by file id' => [PreviewController::class, 'getPreviewByFileId', FilesTokenScopes::FILES_READ], + ]; + } + + #[DataProvider('pinnedScopes')] + public function testPinnedScopes(string $class, string $method, string $scope): void { + $attribute = (new \ReflectionMethod($class, $method))->getAttributes(TokenScope::class)[0] + ?? (new \ReflectionClass($class))->getAttributes(TokenScope::class)[0]; + $this->assertSame($scope, $attribute->newInstance()->scope); + } + + /** + * These hand files to other apps, so no scoped token may reach them + */ + public static function refusedControllers(): array { + return [ + [TemplateController::class], + [DirectEditingController::class], + [DirectEditingViewController::class], + [ConversionApiController::class], + ]; + } + + #[DataProvider('refusedControllers')] + public function testRefusedControllersStayUnannotated(string $class): void { + $class = new \ReflectionClass($class); + $this->assertSame([], $class->getAttributes(TokenScope::class)); + foreach ($class->getMethods(\ReflectionMethod::IS_PUBLIC) as $method) { + $this->assertSame([], $method->getAttributes(TokenScope::class), $method->getName()); + } + } + + private function middleware(?array $scopes): TokenScopeMiddleware { + $lockdownManager = new LockdownManager(fn () => $this->createMock(ISession::class), new TokenScopes()); + $token = new PublicKeyToken(); + $token->setScope($scopes === null ? [] : [TokenScopes::KEY => $scopes]); + $lockdownManager->setToken($token); + + return new TokenScopeMiddleware($this->reflector, $lockdownManager); + } +} diff --git a/tests/lib/Lockdown/LockdownManagerTest.php b/tests/lib/Lockdown/LockdownManagerTest.php index f3f5dd2a3c07e..9a2ecdd4a0c28 100644 --- a/tests/lib/Lockdown/LockdownManagerTest.php +++ b/tests/lib/Lockdown/LockdownManagerTest.php @@ -82,6 +82,25 @@ public function testHasScope(array $scope, string $requested, bool $expected): v $this->assertSame($expected, $this->managerWithScope($scope)->hasScope($requested)); } + public function testIsScopedWithoutToken(): void { + $manager = new LockdownManager($this->sessionCallback, new TokenScopes()); + $this->assertFalse($manager->isScoped()); + } + + public static function isScoped(): array { + return [ + 'legacy' => [[IToken::SCOPE_FILESYSTEM => true], false], + 'scoped' => [[TokenScopes::KEY => ['calendar:read']], true], + 'empty scopes' => [[TokenScopes::KEY => []], true], + 'malformed' => [[TokenScopes::KEY => 'calendar:read'], true], + ]; + } + + #[DataProvider('isScoped')] + public function testIsScoped(array $scope, bool $expected): void { + $this->assertSame($expected, $this->managerWithScope($scope)->isScoped()); + } + private function managerWithScope(array $scope): LockdownManager { $token = new PublicKeyToken(); $token->setScope($scope); From 32b83dab6d3091bc75c6ec424fe67a95b19d5b2e Mon Sep 17 00:00:00 2001 From: Peter Ringelmann Date: Thu, 8 Oct 2026 15:23:15 +0200 Subject: [PATCH 2/9] test(auth): cover scoped app passwords in the auth integration tests Signed-off-by: Peter Ringelmann --- build/integration/features/auth.feature | 26 ++++++++++++++ build/integration/features/bootstrap/Auth.php | 35 +++++++++++++++---- 2 files changed, 55 insertions(+), 6 deletions(-) diff --git a/build/integration/features/auth.feature b/build/integration/features/auth.feature index 4601221c808a9..c924c90ef2ce3 100644 --- a/build/integration/features/auth.feature +++ b/build/integration/features/auth.feature @@ -113,3 +113,29 @@ Feature: auth Scenario: Creating an unrestricted auth token with regular login should work When a new unrestricted client token is added Then the HTTP status code should be "200" + + # SCOPED TOKENS + Scenario: a files token reaches the files OCS API + Given a new client token limited to "files:read files:write" is added + When requesting "/ocs/v2.php/apps/files_sharing/api/v1/shares" with "GET" using scoped basic token auth + Then the HTTP status code should be "200" + + Scenario: a files token reaches capabilities + Given a new client token limited to "files:read files:write" is added + When requesting "/ocs/v2.php/cloud/capabilities" with "GET" using scoped basic token auth + Then the HTTP status code should be "200" + + Scenario: a files token without files:share cannot create shares + Given a new client token limited to "files:read files:write" is added + When requesting "/ocs/v2.php/apps/files_sharing/api/v1/shares" with "POST" using scoped basic token auth + Then the HTTP status code should be "403" + + Scenario: a files token is refused on unified search + Given a new client token limited to "files:read files:write" is added + When requesting "/ocs/v2.php/search/providers" with "GET" using scoped basic token auth + Then the HTTP status code should be "403" + + Scenario: a files token gets no page render + Given a new client token limited to "files:read files:write" is added + When requesting "/index.php/apps/files" with "GET" using scoped basic token auth + Then the HTTP status code should be "403" diff --git a/build/integration/features/bootstrap/Auth.php b/build/integration/features/bootstrap/Auth.php index a3f549769f2dc..de044003ac078 100644 --- a/build/integration/features/bootstrap/Auth.php +++ b/build/integration/features/bootstrap/Auth.php @@ -9,6 +9,7 @@ use GuzzleHttp\Cookie\CookieJar; use GuzzleHttp\Exception\ClientException; use GuzzleHttp\Exception\ServerException; +use PHPUnit\Framework\Assert; require __DIR__ . '/autoload.php'; @@ -17,6 +18,8 @@ trait Auth { private $unrestrictedClientToken; /** @var string */ private $restrictedClientToken; + /** @var string */ + private $scopedClientToken; /** @var Client */ private $client; /** @var string */ @@ -106,8 +109,22 @@ private function createClientToken($loginViaWeb = true) { */ public function aNewRestrictedClientTokenIsAdded() { $tokenObj = $this->createClientToken(); - $newCreatedTokenId = $tokenObj->deviceToken->id; - $fullUrl = substr($this->baseUrl, 0, -5) . '/index.php/settings/personal/authtokens/' . $newCreatedTokenId; + $this->updateClientTokenScope($tokenObj->deviceToken->id, ['filesystem' => false]); + $this->restrictedClientToken = $tokenObj->token; + } + + /** + * @Given a new client token limited to :scopes is added + */ + public function aNewClientTokenLimitedToIsAdded(string $scopes): void { + $tokenObj = $this->createClientToken(); + $this->updateClientTokenScope($tokenObj->deviceToken->id, ['scopes' => explode(' ', $scopes)]); + Assert::assertEquals(200, $this->response->getStatusCode()); + $this->scopedClientToken = $tokenObj->token; + } + + private function updateClientTokenScope(int $id, array $scope): void { + $fullUrl = substr($this->baseUrl, 0, -5) . '/index.php/settings/personal/authtokens/' . $id; $client = new Client(); $options = [ 'auth' => ['user0', '123456'], @@ -116,14 +133,11 @@ public function aNewRestrictedClientTokenIsAdded() { ], 'json' => [ 'name' => md5(microtime()), - 'scope' => [ - 'filesystem' => false, - ], + 'scope' => $scope, ], 'cookies' => $this->cookieJar, ]; $this->response = $client->request('PUT', $fullUrl, $options); - $this->restrictedClientToken = $tokenObj->token; } /** @@ -170,6 +184,15 @@ public function requestingWithRestrictedBasicTokenAuth($url, $method) { $this->sendRequest($url, $method, 'basic ' . base64_encode('user0:' . $this->restrictedClientToken), true); } + /** + * @When requesting :url with :method using scoped basic token auth + * + * No cookies: the browser session that created the token would win over it + */ + public function requestingWithScopedBasicTokenAuth(string $url, string $method): void { + $this->sendRequest($url, $method, 'basic ' . base64_encode('user0:' . $this->scopedClientToken)); + } + /** * @When requesting :url with :method using an unrestricted client token * From 46ba4396909eb16674e8ef1293047a3ff6d8b069 Mon Sep 17 00:00:00 2001 From: Peter Ringelmann Date: Fri, 16 Oct 2026 12:55:01 +0200 Subject: [PATCH 3/9] refactor(auth): use the app-owned scope names on routes Signed-off-by: Peter Ringelmann --- apps/dav/lib/Controller/DirectController.php | 4 ++-- apps/dav/lib/TokenScopes.php | 8 +++---- .../lib/Controller/ShareAPIController.php | 9 +++---- .../lib/Controller/ShareesAPIController.php | 4 ++-- build/integration/features/auth.feature | 12 +++++----- .../Security/TokenScopeMiddlewareTest.php | 24 ++++++++++--------- tests/lib/Lockdown/LockdownManagerTest.php | 4 ++-- 7 files changed, 34 insertions(+), 31 deletions(-) diff --git a/apps/dav/lib/Controller/DirectController.php b/apps/dav/lib/Controller/DirectController.php index dd587908d4366..1943a6cae0e63 100644 --- a/apps/dav/lib/Controller/DirectController.php +++ b/apps/dav/lib/Controller/DirectController.php @@ -12,7 +12,6 @@ use OC\AppFramework\Http\Attributes\TokenScope; use OCA\DAV\Db\Direct; use OCA\DAV\Db\DirectMapper; -use OCA\Files\TokenScopes; use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\NoAdminRequired; use OCP\AppFramework\Http\DataResponse; @@ -60,7 +59,8 @@ public function __construct( * 200: Direct link returned */ #[NoAdminRequired] - #[TokenScope(scope: TokenScopes::FILES_SHARE)] + // Not OCA\Files_Sharing\TokenScopes::SHARES_WRITE: files_sharing can be disabled, and its classes don't load then + #[TokenScope(scope: 'files_sharing:shares:write')] public function getUrl(int $fileId, int $expirationTime = 60 * 60 * 8): DataResponse { if (!$this->shareManager->shareApiAllowLinks()) { throw new OCSForbiddenException('Creating direct links is disabled'); diff --git a/apps/dav/lib/TokenScopes.php b/apps/dav/lib/TokenScopes.php index 1e2ebec9c03ee..f7510d2570d80 100644 --- a/apps/dav/lib/TokenScopes.php +++ b/apps/dav/lib/TokenScopes.php @@ -13,8 +13,8 @@ * App password scopes for calendars and address books */ class TokenScopes { - public const CALENDAR_READ = 'calendar:read'; - public const CALENDAR_WRITE = 'calendar:write'; - public const CONTACTS_READ = 'contacts:read'; - public const CONTACTS_WRITE = 'contacts:write'; + public const CALENDAR_READ = 'dav:calendar:read'; + public const CALENDAR_WRITE = 'dav:calendar:write'; + public const CONTACTS_READ = 'dav:contacts:read'; + public const CONTACTS_WRITE = 'dav:contacts:write'; } diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index ab7510546456a..5dc98f0c5a092 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -22,6 +22,7 @@ use OCA\Files_Sharing\External\Storage; use OCA\Files_Sharing\ResponseDefinitions; use OCA\Files_Sharing\SharedStorage; +use OCA\Files_Sharing\TokenScopes as SharingTokenScopes; use OCA\GlobalSiteSelector\Service\SlaveService; use OCP\App\IAppManager; use OCP\AppFramework\Http; @@ -550,7 +551,7 @@ public function getShare(string $id, bool $include_tags = false): DataResponse { * 200: Share deleted successfully */ #[NoAdminRequired] - #[TokenScope(scope: TokenScopes::FILES_SHARE)] + #[TokenScope(scope: SharingTokenScopes::SHARES_WRITE)] public function deleteShare(string $id): DataResponse { try { $share = $this->getShareById($id); @@ -615,7 +616,7 @@ public function deleteShare(string $id): DataResponse { */ #[NoAdminRequired] #[UserRateLimit(limit: 60, period: 600)] - #[TokenScope(scope: TokenScopes::FILES_SHARE)] + #[TokenScope(scope: SharingTokenScopes::SHARES_WRITE)] public function createShare( ?string $path = null, ?int $permissions = null, @@ -1252,7 +1253,7 @@ private function hasPermission(int $permissionsSet, int $permissionsToCheck): bo * 200: Share updated successfully */ #[NoAdminRequired] - #[TokenScope(scope: TokenScopes::FILES_SHARE)] + #[TokenScope(scope: SharingTokenScopes::SHARES_WRITE)] public function updateShare( string $id, ?int $permissions = null, @@ -2166,7 +2167,7 @@ private function checkInheritedAttributes(IShare $share): void { */ #[NoAdminRequired] #[UserRateLimit(limit: 10, period: 600)] - #[TokenScope(scope: TokenScopes::FILES_SHARE)] + #[TokenScope(scope: SharingTokenScopes::SHARES_WRITE)] public function sendShareEmail(string $id, $password = ''): DataResponse { try { $share = $this->getShareById($id); diff --git a/apps/files_sharing/lib/Controller/ShareesAPIController.php b/apps/files_sharing/lib/Controller/ShareesAPIController.php index 093589b3bfd36..0a1f1043556f1 100644 --- a/apps/files_sharing/lib/Controller/ShareesAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareesAPIController.php @@ -13,8 +13,8 @@ use OC\AppFramework\Http\Attributes\TokenScope; use OC\Collaboration\Collaborators\SearchResult; use OCA\FederatedFileSharing\FederatedShareProvider; -use OCA\Files\TokenScopes; use OCA\Files_Sharing\ResponseDefinitions; +use OCA\Files_Sharing\TokenScopes; use OCP\App\IAppManager; use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\NoAdminRequired; @@ -40,7 +40,7 @@ * @psalm-import-type Files_SharingShareesSearchResult from ResponseDefinitions * @psalm-import-type Files_SharingShareesRecommendedResult from ResponseDefinitions */ -#[TokenScope(scope: TokenScopes::FILES_SHARE)] +#[TokenScope(scope: TokenScopes::SHARES_WRITE)] class ShareesAPIController extends OCSController { protected int $offset = 0; diff --git a/build/integration/features/auth.feature b/build/integration/features/auth.feature index c924c90ef2ce3..3eadae6d2f6a2 100644 --- a/build/integration/features/auth.feature +++ b/build/integration/features/auth.feature @@ -116,26 +116,26 @@ Feature: auth # SCOPED TOKENS Scenario: a files token reaches the files OCS API - Given a new client token limited to "files:read files:write" is added + Given a new client token limited to "files:files:read files:files:write" is added When requesting "/ocs/v2.php/apps/files_sharing/api/v1/shares" with "GET" using scoped basic token auth Then the HTTP status code should be "200" Scenario: a files token reaches capabilities - Given a new client token limited to "files:read files:write" is added + Given a new client token limited to "files:files:read files:files:write" is added When requesting "/ocs/v2.php/cloud/capabilities" with "GET" using scoped basic token auth Then the HTTP status code should be "200" - Scenario: a files token without files:share cannot create shares - Given a new client token limited to "files:read files:write" is added + Scenario: a files token without files_sharing:shares:write cannot create shares + Given a new client token limited to "files:files:read files:files:write" is added When requesting "/ocs/v2.php/apps/files_sharing/api/v1/shares" with "POST" using scoped basic token auth Then the HTTP status code should be "403" Scenario: a files token is refused on unified search - Given a new client token limited to "files:read files:write" is added + Given a new client token limited to "files:files:read files:files:write" is added When requesting "/ocs/v2.php/search/providers" with "GET" using scoped basic token auth Then the HTTP status code should be "403" Scenario: a files token gets no page render - Given a new client token limited to "files:read files:write" is added + Given a new client token limited to "files:files:read files:files:write" is added When requesting "/index.php/apps/files" with "GET" using scoped basic token auth Then the HTTP status code should be "403" diff --git a/tests/lib/AppFramework/Middleware/Security/TokenScopeMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/TokenScopeMiddlewareTest.php index 61ee71d49d5ff..89e7d857d34f1 100644 --- a/tests/lib/AppFramework/Middleware/Security/TokenScopeMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/TokenScopeMiddlewareTest.php @@ -18,6 +18,7 @@ use OC\Core\Controller\PreviewController; use OC\Lockdown\LockdownManager; use OCA\DAV\Controller\DirectController; +use OCA\DAV\TokenScopes as DavTokenScopes; use OCA\Files\Controller\ConversionApiController; use OCA\Files\Controller\DirectEditingController; use OCA\Files\Controller\DirectEditingViewController; @@ -25,6 +26,7 @@ use OCA\Files\TokenScopes as FilesTokenScopes; use OCA\Files_Sharing\Controller\ShareAPIController; use OCA\Files_Sharing\Controller\ShareesAPIController; +use OCA\Files_Sharing\TokenScopes as SharingTokenScopes; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\NoAdminRequired; @@ -45,7 +47,7 @@ public function anyToken(): void { } #[NoAdminRequired] - #[TokenScope(scope: FilesTokenScopes::FILES_SHARE)] + #[TokenScope(scope: SharingTokenScopes::SHARES_WRITE)] public function share(): void { } @@ -65,14 +67,14 @@ public function files(): void { } #[NoAdminRequired] - #[TokenScope(scope: FilesTokenScopes::FILES_SHARE)] + #[TokenScope(scope: SharingTokenScopes::SHARES_WRITE)] public function share(): void { } } class TokenScopeMiddlewareTest extends TestCase { private const FILES = [FilesTokenScopes::FILES_READ, FilesTokenScopes::FILES_WRITE]; - private const CALENDAR = ['calendar:read', 'calendar:write']; + private const CALENDAR = [DavTokenScopes::CALENDAR_READ, DavTokenScopes::CALENDAR_WRITE]; private IRequest&MockObject $request; private ControllerMethodReflector $reflector; @@ -88,7 +90,7 @@ public static function requests(): array { return [ 'any token' => [[], TokenScopeTestController::class, 'anyToken', true], 'scope missing' => [self::FILES, TokenScopeTestController::class, 'share', false], - 'scope held' => [[...self::FILES, FilesTokenScopes::FILES_SHARE], TokenScopeTestController::class, 'share', true], + 'scope held' => [[...self::FILES, SharingTokenScopes::SHARES_WRITE], TokenScopeTestController::class, 'share', true], 'unannotated' => [self::FILES, TokenScopeTestController::class, 'unannotated', false], 'unannotated, legacy token' => [null, TokenScopeTestController::class, 'unannotated', true], 'admin only' => [self::FILES, TokenScopeTestController::class, 'adminOnly', false], @@ -132,13 +134,13 @@ public function testOtherResponsesPass(): void { */ public static function pinnedScopes(): array { return [ - 'create share' => [ShareAPIController::class, 'createShare', FilesTokenScopes::FILES_SHARE], - 'update share' => [ShareAPIController::class, 'updateShare', FilesTokenScopes::FILES_SHARE], - 'delete share' => [ShareAPIController::class, 'deleteShare', FilesTokenScopes::FILES_SHARE], - 'send share email' => [ShareAPIController::class, 'sendShareEmail', FilesTokenScopes::FILES_SHARE], - 'sharee search' => [ShareesAPIController::class, 'search', FilesTokenScopes::FILES_SHARE], - 'recommended sharees' => [ShareesAPIController::class, 'findRecommended', FilesTokenScopes::FILES_SHARE], - 'direct link' => [DirectController::class, 'getUrl', FilesTokenScopes::FILES_SHARE], + 'create share' => [ShareAPIController::class, 'createShare', SharingTokenScopes::SHARES_WRITE], + 'update share' => [ShareAPIController::class, 'updateShare', SharingTokenScopes::SHARES_WRITE], + 'delete share' => [ShareAPIController::class, 'deleteShare', SharingTokenScopes::SHARES_WRITE], + 'send share email' => [ShareAPIController::class, 'sendShareEmail', SharingTokenScopes::SHARES_WRITE], + 'sharee search' => [ShareesAPIController::class, 'search', SharingTokenScopes::SHARES_WRITE], + 'recommended sharees' => [ShareesAPIController::class, 'findRecommended', SharingTokenScopes::SHARES_WRITE], + 'direct link' => [DirectController::class, 'getUrl', SharingTokenScopes::SHARES_WRITE], 'preview' => [PreviewController::class, 'getPreview', FilesTokenScopes::FILES_READ], 'preview by file id' => [PreviewController::class, 'getPreviewByFileId', FilesTokenScopes::FILES_READ], ]; diff --git a/tests/lib/Lockdown/LockdownManagerTest.php b/tests/lib/Lockdown/LockdownManagerTest.php index 9a2ecdd4a0c28..c27c2ca6e2850 100644 --- a/tests/lib/Lockdown/LockdownManagerTest.php +++ b/tests/lib/Lockdown/LockdownManagerTest.php @@ -90,9 +90,9 @@ public function testIsScopedWithoutToken(): void { public static function isScoped(): array { return [ 'legacy' => [[IToken::SCOPE_FILESYSTEM => true], false], - 'scoped' => [[TokenScopes::KEY => ['calendar:read']], true], + 'scoped' => [[TokenScopes::KEY => ['dav:calendar:read']], true], 'empty scopes' => [[TokenScopes::KEY => []], true], - 'malformed' => [[TokenScopes::KEY => 'calendar:read'], true], + 'malformed' => [[TokenScopes::KEY => 'dav:calendar:read'], true], ]; } From 16ab79d20498b4f71374d12a9108518b198c53ce Mon Sep 17 00:00:00 2001 From: Peter Ringelmann Date: Tue, 20 Oct 2026 15:32:27 +0200 Subject: [PATCH 4/9] refactor(auth): mark routes open to every app password with NoTokenScopeRequired Signed-off-by: Peter Ringelmann --- .../lib/Controller/OauthApiController.php | 4 ++-- .../lib/Controller/UsersController.php | 4 ++-- .../theming/lib/Controller/IconController.php | 8 +++---- .../lib/Controller/ThemingController.php | 8 +++---- .../lib/Controller/UserThemeController.php | 4 ++-- core/Controller/AppPasswordController.php | 6 ++--- core/Controller/AvatarController.php | 6 ++--- core/Controller/GuestAvatarController.php | 6 ++--- core/Controller/OCSController.php | 4 ++-- core/Controller/PreviewController.php | 3 ++- core/Controller/WipeController.php | 6 ++--- lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + .../Http/Attributes/NoTokenScopeRequired.php | 22 +++++++++++++++++++ .../Http/Attributes/TokenScope.php | 7 +++--- .../Security/TokenScopeMiddleware.php | 15 +++++++++---- .../Security/TokenScopeMiddlewareTest.php | 19 +++++++++++----- 17 files changed, 83 insertions(+), 41 deletions(-) create mode 100644 lib/private/AppFramework/Http/Attributes/NoTokenScopeRequired.php diff --git a/apps/oauth2/lib/Controller/OauthApiController.php b/apps/oauth2/lib/Controller/OauthApiController.php index 4c23bda6f3290..6a8f314f1776d 100644 --- a/apps/oauth2/lib/Controller/OauthApiController.php +++ b/apps/oauth2/lib/Controller/OauthApiController.php @@ -9,7 +9,7 @@ namespace OCA\OAuth2\Controller; -use OC\AppFramework\Http\Attributes\TokenScope; +use OC\AppFramework\Http\Attributes\NoTokenScopeRequired; use OC\Authentication\Token\IProvider as TokenProvider; use OCA\OAuth2\Db\AccessTokenMapper; use OCA\OAuth2\Db\ClientMapper; @@ -84,7 +84,7 @@ public function __construct( #[PublicPage] #[NoCSRFRequired] #[BruteForceProtection(action: 'oauth2GetToken')] - #[TokenScope] + #[NoTokenScopeRequired] public function getToken( string $grant_type, ?string $code, ?string $refresh_token, ?string $client_id, ?string $client_secret, diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index d30eb9a882868..003b8f934d7b7 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -11,7 +11,7 @@ namespace OCA\Provisioning_API\Controller; use InvalidArgumentException; -use OC\AppFramework\Http\Attributes\TokenScope; +use OC\AppFramework\Http\Attributes\NoTokenScopeRequired; use OC\AppFramework\Http\PaginationTrait; use OC\Authentication\Token\RemoteWipe; use OC\Group\DisplayNameCache as GroupDisplayNameCache; @@ -706,7 +706,7 @@ public function getUser(string $userId): DataResponse { */ #[NoAdminRequired] #[NoSubAdminRequired] - #[TokenScope] + #[NoTokenScopeRequired] public function getCurrentUser(): DataResponse { $user = $this->userSession->getUser(); if ($user) { diff --git a/apps/theming/lib/Controller/IconController.php b/apps/theming/lib/Controller/IconController.php index 38ea0d93a3adc..371ec99a8b3c1 100644 --- a/apps/theming/lib/Controller/IconController.php +++ b/apps/theming/lib/Controller/IconController.php @@ -7,7 +7,7 @@ namespace OCA\Theming\Controller; -use OC\AppFramework\Http\Attributes\TokenScope; +use OC\AppFramework\Http\Attributes\NoTokenScopeRequired; use OC\IntegrityCheck\Helpers\FileAccessHelper; use OCA\Theming\IconBuilder; use OCA\Theming\ImageManager; @@ -54,7 +54,7 @@ public function __construct( #[PublicPage] #[NoCSRFRequired] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] - #[TokenScope] + #[NoTokenScopeRequired] public function getThemedIcon(string $app, string $image): Response { if ($app !== 'core' && !$this->appManager->isEnabledForUser($app)) { $app = 'core'; @@ -89,7 +89,7 @@ public function getThemedIcon(string $app, string $image): Response { #[PublicPage] #[NoCSRFRequired] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] - #[TokenScope] + #[NoTokenScopeRequired] public function getFavicon(string $app = 'core'): Response { if ($app !== 'core' && !$this->appManager->isEnabledForUser($app)) { $app = 'core'; @@ -139,7 +139,7 @@ public function getFavicon(string $app = 'core'): Response { #[PublicPage] #[NoCSRFRequired] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] - #[TokenScope] + #[NoTokenScopeRequired] public function getTouchIcon(string $app = 'core'): Response { if ($app !== 'core' && !$this->appManager->isEnabledForUser($app)) { $app = 'core'; diff --git a/apps/theming/lib/Controller/ThemingController.php b/apps/theming/lib/Controller/ThemingController.php index daa12b94d5977..619160c445c68 100644 --- a/apps/theming/lib/Controller/ThemingController.php +++ b/apps/theming/lib/Controller/ThemingController.php @@ -8,7 +8,7 @@ namespace OCA\Theming\Controller; use InvalidArgumentException; -use OC\AppFramework\Http\Attributes\TokenScope; +use OC\AppFramework\Http\Attributes\NoTokenScopeRequired; use OCA\Theming\ImageManager; use OCA\Theming\Service\ThemesService; use OCA\Theming\Settings\Admin; @@ -361,7 +361,7 @@ public function undoAll(): DataResponse { #[PublicPage] #[NoCSRFRequired] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] - #[TokenScope] + #[NoTokenScopeRequired] public function getImage(string $key, bool $useSvg = true) { try { $useSvg = $useSvg && $this->imageManager->canConvert('SVG'); @@ -402,7 +402,7 @@ public function getImage(string $key, bool $useSvg = true) { #[NoTwoFactorRequired] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] #[NoSameSiteCookieRequired] - #[TokenScope] + #[NoTokenScopeRequired] public function getThemeStylesheet(string $themeId, bool $plain = false, bool $withCustomCss = false) { $themes = $this->themesService->getThemes(); if (!in_array($themeId, array_keys($themes), true)) { @@ -459,7 +459,7 @@ public function getThemeStylesheet(string $themeId, bool $plain = false, bool $w #[NoCSRFRequired] #[BruteForceProtection(action: 'manifest')] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] - #[TokenScope] + #[NoTokenScopeRequired] public function getManifest(string $app): JSONResponse { $cacheBusterValue = (string)$this->appConfig->getAppValueInt('cachebuster'); if ($app === 'core' || $app === 'settings') { diff --git a/apps/theming/lib/Controller/UserThemeController.php b/apps/theming/lib/Controller/UserThemeController.php index ef4f7c9b65a29..f07b4102aff46 100644 --- a/apps/theming/lib/Controller/UserThemeController.php +++ b/apps/theming/lib/Controller/UserThemeController.php @@ -9,7 +9,7 @@ namespace OCA\Theming\Controller; -use OC\AppFramework\Http\Attributes\TokenScope; +use OC\AppFramework\Http\Attributes\NoTokenScopeRequired; use OCA\Theming\AppInfo\Application; use OCA\Theming\ITheme; use OCA\Theming\ResponseDefinitions; @@ -134,7 +134,7 @@ private function validateTheme(string $themeId): ITheme { #[NoAdminRequired] #[NoCSRFRequired] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] - #[TokenScope] + #[NoTokenScopeRequired] public function getBackground(): Response { $file = $this->backgroundService->getBackground(); if ($file !== null) { diff --git a/core/Controller/AppPasswordController.php b/core/Controller/AppPasswordController.php index a641a3488f377..beab42394abb6 100644 --- a/core/Controller/AppPasswordController.php +++ b/core/Controller/AppPasswordController.php @@ -9,7 +9,7 @@ namespace OC\Core\Controller; -use OC\AppFramework\Http\Attributes\TokenScope; +use OC\AppFramework\Http\Attributes\NoTokenScopeRequired; use OC\Authentication\Events\AppPasswordCreatedEvent; use OC\Authentication\Token\IProvider; use OC\Authentication\Token\IToken; @@ -121,7 +121,7 @@ public function getAppPassword(): DataResponse { */ #[NoAdminRequired] #[ApiRoute(verb: 'DELETE', url: '/apppassword', root: '/core')] - #[TokenScope] + #[NoTokenScopeRequired] public function deleteAppPassword(): DataResponse { if (!$this->session->exists('app_password')) { throw new OCSForbiddenException('no app password in use'); @@ -149,7 +149,7 @@ public function deleteAppPassword(): DataResponse { */ #[NoAdminRequired] #[ApiRoute(verb: 'POST', url: '/apppassword/rotate', root: '/core')] - #[TokenScope] + #[NoTokenScopeRequired] public function rotateAppPassword(): DataResponse { if (!$this->session->exists('app_password')) { throw new OCSForbiddenException('no app password in use'); diff --git a/core/Controller/AvatarController.php b/core/Controller/AvatarController.php index 36c62b6d7a644..155f2de3ab919 100644 --- a/core/Controller/AvatarController.php +++ b/core/Controller/AvatarController.php @@ -8,7 +8,7 @@ namespace OC\Core\Controller; -use OC\AppFramework\Http\Attributes\TokenScope; +use OC\AppFramework\Http\Attributes\NoTokenScopeRequired; use OC\AppFramework\Utility\TimeFactory; use OC\Avatar\AvatarManager; use OCP\AppFramework\Controller; @@ -86,7 +86,7 @@ public function __construct( #[FrontpageRoute(verb: 'GET', url: '/avatar/{userId}/{size}/dark')] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] #[NoSameSiteCookieRequired] - #[TokenScope] + #[NoTokenScopeRequired] public function getAvatarDark(string $userId, int $size, bool $guestFallback = false, string $v = '') { if ($size <= 64) { if ($size !== 64) { @@ -137,7 +137,7 @@ public function getAvatarDark(string $userId, int $size, bool $guestFallback = f #[FrontpageRoute(verb: 'GET', url: '/avatar/{userId}/{size}')] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] #[NoSameSiteCookieRequired] - #[TokenScope] + #[NoTokenScopeRequired] public function getAvatar(string $userId, int $size, bool $guestFallback = false, string $v = '') { if ($size <= 64) { if ($size !== 64) { diff --git a/core/Controller/GuestAvatarController.php b/core/Controller/GuestAvatarController.php index 8115ad881b90e..e2f6d87547d97 100644 --- a/core/Controller/GuestAvatarController.php +++ b/core/Controller/GuestAvatarController.php @@ -7,7 +7,7 @@ namespace OC\Core\Controller; -use OC\AppFramework\Http\Attributes\TokenScope; +use OC\AppFramework\Http\Attributes\NoTokenScopeRequired; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\FrontpageRoute; @@ -51,7 +51,7 @@ public function __construct( #[NoCSRFRequired] #[FrontpageRoute(verb: 'GET', url: '/avatar/guest/{guestName}/{size}')] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] - #[TokenScope] + #[NoTokenScopeRequired] public function getAvatar(string $guestName, int $size, ?bool $darkTheme = false) { $darkTheme = $darkTheme ?? false; @@ -104,7 +104,7 @@ public function getAvatar(string $guestName, int $size, ?bool $darkTheme = false #[NoCSRFRequired] #[FrontpageRoute(verb: 'GET', url: '/avatar/guest/{guestName}/{size}/dark')] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] - #[TokenScope] + #[NoTokenScopeRequired] public function getAvatarDark(string $guestName, int $size) { return $this->getAvatar($guestName, $size, true); } diff --git a/core/Controller/OCSController.php b/core/Controller/OCSController.php index e03d05e851380..f47618d7030ce 100644 --- a/core/Controller/OCSController.php +++ b/core/Controller/OCSController.php @@ -7,7 +7,7 @@ namespace OC\Core\Controller; -use OC\AppFramework\Http\Attributes\TokenScope; +use OC\AppFramework\Http\Attributes\NoTokenScopeRequired; use OC\CapabilitiesManager; use OC\Security\IdentityProof\Manager; use OCP\AppFramework\Http; @@ -63,7 +63,7 @@ public function getConfig(): DataResponse { */ #[PublicPage] #[ApiRoute(verb: 'GET', url: '/capabilities', root: '/cloud')] - #[TokenScope] + #[NoTokenScopeRequired] public function getCapabilities(): DataResponse { $result = []; $result['version'] = [ diff --git a/core/Controller/PreviewController.php b/core/Controller/PreviewController.php index c2d86d3d1858f..6bb2a71104ff8 100644 --- a/core/Controller/PreviewController.php +++ b/core/Controller/PreviewController.php @@ -9,6 +9,7 @@ namespace OC\Core\Controller; +use OC\AppFramework\Http\Attributes\NoTokenScopeRequired; use OC\AppFramework\Http\Attributes\TokenScope; use OCA\Files\TokenScopes; use OCP\AppFramework\Controller; @@ -197,7 +198,7 @@ private function fetchPreview( #[PublicPage] #[FrontpageRoute(verb: 'GET', url: '/core/mimeicon')] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] - #[TokenScope] + #[NoTokenScopeRequired] public function getMimeIconUrl(string $mime = 'application/octet-stream') { $url = $this->mimeIconProvider->getMimeIconUrl($mime); if ($url === null) { diff --git a/core/Controller/WipeController.php b/core/Controller/WipeController.php index bacc4897a4d6b..0d98e0bc013ab 100644 --- a/core/Controller/WipeController.php +++ b/core/Controller/WipeController.php @@ -9,7 +9,7 @@ namespace OC\Core\Controller; -use OC\AppFramework\Http\Attributes\TokenScope; +use OC\AppFramework\Http\Attributes\NoTokenScopeRequired; use OC\Authentication\Token\RemoteWipe; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; @@ -46,7 +46,7 @@ public function __construct( #[NoCSRFRequired] #[AnonRateLimit(limit: 10, period: 300)] #[FrontpageRoute(verb: 'POST', url: '/core/wipe/check')] - #[TokenScope] + #[NoTokenScopeRequired] public function checkWipe(string $token): JSONResponse { try { if ($this->remoteWipe->start($token)) { @@ -75,7 +75,7 @@ public function checkWipe(string $token): JSONResponse { #[NoCSRFRequired] #[AnonRateLimit(limit: 10, period: 300)] #[FrontpageRoute(verb: 'POST', url: '/core/wipe/success')] - #[TokenScope] + #[NoTokenScopeRequired] public function wipeDone(string $token): JSONResponse { try { if ($this->remoteWipe->finish($token)) { diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 1745eff0ff8a5..f4522a00ad2ed 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1221,6 +1221,7 @@ 'OC\\AppFramework\\DependencyInjection\\DIContainer' => $baseDir . '/lib/private/AppFramework/DependencyInjection/DIContainer.php', 'OC\\AppFramework\\Http' => $baseDir . '/lib/private/AppFramework/Http.php', 'OC\\AppFramework\\Http\\Attributes\\FederationRateLimit' => $baseDir . '/lib/private/AppFramework/Http/Attributes/FederationRateLimit.php', + 'OC\\AppFramework\\Http\\Attributes\\NoTokenScopeRequired' => $baseDir . '/lib/private/AppFramework/Http/Attributes/NoTokenScopeRequired.php', 'OC\\AppFramework\\Http\\Attributes\\TokenScope' => $baseDir . '/lib/private/AppFramework/Http/Attributes/TokenScope.php', 'OC\\AppFramework\\Http\\Attributes\\TwoFactorSetUpDoneRequired' => $baseDir . '/lib/private/AppFramework/Http/Attributes/TwoFactorSetUpDoneRequired.php', 'OC\\AppFramework\\Http\\Dispatcher' => $baseDir . '/lib/private/AppFramework/Http/Dispatcher.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 634ae9e58e7ac..b490ecbe299c4 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1262,6 +1262,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\AppFramework\\DependencyInjection\\DIContainer' => __DIR__ . '/../../..' . '/lib/private/AppFramework/DependencyInjection/DIContainer.php', 'OC\\AppFramework\\Http' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Http.php', 'OC\\AppFramework\\Http\\Attributes\\FederationRateLimit' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Http/Attributes/FederationRateLimit.php', + 'OC\\AppFramework\\Http\\Attributes\\NoTokenScopeRequired' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Http/Attributes/NoTokenScopeRequired.php', 'OC\\AppFramework\\Http\\Attributes\\TokenScope' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Http/Attributes/TokenScope.php', 'OC\\AppFramework\\Http\\Attributes\\TwoFactorSetUpDoneRequired' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Http/Attributes/TwoFactorSetUpDoneRequired.php', 'OC\\AppFramework\\Http\\Dispatcher' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Http/Dispatcher.php', diff --git a/lib/private/AppFramework/Http/Attributes/NoTokenScopeRequired.php b/lib/private/AppFramework/Http/Attributes/NoTokenScopeRequired.php new file mode 100644 index 0000000000000..3b27c82750163 --- /dev/null +++ b/lib/private/AppFramework/Http/Attributes/NoTokenScopeRequired.php @@ -0,0 +1,22 @@ +tokenScope($controller); - if ($attribute === null - || ($attribute->scope !== null && !$this->lockdownManager->hasScope($attribute->scope))) { + if ($attribute instanceof NoTokenScopeRequired) { + return; + } + if ($attribute === null || !$this->lockdownManager->hasScope($attribute->scope)) { throw new TokenScopeException(); } } @@ -51,9 +54,13 @@ public function beforeController(Controller $controller, string $methodName): vo /** * The method's attribute, else its class's */ - private function tokenScope(Controller $controller): ?TokenScope { + private function tokenScope(Controller $controller): TokenScope|NoTokenScopeRequired|null { + $class = new ReflectionClass($controller); $attribute = $this->reflector->getAttribute(TokenScope::class) - ?? (new ReflectionClass($controller))->getAttributes(TokenScope::class)[0] ?? null; + ?? $this->reflector->getAttribute(NoTokenScopeRequired::class) + ?? $class->getAttributes(TokenScope::class)[0] + ?? $class->getAttributes(NoTokenScopeRequired::class)[0] + ?? null; return $attribute?->newInstance(); } diff --git a/tests/lib/AppFramework/Middleware/Security/TokenScopeMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/TokenScopeMiddlewareTest.php index 89e7d857d34f1..218b0b26c413d 100644 --- a/tests/lib/AppFramework/Middleware/Security/TokenScopeMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/TokenScopeMiddlewareTest.php @@ -9,6 +9,7 @@ namespace Test\AppFramework\Middleware\Security; +use OC\AppFramework\Http\Attributes\NoTokenScopeRequired; use OC\AppFramework\Http\Attributes\TokenScope; use OC\AppFramework\Middleware\Security\Exceptions\TokenScopeException; use OC\AppFramework\Middleware\Security\TokenScopeMiddleware; @@ -42,7 +43,7 @@ class TokenScopeTestController extends Controller { #[NoAdminRequired] - #[TokenScope] + #[NoTokenScopeRequired] public function anyToken(): void { } @@ -55,7 +56,7 @@ public function share(): void { public function unannotated(): void { } - #[TokenScope] + #[NoTokenScopeRequired] public function adminOnly(): void { } } @@ -70,6 +71,11 @@ public function files(): void { #[TokenScope(scope: SharingTokenScopes::SHARES_WRITE)] public function share(): void { } + + #[NoAdminRequired] + #[NoTokenScopeRequired] + public function anyToken(): void { + } } class TokenScopeMiddlewareTest extends TestCase { @@ -97,6 +103,7 @@ public static function requests(): array { 'class attribute' => [self::FILES, TokenScopeTestClassController::class, 'files', true], 'class attribute, other scope' => [self::CALENDAR, TokenScopeTestClassController::class, 'files', false], 'method overrides class' => [self::FILES, TokenScopeTestClassController::class, 'share', false], + 'method opens what the class limits' => [self::CALENDAR, TokenScopeTestClassController::class, 'anyToken', true], ]; } @@ -168,9 +175,11 @@ public static function refusedControllers(): array { #[DataProvider('refusedControllers')] public function testRefusedControllersStayUnannotated(string $class): void { $class = new \ReflectionClass($class); - $this->assertSame([], $class->getAttributes(TokenScope::class)); - foreach ($class->getMethods(\ReflectionMethod::IS_PUBLIC) as $method) { - $this->assertSame([], $method->getAttributes(TokenScope::class), $method->getName()); + foreach ([TokenScope::class, NoTokenScopeRequired::class] as $attribute) { + $this->assertSame([], $class->getAttributes($attribute)); + foreach ($class->getMethods(\ReflectionMethod::IS_PUBLIC) as $method) { + $this->assertSame([], $method->getAttributes($attribute), $method->getName()); + } } } From 787713f2fffb5acf6a3526dc2a2ed48eec4c0cae Mon Sep 17 00:00:00 2001 From: Peter Ringelmann Date: Tue, 20 Oct 2026 16:55:08 +0200 Subject: [PATCH 5/9] fix(auth): keep only the app password routes open to every scoped token Signed-off-by: Peter Ringelmann --- apps/oauth2/lib/Controller/OauthApiController.php | 2 -- apps/provisioning_api/lib/Controller/UsersController.php | 2 -- apps/theming/lib/Controller/IconController.php | 4 ---- apps/theming/lib/Controller/ThemingController.php | 4 ---- apps/theming/lib/Controller/UserThemeController.php | 2 -- build/integration/features/auth.feature | 4 ++-- core/Controller/AvatarController.php | 3 --- core/Controller/GuestAvatarController.php | 3 --- core/Controller/OCSController.php | 2 -- core/Controller/PreviewController.php | 2 -- core/Controller/WipeController.php | 3 --- .../AppFramework/Http/Attributes/NoTokenScopeRequired.php | 2 +- 12 files changed, 3 insertions(+), 30 deletions(-) diff --git a/apps/oauth2/lib/Controller/OauthApiController.php b/apps/oauth2/lib/Controller/OauthApiController.php index 6a8f314f1776d..8dc05851cf801 100644 --- a/apps/oauth2/lib/Controller/OauthApiController.php +++ b/apps/oauth2/lib/Controller/OauthApiController.php @@ -9,7 +9,6 @@ namespace OCA\OAuth2\Controller; -use OC\AppFramework\Http\Attributes\NoTokenScopeRequired; use OC\Authentication\Token\IProvider as TokenProvider; use OCA\OAuth2\Db\AccessTokenMapper; use OCA\OAuth2\Db\ClientMapper; @@ -84,7 +83,6 @@ public function __construct( #[PublicPage] #[NoCSRFRequired] #[BruteForceProtection(action: 'oauth2GetToken')] - #[NoTokenScopeRequired] public function getToken( string $grant_type, ?string $code, ?string $refresh_token, ?string $client_id, ?string $client_secret, diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index 003b8f934d7b7..953326fa130b5 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -11,7 +11,6 @@ namespace OCA\Provisioning_API\Controller; use InvalidArgumentException; -use OC\AppFramework\Http\Attributes\NoTokenScopeRequired; use OC\AppFramework\Http\PaginationTrait; use OC\Authentication\Token\RemoteWipe; use OC\Group\DisplayNameCache as GroupDisplayNameCache; @@ -706,7 +705,6 @@ public function getUser(string $userId): DataResponse { */ #[NoAdminRequired] #[NoSubAdminRequired] - #[NoTokenScopeRequired] public function getCurrentUser(): DataResponse { $user = $this->userSession->getUser(); if ($user) { diff --git a/apps/theming/lib/Controller/IconController.php b/apps/theming/lib/Controller/IconController.php index 371ec99a8b3c1..69ef91bf5b088 100644 --- a/apps/theming/lib/Controller/IconController.php +++ b/apps/theming/lib/Controller/IconController.php @@ -7,7 +7,6 @@ namespace OCA\Theming\Controller; -use OC\AppFramework\Http\Attributes\NoTokenScopeRequired; use OC\IntegrityCheck\Helpers\FileAccessHelper; use OCA\Theming\IconBuilder; use OCA\Theming\ImageManager; @@ -54,7 +53,6 @@ public function __construct( #[PublicPage] #[NoCSRFRequired] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] - #[NoTokenScopeRequired] public function getThemedIcon(string $app, string $image): Response { if ($app !== 'core' && !$this->appManager->isEnabledForUser($app)) { $app = 'core'; @@ -89,7 +87,6 @@ public function getThemedIcon(string $app, string $image): Response { #[PublicPage] #[NoCSRFRequired] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] - #[NoTokenScopeRequired] public function getFavicon(string $app = 'core'): Response { if ($app !== 'core' && !$this->appManager->isEnabledForUser($app)) { $app = 'core'; @@ -139,7 +136,6 @@ public function getFavicon(string $app = 'core'): Response { #[PublicPage] #[NoCSRFRequired] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] - #[NoTokenScopeRequired] public function getTouchIcon(string $app = 'core'): Response { if ($app !== 'core' && !$this->appManager->isEnabledForUser($app)) { $app = 'core'; diff --git a/apps/theming/lib/Controller/ThemingController.php b/apps/theming/lib/Controller/ThemingController.php index 619160c445c68..76c1129366701 100644 --- a/apps/theming/lib/Controller/ThemingController.php +++ b/apps/theming/lib/Controller/ThemingController.php @@ -8,7 +8,6 @@ namespace OCA\Theming\Controller; use InvalidArgumentException; -use OC\AppFramework\Http\Attributes\NoTokenScopeRequired; use OCA\Theming\ImageManager; use OCA\Theming\Service\ThemesService; use OCA\Theming\Settings\Admin; @@ -361,7 +360,6 @@ public function undoAll(): DataResponse { #[PublicPage] #[NoCSRFRequired] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] - #[NoTokenScopeRequired] public function getImage(string $key, bool $useSvg = true) { try { $useSvg = $useSvg && $this->imageManager->canConvert('SVG'); @@ -402,7 +400,6 @@ public function getImage(string $key, bool $useSvg = true) { #[NoTwoFactorRequired] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] #[NoSameSiteCookieRequired] - #[NoTokenScopeRequired] public function getThemeStylesheet(string $themeId, bool $plain = false, bool $withCustomCss = false) { $themes = $this->themesService->getThemes(); if (!in_array($themeId, array_keys($themes), true)) { @@ -459,7 +456,6 @@ public function getThemeStylesheet(string $themeId, bool $plain = false, bool $w #[NoCSRFRequired] #[BruteForceProtection(action: 'manifest')] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] - #[NoTokenScopeRequired] public function getManifest(string $app): JSONResponse { $cacheBusterValue = (string)$this->appConfig->getAppValueInt('cachebuster'); if ($app === 'core' || $app === 'settings') { diff --git a/apps/theming/lib/Controller/UserThemeController.php b/apps/theming/lib/Controller/UserThemeController.php index f07b4102aff46..fa3dc4219b1da 100644 --- a/apps/theming/lib/Controller/UserThemeController.php +++ b/apps/theming/lib/Controller/UserThemeController.php @@ -9,7 +9,6 @@ namespace OCA\Theming\Controller; -use OC\AppFramework\Http\Attributes\NoTokenScopeRequired; use OCA\Theming\AppInfo\Application; use OCA\Theming\ITheme; use OCA\Theming\ResponseDefinitions; @@ -134,7 +133,6 @@ private function validateTheme(string $themeId): ITheme { #[NoAdminRequired] #[NoCSRFRequired] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] - #[NoTokenScopeRequired] public function getBackground(): Response { $file = $this->backgroundService->getBackground(); if ($file !== null) { diff --git a/build/integration/features/auth.feature b/build/integration/features/auth.feature index 3eadae6d2f6a2..cdcf0a720779b 100644 --- a/build/integration/features/auth.feature +++ b/build/integration/features/auth.feature @@ -120,10 +120,10 @@ Feature: auth When requesting "/ocs/v2.php/apps/files_sharing/api/v1/shares" with "GET" using scoped basic token auth Then the HTTP status code should be "200" - Scenario: a files token reaches capabilities + Scenario: a files token is refused on capabilities Given a new client token limited to "files:files:read files:files:write" is added When requesting "/ocs/v2.php/cloud/capabilities" with "GET" using scoped basic token auth - Then the HTTP status code should be "200" + Then the HTTP status code should be "403" Scenario: a files token without files_sharing:shares:write cannot create shares Given a new client token limited to "files:files:read files:files:write" is added diff --git a/core/Controller/AvatarController.php b/core/Controller/AvatarController.php index 155f2de3ab919..997c082cd5b33 100644 --- a/core/Controller/AvatarController.php +++ b/core/Controller/AvatarController.php @@ -8,7 +8,6 @@ namespace OC\Core\Controller; -use OC\AppFramework\Http\Attributes\NoTokenScopeRequired; use OC\AppFramework\Utility\TimeFactory; use OC\Avatar\AvatarManager; use OCP\AppFramework\Controller; @@ -86,7 +85,6 @@ public function __construct( #[FrontpageRoute(verb: 'GET', url: '/avatar/{userId}/{size}/dark')] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] #[NoSameSiteCookieRequired] - #[NoTokenScopeRequired] public function getAvatarDark(string $userId, int $size, bool $guestFallback = false, string $v = '') { if ($size <= 64) { if ($size !== 64) { @@ -137,7 +135,6 @@ public function getAvatarDark(string $userId, int $size, bool $guestFallback = f #[FrontpageRoute(verb: 'GET', url: '/avatar/{userId}/{size}')] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] #[NoSameSiteCookieRequired] - #[NoTokenScopeRequired] public function getAvatar(string $userId, int $size, bool $guestFallback = false, string $v = '') { if ($size <= 64) { if ($size !== 64) { diff --git a/core/Controller/GuestAvatarController.php b/core/Controller/GuestAvatarController.php index e2f6d87547d97..c78350cdf00d8 100644 --- a/core/Controller/GuestAvatarController.php +++ b/core/Controller/GuestAvatarController.php @@ -7,7 +7,6 @@ namespace OC\Core\Controller; -use OC\AppFramework\Http\Attributes\NoTokenScopeRequired; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\FrontpageRoute; @@ -51,7 +50,6 @@ public function __construct( #[NoCSRFRequired] #[FrontpageRoute(verb: 'GET', url: '/avatar/guest/{guestName}/{size}')] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] - #[NoTokenScopeRequired] public function getAvatar(string $guestName, int $size, ?bool $darkTheme = false) { $darkTheme = $darkTheme ?? false; @@ -104,7 +102,6 @@ public function getAvatar(string $guestName, int $size, ?bool $darkTheme = false #[NoCSRFRequired] #[FrontpageRoute(verb: 'GET', url: '/avatar/guest/{guestName}/{size}/dark')] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] - #[NoTokenScopeRequired] public function getAvatarDark(string $guestName, int $size) { return $this->getAvatar($guestName, $size, true); } diff --git a/core/Controller/OCSController.php b/core/Controller/OCSController.php index f47618d7030ce..144012ac35f86 100644 --- a/core/Controller/OCSController.php +++ b/core/Controller/OCSController.php @@ -7,7 +7,6 @@ namespace OC\Core\Controller; -use OC\AppFramework\Http\Attributes\NoTokenScopeRequired; use OC\CapabilitiesManager; use OC\Security\IdentityProof\Manager; use OCP\AppFramework\Http; @@ -63,7 +62,6 @@ public function getConfig(): DataResponse { */ #[PublicPage] #[ApiRoute(verb: 'GET', url: '/capabilities', root: '/cloud')] - #[NoTokenScopeRequired] public function getCapabilities(): DataResponse { $result = []; $result['version'] = [ diff --git a/core/Controller/PreviewController.php b/core/Controller/PreviewController.php index 6bb2a71104ff8..aed3eeae5e49e 100644 --- a/core/Controller/PreviewController.php +++ b/core/Controller/PreviewController.php @@ -9,7 +9,6 @@ namespace OC\Core\Controller; -use OC\AppFramework\Http\Attributes\NoTokenScopeRequired; use OC\AppFramework\Http\Attributes\TokenScope; use OCA\Files\TokenScopes; use OCP\AppFramework\Controller; @@ -198,7 +197,6 @@ private function fetchPreview( #[PublicPage] #[FrontpageRoute(verb: 'GET', url: '/core/mimeicon')] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] - #[NoTokenScopeRequired] public function getMimeIconUrl(string $mime = 'application/octet-stream') { $url = $this->mimeIconProvider->getMimeIconUrl($mime); if ($url === null) { diff --git a/core/Controller/WipeController.php b/core/Controller/WipeController.php index 0d98e0bc013ab..f495ba5a692a4 100644 --- a/core/Controller/WipeController.php +++ b/core/Controller/WipeController.php @@ -9,7 +9,6 @@ namespace OC\Core\Controller; -use OC\AppFramework\Http\Attributes\NoTokenScopeRequired; use OC\Authentication\Token\RemoteWipe; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; @@ -46,7 +45,6 @@ public function __construct( #[NoCSRFRequired] #[AnonRateLimit(limit: 10, period: 300)] #[FrontpageRoute(verb: 'POST', url: '/core/wipe/check')] - #[NoTokenScopeRequired] public function checkWipe(string $token): JSONResponse { try { if ($this->remoteWipe->start($token)) { @@ -75,7 +73,6 @@ public function checkWipe(string $token): JSONResponse { #[NoCSRFRequired] #[AnonRateLimit(limit: 10, period: 300)] #[FrontpageRoute(verb: 'POST', url: '/core/wipe/success')] - #[NoTokenScopeRequired] public function wipeDone(string $token): JSONResponse { try { if ($this->remoteWipe->finish($token)) { diff --git a/lib/private/AppFramework/Http/Attributes/NoTokenScopeRequired.php b/lib/private/AppFramework/Http/Attributes/NoTokenScopeRequired.php index 3b27c82750163..69cf0211a4241 100644 --- a/lib/private/AppFramework/Http/Attributes/NoTokenScopeRequired.php +++ b/lib/private/AppFramework/Http/Attributes/NoTokenScopeRequired.php @@ -13,7 +13,7 @@ /** * Opens a controller method, or every method of a controller class, to every app password, - * whatever its scopes. For what any client needs to work, such as capabilities, avatars and theming. + * whatever its scopes. For what a token does to itself, such as revoking it. * * A method's attribute replaces its class's, like with TokenScope. */ From f7e03f6b41a61b1172f4ed5545d6ae2de11db932 Mon Sep 17 00:00:00 2001 From: Peter Ringelmann Date: Tue, 20 Oct 2026 17:22:32 +0200 Subject: [PATCH 6/9] feat(auth): make the token scope attribute available to apps as unstable API Signed-off-by: Peter Ringelmann --- .../Controller/CalendarExportController.php | 2 +- .../Controller/CalendarImportController.php | 2 +- .../Controller/ContactsImportController.php | 2 +- apps/dav/lib/Controller/DirectController.php | 2 +- .../FederatedCalendarController.php | 2 +- .../lib/Controller/OutOfOfficeController.php | 2 +- .../Controller/UpcomingEventsController.php | 2 +- apps/files/lib/Controller/ApiController.php | 2 +- .../Controller/OpenLocalEditorController.php | 2 +- .../TransferOwnershipController.php | 2 +- apps/files/lib/Controller/ViewController.php | 2 +- .../lib/Controller/AcceptController.php | 2 +- .../Controller/DeletedShareAPIController.php | 2 +- .../Controller/ExternalSharesController.php | 2 +- .../Controller/PublicPreviewController.php | 2 +- .../lib/Controller/RemoteController.php | 2 +- .../lib/Controller/SettingsController.php | 2 +- .../lib/Controller/ShareAPIController.php | 6 ++--- .../lib/Controller/ShareController.php | 2 +- .../lib/Controller/ShareInfoController.php | 2 +- .../lib/Controller/ShareesAPIController.php | 2 +- .../lib/Controller/PreviewController.php | 2 +- .../lib/Controller/PreviewController.php | 2 +- core/Controller/PreviewController.php | 2 +- lib/composer/composer/autoload_classmap.php | 2 +- lib/composer/composer/autoload_static.php | 2 +- .../Http/Attributes/NoTokenScopeRequired.php | 8 +++--- .../Security/TokenScopeMiddleware.php | 27 ++++++++++++++----- .../Http/Attribute}/TokenScope.php | 13 ++++++--- .../Security/TokenScopeMiddlewareTest.php | 25 +++++++++++++---- 30 files changed, 81 insertions(+), 48 deletions(-) rename lib/{private/AppFramework/Http/Attributes => unstable/AppFramework/Http/Attribute}/TokenScope.php (59%) diff --git a/apps/dav/lib/Controller/CalendarExportController.php b/apps/dav/lib/Controller/CalendarExportController.php index 344f9387cb721..7cffdfc3b1281 100644 --- a/apps/dav/lib/Controller/CalendarExportController.php +++ b/apps/dav/lib/Controller/CalendarExportController.php @@ -8,7 +8,7 @@ namespace OCA\DAV\Controller; -use OC\AppFramework\Http\Attributes\TokenScope; +use NCU\AppFramework\Http\Attribute\TokenScope; use OCA\DAV\AppInfo\Application; use OCA\DAV\CalDAV\Export\ExportService; use OCA\DAV\TokenScopes; diff --git a/apps/dav/lib/Controller/CalendarImportController.php b/apps/dav/lib/Controller/CalendarImportController.php index 446b9d4f313e1..f47c8c88bf91e 100644 --- a/apps/dav/lib/Controller/CalendarImportController.php +++ b/apps/dav/lib/Controller/CalendarImportController.php @@ -9,7 +9,7 @@ namespace OCA\DAV\Controller; use InvalidArgumentException; -use OC\AppFramework\Http\Attributes\TokenScope; +use NCU\AppFramework\Http\Attribute\TokenScope; /** * @psalm-type CalendarImportResult = array{items: list, total: non-negative-int} */ diff --git a/apps/dav/lib/Controller/ContactsImportController.php b/apps/dav/lib/Controller/ContactsImportController.php index 6d05260e04aaa..11f658befadd3 100644 --- a/apps/dav/lib/Controller/ContactsImportController.php +++ b/apps/dav/lib/Controller/ContactsImportController.php @@ -9,7 +9,7 @@ namespace OCA\DAV\Controller; use InvalidArgumentException; -use OC\AppFramework\Http\Attributes\TokenScope; +use NCU\AppFramework\Http\Attribute\TokenScope; /** * @psalm-type ContactsImportResult = array{items: list, total: non-negative-int} */ diff --git a/apps/dav/lib/Controller/DirectController.php b/apps/dav/lib/Controller/DirectController.php index 1943a6cae0e63..7934a2db32e10 100644 --- a/apps/dav/lib/Controller/DirectController.php +++ b/apps/dav/lib/Controller/DirectController.php @@ -9,7 +9,7 @@ namespace OCA\DAV\Controller; -use OC\AppFramework\Http\Attributes\TokenScope; +use NCU\AppFramework\Http\Attribute\TokenScope; use OCA\DAV\Db\Direct; use OCA\DAV\Db\DirectMapper; use OCP\AppFramework\Http; diff --git a/apps/dav/lib/Controller/FederatedCalendarController.php b/apps/dav/lib/Controller/FederatedCalendarController.php index 4f7e6f4b84218..7f565a4af14c0 100644 --- a/apps/dav/lib/Controller/FederatedCalendarController.php +++ b/apps/dav/lib/Controller/FederatedCalendarController.php @@ -9,7 +9,7 @@ namespace OCA\DAV\Controller; -use OC\AppFramework\Http\Attributes\TokenScope; +use NCU\AppFramework\Http\Attribute\TokenScope; use OCA\DAV\AppInfo\Application; use OCA\DAV\CalDAV\Federation\FederatedCalendarEntity; use OCA\DAV\CalDAV\Federation\FederatedCalendarInvitationService; diff --git a/apps/dav/lib/Controller/OutOfOfficeController.php b/apps/dav/lib/Controller/OutOfOfficeController.php index 7928112e23f6c..5b917f2ac2831 100644 --- a/apps/dav/lib/Controller/OutOfOfficeController.php +++ b/apps/dav/lib/Controller/OutOfOfficeController.php @@ -10,7 +10,7 @@ namespace OCA\DAV\Controller; use DateTimeImmutable; -use OC\AppFramework\Http\Attributes\TokenScope; +use NCU\AppFramework\Http\Attribute\TokenScope; use OCA\DAV\ResponseDefinitions; use OCA\DAV\Service\AbsenceService; use OCA\DAV\TokenScopes; diff --git a/apps/dav/lib/Controller/UpcomingEventsController.php b/apps/dav/lib/Controller/UpcomingEventsController.php index bedbe596b4587..60c480d65a1e2 100644 --- a/apps/dav/lib/Controller/UpcomingEventsController.php +++ b/apps/dav/lib/Controller/UpcomingEventsController.php @@ -9,7 +9,7 @@ namespace OCA\DAV\Controller; -use OC\AppFramework\Http\Attributes\TokenScope; +use NCU\AppFramework\Http\Attribute\TokenScope; use OCA\DAV\AppInfo\Application; use OCA\DAV\CalDAV\UpcomingEvent; use OCA\DAV\CalDAV\UpcomingEventsService; diff --git a/apps/files/lib/Controller/ApiController.php b/apps/files/lib/Controller/ApiController.php index 348592dccd0c5..dac95e914aa76 100644 --- a/apps/files/lib/Controller/ApiController.php +++ b/apps/files/lib/Controller/ApiController.php @@ -8,7 +8,7 @@ namespace OCA\Files\Controller; -use OC\AppFramework\Http\Attributes\TokenScope; +use NCU\AppFramework\Http\Attribute\TokenScope; use OC\Files\Node\Node; use OCA\Files\Helper; use OCA\Files\ResponseDefinitions; diff --git a/apps/files/lib/Controller/OpenLocalEditorController.php b/apps/files/lib/Controller/OpenLocalEditorController.php index db35dda921786..e124f5623ca4d 100644 --- a/apps/files/lib/Controller/OpenLocalEditorController.php +++ b/apps/files/lib/Controller/OpenLocalEditorController.php @@ -9,7 +9,7 @@ namespace OCA\Files\Controller; -use OC\AppFramework\Http\Attributes\TokenScope; +use NCU\AppFramework\Http\Attribute\TokenScope; use OCA\Files\Db\OpenLocalEditor; use OCA\Files\Db\OpenLocalEditorMapper; use OCA\Files\TokenScopes; diff --git a/apps/files/lib/Controller/TransferOwnershipController.php b/apps/files/lib/Controller/TransferOwnershipController.php index a12bfc405fbae..967b159523a90 100644 --- a/apps/files/lib/Controller/TransferOwnershipController.php +++ b/apps/files/lib/Controller/TransferOwnershipController.php @@ -9,7 +9,7 @@ namespace OCA\Files\Controller; -use OC\AppFramework\Http\Attributes\TokenScope; +use NCU\AppFramework\Http\Attribute\TokenScope; use OCA\Files\BackgroundJob\TransferOwnership; use OCA\Files\Db\TransferOwnership as TransferOwnershipEntity; use OCA\Files\Db\TransferOwnershipMapper; diff --git a/apps/files/lib/Controller/ViewController.php b/apps/files/lib/Controller/ViewController.php index be2b9ee427361..d9b97b3301b8a 100644 --- a/apps/files/lib/Controller/ViewController.php +++ b/apps/files/lib/Controller/ViewController.php @@ -8,7 +8,7 @@ namespace OCA\Files\Controller; -use OC\AppFramework\Http\Attributes\TokenScope; +use NCU\AppFramework\Http\Attribute\TokenScope; use OC\Files\FilenameValidator; use OC\Files\Filesystem; use OCA\Files\AppInfo\Application; diff --git a/apps/files_sharing/lib/Controller/AcceptController.php b/apps/files_sharing/lib/Controller/AcceptController.php index 94b4c03e2cf5b..67a3d6ba4727b 100644 --- a/apps/files_sharing/lib/Controller/AcceptController.php +++ b/apps/files_sharing/lib/Controller/AcceptController.php @@ -9,7 +9,7 @@ namespace OCA\Files_Sharing\Controller; -use OC\AppFramework\Http\Attributes\TokenScope; +use NCU\AppFramework\Http\Attribute\TokenScope; use OCA\Files\TokenScopes; use OCA\Files_Sharing\AppInfo\Application; use OCP\AppFramework\Controller; diff --git a/apps/files_sharing/lib/Controller/DeletedShareAPIController.php b/apps/files_sharing/lib/Controller/DeletedShareAPIController.php index 57a60908608de..c7250a22cde70 100644 --- a/apps/files_sharing/lib/Controller/DeletedShareAPIController.php +++ b/apps/files_sharing/lib/Controller/DeletedShareAPIController.php @@ -9,7 +9,7 @@ namespace OCA\Files_Sharing\Controller; -use OC\AppFramework\Http\Attributes\TokenScope; +use NCU\AppFramework\Http\Attribute\TokenScope; use OCA\Deck\Sharing\ShareAPIHelper; use OCA\Files\TokenScopes; use OCA\Files_Sharing\ResponseDefinitions; diff --git a/apps/files_sharing/lib/Controller/ExternalSharesController.php b/apps/files_sharing/lib/Controller/ExternalSharesController.php index 61ade0a97e141..664ca5ad0755c 100644 --- a/apps/files_sharing/lib/Controller/ExternalSharesController.php +++ b/apps/files_sharing/lib/Controller/ExternalSharesController.php @@ -8,7 +8,7 @@ namespace OCA\Files_Sharing\Controller; -use OC\AppFramework\Http\Attributes\TokenScope; +use NCU\AppFramework\Http\Attribute\TokenScope; use OCA\Files\TokenScopes; use OCA\Files_Sharing\BackgroundJob\ExternalShareScanJob; use OCA\Files_Sharing\External\Manager; diff --git a/apps/files_sharing/lib/Controller/PublicPreviewController.php b/apps/files_sharing/lib/Controller/PublicPreviewController.php index 991bb463c22ff..1ec86212ef8ec 100644 --- a/apps/files_sharing/lib/Controller/PublicPreviewController.php +++ b/apps/files_sharing/lib/Controller/PublicPreviewController.php @@ -7,7 +7,7 @@ namespace OCA\Files_Sharing\Controller; -use OC\AppFramework\Http\Attributes\TokenScope; +use NCU\AppFramework\Http\Attribute\TokenScope; use OCA\Files\TokenScopes; use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\NoCSRFRequired; diff --git a/apps/files_sharing/lib/Controller/RemoteController.php b/apps/files_sharing/lib/Controller/RemoteController.php index 1cb5b9ff91bd5..ae1fe5c5e098d 100644 --- a/apps/files_sharing/lib/Controller/RemoteController.php +++ b/apps/files_sharing/lib/Controller/RemoteController.php @@ -8,7 +8,7 @@ namespace OCA\Files_Sharing\Controller; -use OC\AppFramework\Http\Attributes\TokenScope; +use NCU\AppFramework\Http\Attribute\TokenScope; use OCA\Files\TokenScopes; use OCA\Files_Sharing\External\ExternalShare; use OCA\Files_Sharing\External\Manager; diff --git a/apps/files_sharing/lib/Controller/SettingsController.php b/apps/files_sharing/lib/Controller/SettingsController.php index 0a8a4ea46a829..c52c61e69b05f 100644 --- a/apps/files_sharing/lib/Controller/SettingsController.php +++ b/apps/files_sharing/lib/Controller/SettingsController.php @@ -9,7 +9,7 @@ namespace OCA\Files_Sharing\Controller; -use OC\AppFramework\Http\Attributes\TokenScope; +use NCU\AppFramework\Http\Attribute\TokenScope; use OCA\Files\TokenScopes; use OCA\Files_Sharing\AppInfo\Application; use OCP\AppFramework\Controller; diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index 5dc98f0c5a092..58f4573986e8f 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -10,14 +10,14 @@ namespace OCA\Files_Sharing\Controller; use Exception; -use OC\AppFramework\Http\Attributes\TokenScope; +use NCU\AppFramework\Http\Attribute\TokenScope; use OC\Core\AppInfo\ConfigLexicon; use OC\Files\FileInfo; use OC\Files\Storage\Wrapper\Wrapper; use OCA\Circles\Api\v1\Circles; use OCA\Deck\Sharing\ShareAPIHelper; use OCA\Federation\TrustedServers; -use OCA\Files\TokenScopes; +use OCA\Files\TokenScopes as FilesTokenScopes; use OCA\Files_Sharing\Exceptions\SharingRightsException; use OCA\Files_Sharing\External\Storage; use OCA\Files_Sharing\ResponseDefinitions; @@ -77,7 +77,7 @@ * * @psalm-import-type Files_SharingShare from ResponseDefinitions */ -#[TokenScope(scope: TokenScopes::FILES_WRITE)] +#[TokenScope(scope: FilesTokenScopes::FILES_WRITE)] class ShareAPIController extends OCSController { /** Maximum length of a custom share token, matching the oc_share.token database column. */ diff --git a/apps/files_sharing/lib/Controller/ShareController.php b/apps/files_sharing/lib/Controller/ShareController.php index e0668dcc8f1ed..85bbde0e28712 100644 --- a/apps/files_sharing/lib/Controller/ShareController.php +++ b/apps/files_sharing/lib/Controller/ShareController.php @@ -8,7 +8,7 @@ namespace OCA\Files_Sharing\Controller; -use OC\AppFramework\Http\Attributes\TokenScope; +use NCU\AppFramework\Http\Attribute\TokenScope; use OC\ServerNotAvailableException; use OCA\DAV\Connector\Sabre\PublicAuth; use OCA\FederatedFileSharing\FederatedShareProvider; diff --git a/apps/files_sharing/lib/Controller/ShareInfoController.php b/apps/files_sharing/lib/Controller/ShareInfoController.php index 912a223b6646f..fa3afcd964847 100644 --- a/apps/files_sharing/lib/Controller/ShareInfoController.php +++ b/apps/files_sharing/lib/Controller/ShareInfoController.php @@ -7,7 +7,7 @@ namespace OCA\Files_Sharing\Controller; -use OC\AppFramework\Http\Attributes\TokenScope; +use NCU\AppFramework\Http\Attribute\TokenScope; use OCA\Files\TokenScopes; use OCA\Files_Sharing\ResponseDefinitions; use OCP\AppFramework\ApiController; diff --git a/apps/files_sharing/lib/Controller/ShareesAPIController.php b/apps/files_sharing/lib/Controller/ShareesAPIController.php index 0a1f1043556f1..25ca3e3cac9e1 100644 --- a/apps/files_sharing/lib/Controller/ShareesAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareesAPIController.php @@ -10,7 +10,7 @@ namespace OCA\Files_Sharing\Controller; use Generator; -use OC\AppFramework\Http\Attributes\TokenScope; +use NCU\AppFramework\Http\Attribute\TokenScope; use OC\Collaboration\Collaborators\SearchResult; use OCA\FederatedFileSharing\FederatedShareProvider; use OCA\Files_Sharing\ResponseDefinitions; diff --git a/apps/files_trashbin/lib/Controller/PreviewController.php b/apps/files_trashbin/lib/Controller/PreviewController.php index ef6b7d3c22d8a..b88d2de92e635 100644 --- a/apps/files_trashbin/lib/Controller/PreviewController.php +++ b/apps/files_trashbin/lib/Controller/PreviewController.php @@ -9,7 +9,7 @@ namespace OCA\Files_Trashbin\Controller; -use OC\AppFramework\Http\Attributes\TokenScope; +use NCU\AppFramework\Http\Attribute\TokenScope; use OCA\Files\TokenScopes; use OCA\Files_Trashbin\Trash\ITrashManager; use OCP\AppFramework\Controller; diff --git a/apps/files_versions/lib/Controller/PreviewController.php b/apps/files_versions/lib/Controller/PreviewController.php index 82af152f21ce0..6a6a1fedfba4f 100644 --- a/apps/files_versions/lib/Controller/PreviewController.php +++ b/apps/files_versions/lib/Controller/PreviewController.php @@ -7,7 +7,7 @@ namespace OCA\Files_Versions\Controller; -use OC\AppFramework\Http\Attributes\TokenScope; +use NCU\AppFramework\Http\Attribute\TokenScope; use OCA\Files\TokenScopes; use OCA\Files_Versions\Versions\IVersionManager; use OCP\AppFramework\Controller; diff --git a/core/Controller/PreviewController.php b/core/Controller/PreviewController.php index aed3eeae5e49e..cf027d966ac8f 100644 --- a/core/Controller/PreviewController.php +++ b/core/Controller/PreviewController.php @@ -9,7 +9,7 @@ namespace OC\Core\Controller; -use OC\AppFramework\Http\Attributes\TokenScope; +use NCU\AppFramework\Http\Attribute\TokenScope; use OCA\Files\TokenScopes; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index f4522a00ad2ed..8172a8d97e45f 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -7,6 +7,7 @@ return array( 'Composer\\InstalledVersions' => $vendorDir . '/composer/InstalledVersions.php', + 'NCU\\AppFramework\\Http\\Attribute\\TokenScope' => $baseDir . '/lib/unstable/AppFramework/Http/Attribute/TokenScope.php', 'NCU\\Config\\Exceptions\\IncorrectTypeException' => $baseDir . '/lib/unstable/Config/Exceptions/IncorrectTypeException.php', 'NCU\\Config\\Exceptions\\TypeConflictException' => $baseDir . '/lib/unstable/Config/Exceptions/TypeConflictException.php', 'NCU\\Config\\Exceptions\\UnknownKeyException' => $baseDir . '/lib/unstable/Config/Exceptions/UnknownKeyException.php', @@ -1222,7 +1223,6 @@ 'OC\\AppFramework\\Http' => $baseDir . '/lib/private/AppFramework/Http.php', 'OC\\AppFramework\\Http\\Attributes\\FederationRateLimit' => $baseDir . '/lib/private/AppFramework/Http/Attributes/FederationRateLimit.php', 'OC\\AppFramework\\Http\\Attributes\\NoTokenScopeRequired' => $baseDir . '/lib/private/AppFramework/Http/Attributes/NoTokenScopeRequired.php', - 'OC\\AppFramework\\Http\\Attributes\\TokenScope' => $baseDir . '/lib/private/AppFramework/Http/Attributes/TokenScope.php', 'OC\\AppFramework\\Http\\Attributes\\TwoFactorSetUpDoneRequired' => $baseDir . '/lib/private/AppFramework/Http/Attributes/TwoFactorSetUpDoneRequired.php', 'OC\\AppFramework\\Http\\Dispatcher' => $baseDir . '/lib/private/AppFramework/Http/Dispatcher.php', 'OC\\AppFramework\\Http\\Output' => $baseDir . '/lib/private/AppFramework/Http/Output.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index b490ecbe299c4..d830c2f16413d 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -48,6 +48,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 public static $classMap = array ( 'Composer\\InstalledVersions' => __DIR__ . '/..' . '/composer/InstalledVersions.php', + 'NCU\\AppFramework\\Http\\Attribute\\TokenScope' => __DIR__ . '/../../..' . '/lib/unstable/AppFramework/Http/Attribute/TokenScope.php', 'NCU\\Config\\Exceptions\\IncorrectTypeException' => __DIR__ . '/../../..' . '/lib/unstable/Config/Exceptions/IncorrectTypeException.php', 'NCU\\Config\\Exceptions\\TypeConflictException' => __DIR__ . '/../../..' . '/lib/unstable/Config/Exceptions/TypeConflictException.php', 'NCU\\Config\\Exceptions\\UnknownKeyException' => __DIR__ . '/../../..' . '/lib/unstable/Config/Exceptions/UnknownKeyException.php', @@ -1263,7 +1264,6 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\AppFramework\\Http' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Http.php', 'OC\\AppFramework\\Http\\Attributes\\FederationRateLimit' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Http/Attributes/FederationRateLimit.php', 'OC\\AppFramework\\Http\\Attributes\\NoTokenScopeRequired' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Http/Attributes/NoTokenScopeRequired.php', - 'OC\\AppFramework\\Http\\Attributes\\TokenScope' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Http/Attributes/TokenScope.php', 'OC\\AppFramework\\Http\\Attributes\\TwoFactorSetUpDoneRequired' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Http/Attributes/TwoFactorSetUpDoneRequired.php', 'OC\\AppFramework\\Http\\Dispatcher' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Http/Dispatcher.php', 'OC\\AppFramework\\Http\\Output' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Http/Output.php', diff --git a/lib/private/AppFramework/Http/Attributes/NoTokenScopeRequired.php b/lib/private/AppFramework/Http/Attributes/NoTokenScopeRequired.php index 69cf0211a4241..ded8fd25aef4f 100644 --- a/lib/private/AppFramework/Http/Attributes/NoTokenScopeRequired.php +++ b/lib/private/AppFramework/Http/Attributes/NoTokenScopeRequired.php @@ -12,11 +12,9 @@ use Attribute; /** - * Opens a controller method, or every method of a controller class, to every app password, - * whatever its scopes. For what a token does to itself, such as revoking it. - * - * A method's attribute replaces its class's, like with TokenScope. + * Opens a controller method to every app password, whatever its scopes, over a TokenScope + * on its class. For what a token does to itself, such as revoking it. */ -#[Attribute(Attribute::TARGET_METHOD | Attribute::TARGET_CLASS)] +#[Attribute(Attribute::TARGET_METHOD)] class NoTokenScopeRequired { } diff --git a/lib/private/AppFramework/Middleware/Security/TokenScopeMiddleware.php b/lib/private/AppFramework/Middleware/Security/TokenScopeMiddleware.php index 9901d1bfdd616..f2ab84dc7b9b1 100644 --- a/lib/private/AppFramework/Middleware/Security/TokenScopeMiddleware.php +++ b/lib/private/AppFramework/Middleware/Security/TokenScopeMiddleware.php @@ -8,11 +8,12 @@ namespace OC\AppFramework\Middleware\Security; +use NCU\AppFramework\Http\Attribute\TokenScope; use OC\AppFramework\Http\Attributes\NoTokenScopeRequired; -use OC\AppFramework\Http\Attributes\TokenScope; use OC\AppFramework\Middleware\Security\Exceptions\TokenScopeException; use OC\AppFramework\Utility\ControllerMethodReflector; use OC\Lockdown\LockdownManager; +use OCP\App\IAppManager; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\NoAdminRequired; @@ -27,6 +28,8 @@ class TokenScopeMiddleware extends Middleware { public function __construct( private ControllerMethodReflector $reflector, private LockdownManager $lockdownManager, + private IAppManager $appManager, + private string $appName, ) { } @@ -43,23 +46,35 @@ public function beforeController(Controller $controller, string $methodName): vo } $attribute = $this->tokenScope($controller); - if ($attribute instanceof NoTokenScopeRequired) { + if ($attribute instanceof NoTokenScopeRequired && $this->isShipped()) { return; } - if ($attribute === null || !$this->lockdownManager->hasScope($attribute->scope)) { + if (!$attribute instanceof TokenScope + || !$this->mayRequire($attribute->scope) + || !$this->lockdownManager->hasScope($attribute->scope)) { throw new TokenScopeException(); } } + /** + * Apps that aren't shipped may only require scopes named after themselves, + * so they can't open their routes to app passwords limited to another app + */ + private function mayRequire(string $scope): bool { + return $this->isShipped() || str_starts_with($scope, $this->appName . ':'); + } + + private function isShipped(): bool { + return $this->appName === 'core' || $this->appManager->isShipped($this->appName); + } + /** * The method's attribute, else its class's */ private function tokenScope(Controller $controller): TokenScope|NoTokenScopeRequired|null { - $class = new ReflectionClass($controller); $attribute = $this->reflector->getAttribute(TokenScope::class) ?? $this->reflector->getAttribute(NoTokenScopeRequired::class) - ?? $class->getAttributes(TokenScope::class)[0] - ?? $class->getAttributes(NoTokenScopeRequired::class)[0] + ?? (new ReflectionClass($controller))->getAttributes(TokenScope::class)[0] ?? null; return $attribute?->newInstance(); } diff --git a/lib/private/AppFramework/Http/Attributes/TokenScope.php b/lib/unstable/AppFramework/Http/Attribute/TokenScope.php similarity index 59% rename from lib/private/AppFramework/Http/Attributes/TokenScope.php rename to lib/unstable/AppFramework/Http/Attribute/TokenScope.php index de3e67044e2f5..4a70c56548f4c 100644 --- a/lib/private/AppFramework/Http/Attributes/TokenScope.php +++ b/lib/unstable/AppFramework/Http/Attribute/TokenScope.php @@ -7,21 +7,26 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ -namespace OC\AppFramework\Http\Attributes; +namespace NCU\AppFramework\Http\Attribute; use Attribute; /** * Opens a controller method, or every method of a controller class, to app passwords - * that hold the scope. Methods with neither this nor NoTokenScopeRequired refuse - * app passwords limited to scopes. + * that hold the scope. Methods without it refuse app passwords limited to scopes. * * A method's attribute replaces its class's, it does not add to it. + * + * Apps may only require scopes named after their own app id, `::`. + * Other scopes are refused on apps that aren't shipped with Nextcloud. + * + * @experimental 36.0.0 */ #[Attribute(Attribute::TARGET_METHOD | Attribute::TARGET_CLASS)] -class TokenScope { +final class TokenScope { /** * @param string $scope the scope the app password needs + * @experimental 36.0.0 */ public function __construct( public readonly string $scope, diff --git a/tests/lib/AppFramework/Middleware/Security/TokenScopeMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/TokenScopeMiddlewareTest.php index 218b0b26c413d..5bd03d4299f3f 100644 --- a/tests/lib/AppFramework/Middleware/Security/TokenScopeMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/TokenScopeMiddlewareTest.php @@ -9,8 +9,8 @@ namespace Test\AppFramework\Middleware\Security; +use NCU\AppFramework\Http\Attribute\TokenScope; use OC\AppFramework\Http\Attributes\NoTokenScopeRequired; -use OC\AppFramework\Http\Attributes\TokenScope; use OC\AppFramework\Middleware\Security\Exceptions\TokenScopeException; use OC\AppFramework\Middleware\Security\TokenScopeMiddleware; use OC\AppFramework\Utility\ControllerMethodReflector; @@ -28,6 +28,7 @@ use OCA\Files_Sharing\Controller\ShareAPIController; use OCA\Files_Sharing\Controller\ShareesAPIController; use OCA\Files_Sharing\TokenScopes as SharingTokenScopes; +use OCP\App\IAppManager; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\NoAdminRequired; @@ -56,6 +57,11 @@ public function share(): void { public function unannotated(): void { } + #[NoAdminRequired] + #[TokenScope(scope: 'deck:boards:read')] + public function deck(): void { + } + #[NoTokenScopeRequired] public function adminOnly(): void { } @@ -104,18 +110,21 @@ public static function requests(): array { 'class attribute, other scope' => [self::CALENDAR, TokenScopeTestClassController::class, 'files', false], 'method overrides class' => [self::FILES, TokenScopeTestClassController::class, 'share', false], 'method opens what the class limits' => [self::CALENDAR, TokenScopeTestClassController::class, 'anyToken', true], + 'app requires its own scope' => [['deck:boards:read'], TokenScopeTestController::class, 'deck', true, 'deck'], + 'app requires another app\'s scope' => [[...self::FILES, SharingTokenScopes::SHARES_WRITE], TokenScopeTestController::class, 'share', false, 'deck'], + 'app opens itself to every token' => [[], TokenScopeTestController::class, 'anyToken', false, 'deck'], ]; } #[DataProvider('requests')] - public function testBeforeController(?array $scopes, string $class, string $method, bool $allowed): void { + public function testBeforeController(?array $scopes, string $class, string $method, bool $allowed, ?string $thirdPartyApp = null): void { $controller = new $class('test', $this->request); $this->reflector->reflect($controller, $method); if (!$allowed) { $this->expectException(TokenScopeException::class); } - $this->middleware($scopes)->beforeController($controller, $method); + $this->middleware($scopes, $thirdPartyApp)->beforeController($controller, $method); $this->addToAssertionCount(1); } @@ -183,12 +192,18 @@ public function testRefusedControllersStayUnannotated(string $class): void { } } - private function middleware(?array $scopes): TokenScopeMiddleware { + /** + * @param string|null $thirdPartyApp app the controller belongs to, null for a shipped app + */ + private function middleware(?array $scopes, ?string $thirdPartyApp = null): TokenScopeMiddleware { $lockdownManager = new LockdownManager(fn () => $this->createMock(ISession::class), new TokenScopes()); $token = new PublicKeyToken(); $token->setScope($scopes === null ? [] : [TokenScopes::KEY => $scopes]); $lockdownManager->setToken($token); - return new TokenScopeMiddleware($this->reflector, $lockdownManager); + $appManager = $this->createMock(IAppManager::class); + $appManager->method('isShipped')->willReturn($thirdPartyApp === null); + + return new TokenScopeMiddleware($this->reflector, $lockdownManager, $appManager, $thirdPartyApp ?? 'files'); } } From e9fac6544cb9f3ec042258c8aca50317876dd859 Mon Sep 17 00:00:00 2001 From: Peter Ringelmann Date: Thu, 22 Oct 2026 14:48:30 +0200 Subject: [PATCH 7/9] fix(auth): refuse page routes before they run for scoped app passwords Signed-off-by: Peter Ringelmann --- apps/files/lib/Controller/ViewController.php | 3 --- .../lib/Controller/AcceptController.php | 2 +- .../lib/Controller/ShareController.php | 2 +- .../Security/TokenScopeMiddlewareTest.php | 23 +++++++++++++++++++ 4 files changed, 25 insertions(+), 5 deletions(-) diff --git a/apps/files/lib/Controller/ViewController.php b/apps/files/lib/Controller/ViewController.php index d9b97b3301b8a..c7506c002d0e2 100644 --- a/apps/files/lib/Controller/ViewController.php +++ b/apps/files/lib/Controller/ViewController.php @@ -8,7 +8,6 @@ namespace OCA\Files\Controller; -use NCU\AppFramework\Http\Attribute\TokenScope; use OC\Files\FilenameValidator; use OC\Files\Filesystem; use OCA\Files\AppInfo\Application; @@ -18,7 +17,6 @@ use OCA\Files\Event\LoadSidebar; use OCA\Files\Service\UserConfig; use OCA\Files\Service\ViewConfig; -use OCA\Files\TokenScopes; use OCP\App\IAppManager; use OCP\AppFramework\Controller; use OCP\AppFramework\Http\Attribute\NoAdminRequired; @@ -48,7 +46,6 @@ * @package OCA\Files\Controller */ #[OpenAPI(scope: OpenAPI::SCOPE_IGNORE)] -#[TokenScope(scope: TokenScopes::FILES_WRITE)] class ViewController extends Controller { public function __construct( diff --git a/apps/files_sharing/lib/Controller/AcceptController.php b/apps/files_sharing/lib/Controller/AcceptController.php index 67a3d6ba4727b..97ce66a1cd2d6 100644 --- a/apps/files_sharing/lib/Controller/AcceptController.php +++ b/apps/files_sharing/lib/Controller/AcceptController.php @@ -32,7 +32,6 @@ use OCP\Share\IShare; #[OpenAPI(scope: OpenAPI::SCOPE_IGNORE)] -#[TokenScope(scope: TokenScopes::FILES_WRITE)] class AcceptController extends Controller { public function __construct( @@ -81,6 +80,7 @@ public function showAccept(string $shareId): Response { } #[NoAdminRequired] + #[TokenScope(scope: TokenScopes::FILES_WRITE)] public function accept(string $shareId): Response { try { $share = $this->shareManager->getShareById($shareId); diff --git a/apps/files_sharing/lib/Controller/ShareController.php b/apps/files_sharing/lib/Controller/ShareController.php index 85bbde0e28712..2e97fc511e01b 100644 --- a/apps/files_sharing/lib/Controller/ShareController.php +++ b/apps/files_sharing/lib/Controller/ShareController.php @@ -56,7 +56,6 @@ * @package OCA\Files_Sharing\Controllers */ #[OpenAPI(scope: OpenAPI::SCOPE_IGNORE)] -#[TokenScope(scope: TokenScopes::FILES_WRITE)] class ShareController extends AuthPublicShareController { protected ?IShare $share = null; @@ -332,6 +331,7 @@ public function showShare($path = ''): TemplateResponse { #[PublicPage] #[NoCSRFRequired] #[NoSameSiteCookieRequired] + #[TokenScope(scope: TokenScopes::FILES_WRITE)] public function downloadShare(string $token, ?string $files = null, string $path = ''): NotFoundResponse|RedirectResponse|DataResponse { $this->userSession->setIncognitoMode(true); diff --git a/tests/lib/AppFramework/Middleware/Security/TokenScopeMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/TokenScopeMiddlewareTest.php index 5bd03d4299f3f..b708ce8fbdb2e 100644 --- a/tests/lib/AppFramework/Middleware/Security/TokenScopeMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/TokenScopeMiddlewareTest.php @@ -24,8 +24,11 @@ use OCA\Files\Controller\DirectEditingController; use OCA\Files\Controller\DirectEditingViewController; use OCA\Files\Controller\TemplateController; +use OCA\Files\Controller\ViewController; use OCA\Files\TokenScopes as FilesTokenScopes; +use OCA\Files_Sharing\Controller\AcceptController; use OCA\Files_Sharing\Controller\ShareAPIController; +use OCA\Files_Sharing\Controller\ShareController; use OCA\Files_Sharing\Controller\ShareesAPIController; use OCA\Files_Sharing\TokenScopes as SharingTokenScopes; use OCP\App\IAppManager; @@ -169,6 +172,26 @@ public function testPinnedScopes(string $class, string $method, string $scope): $this->assertSame($scope, $attribute->newInstance()->scope); } + /** + * Page renders are refused anyway, so they must not run for a scoped token first + */ + public static function pages(): array { + return [ + [ViewController::class, 'index'], + [ViewController::class, 'indexView'], + [ViewController::class, 'indexViewFileid'], + [ViewController::class, 'showFile'], + [ShareController::class, 'showShare'], + [AcceptController::class, 'showAccept'], + ]; + } + + #[DataProvider('pages')] + public function testPagesHaveNoScope(string $class, string $method): void { + $this->assertSame([], (new \ReflectionMethod($class, $method))->getAttributes(TokenScope::class)); + $this->assertSame([], (new \ReflectionClass($class))->getAttributes(TokenScope::class)); + } + /** * These hand files to other apps, so no scoped token may reach them */ From 4aaf21d964a408fe0de666c80c51e8cee742e4ba Mon Sep 17 00:00:00 2001 From: Peter Ringelmann Date: Thu, 22 Oct 2026 18:00:38 +0200 Subject: [PATCH 8/9] docs(auth): explain the token scope checks in the middleware Signed-off-by: Peter Ringelmann --- .../AppFramework/Middleware/Security/TokenScopeMiddleware.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/private/AppFramework/Middleware/Security/TokenScopeMiddleware.php b/lib/private/AppFramework/Middleware/Security/TokenScopeMiddleware.php index f2ab84dc7b9b1..d6a8dcbba01bd 100644 --- a/lib/private/AppFramework/Middleware/Security/TokenScopeMiddleware.php +++ b/lib/private/AppFramework/Middleware/Security/TokenScopeMiddleware.php @@ -39,6 +39,7 @@ public function beforeController(Controller $controller, string $methodName): vo return; } + // Admin-only routes stay closed to app passwords with scopes, even for admins if (!$this->reflector->hasAnnotationOrAttribute('PublicPage', PublicPage::class) && !$this->reflector->hasAnnotationOrAttribute('NoAdminRequired', NoAdminRequired::class) && !$this->reflector->hasAnnotationOrAttribute('SubAdminRequired', SubAdminRequired::class)) { @@ -49,6 +50,7 @@ public function beforeController(Controller $controller, string $methodName): vo if ($attribute instanceof NoTokenScopeRequired && $this->isShipped()) { return; } + // The route must name a scope its app is allowed to require, and the token must hold it if (!$attribute instanceof TokenScope || !$this->mayRequire($attribute->scope) || !$this->lockdownManager->hasScope($attribute->scope)) { @@ -69,7 +71,7 @@ private function isShipped(): bool { } /** - * The method's attribute, else its class's + * A method's attribute replaces its class's, it doesn't add to it */ private function tokenScope(Controller $controller): TokenScope|NoTokenScopeRequired|null { $attribute = $this->reflector->getAttribute(TokenScope::class) From 65fc6f8f3d727437d6cb04e1b9f92ca48458d707 Mon Sep 17 00:00:00 2001 From: Peter Ringelmann Date: Fri, 23 Oct 2026 11:36:11 +0200 Subject: [PATCH 9/9] fix(files_sharing): require only files read for public share downloads Signed-off-by: Peter Ringelmann --- apps/files_sharing/lib/Controller/ShareController.php | 2 +- .../Middleware/Security/TokenScopeMiddlewareTest.php | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/files_sharing/lib/Controller/ShareController.php b/apps/files_sharing/lib/Controller/ShareController.php index 2e97fc511e01b..6383f29d6098c 100644 --- a/apps/files_sharing/lib/Controller/ShareController.php +++ b/apps/files_sharing/lib/Controller/ShareController.php @@ -331,7 +331,7 @@ public function showShare($path = ''): TemplateResponse { #[PublicPage] #[NoCSRFRequired] #[NoSameSiteCookieRequired] - #[TokenScope(scope: TokenScopes::FILES_WRITE)] + #[TokenScope(scope: TokenScopes::FILES_READ)] public function downloadShare(string $token, ?string $files = null, string $path = ''): NotFoundResponse|RedirectResponse|DataResponse { $this->userSession->setIncognitoMode(true); diff --git a/tests/lib/AppFramework/Middleware/Security/TokenScopeMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/TokenScopeMiddlewareTest.php index b708ce8fbdb2e..86497a4e1740c 100644 --- a/tests/lib/AppFramework/Middleware/Security/TokenScopeMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/TokenScopeMiddlewareTest.php @@ -162,6 +162,7 @@ public static function pinnedScopes(): array { 'direct link' => [DirectController::class, 'getUrl', SharingTokenScopes::SHARES_WRITE], 'preview' => [PreviewController::class, 'getPreview', FilesTokenScopes::FILES_READ], 'preview by file id' => [PreviewController::class, 'getPreviewByFileId', FilesTokenScopes::FILES_READ], + 'public share download' => [ShareController::class, 'downloadShare', FilesTokenScopes::FILES_READ], ]; }