Make sure the code is bound to the user session (#18) (#17380)

Co-authored-by: Pedro Igor <pigor.craveiro@gmail.com>
keycloak · Mar 1, 2023 · ec81091 · ec81091
1 parent 008b73d
commit ec81091
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 5 deletions.
diff --git a/services/src/main/java/org/keycloak/protocol/oidc/OIDCLoginProtocol.java b/services/src/main/java/org/keycloak/protocol/oidc/OIDCLoginProtocol.java
@@ -241,7 +241,8 @@ public Response authenticated(AuthenticationSessionModel authSession, UserSessio
                     authSession.getClientNote(OAuth2Constants.SCOPE),
                     authSession.getClientNote(OIDCLoginProtocol.REDIRECT_URI_PARAM),
                     authSession.getClientNote(OIDCLoginProtocol.CODE_CHALLENGE_PARAM),
-                    authSession.getClientNote(OIDCLoginProtocol.CODE_CHALLENGE_METHOD_PARAM));
+                    authSession.getClientNote(OIDCLoginProtocol.CODE_CHALLENGE_METHOD_PARAM),
+                    userSession.getId());
 
             code = OAuth2CodeParser.persistCode(session, clientSession, codeData);
             redirectUri.addParam(OAuth2Constants.CODE, code);

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
@@ -38,6 +38,7 @@ public class OAuth2Code {
     private static final String REDIRECT_URI_PARAM_NOTE = "redirectUri";
     private static final String CODE_CHALLENGE_NOTE = "code_challenge";
     private static final String CODE_CHALLENGE_METHOD_NOTE = "code_challenge_method";
+    private static final String USER_SESSION_ID_NOTE = "user_session_id";
 
     private final String id;
 
@@ -52,17 +53,18 @@ public class OAuth2Code {
     private final String codeChallenge;
 
     private final String codeChallengeMethod;
-
+    private final String userSessionId;
 
     public OAuth2Code(String id, int expiration, String nonce, String scope, String redirectUriParam,
-                      String codeChallenge, String codeChallengeMethod) {
+                      String codeChallenge, String codeChallengeMethod, String userSessionId) {
         this.id = id;
         this.expiration = expiration;
         this.nonce = nonce;
         this.scope = scope;
         this.redirectUriParam = redirectUriParam;
         this.codeChallenge = codeChallenge;
         this.codeChallengeMethod = codeChallengeMethod;
+        this.userSessionId = userSessionId;
     }
 
 
@@ -74,6 +76,7 @@ private OAuth2Code(Map<String, String> data) {
         redirectUriParam = data.get(REDIRECT_URI_PARAM_NOTE);
         codeChallenge = data.get(CODE_CHALLENGE_NOTE);
         codeChallengeMethod = data.get(CODE_CHALLENGE_METHOD_NOTE);
+        userSessionId = data.get(USER_SESSION_ID_NOTE);
     }
 
 
@@ -92,6 +95,7 @@ public Map<String, String> serializeCode() {
         result.put(REDIRECT_URI_PARAM_NOTE, redirectUriParam);
         result.put(CODE_CHALLENGE_NOTE, codeChallenge);
         result.put(CODE_CHALLENGE_METHOD_NOTE, codeChallengeMethod);
+        result.put(USER_SESSION_ID_NOTE, userSessionId);
 
         return result;
     }
@@ -124,4 +128,8 @@ public String getCodeChallenge() {
     public String getCodeChallengeMethod() {
         return codeChallengeMethod;
     }
+
+    public String getUserSessionId() {
+        return userSessionId;
+    }
 }
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
@@ -121,16 +121,23 @@ public static ParseResult parseCode(KeycloakSession session, String code, RealmM
             return result.illegalCode();
         }
 
-        logger.tracef("Successfully verified code '%s'. User session: '%s', client: '%s'", codeUUID, userSessionId, clientUUID);
-
         result.codeData = OAuth2Code.deserializeCode(codeData);
 
+        String persistedUserSessionId = result.codeData.getUserSessionId();
+
+        if (!userSessionId.equals(persistedUserSessionId)) {
+            logger.warnf("Code '%s' is bound to a different session", codeUUID);
+            return result.illegalCode();
+        }
+
         // Finally doublecheck if code is not expired
         int currentTime = Time.currentTime();
         if (currentTime > result.codeData.getExpiration()) {
             return result.expiredCode();
         }
 
+        logger.tracef("Successfully verified code '%s'. User session: '%s', client: '%s'", codeUUID, userSessionId, clientUUID);
+
         return result;
     }
 

diff --git a/...integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/SSOTest.java b/...integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/SSOTest.java
@@ -16,6 +16,7 @@
  */
 package org.keycloak.testsuite.forms;
 
+import org.apache.http.impl.client.CloseableHttpClient;
 import org.jboss.arquillian.drone.api.annotation.Drone;
 import org.jboss.arquillian.graphene.page.Page;
 import org.junit.Assert;
@@ -39,13 +40,17 @@
 import org.keycloak.testsuite.pages.AppPage.RequestType;
 import org.keycloak.testsuite.pages.LoginPage;
 import org.keycloak.testsuite.pages.LoginPasswordUpdatePage;
+import org.keycloak.testsuite.util.MutualTLSUtils;
 import org.keycloak.testsuite.util.OAuthClient;
 import org.openqa.selenium.WebDriver;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertTrue;
 
+import java.io.IOException;
+import javax.ws.rs.core.Response;
+
 /**
  * @author <a href="mailto:sthorger@redhat.com">Stian Thorgersen</a>
  * @author Stan Silvert ssilvert@redhat.com (C) 2016 Red Hat Inc.
@@ -209,4 +214,29 @@ public void loginWithRequiredActionAddedInTheMeantime() {
 
     }
 
+    @Test
+    public void failIfUsingCodeFromADifferentSession() throws IOException {
+        // first client user login
+        oauth.openLoginForm();
+        oauth.doLogin("test-user@localhost", "password");
+        String firstCode = oauth.getCurrentQuery().get(OAuth2Constants.CODE);
+
+        // second client user login
+        OAuthClient oauth2 = new OAuthClient();
+        oauth2.init(driver2);
+        oauth2.doLogin("john-doh@localhost", "password");
+        String secondCode = oauth2.getCurrentQuery().get(OAuth2Constants.CODE);
+        String[] firstCodeParts = firstCode.split("\\.");
+        String[] secondCodeParts = secondCode.split("\\.");
+        secondCodeParts[1] = firstCodeParts[1];
+        secondCode = String.join(".", secondCodeParts);
+
+        OAuthClient.AccessTokenResponse tokenResponse;
+
+        try (CloseableHttpClient client = MutualTLSUtils.newCloseableHttpClientWithOtherKeyStoreAndTrustStore()) {
+            tokenResponse = oauth2.doAccessTokenRequest(secondCode, "password", client);
+        }
+
+        assertEquals(Response.Status.BAD_REQUEST.getStatusCode(), tokenResponse.getStatusCode());
+    }
 }