Repository navigation
DPoP support for token exchange - #47571
Conversation
There was a problem hiding this comment.
@graziang Thanks for the PR!
I've added one comment inline.
However the bigger issue is, that strictly speaking this is backwards incompatible change... Should we perhaps make this as opt-in change and handle that for example with client policy executor?
Another point: For the case when token-exchange is sent by the same client, which token was issued to (in other words, subject_token claim azp is the same as client sending the request), then Keycloak is able to validate DPoP token instead of rejecting it. Should we try to do this then instead of rejecting the request? If we use client policy, it should be likely possible IMO.
| @@ -135,6 +136,15 @@ protected Response tokenExchange() { | |||
| UserSessionModel tokenSession = authResult.session(); | |||
| AccessToken token = authResult.token(); | |||
|
|
|||
| // Reject DPoP-bound tokens as subject_token | |||
| if (DPoPUtil.isDPoPToken(token)) { | |||
There was a problem hiding this comment.
For the access-token, it will be better to check if the token.getType() is DPoP . That is more reliable as checking presence of the claim like cnf is not 100% correct to check that token type is DPoP .
For example the cnf can be present for sender-constrained access tokens, which use X.509 certificate for the binding: https://www.keycloak.org/docs/latest/server_admin/index.html#openid-connect-compatibility-modes . Which raises additional question as those tokens are the similar case like DPoP tokens and hence we might try to apply the same rules for them? But maybe better to handle those tokens as a follow-up rather than use same PR like for DPoP?
Closes keycloak#46092 Signed-off-by: Giuseppe Graziano <g.graziano94@gmail.com>
Closes #46092
Even before this PR DPoP for token exchange was allowed, it is already possible to obtain a DPoP token in a token exchange request.
The only change of this PR is blocking token exchange requests if the
subject_tokenis DPoP-bound. I have also added the documentation.Draft for now as tests are currently missing, waiting for the migration to the new test suite of token exhcange tests in this PR: #47516.