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

Co-authored-by: Pedro Igor <pigor.craveiro@gmail.com>
This commit is contained in:
Stian Thorgersen 2023-04-14 14:42:12 +02:00 committed by GitHub
parent 37e46f3551
commit f4cabea08c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 51 additions and 5 deletions

View file

@ -241,7 +241,8 @@ public class OIDCLoginProtocol implements LoginProtocol {
authSession.getClientNote(OAuth2Constants.SCOPE), authSession.getClientNote(OAuth2Constants.SCOPE),
authSession.getClientNote(OIDCLoginProtocol.REDIRECT_URI_PARAM), authSession.getClientNote(OIDCLoginProtocol.REDIRECT_URI_PARAM),
authSession.getClientNote(OIDCLoginProtocol.CODE_CHALLENGE_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); code = OAuth2CodeParser.persistCode(session, clientSession, codeData);
redirectUri.addParam(OAuth2Constants.CODE, code); redirectUri.addParam(OAuth2Constants.CODE, code);

View file

@ -38,6 +38,7 @@ public class OAuth2Code {
private static final String REDIRECT_URI_PARAM_NOTE = "redirectUri"; private static final String REDIRECT_URI_PARAM_NOTE = "redirectUri";
private static final String CODE_CHALLENGE_NOTE = "code_challenge"; private static final String CODE_CHALLENGE_NOTE = "code_challenge";
private static final String CODE_CHALLENGE_METHOD_NOTE = "code_challenge_method"; 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; private final String id;
@ -52,10 +53,10 @@ public class OAuth2Code {
private final String codeChallenge; private final String codeChallenge;
private final String codeChallengeMethod; private final String codeChallengeMethod;
private final String userSessionId;
public OAuth2Code(String id, int expiration, String nonce, String scope, String redirectUriParam, 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.id = id;
this.expiration = expiration; this.expiration = expiration;
this.nonce = nonce; this.nonce = nonce;
@ -63,6 +64,7 @@ public class OAuth2Code {
this.redirectUriParam = redirectUriParam; this.redirectUriParam = redirectUriParam;
this.codeChallenge = codeChallenge; this.codeChallenge = codeChallenge;
this.codeChallengeMethod = codeChallengeMethod; this.codeChallengeMethod = codeChallengeMethod;
this.userSessionId = userSessionId;
} }
@ -74,6 +76,7 @@ public class OAuth2Code {
redirectUriParam = data.get(REDIRECT_URI_PARAM_NOTE); redirectUriParam = data.get(REDIRECT_URI_PARAM_NOTE);
codeChallenge = data.get(CODE_CHALLENGE_NOTE); codeChallenge = data.get(CODE_CHALLENGE_NOTE);
codeChallengeMethod = data.get(CODE_CHALLENGE_METHOD_NOTE); codeChallengeMethod = data.get(CODE_CHALLENGE_METHOD_NOTE);
userSessionId = data.get(USER_SESSION_ID_NOTE);
} }
@ -92,6 +95,7 @@ public class OAuth2Code {
result.put(REDIRECT_URI_PARAM_NOTE, redirectUriParam); result.put(REDIRECT_URI_PARAM_NOTE, redirectUriParam);
result.put(CODE_CHALLENGE_NOTE, codeChallenge); result.put(CODE_CHALLENGE_NOTE, codeChallenge);
result.put(CODE_CHALLENGE_METHOD_NOTE, codeChallengeMethod); result.put(CODE_CHALLENGE_METHOD_NOTE, codeChallengeMethod);
result.put(USER_SESSION_ID_NOTE, userSessionId);
return result; return result;
} }
@ -124,4 +128,8 @@ public class OAuth2Code {
public String getCodeChallengeMethod() { public String getCodeChallengeMethod() {
return codeChallengeMethod; return codeChallengeMethod;
} }
public String getUserSessionId() {
return userSessionId;
}
} }

View file

@ -121,16 +121,23 @@ public class OAuth2CodeParser {
return result.illegalCode(); return result.illegalCode();
} }
logger.tracef("Successfully verified code '%s'. User session: '%s', client: '%s'", codeUUID, userSessionId, clientUUID);
result.codeData = OAuth2Code.deserializeCode(codeData); 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 // Finally doublecheck if code is not expired
int currentTime = Time.currentTime(); int currentTime = Time.currentTime();
if (currentTime > result.codeData.getExpiration()) { if (currentTime > result.codeData.getExpiration()) {
return result.expiredCode(); return result.expiredCode();
} }
logger.tracef("Successfully verified code '%s'. User session: '%s', client: '%s'", codeUUID, userSessionId, clientUUID);
return result; return result;
} }

View file

@ -16,6 +16,7 @@
*/ */
package org.keycloak.testsuite.forms; package org.keycloak.testsuite.forms;
import org.apache.http.impl.client.CloseableHttpClient;
import org.jboss.arquillian.drone.api.annotation.Drone; import org.jboss.arquillian.drone.api.annotation.Drone;
import org.jboss.arquillian.graphene.page.Page; import org.jboss.arquillian.graphene.page.Page;
import org.junit.Assert; import org.junit.Assert;
@ -39,6 +40,7 @@ import org.keycloak.testsuite.pages.AppPage;
import org.keycloak.testsuite.pages.AppPage.RequestType; import org.keycloak.testsuite.pages.AppPage.RequestType;
import org.keycloak.testsuite.pages.LoginPage; import org.keycloak.testsuite.pages.LoginPage;
import org.keycloak.testsuite.pages.LoginPasswordUpdatePage; import org.keycloak.testsuite.pages.LoginPasswordUpdatePage;
import org.keycloak.testsuite.util.MutualTLSUtils;
import org.keycloak.testsuite.util.OAuthClient; import org.keycloak.testsuite.util.OAuthClient;
import org.openqa.selenium.WebDriver; import org.openqa.selenium.WebDriver;
@ -46,6 +48,9 @@ import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertTrue; 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 <a href="mailto:sthorger@redhat.com">Stian Thorgersen</a>
* @author Stan Silvert ssilvert@redhat.com (C) 2016 Red Hat Inc. * @author Stan Silvert ssilvert@redhat.com (C) 2016 Red Hat Inc.
@ -209,4 +214,29 @@ public class SSOTest extends AbstractTestRealmKeycloakTest {
} }
@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());
}
} }