Skip to content

Commit 54c8fc4

Browse files
cicnavicicnavi
committed
WIP
1 parent 8519e54 commit 54c8fc4

36 files changed

Lines changed: 2116 additions & 487 deletions

‎hooks/hook_cron.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,9 @@ function oidc_hook_cron(array &$croninfo): void
7070
$issuerStateRepository = $container->get(IssuerStateRepository::class);
7171
$issuerStateRepository->removeInvalid();
7272

73-
/** @var \SimpleSAML\Module\oidc\Repositories\PushedAuthorizationRequestRepository $pushedAuthRepo */
74-
$pushedAuthRepo = $container->get(PushedAuthorizationRequestRepository::class);
75-
$pushedAuthRepo->deleteExpired(new DateTimeImmutable());
73+
/** @var \SimpleSAML\Module\oidc\Repositories\PushedAuthorizationRequestRepository $parRepository */
74+
$parRepository = $container->get(PushedAuthorizationRequestRepository::class);
75+
$parRepository->removeExpired();
7676

7777
$croninfo['summary'][] = 'Module `oidc` clean up. Removed expired entries from storage.';
7878
} catch (Exception $e) {

‎psalm.xml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@
5050

5151
<!-- Ignore MissingOverrideAttribute -->
5252
<MissingOverrideAttribute errorLevel="suppress" />
53-
<MixedAssignment errorLevel="suppress" />
5453
</issueHandlers>
5554
</psalm>
5655

‎routing/services/services.yml‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,6 @@ services:
134134
SimpleSAML\OpenID\Did: ~
135135
SimpleSAML\OpenID\Jws:
136136
factory: [ '@SimpleSAML\Module\oidc\Factories\JwsFactory', 'build' ]
137-
SimpleSAML\OpenID\Jar:
138-
factory: [ '@SimpleSAML\Module\oidc\Factories\JarFactory', 'build' ]
139137
SimpleSAML\OpenID\RequestObject:
140138
factory: [ '@SimpleSAML\Module\oidc\Factories\RequestObjectFactory', 'build' ]
141139

‎src/Controllers/Admin/ClientController.php‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -354,17 +354,17 @@ protected function buildClientEntityFromFormData(
354354
$data[ClaimsEnum::IdTokenSignedResponseAlg->value] :
355355
null;
356356

357-
$requirePushedAuth = (bool)($data['require_pushed_authorization_requests'] ?? false);
358-
$requireSignedReqObj = (bool)($data['require_signed_request_object'] ?? false);
357+
$requirePushedAuth = (bool)($data[ClaimsEnum::RequirePushedAuthorizationRequests->value] ?? false);
358+
$requireSignedReqObj = (bool)($data[ClaimsEnum::RequireSignedRequestObject->value] ?? false);
359359
/** @var mixed $rawRequestUris */
360-
$rawRequestUris = $data['request_uris'] ?? null;
360+
$rawRequestUris = $data[ClaimsEnum::RequestUris->value] ?? null;
361361
$requestUris = is_array($rawRequestUris) ? $rawRequestUris : [];
362362

363363
$extraMetadata = [
364364
ClaimsEnum::IdTokenSignedResponseAlg->value => $idTokenSignedResponseAlg,
365-
'require_pushed_authorization_requests' => $requirePushedAuth,
366-
'require_signed_request_object' => $requireSignedReqObj,
367-
'request_uris' => $requestUris,
365+
ClaimsEnum::RequirePushedAuthorizationRequests->value => $requirePushedAuth,
366+
ClaimsEnum::RequireSignedRequestObject->value => $requireSignedReqObj,
367+
ClaimsEnum::RequestUris->value => $requestUris,
368368
];
369369

370370
$allowedResponseModes = is_array($data[ClientEntity::KEY_ALLOWED_RESPONSE_MODES]) ?

‎src/Controllers/PushedAuthorizationController.php‎

Lines changed: 113 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -17,18 +17,19 @@
1717
use Psr\Http\Message\ResponseInterface;
1818
use Psr\Http\Message\ServerRequestInterface;
1919
use SimpleSAML\Module\oidc\Bridges\PsrHttpBridge;
20-
use SimpleSAML\Module\oidc\Entities\PushedAuthorizationRequestEntity;
20+
use SimpleSAML\Module\oidc\Factories\Entities\PushedAuthorizationRequestEntityFactory;
2121
use SimpleSAML\Module\oidc\Helpers;
22-
use SimpleSAML\Module\oidc\ModuleConfig;
2322
use SimpleSAML\Module\oidc\Repositories\PushedAuthorizationRequestRepository;
2423
use SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException;
24+
use SimpleSAML\Module\oidc\Server\RequestRules\Interfaces\ResultBagInterface;
2525
use SimpleSAML\Module\oidc\Server\RequestRules\RequestRulesManager;
2626
use SimpleSAML\Module\oidc\Server\RequestRules\Result;
2727
use SimpleSAML\Module\oidc\Server\RequestRules\ResultBag;
2828
use SimpleSAML\Module\oidc\Server\RequestRules\Rules\ClientRedirectUriRule;
2929
use SimpleSAML\Module\oidc\Server\RequestRules\Rules\ClientRule;
3030
use SimpleSAML\Module\oidc\Server\RequestRules\Rules\CodeChallengeMethodRule;
3131
use SimpleSAML\Module\oidc\Server\RequestRules\Rules\CodeChallengeRule;
32+
use SimpleSAML\Module\oidc\Server\RequestRules\Rules\RequestObjectRule;
3233
use SimpleSAML\Module\oidc\Server\RequestRules\Rules\RequiredOpenIdScopeRule;
3334
use SimpleSAML\Module\oidc\Server\RequestRules\Rules\ResponseModeRule;
3435
use SimpleSAML\Module\oidc\Server\RequestRules\Rules\ScopeRule;
@@ -37,9 +38,8 @@
3738
use SimpleSAML\Module\oidc\Services\ErrorResponder;
3839
use SimpleSAML\Module\oidc\Services\LoggerService;
3940
use SimpleSAML\Module\oidc\Utils\AuthenticatedOAuth2ClientResolver;
40-
use SimpleSAML\Module\oidc\Utils\JwksResolver;
4141
use SimpleSAML\OpenID\Codebooks\HttpMethodsEnum;
42-
use SimpleSAML\OpenID\RequestObject;
42+
use SimpleSAML\OpenID\Codebooks\ParamsEnum;
4343
use Symfony\Component\HttpFoundation\Request;
4444
use Symfony\Component\HttpFoundation\Response;
4545

@@ -48,31 +48,33 @@ class PushedAuthorizationController
4848
public function __construct(
4949
private readonly AuthenticatedOAuth2ClientResolver $authenticatedOAuth2ClientResolver,
5050
private readonly PushedAuthorizationRequestRepository $pushedAuthorizationRequestRepository,
51+
private readonly PushedAuthorizationRequestEntityFactory $pushedAuthorizationRequestEntityFactory,
5152
private readonly RequestRulesManager $requestRulesManager,
52-
private readonly JwksResolver $jwksResolver,
53-
private readonly RequestObject $requestObject,
54-
private readonly ModuleConfig $moduleConfig,
5553
private readonly PsrHttpBridge $psrHttpBridge,
5654
private readonly ErrorResponder $errorResponder,
5755
private readonly Helpers $helpers,
5856
private readonly LoggerService $logger,
5957
) {
6058
}
6159

60+
/**
61+
* @throws \League\OAuth2\Server\Exception\OAuthServerException
62+
* @throws \Throwable
63+
*/
6264
public function __invoke(ServerRequestInterface $request): ResponseInterface
6365
{
6466
$this->logger->debug('PushedAuthorizationController::__invoke');
6567

66-
if (strtoupper($request->getMethod()) !== 'POST') {
68+
if (strtoupper($request->getMethod()) !== HttpMethodsEnum::POST->value) {
6769
return $this->psrHttpBridge->getResponseFactory()->createResponse()
6870
->withStatus(405)
69-
->withHeader('Allow', 'POST');
71+
->withHeader('Allow', HttpMethodsEnum::POST->value);
7072
}
7173

72-
// 1. Authenticate client
74+
// Authenticate the client in the same way as at the token endpoint.
7375
$resolvedAuth = $this->authenticatedOAuth2ClientResolver->forAnySupportedMethod($request);
7476
if (is_null($resolvedAuth)) {
75-
throw OidcServerException::accessDenied('Client authentication failed');
77+
throw OidcServerException::accessDenied('Client authentication failed.');
7678
}
7779

7880
$client = $resolvedAuth->getClient();
@@ -81,98 +83,63 @@ public function __invoke(ServerRequestInterface $request): ResponseInterface
8183
throw OidcServerException::accessDenied('Confidential client must authenticate.');
8284
}
8385

84-
// 2. Parse request params
8586
$bodyParams = $request->getParsedBody();
86-
$params = is_array($bodyParams) ? $bodyParams : [];
87+
$bodyParams = is_array($bodyParams) ? $bodyParams : [];
8788

88-
// 3. Reject request_uri in PAR body
89-
if (isset($params['request_uri'])) {
89+
// The request_uri authorization request parameter must not be used in pushed authorization requests.
90+
if (array_key_exists(ParamsEnum::RequestUri->value, $bodyParams)) {
9091
throw OidcServerException::invalidRequest(
91-
'request_uri',
92-
'The request_uri parameter MUST NOT be provided in pushed authorization requests.',
92+
ParamsEnum::RequestUri->value,
93+
'The request_uri parameter must not be used in pushed authorization requests.',
9394
);
9495
}
9596

96-
// 4. Handle JAR in PAR (request parameter)
97-
if (isset($params['request'])) {
98-
try {
99-
$requestObject = $this->requestObject->jarRequestObjectFactory()->fromToken((string)$params['request']);
100-
$jwks = $this->jwksResolver->forClient($client);
101-
if (is_null($jwks)) {
102-
throw OidcServerException::invalidRequest(
103-
'request',
104-
'Client JWKS not available for signature verification.',
105-
);
106-
}
107-
$requestObject->verifyWithKeySet($jwks);
108-
109-
if ($requestObject->getClientId() !== $client->getIdentifier()) {
110-
throw OidcServerException::invalidRequest(
111-
'request',
112-
'Client ID in request object does not match authenticated client.',
113-
);
114-
}
115-
116-
$params = array_merge($params, $requestObject->getPayload());
117-
unset($params['request']);
118-
} catch (\Throwable $t) {
119-
throw OidcServerException::invalidRequest('request', 'Invalid request object: ' . $t->getMessage());
120-
}
121-
}
122-
123-
// 5. Build mock request with merged params and run validation rules
124-
$psrRequest = $request->withParsedBody($params)->withQueryParams([]);
125-
97+
// Validate the pushed params as we would an authorization request sent to the authorization endpoint.
98+
// Note that the rules transparently take the Request Object (request param) into account, with
99+
// RequestObjectRule doing its validation (signature, signed-required policy...).
126100
$resultBag = new ResultBag();
127101
$resultBag->add(new Result(ClientRule::class, $client));
128-
129102
$this->requestRulesManager->predefineResultBag($resultBag);
130103

104+
$this->requestRulesManager->setData('default_scope', '');
105+
$this->requestRulesManager->setData('scope_delimiter_string', ' ');
106+
131107
$rulesToExecute = [
132108
StateRule::class,
133109
ClientRedirectUriRule::class,
110+
RequestObjectRule::class,
134111
ResponseModeRule::class,
135112
ScopeRule::class,
136113
RequiredOpenIdScopeRule::class,
137114
CodeChallengeRule::class,
138115
CodeChallengeMethodRule::class,
139116
];
140117

141-
$this->requestRulesManager->setData('default_scope', '');
142-
$this->requestRulesManager->setData('scope_delimiter_string', ' ');
143-
144-
$this->requestRulesManager->check(
145-
$psrRequest,
118+
$resultBag = $this->requestRulesManager->check(
119+
$request,
146120
$rulesToExecute,
147121
new QueryResponseMode(),
148122
[HttpMethodsEnum::POST],
149123
);
150124

151-
// 6. Generate request_uri
152-
$hex = bin2hex(random_bytes(32));
153-
$requestUri = 'urn:ietf:params:oauth:request_uri:' . $hex;
154-
155-
// 7. Persist entity
156-
$ttl = $this->moduleConfig->getParRequestUriTtl();
157-
$expiresAt = $this->helpers->dateTime()->getUtc()->add($ttl);
158-
159-
// Make sure we carry forward all validated params
160-
$entity = new PushedAuthorizationRequestEntity(
161-
requestUri: $requestUri,
162-
clientId: $client->getIdentifier(),
163-
parameters: $params,
164-
expiresAt: \DateTimeImmutable::createFromInterface($expiresAt),
165-
isConsumed: false,
125+
$parameters = $this->resolveParametersToPersist($resultBag, $bodyParams, $client->getIdentifier());
126+
127+
$parEntity = $this->pushedAuthorizationRequestEntityFactory->buildNew(
128+
$client->getIdentifier(),
129+
$parameters,
166130
);
167131

168-
$this->pushedAuthorizationRequestRepository->persist($entity);
132+
$this->pushedAuthorizationRequestRepository->persist($parEntity);
169133

170-
// 8. Respond
171-
$expiresIn = $this->helpers->dateTime()->getSecondsToExpirationTime($expiresAt->getTimestamp());
172-
$responseBody = json_encode([
173-
'request_uri' => $requestUri,
174-
'expires_in' => $expiresIn,
175-
], JSON_THROW_ON_ERROR);
134+
$responseBody = json_encode(
135+
[
136+
'request_uri' => $parEntity->getRequestUri(),
137+
'expires_in' => $this->helpers->dateTime()->getSecondsToExpirationTime(
138+
$parEntity->getExpiresAt()->getTimestamp(),
139+
),
140+
],
141+
JSON_THROW_ON_ERROR,
142+
);
176143

177144
$response = $this->psrHttpBridge->getResponseFactory()->createResponse()
178145
->withStatus(201)
@@ -184,17 +151,85 @@ public function __invoke(ServerRequestInterface $request): ResponseInterface
184151
return $response;
185152
}
186153

154+
/**
155+
* Resolve the authorization request parameters which are to be persisted for later use at the
156+
* authorization endpoint.
157+
*
158+
* @param mixed[] $bodyParams
159+
* @return mixed[]
160+
* @throws \League\OAuth2\Server\Exception\OAuthServerException
161+
*/
162+
protected function resolveParametersToPersist(
163+
ResultBagInterface $resultBag,
164+
array $bodyParams,
165+
string $clientId,
166+
): array {
167+
// If a body client_id param was provided, it must match the authenticated client.
168+
if (
169+
array_key_exists(ParamsEnum::ClientId->value, $bodyParams) &&
170+
$bodyParams[ParamsEnum::ClientId->value] !== $clientId
171+
) {
172+
throw OidcServerException::invalidRequest(
173+
ParamsEnum::ClientId->value,
174+
'The client_id parameter does not match the authenticated client.',
175+
);
176+
}
177+
178+
$requestObjectResult = $resultBag->get(RequestObjectRule::class);
179+
180+
if ($requestObjectResult !== null) {
181+
// Request Object (JAR) was used. Per RFC 9126, all authorization request parameters must appear
182+
// as claims of the Request Object, so only use its (validated) payload.
183+
/** @psalm-suppress MixedAssignment */
184+
$parameters = $resultBag->getOrFail(RequestObjectRule::class)->getValue();
185+
$parameters = is_array($parameters) ? $parameters : [];
186+
187+
/** @psalm-suppress MixedAssignment */
188+
$clientIdClaim = $parameters[ParamsEnum::ClientId->value] ?? null;
189+
if (!is_null($clientIdClaim) && $clientIdClaim !== $clientId) {
190+
throw OidcServerException::invalidRequest(
191+
ParamsEnum::ClientId->value,
192+
'The client_id claim in request object does not match the authenticated client.',
193+
);
194+
}
195+
} else {
196+
// Plain pushed authorization request. Make sure not to persist client authentication related
197+
// params (they are not part of the authorization request itself).
198+
$parameters = $bodyParams;
199+
unset(
200+
$parameters[ParamsEnum::ClientSecret->value],
201+
$parameters[ParamsEnum::ClientAssertion->value],
202+
$parameters[ParamsEnum::ClientAssertionType->value],
203+
);
204+
}
205+
206+
unset(
207+
$parameters[ParamsEnum::Request->value],
208+
$parameters[ParamsEnum::RequestUri->value],
209+
);
210+
211+
// Bind the parameters to the authenticated client.
212+
$parameters[ParamsEnum::ClientId->value] = $clientId;
213+
214+
return $parameters;
215+
}
216+
187217
public function par(Request $request): Response
188218
{
189219
try {
190220
$psrRequest = $this->psrHttpBridge->getPsrHttpFactory()->createRequest($request);
191221
$psrResponse = $this->__invoke($psrRequest);
192222
return $this->psrHttpBridge->getHttpFoundationFactory()->createResponse($psrResponse);
193223
} catch (OAuthServerException $exception) {
194-
return $this->errorResponder->forException($exception);
224+
// Per RFC 9126, the error response format is the one specified for the token endpoint, so make
225+
// sure we never redirect (regardless of any redirect URI contained in the exception).
226+
return $this->errorResponder->forExceptionJson($exception);
195227
} catch (\Throwable $exception) {
196-
return $this->errorResponder->forException(
197-
OidcServerException::invalidRequest('request', $exception->getMessage()),
228+
$this->logger->error(
229+
'PushedAuthorizationController: error processing request: ' . $exception->getMessage(),
230+
);
231+
return $this->errorResponder->forExceptionJson(
232+
OidcServerException::serverError('Unable to process pushed authorization request.'),
198233
);
199234
}
200235
}

‎src/Entities/ClientEntity.php‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -232,9 +232,9 @@ public function toArray(): array
232232

233233
// Extra metadata
234234
ClaimsEnum::IdTokenSignedResponseAlg->value => $this->getIdTokenSignedResponseAlg(),
235-
'require_pushed_authorization_requests' => $this->getRequirePushedAuthorizationRequests(),
236-
'require_signed_request_object' => $this->getRequireSignedRequestObject(),
237-
'request_uris' => $this->getRequestUris(),
235+
ClaimsEnum::RequirePushedAuthorizationRequests->value => $this->getRequirePushedAuthorizationRequests(),
236+
ClaimsEnum::RequireSignedRequestObject->value => $this->getRequireSignedRequestObject(),
237+
ClaimsEnum::RequestUris->value => $this->getRequestUris(),
238238
];
239239
}
240240

@@ -416,7 +416,7 @@ public function getRequirePushedAuthorizationRequests(): bool
416416
return false;
417417
}
418418

419-
return (bool)($this->extraMetadata['require_pushed_authorization_requests'] ?? false);
419+
return (bool)($this->extraMetadata[ClaimsEnum::RequirePushedAuthorizationRequests->value] ?? false);
420420
}
421421

422422
public function getRequireSignedRequestObject(): bool
@@ -425,7 +425,7 @@ public function getRequireSignedRequestObject(): bool
425425
return false;
426426
}
427427

428-
return (bool)($this->extraMetadata['require_signed_request_object'] ?? false);
428+
return (bool)($this->extraMetadata[ClaimsEnum::RequireSignedRequestObject->value] ?? false);
429429
}
430430

431431
/**
@@ -438,7 +438,7 @@ public function getRequestUris(): array
438438
}
439439

440440
/** @var mixed $uris */
441-
$uris = $this->extraMetadata['request_uris'] ?? null;
441+
$uris = $this->extraMetadata[ClaimsEnum::RequestUris->value] ?? null;
442442
if (!is_array($uris)) {
443443
return [];
444444
}

0 commit comments

Comments
 (0)