Skip to content

Commit d18509f

Browse files
authored
Harden ID Token Hint implementation (#352)
* Initial id_token_hint implementation * Switch to deficated ID Token Hint abstraction + hardening
1 parent 12fb304 commit d18509f

15 files changed

Lines changed: 618 additions & 33 deletions

composer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
"psr/log": "^3",
3333
"psr/simple-cache": "^3",
3434
"simplesamlphp/composer-module-installer": "^1.3",
35-
"simplesamlphp/openid": "~0.3.8",
35+
"simplesamlphp/openid": "~0.3.12",
3636
"spomky-labs/base64url": "^2.0",
3737
"symfony/cache": "^7.4",
3838
"symfony/expression-language": "^7.4",

docs/6-oidc-upgrade.md

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,36 @@ core login form simply ignore it, and an incorrect value can be corrected by
214214
the user (or the login fails with invalid credentials). No error is raised if
215215
the parameter is present but unused. This also applies to forced
216216
re-authentication triggered by `prompt=login` or an expired `max_age`.
217+
- Support for the `id_token_hint` parameter on the authorization endpoint
218+
(previously ignored; it was already supported on the end session endpoint). The
219+
parameter carries an ID Token previously issued by this OP as a hint about the
220+
End-User's session with the requesting client. When present, it is validated
221+
(issuer, signature, and — since the hint represents a session with the
222+
requesting client — that the client is an audience of the hint) and, after
223+
authentication, the subject (`sub`) that would be issued for the authenticated
224+
End-User is compared to the subject in the `id_token_hint`. If
225+
they differ, a `login_required` error is returned rather than issuing a
226+
token/code for a different End-User than the one the hint identifies. This
227+
applies to all prompt modes: with `prompt=none` it prevents a silent response
228+
for a mismatched cookie session, and with interactive authentication it rejects
229+
the request when a different End-User authenticated than requested (the client
230+
can then retry, e.g. with `prompt=login`). An otherwise-valid `id_token_hint`
231+
whose ID Token has expired is accepted (as recommended by the specification,
232+
since a hint is commonly sent after it has expired); its signature, issuer and
233+
`nbf`/`iat` timestamps are still validated. A malformed, wrongly-issued or
234+
improperly-signed `id_token_hint` results in an `invalid_request` error
235+
redirected back to the client.
236+
- The ID Token subject (`sub`) is now resolved consistently for a given
237+
End-User, independently of the flow, the granted scopes and the client's
238+
`add_claims_to_id_token` setting. Previously, when a `sub` claim mapping was
239+
configured, the mapped value was only applied if the user's claims were
240+
released in the ID Token, so the same End-User could receive different `sub`
241+
values depending on the flow and client. Deployments that did not customize the
242+
`sub` claim mapping are unaffected (the user identifier attributes are used for
243+
`sub` by default, which already produced the same value in both cases). If you
244+
did map `sub` to a different attribute, clients relying on the previously
245+
inconsistent value may see a changed `sub` in flows where the claims were not
246+
released.
217247
- Logging has been improved for authentication flows. It should now be easier
218248
to find information about what went wrong by looking at the relevant log entries.
219249

src/Controllers/AuthorizationController.php

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,13 @@ public function __invoke(ServerRequestInterface $request): ResponseInterface
7777
$state ??= $this->authenticationService->manageState($queryParameters);
7878
$authorizationRequest = $this->authenticationService->getAuthorizationRequestFromState($state);
7979

80+
// Validate any id_token_hint against the authenticated End-User before the user is resolved and (as a side
81+
// effect) associated with the client, so that a mismatched request leaves no relying party association
82+
// behind (which could otherwise later receive a back-channel logout for that End-User).
83+
if ($authorizationRequest instanceof AuthorizationRequest) {
84+
$this->validateIdTokenHint($authorizationRequest, $state);
85+
}
86+
8087
$user = $this->authenticationService->getAuthenticateUser($state);
8188

8289
$authorizationRequest->setUser($user);
@@ -180,6 +187,70 @@ protected function validatePostAuthnAuthorizationRequest(AuthorizationRequest $a
180187
$this->validateAcr($authorizationRequest);
181188
}
182189

190+
/**
191+
* Validate the `id_token_hint` authorization request parameter (if any) against the authenticated End-User.
192+
*
193+
* The hint is an ID Token previously issued by this OP; its issuer and signature were already validated early
194+
* (IdTokenHintRule) and its subject carried on the authorization request. Here, using the released post-authproc
195+
* attributes (the same ones from which the issued subject is derived), we verify that the authenticated End-User
196+
* matches the subject in the hint. Per OpenID Connect Core, the request must not be satisfied for a different
197+
* End-User than the one the hint identifies; if they differ we return `login_required` (the specification's
198+
* suggested error) rather than issuing a token/code for the wrong user. This applies to all prompt modes: with
199+
* `prompt=none` it prevents a silent response for a mismatched cookie session, and with interactive
200+
* authentication it rejects the request when a different End-User authenticated than the hint asked for (the
201+
* client can then retry, e.g. with `prompt=login`).
202+
*
203+
* This runs before the End-User is resolved and associated with the client, so a mismatch does not leave a
204+
* relying party association behind.
205+
*
206+
* @param array<array-key,mixed>|null $state
207+
* @throws \SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException
208+
*/
209+
protected function validateIdTokenHint(AuthorizationRequest $authorizationRequest, ?array $state): void
210+
{
211+
$hintSubject = $authorizationRequest->getIdTokenHintSubject();
212+
if ($hintSubject === null) {
213+
return;
214+
}
215+
216+
// No released attributes means no End-User to match the hint against, so the request can not be satisfied
217+
// for the End-User the hint identifies (subjectMatchesAttributes() returns false for an empty set).
218+
$attributes = (isset($state['Attributes']) && is_array($state['Attributes'])) ? $state['Attributes'] : [];
219+
220+
if ($this->authenticationService->subjectMatchesAttributes($hintSubject, $attributes)) {
221+
return;
222+
}
223+
224+
$this->loggerService->notice(
225+
'Authorization request rejected: the authenticated End-User does not match the `id_token_hint` subject.',
226+
['client_id' => $authorizationRequest->getClient()->getIdentifier()],
227+
);
228+
229+
throw OidcServerException::loginRequired(
230+
'Authenticated End-User does not match the id_token_hint subject.',
231+
$this->resolveRedirectUri($authorizationRequest),
232+
null,
233+
$authorizationRequest->getState(),
234+
$authorizationRequest->getResponseMode(),
235+
);
236+
}
237+
238+
/**
239+
* Resolve the redirect URI to use for redirected error responses: the one validated for this request, or the
240+
* client's first registered redirect URI as a fallback.
241+
*/
242+
protected function resolveRedirectUri(AuthorizationRequest $authorizationRequest): ?string
243+
{
244+
$redirectUri = $authorizationRequest->getRedirectUri();
245+
if ($redirectUri !== null) {
246+
return $redirectUri;
247+
}
248+
249+
$clientRedirectUri = $authorizationRequest->getClient()->getRedirectUri();
250+
251+
return is_array($clientRedirectUri) ? ($clientRedirectUri[0] ?? null) : $clientRedirectUri;
252+
}
253+
183254
/**
184255
* @throws \Exception
185256
*/

src/Server/Grants/AuthCodeGrant.php

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
use SimpleSAML\Module\oidc\Server\RequestRules\Rules\CodeChallengeMethodRule;
5555
use SimpleSAML\Module\oidc\Server\RequestRules\Rules\CodeChallengeRule;
5656
use SimpleSAML\Module\oidc\Server\RequestRules\Rules\CodeVerifierRule;
57+
use SimpleSAML\Module\oidc\Server\RequestRules\Rules\IdTokenHintRule;
5758
use SimpleSAML\Module\oidc\Server\RequestRules\Rules\IssuerStateRule;
5859
use SimpleSAML\Module\oidc\Server\RequestRules\Rules\LoginHintRule;
5960
use SimpleSAML\Module\oidc\Server\RequestRules\Rules\MaxAgeRule;
@@ -861,6 +862,9 @@ public function validateAuthorizationRequestWithRequestRules(
861862
// LoginHintRule must run before PromptRule and MaxAgeRule, which consume its result when they
862863
// trigger re-authentication (prompt=login / expired max_age) to pre-fill the username.
863864
LoginHintRule::class,
865+
// IdTokenHintRule must run before PromptRule, which consumes its result to enforce that a prompt=none
866+
// request is only satisfied for the End-User identified by the id_token_hint.
867+
IdTokenHintRule::class,
864868
PromptRule::class,
865869
MaxAgeRule::class,
866870
ScopeRule::class,
@@ -1000,6 +1004,15 @@ public function validateAuthorizationRequestWithRequestRules(
10001004
$this->loggerService->debug('AuthCodeGrant: Login hint present: ', ['loginHintPresent' => $loginHintPresent]);
10011005
$authorizationRequest->setLoginHint($loginHint);
10021006

1007+
// Carry the id_token_hint subject (if the hint was provided and validated by IdTokenHintRule) so that,
1008+
// after authentication, the authenticated End-User can be verified against the one the hint identifies.
1009+
$idTokenHint = $resultBag->getOrFail(IdTokenHintRule::class)->getValue();
1010+
$this->loggerService->debug(
1011+
'AuthCodeGrant: ID Token hint present: ',
1012+
['idTokenHintPresent' => $idTokenHint !== null],
1013+
);
1014+
$authorizationRequest->setIdTokenHintSubject($idTokenHint?->getSubject());
1015+
10031016

10041017
$authorizationRequest->setIsVciRequest($isVciAuthorizationCodeRequest);
10051018
$flowType = $isVciAuthorizationCodeRequest ?

src/Server/Grants/ImplicitGrant.php

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
use SimpleSAML\Module\oidc\Server\RequestRules\Rules\AddClaimsToIdTokenRule;
2626
use SimpleSAML\Module\oidc\Server\RequestRules\Rules\ClientRedirectUriRule;
2727
use SimpleSAML\Module\oidc\Server\RequestRules\Rules\ClientRule;
28+
use SimpleSAML\Module\oidc\Server\RequestRules\Rules\IdTokenHintRule;
2829
use SimpleSAML\Module\oidc\Server\RequestRules\Rules\LoginHintRule;
2930
use SimpleSAML\Module\oidc\Server\RequestRules\Rules\MaxAgeRule;
3031
use SimpleSAML\Module\oidc\Server\RequestRules\Rules\PromptRule;
@@ -130,6 +131,9 @@ public function validateAuthorizationRequestWithRequestRules(
130131
// LoginHintRule must run before PromptRule and MaxAgeRule, which consume its result when they
131132
// trigger re-authentication (prompt=login / expired max_age) to pre-fill the username.
132133
LoginHintRule::class,
134+
// IdTokenHintRule must run before PromptRule, which consumes its result to enforce that a prompt=none
135+
// request is only satisfied for the End-User identified by the id_token_hint.
136+
IdTokenHintRule::class,
133137
PromptRule::class,
134138
MaxAgeRule::class,
135139
RequiredOpenIdScopeRule::class,
@@ -201,6 +205,11 @@ public function validateAuthorizationRequestWithRequestRules(
201205
$loginHint = $resultBag->getOrFail(LoginHintRule::class)->getValue();
202206
$authorizationRequest->setLoginHint($loginHint);
203207

208+
// Carry the id_token_hint subject (if the hint was provided and validated by IdTokenHintRule) so that,
209+
// after authentication, the authenticated End-User can be verified against the one the hint identifies.
210+
$idTokenHint = $resultBag->getOrFail(IdTokenHintRule::class)->getValue();
211+
$authorizationRequest->setIdTokenHintSubject($idTokenHint?->getSubject());
212+
204213
$responseMode = $resultBag->getOrFail(ResponseModeRule::class)->getValue();
205214
$authorizationRequest->setResponseMode($responseMode);
206215

src/Server/RequestRules/Rules/IdTokenHintRule.php

Lines changed: 59 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
use SimpleSAML\OpenID\Jwks;
2121

2222
/**
23-
* @extends AbstractRule<\SimpleSAML\OpenID\Core\IdToken|null>
23+
* @extends AbstractRule<\SimpleSAML\OpenID\Core\IdTokenHint|null>
2424
*/
2525
class IdTokenHintRule extends AbstractRule
2626
{
@@ -52,6 +52,13 @@ public function checkRule(
5252
): ?Result {
5353
$state = $currentResultBag->getOrFail(StateRule::class)->getValue();
5454

55+
// When this rule runs in the authorization flow, the redirect URI has already been validated and is
56+
// available in the result bag, so validation errors can be redirected back to the client (as required at
57+
// the authorization endpoint). In the logout (end session) flow there is no ClientRedirectUriRule, so this
58+
// resolves to null and the error is returned directly, preserving the previous behavior.
59+
$redirectUriValue = $currentResultBag->get(ClientRedirectUriRule::class)?->getValue();
60+
$redirectUri = is_string($redirectUriValue) ? $redirectUriValue : null;
61+
5562
$idTokenHintParam = $this->requestParamsResolver->getAsStringBasedOnAllowedMethods(
5663
ParamsEnum::IdTokenHint->value,
5764
$request,
@@ -63,52 +70,96 @@ public function checkRule(
6370
}
6471

6572
if (empty($idTokenHintParam)) {
66-
$loggerService->notice('End session request rejected: `id_token_hint` was provided but empty.');
73+
$loggerService->notice('Request rejected: `id_token_hint` was provided but empty.');
6774
throw OidcServerException::invalidRequest(
6875
ParamsEnum::IdTokenHint->value,
6976
'Received empty id_token_hint',
7077
null,
71-
null,
78+
$redirectUri,
7279
$state,
80+
$responseMode,
7381
);
7482
}
7583

7684
$jwks = $this->jwks->jwksDecoratorFactory()->fromJwkDecorators(
7785
...$this->moduleConfig->getProtocolSignatureKeyPairBag()->getAllPublicKeys(),
7886
)->jsonSerialize();
7987

80-
$idTokenHint = $this->core->idTokenFactory()->fromToken($idTokenHintParam);
88+
// Parsing constructs and validates the ID Token Hint (structure and required claims), throwing on any
89+
// problem. We translate those failures into a protocol-level invalid_request error (which is redirected back
90+
// to the client in the authorization flow) instead of letting a raw exception surface as an HTTP 500. The
91+
// dedicated IdTokenHint abstraction deliberately does not validate the `exp` claim, so an otherwise-valid but
92+
// expired hint is accepted (as recommended by OpenID Connect Core, since a hint is commonly sent after it has
93+
// expired); the `nbf` and `iat` timestamps are still validated.
94+
try {
95+
$idTokenHint = $this->core->idTokenHintFactory()->fromToken($idTokenHintParam);
96+
} catch (\Throwable $exception) {
97+
$loggerService->notice(
98+
'Request rejected: `id_token_hint` could not be parsed or validated.',
99+
['exception' => $exception->getMessage()],
100+
);
101+
throw OidcServerException::invalidRequest(
102+
ParamsEnum::IdTokenHint->value,
103+
$exception->getMessage(),
104+
null,
105+
$redirectUri,
106+
$state,
107+
$responseMode,
108+
);
109+
}
81110

82111
if ($idTokenHint->getIssuer() !== $this->moduleConfig->getIssuer()) {
83112
$loggerService->notice(
84-
'End session request rejected: `id_token_hint` was not issued by this OP.',
113+
'Request rejected: `id_token_hint` was not issued by this OP.',
85114
['issuer' => $idTokenHint->getIssuer(), 'expected_issuer' => $this->moduleConfig->getIssuer()],
86115
);
87116
throw OidcServerException::invalidRequest(
88117
ParamsEnum::IdTokenHint->value,
89118
'Invalid ID Token Hint Issuer',
90119
null,
91-
null,
120+
$redirectUri,
92121
$state,
122+
$responseMode,
93123
);
94124
}
95125

96126
try {
97127
$idTokenHint->verifyWithKeySet($jwks);
98128
} catch (\Throwable $exception) {
99129
$loggerService->notice(
100-
'End session request rejected: `id_token_hint` signature verification failed.',
130+
'Request rejected: `id_token_hint` signature verification failed.',
101131
['exception' => $exception->getMessage()],
102132
);
103133
throw OidcServerException::invalidRequest(
104134
ParamsEnum::IdTokenHint->value,
105135
$exception->getMessage(),
106136
null,
107-
null,
137+
$redirectUri,
108138
$state,
139+
$responseMode,
109140
);
110141
}
111142

143+
// In the authorization flow the requesting client is known (ClientRule). An id_token_hint represents the
144+
// End-User's session with the requesting client, so require that client to be an audience of the hint. This
145+
// binds the hint to the requesting client and rejects a token that was issued to a different client. In the
146+
// logout (end session) flow there is no ClientRule in the result bag, so this is skipped, preserving that
147+
// flow's behavior.
148+
$client = $currentResultBag->get(ClientRule::class)?->getValue();
149+
if ($client !== null && !in_array($client->getIdentifier(), $idTokenHint->getAudience(), true)) {
150+
$loggerService->notice(
151+
'Request rejected: `id_token_hint` was not issued to the requesting client.',
152+
['client_id' => $client->getIdentifier()],
153+
);
154+
throw OidcServerException::invalidRequest(
155+
ParamsEnum::IdTokenHint->value,
156+
'ID Token Hint audience does not include the requesting client',
157+
null,
158+
$redirectUri,
159+
$state,
160+
$responseMode,
161+
);
162+
}
112163

113164
return new Result($this->getKey(), $idTokenHint);
114165
}

0 commit comments

Comments
 (0)