Skip to content

Commit f1e2af5

Browse files
cicnavicicnavi
committed
Move to generic client instantiation to client repo
1 parent 36d09d0 commit f1e2af5

File tree

6 files changed

+58
-30
lines changed

6 files changed

+58
-30
lines changed

‎src/Admin/Authorization.php‎

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,20 @@
99
use SimpleSAML\Module\oidc\Bridges\SspBridge;
1010
use SimpleSAML\Module\oidc\Exceptions\AuthorizationException;
1111
use SimpleSAML\Module\oidc\Services\AuthContextService;
12+
use SimpleSAML\Module\oidc\Services\LoggerService;
1213

1314
class Authorization
1415
{
1516
public function __construct(
1617
protected readonly SspBridge $sspBridge,
1718
protected readonly AuthContextService $authContextService,
19+
protected readonly LoggerService $loggerService,
1820
) {
1921
}
2022

2123
public function isAdmin(): bool
2224
{
25+
$this->loggerService->debug('Authorization::isAdmin');
2326
return $this->sspBridge->utils()->auth()->isAdmin();
2427
}
2528

@@ -28,10 +31,19 @@ public function isAdmin(): bool
2831
*/
2932
public function requireAdmin(bool $forceAdminAuthentication = false): void
3033
{
34+
$this->loggerService->debug('Authorization::requireAdmin');
35+
$this->loggerService->debug(
36+
'Authorization: Force admin authentication:',
37+
['forceAdminAuthentication' => $forceAdminAuthentication],
38+
);
3139
if ($forceAdminAuthentication) {
40+
$this->loggerService->debug('Authorization: Forcing admin authentication.');
3241
try {
3342
$this->sspBridge->utils()->auth()->requireAdmin();
3443
} catch (Exception $exception) {
44+
$this->loggerService->error(
45+
'Authorization: Forcing admin authentication failed: ' . $exception->getMessage(),
46+
);
3547
throw new AuthorizationException(
3648
Translate::noop('Unable to initiate SimpleSAMLphp admin authentication.'),
3749
$exception->getCode(),
@@ -41,7 +53,10 @@ public function requireAdmin(bool $forceAdminAuthentication = false): void
4153
}
4254

4355
if (! $this->isAdmin()) {
56+
$this->loggerService->error('Authorization: User is NOT admin.');
4457
throw new AuthorizationException(Translate::noop('SimpleSAMLphp admin access required.'));
58+
} else {
59+
$this->loggerService->debug('Authorization: User is admin.');
4560
}
4661
}
4762

@@ -50,16 +65,29 @@ public function requireAdmin(bool $forceAdminAuthentication = false): void
5065
*/
5166
public function requireAdminOrUserWithPermission(string $permission): void
5267
{
68+
$this->loggerService->debug('Authorization::requireAdminOrUserWithPermission');
69+
$this->loggerService->debug('Authorization: For permission: ' . $permission);
70+
5371
if ($this->isAdmin()) {
72+
$this->loggerService->debug('Authorization: User is admin, returning.');
5473
return;
74+
} else {
75+
$this->loggerService->debug('Authorization: User is not (authenticated as) admin.');
5576
}
5677

5778
try {
79+
$this->loggerService->debug('Authorization: Checking for user permission.');
5880
$this->authContextService->requirePermission($permission);
59-
} catch (\Exception) {
60-
// TODO mivanci v7 log this exception
81+
$this->loggerService->debug('Authorization: User has permission, returning.');
82+
return;
83+
} catch (\Exception $exception) {
84+
$this->loggerService->warning(
85+
'Authorization: User permission check failed: ' . $exception->getMessage(),
86+
);
6187
}
6288

89+
$this->loggerService->debug('Authorization: Falling back to admin authentication.');
90+
6391
// If we get here, the user does not have the required permission, or permissions are not enabled.
6492
// Fallback to admin authentication.
6593
$this->requireAdmin(true);

‎src/Controllers/VerifiableCredentials/CredentialIssuerCredentialController.php‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,21 +69,22 @@ public function __construct(
6969
*/
7070
public function credential(Request $request): Response
7171
{
72+
$this->loggerService->debug('CredentialIssuerCredentialController::credential');
73+
7274
$requestData = $this->requestParamsResolver->getAllFromRequestBasedOnAllowedMethods(
7375
$this->psrHttpBridge->getPsrHttpFactory()->createRequest($request),
7476
[HttpMethodsEnum::POST],
7577
);
7678

7779
$this->loggerService->debug(
78-
'CredentialIssuerCredentialController::credential: Verifiable Credential request data: ',
80+
'CredentialIssuerCredentialController: Request data: ',
7981
$requestData,
8082
);
8183

8284
$authorization = $this->resourceServer->validateAuthenticatedRequest(
8385
$this->psrHttpBridge->getPsrHttpFactory()->createRequest($request),
8486
);
8587

86-
// TODO mivanci validate access token
8788
$accessToken = $this->accessTokenRepository->findById(
8889
(string)$authorization->getAttribute('oauth_access_token_id'),
8990
);
@@ -109,8 +110,8 @@ public function credential(Request $request): Response
109110
$flowType->isVciFlow() === false
110111
) {
111112
$this->loggerService->warning(
112-
'CredentialIssuerCredentialController::credential: Access token is not intended for verifiable' .
113-
' credential issuance.',
113+
'CredentialIssuerCredentialController::credential: Access token is not intended for Verifiable' .
114+
' Credential Issuance.',
114115
['access_token' => $accessToken],
115116
);
116117
return $this->routes->newJsonErrorResponse(

‎src/Factories/CredentialOfferUriFactory.php‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
use SimpleSAML\Module\oidc\Entities\ScopeEntity;
1414
use SimpleSAML\Module\oidc\Entities\UserEntity;
1515
use SimpleSAML\Module\oidc\Factories\Entities\AuthCodeEntityFactory;
16-
use SimpleSAML\Module\oidc\Factories\Entities\ClientEntityFactory;
1716
use SimpleSAML\Module\oidc\Factories\Entities\IssuerStateEntityFactory;
1817
use SimpleSAML\Module\oidc\Factories\Entities\UserEntityFactory;
1918
use SimpleSAML\Module\oidc\ModuleConfig;
@@ -36,7 +35,6 @@ public function __construct(
3635
protected readonly SspBridge $sspBridge,
3736
protected readonly AuthCodeRepository $authCodeRepository,
3837
protected readonly AuthCodeEntityFactory $authCodeEntityFactory,
39-
protected readonly ClientEntityFactory $clientEntityFactory,
4038
protected readonly ClientRepository $clientRepository,
4139
protected readonly LoggerService $loggerService,
4240
protected readonly UserRepository $userRepository,
@@ -132,13 +130,9 @@ public function buildPreAuthorized(
132130
);
133131

134132
// Currently, we need a dedicated client for which the PreAuthZed code will be bound to.
135-
// TODO mivanci: Remove requirement for dedicated client for (pre-)authorization codes.
136-
$client = $this->clientEntityFactory->getGenericForVci();
137-
if ($this->clientRepository->findById($client->getIdentifier()) === null) {
138-
$this->clientRepository->add($client);
139-
} else {
140-
$this->clientRepository->update($client);
141-
}
133+
// TODO mivanci: Remove requirement for dedicated client for (pre-)authorization codes once the dynamic
134+
// client registration is enabled.
135+
$client = $this->clientRepository->getGenericForVci();
142136

143137
$userId = null;
144138
try {

‎src/Repositories/ClientRepository.php‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -564,4 +564,16 @@ protected function preparePdoState(array $state): array
564564

565565
return $state;
566566
}
567+
568+
public function getGenericForVci(): ClientEntityInterface
569+
{
570+
$client = $this->clientEntityFactory->getGenericForVci();
571+
if ($this->findById($client->getIdentifier()) === null) {
572+
$this->add($client);
573+
} else {
574+
$this->update($client);
575+
}
576+
577+
return $client;
578+
}
567579
}

‎src/Server/RequestRules/Rules/ClientRule.php‎

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ public function checkRule(
132132
'Falling back to generic VCI client.',
133133
);
134134

135-
return new Result($this->getKey(), $this->getGenericVciClient());
135+
return new Result($this->getKey(), $this->clientRepository->getGenericForVci());
136136
} else {
137137
$this->loggerService->debug(
138138
'ClientRule: Not a VCI request, or VCI capabilities not enabled, or VCI with non-registered' .
@@ -365,16 +365,4 @@ public function resolveFromFederation(
365365

366366
return $registrationClient;
367367
}
368-
369-
protected function getGenericVciClient(): ClientEntityInterface
370-
{
371-
$client = $this->clientEntityFactory->getGenericForVci();
372-
if ($this->clientRepository->findById($client->getIdentifier()) === null) {
373-
$this->clientRepository->add($client);
374-
} else {
375-
$this->clientRepository->update($client);
376-
}
377-
378-
return $client;
379-
}
380368
}

‎tests/unit/src/Admin/AuthorizationTest.php‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
use SimpleSAML\Module\oidc\Bridges\SspBridge\Utils;
1414
use SimpleSAML\Module\oidc\Exceptions\AuthorizationException;
1515
use SimpleSAML\Module\oidc\Services\AuthContextService;
16+
use SimpleSAML\Module\oidc\Services\LoggerService;
1617
use SimpleSAML\Utils\Auth;
1718

1819
#[CoversClass(Authorization::class)]
@@ -22,6 +23,7 @@ class AuthorizationTest extends TestCase
2223
protected MockObject $sspBridgeUtilsMock;
2324
protected MockObject $sspBridgeUtilsAuthMock;
2425
protected MockObject $authContextServiceMock;
26+
protected MockObject $loggerServiceMock;
2527

2628
protected function setUp(): void
2729
{
@@ -32,16 +34,19 @@ protected function setUp(): void
3234
$this->sspBridgeUtilsMock->method('auth')->willReturn($this->sspBridgeUtilsAuthMock);
3335

3436
$this->authContextServiceMock = $this->createMock(AuthContextService::class);
37+
$this->loggerServiceMock = $this->createMock(LoggerService::class);
3538
}
3639

3740
protected function sut(
3841
?SspBridge $sspBridge = null,
3942
?AuthContextService $authContextService = null,
43+
?LoggerService $loggerService = null,
4044
): Authorization {
4145
$sspBridge ??= $this->sspBridgeMock;
4246
$authContextService ??= $this->authContextServiceMock;
47+
$loggerService ??= $this->loggerServiceMock;
4348

44-
return new Authorization($sspBridge, $authContextService);
49+
return new Authorization($sspBridge, $authContextService, $loggerService);
4550
}
4651

4752
public function testCanCreateInstance(): void
@@ -100,7 +105,7 @@ public function testRequireAdminOrUserWithPermissionReturnsIfUser(): void
100105
false,
101106
true, // After requireAdmin called, isAdmin will return true
102107
);
103-
$this->sspBridgeUtilsAuthMock->expects($this->once())->method('requireAdmin');
108+
$this->sspBridgeUtilsAuthMock->expects($this->never())->method('requireAdmin');
104109
$this->authContextServiceMock->expects($this->once())->method('requirePermission');
105110

106111
$this->sut()->requireAdminOrUserWithPermission('permission');

0 commit comments

Comments
 (0)