diff --git a/apps/files_sharing/lib/Controller/PublicPreviewController.php b/apps/files_sharing/lib/Controller/PublicPreviewController.php index d917f6e0ebb..9056161af84 100644 --- a/apps/files_sharing/lib/Controller/PublicPreviewController.php +++ b/apps/files_sharing/lib/Controller/PublicPreviewController.php @@ -8,6 +8,7 @@ namespace OCA\Files_Sharing\Controller; 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 @@ class PublicPreviewController extends PublicShareController { } /** - * @NoSameSiteCookieRequired - * * Get a direct link preview for a shared file * * @param string $token Token of the share @@ -159,6 +158,7 @@ class PublicPreviewController extends PublicShareController { #[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 3c51a95f1d5..ab7556e1af9 100644 --- a/apps/files_sharing/lib/Controller/ShareController.php +++ b/apps/files_sharing/lib/Controller/ShareController.php @@ -15,12 +15,12 @@ use OCA\Files_Sharing\Event\ShareLinkAccessedEvent; 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 @@ class ShareController extends AuthPublicShareController { } /** - * @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 ff8012c4ecd..467caef40a0 100644 --- a/apps/theming/lib/Controller/ThemingController.php +++ b/apps/theming/lib/Controller/ThemingController.php @@ -17,6 +17,8 @@ use OCP\AppFramework\Http; 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 @@ class ThemingController extends Controller { } /** - * @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; @@ -167,13 +166,10 @@ class ThemingController extends Controller { } /** - * @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': @@ -218,7 +214,6 @@ class ThemingController extends Controller { } /** - * @return DataResponse * @throws NotPermittedException */ #[AuthorizedAdminSetting(settings: Admin::class)] @@ -383,9 +378,6 @@ class ThemingController extends Controller { } /** - * @NoSameSiteCookieRequired - * @NoTwoFactorRequired - * * Get the CSS stylesheet for a theme * * @param string $themeId ID of the theme @@ -398,7 +390,9 @@ class ThemingController extends Controller { */ #[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 4b141d200f1..748f3b3b4db 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -3210,13 +3210,6 @@ - - - - - - - request->server]]> diff --git a/core/Controller/AvatarController.php b/core/Controller/AvatarController.php index 1413c9d678e..3d6f79151e8 100644 --- a/core/Controller/AvatarController.php +++ b/core/Controller/AvatarController.php @@ -13,6 +13,7 @@ use OCP\AppFramework\Http; 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 @@ class AvatarController extends Controller { } /** - * @NoSameSiteCookieRequired - * * Get the dark avatar * * @param string $userId ID of the user @@ -67,6 +66,7 @@ class AvatarController extends Controller { #[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 @@ class AvatarController extends Controller { /** - * @NoSameSiteCookieRequired - * * Get the avatar * * @param string $userId ID of the user @@ -119,6 +117,7 @@ class AvatarController extends Controller { #[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 edf7c26e94c..85b187f42e6 100644 --- a/core/Controller/CSRFTokenController.php +++ b/core/Controller/CSRFTokenController.php @@ -13,6 +13,7 @@ use OCP\AppFramework\Controller; 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 @@ class CSRFTokenController extends Controller { * * 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 0e83ef283a9..836bba6d8b7 100644 --- a/core/Controller/ClientFlowLoginController.php +++ b/core/Controller/ClientFlowLoginController.php @@ -17,6 +17,7 @@ use OCP\AppFramework\Http; 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; @@ -171,13 +172,11 @@ class ClientFlowLoginController extends Controller { 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 9a1694a93be..85b82b388f4 100644 --- a/core/Controller/ClientFlowLoginV2Controller.php +++ b/core/Controller/ClientFlowLoginV2Controller.php @@ -18,6 +18,7 @@ use OCP\AppFramework\Http; 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; @@ -142,14 +143,12 @@ class ClientFlowLoginV2Controller extends Controller { ); } - /** - * @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 37e7edc530f..b6b0777fe40 100644 --- a/core/Controller/CssController.php +++ b/core/Controller/CssController.php @@ -13,11 +13,11 @@ use OCP\AppFramework\Controller; 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 @@ class CssController extends Controller { } /** - * @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 bb40a4f2117..f214d3043c2 100644 --- a/core/Controller/JsController.php +++ b/core/Controller/JsController.php @@ -13,11 +13,12 @@ use OCP\AppFramework\Controller; 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 @@ class JsController extends Controller { } /** - * @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 @@ class JsController extends Controller { } /** - * @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 083ad4b209f..db42a3608b5 100644 --- a/core/Controller/OCJSController.php +++ b/core/Controller/OCJSController.php @@ -16,6 +16,7 @@ use OCP\AppFramework\Controller; 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 @@ class OCJSController extends Controller { ); } - /** - * @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 8072bb8e92a..a10729e456d 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 @@ class TwoFactorChallengeController extends Controller { 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 @@ class TwoFactorChallengeController extends Controller { 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 @@ class TwoFactorChallengeController extends Controller { 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 0dea402f127..02f8ab8dad3 100644 --- a/core/Middleware/TwoFactorMiddleware.php +++ b/core/Middleware/TwoFactorMiddleware.php @@ -10,6 +10,8 @@ declare(strict_types=1); 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\Core\Controller\TwoFactorChallengeController; 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 @@ class TwoFactorMiddleware extends Middleware { private Session $userSession, private ISession $session, private IURLGenerator $urlGenerator, - private IControllerMethodReflector $reflector, + private MiddlewareUtils $middlewareUtils, private IRequest $request, ) { } @@ -43,7 +46,9 @@ class TwoFactorMiddleware extends Middleware { * @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 @@ class TwoFactorMiddleware extends Middleware { 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 @@ class TwoFactorMiddleware extends Middleware { || $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 @@ class TwoFactorMiddleware extends Middleware { // 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/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 75ebf94deae..0cec2593039 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -93,6 +93,8 @@ return array( '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', @@ -1092,6 +1094,7 @@ return array( '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', @@ -1100,6 +1103,7 @@ return array( '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 a9c3196a0b0..c07b6d127cc 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -134,6 +134,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', @@ -1133,6 +1135,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', @@ -1141,6 +1144,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 78951d99364..82b840b8072 100644 --- a/lib/private/AppFramework/DependencyInjection/DIContainer.php +++ b/lib/private/AppFramework/DependencyInjection/DIContainer.php @@ -17,6 +17,7 @@ use OC\AppFramework\Middleware\AdditionalScriptsMiddleware; 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\SameSiteCookieMiddleware; 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\IAppContainer; 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 @@ class DIContainer extends SimpleContainer implements IAppContainer { 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 @@ class DIContainer extends SimpleContainer implements IAppContainer { $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 00000000000..377d4b959ab --- /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 @@ class Dispatcher { * @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 @@ class Dispatcher { $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 00000000000..94ac2d1f44c --- /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 4453f5a7d4b..2d87f616a9d 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\AppFramework\Middleware; use OCP\IRequest; use OCP\ISession; use OCP\Security\Bruteforce\IThrottler; -use Psr\Log\LoggerInterface; +use Override; use ReflectionMethod; /** @@ -31,45 +31,23 @@ use ReflectionMethod; * 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 @@ class CORSMiddleware extends Middleware { } } - /** - * @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 @@ class CORSMiddleware extends Middleware { /** * 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 097ed1b2b8f..7334b56bac1 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 e3a293e0fd9..d0ca1b2ebe1 100644 --- a/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php +++ b/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php @@ -8,6 +8,7 @@ declare(strict_types=1); */ 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\NotAdminException; 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 @@ class SecurityMiddleware extends Middleware { $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 @@ class SecurityMiddleware extends Middleware { 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 @@ class SecurityMiddleware extends Middleware { 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 @@ class SecurityMiddleware extends Middleware { } // 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 @@ class SecurityMiddleware extends Middleware { } 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 @@ class SecurityMiddleware extends Middleware { || 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 00000000000..5bfa3b54242 --- /dev/null +++ b/lib/public/AppFramework/Http/Attribute/NoSameSiteCookieRequired.php @@ -0,0 +1,25 @@ +getMock(); $this->session = $this->createMock(ISession::class); $this->urlGenerator = $this->createMock(IURLGenerator::class); - $this->reflector = $this->createMock(IControllerMethodReflector::class); + $this->reflector = $this->createMock(ControllerMethodReflector::class); + $this->logger = $this->createMock(LoggerInterface::class); $this->request = new Request( [ 'server' => [ @@ -78,8 +102,7 @@ class TwoFactorMiddlewareTest extends TestCase { $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 @@ class TwoFactorMiddlewareTest extends TestCase { $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 @@ class TwoFactorMiddlewareTest extends TestCase { $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 @@ class TwoFactorMiddlewareTest extends TestCase { ->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 @@ class TwoFactorMiddlewareTest extends TestCase { ->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 @@ class TwoFactorMiddlewareTest extends TestCase { $user = $this->createMock(IUser::class); - $this->reflector - ->method('hasAnnotation') - ->willReturn(false); $this->userSession->expects($this->once()) ->method('isLoggedIn') ->willReturn(true); @@ -173,7 +201,7 @@ class TwoFactorMiddlewareTest extends TestCase { ->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 @@ class TwoFactorMiddlewareTest extends TestCase { ->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 @@ class TwoFactorMiddlewareTest extends TestCase { ->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 @@ class TwoFactorMiddlewareTest extends TestCase { $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 @@ class TwoFactorMiddlewareTest extends TestCase { ]; } - #[\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 @@ class TwoFactorMiddlewareTest extends TestCase { $user = $this->createMock(IUser::class); - $this->reflector - ->method('hasAnnotation') - ->willReturn(false); $this->userSession ->method('getUser') ->willReturn($user); @@ -278,9 +300,9 @@ class TwoFactorMiddlewareTest extends TestCase { $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 c325ae638fb..97541612429 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 Psr\Log\LoggerInterface; 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 @@ class CORSMiddlewareTest extends \Test\TestCase { $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 @@ class CORSMiddlewareTest extends \Test\TestCase { $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 @@ class CORSMiddlewareTest extends \Test\TestCase { $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 @@ class CORSMiddlewareTest extends \Test\TestCase { $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 @@ class CORSMiddlewareTest extends \Test\TestCase { $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 @@ class CORSMiddlewareTest extends \Test\TestCase { $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 @@ class CORSMiddlewareTest extends \Test\TestCase { ->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 @@ class CORSMiddlewareTest extends \Test\TestCase { ->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 @@ class CORSMiddlewareTest extends \Test\TestCase { ->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 @@ class CORSMiddlewareTest extends \Test\TestCase { $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 @@ class CORSMiddlewareTest extends \Test\TestCase { $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 @@ class CORSMiddlewareTest extends \Test\TestCase { $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 7800371f68f..ebcd3d4613b 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 HasAnnotationController extends Controller { + #[NoSameSiteCookieRequired] + public function foo(): Response { + return new Response(); + } +} + +class NoAnnotationController extends Controller { + public function foo(): Response { + return new Response(); + } +} + class SameSiteCookieMiddlewareTest extends TestCase { - /** @var SameSiteCookieMiddleware */ - private $middleware; - - /** @var Request|\PHPUnit\Framework\MockObject\MockObject */ - private $request; - - /** @var ControllerMethodReflector|\PHPUnit\Framework\MockObject\MockObject */ - private $reflector; + 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 @@ class SameSiteCookieMiddlewareTest extends TestCase { ->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 @@ class SameSiteCookieMiddlewareTest extends TestCase { $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 @@ class SameSiteCookieMiddlewareTest extends TestCase { $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 @@ class SameSiteCookieMiddlewareTest extends TestCase { ->willReturn('/myrequri'); $middleware = $this->getMockBuilder(SameSiteCookieMiddleware::class) - ->setConstructorArgs([$this->request, $this->reflector]) + ->setConstructorArgs([$this->request, new MiddlewareUtils($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 0c6fc21357d..62886edd7b2 100644 --- a/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php @@ -10,6 +10,7 @@ namespace Test\AppFramework\Middleware\Security; 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\IURLGenerator; 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 @@ class SecurityMiddlewareTest extends \Test\TestCase { $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 @@ class SecurityMiddlewareTest extends \Test\TestCase { return new SecurityMiddleware( $this->request, - $this->reader, + $this->middlewareUtils, $this->navigationManager, $this->urlGenerator, $this->logger,