diff --git a/apps/files_sharing/lib/Controller/PublicPreviewController.php b/apps/files_sharing/lib/Controller/PublicPreviewController.php index d917f6e0ebbb5..9056161af84fe 100644 --- a/apps/files_sharing/lib/Controller/PublicPreviewController.php +++ b/apps/files_sharing/lib/Controller/PublicPreviewController.php @@ -8,6 +8,7 @@ use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\NoCSRFRequired; +use OCP\AppFramework\Http\Attribute\NoSameSiteCookieRequired; use OCP\AppFramework\Http\Attribute\OpenAPI; use OCP\AppFramework\Http\Attribute\PublicPage; use OCP\AppFramework\Http\DataResponse; @@ -144,8 +145,6 @@ public function getPreview( } /** - * @NoSameSiteCookieRequired - * * Get a direct link preview for a shared file * * @param string $token Token of the share @@ -159,6 +158,7 @@ public function getPreview( #[PublicPage] #[NoCSRFRequired] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] + #[NoSameSiteCookieRequired] public function directLink(string $token) { // No token no image if ($token === '') { diff --git a/apps/files_sharing/lib/Controller/ShareController.php b/apps/files_sharing/lib/Controller/ShareController.php index 3c51a95f1d501..ab7556e1af963 100644 --- a/apps/files_sharing/lib/Controller/ShareController.php +++ b/apps/files_sharing/lib/Controller/ShareController.php @@ -15,12 +15,12 @@ use OCP\Accounts\IAccountManager; use OCP\AppFramework\AuthPublicShareController; use OCP\AppFramework\Http\Attribute\NoCSRFRequired; +use OCP\AppFramework\Http\Attribute\NoSameSiteCookieRequired; use OCP\AppFramework\Http\Attribute\OpenAPI; use OCP\AppFramework\Http\Attribute\PublicPage; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Http\NotFoundResponse; use OCP\AppFramework\Http\RedirectResponse; -use OCP\AppFramework\Http\Response; use OCP\AppFramework\Http\TemplateResponse; use OCP\Constants; use OCP\Defaults; @@ -343,18 +343,13 @@ public function showShare($path = ''): TemplateResponse { } /** - * @NoSameSiteCookieRequired - * - * @param string $token - * @param string|null $files - * @param string $path - * @return void|Response * @throws NotFoundException * @deprecated 31.0.0 Users are encouraged to use the DAV endpoint */ #[PublicPage] #[NoCSRFRequired] - public function downloadShare($token, $files = null, $path = '') { + #[NoSameSiteCookieRequired] + public function downloadShare(string $token, ?string $files = null, string $path = ''): NotFoundResponse|RedirectResponse|DataResponse { \OC_User::setIncognitoMode(true); $share = $this->shareManager->getShareByToken($token); diff --git a/apps/theming/lib/Controller/ThemingController.php b/apps/theming/lib/Controller/ThemingController.php index 8bb9841ae5596..31686857a667f 100644 --- a/apps/theming/lib/Controller/ThemingController.php +++ b/apps/theming/lib/Controller/ThemingController.php @@ -17,6 +17,8 @@ use OCP\AppFramework\Http\Attribute\AuthorizedAdminSetting; use OCP\AppFramework\Http\Attribute\BruteForceProtection; use OCP\AppFramework\Http\Attribute\NoCSRFRequired; +use OCP\AppFramework\Http\Attribute\NoSameSiteCookieRequired; +use OCP\AppFramework\Http\Attribute\NoTwoFactorRequired; use OCP\AppFramework\Http\Attribute\OpenAPI; use OCP\AppFramework\Http\Attribute\PublicPage; use OCP\AppFramework\Http\ContentSecurityPolicy; @@ -61,13 +63,10 @@ public function __construct( } /** - * @param string $setting - * @param string $value - * @return DataResponse * @throws NotPermittedException */ #[AuthorizedAdminSetting(settings: Admin::class)] - public function updateStylesheet($setting, $value) { + public function updateStylesheet(string $setting, string $value): DataResponse { $value = trim($value); $error = null; $saved = false; @@ -153,13 +152,10 @@ public function updateStylesheet($setting, $value) { } /** - * @param string $setting - * @param mixed $value - * @return DataResponse * @throws NotPermittedException */ #[AuthorizedAdminSetting(settings: Admin::class)] - public function updateAppMenu($setting, $value) { + public function updateAppMenu(string $setting, mixed $value): DataResponse { $error = null; switch ($setting) { case 'defaultApps': @@ -204,7 +200,6 @@ private function isValidUrl(string $url): bool { } /** - * @return DataResponse * @throws NotPermittedException */ #[AuthorizedAdminSetting(settings: Admin::class)] @@ -366,9 +361,6 @@ public function getImage(string $key, bool $useSvg = true) { } /** - * @NoSameSiteCookieRequired - * @NoTwoFactorRequired - * * Get the CSS stylesheet for a theme * * @param string $themeId ID of the theme @@ -381,7 +373,9 @@ public function getImage(string $key, bool $useSvg = true) { */ #[PublicPage] #[NoCSRFRequired] + #[NoTwoFactorRequired] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] + #[NoSameSiteCookieRequired] public function getThemeStylesheet(string $themeId, bool $plain = false, bool $withCustomCss = false) { $themes = $this->themesService->getThemes(); if (!in_array($themeId, array_keys($themes))) { diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index 8cf88137835d1..064de4d49df6d 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -3324,13 +3324,6 @@ - - - - - - - request->server]]> diff --git a/core/Controller/AvatarController.php b/core/Controller/AvatarController.php index 1413c9d678e9a..3d6f79151e80c 100644 --- a/core/Controller/AvatarController.php +++ b/core/Controller/AvatarController.php @@ -13,6 +13,7 @@ use OCP\AppFramework\Http\Attribute\FrontpageRoute; use OCP\AppFramework\Http\Attribute\NoAdminRequired; use OCP\AppFramework\Http\Attribute\NoCSRFRequired; +use OCP\AppFramework\Http\Attribute\NoSameSiteCookieRequired; use OCP\AppFramework\Http\Attribute\OpenAPI; use OCP\AppFramework\Http\Attribute\PublicPage; use OCP\AppFramework\Http\FileDisplayResponse; @@ -50,8 +51,6 @@ public function __construct( } /** - * @NoSameSiteCookieRequired - * * Get the dark avatar * * @param string $userId ID of the user @@ -67,6 +66,7 @@ public function __construct( #[PublicPage] #[FrontpageRoute(verb: 'GET', url: '/avatar/{userId}/{size}/dark')] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] + #[NoSameSiteCookieRequired] public function getAvatarDark(string $userId, int $size, bool $guestFallback = false) { if ($size <= 64) { if ($size !== 64) { @@ -102,8 +102,6 @@ public function getAvatarDark(string $userId, int $size, bool $guestFallback = f /** - * @NoSameSiteCookieRequired - * * Get the avatar * * @param string $userId ID of the user @@ -119,6 +117,7 @@ public function getAvatarDark(string $userId, int $size, bool $guestFallback = f #[PublicPage] #[FrontpageRoute(verb: 'GET', url: '/avatar/{userId}/{size}')] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] + #[NoSameSiteCookieRequired] public function getAvatar(string $userId, int $size, bool $guestFallback = false) { if ($size <= 64) { if ($size !== 64) { diff --git a/core/Controller/CSRFTokenController.php b/core/Controller/CSRFTokenController.php index edf7c26e94c86..85b187f42e6f9 100644 --- a/core/Controller/CSRFTokenController.php +++ b/core/Controller/CSRFTokenController.php @@ -13,6 +13,7 @@ use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\FrontpageRoute; use OCP\AppFramework\Http\Attribute\NoCSRFRequired; +use OCP\AppFramework\Http\Attribute\NoTwoFactorRequired; use OCP\AppFramework\Http\Attribute\OpenAPI; use OCP\AppFramework\Http\Attribute\PublicPage; use OCP\AppFramework\Http\JSONResponse; @@ -34,13 +35,12 @@ public function __construct( * * 200: CSRF token returned * 403: Strict cookie check failed - * - * @NoTwoFactorRequired */ #[PublicPage] #[NoCSRFRequired] #[FrontpageRoute(verb: 'GET', url: '/csrftoken')] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] + #[NoTwoFactorRequired] public function index(): JSONResponse { if (!$this->request->passesStrictCookieCheck()) { return new JSONResponse([], Http::STATUS_FORBIDDEN); diff --git a/core/Controller/ClientFlowLoginController.php b/core/Controller/ClientFlowLoginController.php index 4464af890c43d..b53852a4b7be2 100644 --- a/core/Controller/ClientFlowLoginController.php +++ b/core/Controller/ClientFlowLoginController.php @@ -17,6 +17,7 @@ use OCP\AppFramework\Http\Attribute\FrontpageRoute; use OCP\AppFramework\Http\Attribute\NoAdminRequired; use OCP\AppFramework\Http\Attribute\NoCSRFRequired; +use OCP\AppFramework\Http\Attribute\NoSameSiteCookieRequired; use OCP\AppFramework\Http\Attribute\OpenAPI; use OCP\AppFramework\Http\Attribute\PasswordConfirmationRequired; use OCP\AppFramework\Http\Attribute\PublicPage; @@ -157,13 +158,11 @@ public function showAuthPickerPage(string $clientIdentifier = '', string $user = return $response; } - /** - * @NoSameSiteCookieRequired - */ #[NoAdminRequired] #[NoCSRFRequired] #[UseSession] #[FrontpageRoute(verb: 'GET', url: '/login/flow/grant')] + #[NoSameSiteCookieRequired] public function grantPage( string $stateToken = '', string $clientIdentifier = '', diff --git a/core/Controller/ClientFlowLoginV2Controller.php b/core/Controller/ClientFlowLoginV2Controller.php index 8c0c1e8179d92..4fc7e6f1ba5c1 100644 --- a/core/Controller/ClientFlowLoginV2Controller.php +++ b/core/Controller/ClientFlowLoginV2Controller.php @@ -18,6 +18,7 @@ use OCP\AppFramework\Http\Attribute\FrontpageRoute; use OCP\AppFramework\Http\Attribute\NoAdminRequired; use OCP\AppFramework\Http\Attribute\NoCSRFRequired; +use OCP\AppFramework\Http\Attribute\NoSameSiteCookieRequired; use OCP\AppFramework\Http\Attribute\OpenAPI; use OCP\AppFramework\Http\Attribute\PasswordConfirmationRequired; use OCP\AppFramework\Http\Attribute\PublicPage; @@ -137,14 +138,12 @@ public function showAuthPickerPage(string $user = '', int $direct = 0): Standalo ); } - /** - * @NoSameSiteCookieRequired - */ #[NoAdminRequired] #[NoCSRFRequired] #[OpenAPI(scope: OpenAPI::SCOPE_IGNORE)] #[UseSession] #[FrontpageRoute(verb: 'GET', url: '/login/v2/grant')] + #[NoSameSiteCookieRequired] public function grantPage(?string $stateToken, int $direct = 0): StandaloneTemplateResponse { if ($stateToken === null) { return $this->stateTokenMissingResponse(); diff --git a/core/Controller/CssController.php b/core/Controller/CssController.php index 37e7edc530ff5..b6b0777fe40ae 100644 --- a/core/Controller/CssController.php +++ b/core/Controller/CssController.php @@ -13,11 +13,11 @@ use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\FrontpageRoute; use OCP\AppFramework\Http\Attribute\NoCSRFRequired; +use OCP\AppFramework\Http\Attribute\NoSameSiteCookieRequired; use OCP\AppFramework\Http\Attribute\OpenAPI; use OCP\AppFramework\Http\Attribute\PublicPage; use OCP\AppFramework\Http\FileDisplayResponse; use OCP\AppFramework\Http\NotFoundResponse; -use OCP\AppFramework\Http\Response; use OCP\AppFramework\Utility\ITimeFactory; use OCP\Files\IAppData; use OCP\Files\NotFoundException; @@ -41,21 +41,19 @@ public function __construct( } /** - * @NoSameSiteCookieRequired - * * @param string $fileName css filename with extension * @param string $appName css folder name - * @return FileDisplayResponse|NotFoundResponse */ #[PublicPage] #[NoCSRFRequired] #[FrontpageRoute(verb: 'GET', url: '/css/{appName}/{fileName}')] - public function getCss(string $fileName, string $appName): Response { + #[NoSameSiteCookieRequired] + public function getCss(string $fileName, string $appName): FileDisplayResponse|NotFoundResponse { try { $folder = $this->appData->getFolder($appName); $gzip = false; $file = $this->getFile($folder, $fileName, $gzip); - } catch (NotFoundException $e) { + } catch (NotFoundException) { return new NotFoundResponse(); } diff --git a/core/Controller/JsController.php b/core/Controller/JsController.php index bb40a4f21170a..f214d3043c214 100644 --- a/core/Controller/JsController.php +++ b/core/Controller/JsController.php @@ -13,11 +13,12 @@ use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\FrontpageRoute; use OCP\AppFramework\Http\Attribute\NoCSRFRequired; +use OCP\AppFramework\Http\Attribute\NoSameSiteCookieRequired; +use OCP\AppFramework\Http\Attribute\NoTwoFactorRequired; use OCP\AppFramework\Http\Attribute\OpenAPI; use OCP\AppFramework\Http\Attribute\PublicPage; use OCP\AppFramework\Http\FileDisplayResponse; use OCP\AppFramework\Http\NotFoundResponse; -use OCP\AppFramework\Http\Response; use OCP\AppFramework\Utility\ITimeFactory; use OCP\Files\IAppData; use OCP\Files\NotFoundException; @@ -41,17 +42,15 @@ public function __construct( } /** - * @NoSameSiteCookieRequired - * @NoTwoFactorRequired - * * @param string $fileName js filename with extension * @param string $appName js folder name - * @return FileDisplayResponse|NotFoundResponse */ #[PublicPage] #[NoCSRFRequired] #[FrontpageRoute(verb: 'GET', url: '/js/{appName}/{fileName}')] - public function getJs(string $fileName, string $appName): Response { + #[NoTwoFactorRequired] + #[NoSameSiteCookieRequired] + public function getJs(string $fileName, string $appName): FileDisplayResponse|NotFoundResponse { try { $folder = $this->appData->getFolder($appName); $gzip = false; @@ -76,15 +75,11 @@ public function getJs(string $fileName, string $appName): Response { } /** - * @NoTwoFactorRequired - * - * @param ISimpleFolder $folder - * @param string $fileName * @param bool $gzip is set to true if we use the gzip file - * @return ISimpleFile * * @throws NotFoundException */ + #[NoTwoFactorRequired] private function getFile(ISimpleFolder $folder, string $fileName, bool &$gzip): ISimpleFile { $encoding = $this->request->getHeader('Accept-Encoding'); diff --git a/core/Controller/OCJSController.php b/core/Controller/OCJSController.php index 083ad4b209f4e..db42a3608b5c4 100644 --- a/core/Controller/OCJSController.php +++ b/core/Controller/OCJSController.php @@ -16,6 +16,7 @@ use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\FrontpageRoute; use OCP\AppFramework\Http\Attribute\NoCSRFRequired; +use OCP\AppFramework\Http\Attribute\NoTwoFactorRequired; use OCP\AppFramework\Http\Attribute\OpenAPI; use OCP\AppFramework\Http\Attribute\PublicPage; use OCP\AppFramework\Http\DataDisplayResponse; @@ -75,12 +76,10 @@ public function __construct( ); } - /** - * @NoTwoFactorRequired - */ #[PublicPage] #[NoCSRFRequired] #[FrontpageRoute(verb: 'GET', url: '/core/js/oc.js')] + #[NoTwoFactorRequired] public function getConfig(): DataDisplayResponse { $data = $this->helper->getConfig(); diff --git a/core/Controller/TwoFactorChallengeController.php b/core/Controller/TwoFactorChallengeController.php index 8072bb8e92a04..a10729e456d49 100644 --- a/core/Controller/TwoFactorChallengeController.php +++ b/core/Controller/TwoFactorChallengeController.php @@ -7,6 +7,7 @@ */ namespace OC\Core\Controller; +use OC\AppFramework\Http\Attributes\TwoFactorSetUpDoneRequired; use OC\Authentication\TwoFactorAuth\Manager; use OC_User; use OCP\AppFramework\Controller; @@ -67,16 +68,11 @@ private function splitProvidersAndBackupCodes(array $providers): array { return [$regular, $backup]; } - /** - * @TwoFactorSetUpDoneRequired - * - * @param string $redirect_url - * @return StandaloneTemplateResponse - */ #[NoAdminRequired] #[NoCSRFRequired] #[FrontpageRoute(verb: 'GET', url: '/login/selectchallenge')] - public function selectChallenge($redirect_url) { + #[TwoFactorSetUpDoneRequired] + public function selectChallenge(string $redirect_url): StandaloneTemplateResponse { $user = $this->userSession->getUser(); $providerSet = $this->twoFactorManager->getProviderSet($user); $allProviders = $providerSet->getProviders(); @@ -95,18 +91,12 @@ public function selectChallenge($redirect_url) { return new StandaloneTemplateResponse($this->appName, 'twofactorselectchallenge', $data, 'guest'); } - /** - * @TwoFactorSetUpDoneRequired - * - * @param string $challengeProviderId - * @param string $redirect_url - * @return StandaloneTemplateResponse|RedirectResponse - */ #[NoAdminRequired] #[NoCSRFRequired] #[UseSession] + #[TwoFactorSetUpDoneRequired] #[FrontpageRoute(verb: 'GET', url: '/login/challenge/{challengeProviderId}')] - public function showChallenge($challengeProviderId, $redirect_url) { + public function showChallenge(string $challengeProviderId, string $redirect_url): StandaloneTemplateResponse|RedirectResponse { $user = $this->userSession->getUser(); $providerSet = $this->twoFactorManager->getProviderSet($user); $provider = $providerSet->getProvider($challengeProviderId); @@ -148,21 +138,13 @@ public function showChallenge($challengeProviderId, $redirect_url) { return $response; } - /** - * @TwoFactorSetUpDoneRequired - * - * - * @param string $challengeProviderId - * @param string $challenge - * @param string $redirect_url - * @return RedirectResponse - */ #[NoAdminRequired] #[NoCSRFRequired] #[UseSession] #[FrontpageRoute(verb: 'POST', url: '/login/challenge/{challengeProviderId}')] + #[TwoFactorSetUpDoneRequired] #[UserRateLimit(limit: 5, period: 100)] - public function solveChallenge($challengeProviderId, $challenge, $redirect_url = null) { + public function solveChallenge(string $challengeProviderId, string $challenge, ?string $redirect_url = null): RedirectResponse { $user = $this->userSession->getUser(); $provider = $this->twoFactorManager->getProvider($user, $challengeProviderId); if (is_null($provider)) { diff --git a/core/Middleware/TwoFactorMiddleware.php b/core/Middleware/TwoFactorMiddleware.php index 0dea402f127ab..02f8ab8dad316 100644 --- a/core/Middleware/TwoFactorMiddleware.php +++ b/core/Middleware/TwoFactorMiddleware.php @@ -10,6 +10,8 @@ namespace OC\Core\Middleware; use Exception; +use OC\AppFramework\Http\Attributes\TwoFactorSetUpDoneRequired; +use OC\AppFramework\Middleware\MiddlewareUtils; use OC\Authentication\Exceptions\TwoFactorAuthRequiredException; use OC\Authentication\Exceptions\UserAlreadyLoggedInException; use OC\Authentication\TwoFactorAuth\Manager; @@ -18,14 +20,15 @@ use OC\User\Session; use OCA\TwoFactorNextcloudNotification\Controller\APIController; use OCP\AppFramework\Controller; +use OCP\AppFramework\Http\Attribute\NoTwoFactorRequired; use OCP\AppFramework\Http\RedirectResponse; use OCP\AppFramework\Middleware; -use OCP\AppFramework\Utility\IControllerMethodReflector; use OCP\Authentication\TwoFactorAuth\ALoginSetupController; use OCP\IRequest; use OCP\ISession; use OCP\IURLGenerator; use OCP\IUser; +use ReflectionMethod; class TwoFactorMiddleware extends Middleware { public function __construct( @@ -33,7 +36,7 @@ public function __construct( private Session $userSession, private ISession $session, private IURLGenerator $urlGenerator, - private IControllerMethodReflector $reflector, + private MiddlewareUtils $middlewareUtils, private IRequest $request, ) { } @@ -43,7 +46,9 @@ public function __construct( * @param string $methodName */ public function beforeController($controller, $methodName) { - if ($this->reflector->hasAnnotation('NoTwoFactorRequired')) { + $reflectionMethod = new ReflectionMethod($controller, $methodName); + + if ($this->middlewareUtils->hasAnnotationOrAttribute($reflectionMethod, 'NoTwoFactorRequired', NoTwoFactorRequired::class)) { // Route handler explicitly marked to work without finished 2FA are // not blocked return; @@ -56,7 +61,7 @@ public function beforeController($controller, $methodName) { if ($controller instanceof TwoFactorChallengeController && $this->userSession->getUser() !== null - && !$this->reflector->hasAnnotation('TwoFactorSetUpDoneRequired')) { + && !$reflectionMethod->getAttributes(TwoFactorSetUpDoneRequired::class)) { $providers = $this->twoFactorManager->getProviderSet($this->userSession->getUser()); if (!($providers->getPrimaryProviders() === [] && !$providers->isProviderMissing())) { @@ -86,7 +91,7 @@ public function beforeController($controller, $methodName) { || $this->session->exists('app_api') // authenticated using an AppAPI Auth || $this->twoFactorManager->isTwoFactorAuthenticated($user)) { - $this->checkTwoFactor($controller, $methodName, $user); + $this->checkTwoFactor($controller, $user); } elseif ($controller instanceof TwoFactorChallengeController) { // Allow access to the two-factor controllers only if two-factor authentication // is in progress. @@ -96,7 +101,7 @@ public function beforeController($controller, $methodName) { // TODO: dont check/enforce 2FA if a auth token is used } - private function checkTwoFactor(Controller $controller, $methodName, IUser $user) { + private function checkTwoFactor(Controller $controller, IUser $user) { // If two-factor auth is in progress disallow access to any controllers // defined within "LoginController". $needsSecondFactor = $this->twoFactorManager->needsSecondFactor($user); diff --git a/lib/composer/composer/LICENSE b/lib/composer/composer/LICENSE index 62ecfd8d0046b..f27399a042d95 100644 --- a/lib/composer/composer/LICENSE +++ b/lib/composer/composer/LICENSE @@ -1,3 +1,4 @@ + Copyright (c) Nils Adermann, Jordi Boggiano Permission is hereby granted, free of charge, to any person obtaining a copy @@ -17,3 +18,4 @@ AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 0b6dd29f2040b..002d4904fedf3 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -92,6 +92,8 @@ 'OCP\\AppFramework\\Http\\Attribute\\IgnoreOpenAPI' => $baseDir . '/lib/public/AppFramework/Http/Attribute/IgnoreOpenAPI.php', 'OCP\\AppFramework\\Http\\Attribute\\NoAdminRequired' => $baseDir . '/lib/public/AppFramework/Http/Attribute/NoAdminRequired.php', 'OCP\\AppFramework\\Http\\Attribute\\NoCSRFRequired' => $baseDir . '/lib/public/AppFramework/Http/Attribute/NoCSRFRequired.php', + 'OCP\\AppFramework\\Http\\Attribute\\NoSameSiteCookieRequired' => $baseDir . '/lib/public/AppFramework/Http/Attribute/NoSameSiteCookieRequired.php', + 'OCP\\AppFramework\\Http\\Attribute\\NoTwoFactorRequired' => $baseDir . '/lib/public/AppFramework/Http/Attribute/NoTwoFactorRequired.php', 'OCP\\AppFramework\\Http\\Attribute\\OpenAPI' => $baseDir . '/lib/public/AppFramework/Http/Attribute/OpenAPI.php', 'OCP\\AppFramework\\Http\\Attribute\\PasswordConfirmationRequired' => $baseDir . '/lib/public/AppFramework/Http/Attribute/PasswordConfirmationRequired.php', 'OCP\\AppFramework\\Http\\Attribute\\PublicPage' => $baseDir . '/lib/public/AppFramework/Http/Attribute/PublicPage.php', @@ -1053,6 +1055,7 @@ 'OC\\AppFramework\\Bootstrap\\ServiceRegistration' => $baseDir . '/lib/private/AppFramework/Bootstrap/ServiceRegistration.php', 'OC\\AppFramework\\DependencyInjection\\DIContainer' => $baseDir . '/lib/private/AppFramework/DependencyInjection/DIContainer.php', 'OC\\AppFramework\\Http' => $baseDir . '/lib/private/AppFramework/Http.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', 'OC\\AppFramework\\Http\\Request' => $baseDir . '/lib/private/AppFramework/Http/Request.php', @@ -1061,6 +1064,7 @@ 'OC\\AppFramework\\Middleware\\CompressionMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/CompressionMiddleware.php', 'OC\\AppFramework\\Middleware\\FlowV2EphemeralSessionsMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/FlowV2EphemeralSessionsMiddleware.php', 'OC\\AppFramework\\Middleware\\MiddlewareDispatcher' => $baseDir . '/lib/private/AppFramework/Middleware/MiddlewareDispatcher.php', + 'OC\\AppFramework\\Middleware\\MiddlewareUtils' => $baseDir . '/lib/private/AppFramework/Middleware/MiddlewareUtils.php', 'OC\\AppFramework\\Middleware\\NotModifiedMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/NotModifiedMiddleware.php', 'OC\\AppFramework\\Middleware\\OCSMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/OCSMiddleware.php', 'OC\\AppFramework\\Middleware\\PublicShare\\Exceptions\\NeedAuthenticationException' => $baseDir . '/lib/private/AppFramework/Middleware/PublicShare/Exceptions/NeedAuthenticationException.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 9c34d2cd4debd..9001b80fcdd3c 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -133,6 +133,8 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OCP\\AppFramework\\Http\\Attribute\\IgnoreOpenAPI' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/IgnoreOpenAPI.php', 'OCP\\AppFramework\\Http\\Attribute\\NoAdminRequired' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/NoAdminRequired.php', 'OCP\\AppFramework\\Http\\Attribute\\NoCSRFRequired' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/NoCSRFRequired.php', + 'OCP\\AppFramework\\Http\\Attribute\\NoSameSiteCookieRequired' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/NoSameSiteCookieRequired.php', + 'OCP\\AppFramework\\Http\\Attribute\\NoTwoFactorRequired' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/NoTwoFactorRequired.php', 'OCP\\AppFramework\\Http\\Attribute\\OpenAPI' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/OpenAPI.php', 'OCP\\AppFramework\\Http\\Attribute\\PasswordConfirmationRequired' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/PasswordConfirmationRequired.php', 'OCP\\AppFramework\\Http\\Attribute\\PublicPage' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/PublicPage.php', @@ -1094,6 +1096,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\AppFramework\\Bootstrap\\ServiceRegistration' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Bootstrap/ServiceRegistration.php', 'OC\\AppFramework\\DependencyInjection\\DIContainer' => __DIR__ . '/../../..' . '/lib/private/AppFramework/DependencyInjection/DIContainer.php', 'OC\\AppFramework\\Http' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Http.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', 'OC\\AppFramework\\Http\\Request' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Http/Request.php', @@ -1102,6 +1105,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\AppFramework\\Middleware\\CompressionMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/CompressionMiddleware.php', 'OC\\AppFramework\\Middleware\\FlowV2EphemeralSessionsMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/FlowV2EphemeralSessionsMiddleware.php', 'OC\\AppFramework\\Middleware\\MiddlewareDispatcher' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/MiddlewareDispatcher.php', + 'OC\\AppFramework\\Middleware\\MiddlewareUtils' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/MiddlewareUtils.php', 'OC\\AppFramework\\Middleware\\NotModifiedMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/NotModifiedMiddleware.php', 'OC\\AppFramework\\Middleware\\OCSMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/OCSMiddleware.php', 'OC\\AppFramework\\Middleware\\PublicShare\\Exceptions\\NeedAuthenticationException' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/PublicShare/Exceptions/NeedAuthenticationException.php', diff --git a/lib/private/AppFramework/DependencyInjection/DIContainer.php b/lib/private/AppFramework/DependencyInjection/DIContainer.php index 78951d99364db..82b840b807282 100644 --- a/lib/private/AppFramework/DependencyInjection/DIContainer.php +++ b/lib/private/AppFramework/DependencyInjection/DIContainer.php @@ -17,6 +17,7 @@ use OC\AppFramework\Middleware\CompressionMiddleware; use OC\AppFramework\Middleware\FlowV2EphemeralSessionsMiddleware; use OC\AppFramework\Middleware\MiddlewareDispatcher; +use OC\AppFramework\Middleware\MiddlewareUtils; use OC\AppFramework\Middleware\NotModifiedMiddleware; use OC\AppFramework\Middleware\OCSMiddleware; use OC\AppFramework\Middleware\PublicShare\PublicShareMiddleware; @@ -31,6 +32,7 @@ use OC\AppFramework\Middleware\Security\SecurityMiddleware; use OC\AppFramework\Middleware\SessionMiddleware; use OC\AppFramework\ScopedPsrLogger; +use OC\AppFramework\Utility\ControllerMethodReflector; use OC\AppFramework\Utility\SimpleContainer; use OC\Core\Middleware\TwoFactorMiddleware; use OC\Diagnostics\EventLogger; @@ -44,7 +46,6 @@ use OCP\AppFramework\QueryException; use OCP\AppFramework\Services\IAppConfig; use OCP\AppFramework\Services\IInitialState; -use OCP\AppFramework\Utility\IControllerMethodReflector; use OCP\Files\AppData\IAppDataFactory; use OCP\Files\Folder; use OCP\Files\IAppData; @@ -155,7 +156,7 @@ public function __construct(string $appName, array $urlParams = [], ?ServerConta return new Dispatcher( $c->get(Http::class), $c->get(MiddlewareDispatcher::class), - $c->get(IControllerMethodReflector::class), + $c->get(ControllerMethodReflector::class), $c->get(IRequest::class), $c->get(IConfig::class), $c->get(IDBConnection::class), @@ -193,7 +194,7 @@ public function __construct(string $appName, array $urlParams = [], ?ServerConta $securityMiddleware = new SecurityMiddleware( $c->get(IRequest::class), - $c->get(IControllerMethodReflector::class), + $c->get(MiddlewareUtils::class), $c->get(INavigationManager::class), $c->get(IURLGenerator::class), $c->get(LoggerInterface::class), diff --git a/lib/private/AppFramework/Http/Attributes/TwoFactorSetUpDoneRequired.php b/lib/private/AppFramework/Http/Attributes/TwoFactorSetUpDoneRequired.php new file mode 100644 index 0000000000000..377d4b959ab60 --- /dev/null +++ b/lib/private/AppFramework/Http/Attributes/TwoFactorSetUpDoneRequired.php @@ -0,0 +1,18 @@ +protocol = $protocol; - $this->middlewareDispatcher = $middlewareDispatcher; - $this->reflector = $reflector; - $this->request = $request; - $this->config = $config; - $this->connection = $connection; - $this->logger = $logger; - $this->eventLogger = $eventLogger; - $this->appContainer = $appContainer; } @@ -92,16 +50,15 @@ public function __construct( * @param Controller $controller the controller which will be called * @param string $methodName the method name which will be called on * the controller - * @return array $array[0] contains the http status header as a string, - * $array[1] contains response headers as an array, - * $array[2] contains response cookies as an array, - * $array[3] contains the response output as a string, - * $array[4] contains the response object + * @return array{0: string, 1: array, 2: array, 3: string, 4: Response} + * $array[0] contains the http status header as a string, + * $array[1] contains response headers as an array, + * $array[2] contains response cookies as an array, + * $array[3] contains the response output as a string, + * $array[4] contains the response object * @throws \Exception */ public function dispatch(Controller $controller, string $methodName): array { - $out = [null, [], null]; - try { // prefill reflector with everything that's needed for the // middlewares @@ -156,15 +113,15 @@ public function dispatch(Controller $controller, string $methodName): array { $controller, $methodName, $response); // depending on the cache object the headers need to be changed - $out[0] = $this->protocol->getStatusHeader($response->getStatus()); - $out[1] = array_merge($response->getHeaders()); - $out[2] = $response->getCookies(); - $out[3] = $this->middlewareDispatcher->beforeOutput( - $controller, $methodName, $response->render() - ); - $out[4] = $response; - - return $out; + return [ + $this->protocol->getStatusHeader($response->getStatus()), + array_merge($response->getHeaders()), + $response->getCookies(), + $this->middlewareDispatcher->beforeOutput( + $controller, $methodName, $response->render() + ), + $response, + ]; } diff --git a/lib/private/AppFramework/Middleware/MiddlewareUtils.php b/lib/private/AppFramework/Middleware/MiddlewareUtils.php new file mode 100644 index 0000000000000..94ac2d1f44c91 --- /dev/null +++ b/lib/private/AppFramework/Middleware/MiddlewareUtils.php @@ -0,0 +1,70 @@ + $attributeClass + * @return boolean + */ + public function hasAnnotationOrAttribute(ReflectionMethod $reflectionMethod, ?string $annotationName, string $attributeClass): bool { + if (!empty($reflectionMethod->getAttributes($attributeClass))) { + return true; + } + + if ($annotationName && $this->reflector->hasAnnotation($annotationName)) { + $this->logger->debug($reflectionMethod->getDeclaringClass()->getName() . '::' . $reflectionMethod->getName() . ' uses the @' . $annotationName . ' annotation and should use the #[' . $attributeClass . '] attribute instead'); + return true; + } + + return false; + } + + /** + * @param ReflectionMethod $reflectionMethod + * @return string[] + */ + public function getAuthorizedAdminSettingClasses(ReflectionMethod $reflectionMethod): array { + $classes = []; + if ($this->reflector->hasAnnotation('AuthorizedAdminSetting')) { + $classes = explode(';', $this->reflector->getAnnotationParameter('AuthorizedAdminSetting', 'settings')); + } + + $attributes = $reflectionMethod->getAttributes(AuthorizedAdminSetting::class); + if (!empty($attributes)) { + foreach ($attributes as $attribute) { + /** @var AuthorizedAdminSetting $setting */ + $setting = $attribute->newInstance(); + $classes[] = $setting->getSettings(); + } + } + + return $classes; + } +} diff --git a/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php b/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php index 4453f5a7d4b4f..2d87f616a9df1 100644 --- a/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php +++ b/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php @@ -7,8 +7,8 @@ */ namespace OC\AppFramework\Middleware\Security; +use OC\AppFramework\Middleware\MiddlewareUtils; use OC\AppFramework\Middleware\Security\Exceptions\SecurityException; -use OC\AppFramework\Utility\ControllerMethodReflector; use OC\Authentication\Exceptions\PasswordLoginForbiddenException; use OC\User\Session; use OCP\AppFramework\Controller; @@ -21,7 +21,7 @@ use OCP\IRequest; use OCP\ISession; use OCP\Security\Bruteforce\IThrottler; -use Psr\Log\LoggerInterface; +use Override; use ReflectionMethod; /** @@ -31,45 +31,23 @@ * https://developer.mozilla.org/en-US/docs/Web/HTTP/Access_control_CORS */ class CORSMiddleware extends Middleware { - /** @var IRequest */ - private $request; - /** @var ControllerMethodReflector */ - private $reflector; - /** @var Session */ - private $session; - /** @var IThrottler */ - private $throttler; public function __construct( - IRequest $request, - ControllerMethodReflector $reflector, - Session $session, - IThrottler $throttler, - private readonly LoggerInterface $logger, + private readonly IRequest $request, + private readonly MiddlewareUtils $middlewareUtils, + private readonly Session $session, + private readonly IThrottler $throttler, ) { - $this->request = $request; - $this->reflector = $reflector; - $this->session = $session; - $this->throttler = $throttler; } - /** - * This is being run in normal order before the controller is being - * called which allows several modifications and checks - * - * @param Controller $controller the controller that is being called - * @param string $methodName the name of the method that will be called on - * the controller - * @throws SecurityException - * @since 6.0.0 - */ - public function beforeController($controller, $methodName) { + #[Override] + public function beforeController(Controller $controller, string $methodName): void { $reflectionMethod = new ReflectionMethod($controller, $methodName); // ensure that @CORS annotated API routes are not used in conjunction // with session authentication since this enables CSRF attack vectors - if ($this->hasAnnotationOrAttribute($reflectionMethod, 'CORS', CORS::class) - && (!$this->hasAnnotationOrAttribute($reflectionMethod, 'PublicPage', PublicPage::class) || $this->session->isLoggedIn())) { + if ($this->middlewareUtils->hasAnnotationOrAttribute($reflectionMethod, 'CORS', CORS::class) + && (!$this->middlewareUtils->hasAnnotationOrAttribute($reflectionMethod, 'PublicPage', PublicPage::class) || $this->session->isLoggedIn())) { $user = array_key_exists('PHP_AUTH_USER', $this->request->server) ? $this->request->server['PHP_AUTH_USER'] : null; $pass = array_key_exists('PHP_AUTH_PW', $this->request->server) ? $this->request->server['PHP_AUTH_PW'] : null; @@ -92,45 +70,13 @@ public function beforeController($controller, $methodName) { } } - /** - * @template T - * - * @param ReflectionMethod $reflectionMethod - * @param string $annotationName - * @param class-string $attributeClass - * @return boolean - */ - protected function hasAnnotationOrAttribute(ReflectionMethod $reflectionMethod, string $annotationName, string $attributeClass): bool { - if ($this->reflector->hasAnnotation($annotationName)) { - $this->logger->debug($reflectionMethod->getDeclaringClass()->getName() . '::' . $reflectionMethod->getName() . ' uses the @' . $annotationName . ' annotation and should use the #[' . $attributeClass . '] attribute instead'); - return true; - } - - - if (!empty($reflectionMethod->getAttributes($attributeClass))) { - return true; - } - - return false; - } - - /** - * This is being run after a successful controller method call and allows - * the manipulation of a Response object. The middleware is run in reverse order - * - * @param Controller $controller the controller that is being called - * @param string $methodName the name of the method that will be called on - * the controller - * @param Response $response the generated response from the controller - * @return Response a Response object - * @throws SecurityException - */ - public function afterController($controller, $methodName, Response $response) { + #[Override] + public function afterController(Controller $controller, string $methodName, Response $response): Response { // only react if it's a CORS request and if the request sends origin and if (isset($this->request->server['HTTP_ORIGIN'])) { $reflectionMethod = new ReflectionMethod($controller, $methodName); - if ($this->hasAnnotationOrAttribute($reflectionMethod, 'CORS', CORS::class)) { + if ($this->middlewareUtils->hasAnnotationOrAttribute($reflectionMethod, 'CORS', CORS::class)) { // allow credentials headers must not be true or CSRF is possible // otherwise foreach ($response->getHeaders() as $header => $value) { @@ -151,15 +97,9 @@ public function afterController($controller, $methodName, Response $response) { /** * If an SecurityException is being caught return a JSON error response - * - * @param Controller $controller the controller that is being called - * @param string $methodName the name of the method that will be called on - * the controller - * @param \Exception $exception the thrown exception - * @throws \Exception the passed in exception if it can't handle it - * @return Response a Response object or null in case that the exception could not be handled */ - public function afterException($controller, $methodName, \Exception $exception) { + #[Override] + public function afterException(Controller $controller, string $methodName, \Exception $exception): Response { if ($exception instanceof SecurityException) { $response = new JSONResponse(['message' => $exception->getMessage()]); if ($exception->getCode() !== 0) { diff --git a/lib/private/AppFramework/Middleware/Security/SameSiteCookieMiddleware.php b/lib/private/AppFramework/Middleware/Security/SameSiteCookieMiddleware.php index 097ed1b2b8f01..7334b56bac1b1 100644 --- a/lib/private/AppFramework/Middleware/Security/SameSiteCookieMiddleware.php +++ b/lib/private/AppFramework/Middleware/Security/SameSiteCookieMiddleware.php @@ -1,5 +1,7 @@ reflector->hasAnnotation('NoSameSiteCookieRequired'); + $reflectionMethod = new ReflectionMethod($controller, $methodName); + $noSSC = $this->middlewareUtils->hasAnnotationOrAttribute($reflectionMethod, 'NoSameSiteCookieRequired', NoSameSiteCookieRequired::class); if ($noSSC) { return; } diff --git a/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php b/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php index e3a293e0fd9d0..d0ca1b2ebe18d 100644 --- a/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php +++ b/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php @@ -8,6 +8,7 @@ */ namespace OC\AppFramework\Middleware\Security; +use OC\AppFramework\Middleware\MiddlewareUtils; use OC\AppFramework\Middleware\Security\Exceptions\AdminIpNotAllowedException; use OC\AppFramework\Middleware\Security\Exceptions\AppNotEnabledException; use OC\AppFramework\Middleware\Security\Exceptions\CrossSiteRequestForgeryException; @@ -16,7 +17,6 @@ use OC\AppFramework\Middleware\Security\Exceptions\NotLoggedInException; use OC\AppFramework\Middleware\Security\Exceptions\SecurityException; use OC\AppFramework\Middleware\Security\Exceptions\StrictCookieMissingException; -use OC\AppFramework\Utility\ControllerMethodReflector; use OC\Settings\AuthorizedGroupMapper; use OC\User\Session; use OCP\App\AppPathNotFoundException; @@ -59,20 +59,20 @@ class SecurityMiddleware extends Middleware { private ?bool $isSubAdmin = null; public function __construct( - private IRequest $request, - private ControllerMethodReflector $reflector, - private INavigationManager $navigationManager, - private IURLGenerator $urlGenerator, - private LoggerInterface $logger, - private string $appName, - private bool $isLoggedIn, - private IGroupManager $groupManager, - private ISubAdmin $subAdminManager, - private IAppManager $appManager, - private IL10N $l10n, - private AuthorizedGroupMapper $groupAuthorizationMapper, - private IUserSession $userSession, - private IRemoteAddress $remoteAddress, + private readonly IRequest $request, + private readonly MiddlewareUtils $middlewareUtils, + private readonly INavigationManager $navigationManager, + private readonly IURLGenerator $urlGenerator, + private readonly LoggerInterface $logger, + private readonly string $appName, + private readonly bool $isLoggedIn, + private readonly IGroupManager $groupManager, + private readonly ISubAdmin $subAdminManager, + private readonly IAppManager $appManager, + private readonly IL10N $l10n, + private readonly AuthorizedGroupMapper $groupAuthorizationMapper, + private readonly IUserSession $userSession, + private readonly IRemoteAddress $remoteAddress, ) { } @@ -115,15 +115,15 @@ public function beforeController($controller, $methodName) { $reflectionMethod = new ReflectionMethod($controller, $methodName); // security checks - $isPublicPage = $this->hasAnnotationOrAttribute($reflectionMethod, 'PublicPage', PublicPage::class); + $isPublicPage = $this->middlewareUtils->hasAnnotationOrAttribute($reflectionMethod, 'PublicPage', PublicPage::class); - if ($this->hasAnnotationOrAttribute($reflectionMethod, 'ExAppRequired', ExAppRequired::class)) { + if ($this->middlewareUtils->hasAnnotationOrAttribute($reflectionMethod, 'ExAppRequired', ExAppRequired::class)) { if (!$this->userSession instanceof Session || $this->userSession->getSession()->get('app_api') !== true) { throw new ExAppRequiredException(); } } elseif (!$isPublicPage) { $authorized = false; - if ($this->hasAnnotationOrAttribute($reflectionMethod, null, AppApiAdminAccessWithoutUser::class)) { + if ($this->middlewareUtils->hasAnnotationOrAttribute($reflectionMethod, null, AppApiAdminAccessWithoutUser::class)) { // this attribute allows ExApp to access admin endpoints only if "userId" is "null" if ($this->userSession instanceof Session && $this->userSession->getSession()->get('app_api') === true && $this->userSession->getUser() === null) { $authorized = true; @@ -134,15 +134,15 @@ public function beforeController($controller, $methodName) { throw new NotLoggedInException(); } - if (!$authorized && $this->hasAnnotationOrAttribute($reflectionMethod, 'AuthorizedAdminSetting', AuthorizedAdminSetting::class)) { + if (!$authorized && $this->middlewareUtils->hasAnnotationOrAttribute($reflectionMethod, 'AuthorizedAdminSetting', AuthorizedAdminSetting::class)) { $authorized = $this->isAdminUser(); - if (!$authorized && $this->hasAnnotationOrAttribute($reflectionMethod, 'SubAdminRequired', SubAdminRequired::class)) { + if (!$authorized && $this->middlewareUtils->hasAnnotationOrAttribute($reflectionMethod, 'SubAdminRequired', SubAdminRequired::class)) { $authorized = $this->isSubAdmin(); } if (!$authorized) { - $settingClasses = $this->getAuthorizedAdminSettingClasses($reflectionMethod); + $settingClasses = $this->middlewareUtils->getAuthorizedAdminSettingClasses($reflectionMethod); $authorizedClasses = $this->groupAuthorizationMapper->findAllClassesForUser($this->userSession->getUser()); foreach ($settingClasses as $settingClass) { $authorized = in_array($settingClass, $authorizedClasses, true); @@ -159,24 +159,24 @@ public function beforeController($controller, $methodName) { throw new AdminIpNotAllowedException($this->l10n->t('Your current IP address doesn\'t allow you to perform admin actions')); } } - if ($this->hasAnnotationOrAttribute($reflectionMethod, 'SubAdminRequired', SubAdminRequired::class) + if ($this->middlewareUtils->hasAnnotationOrAttribute($reflectionMethod, 'SubAdminRequired', SubAdminRequired::class) && !$this->isSubAdmin() && !$this->isAdminUser() && !$authorized) { throw new NotAdminException($this->l10n->t('Logged in account must be an admin or sub admin')); } - if (!$this->hasAnnotationOrAttribute($reflectionMethod, 'SubAdminRequired', SubAdminRequired::class) - && !$this->hasAnnotationOrAttribute($reflectionMethod, 'NoAdminRequired', NoAdminRequired::class) + if (!$this->middlewareUtils->hasAnnotationOrAttribute($reflectionMethod, 'SubAdminRequired', SubAdminRequired::class) + && !$this->middlewareUtils->hasAnnotationOrAttribute($reflectionMethod, 'NoAdminRequired', NoAdminRequired::class) && !$this->isAdminUser() && !$authorized) { throw new NotAdminException($this->l10n->t('Logged in account must be an admin')); } - if ($this->hasAnnotationOrAttribute($reflectionMethod, 'SubAdminRequired', SubAdminRequired::class) + if ($this->middlewareUtils->hasAnnotationOrAttribute($reflectionMethod, 'SubAdminRequired', SubAdminRequired::class) && !$this->remoteAddress->allowsAdminActions()) { throw new AdminIpNotAllowedException($this->l10n->t('Your current IP address doesn\'t allow you to perform admin actions')); } - if (!$this->hasAnnotationOrAttribute($reflectionMethod, 'SubAdminRequired', SubAdminRequired::class) - && !$this->hasAnnotationOrAttribute($reflectionMethod, 'NoAdminRequired', NoAdminRequired::class) + if (!$this->middlewareUtils->hasAnnotationOrAttribute($reflectionMethod, 'SubAdminRequired', SubAdminRequired::class) + && !$this->middlewareUtils->hasAnnotationOrAttribute($reflectionMethod, 'NoAdminRequired', NoAdminRequired::class) && !$this->remoteAddress->allowsAdminActions()) { throw new AdminIpNotAllowedException($this->l10n->t('Your current IP address doesn\'t allow you to perform admin actions')); } @@ -184,8 +184,8 @@ public function beforeController($controller, $methodName) { } // Check for strict cookie requirement - if ($this->hasAnnotationOrAttribute($reflectionMethod, 'StrictCookieRequired', StrictCookiesRequired::class) - || !$this->hasAnnotationOrAttribute($reflectionMethod, 'NoCSRFRequired', NoCSRFRequired::class)) { + if ($this->middlewareUtils->hasAnnotationOrAttribute($reflectionMethod, 'StrictCookieRequired', StrictCookiesRequired::class) + || !$this->middlewareUtils->hasAnnotationOrAttribute($reflectionMethod, 'NoCSRFRequired', NoCSRFRequired::class)) { if (!$this->request->passesStrictCookieCheck()) { throw new StrictCookieMissingException(); } @@ -224,7 +224,7 @@ public function beforeController($controller, $methodName) { } private function isInvalidCSRFRequired(ReflectionMethod $reflectionMethod): bool { - if ($this->hasAnnotationOrAttribute($reflectionMethod, 'NoCSRFRequired', NoCSRFRequired::class)) { + if ($this->middlewareUtils->hasAnnotationOrAttribute($reflectionMethod, 'NoCSRFRequired', NoCSRFRequired::class)) { return false; } @@ -236,49 +236,6 @@ private function isValidOCSRequest(): bool { || str_starts_with($this->request->getHeader('Authorization'), 'Bearer '); } - /** - * @template T - * - * @param ReflectionMethod $reflectionMethod - * @param ?string $annotationName - * @param class-string $attributeClass - * @return boolean - */ - protected function hasAnnotationOrAttribute(ReflectionMethod $reflectionMethod, ?string $annotationName, string $attributeClass): bool { - if (!empty($reflectionMethod->getAttributes($attributeClass))) { - return true; - } - - if ($annotationName && $this->reflector->hasAnnotation($annotationName)) { - $this->logger->debug($reflectionMethod->getDeclaringClass()->getName() . '::' . $reflectionMethod->getName() . ' uses the @' . $annotationName . ' annotation and should use the #[' . $attributeClass . '] attribute instead'); - return true; - } - - return false; - } - - /** - * @param ReflectionMethod $reflectionMethod - * @return string[] - */ - protected function getAuthorizedAdminSettingClasses(ReflectionMethod $reflectionMethod): array { - $classes = []; - if ($this->reflector->hasAnnotation('AuthorizedAdminSetting')) { - $classes = explode(';', $this->reflector->getAnnotationParameter('AuthorizedAdminSetting', 'settings')); - } - - $attributes = $reflectionMethod->getAttributes(AuthorizedAdminSetting::class); - if (!empty($attributes)) { - foreach ($attributes as $attribute) { - /** @var AuthorizedAdminSetting $setting */ - $setting = $attribute->newInstance(); - $classes[] = $setting->getSettings(); - } - } - - return $classes; - } - /** * If an SecurityException is being caught, ajax requests return a JSON error * response and non ajax requests redirect to the index diff --git a/lib/public/AppFramework/Http/Attribute/NoSameSiteCookieRequired.php b/lib/public/AppFramework/Http/Attribute/NoSameSiteCookieRequired.php new file mode 100644 index 0000000000000..5bbb65cbe2926 --- /dev/null +++ b/lib/public/AppFramework/Http/Attribute/NoSameSiteCookieRequired.php @@ -0,0 +1,25 @@ +session = $this->createMock(ISession::class); $this->urlGenerator = $this->createMock(IURLGenerator::class); $this->reflector = $this->createMock(IControllerMethodReflector::class); + $this->logger = $this->createMock(LoggerInterface::class); $this->request = new Request( [ 'server' => [ @@ -78,8 +102,7 @@ protected function setUp(): void { $this->createMock(IConfig::class) ); - $this->middleware = new TwoFactorMiddleware($this->twoFactorManager, $this->userSession, $this->session, $this->urlGenerator, $this->reflector, $this->request); - $this->controller = $this->createMock(Controller::class); + $this->middleware = new TwoFactorMiddleware($this->twoFactorManager, $this->userSession, $this->session, $this->urlGenerator, new MiddlewareUtils($this->reflector, $this->logger), $this->request); } public function testBeforeControllerNotLoggedIn(): void { @@ -90,12 +113,14 @@ public function testBeforeControllerNotLoggedIn(): void { $this->userSession->expects($this->never()) ->method('getUser'); - $this->middleware->beforeController($this->controller, 'index'); + $controller = $this->getMockBuilder(NoTwoFactorAnnotationController::class) + ->disableOriginalConstructor() + ->getMock(); + $this->middleware->beforeController($controller, 'index'); } public function testBeforeSetupController(): void { $user = $this->createMock(IUser::class); - $controller = $this->createMock(ALoginSetupController::class); $this->userSession->expects($this->any()) ->method('getUser') ->willReturn($user); @@ -105,7 +130,7 @@ public function testBeforeSetupController(): void { $this->userSession->expects($this->never()) ->method('isLoggedIn'); - $this->middleware->beforeController($controller, 'create'); + $this->middleware->beforeController(new LoginSetupController('foo', $this->request), 'index'); } public function testBeforeControllerNoTwoFactorCheckNeeded(): void { @@ -122,7 +147,10 @@ public function testBeforeControllerNoTwoFactorCheckNeeded(): void { ->with($user) ->willReturn(false); - $this->middleware->beforeController($this->controller, 'index'); + $controller = $this->getMockBuilder(NoTwoFactorAnnotationController::class) + ->disableOriginalConstructor() + ->getMock(); + $this->middleware->beforeController($controller, 'index'); } @@ -146,7 +174,10 @@ public function testBeforeControllerTwoFactorAuthRequired(): void { ->with($user) ->willReturn(true); - $this->middleware->beforeController($this->controller, 'index'); + $controller = $this->getMockBuilder(NoTwoFactorAnnotationController::class) + ->disableOriginalConstructor() + ->getMock(); + $this->middleware->beforeController($controller, 'index'); } @@ -155,9 +186,6 @@ public function testBeforeControllerUserAlreadyLoggedIn(): void { $user = $this->createMock(IUser::class); - $this->reflector - ->method('hasAnnotation') - ->willReturn(false); $this->userSession->expects($this->once()) ->method('isLoggedIn') ->willReturn(true); @@ -173,7 +201,7 @@ public function testBeforeControllerUserAlreadyLoggedIn(): void { ->with($user) ->willReturn(false); - $twoFactorChallengeController = $this->getMockBuilder(TwoFactorChallengeController::class) + $twoFactorChallengeController = $this->getMockBuilder(NoTwoFactorChallengeAnnotationController::class) ->disableOriginalConstructor() ->getMock(); $this->middleware->beforeController($twoFactorChallengeController, 'index'); @@ -188,7 +216,8 @@ public function testAfterExceptionTwoFactorAuthRequired(): void { ->willReturn('test/url'); $expected = new RedirectResponse('test/url'); - $this->assertEquals($expected, $this->middleware->afterException($this->controller, 'index', $ex)); + $controller = new HasTwoFactorAnnotationController('foo', $this->request); + $this->assertEquals($expected, $this->middleware->afterException($controller, 'index', $ex)); } public function testAfterException(): void { @@ -200,17 +229,13 @@ public function testAfterException(): void { ->willReturn('redirect/url'); $expected = new RedirectResponse('redirect/url'); - $this->assertEquals($expected, $this->middleware->afterException($this->controller, 'index', $ex)); + $controller = new HasTwoFactorAnnotationController('foo', $this->request); + $this->assertEquals($expected, $this->middleware->afterException($controller, 'index', $ex)); } public function testRequires2FASetupDoneAnnotated(): void { $user = $this->createMock(IUser::class); - $this->reflector - ->method('hasAnnotation') - ->willReturnCallback(function (string $annotation) { - return $annotation === 'TwoFactorSetUpDoneRequired'; - }); $this->userSession->expects($this->once()) ->method('isLoggedIn') ->willReturn(true); @@ -228,10 +253,10 @@ public function testRequires2FASetupDoneAnnotated(): void { $this->expectException(UserAlreadyLoggedInException::class); - $twoFactorChallengeController = $this->getMockBuilder(TwoFactorChallengeController::class) + $controller = $this->getMockBuilder(HasTwoFactorSetUpDoneAnnotationController::class) ->disableOriginalConstructor() ->getMock(); - $this->middleware->beforeController($twoFactorChallengeController, 'index'); + $this->middleware->beforeController($controller, 'index'); } public static function dataRequires2FASetupDone(): array { @@ -243,7 +268,7 @@ public static function dataRequires2FASetupDone(): array { ]; } - #[\PHPUnit\Framework\Attributes\DataProvider('dataRequires2FASetupDone')] + #[DataProvider('dataRequires2FASetupDone')] public function testRequires2FASetupDone(bool $hasProvider, bool $missingProviders, bool $expectEception): void { if ($hasProvider) { $provider = $this->createMock(IProvider::class); @@ -257,9 +282,6 @@ public function testRequires2FASetupDone(bool $hasProvider, bool $missingProvide $user = $this->createMock(IUser::class); - $this->reflector - ->method('hasAnnotation') - ->willReturn(false); $this->userSession ->method('getUser') ->willReturn($user); @@ -278,9 +300,9 @@ public function testRequires2FASetupDone(bool $hasProvider, bool $missingProvide $this->assertTrue(true); } - $twoFactorChallengeController = $this->getMockBuilder(TwoFactorChallengeController::class) + $controller = $this->getMockBuilder(NoTwoFactorChallengeAnnotationController::class) ->disableOriginalConstructor() ->getMock(); - $this->middleware->beforeController($twoFactorChallengeController, 'index'); + $this->middleware->beforeController($controller, 'index'); } } diff --git a/tests/lib/AppFramework/Middleware/Security/CORSMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/CORSMiddlewareTest.php index c325ae638fb96..97541612429e0 100644 --- a/tests/lib/AppFramework/Middleware/Security/CORSMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/CORSMiddlewareTest.php @@ -8,6 +8,7 @@ namespace Test\AppFramework\Middleware\Security; use OC\AppFramework\Http\Request; +use OC\AppFramework\Middleware\MiddlewareUtils; use OC\AppFramework\Middleware\Security\CORSMiddleware; use OC\AppFramework\Middleware\Security\Exceptions\SecurityException; use OC\AppFramework\Utility\ControllerMethodReflector; @@ -24,14 +25,10 @@ use Test\AppFramework\Middleware\Security\Mock\CORSMiddlewareController; class CORSMiddlewareTest extends \Test\TestCase { - /** @var ControllerMethodReflector */ - private $reflector; - /** @var Session|MockObject */ - private $session; - /** @var IThrottler|MockObject */ - private $throttler; - /** @var CORSMiddlewareController */ - private $controller; + private ControllerMethodReflector $reflector; + private Session&MockObject $session; + private IThrottler&MockObject $throttler; + private CORSMiddlewareController $controller; private LoggerInterface $logger; protected function setUp(): void { @@ -65,7 +62,7 @@ public function testSetCORSAPIHeader(string $method): void { $this->createMock(IConfig::class) ); $this->reflector->reflect($this->controller, $method); - $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger); + $middleware = new CORSMiddleware($request, new MiddlewareUtils($this->reflector, $this->logger), $this->session, $this->throttler); $response = $middleware->afterController($this->controller, $method, new Response()); $headers = $response->getHeaders(); @@ -82,7 +79,7 @@ public function testNoAnnotationNoCORSHEADER(): void { $this->createMock(IRequestId::class), $this->createMock(IConfig::class) ); - $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger); + $middleware = new CORSMiddleware($request, new MiddlewareUtils($this->reflector, $this->logger), $this->session, $this->throttler); $response = $middleware->afterController($this->controller, __FUNCTION__, new Response()); $headers = $response->getHeaders(); @@ -104,7 +101,7 @@ public function testNoOriginHeaderNoCORSHEADER(string $method): void { $this->createMock(IConfig::class) ); $this->reflector->reflect($this->controller, $method); - $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger); + $middleware = new CORSMiddleware($request, new MiddlewareUtils($this->reflector, $this->logger), $this->session, $this->throttler); $response = $middleware->afterController($this->controller, $method, new Response()); $headers = $response->getHeaders(); @@ -132,7 +129,7 @@ public function testCorsIgnoredIfWithCredentialsHeaderPresent(string $method): v $this->createMock(IConfig::class) ); $this->reflector->reflect($this->controller, $method); - $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger); + $middleware = new CORSMiddleware($request, new MiddlewareUtils($this->reflector, $this->logger), $this->session, $this->throttler, $this->logger); $response = new Response(); $response->addHeader('AcCess-control-Allow-Credentials ', 'TRUE'); @@ -156,7 +153,7 @@ public function testNoCORSOnAnonymousPublicPage(string $method): void { $this->createMock(IConfig::class) ); $this->reflector->reflect($this->controller, $method); - $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger); + $middleware = new CORSMiddleware($request, new MiddlewareUtils($this->reflector, $this->logger), $this->session, $this->throttler, $this->logger); $this->session->expects($this->once()) ->method('isLoggedIn') ->willReturn(false); @@ -188,7 +185,7 @@ public function testCORSShouldNeverAllowCookieAuth(string $method): void { $this->createMock(IConfig::class) ); $this->reflector->reflect($this->controller, $method); - $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger); + $middleware = new CORSMiddleware($request, new MiddlewareUtils($this->reflector, $this->logger), $this->session, $this->throttler); $this->session->expects($this->once()) ->method('isLoggedIn') ->willReturn(true); @@ -227,7 +224,7 @@ public function testCORSShouldRelogin(string $method): void { ->with($this->equalTo('user'), $this->equalTo('pass')) ->willReturn(true); $this->reflector->reflect($this->controller, $method); - $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger); + $middleware = new CORSMiddleware($request, new MiddlewareUtils($this->reflector, $this->logger), $this->session, $this->throttler); $middleware->beforeController($this->controller, $method); } @@ -258,7 +255,7 @@ public function testCORSShouldFailIfPasswordLoginIsForbidden(string $method): vo ->with($this->equalTo('user'), $this->equalTo('pass')) ->willThrowException(new PasswordLoginForbiddenException); $this->reflector->reflect($this->controller, $method); - $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger); + $middleware = new CORSMiddleware($request, new MiddlewareUtils($this->reflector, $this->logger), $this->session, $this->throttler); $middleware->beforeController($this->controller, $method); } @@ -289,7 +286,7 @@ public function testCORSShouldNotAllowCookieAuth(string $method): void { ->with($this->equalTo('user'), $this->equalTo('pass')) ->willReturn(false); $this->reflector->reflect($this->controller, $method); - $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger); + $middleware = new CORSMiddleware($request, new MiddlewareUtils($this->reflector, $this->logger), $this->session, $this->throttler); $middleware->beforeController($this->controller, $method); } @@ -303,7 +300,7 @@ public function testAfterExceptionWithSecurityExceptionNoStatus(): void { $this->createMock(IRequestId::class), $this->createMock(IConfig::class) ); - $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger); + $middleware = new CORSMiddleware($request, new MiddlewareUtils($this->reflector, $this->logger), $this->session, $this->throttler); $response = $middleware->afterException($this->controller, __FUNCTION__, new SecurityException('A security exception')); $expected = new JSONResponse(['message' => 'A security exception'], 500); @@ -319,7 +316,7 @@ public function testAfterExceptionWithSecurityExceptionWithStatus(): void { $this->createMock(IRequestId::class), $this->createMock(IConfig::class) ); - $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger); + $middleware = new CORSMiddleware($request, new MiddlewareUtils($this->reflector, $this->logger), $this->session, $this->throttler); $response = $middleware->afterException($this->controller, __FUNCTION__, new SecurityException('A security exception', 501)); $expected = new JSONResponse(['message' => 'A security exception'], 501); @@ -338,7 +335,7 @@ public function testAfterExceptionWithRegularException(): void { $this->createMock(IRequestId::class), $this->createMock(IConfig::class) ); - $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger); + $middleware = new CORSMiddleware($request, new MiddlewareUtils($this->reflector, $this->logger), $this->session, $this->throttler); $middleware->afterException($this->controller, __FUNCTION__, new \Exception('A regular exception')); } } diff --git a/tests/lib/AppFramework/Middleware/Security/SameSiteCookieMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/SameSiteCookieMiddlewareTest.php index 7800371f68f33..d6d68f33128a3 100644 --- a/tests/lib/AppFramework/Middleware/Security/SameSiteCookieMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/SameSiteCookieMiddlewareTest.php @@ -8,37 +8,52 @@ namespace Test\AppFramework\Middleware\Security; use OC\AppFramework\Http\Request; +use OC\AppFramework\Middleware\MiddlewareUtils; use OC\AppFramework\Middleware\Security\Exceptions\LaxSameSiteCookieFailedException; use OC\AppFramework\Middleware\Security\Exceptions\SecurityException; use OC\AppFramework\Middleware\Security\SameSiteCookieMiddleware; use OC\AppFramework\Utility\ControllerMethodReflector; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; +use OCP\AppFramework\Http\Attribute\NoSameSiteCookieRequired; +use OCP\AppFramework\Http\Response; +use PHPUnit\Framework\MockObject\MockObject; +use Psr\Log\LoggerInterface; use Test\TestCase; -class SameSiteCookieMiddlewareTest extends TestCase { - /** @var SameSiteCookieMiddleware */ - private $middleware; +class HasAnnotationController extends Controller { + #[NoSameSiteCookieRequired] + public function foo(): Response { + return new Response(); + } +} - /** @var Request|\PHPUnit\Framework\MockObject\MockObject */ - private $request; +class NoAnnotationController extends Controller { + public function foo(): Response { + return new Response(); + } +} - /** @var ControllerMethodReflector|\PHPUnit\Framework\MockObject\MockObject */ - private $reflector; +class SameSiteCookieMiddlewareTest extends TestCase { + private SameSiteCookieMiddleware $middleware; + private Request&MockObject $request; + private ControllerMethodReflector&MockObject $reflector; + private LoggerInterface&MockObject $logger; protected function setUp(): void { parent::setUp(); $this->request = $this->createMock(Request::class); + $this->logger = $this->createMock(LoggerInterface::class); $this->reflector = $this->createMock(ControllerMethodReflector::class); - $this->middleware = new SameSiteCookieMiddleware($this->request, $this->reflector); + $this->middleware = new SameSiteCookieMiddleware($this->request, new MiddlewareUtils($this->reflector, $this->logger)); } public function testBeforeControllerNoIndex(): void { $this->request->method('getScriptName') ->willReturn('/ocs/v2.php'); - $this->middleware->beforeController($this->createMock(Controller::class), 'foo'); + $this->middleware->beforeController(new NoAnnotationController('foo', $this->request), 'foo'); $this->addToAssertionCount(1); } @@ -50,7 +65,7 @@ public function testBeforeControllerIndexHasAnnotation(): void { ->with('NoSameSiteCookieRequired') ->willReturn(true); - $this->middleware->beforeController($this->createMock(Controller::class), 'foo'); + $this->middleware->beforeController(new HasAnnotationController('foo', $this->request), 'foo'); $this->addToAssertionCount(1); } @@ -65,7 +80,7 @@ public function testBeforeControllerIndexNoAnnotationPassingCheck(): void { $this->request->method('passesLaxCookieCheck') ->willReturn(true); - $this->middleware->beforeController($this->createMock(Controller::class), 'foo'); + $this->middleware->beforeController(new NoAnnotationController('foo', $this->request), 'foo'); $this->addToAssertionCount(1); } @@ -82,14 +97,14 @@ public function testBeforeControllerIndexNoAnnotationFailingCheck(): void { $this->request->method('passesLaxCookieCheck') ->willReturn(false); - $this->middleware->beforeController($this->createMock(Controller::class), 'foo'); + $this->middleware->beforeController(new NoAnnotationController('foo', $this->request), 'foo'); } public function testAfterExceptionNoLaxCookie(): void { $ex = new SecurityException(); try { - $this->middleware->afterException($this->createMock(Controller::class), 'foo', $ex); + $this->middleware->afterException(new NoAnnotationController('foo', $this->request), 'foo', $ex); $this->fail(); } catch (\Exception $e) { $this->assertSame($ex, $e); @@ -103,14 +118,14 @@ public function testAfterExceptionLaxCookie(): void { ->willReturn('/myrequri'); $middleware = $this->getMockBuilder(SameSiteCookieMiddleware::class) - ->setConstructorArgs([$this->request, $this->reflector]) + ->setConstructorArgs([$this->request, $this->reflector, $this->logger]) ->onlyMethods(['setSameSiteCookie']) ->getMock(); $middleware->expects($this->once()) ->method('setSameSiteCookie'); - $resp = $middleware->afterException($this->createMock(Controller::class), 'foo', $ex); + $resp = $middleware->afterException(new NoAnnotationController('foo', $this->request), 'foo', $ex); $this->assertSame(Http::STATUS_FOUND, $resp->getStatus()); diff --git a/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php index 0c6fc21357d9a..62886edd7b276 100644 --- a/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php @@ -10,6 +10,7 @@ use OC\AppFramework\Http; use OC\AppFramework\Http\Request; +use OC\AppFramework\Middleware\MiddlewareUtils; use OC\AppFramework\Middleware\Security\Exceptions\AppNotEnabledException; use OC\AppFramework\Middleware\Security\Exceptions\CrossSiteRequestForgeryException; use OC\AppFramework\Middleware\Security\Exceptions\ExAppRequiredException; @@ -37,38 +38,26 @@ use OCP\IUser; use OCP\IUserSession; use OCP\Security\Ip\IRemoteAddress; +use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; use Test\AppFramework\Middleware\Security\Mock\NormalController; use Test\AppFramework\Middleware\Security\Mock\OCSController; use Test\AppFramework\Middleware\Security\Mock\SecurityMiddlewareController; class SecurityMiddlewareTest extends \Test\TestCase { - /** @var SecurityMiddleware|\PHPUnit\Framework\MockObject\MockObject */ - private $middleware; - /** @var SecurityMiddlewareController */ - private $controller; - /** @var SecurityException */ - private $secException; - /** @var SecurityException */ - private $secAjaxException; - /** @var IRequest|\PHPUnit\Framework\MockObject\MockObject */ - private $request; - /** @var ControllerMethodReflector */ - private $reader; - /** @var LoggerInterface|\PHPUnit\Framework\MockObject\MockObject */ - private $logger; - /** @var INavigationManager|\PHPUnit\Framework\MockObject\MockObject */ - private $navigationManager; - /** @var IURLGenerator|\PHPUnit\Framework\MockObject\MockObject */ - private $urlGenerator; - /** @var IAppManager|\PHPUnit\Framework\MockObject\MockObject */ - private $appManager; - /** @var IL10N|\PHPUnit\Framework\MockObject\MockObject */ - private $l10n; - /** @var IUserSession|\PHPUnit\Framework\MockObject\MockObject */ - private $userSession; - /** @var AuthorizedGroupMapper|\PHPUnit\Framework\MockObject\MockObject */ - private $authorizedGroupMapper; + private SecurityMiddleware $middleware; + private ControllerMethodReflector $reader; + private SecurityMiddlewareController $controller; + private SecurityException $secAjaxException; + private IRequest|MockObject $request; + private MiddlewareUtils $middlewareUtils; + private LoggerInterface&MockObject $logger; + private INavigationManager&MockObject $navigationManager; + private IURLGenerator&MockObject $urlGenerator; + private IAppManager&MockObject $appManager; + private IL10N&MockObject $l10n; + private IUserSession&MockObject $userSession; + private AuthorizedGroupMapper&MockObject $authorizedGroupMapper; protected function setUp(): void { parent::setUp(); @@ -88,8 +77,8 @@ protected function setUp(): void { $this->navigationManager = $this->createMock(INavigationManager::class); $this->urlGenerator = $this->createMock(IURLGenerator::class); $this->l10n = $this->createMock(IL10N::class); + $this->middlewareUtils = new MiddlewareUtils($this->reader, $this->logger); $this->middleware = $this->getMiddleware(true, true, false); - $this->secException = new SecurityException('hey', false); $this->secAjaxException = new SecurityException('hey', true); } @@ -110,7 +99,7 @@ private function getMiddleware(bool $isLoggedIn, bool $isAdminUser, bool $isSubA return new SecurityMiddleware( $this->request, - $this->reader, + $this->middlewareUtils, $this->navigationManager, $this->urlGenerator, $this->logger,