diff --git a/lib/Attributes/OnlyUnauthenticatedUsers.php b/lib/Attributes/OnlyUnauthenticatedUsers.php new file mode 100644 index 000000000..eca5e8e7f --- /dev/null +++ b/lib/Attributes/OnlyUnauthenticatedUsers.php @@ -0,0 +1,14 @@ +request->getParam('originalUrl', ''); if (!$this->trustedDomainHelper->isTrustedUrl($originalUrl)) { @@ -287,6 +288,7 @@ public function login(int $idp = 1): Http\RedirectResponse|Http\TemplateResponse */ #[PublicPage] #[NoCSRFRequired] + #[OnlyUnauthenticatedUsers] public function getMetadata(int $idp = 1): Http\DataDownloadResponse { $settings = new Settings($this->samlSettings->getOneLoginSettingsArray($idp)); $metadata = $settings->getSPMetadata(); @@ -302,7 +304,6 @@ public function getMetadata(int $idp = 1): Http\DataDownloadResponse { } /** - * @OnlyUnauthenticatedUsers * @NoSameSiteCookieRequired * * @return Http\RedirectResponse @@ -312,6 +313,7 @@ public function getMetadata(int $idp = 1): Http\DataDownloadResponse { #[PublicPage] #[NoCSRFRequired] #[UseSession] + #[OnlyUnauthenticatedUsers] public function assertionConsumerService(): Http\RedirectResponse { // Fetch and decrypt the cookie $cookie = $this->request->getCookie('saml_data'); @@ -536,29 +538,23 @@ private function tryProcessSLOResponse(?int $idp): array { return [null, null]; } - /** - * @OnlyUnauthenticatedUsers - */ #[PublicPage] #[NoCSRFRequired] + #[OnlyUnauthenticatedUsers] public function notProvisioned(): Http\TemplateResponse { return new Http\TemplateResponse($this->appName, 'notProvisioned', [], 'guest'); } - /** - * @OnlyUnauthenticatedUsers - */ #[PublicPage] #[NoCSRFRequired] + #[OnlyUnauthenticatedUsers] public function notPermitted(): Http\TemplateResponse { return new Http\TemplateResponse($this->appName, 'notPermitted', [], 'guest'); } - /** - * @OnlyUnauthenticatedUsers - */ #[PublicPage] #[NoCSRFRequired] + #[OnlyUnauthenticatedUsers] public function genericError(string $message): Http\TemplateResponse { if (empty($message)) { $message = $this->l->t('Unknown error, please check the log file for more details.'); @@ -566,11 +562,9 @@ public function genericError(string $message): Http\TemplateResponse { return new Http\TemplateResponse($this->appName, 'error', ['message' => $message], 'guest'); } - /** - * @OnlyUnauthenticatedUsers - */ #[PublicPage] #[NoCSRFRequired] + #[OnlyUnauthenticatedUsers] public function selectUserBackEnd(string $redirectUrl = ''): Http\TemplateResponse { $attributes = ['loginUrls' => []]; diff --git a/lib/Middleware/OnlyLoggedInMiddleware.php b/lib/Middleware/OnlyLoggedInMiddleware.php index c32216d81..1d7404b6a 100644 --- a/lib/Middleware/OnlyLoggedInMiddleware.php +++ b/lib/Middleware/OnlyLoggedInMiddleware.php @@ -7,9 +7,9 @@ namespace OCA\User_SAML\Middleware; +use OCA\User_SAML\Attributes\OnlyUnauthenticatedUsers; use OCP\AppFramework\Http\RedirectResponse; use OCP\AppFramework\Middleware; -use OCP\AppFramework\Utility\IControllerMethodReflector; use OCP\IURLGenerator; use OCP\IUserSession; use Override; @@ -23,7 +23,6 @@ class OnlyLoggedInMiddleware extends Middleware { public function __construct( - private readonly IControllerMethodReflector $reflector, private readonly IUserSession $userSession, private readonly IURLGenerator $urlGenerator, ) { @@ -36,7 +35,7 @@ public function __construct( */ #[Override] public function beforeController($controller, $methodName): void { - if ($this->reflector->hasAnnotation('OnlyUnauthenticatedUsers') && $this->userSession->isLoggedIn()) { + if ($this->hasAttribute($controller, $methodName) && $this->userSession->isLoggedIn()) { throw new \Exception('User is already logged-in'); } } @@ -55,4 +54,9 @@ public function afterException($controller, $methodName, \Exception $exception): throw $exception; } + + protected function hasAttribute(object $controller, string $methodName): bool { + $reflectionMethod = new \ReflectionMethod($controller, $methodName); + return !empty($reflectionMethod->getAttributes(OnlyUnauthenticatedUsers::class)); + } } diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml index fe6c27c52..54c84674d 100644 --- a/tests/psalm-baseline.xml +++ b/tests/psalm-baseline.xml @@ -9,11 +9,6 @@ - - - - - diff --git a/tests/unit/Middleware/OnlyLoggedInMiddlewareTest.php b/tests/unit/Middleware/OnlyLoggedInMiddlewareTest.php index 0e3f24323..287ccd55e 100644 --- a/tests/unit/Middleware/OnlyLoggedInMiddlewareTest.php +++ b/tests/unit/Middleware/OnlyLoggedInMiddlewareTest.php @@ -11,7 +11,6 @@ use OCA\User_SAML\Middleware\OnlyLoggedInMiddleware; use OCP\AppFramework\Controller; use OCP\AppFramework\Http\RedirectResponse; -use OCP\AppFramework\Utility\IControllerMethodReflector; use OCP\IURLGenerator; use OCP\IUserSession; use Override; @@ -19,56 +18,59 @@ class OnlyLoggedInMiddlewareTest extends \Test\TestCase { protected IURLGenerator&MockObject $urlGenerator; - private IControllerMethodReflector&MockObject $reflector; private IUserSession&MockObject $userSession; - private OnlyLoggedInMiddleware $onlyLoggedInMiddleware; + private OnlyLoggedInMiddleware&MockObject $onlyLoggedInMiddleware; #[Override] protected function setUp(): void { - $this->reflector = $this->createMock(IControllerMethodReflector::class); $this->userSession = $this->createMock(IUserSession::class); $this->urlGenerator = $this->createMock(IURLGenerator::class); - $this->onlyLoggedInMiddleware = new OnlyLoggedInMiddleware( - $this->reflector, - $this->userSession, - $this->urlGenerator - ); + $this->onlyLoggedInMiddleware = $this->getMockBuilder(OnlyLoggedInMiddleware::class) + ->setConstructorArgs([ + $this->userSession, + $this->urlGenerator, + ]) + ->onlyMethods(['hasAttribute']) + ->getMock(); parent::setUp(); } public function testBeforeControllerWithoutAnnotation(): void { - $this->reflector + $controller = $this->createMock(Controller::class); + $this->onlyLoggedInMiddleware ->expects($this->once()) - ->method('hasAnnotation') - ->with('OnlyUnauthenticatedUsers') + ->method('hasAttribute') + ->with($controller, 'bar') ->willReturn(false); $this->userSession ->expects($this->never()) ->method('isLoggedIn'); - $this->onlyLoggedInMiddleware->beforeController($this->createMock(Controller::class), 'bar'); + $this->onlyLoggedInMiddleware->beforeController($controller, 'bar'); } public function testBeforeControllerWithAnnotationAndNotLoggedIn(): void { - $this->reflector + $controller = $this->createMock(Controller::class); + $this->onlyLoggedInMiddleware ->expects($this->once()) - ->method('hasAnnotation') - ->with('OnlyUnauthenticatedUsers') + ->method('hasAttribute') + ->with($controller, 'bar') ->willReturn(true); $this->userSession ->expects($this->once()) ->method('isLoggedIn') ->willReturn(false); - $this->onlyLoggedInMiddleware->beforeController($this->createMock(Controller::class), 'bar'); + $this->onlyLoggedInMiddleware->beforeController($controller, 'bar'); } public function testBeforeControllerWithAnnotationAndLoggedIn(): void { - $this->reflector + $controller = $this->createMock(Controller::class); + $this->onlyLoggedInMiddleware ->expects($this->once()) - ->method('hasAnnotation') - ->with('OnlyUnauthenticatedUsers') + ->method('hasAttribute') + ->with($controller, 'bar') ->willReturn(true); $this->userSession ->expects($this->once()) @@ -78,7 +80,7 @@ public function testBeforeControllerWithAnnotationAndLoggedIn(): void { $this->expectException(Exception::class); $this->expectExceptionMessage('User is already logged-in'); - $this->onlyLoggedInMiddleware->beforeController($this->createMock(Controller::class), 'bar'); + $this->onlyLoggedInMiddleware->beforeController($controller, 'bar'); } public function testAfterExceptionWithNormalException(): void {