diff --git a/src/Admin/Authorization.php b/src/Admin/Authorization.php index 4a6c80d1..2f7a24c7 100644 --- a/src/Admin/Authorization.php +++ b/src/Admin/Authorization.php @@ -56,13 +56,13 @@ public function requireAdminOrUserWithPermission(string $permission): void try { $this->authContextService->requirePermission($permission); - } catch (Exception $exception) { - throw new AuthorizationException( - Translate::noop('User not authorized.'), - $exception->getCode(), - $exception, - ); + } catch (\Exception) { + // TODO mivanci v7 log this exception } + + // If we get here, the user does not have the required permission, or permissions are not enabled. + // Fallback to admin authentication. + $this->requireAdmin(true); } public function getUserId(): string diff --git a/src/Services/AuthContextService.php b/src/Services/AuthContextService.php index b0d6e6db..7783aed2 100644 --- a/src/Services/AuthContextService.php +++ b/src/Services/AuthContextService.php @@ -31,11 +31,6 @@ public function __construct( ) { } - public function isSspAdmin(): bool - { - return $this->sspBridge->utils()->auth()->isAdmin(); - } - /** * @throws \SimpleSAML\Error\Exception * @throws \Exception @@ -57,8 +52,6 @@ public function getAuthUserId(): string */ public function requirePermission(string $neededPermission): void { - $auth = $this->authenticate(); - $permissions = $this->moduleConfig ->config() ->getOptionalConfigItem(ModuleConfig::OPTION_ADMIN_UI_PERMISSIONS, null); @@ -69,6 +62,9 @@ public function requirePermission(string $neededPermission): void if (!$permissions->hasValue($neededPermission)) { throw new RuntimeException('No permission defined for ' . $neededPermission); } + + $auth = $this->authenticate(); + $attributeName = $permissions->getString('attribute'); /** @var string[] $entitlements */ $entitlements = $auth->getAttributes()[$attributeName] ?? []; diff --git a/tests/unit/src/Admin/AuthorizationTest.php b/tests/unit/src/Admin/AuthorizationTest.php index ba2af7c3..e35421c1 100644 --- a/tests/unit/src/Admin/AuthorizationTest.php +++ b/tests/unit/src/Admin/AuthorizationTest.php @@ -95,7 +95,12 @@ public function testRequireAdminOrUserWithPermissionReturnsIfAdmin(): void public function testRequireAdminOrUserWithPermissionReturnsIfUser(): void { - $this->sspBridgeUtilsAuthMock->expects($this->once())->method('isAdmin')->willReturn(false); + $this->sspBridgeUtilsAuthMock->expects($this->atLeastOnce())->method('isAdmin') + ->willReturnOnConsecutiveCalls( + false, + true, // After requireAdmin called, isAdmin will return true + ); + $this->sspBridgeUtilsAuthMock->expects($this->once())->method('requireAdmin'); $this->authContextServiceMock->expects($this->once())->method('requirePermission'); $this->sut()->requireAdminOrUserWithPermission('permission'); @@ -104,9 +109,9 @@ public function testRequireAdminOrUserWithPermissionReturnsIfUser(): void public function testRequireUserWithPermissionThrowsIfUserNotAuthorized(): void { $this->expectException(AuthorizationException::class); - $this->expectExceptionMessage('not authorized'); + $this->expectExceptionMessage('access required'); - $this->sspBridgeUtilsAuthMock->expects($this->once())->method('isAdmin')->willReturn(false); + $this->sspBridgeUtilsAuthMock->expects($this->atLeastOnce())->method('isAdmin')->willReturn(false); $this->authContextServiceMock->expects($this->once())->method('requirePermission') ->willThrowException(new Exception('error'));