From 9a921441ccc1eaeec9903b467eb323b292fa5fcf Mon Sep 17 00:00:00 2001 From: Takashi Norimatsu Date: Thu, 27 Jul 2023 11:00:32 +0900 Subject: [PATCH] Adjustements to the behaviour of dpop_bound_access_tokens switch closes #21920 --- .../keycloak/protocol/oidc/TokenManager.java | 4 +- .../oidc/endpoints/TokenEndpoint.java | 4 +- .../org/keycloak/services/util/DPoPUtil.java | 2 +- .../keycloak/testsuite/oauth/DPoPTest.java | 50 +++++++++---------- 4 files changed, 30 insertions(+), 30 deletions(-) diff --git a/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java b/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java index 3ddfdc5152e..7d0351132fe 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java @@ -498,8 +498,8 @@ public class TokenManager { } if (Profile.isFeatureEnabled(Profile.Feature.DPOP)) { - if (OIDCAdvancedConfigWrapper.fromClientModel(client).isUseDPoP() && client.isPublicClient()) { - DPoP dPoP = (DPoP) session.getAttribute(DPoPUtil.DPOP_SESSION_ATTRIBUTE); + DPoP dPoP = (DPoP) session.getAttribute(DPoPUtil.DPOP_SESSION_ATTRIBUTE); + if (client.isPublicClient() && (OIDCAdvancedConfigWrapper.fromClientModel(client).isUseDPoP() || dPoP != null )) { try { DPoPUtil.validateBinding(refreshToken, dPoP); } catch (VerificationException ex) { diff --git a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenEndpoint.java b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenEndpoint.java index 3a38a2bfa38..1a108d92177 100644 --- a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenEndpoint.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenEndpoint.java @@ -334,7 +334,7 @@ public class TokenEndpoint { private void checkAndRetrieveDPoPProof(boolean isDPoPSupported) { if (!isDPoPSupported) return; - if (clientConfig.isUseDPoP()) { + if (clientConfig.isUseDPoP() || request.getHttpHeaders().getHeaderString(DPoPUtil.DPOP_HTTP_HEADER) != null) { try { dPoP = new DPoPUtil.Validator(session).request(request).uriInfo(session.getContext().getUri()).validate(); session.setAttribute(DPoPUtil.DPOP_SESSION_ATTRIBUTE, dPoP); @@ -547,7 +547,7 @@ public class TokenEndpoint { private void checkAndBindDPoPToken(TokenManager.AccessTokenResponseBuilder responseBuilder, boolean useRefreshToken, boolean isDPoPSupported) { if (!isDPoPSupported) return; - if (clientConfig.isUseDPoP()) { + if (clientConfig.isUseDPoP() || dPoP != null) { DPoPUtil.bindToken(responseBuilder.getAccessToken(), dPoP); // TODO Probably uncomment as the accessToken type "DPoP" will have more sense than "Bearer". It will require some changes in the introspection endpoint too... // responseBuilder.getAccessToken().type(DPoPUtil.DPOP_TOKEN_TYPE); diff --git a/services/src/main/java/org/keycloak/services/util/DPoPUtil.java b/services/src/main/java/org/keycloak/services/util/DPoPUtil.java index 4f09463fc3d..7c27a37c035 100644 --- a/services/src/main/java/org/keycloak/services/util/DPoPUtil.java +++ b/services/src/main/java/org/keycloak/services/util/DPoPUtil.java @@ -69,7 +69,7 @@ public class DPoPUtil { DISABLED } - private static final String DPOP_HTTP_HEADER = "DPoP"; + public static final String DPOP_HTTP_HEADER = "DPoP"; private static final String DPOP_JWT_HEADER_TYPE = "dpop+jwt"; private static final String DPOP_ATH_ALG = "RS256"; diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/DPoPTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/DPoPTest.java index 653fdbe4169..1b7c1f335f6 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/DPoPTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/DPoPTest.java @@ -226,37 +226,37 @@ public class DPoPTest extends AbstractTestRealmKeycloakTest { public void testDPoPDisabledByPublicClient() throws Exception { changeDPoPBound(TEST_PUBLIC_CLIENT_ID, false); + try { + // with DPoP proof + testDPoPByPublicClient(); - oauth.clientId(TEST_PUBLIC_CLIENT_ID); - oauth.doLogin(TEST_USER_NAME, TEST_USER_PASSWORD); + // without DPoP proof + oauth.clientId(TEST_PUBLIC_CLIENT_ID); + oauth.dpopProof(null); + oauth.doLogin(TEST_USER_NAME, TEST_USER_PASSWORD); - String dpopProofEcEncoded = generateSignedDPoPProof(UUID.randomUUID().toString(), HttpMethod.POST.toString(), oauth.getAccessTokenUrl(), Long.valueOf(Time.currentTime()), Algorithm.ES256, jwsEcHeader, ecKeyPair.getPrivate()); + String code = oauth.getCurrentQuery().get(OAuth2Constants.CODE); + OAuthClient.AccessTokenResponse response = oauth.doAccessTokenRequest(code, null); - String code = oauth.getCurrentQuery().get(OAuth2Constants.CODE); - oauth.dpopProof(dpopProofEcEncoded); - OAuthClient.AccessTokenResponse response = oauth.doAccessTokenRequest(code, null); + assertEquals(Status.OK.getStatusCode(), response.getStatusCode()); + AccessToken accessToken = oauth.verifyToken(response.getAccessToken()); + assertEquals(null, accessToken.getConfirmation()); + RefreshToken refreshToken = oauth.parseRefreshToken(response.getRefreshToken()); + assertEquals(null, refreshToken.getConfirmation()); - assertEquals(Status.OK.getStatusCode(), response.getStatusCode()); - AccessToken accessToken = oauth.verifyToken(response.getAccessToken()); - assertEquals(null, accessToken.getConfirmation()); - RefreshToken refreshToken = oauth.parseRefreshToken(response.getRefreshToken()); - assertEquals(null, refreshToken.getConfirmation()); + // token refresh + response = oauth.doRefreshTokenRequest(response.getRefreshToken(), null); - // token refresh - dpopProofEcEncoded = generateSignedDPoPProof(UUID.randomUUID().toString(), HttpMethod.POST.toString(), oauth.getAccessTokenUrl(), Long.valueOf(Time.currentTime()), Algorithm.ES256, jwsEcHeader, ecKeyPair.getPrivate()); + assertEquals(Status.OK.getStatusCode(), response.getStatusCode()); + accessToken = oauth.verifyToken(response.getAccessToken()); + assertEquals(null, accessToken.getConfirmation()); + refreshToken = oauth.parseRefreshToken(response.getRefreshToken()); + assertEquals(null, refreshToken.getConfirmation()); - oauth.dpopProof(dpopProofEcEncoded); - response = oauth.doRefreshTokenRequest(response.getRefreshToken(), null); - - assertEquals(Status.OK.getStatusCode(), response.getStatusCode()); - accessToken = oauth.verifyToken(response.getAccessToken()); - assertEquals(null, accessToken.getConfirmation()); - refreshToken = oauth.parseRefreshToken(response.getRefreshToken()); - assertEquals(null, refreshToken.getConfirmation()); - - oauth.idTokenHint(response.getIdToken()).openLogout(); - - changeDPoPBound(TEST_PUBLIC_CLIENT_ID, true); + oauth.idTokenHint(response.getIdToken()).openLogout(); + } finally { + changeDPoPBound(TEST_PUBLIC_CLIENT_ID, true); + } } @Test