Skip to content

Commit 0080cf2

Browse files
authored
Fallback to admin auth if user permissions not available (#305)
* Fallback to admin auth if user permissions throw * Check if permissions are enabled before enforcing them
1 parent eda4893 commit 0080cf2

File tree

3 files changed

+17
-16
lines changed

3 files changed

+17
-16
lines changed

src/Admin/Authorization.php

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -56,13 +56,13 @@ public function requireAdminOrUserWithPermission(string $permission): void
5656

5757
try {
5858
$this->authContextService->requirePermission($permission);
59-
} catch (Exception $exception) {
60-
throw new AuthorizationException(
61-
Translate::noop('User not authorized.'),
62-
$exception->getCode(),
63-
$exception,
64-
);
59+
} catch (\Exception) {
60+
// TODO mivanci v7 log this exception
6561
}
62+
63+
// If we get here, the user does not have the required permission, or permissions are not enabled.
64+
// Fallback to admin authentication.
65+
$this->requireAdmin(true);
6666
}
6767

6868
public function getUserId(): string

src/Services/AuthContextService.php

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,6 @@ public function __construct(
3131
) {
3232
}
3333

34-
public function isSspAdmin(): bool
35-
{
36-
return $this->sspBridge->utils()->auth()->isAdmin();
37-
}
38-
3934
/**
4035
* @throws \SimpleSAML\Error\Exception
4136
* @throws \Exception
@@ -57,8 +52,6 @@ public function getAuthUserId(): string
5752
*/
5853
public function requirePermission(string $neededPermission): void
5954
{
60-
$auth = $this->authenticate();
61-
6255
$permissions = $this->moduleConfig
6356
->config()
6457
->getOptionalConfigItem(ModuleConfig::OPTION_ADMIN_UI_PERMISSIONS, null);
@@ -69,6 +62,9 @@ public function requirePermission(string $neededPermission): void
6962
if (!$permissions->hasValue($neededPermission)) {
7063
throw new RuntimeException('No permission defined for ' . $neededPermission);
7164
}
65+
66+
$auth = $this->authenticate();
67+
7268
$attributeName = $permissions->getString('attribute');
7369
/** @var string[] $entitlements */
7470
$entitlements = $auth->getAttributes()[$attributeName] ?? [];

tests/unit/src/Admin/AuthorizationTest.php

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,12 @@ public function testRequireAdminOrUserWithPermissionReturnsIfAdmin(): void
9595

9696
public function testRequireAdminOrUserWithPermissionReturnsIfUser(): void
9797
{
98-
$this->sspBridgeUtilsAuthMock->expects($this->once())->method('isAdmin')->willReturn(false);
98+
$this->sspBridgeUtilsAuthMock->expects($this->atLeastOnce())->method('isAdmin')
99+
->willReturnOnConsecutiveCalls(
100+
false,
101+
true, // After requireAdmin called, isAdmin will return true
102+
);
103+
$this->sspBridgeUtilsAuthMock->expects($this->once())->method('requireAdmin');
99104
$this->authContextServiceMock->expects($this->once())->method('requirePermission');
100105

101106
$this->sut()->requireAdminOrUserWithPermission('permission');
@@ -104,9 +109,9 @@ public function testRequireAdminOrUserWithPermissionReturnsIfUser(): void
104109
public function testRequireUserWithPermissionThrowsIfUserNotAuthorized(): void
105110
{
106111
$this->expectException(AuthorizationException::class);
107-
$this->expectExceptionMessage('not authorized');
112+
$this->expectExceptionMessage('access required');
108113

109-
$this->sspBridgeUtilsAuthMock->expects($this->once())->method('isAdmin')->willReturn(false);
114+
$this->sspBridgeUtilsAuthMock->expects($this->atLeastOnce())->method('isAdmin')->willReturn(false);
110115
$this->authContextServiceMock->expects($this->once())->method('requirePermission')
111116
->willThrowException(new Exception('error'));
112117

0 commit comments

Comments
 (0)