diff options
author | Joas Schilling <coding@schilljs.com> | 2023-04-24 17:13:18 +0200 |
---|---|---|
committer | Joas Schilling <coding@schilljs.com> | 2023-04-25 14:50:32 +0200 |
commit | ecb8b55c5c01ca5cfbf23ef241536ef76c8f277d (patch) | |
tree | c07f24f3837a96ea963e45092b08a73658c10ace | |
parent | 2abefff2899952ea422d708fbda611f1695125fd (diff) | |
download | nextcloud-server-ecb8b55c5c01ca5cfbf23ef241536ef76c8f277d.tar.gz nextcloud-server-ecb8b55c5c01ca5cfbf23ef241536ef76c8f277d.zip |
feat(security): Add PHP \Attribute for remaining security annotations
Signed-off-by: Joas Schilling <coding@schilljs.com>
24 files changed, 1278 insertions, 278 deletions
diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index a4bab729592..ff439860e38 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -37,7 +37,15 @@ return array( 'OCP\\AppFramework\\Http' => $baseDir . '/lib/public/AppFramework/Http.php', 'OCP\\AppFramework\\Http\\Attribute\\ARateLimit' => $baseDir . '/lib/public/AppFramework/Http/Attribute/ARateLimit.php', 'OCP\\AppFramework\\Http\\Attribute\\AnonRateLimit' => $baseDir . '/lib/public/AppFramework/Http/Attribute/AnonRateLimit.php', + 'OCP\\AppFramework\\Http\\Attribute\\AuthorizedAdminSetting' => $baseDir . '/lib/public/AppFramework/Http/Attribute/AuthorizedAdminSetting.php', 'OCP\\AppFramework\\Http\\Attribute\\BruteForceProtection' => $baseDir . '/lib/public/AppFramework/Http/Attribute/BruteForceProtection.php', + 'OCP\\AppFramework\\Http\\Attribute\\CORS' => $baseDir . '/lib/public/AppFramework/Http/Attribute/CORS.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\\PasswordConfirmationRequired' => $baseDir . '/lib/public/AppFramework/Http/Attribute/PasswordConfirmationRequired.php', + 'OCP\\AppFramework\\Http\\Attribute\\PublicPage' => $baseDir . '/lib/public/AppFramework/Http/Attribute/PublicPage.php', + 'OCP\\AppFramework\\Http\\Attribute\\StrictCookiesRequired' => $baseDir . '/lib/public/AppFramework/Http/Attribute/StrictCookiesRequired.php', + 'OCP\\AppFramework\\Http\\Attribute\\SubAdminRequired' => $baseDir . '/lib/public/AppFramework/Http/Attribute/SubAdminRequired.php', 'OCP\\AppFramework\\Http\\Attribute\\UseSession' => $baseDir . '/lib/public/AppFramework/Http/Attribute/UseSession.php', 'OCP\\AppFramework\\Http\\Attribute\\UserRateLimit' => $baseDir . '/lib/public/AppFramework/Http/Attribute/UserRateLimit.php', 'OCP\\AppFramework\\Http\\ContentSecurityPolicy' => $baseDir . '/lib/public/AppFramework/Http/ContentSecurityPolicy.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 8a8de63fc4c..8be2f500636 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -70,7 +70,15 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OCP\\AppFramework\\Http' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http.php', 'OCP\\AppFramework\\Http\\Attribute\\ARateLimit' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/ARateLimit.php', 'OCP\\AppFramework\\Http\\Attribute\\AnonRateLimit' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/AnonRateLimit.php', + 'OCP\\AppFramework\\Http\\Attribute\\AuthorizedAdminSetting' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/AuthorizedAdminSetting.php', 'OCP\\AppFramework\\Http\\Attribute\\BruteForceProtection' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/BruteForceProtection.php', + 'OCP\\AppFramework\\Http\\Attribute\\CORS' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/CORS.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\\PasswordConfirmationRequired' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/PasswordConfirmationRequired.php', + 'OCP\\AppFramework\\Http\\Attribute\\PublicPage' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/PublicPage.php', + 'OCP\\AppFramework\\Http\\Attribute\\StrictCookiesRequired' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/StrictCookiesRequired.php', + 'OCP\\AppFramework\\Http\\Attribute\\SubAdminRequired' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/SubAdminRequired.php', 'OCP\\AppFramework\\Http\\Attribute\\UseSession' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/UseSession.php', 'OCP\\AppFramework\\Http\\Attribute\\UserRateLimit' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/UserRateLimit.php', 'OCP\\AppFramework\\Http\\ContentSecurityPolicy' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/ContentSecurityPolicy.php', diff --git a/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php b/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php index 30ba8d8d6e4..e177a612d96 100644 --- a/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php +++ b/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php @@ -33,10 +33,13 @@ use OC\Security\Bruteforce\Throttler; use OC\User\Session; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; +use OCP\AppFramework\Http\Attribute\CORS; +use OCP\AppFramework\Http\Attribute\PublicPage; use OCP\AppFramework\Http\JSONResponse; use OCP\AppFramework\Http\Response; use OCP\AppFramework\Middleware; use OCP\IRequest; +use ReflectionMethod; /** * This middleware sets the correct CORS headers on a response if the @@ -81,9 +84,12 @@ class CORSMiddleware extends Middleware { * @since 6.0.0 */ public function beforeController($controller, $methodName) { + $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->reflector->hasAnnotation('CORS') && (!$this->reflector->hasAnnotation('PublicPage') || $this->session->isLoggedIn())) { + if ($this->hasAnnotationOrAttribute($reflectionMethod, 'CORS', CORS::class) && + (!$this->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; @@ -103,7 +109,28 @@ class CORSMiddleware extends Middleware { } /** - * This is being run after a successful controllermethod call and allows + * @template T + * + * @param ReflectionMethod $reflectionMethod + * @param string $annotationName + * @param class-string<T> $attributeClass + * @return boolean + */ + protected function hasAnnotationOrAttribute(ReflectionMethod $reflectionMethod, string $annotationName, string $attributeClass): bool { + if ($this->reflector->hasAnnotation($annotationName)) { + 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 @@ -114,23 +141,25 @@ class CORSMiddleware extends Middleware { * @throws SecurityException */ public function afterController($controller, $methodName, Response $response) { - // only react if its a CORS request and if the request sends origin and + // only react if it's a CORS request and if the request sends origin and - if (isset($this->request->server['HTTP_ORIGIN']) && - $this->reflector->hasAnnotation('CORS')) { - // allow credentials headers must not be true or CSRF is possible - // otherwise - foreach ($response->getHeaders() as $header => $value) { - if (strtolower($header) === 'access-control-allow-credentials' && - strtolower(trim($value)) === 'true') { - $msg = 'Access-Control-Allow-Credentials must not be '. - 'set to true in order to prevent CSRF'; - throw new SecurityException($msg); + if (isset($this->request->server['HTTP_ORIGIN'])) { + $reflectionMethod = new ReflectionMethod($controller, $methodName); + if ($this->hasAnnotationOrAttribute($reflectionMethod, 'CORS', CORS::class)) { + // allow credentials headers must not be true or CSRF is possible + // otherwise + foreach ($response->getHeaders() as $header => $value) { + if (strtolower($header) === 'access-control-allow-credentials' && + strtolower(trim($value)) === 'true') { + $msg = 'Access-Control-Allow-Credentials must not be '. + 'set to true in order to prevent CSRF'; + throw new SecurityException($msg); + } } - } - $origin = $this->request->server['HTTP_ORIGIN']; - $response->addHeader('Access-Control-Allow-Origin', $origin); + $origin = $this->request->server['HTTP_ORIGIN']; + $response->addHeader('Access-Control-Allow-Origin', $origin); + } } return $response; } diff --git a/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php b/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php index 0ee9fdff881..a72a7a40016 100644 --- a/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php +++ b/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php @@ -26,11 +26,13 @@ namespace OC\AppFramework\Middleware\Security; use OC\AppFramework\Middleware\Security\Exceptions\NotConfirmedException; use OC\AppFramework\Utility\ControllerMethodReflector; use OCP\AppFramework\Controller; +use OCP\AppFramework\Http\Attribute\PasswordConfirmationRequired; use OCP\AppFramework\Middleware; use OCP\AppFramework\Utility\ITimeFactory; use OCP\ISession; use OCP\IUserSession; use OCP\User\Backend\IPasswordConfirmationBackend; +use ReflectionMethod; class PasswordConfirmationMiddleware extends Middleware { /** @var ControllerMethodReflector */ @@ -68,7 +70,9 @@ class PasswordConfirmationMiddleware extends Middleware { * @throws NotConfirmedException */ public function beforeController($controller, $methodName) { - if ($this->reflector->hasAnnotation('PasswordConfirmationRequired')) { + $reflectionMethod = new ReflectionMethod($controller, $methodName); + + if ($this->hasAnnotationOrAttribute($reflectionMethod, 'PasswordConfirmationRequired', PasswordConfirmationRequired::class)) { $user = $this->userSession->getUser(); $backendClassName = ''; if ($user !== null) { @@ -89,4 +93,24 @@ class PasswordConfirmationMiddleware extends Middleware { } } } + + /** + * @template T + * + * @param ReflectionMethod $reflectionMethod + * @param string $annotationName + * @param class-string<T> $attributeClass + * @return boolean + */ + protected function hasAnnotationOrAttribute(ReflectionMethod $reflectionMethod, string $annotationName, string $attributeClass): bool { + if (!empty($reflectionMethod->getAttributes($attributeClass))) { + return true; + } + + if ($this->reflector->hasAnnotation($annotationName)) { + return true; + } + + return false; + } } diff --git a/lib/private/AppFramework/Middleware/Security/ReloadExecutionMiddleware.php b/lib/private/AppFramework/Middleware/Security/ReloadExecutionMiddleware.php index 5a635e3f284..a6a07538345 100644 --- a/lib/private/AppFramework/Middleware/Security/ReloadExecutionMiddleware.php +++ b/lib/private/AppFramework/Middleware/Security/ReloadExecutionMiddleware.php @@ -58,7 +58,7 @@ class ReloadExecutionMiddleware extends Middleware { return new RedirectResponse($this->urlGenerator->linkToRouteAbsolute( 'core.login.showLoginForm', - ['clear' => true] // this param the the code in login.js may be removed when the "Clear-Site-Data" is working in the browsers + ['clear' => true] // this param the code in login.js may be removed when the "Clear-Site-Data" is working in the browsers )); } diff --git a/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php b/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php index eb8c1b8dc43..8e0794f9645 100644 --- a/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php +++ b/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php @@ -48,6 +48,12 @@ use OC\Settings\AuthorizedGroupMapper; use OCP\App\AppPathNotFoundException; use OCP\App\IAppManager; use OCP\AppFramework\Controller; +use OCP\AppFramework\Http\Attribute\AuthorizedAdminSetting; +use OCP\AppFramework\Http\Attribute\NoAdminRequired; +use OCP\AppFramework\Http\Attribute\NoCSRFRequired; +use OCP\AppFramework\Http\Attribute\PublicPage; +use OCP\AppFramework\Http\Attribute\StrictCookiesRequired; +use OCP\AppFramework\Http\Attribute\SubAdminRequired; use OCP\AppFramework\Http\JSONResponse; use OCP\AppFramework\Http\RedirectResponse; use OCP\AppFramework\Http\Response; @@ -61,6 +67,7 @@ use OCP\IURLGenerator; use OCP\IUserSession; use OCP\Util; use Psr\Log\LoggerInterface; +use ReflectionMethod; /** * Used to do all the authentication and checking stuff for a controller method @@ -145,22 +152,24 @@ class SecurityMiddleware extends Middleware { $this->navigationManager->setActiveEntry('spreed'); } + $reflectionMethod = new ReflectionMethod($controller, $methodName); + // security checks - $isPublicPage = $this->reflector->hasAnnotation('PublicPage'); + $isPublicPage = $this->hasAnnotationOrAttribute($reflectionMethod, 'PublicPage', PublicPage::class); if (!$isPublicPage) { if (!$this->isLoggedIn) { throw new NotLoggedInException(); } $authorized = false; - if ($this->reflector->hasAnnotation('AuthorizedAdminSetting')) { + if ($this->hasAnnotationOrAttribute($reflectionMethod, 'AuthorizedAdminSetting', AuthorizedAdminSetting::class)) { $authorized = $this->isAdminUser; - if (!$authorized && $this->reflector->hasAnnotation('SubAdminRequired')) { + if (!$authorized && $this->hasAnnotationOrAttribute($reflectionMethod, 'SubAdminRequired', SubAdminRequired::class)) { $authorized = $this->isSubAdmin; } if (!$authorized) { - $settingClasses = explode(';', $this->reflector->getAnnotationParameter('AuthorizedAdminSetting', 'settings')); + $settingClasses = $this->getAuthorizedAdminSettingClasses($reflectionMethod); $authorizedClasses = $this->groupAuthorizationMapper->findAllClassesForUser($this->userSession->getUser()); foreach ($settingClasses as $settingClass) { $authorized = in_array($settingClass, $authorizedClasses, true); @@ -174,14 +183,14 @@ class SecurityMiddleware extends Middleware { throw new NotAdminException($this->l10n->t('Logged in user must be an admin, a sub admin or gotten special right to access this setting')); } } - if ($this->reflector->hasAnnotation('SubAdminRequired') + if ($this->hasAnnotationOrAttribute($reflectionMethod, 'SubAdminRequired', SubAdminRequired::class) && !$this->isSubAdmin && !$this->isAdminUser && !$authorized) { throw new NotAdminException($this->l10n->t('Logged in user must be an admin or sub admin')); } - if (!$this->reflector->hasAnnotation('SubAdminRequired') - && !$this->reflector->hasAnnotation('NoAdminRequired') + if (!$this->hasAnnotationOrAttribute($reflectionMethod, 'SubAdminRequired', SubAdminRequired::class) + && !$this->hasAnnotationOrAttribute($reflectionMethod, 'NoAdminRequired', NoAdminRequired::class) && !$this->isAdminUser && !$authorized) { throw new NotAdminException($this->l10n->t('Logged in user must be an admin')); @@ -189,14 +198,15 @@ class SecurityMiddleware extends Middleware { } // Check for strict cookie requirement - if ($this->reflector->hasAnnotation('StrictCookieRequired') || !$this->reflector->hasAnnotation('NoCSRFRequired')) { + if ($this->hasAnnotationOrAttribute($reflectionMethod, 'StrictCookieRequired', StrictCookiesRequired::class) || + !$this->hasAnnotationOrAttribute($reflectionMethod, 'NoCSRFRequired', NoCSRFRequired::class)) { if (!$this->request->passesStrictCookieCheck()) { throw new StrictCookieMissingException(); } } // CSRF check - also registers the CSRF token since the session may be closed later Util::callRegister(); - if (!$this->reflector->hasAnnotation('NoCSRFRequired')) { + if (!$this->hasAnnotationOrAttribute($reflectionMethod, 'NoCSRFRequired', NoCSRFRequired::class)) { /* * Only allow the CSRF check to fail on OCS Requests. This kind of * hacks around that we have no full token auth in place yet and we @@ -233,6 +243,48 @@ class SecurityMiddleware extends Middleware { } /** + * @template T + * + * @param ReflectionMethod $reflectionMethod + * @param string $annotationName + * @param class-string<T> $attributeClass + * @return boolean + */ + protected function hasAnnotationOrAttribute(ReflectionMethod $reflectionMethod, string $annotationName, string $attributeClass): bool { + if (!empty($reflectionMethod->getAttributes($attributeClass))) { + return true; + } + + if ($this->reflector->hasAnnotation($annotationName)) { + 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/ApiController.php b/lib/public/AppFramework/ApiController.php index 83dfaf93bc6..66c278e62d8 100644 --- a/lib/public/AppFramework/ApiController.php +++ b/lib/public/AppFramework/ApiController.php @@ -23,6 +23,8 @@ */ namespace OCP\AppFramework; +use OCP\AppFramework\Http\Attribute\NoCSRFRequired; +use OCP\AppFramework\Http\Attribute\PublicPage; use OCP\AppFramework\Http\Response; use OCP\IRequest; @@ -70,6 +72,8 @@ abstract class ApiController extends Controller { * @PublicPage * @since 7.0.0 */ + #[NoCSRFRequired] + #[PublicPage] public function preflightedCors() { if (isset($this->request->server['HTTP_ORIGIN'])) { $origin = $this->request->server['HTTP_ORIGIN']; diff --git a/lib/public/AppFramework/AuthPublicShareController.php b/lib/public/AppFramework/AuthPublicShareController.php index 00834506b05..78dd45551ed 100644 --- a/lib/public/AppFramework/AuthPublicShareController.php +++ b/lib/public/AppFramework/AuthPublicShareController.php @@ -28,6 +28,10 @@ declare(strict_types=1); */ namespace OCP\AppFramework; +use OCP\AppFramework\Http\Attribute\BruteForceProtection; +use OCP\AppFramework\Http\Attribute\NoCSRFRequired; +use OCP\AppFramework\Http\Attribute\PublicPage; +use OCP\AppFramework\Http\Attribute\UseSession; use OCP\AppFramework\Http\RedirectResponse; use OCP\AppFramework\Http\TemplateResponse; use OCP\IRequest; @@ -70,6 +74,8 @@ abstract class AuthPublicShareController extends PublicShareController { * * @since 14.0.0 */ + #[NoCSRFRequired] + #[PublicPage] public function showAuthenticate(): TemplateResponse { return new TemplateResponse('core', 'publicshareauth', [], 'guest'); } @@ -129,7 +135,7 @@ abstract class AuthPublicShareController extends PublicShareController { } /** - * Function called after successfull authentication + * Function called after successful authentication * * You can use this to do some logging for example * @@ -147,6 +153,9 @@ abstract class AuthPublicShareController extends PublicShareController { * * @since 14.0.0 */ + #[BruteForceProtection(action: 'publicLinkAuth')] + #[PublicPage] + #[UseSession] final public function authenticate(string $password = '', string $passwordRequest = 'no', string $identityToken = '') { // Already authenticated if ($this->isAuthenticated()) { diff --git a/lib/public/AppFramework/Http/Attribute/AuthorizedAdminSetting.php b/lib/public/AppFramework/Http/Attribute/AuthorizedAdminSetting.php new file mode 100644 index 00000000000..724e78a1958 --- /dev/null +++ b/lib/public/AppFramework/Http/Attribute/AuthorizedAdminSetting.php @@ -0,0 +1,56 @@ +<?php + +declare(strict_types=1); + +/** + * @copyright Copyright (c) 2023 Joas Schilling <coding@schilljs.com> + * + * @author Joas Schilling <coding@schilljs.com> + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +namespace OCP\AppFramework\Http\Attribute; + +use Attribute; +use OCP\Settings\IDelegatedSettings; + +/** + * Attribute for controller methods that should be only accessible with + * full admin or partial admin permissions. + * + * @since 27.0.0 + */ +#[Attribute(Attribute::TARGET_METHOD | Attribute::IS_REPEATABLE)] +class AuthorizedAdminSetting { + /** + * @param class-string<IDelegatedSettings> $settings A settings section the user needs to be able to access + * @since 27.0.0 + */ + public function __construct( + protected string $settings + ) { + } + + /** + * + * @return class-string<IDelegatedSettings> + * @since 27.0.0 + */ + public function getSettings(): string { + return $this->settings; + } +} diff --git a/lib/public/AppFramework/Http/Attribute/CORS.php b/lib/public/AppFramework/Http/Attribute/CORS.php new file mode 100644 index 00000000000..2d87c91ccab --- /dev/null +++ b/lib/public/AppFramework/Http/Attribute/CORS.php @@ -0,0 +1,37 @@ +<?php + +declare(strict_types=1); + +/** + * @copyright Copyright (c) 2023 Joas Schilling <coding@schilljs.com> + * + * @author Joas Schilling <coding@schilljs.com> + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +namespace OCP\AppFramework\Http\Attribute; + +use Attribute; + +/** + * Attribute for controller methods that can also be accessed by not logged-in user + * + * @since 27.0.0 + */ +#[Attribute] +class CORS { +} diff --git a/lib/public/AppFramework/Http/Attribute/NoAdminRequired.php b/lib/public/AppFramework/Http/Attribute/NoAdminRequired.php new file mode 100644 index 00000000000..5e7164523a2 --- /dev/null +++ b/lib/public/AppFramework/Http/Attribute/NoAdminRequired.php @@ -0,0 +1,37 @@ +<?php + +declare(strict_types=1); + +/** + * @copyright Copyright (c) 2023 Joas Schilling <coding@schilljs.com> + * + * @author Joas Schilling <coding@schilljs.com> + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +namespace OCP\AppFramework\Http\Attribute; + +use Attribute; + +/** + * Attribute for controller methods that can be accessed by any logged-in user + * + * @since 27.0.0 + */ +#[Attribute] +class NoAdminRequired { +} diff --git a/lib/public/AppFramework/Http/Attribute/NoCSRFRequired.php b/lib/public/AppFramework/Http/Attribute/NoCSRFRequired.php new file mode 100644 index 00000000000..247cb5c55b5 --- /dev/null +++ b/lib/public/AppFramework/Http/Attribute/NoCSRFRequired.php @@ -0,0 +1,37 @@ +<?php + +declare(strict_types=1); + +/** + * @copyright Copyright (c) 2023 Joas Schilling <coding@schilljs.com> + * + * @author Joas Schilling <coding@schilljs.com> + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +namespace OCP\AppFramework\Http\Attribute; + +use Attribute; + +/** + * Attribute for controller methods that are not CSRF protected + * + * @since 27.0.0 + */ +#[Attribute] +class NoCSRFRequired { +} diff --git a/lib/public/AppFramework/Http/Attribute/PasswordConfirmationRequired.php b/lib/public/AppFramework/Http/Attribute/PasswordConfirmationRequired.php new file mode 100644 index 00000000000..49fc290be1c --- /dev/null +++ b/lib/public/AppFramework/Http/Attribute/PasswordConfirmationRequired.php @@ -0,0 +1,37 @@ +<?php + +declare(strict_types=1); + +/** + * @copyright Copyright (c) 2023 Joas Schilling <coding@schilljs.com> + * + * @author Joas Schilling <coding@schilljs.com> + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +namespace OCP\AppFramework\Http\Attribute; + +use Attribute; + +/** + * Attribute for controller methods that require the password to be confirmed with in the last 30 minutes + * + * @since 27.0.0 + */ +#[Attribute] +class PasswordConfirmationRequired { +} diff --git a/lib/public/AppFramework/Http/Attribute/PublicPage.php b/lib/public/AppFramework/Http/Attribute/PublicPage.php new file mode 100644 index 00000000000..14e7a93b981 --- /dev/null +++ b/lib/public/AppFramework/Http/Attribute/PublicPage.php @@ -0,0 +1,37 @@ +<?php + +declare(strict_types=1); + +/** + * @copyright Copyright (c) 2023 Joas Schilling <coding@schilljs.com> + * + * @author Joas Schilling <coding@schilljs.com> + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +namespace OCP\AppFramework\Http\Attribute; + +use Attribute; + +/** + * Attribute for controller methods that can also be accessed by not logged-in user + * + * @since 27.0.0 + */ +#[Attribute] +class PublicPage { +} diff --git a/lib/public/AppFramework/Http/Attribute/StrictCookiesRequired.php b/lib/public/AppFramework/Http/Attribute/StrictCookiesRequired.php new file mode 100644 index 00000000000..075a1b13c13 --- /dev/null +++ b/lib/public/AppFramework/Http/Attribute/StrictCookiesRequired.php @@ -0,0 +1,37 @@ +<?php + +declare(strict_types=1); + +/** + * @copyright Copyright (c) 2023 Joas Schilling <coding@schilljs.com> + * + * @author Joas Schilling <coding@schilljs.com> + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +namespace OCP\AppFramework\Http\Attribute; + +use Attribute; + +/** + * Attribute for controller methods that require strict cookies + * + * @since 27.0.0 + */ +#[Attribute] +class StrictCookiesRequired { +} diff --git a/lib/public/AppFramework/Http/Attribute/SubAdminRequired.php b/lib/public/AppFramework/Http/Attribute/SubAdminRequired.php new file mode 100644 index 00000000000..dd34ce73c01 --- /dev/null +++ b/lib/public/AppFramework/Http/Attribute/SubAdminRequired.php @@ -0,0 +1,37 @@ +<?php + +declare(strict_types=1); + +/** + * @copyright Copyright (c) 2023 Joas Schilling <coding@schilljs.com> + * + * @author Joas Schilling <coding@schilljs.com> + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +namespace OCP\AppFramework\Http\Attribute; + +use Attribute; + +/** + * Attribute for controller methods that can be accessed by sub-admins + * + * @since 27.0.0 + */ +#[Attribute] +class SubAdminRequired { +} diff --git a/tests/lib/AppFramework/Middleware/Security/CORSMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/CORSMiddlewareTest.php index 986d0e577b7..7c48f7e2712 100644 --- a/tests/lib/AppFramework/Middleware/Security/CORSMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/CORSMiddlewareTest.php @@ -17,11 +17,12 @@ use OC\AppFramework\Middleware\Security\Exceptions\SecurityException; use OC\AppFramework\Utility\ControllerMethodReflector; use OC\Security\Bruteforce\Throttler; use OC\User\Session; -use OCP\AppFramework\Controller; use OCP\AppFramework\Http\JSONResponse; use OCP\AppFramework\Http\Response; use OCP\IConfig; +use OCP\IRequest; use OCP\IRequestId; +use Test\AppFramework\Middleware\Security\Mock\CORSMiddlewareController; class CORSMiddlewareTest extends \Test\TestCase { /** @var ControllerMethodReflector */ @@ -30,7 +31,7 @@ class CORSMiddlewareTest extends \Test\TestCase { private $session; /** @var Throttler */ private $throttler; - /** @var Controller */ + /** @var CORSMiddlewareController */ private $controller; protected function setUp(): void { @@ -38,13 +39,23 @@ class CORSMiddlewareTest extends \Test\TestCase { $this->reflector = new ControllerMethodReflector(); $this->session = $this->createMock(Session::class); $this->throttler = $this->createMock(Throttler::class); - $this->controller = $this->createMock(Controller::class); + $this->controller = new CORSMiddlewareController( + 'test', + $this->createMock(IRequest::class) + ); + } + + public function dataSetCORSAPIHeader(): array { + return [ + ['testSetCORSAPIHeader'], + ['testSetCORSAPIHeaderAttribute'], + ]; } /** - * @CORS + * @dataProvider dataSetCORSAPIHeader */ - public function testSetCORSAPIHeader() { + public function testSetCORSAPIHeader(string $method): void { $request = new Request( [ 'server' => [ @@ -54,16 +65,15 @@ class CORSMiddlewareTest extends \Test\TestCase { $this->createMock(IRequestId::class), $this->createMock(IConfig::class) ); - $this->reflector->reflect($this, __FUNCTION__); + $this->reflector->reflect($this->controller, $method); $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler); - $response = $middleware->afterController($this->controller, __FUNCTION__, new Response()); + $response = $middleware->afterController($this->controller, $method, new Response()); $headers = $response->getHeaders(); $this->assertEquals('test', $headers['Access-Control-Allow-Origin']); } - - public function testNoAnnotationNoCORSHEADER() { + public function testNoAnnotationNoCORSHEADER(): void { $request = new Request( [ 'server' => [ @@ -80,29 +90,41 @@ class CORSMiddlewareTest extends \Test\TestCase { $this->assertFalse(array_key_exists('Access-Control-Allow-Origin', $headers)); } + public function dataNoOriginHeaderNoCORSHEADER(): array { + return [ + ['testNoOriginHeaderNoCORSHEADER'], + ['testNoOriginHeaderNoCORSHEADERAttribute'], + ]; + } /** - * @CORS + * @dataProvider dataNoOriginHeaderNoCORSHEADER */ - public function testNoOriginHeaderNoCORSHEADER() { + public function testNoOriginHeaderNoCORSHEADER(string $method): void { $request = new Request( [], $this->createMock(IRequestId::class), $this->createMock(IConfig::class) ); - $this->reflector->reflect($this, __FUNCTION__); + $this->reflector->reflect($this->controller, $method); $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler); - $response = $middleware->afterController($this->controller, __FUNCTION__, new Response()); + $response = $middleware->afterController($this->controller, $method, new Response()); $headers = $response->getHeaders(); $this->assertFalse(array_key_exists('Access-Control-Allow-Origin', $headers)); } + public function dataCorsIgnoredIfWithCredentialsHeaderPresent(): array { + return [ + ['testCorsIgnoredIfWithCredentialsHeaderPresent'], + ['testCorsAttributeIgnoredIfWithCredentialsHeaderPresent'], + ]; + } /** - * @CORS + * @dataProvider dataCorsIgnoredIfWithCredentialsHeaderPresent */ - public function testCorsIgnoredIfWithCredentialsHeaderPresent() { + public function testCorsIgnoredIfWithCredentialsHeaderPresent(string $method): void { $this->expectException(\OC\AppFramework\Middleware\Security\Exceptions\SecurityException::class); $request = new Request( @@ -114,27 +136,33 @@ class CORSMiddlewareTest extends \Test\TestCase { $this->createMock(IRequestId::class), $this->createMock(IConfig::class) ); - $this->reflector->reflect($this, __FUNCTION__); + $this->reflector->reflect($this->controller, $method); $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler); $response = new Response(); $response->addHeader('AcCess-control-Allow-Credentials ', 'TRUE'); - $middleware->afterController($this->controller, __FUNCTION__, $response); + $middleware->afterController($this->controller, $method, $response); + } + + public function dataNoCORSOnAnonymousPublicPage(): array { + return [ + ['testNoCORSOnAnonymousPublicPage'], + ['testNoCORSOnAnonymousPublicPageAttribute'], + ['testNoCORSAttributeOnAnonymousPublicPage'], + ['testNoCORSAttributeOnAnonymousPublicPageAttribute'], + ]; } /** - * CORS must not be enforced for anonymous users on public pages - * - * @CORS - * @PublicPage + * @dataProvider dataNoCORSOnAnonymousPublicPage */ - public function testNoCORSOnAnonymousPublicPage() { + public function testNoCORSOnAnonymousPublicPage(string $method): void { $request = new Request( [], $this->createMock(IRequestId::class), $this->createMock(IConfig::class) ); - $this->reflector->reflect($this, __FUNCTION__); + $this->reflector->reflect($this->controller, $method); $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler); $this->session->expects($this->once()) ->method('isLoggedIn') @@ -145,25 +173,30 @@ class CORSMiddlewareTest extends \Test\TestCase { ->method('logClientIn') ->with($this->equalTo('user'), $this->equalTo('pass')) ->willReturn(true); - $this->reflector->reflect($this, __FUNCTION__); + $this->reflector->reflect($this->controller, $method); - $middleware->beforeController($this->controller, __FUNCTION__); + $middleware->beforeController($this->controller, $method); + } + + public function dataCORSShouldNeverAllowCookieAuth(): array { + return [ + ['testCORSShouldNeverAllowCookieAuth'], + ['testCORSShouldNeverAllowCookieAuthAttribute'], + ['testCORSAttributeShouldNeverAllowCookieAuth'], + ['testCORSAttributeShouldNeverAllowCookieAuthAttribute'], + ]; } /** - * Even on public pages users logged in using session cookies, - * that do not provide a valid CSRF token are disallowed - * - * @CORS - * @PublicPage + * @dataProvider dataCORSShouldNeverAllowCookieAuth */ - public function testCORSShouldNeverAllowCookieAuth() { + public function testCORSShouldNeverAllowCookieAuth(string $method): void { $request = new Request( [], $this->createMock(IRequestId::class), $this->createMock(IConfig::class) ); - $this->reflector->reflect($this, __FUNCTION__); + $this->reflector->reflect($this->controller, $method); $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler); $this->session->expects($this->once()) ->method('isLoggedIn') @@ -176,13 +209,20 @@ class CORSMiddlewareTest extends \Test\TestCase { ->willReturn(true); $this->expectException(SecurityException::class); - $middleware->beforeController($this->controller, __FUNCTION__); + $middleware->beforeController($this->controller, $method); + } + + public function dataCORSShouldRelogin(): array { + return [ + ['testCORSShouldRelogin'], + ['testCORSAttributeShouldRelogin'], + ]; } /** - * @CORS + * @dataProvider dataCORSShouldRelogin */ - public function testCORSShouldRelogin() { + public function testCORSShouldRelogin(string $method): void { $request = new Request( ['server' => [ 'PHP_AUTH_USER' => 'user', @@ -197,16 +237,23 @@ class CORSMiddlewareTest extends \Test\TestCase { ->method('logClientIn') ->with($this->equalTo('user'), $this->equalTo('pass')) ->willReturn(true); - $this->reflector->reflect($this, __FUNCTION__); + $this->reflector->reflect($this->controller, $method); $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler); - $middleware->beforeController($this->controller, __FUNCTION__); + $middleware->beforeController($this->controller, $method); + } + + public function dataCORSShouldFailIfPasswordLoginIsForbidden(): array { + return [ + ['testCORSShouldFailIfPasswordLoginIsForbidden'], + ['testCORSAttributeShouldFailIfPasswordLoginIsForbidden'], + ]; } /** - * @CORS + * @dataProvider dataCORSShouldFailIfPasswordLoginIsForbidden */ - public function testCORSShouldFailIfPasswordLoginIsForbidden() { + public function testCORSShouldFailIfPasswordLoginIsForbidden(string $method): void { $this->expectException(\OC\AppFramework\Middleware\Security\Exceptions\SecurityException::class); $request = new Request( @@ -223,16 +270,23 @@ class CORSMiddlewareTest extends \Test\TestCase { ->method('logClientIn') ->with($this->equalTo('user'), $this->equalTo('pass')) ->will($this->throwException(new \OC\Authentication\Exceptions\PasswordLoginForbiddenException)); - $this->reflector->reflect($this, __FUNCTION__); + $this->reflector->reflect($this->controller, $method); $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler); - $middleware->beforeController($this->controller, __FUNCTION__); + $middleware->beforeController($this->controller, $method); + } + + public function dataCORSShouldNotAllowCookieAuth(): array { + return [ + ['testCORSShouldNotAllowCookieAuth'], + ['testCORSAttributeShouldNotAllowCookieAuth'], + ]; } /** - * @CORS + * @dataProvider dataCORSShouldNotAllowCookieAuth */ - public function testCORSShouldNotAllowCookieAuth() { + public function testCORSShouldNotAllowCookieAuth(string $method): void { $this->expectException(\OC\AppFramework\Middleware\Security\Exceptions\SecurityException::class); $request = new Request( @@ -249,10 +303,10 @@ class CORSMiddlewareTest extends \Test\TestCase { ->method('logClientIn') ->with($this->equalTo('user'), $this->equalTo('pass')) ->willReturn(false); - $this->reflector->reflect($this, __FUNCTION__); + $this->reflector->reflect($this->controller, $method); $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler); - $middleware->beforeController($this->controller, __FUNCTION__); + $middleware->beforeController($this->controller, $method); } public function testAfterExceptionWithSecurityExceptionNoStatus() { @@ -287,7 +341,6 @@ class CORSMiddlewareTest extends \Test\TestCase { $this->assertEquals($expected, $response); } - public function testAfterExceptionWithRegularException() { $this->expectException(\Exception::class); $this->expectExceptionMessage('A regular exception'); diff --git a/tests/lib/AppFramework/Middleware/Security/Mock/CORSMiddlewareController.php b/tests/lib/AppFramework/Middleware/Security/Mock/CORSMiddlewareController.php new file mode 100644 index 00000000000..44e6c7a588b --- /dev/null +++ b/tests/lib/AppFramework/Middleware/Security/Mock/CORSMiddlewareController.php @@ -0,0 +1,160 @@ +<?php + +declare(strict_types=1); + +/** + * @copyright Copyright (c) 2023 Joas Schilling <coding@schilljs.com> + * + * @author Joas Schilling <coding@schilljs.com> + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +namespace Test\AppFramework\Middleware\Security\Mock; + +use OCP\AppFramework\Http\Attribute\CORS; +use OCP\AppFramework\Http\Attribute\PublicPage; + +class CORSMiddlewareController extends \OCP\AppFramework\Controller { + /** + * @CORS + */ + public function testSetCORSAPIHeader() { + } + + #[CORS] + public function testSetCORSAPIHeaderAttribute() { + } + + public function testNoAnnotationNoCORSHEADER() { + } + + /** + * @CORS + */ + public function testNoOriginHeaderNoCORSHEADER() { + } + + #[CORS] + public function testNoOriginHeaderNoCORSHEADERAttribute() { + } + + /** + * @CORS + */ + public function testCorsIgnoredIfWithCredentialsHeaderPresent() { + } + + #[CORS] + public function testCorsAttributeIgnoredIfWithCredentialsHeaderPresent() { + } + + /** + * CORS must not be enforced for anonymous users on public pages + * + * @CORS + * @PublicPage + */ + public function testNoCORSOnAnonymousPublicPage() { + } + + /** + * CORS must not be enforced for anonymous users on public pages + * + * @CORS + */ + #[PublicPage] + public function testNoCORSOnAnonymousPublicPageAttribute() { + } + + /** + * @PublicPage + */ + #[CORS] + public function testNoCORSAttributeOnAnonymousPublicPage() { + } + + #[CORS] + #[PublicPage] + public function testNoCORSAttributeOnAnonymousPublicPageAttribute() { + } + + /** + * @CORS + * @PublicPage + */ + public function testCORSShouldNeverAllowCookieAuth() { + } + + /** + * @CORS + */ + #[PublicPage] + public function testCORSShouldNeverAllowCookieAuthAttribute() { + } + + /** + * @PublicPage + */ + #[CORS] + public function testCORSAttributeShouldNeverAllowCookieAuth() { + } + + #[CORS] + #[PublicPage] + public function testCORSAttributeShouldNeverAllowCookieAuthAttribute() { + } + + /** + * @CORS + */ + public function testCORSShouldRelogin() { + } + + #[CORS] + public function testCORSAttributeShouldRelogin() { + } + + /** + * @CORS + */ + public function testCORSShouldFailIfPasswordLoginIsForbidden() { + } + + #[CORS] + public function testCORSAttributeShouldFailIfPasswordLoginIsForbidden() { + } + + /** + * @CORS + */ + public function testCORSShouldNotAllowCookieAuth() { + } + + #[CORS] + public function testCORSAttributeShouldNotAllowCookieAuth() { + } + + public function testAfterExceptionWithSecurityExceptionNoStatus() { + } + + public function testAfterExceptionWithSecurityExceptionWithStatus() { + } + + + public function testAfterExceptionWithRegularException() { + } +} diff --git a/tests/lib/AppFramework/Middleware/Security/Mock/NormalController.php b/tests/lib/AppFramework/Middleware/Security/Mock/NormalController.php new file mode 100644 index 00000000000..e732b89e308 --- /dev/null +++ b/tests/lib/AppFramework/Middleware/Security/Mock/NormalController.php @@ -0,0 +1,31 @@ +<?php + +declare(strict_types=1); + +/** + * @copyright Copyright (c) 2023 Joas Schilling <coding@schilljs.com> + * + * @author Joas Schilling <coding@schilljs.com> + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +namespace Test\AppFramework\Middleware\Security\Mock; + +class NormalController extends \OCP\AppFramework\Controller { + public function foo() { + } +} diff --git a/tests/lib/AppFramework/Middleware/Security/Mock/OCSController.php b/tests/lib/AppFramework/Middleware/Security/Mock/OCSController.php new file mode 100644 index 00000000000..d053124fe19 --- /dev/null +++ b/tests/lib/AppFramework/Middleware/Security/Mock/OCSController.php @@ -0,0 +1,31 @@ +<?php + +declare(strict_types=1); + +/** + * @copyright Copyright (c) 2023 Joas Schilling <coding@schilljs.com> + * + * @author Joas Schilling <coding@schilljs.com> + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +namespace Test\AppFramework\Middleware\Security\Mock; + +class OCSController extends \OCP\AppFramework\OCSController { + public function foo() { + } +} diff --git a/tests/lib/AppFramework/Middleware/Security/Mock/PasswordConfirmationMiddlewareController.php b/tests/lib/AppFramework/Middleware/Security/Mock/PasswordConfirmationMiddlewareController.php new file mode 100644 index 00000000000..5b83575f711 --- /dev/null +++ b/tests/lib/AppFramework/Middleware/Security/Mock/PasswordConfirmationMiddlewareController.php @@ -0,0 +1,49 @@ +<?php + +declare(strict_types=1); + +/** + * @copyright Copyright (c) 2023 Joas Schilling <coding@schilljs.com> + * + * @author Joas Schilling <coding@schilljs.com> + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +namespace Test\AppFramework\Middleware\Security\Mock; + +use OCP\AppFramework\Http\Attribute\PasswordConfirmationRequired; + +class PasswordConfirmationMiddlewareController extends \OCP\AppFramework\Controller { + public function testNoAnnotationNorAttribute() { + } + + /** + * @TestAnnotation + */ + public function testDifferentAnnotation() { + } + + /** + * @PasswordConfirmationRequired + */ + public function testAnnotation() { + } + + #[PasswordConfirmationRequired] + public function testAttribute() { + } +} diff --git a/tests/lib/AppFramework/Middleware/Security/Mock/SecurityMiddlewareController.php b/tests/lib/AppFramework/Middleware/Security/Mock/SecurityMiddlewareController.php new file mode 100644 index 00000000000..b0a59faba78 --- /dev/null +++ b/tests/lib/AppFramework/Middleware/Security/Mock/SecurityMiddlewareController.php @@ -0,0 +1,175 @@ +<?php + +declare(strict_types=1); + +/** + * @copyright Copyright (c) 2023 Joas Schilling <coding@schilljs.com> + * + * @author Joas Schilling <coding@schilljs.com> + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +namespace Test\AppFramework\Middleware\Security\Mock; + +use OCP\AppFramework\Http\Attribute\NoAdminRequired; +use OCP\AppFramework\Http\Attribute\NoCSRFRequired; +use OCP\AppFramework\Http\Attribute\PublicPage; +use OCP\AppFramework\Http\Attribute\StrictCookiesRequired; +use OCP\AppFramework\Http\Attribute\SubAdminRequired; + +class SecurityMiddlewareController extends \OCP\AppFramework\Controller { + /** + * @PublicPage + * @NoCSRFRequired + */ + public function testAnnotationNoCSRFRequiredPublicPage() { + } + + /** + * @NoCSRFRequired + */ + #[PublicPage] + public function testAnnotationNoCSRFRequiredAttributePublicPage() { + } + + /** + * @PublicPage + */ + #[NoCSRFRequired] + public function testAnnotationPublicPageAttributeNoCSRFRequired() { + } + + #[NoCSRFRequired] + #[PublicPage] + public function testAttributeNoCSRFRequiredPublicPage() { + } + + public function testNoAnnotationNorAttribute() { + } + + /** + * @NoCSRFRequired + */ + public function testAnnotationNoCSRFRequired() { + } + + #[NoCSRFRequired] + public function testAttributeNoCSRFRequired() { + } + + /** + * @PublicPage + */ + public function testAnnotationPublicPage() { + } + + #[PublicPage] + public function testAttributePublicPage() { + } + + /** + * @PublicPage + * @StrictCookieRequired + */ + public function testAnnotationPublicPageStrictCookieRequired() { + } + + /** + * @StrictCookieRequired + */ + #[PublicPage] + public function testAnnotationStrictCookieRequiredAttributePublicPage() { + } + + /** + * @PublicPage + */ + #[StrictCookiesRequired] + public function testAnnotationPublicPageAttributeStrictCookiesRequired() { + } + + #[PublicPage] + #[StrictCookiesRequired] + public function testAttributePublicPageStrictCookiesRequired() { + } + + /** + * @PublicPage + * @NoCSRFRequired + * @StrictCookieRequired + */ + public function testAnnotationNoCSRFRequiredPublicPageStrictCookieRequired() { + } + + #[NoCSRFRequired] + #[PublicPage] + #[StrictCookiesRequired] + public function testAttributeNoCSRFRequiredPublicPageStrictCookiesRequired() { + } + + /** + * @NoCSRFRequired + * @NoAdminRequired + */ + public function testAnnotationNoAdminRequiredNoCSRFRequired() { + } + + #[NoAdminRequired] + #[NoCSRFRequired] + public function testAttributeNoAdminRequiredNoCSRFRequired() { + } + + /** + * @NoCSRFRequired + * @SubAdminRequired + */ + public function testAnnotationNoCSRFRequiredSubAdminRequired() { + } + + /** + * @SubAdminRequired + */ + #[NoCSRFRequired] + public function testAnnotationNoCSRFRequiredAttributeSubAdminRequired() { + } + + /** + * @NoCSRFRequired + */ + #[SubAdminRequired] + public function testAnnotationSubAdminRequiredAttributeNoCSRFRequired() { + } + + #[NoCSRFRequired] + #[SubAdminRequired] + public function testAttributeNoCSRFRequiredSubAdminRequired() { + } + + /** + * @PublicPage + * @NoAdminRequired + * @NoCSRFRequired + */ + public function testAnnotationNoAdminRequiredNoCSRFRequiredPublicPage() { + } + + #[NoAdminRequired] + #[NoCSRFRequired] + #[PublicPage] + public function testAttributeNoAdminRequiredNoCSRFRequiredPublicPage() { + } +} diff --git a/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php index 3153d7f0b08..3752259c61b 100644 --- a/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php @@ -26,11 +26,12 @@ namespace Test\AppFramework\Middleware\Security; use OC\AppFramework\Middleware\Security\Exceptions\NotConfirmedException; use OC\AppFramework\Middleware\Security\PasswordConfirmationMiddleware; use OC\AppFramework\Utility\ControllerMethodReflector; -use OCP\AppFramework\Controller; use OCP\AppFramework\Utility\ITimeFactory; +use OCP\IRequest; use OCP\ISession; use OCP\IUser; use OCP\IUserSession; +use Test\AppFramework\Middleware\Security\Mock\PasswordConfirmationMiddlewareController; use Test\TestCase; class PasswordConfirmationMiddlewareTest extends TestCase { @@ -44,8 +45,8 @@ class PasswordConfirmationMiddlewareTest extends TestCase { private $user; /** @var PasswordConfirmationMiddleware */ private $middleware; - /** @var Controller */ - private $contoller; + /** @var PasswordConfirmationMiddlewareController */ + private $controller; /** @var ITimeFactory|\PHPUnit\Framework\MockObject\MockObject */ private $timeFactory; @@ -54,8 +55,11 @@ class PasswordConfirmationMiddlewareTest extends TestCase { $this->session = $this->createMock(ISession::class); $this->userSession = $this->createMock(IUserSession::class); $this->user = $this->createMock(IUser::class); - $this->contoller = $this->createMock(Controller::class); $this->timeFactory = $this->createMock(ITimeFactory::class); + $this->controller = new PasswordConfirmationMiddlewareController( + 'test', + $this->createMock(IRequest::class) + ); $this->middleware = new PasswordConfirmationMiddleware( $this->reflector, @@ -65,35 +69,59 @@ class PasswordConfirmationMiddlewareTest extends TestCase { ); } - public function testNoAnnotation() { - $this->reflector->reflect(__CLASS__, __FUNCTION__); + public function testNoAnnotationNorAttribute() { + $this->reflector->reflect($this->controller, __FUNCTION__); $this->session->expects($this->never()) ->method($this->anything()); $this->userSession->expects($this->never()) ->method($this->anything()); - $this->middleware->beforeController($this->contoller, __FUNCTION__); + $this->middleware->beforeController($this->controller, __FUNCTION__); } - /** - * @TestAnnotation - */ public function testDifferentAnnotation() { - $this->reflector->reflect(__CLASS__, __FUNCTION__); + $this->reflector->reflect($this->controller, __FUNCTION__); $this->session->expects($this->never()) ->method($this->anything()); $this->userSession->expects($this->never()) ->method($this->anything()); - $this->middleware->beforeController($this->contoller, __FUNCTION__); + $this->middleware->beforeController($this->controller, __FUNCTION__); } /** - * @PasswordConfirmationRequired * @dataProvider dataProvider */ public function testAnnotation($backend, $lastConfirm, $currentTime, $exception) { - $this->reflector->reflect(__CLASS__, __FUNCTION__); + $this->reflector->reflect($this->controller, __FUNCTION__); + + $this->user->method('getBackendClassName') + ->willReturn($backend); + $this->userSession->method('getUser') + ->willReturn($this->user); + + $this->session->method('get') + ->with('last-password-confirm') + ->willReturn($lastConfirm); + + $this->timeFactory->method('getTime') + ->willReturn($currentTime); + + $thrown = false; + try { + $this->middleware->beforeController($this->controller, __FUNCTION__); + } catch (NotConfirmedException $e) { + $thrown = true; + } + + $this->assertSame($exception, $thrown); + } + + /** + * @dataProvider dataProvider + */ + public function testAttribute($backend, $lastConfirm, $currentTime, $exception) { + $this->reflector->reflect($this->controller, __FUNCTION__); $this->user->method('getBackendClassName') ->willReturn($backend); @@ -109,7 +137,7 @@ class PasswordConfirmationMiddlewareTest extends TestCase { $thrown = false; try { - $this->middleware->beforeController($this->contoller, __FUNCTION__); + $this->middleware->beforeController($this->controller, __FUNCTION__); } catch (NotConfirmedException $e) { $thrown = true; } diff --git a/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php index c68ada87657..7c59a8c1452 100644 --- a/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php @@ -34,7 +34,6 @@ use OC\AppFramework\Middleware\Security\SecurityMiddleware; use OC\AppFramework\Utility\ControllerMethodReflector; use OC\Settings\AuthorizedGroupMapper; use OCP\App\IAppManager; -use OCP\AppFramework\Controller; use OCP\AppFramework\Http\JSONResponse; use OCP\AppFramework\Http\RedirectResponse; use OCP\AppFramework\Http\TemplateResponse; @@ -46,11 +45,14 @@ use OCP\IRequestId; use OCP\IURLGenerator; use OCP\IUserSession; 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 Controller|\PHPUnit\Framework\MockObject\MockObject */ + /** @var SecurityMiddlewareController */ private $controller; /** @var SecurityException */ private $secException; @@ -80,12 +82,15 @@ class SecurityMiddlewareTest extends \Test\TestCase { $this->authorizedGroupMapper = $this->createMock(AuthorizedGroupMapper::class); $this->userSession = $this->createMock(IUserSession::class); - $this->controller = $this->createMock(Controller::class); + $this->request = $this->createMock(IRequest::class); + $this->controller = new SecurityMiddlewareController( + 'test', + $this->request + ); $this->reader = new ControllerMethodReflector(); $this->logger = $this->createMock(LoggerInterface::class); $this->navigationManager = $this->createMock(INavigationManager::class); $this->urlGenerator = $this->createMock(IURLGenerator::class); - $this->request = $this->createMock(IRequest::class); $this->l10n = $this->createMock(IL10N::class); $this->middleware = $this->getMiddleware(true, true, false); $this->secException = new SecurityException('hey', false); @@ -115,18 +120,78 @@ class SecurityMiddlewareTest extends \Test\TestCase { ); } + public function dataNoCSRFRequiredPublicPage(): array { + return [ + ['testAnnotationNoCSRFRequiredPublicPage'], + ['testAnnotationNoCSRFRequiredAttributePublicPage'], + ['testAnnotationPublicPageAttributeNoCSRFRequired'], + ['testAttributeNoCSRFRequiredPublicPage'], + ]; + } + + public function dataPublicPage(): array { + return [ + ['testAnnotationPublicPage'], + ['testAttributePublicPage'], + ]; + } + + public function dataNoCSRFRequired(): array { + return [ + ['testAnnotationNoCSRFRequired'], + ['testAttributeNoCSRFRequired'], + ]; + } + + public function dataPublicPageStrictCookieRequired(): array { + return [ + ['testAnnotationPublicPageStrictCookieRequired'], + ['testAnnotationStrictCookieRequiredAttributePublicPage'], + ['testAnnotationPublicPageAttributeStrictCookiesRequired'], + ['testAttributePublicPageStrictCookiesRequired'], + ]; + } + + public function dataNoCSRFRequiredPublicPageStrictCookieRequired(): array { + return [ + ['testAnnotationNoCSRFRequiredPublicPageStrictCookieRequired'], + ['testAttributeNoCSRFRequiredPublicPageStrictCookiesRequired'], + ]; + } + + public function dataNoAdminRequiredNoCSRFRequired(): array { + return [ + ['testAnnotationNoAdminRequiredNoCSRFRequired'], + ['testAttributeNoAdminRequiredNoCSRFRequired'], + ]; + } + + public function dataNoAdminRequiredNoCSRFRequiredPublicPage(): array { + return [ + ['testAnnotationNoAdminRequiredNoCSRFRequiredPublicPage'], + ['testAttributeNoAdminRequiredNoCSRFRequiredPublicPage'], + ]; + } + + public function dataNoCSRFRequiredSubAdminRequired(): array { + return [ + ['testAnnotationNoCSRFRequiredSubAdminRequired'], + ['testAnnotationNoCSRFRequiredAttributeSubAdminRequired'], + ['testAnnotationSubAdminRequiredAttributeNoCSRFRequired'], + ['testAttributeNoCSRFRequiredSubAdminRequired'], + ]; + } /** - * @PublicPage - * @NoCSRFRequired + * @dataProvider dataNoCSRFRequiredPublicPage */ - public function testSetNavigationEntry() { + public function testSetNavigationEntry(string $method): void { $this->navigationManager->expects($this->once()) ->method('setActiveEntry') ->with($this->equalTo('files')); - $this->reader->reflect(__CLASS__, __FUNCTION__); - $this->middleware->beforeController($this->controller, __FUNCTION__); + $this->reader->reflect($this->controller, $method); + $this->middleware->beforeController($this->controller, $method); } @@ -146,7 +211,7 @@ class SecurityMiddlewareTest extends \Test\TestCase { $sec = $this->getMiddleware($isLoggedIn, $isAdminUser, false); try { - $this->reader->reflect(__CLASS__, $method); + $this->reader->reflect($this->controller, $method); $sec->beforeController($this->controller, $method); } catch (SecurityException $ex) { $this->assertEquals($status, $ex->getCode()); @@ -159,75 +224,71 @@ class SecurityMiddlewareTest extends \Test\TestCase { } } - public function testAjaxStatusLoggedInCheck() { + public function testAjaxStatusLoggedInCheck(): void { $this->ajaxExceptionStatus( - __FUNCTION__, + 'testNoAnnotationNorAttribute', 'isLoggedIn', Http::STATUS_UNAUTHORIZED ); } /** - * @NoCSRFRequired + * @dataProvider dataNoCSRFRequired */ - public function testAjaxNotAdminCheck() { + public function testAjaxNotAdminCheck(string $method): void { $this->ajaxExceptionStatus( - __FUNCTION__, + $method, 'isAdminUser', Http::STATUS_FORBIDDEN ); } /** - * @PublicPage + * @dataProvider dataPublicPage */ - public function testAjaxStatusCSRFCheck() { + public function testAjaxStatusCSRFCheck(string $method): void { $this->ajaxExceptionStatus( - __FUNCTION__, + $method, 'passesCSRFCheck', Http::STATUS_PRECONDITION_FAILED ); } /** - * @PublicPage - * @NoCSRFRequired + * @dataProvider dataNoCSRFRequiredPublicPage */ - public function testAjaxStatusAllGood() { + public function testAjaxStatusAllGood(string $method): void { $this->ajaxExceptionStatus( - __FUNCTION__, + $method, 'isLoggedIn', 0 ); $this->ajaxExceptionStatus( - __FUNCTION__, + $method, 'isAdminUser', 0 ); $this->ajaxExceptionStatus( - __FUNCTION__, + $method, 'passesCSRFCheck', 0 ); } - /** - * @PublicPage - * @NoCSRFRequired + * @dataProvider dataNoCSRFRequiredPublicPage */ - public function testNoChecks() { + public function testNoChecks(string $method): void { $this->request->expects($this->never()) ->method('passesCSRFCheck') ->willReturn(false); $sec = $this->getMiddleware(false, false, false); - $this->reader->reflect(__CLASS__, __FUNCTION__); - $sec->beforeController($this->controller, __FUNCTION__); + $this->reader->reflect($this->controller, $method); + $sec->beforeController($this->controller, $method); } - /** * @param string $method * @param string $expects @@ -250,15 +311,15 @@ class SecurityMiddlewareTest extends \Test\TestCase { $this->addToAssertionCount(1); } - $this->reader->reflect(__CLASS__, $method); + $this->reader->reflect($this->controller, $method); $sec->beforeController($this->controller, $method); } /** - * @PublicPage + * @dataProvider dataPublicPage */ - public function testCsrfCheck() { + public function testCsrfCheck(string $method): void { $this->expectException(\OC\AppFramework\Middleware\Security\Exceptions\CrossSiteRequestForgeryException::class); $this->request->expects($this->once()) @@ -267,28 +328,26 @@ class SecurityMiddlewareTest extends \Test\TestCase { $this->request->expects($this->once()) ->method('passesStrictCookieCheck') ->willReturn(true); - $this->reader->reflect(__CLASS__, __FUNCTION__); - $this->middleware->beforeController($this->controller, __FUNCTION__); + $this->reader->reflect($this->controller, $method); + $this->middleware->beforeController($this->controller, $method); } - /** - * @PublicPage - * @NoCSRFRequired + * @dataProvider dataNoCSRFRequiredPublicPage */ - public function testNoCsrfCheck() { + public function testNoCsrfCheck(string $method) { $this->request->expects($this->never()) ->method('passesCSRFCheck') ->willReturn(false); - $this->reader->reflect(__CLASS__, __FUNCTION__); - $this->middleware->beforeController($this->controller, __FUNCTION__); + $this->reader->reflect($this->controller, $method); + $this->middleware->beforeController($this->controller, $method); } /** - * @PublicPage + * @dataProvider dataPublicPage */ - public function testPassesCsrfCheck() { + public function testPassesCsrfCheck(string $method): void { $this->request->expects($this->once()) ->method('passesCSRFCheck') ->willReturn(true); @@ -296,14 +355,14 @@ class SecurityMiddlewareTest extends \Test\TestCase { ->method('passesStrictCookieCheck') ->willReturn(true); - $this->reader->reflect(__CLASS__, __FUNCTION__); - $this->middleware->beforeController($this->controller, __FUNCTION__); + $this->reader->reflect($this->controller, $method); + $this->middleware->beforeController($this->controller, $method); } /** - * @PublicPage + * @dataProvider dataPublicPage */ - public function testFailCsrfCheck() { + public function testFailCsrfCheck(string $method): void { $this->expectException(\OC\AppFramework\Middleware\Security\Exceptions\CrossSiteRequestForgeryException::class); $this->request->expects($this->once()) @@ -313,16 +372,15 @@ class SecurityMiddlewareTest extends \Test\TestCase { ->method('passesStrictCookieCheck') ->willReturn(true); - $this->reader->reflect(__CLASS__, __FUNCTION__); - $this->middleware->beforeController($this->controller, __FUNCTION__); + $this->reader->reflect($this->controller, $method); + $this->middleware->beforeController($this->controller, $method); } /** - * @PublicPage - * @StrictCookieRequired + * @dataProvider dataPublicPageStrictCookieRequired */ - public function testStrictCookieRequiredCheck() { - $this->expectException(\OC\Appframework\Middleware\Security\Exceptions\StrictCookieMissingException::class); + public function testStrictCookieRequiredCheck(string $method): void { + $this->expectException(\OC\AppFramework\Middleware\Security\Exceptions\StrictCookieMissingException::class); $this->request->expects($this->never()) ->method('passesCSRFCheck'); @@ -330,68 +388,57 @@ class SecurityMiddlewareTest extends \Test\TestCase { ->method('passesStrictCookieCheck') ->willReturn(false); - $this->reader->reflect(__CLASS__, __FUNCTION__); - $this->middleware->beforeController($this->controller, __FUNCTION__); + $this->reader->reflect($this->controller, $method); + $this->middleware->beforeController($this->controller, $method); } - /** - * @PublicPage - * @NoCSRFRequired + * @dataProvider dataNoCSRFRequiredPublicPage */ - public function testNoStrictCookieRequiredCheck() { + public function testNoStrictCookieRequiredCheck(string $method): void { $this->request->expects($this->never()) ->method('passesStrictCookieCheck') ->willReturn(false); - $this->reader->reflect(__CLASS__, __FUNCTION__); - $this->middleware->beforeController($this->controller, __FUNCTION__); + $this->reader->reflect($this->controller, $method); + $this->middleware->beforeController($this->controller, $method); } /** - * @PublicPage - * @NoCSRFRequired - * @StrictCookieRequired + * @dataProvider dataNoCSRFRequiredPublicPageStrictCookieRequired */ - public function testPassesStrictCookieRequiredCheck() { + public function testPassesStrictCookieRequiredCheck(string $method): void { $this->request ->expects($this->once()) ->method('passesStrictCookieCheck') ->willReturn(true); - $this->reader->reflect(__CLASS__, __FUNCTION__); - $this->middleware->beforeController($this->controller, __FUNCTION__); + $this->reader->reflect($this->controller, $method); + $this->middleware->beforeController($this->controller, $method); } - public function dataCsrfOcsController() { - $controller = $this->getMockBuilder('OCP\AppFramework\Controller') - ->disableOriginalConstructor() - ->getMock(); - $ocsController = $this->getMockBuilder('OCP\AppFramework\OCSController') - ->disableOriginalConstructor() - ->getMock(); - + public function dataCsrfOcsController(): array { return [ - [$controller, false, false, true], - [$controller, false, true, true], - [$controller, true, false, true], - [$controller, true, true, true], - - [$ocsController, false, false, true], - [$ocsController, false, true, false], - [$ocsController, true, false, false], - [$ocsController, true, true, false], + [NormalController::class, false, false, true], + [NormalController::class, false, true, true], + [NormalController::class, true, false, true], + [NormalController::class, true, true, true], + + [OCSController::class, false, false, true], + [OCSController::class, false, true, false], + [OCSController::class, true, false, false], + [OCSController::class, true, true, false], ]; } /** * @dataProvider dataCsrfOcsController - * @param Controller $controller + * @param string $controllerClass * @param bool $hasOcsApiHeader * @param bool $hasBearerAuth * @param bool $exception */ - public function testCsrfOcsController(Controller $controller, bool $hasOcsApiHeader, bool $hasBearerAuth, bool $exception) { + public function testCsrfOcsController(string $controllerClass, bool $hasOcsApiHeader, bool $hasBearerAuth, bool $exception): void { $this->request ->method('getHeader') ->willReturnCallback(function ($header) use ($hasOcsApiHeader, $hasBearerAuth) { @@ -407,6 +454,8 @@ class SecurityMiddlewareTest extends \Test\TestCase { ->method('passesStrictCookieCheck') ->willReturn(true); + $controller = new $controllerClass('test', $this->request); + try { $this->middleware->beforeController($controller, 'foo'); $this->assertFalse($exception); @@ -416,71 +465,117 @@ class SecurityMiddlewareTest extends \Test\TestCase { } /** - * @NoCSRFRequired - * @NoAdminRequired + * @dataProvider dataNoAdminRequiredNoCSRFRequired */ - public function testLoggedInCheck() { - $this->securityCheck(__FUNCTION__, 'isLoggedIn'); + public function testLoggedInCheck(string $method): void { + $this->securityCheck($method, 'isLoggedIn'); } - /** - * @NoCSRFRequired - * @NoAdminRequired + * @dataProvider dataNoAdminRequiredNoCSRFRequired */ - public function testFailLoggedInCheck() { - $this->securityCheck(__FUNCTION__, 'isLoggedIn', true); + public function testFailLoggedInCheck(string $method): void { + $this->securityCheck($method, 'isLoggedIn', true); } - /** - * @NoCSRFRequired + * @dataProvider dataNoCSRFRequired */ - public function testIsAdminCheck() { - $this->securityCheck(__FUNCTION__, 'isAdminUser'); + public function testIsAdminCheck(string $method): void { + $this->securityCheck($method, 'isAdminUser'); } /** - * @NoCSRFRequired - * @SubAdminRequired + * @dataProvider dataNoCSRFRequiredSubAdminRequired */ - public function testIsNotSubAdminCheck() { - $this->reader->reflect(__CLASS__, __FUNCTION__); + public function testIsNotSubAdminCheck(string $method): void { + $this->reader->reflect($this->controller, $method); $sec = $this->getMiddleware(true, false, false); $this->expectException(SecurityException::class); - $sec->beforeController($this, __METHOD__); + $sec->beforeController($this->controller, $method); } /** - * @NoCSRFRequired - * @SubAdminRequired + * @dataProvider dataNoCSRFRequiredSubAdminRequired */ - public function testIsSubAdminCheck() { - $this->reader->reflect(__CLASS__, __FUNCTION__); + public function testIsSubAdminCheck(string $method): void { + $this->reader->reflect($this->controller, $method); $sec = $this->getMiddleware(true, false, true); - $sec->beforeController($this, __METHOD__); + $sec->beforeController($this->controller, $method); $this->addToAssertionCount(1); } /** - * @NoCSRFRequired - * @SubAdminRequired + * @dataProvider dataNoCSRFRequiredSubAdminRequired */ - public function testIsSubAdminAndAdminCheck() { - $this->reader->reflect(__CLASS__, __FUNCTION__); + public function testIsSubAdminAndAdminCheck(string $method): void { + $this->reader->reflect($this->controller, $method); $sec = $this->getMiddleware(true, true, true); - $sec->beforeController($this, __METHOD__); + $sec->beforeController($this->controller, $method); $this->addToAssertionCount(1); } /** - * @NoCSRFRequired + * @dataProvider dataNoCSRFRequired */ - public function testFailIsAdminCheck() { - $this->securityCheck(__FUNCTION__, 'isAdminUser', true); + public function testFailIsAdminCheck(string $method): void { + $this->securityCheck($method, 'isAdminUser', true); + } + + /** + * @dataProvider dataNoAdminRequiredNoCSRFRequiredPublicPage + */ + public function testRestrictedAppLoggedInPublicPage(string $method): void { + $middleware = $this->getMiddleware(true, false, false); + $this->reader->reflect($this->controller, $method); + + $this->appManager->method('getAppPath') + ->with('files') + ->willReturn('foo'); + + $this->appManager->method('isEnabledForUser') + ->with('files') + ->willReturn(false); + + $middleware->beforeController($this->controller, $method); + $this->addToAssertionCount(1); + } + + /** + * @dataProvider dataNoAdminRequiredNoCSRFRequiredPublicPage + */ + public function testRestrictedAppNotLoggedInPublicPage(string $method): void { + $middleware = $this->getMiddleware(false, false, false); + $this->reader->reflect($this->controller, $method); + + $this->appManager->method('getAppPath') + ->with('files') + ->willReturn('foo'); + + $this->appManager->method('isEnabledForUser') + ->with('files') + ->willReturn(false); + + $middleware->beforeController($this->controller, $method); + $this->addToAssertionCount(1); + } + + /** + * @dataProvider dataNoAdminRequiredNoCSRFRequired + */ + public function testRestrictedAppLoggedIn(string $method): void { + $middleware = $this->getMiddleware(true, false, false, false); + $this->reader->reflect($this->controller, $method); + + $this->appManager->method('getAppPath') + ->with('files') + ->willReturn('foo'); + + $this->expectException(AppNotEnabledException::class); + $middleware->beforeController($this->controller, $method); } @@ -602,75 +697,4 @@ class SecurityMiddlewareTest extends \Test\TestCase { $this->assertTrue($response instanceof JSONResponse); } - - public function dataRestrictedApp() { - return [ - [false, false, false,], - [false, false, true,], - [false, true, false,], - [false, true, true,], - [ true, false, false,], - [ true, false, true,], - [ true, true, false,], - [ true, true, true,], - ]; - } - - /** - * @PublicPage - * @NoAdminRequired - * @NoCSRFRequired - */ - public function testRestrictedAppLoggedInPublicPage() { - $middleware = $this->getMiddleware(true, false, false); - $this->reader->reflect(__CLASS__, __FUNCTION__); - - $this->appManager->method('getAppPath') - ->with('files') - ->willReturn('foo'); - - $this->appManager->method('isEnabledForUser') - ->with('files') - ->willReturn(false); - - $middleware->beforeController($this->controller, __FUNCTION__); - $this->addToAssertionCount(1); - } - - /** - * @PublicPage - * @NoAdminRequired - * @NoCSRFRequired - */ - public function testRestrictedAppNotLoggedInPublicPage() { - $middleware = $this->getMiddleware(false, false, false); - $this->reader->reflect(__CLASS__, __FUNCTION__); - - $this->appManager->method('getAppPath') - ->with('files') - ->willReturn('foo'); - - $this->appManager->method('isEnabledForUser') - ->with('files') - ->willReturn(false); - - $middleware->beforeController($this->controller, __FUNCTION__); - $this->addToAssertionCount(1); - } - - /** - * @NoAdminRequired - * @NoCSRFRequired - */ - public function testRestrictedAppLoggedIn() { - $middleware = $this->getMiddleware(true, false, false, false); - $this->reader->reflect(__CLASS__, __FUNCTION__); - - $this->appManager->method('getAppPath') - ->with('files') - ->willReturn('foo'); - - $this->expectException(AppNotEnabledException::class); - $middleware->beforeController($this->controller, __FUNCTION__); - } } |