upstream/nextcloud
1
0
Fork 0
mirror of https://github.com/nextcloud/server.git synced 2026-02-03 20:41:22 -05:00
Code Issues Projects Releases Packages Wiki Activity Actions 15

refactor: Move hasAnnotationOrAttribute to MiddlewareUtils

Browse source 
Signed-off-by: Carl Schwan <carlschwan@kde.org>
This commit is contained in:
Carl Schwan 2026-01-12 13:15:47 +01:00
parent 6408ed0b51
commit f81475445d
No known key found for this signature in database
GPG key ID: 02325448204E452A
14 changed files with 194 additions and 333 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

7
build/psalm-baseline.xml
View file

@ -3210,13 +3210,6 @@
</DeprecatedMethod>
</file>
<file src="core/Middleware/TwoFactorMiddleware.php">
<DeprecatedInterface>
<code><![CDATA[private]]></code>
</DeprecatedInterface>
<DeprecatedMethod>
<code><![CDATA[hasAnnotation]]></code>
<code><![CDATA[hasAnnotation]]></code>
</DeprecatedMethod>
<NoInterfaceProperties>
<code><![CDATA[$this->request->server]]></code>
</NoInterfaceProperties>

30
core/Middleware/TwoFactorMiddleware.php
View file

@ -11,6 +11,7 @@ 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;
@ -22,13 +23,11 @@ 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 Psr\Log\LoggerInterface;
use ReflectionMethod;
class TwoFactorMiddleware extends Middleware {
@ -37,9 +36,8 @@ class TwoFactorMiddleware extends Middleware {
private Session $userSession,
private ISession $session,
private IURLGenerator $urlGenerator,
private IControllerMethodReflector $reflector,
private MiddlewareUtils $middlewareUtils,
private IRequest $request,
private LoggerInterface $logger,
) {
}
@ -50,7 +48,7 @@ class TwoFactorMiddleware extends Middleware {
public function beforeController($controller, $methodName) {
$reflectionMethod = new ReflectionMethod($controller, $methodName);
if ($this->hasAnnotationOrAttribute($reflectionMethod, 'NoTwoFactorRequired', NoTwoFactorRequired::class)) {
if ($this->middlewareUtils->hasAnnotationOrAttribute($reflectionMethod, 'NoTwoFactorRequired', NoTwoFactorRequired::class)) {
// Route handler explicitly marked to work without finished 2FA are
// not blocked
return;
@ -137,26 +135,4 @@ class TwoFactorMiddleware extends Middleware {
throw $exception;
}
/**
* @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 ($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;
}
}

1
lib/composer/composer/autoload_classmap.php
View file

@ -1103,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',

1
lib/composer/composer/autoload_static.php
View file

@ -1144,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',

7
lib/private/AppFramework/DependencyInjection/DIContainer.php
View file

@ -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),

91
lib/private/AppFramework/Http/Dispatcher.php
View file

@ -26,64 +26,22 @@ use Psr\Log\LoggerInterface;
* Class to dispatch the request to the middleware dispatcher
*/
class Dispatcher {
/** @var MiddlewareDispatcher */
private $middlewareDispatcher;
/** @var Http */
private $protocol;
/** @var ControllerMethodReflector */
private $reflector;
/** @var IRequest */
private $request;
/** @var IConfig */
private $config;
/** @var ConnectionAdapter */
private $connection;
/** @var LoggerInterface */
private $logger;
/** @var IEventLogger */
private $eventLogger;
private ContainerInterface $appContainer;
/**
* @param Http $protocol the http protocol with contains all status headers
* @param MiddlewareDispatcher $middlewareDispatcher the dispatcher which
* runs the middleware
* @param ControllerMethodReflector $reflector the reflector that is used to inject
* the arguments for the controller
* @param IRequest $request the incoming request
* @param IConfig $config
* @param ConnectionAdapter $connection
* @param LoggerInterface $logger
* @param IEventLogger $eventLogger
*/
public function __construct(
Http $protocol,
MiddlewareDispatcher $middlewareDispatcher,
ControllerMethodReflector $reflector,
IRequest $request,
IConfig $config,
ConnectionAdapter $connection,
LoggerInterface $logger,
IEventLogger $eventLogger,
ContainerInterface $appContainer,
private readonly Http $protocol,
private readonly MiddlewareDispatcher $middlewareDispatcher,
private readonly ControllerMethodReflector $reflector,
private readonly IRequest $request,
private readonly IConfig $config,
private readonly ConnectionAdapter $connection,
private readonly LoggerInterface $logger,
private readonly IEventLogger $eventLogger,
private readonly ContainerInterface $appContainer,
) {
$this->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,
];
}

70
lib/private/AppFramework/Middleware/MiddlewareUtils.php Normal file
View file

@ -0,0 +1,70 @@
<?php
declare(strict_types=1);
/**
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH
* SPDX-FileContributor: Carl Schwan
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
namespace OC\AppFramework\Middleware;
use OC\AppFramework\Utility\ControllerMethodReflector;
use OCP\AppFramework\Http\Attribute\AuthorizedAdminSetting;
use Psr\Log\LoggerInterface;
use ReflectionMethod;
/**
* Temporary helper to abstract IControllerMethodReflector and ReflectionMethod
*/
class MiddlewareUtils {
public function __construct(
private readonly ControllerMethodReflector $reflector,
private readonly LoggerInterface $logger,
) {
}
/**
* @template T
*
* @param ReflectionMethod $reflectionMethod
* @param ?string $annotationName
* @param class-string<T> $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;
}
}

90
lib/private/AppFramework/Middleware/Security/CORSMiddleware.php
View file

@ -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<T> $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) {

29
lib/private/AppFramework/Middleware/Security/SameSiteCookieMiddleware.php
View file

@ -9,20 +9,18 @@ declare(strict_types=1);
namespace OC\AppFramework\Middleware\Security;
use OC\AppFramework\Http\Request;
use OC\AppFramework\Middleware\MiddlewareUtils;
use OC\AppFramework\Middleware\Security\Exceptions\LaxSameSiteCookieFailedException;
use OC\AppFramework\Utility\ControllerMethodReflector;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\Attribute\NoSameSiteCookieRequired;
use OCP\AppFramework\Http\Response;
use OCP\AppFramework\Middleware;
use Psr\Log\LoggerInterface;
use ReflectionMethod;
class SameSiteCookieMiddleware extends Middleware {
public function __construct(
private readonly Request $request,
private readonly ControllerMethodReflector $reflector,
private readonly LoggerInterface $logger,
private readonly MiddlewareUtils $middlewareUtils,
) {
}
@ -36,7 +34,7 @@ class SameSiteCookieMiddleware extends Middleware {
}
$reflectionMethod = new ReflectionMethod($controller, $methodName);
$noSSC = $this->hasAnnotationOrAttribute($reflectionMethod, 'NoSameSiteCookieRequired', NoSameSiteCookieRequired::class);
$noSSC = $this->middlewareUtils->hasAnnotationOrAttribute($reflectionMethod, 'NoSameSiteCookieRequired', NoSameSiteCookieRequired::class);
if ($noSSC) {
return;
}
@ -87,25 +85,4 @@ class SameSiteCookieMiddleware 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 ($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;
}
}

103
lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php
View file

@ -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<T> $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

9
tests/Core/Middleware/TwoFactorMiddlewareTest.php
View file

@ -10,6 +10,8 @@ namespace Test\Core\Middleware;
use OC\AppFramework\Http\Attributes\TwoFactorSetUpDoneRequired;
use OC\AppFramework\Http\Request;
use OC\AppFramework\Middleware\MiddlewareUtils;
use OC\AppFramework\Utility\ControllerMethodReflector;
use OC\Authentication\Exceptions\TwoFactorAuthRequiredException;
use OC\Authentication\Exceptions\UserAlreadyLoggedInException;
use OC\Authentication\TwoFactorAuth\Manager;
@ -21,7 +23,6 @@ use OCP\AppFramework\Controller;
use OCP\AppFramework\Http\Attribute\NoTwoFactorRequired;
use OCP\AppFramework\Http\RedirectResponse;
use OCP\AppFramework\Http\Response;
use OCP\AppFramework\Utility\IControllerMethodReflector;
use OCP\Authentication\TwoFactorAuth\ALoginSetupController;
use OCP\Authentication\TwoFactorAuth\IProvider;
use OCP\IConfig;
@ -73,7 +74,7 @@ class TwoFactorMiddlewareTest extends TestCase {
private IUserSession&MockObject $userSession;
private ISession&MockObject $session;
private IURLGenerator&MockObject $urlGenerator;
private IControllerMethodReflector&MockObject $reflector;
private ControllerMethodReflector&MockObject $reflector;
private IRequest $request;
private TwoFactorMiddleware $middleware;
private LoggerInterface&MockObject $logger;
@ -89,7 +90,7 @@ class TwoFactorMiddlewareTest extends TestCase {
->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(
[
@ -101,7 +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->logger);
$this->middleware = new TwoFactorMiddleware($this->twoFactorManager, $this->userSession, $this->session, $this->urlGenerator, new MiddlewareUtils($this->reflector, $this->logger), $this->request);
}
public function testBeforeControllerNotLoggedIn(): void {

37
tests/lib/AppFramework/Middleware/Security/CORSMiddlewareTest.php
View file

@ -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'));
}
}

7
tests/lib/AppFramework/Middleware/Security/SameSiteCookieMiddlewareTest.php
View file

@ -8,6 +8,7 @@
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;
@ -43,9 +44,9 @@ class SameSiteCookieMiddlewareTest extends TestCase {
parent::setUp();
$this->request = $this->createMock(Request::class);
$this->reflector = $this->createMock(ControllerMethodReflector::class);
$this->logger = $this->createMock(LoggerInterface::class);
$this->middleware = new SameSiteCookieMiddleware($this->request, $this->reflector, $this->logger);
$this->reflector = $this->createMock(ControllerMethodReflector::class);
$this->middleware = new SameSiteCookieMiddleware($this->request, new MiddlewareUtils($this->reflector, $this->logger));
}
public function testBeforeControllerNoIndex(): void {
@ -117,7 +118,7 @@ class SameSiteCookieMiddlewareTest extends TestCase {
->willReturn('/myrequri');
$middleware = $this->getMockBuilder(SameSiteCookieMiddleware::class)
->setConstructorArgs([$this->request, $this->reflector, $this->logger])
->setConstructorArgs([$this->request, new MiddlewareUtils($this->reflector, $this->logger)])
->onlyMethods(['setSameSiteCookie'])
->getMock();

45
tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php
View file

@ -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,