diff --git a/server-spi/src/main/java/org/keycloak/models/DefaultActionTokenKey.java b/server-spi/src/main/java/org/keycloak/models/DefaultActionTokenKey.java index 11edf1e826a..5a0797e0af4 100644 --- a/server-spi/src/main/java/org/keycloak/models/DefaultActionTokenKey.java +++ b/server-spi/src/main/java/org/keycloak/models/DefaultActionTokenKey.java @@ -21,6 +21,7 @@ import java.util.Base64; import java.util.UUID; import java.util.regex.Pattern; +import org.keycloak.common.util.SecretGenerator; import org.keycloak.representations.JsonWebToken; import com.fasterxml.jackson.annotation.JsonIgnore; @@ -47,7 +48,7 @@ public class DefaultActionTokenKey extends JsonWebToken implements SingleUseObje this.subject = userId; this.type = actionId; this.exp = Long.valueOf(absoluteExpirationInSecs); - this.actionVerificationNonce = actionVerificationNonce == null ? UUID.randomUUID() : actionVerificationNonce; + this.actionVerificationNonce = actionVerificationNonce == null ? SecretGenerator.getInstance().generateSecureUUID() : actionVerificationNonce; } @JsonIgnore diff --git a/services/src/main/java/org/keycloak/protocol/oidc/par/endpoints/ParEndpoint.java b/services/src/main/java/org/keycloak/protocol/oidc/par/endpoints/ParEndpoint.java index 99427f62e43..22fcdb8a915 100644 --- a/services/src/main/java/org/keycloak/protocol/oidc/par/endpoints/ParEndpoint.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/par/endpoints/ParEndpoint.java @@ -20,7 +20,6 @@ package org.keycloak.protocol.oidc.par.endpoints; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.UUID; import jakarta.ws.rs.Consumes; import jakarta.ws.rs.POST; @@ -33,6 +32,7 @@ import jakarta.ws.rs.core.UriBuilder; import org.keycloak.OAuthErrorException; import org.keycloak.common.Profile; +import org.keycloak.common.util.SecretGenerator; import org.keycloak.events.Details; import org.keycloak.events.EventBuilder; import org.keycloak.events.EventType; @@ -62,8 +62,9 @@ public class ParEndpoint extends AbstractParEndpoint { public static final String PAR_CREATED_TIME = "par.created.time"; public static final String PAR_DPOP_PROOF_JKT = "par.dpop.proof.jkt"; - private static final String REQUEST_URI_PREFIX = "urn:ietf:params:oauth:request_uri:"; + public static final String REQUEST_URI_PREFIX = "urn:ietf:params:oauth:request_uri:"; public static final int REQUEST_URI_PREFIX_LENGTH = REQUEST_URI_PREFIX.length(); + public static final String CACHE_KEY_PREFIX = "par:"; private final HttpRequest httpRequest; @@ -165,7 +166,7 @@ public class ParEndpoint extends AbstractParEndpoint { Map params = new HashMap<>(); - String key = UUID.randomUUID().toString(); + String key = SecretGenerator.getInstance().generateSecureID(); String requestUri = REQUEST_URI_PREFIX + key; int expiresIn = realm.getParPolicy().getRequestUriLifespan(); @@ -180,7 +181,7 @@ public class ParEndpoint extends AbstractParEndpoint { } SingleUseObjectProvider singleUseStore = session.singleUseObjects(); - singleUseStore.put(key, expiresIn, params); + singleUseStore.put(CACHE_KEY_PREFIX + key, expiresIn, params); ParResponse parResponse = new ParResponse(requestUri, expiresIn); diff --git a/services/src/main/java/org/keycloak/protocol/oidc/par/endpoints/request/AuthzEndpointParParser.java b/services/src/main/java/org/keycloak/protocol/oidc/par/endpoints/request/AuthzEndpointParParser.java index 20c78174e7a..f024d5f541b 100644 --- a/services/src/main/java/org/keycloak/protocol/oidc/par/endpoints/request/AuthzEndpointParParser.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/par/endpoints/request/AuthzEndpointParParser.java @@ -32,6 +32,7 @@ import org.keycloak.protocol.oidc.par.endpoints.ParEndpoint; import org.jboss.logging.Logger; +import static org.keycloak.protocol.oidc.par.endpoints.ParEndpoint.CACHE_KEY_PREFIX; import static org.keycloak.protocol.oidc.par.endpoints.ParEndpoint.PAR_CREATED_TIME; import static org.keycloak.protocol.oidc.par.endpoints.ParEndpoint.PAR_DPOP_PROOF_JKT; @@ -60,7 +61,7 @@ public class AuthzEndpointParParser extends AuthzEndpointRequestParser { logger.warnf(re,"Unable to parse request_uri: %s", requestUri); throw new RuntimeException("Unable to parse request_uri"); } - Map retrievedRequest = singleUseStore.remove(key); + Map retrievedRequest = singleUseStore.remove(CACHE_KEY_PREFIX + key); if (retrievedRequest == null) { throw new RuntimeException("PAR not found. not issued or used multiple times."); } diff --git a/services/src/main/java/org/keycloak/protocol/oidc/utils/OAuth2Code.java b/services/src/main/java/org/keycloak/protocol/oidc/utils/OAuth2Code.java index ca2c2482741..8fe06bfbfb9 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/utils/OAuth2Code.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/utils/OAuth2Code.java @@ -31,8 +31,8 @@ import java.util.Map; */ public class OAuth2Code { - private static final String ID_NOTE = "id"; - private static final String EXPIRATION_NOTE = "exp"; + public static final String ID_NOTE = "id"; + public static final String EXPIRATION_NOTE = "exp"; private static final String NONCE_NOTE = "nonce"; private static final String SCOPE_NOTE = "scope"; private static final String RESOURCE_NOTE = "resource"; @@ -40,7 +40,7 @@ public class OAuth2Code { private static final String CODE_CHALLENGE_NOTE = "code_challenge"; private static final String CODE_CHALLENGE_METHOD_NOTE = "code_challenge_method"; private static final String DPOP_JKT_NOTE = "dpop_jkt"; - private static final String USER_SESSION_ID_NOTE = "user_session_id"; + public static final String USER_SESSION_ID_NOTE = "user_session_id"; private final String id; diff --git a/services/src/main/java/org/keycloak/protocol/oidc/utils/OAuth2CodeParser.java b/services/src/main/java/org/keycloak/protocol/oidc/utils/OAuth2CodeParser.java index 6c0383beae1..fc89b36a59a 100644 --- a/services/src/main/java/org/keycloak/protocol/oidc/utils/OAuth2CodeParser.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/utils/OAuth2CodeParser.java @@ -39,6 +39,7 @@ public class OAuth2CodeParser { private static final Logger logger = Logger.getLogger(OAuth2CodeParser.class); private static final Pattern DOT = Pattern.compile("\\."); + private static final String CACHE_KEY_PREFIX = "code:"; /** * Will persist the code to the cache and return the object with the codeData and code correctly set @@ -54,7 +55,7 @@ public class OAuth2CodeParser { } Map serialized = codeData.serializeCode(); - codeStore.put(key, clientSession.getUserSession().getRealm().getAccessCodeLifespan(), serialized); + codeStore.put(CACHE_KEY_PREFIX + key, clientSession.getUserSession().getRealm().getAccessCodeLifespan(), serialized); return key + "." + clientSession.getUserSession().getId() + "." + clientSession.getClient().getId(); } @@ -94,7 +95,7 @@ public class OAuth2CodeParser { result.clientSession = userSession.getAuthenticatedClientSessionByClient(clientUUID); SingleUseObjectProvider codeStore = session.singleUseObjects(); - Map codeData = codeStore.remove(codeUUID); + Map codeData = codeStore.remove(CACHE_KEY_PREFIX + codeUUID); // Either code not available or was already used if (codeData == null) { diff --git a/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureParContentsExecutor.java b/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureParContentsExecutor.java index e3c3328fd8c..9e376f3202c 100644 --- a/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureParContentsExecutor.java +++ b/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureParContentsExecutor.java @@ -85,7 +85,7 @@ public class SecureParContentsExecutor implements ClientPolicyExecutorProvider requestParametersFromPAR = singleUseStore.get(key); + Map requestParametersFromPAR = singleUseStore.get(ParEndpoint.CACHE_KEY_PREFIX + key); if (requestParametersFromPAR == null) { throw new ClientPolicyException(OAuthErrorException.INVALID_REQUEST, "PAR not found. not issued or used multiple times."); } @@ -100,7 +100,7 @@ public class SecureParContentsExecutor implements ClientPolicyExecutorProvider params = UriUtils.decodeQueryString(queryString); + String key = params.getFirst(Constants.KEY); + TokenVerifier tokenVerifier = TokenVerifier.create(key, DefaultActionTokenKey.class); + DefaultActionTokenKey aToken = tokenVerifier.getToken(); + String serializedKey1 = aToken.serializeKey(); + String serializedKey2 = serializedKey1 + SingleUseObjectProvider.REVOKED_KEY; + + // Try to send PAR request with manually created requestUri related to the manually created serialized keys + String state = "testSuccessfulSinglePar"; + oauth.loginForm() + .requestUri(REQUEST_URI_PREFIX + serializedKey1) + .state(state) + .open(); + oauth.loginForm() + .requestUri(REQUEST_URI_PREFIX + serializedKey2) + .state(state) + .open(); + events.clear(); + + assertSecondPasswordResetFails(changePasswordUrl, oauth.getClientId()); // KC_RESTART doesn't exist, it was deleted after first successful reset-password flow was finished + } + @Test public void resetPasswordForceLogin() throws IOException { // add the force login option in the reset-credential-email authenticator diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/par/ParTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/par/ParTest.java index 79b45753554..06b23fe9653 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/par/ParTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/par/ParTest.java @@ -38,6 +38,7 @@ import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.protocol.oidc.OIDCAdvancedConfigWrapper; import org.keycloak.protocol.oidc.OIDCConfigAttributes; import org.keycloak.protocol.oidc.OIDCLoginProtocol; +import org.keycloak.protocol.oidc.utils.OAuth2Code; import org.keycloak.representations.AccessToken; import org.keycloak.representations.IDToken; import org.keycloak.representations.RefreshToken; @@ -54,6 +55,7 @@ import org.keycloak.testsuite.client.resources.TestApplicationResourceUrls; import org.keycloak.testsuite.client.resources.TestOIDCEndpointsApplicationResource; import org.keycloak.testsuite.rest.resource.TestingOIDCEndpointsApplicationResource; import org.keycloak.testsuite.services.clientpolicy.executor.TestRaiseExceptionExecutorFactory; +import org.keycloak.testsuite.util.AccountHelper; import org.keycloak.testsuite.util.ClientBuilder; import org.keycloak.testsuite.util.ClientPoliciesUtil.ClientPoliciesBuilder; import org.keycloak.testsuite.util.ClientPoliciesUtil.ClientPolicyBuilder; @@ -72,6 +74,7 @@ import org.keycloak.util.JsonSerialization; import org.junit.Assert; import org.junit.Test; +import static org.keycloak.OAuthErrorException.INVALID_GRANT; import static org.keycloak.testsuite.AbstractAdminTest.loadJson; import static org.keycloak.testsuite.admin.ApiUtil.findUserByUsername; import static org.keycloak.testsuite.util.ClientPoliciesUtil.createClientRolesConditionConfig; @@ -82,6 +85,7 @@ import static org.hamcrest.Matchers.startsWith; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; @@ -300,6 +304,52 @@ public class ParTest extends AbstractClientPoliciesTest { } } + + // Test manually created PAR request cannot be used to obtain OAuth2 code + @Test + public void testParDoNotClashWithAuthorizationCode() throws Exception { + try { + // setup PAR realm settings + int requestUriLifespan = 45; + setParRealmSettings(requestUriLifespan); + + // Step 1 - Login as regular user and obtain sessionId + oauth.doLogin(TEST_USER2_NAME, TEST_USER2_PASSWORD); + AuthorizationEndpointResponse authzResponse = oauth.parseLoginResponse(); + String sessionId = authzResponse.getSessionState(); + String code = authzResponse.getCode(); + assertNotNull(sessionId); + assertNotNull(code); + + // Step 2: PAR request with some custom injected parameters + ParRequest pReq = new ParRequest(oauth) { + + @Override + protected void initRequest() { + super.initRequest(); + parameter(OAuth2Code.ID_NOTE, "some-id"); + parameter(OAuth2Code.USER_SESSION_ID_NOTE, sessionId); + parameter(OAuth2Code.EXPIRATION_NOTE, String.valueOf(Time.currentTime() + 9999)); + } + }; + ParResponse pResp = pReq.send(); + assertEquals(201, pResp.getStatusCode()); + String requestUri = pResp.getRequestUri(); + + // Step 3: Attempt to exchange code for token with the "fake code" from PAR + String parId = requestUri.substring(requestUri.lastIndexOf(":" ) + 1); + String clientUUID = ApiUtil.findClientByClientId(adminClient.realm("test"), oauth.getClientId()).toRepresentation().getId(); + String fakeCode = parId + "." + sessionId + "." + clientUUID; + AccessTokenResponse response = oauth.doAccessTokenRequest(fakeCode); + assertEquals(400, response.getStatusCode()); + assertEquals(INVALID_GRANT, response.getError()); + } finally { + // Logout + AccountHelper.logout(adminClient.realm(oauth.getRealm()), TEST_USER2_NAME); + restoreParRealmSettings(); + } + } + @Test public void testWrongSigningAlgorithmForRequestObject() throws Exception { try {