libre.sh/keycloak-scim
3
0
Fork 0
Code Issues 15 Pull requests Releases Packages Activity Actions

RelayState max length not respected

Browse source 
Fixes: #10227
This commit is contained in:
evtr 2022-09-06 22:01:14 +02:00 committed by GitHub
parent f57560afd3
commit 4469bdc0a9
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 1920 additions and 9 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

52
server-spi-private/src/main/java/org/keycloak/broker/provider/util/IdentityBrokerState.java
View file

@ -17,6 +17,14 @@
package org.keycloak.broker.provider.util;
import org.keycloak.authorization.policy.evaluation.Realm;
import org.keycloak.models.ClientModel;
import org.keycloak.models.RealmModel;
import java.nio.BufferUnderflowException;
import java.nio.ByteBuffer;
import java.util.Base64;
import java.util.UUID;
import java.util.regex.Pattern;
/**
@ -30,20 +38,56 @@ public class IdentityBrokerState {
private static final Pattern DOT = Pattern.compile("\\.");
public static IdentityBrokerState decoded(String state, String clientId, String tabId) {
String encodedState = state + "." + tabId + "." + clientId;
public static IdentityBrokerState decoded(String state, String clientId, String clientClientId, String tabId) {
return new IdentityBrokerState(state, clientId, tabId, encodedState);
String clientIdEncoded = clientClientId; // Default use the client.clientId
if (clientId != null) {
// According to (http://docs.oasis-open.org/security/saml/v2.0/saml-bindings-2.0-os.pdf) there is a limit on the relaystate of 80 bytes.
// in order to try to adher to the SAML specification we use an encoded value of the client.id (probably UUID) instead of the with
// probability bigger client.clientId. If the client.id is not in UUID format we just use the client.clientid as is
try {
UUID clientDbUuid = UUID.fromString(clientId);
ByteBuffer bb = ByteBuffer.wrap(new byte[16]);
bb.putLong(clientDbUuid.getMostSignificantBits());
bb.putLong(clientDbUuid.getLeastSignificantBits());
byte[] clientUuidBytes = bb.array();
clientIdEncoded = Base64.getEncoder().encodeToString(clientUuidBytes).replace("=", "");
} catch (IllegalArgumentException e) {
// Ignore...the clientid in the database was not in UUID format. Just use as is.
}
}
String encodedState = state + "." + tabId + "." + clientIdEncoded;
return new IdentityBrokerState(state, clientClientId, tabId, encodedState);
}
public static IdentityBrokerState encoded(String encodedState) {
public static IdentityBrokerState encoded(String encodedState, RealmModel realmModel) {
String[] decoded = DOT.split(encodedState, 3);
String state =(decoded.length > 0) ? decoded[0] : null;
String tabId = (decoded.length > 1) ? decoded[1] : null;
String clientId = (decoded.length > 2) ? decoded[2] : null;
if (clientId != null) {
try {
// If this decoding succeeds it was the result of the encoding of a UUID client.id - if it fails we interpret it as client.clientId
// in accordance to the method decoded above
byte[] decodedClientId = Base64.getDecoder().decode(clientId);
ByteBuffer bb = ByteBuffer.wrap(decodedClientId);
long first = bb.getLong();
long second = bb.getLong();
UUID clientDbUuid = new UUID(first, second);
String clientIdInDb = clientDbUuid.toString();
ClientModel clientModel = realmModel.getClientById(clientIdInDb);
if (clientModel != null) {
clientId = clientModel.getClientId();
}
} catch (IllegalArgumentException | BufferUnderflowException e) {
// Ignore...the clientid was not in encoded uuid format. Just use as it is.
}
}
return new IdentityBrokerState(state, clientId, tabId, encodedState);
}

81
server-spi-private/src/test/java/org/keycloak/broker/provider/util/IdentityBrokerStateTest.java Normal file
View file

@ -0,0 +1,81 @@
package org.keycloak.broker.provider.util;
import org.junit.Assert;
import org.junit.Test;
import org.keycloak.models.*;
public class IdentityBrokerStateTest {
@Test
public void testDecodedWithClientIdNotUuid() {
// Given
String state = "gNrGamIDGKpKSI9yOrcFzYTKoFGH779_WNCacAelkhk";
String clientId = "something not a uuid";
String clientClientId = "http://i.am.an.url";
String tabId = "vpISZLVDAc0";
// When
IdentityBrokerState encodedState = IdentityBrokerState.decoded(state, clientId, clientClientId, tabId);
// Then
Assert.assertNotNull(encodedState);
Assert.assertEquals(clientClientId, encodedState.getClientId());
Assert.assertEquals(tabId, encodedState.getTabId());
Assert.assertEquals("gNrGamIDGKpKSI9yOrcFzYTKoFGH779_WNCacAelkhk.vpISZLVDAc0.http://i.am.an.url", encodedState.getEncoded());
}
@Test
public void testDecodedWithClientIdAnActualUuid() {
// Given
String state = "gNrGamIDGKpKSI9yOrcFzYTKoFGH779_WNCacAelkhk";
String clientId = "ed49448c-14cf-471e-a83a-063d0dc3bc8c";
String clientClientId = "http://i.am.an.url";
String tabId = "vpISZLVDAc0";
// When
IdentityBrokerState encodedState = IdentityBrokerState.decoded(state, clientId, clientClientId, tabId);
// Then
Assert.assertNotNull(encodedState);
Assert.assertEquals(clientClientId, encodedState.getClientId());
Assert.assertEquals(tabId, encodedState.getTabId());
Assert.assertEquals("gNrGamIDGKpKSI9yOrcFzYTKoFGH779_WNCacAelkhk.vpISZLVDAc0.7UlEjBTPRx6oOgY9DcO8jA", encodedState.getEncoded());
}
@Test
public void testEncodedWithClientIdUUid() {
// Given
String encoded = "gNrGamIDGKpKSI9yOrcFzYTKoFGH779_WNCacAelkhk.vpISZLVDAc0.7UlEjBTPRx6oOgY9DcO8jA";
String clientId = "ed49448c-14cf-471e-a83a-063d0dc3bc8c";
String clientClientId = "my-client-id";
ClientModel clientModel = new IdentityBrokerStateTestHelpers.TestClientModel(clientId, clientClientId);
RealmModel realmModel = new IdentityBrokerStateTestHelpers.TestRealmModel(clientId, clientClientId, clientModel);
// When
IdentityBrokerState decodedState = IdentityBrokerState.encoded(encoded, realmModel);
// Then
Assert.assertNotNull(decodedState);
Assert.assertEquals(clientClientId, decodedState.getClientId());
}
@Test
public void testEncodedWithClientIdNotUUid() {
// Given
String encoded = "gNrGamIDGKpKSI9yOrcFzYTKoFGH779_WNCacAelkhk.vpISZLVDAc0.http://i.am.an.url";
String clientId = "http://i.am.an.url";
ClientModel clientModel = new IdentityBrokerStateTestHelpers.TestClientModel(clientId, clientId);
RealmModel realmModel = new IdentityBrokerStateTestHelpers.TestRealmModel(clientId, clientId, clientModel);
// When
IdentityBrokerState decodedState = IdentityBrokerState.encoded(encoded, realmModel);
// Then
Assert.assertNotNull(decodedState);
Assert.assertEquals("http://i.am.an.url", decodedState.getClientId());
}
}

1787
server-spi-private/src/test/java/org/keycloak/broker/provider/util/IdentityBrokerStateTestHelpers.java Normal file
View file

File diff suppressed because it is too large Load diff

3
services/src/main/java/org/keycloak/broker/oidc/AbstractOAuth2IdentityProvider.java
View file

@ -550,8 +550,7 @@ public abstract class AbstractOAuth2IdentityProvider<C extends OAuth2IdentityPro
}
RealmModel realm = context.getRealm();
IdentityBrokerState idpBrokerState = IdentityBrokerState.encoded(stateParam);
IdentityBrokerState idpBrokerState = IdentityBrokerState.encoded(stateParam, realm);
ClientModel client = realm.getClientByClientId(idpBrokerState.getClientId());
AuthenticationSessionModel authSession = ClientSessionCode.getClientSession(

4
services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java
View file

@ -1040,7 +1040,7 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal
@Override
public AuthenticationSessionModel getAndVerifyAuthenticationSession(String encodedCode) {
IdentityBrokerState state = IdentityBrokerState.encoded(encodedCode);
IdentityBrokerState state = IdentityBrokerState.encoded(encodedCode, realmModel);
String code = state.getDecodedState();
String clientId = state.getClientId();
String tabId = state.getTabId();
@ -1131,7 +1131,7 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal
if (clientSessionCode != null) {
authSession = clientSessionCode.getClientSession();
String relayState = clientSessionCode.getOrGenerateCode();
encodedState = IdentityBrokerState.decoded(relayState, authSession.getClient().getClientId(), authSession.getTabId());
encodedState = IdentityBrokerState.decoded(relayState, authSession.getClient().getId(), authSession.getClient().getClientId(), authSession.getTabId());
}
return new AuthenticationRequest(this.session, this.realmModel, authSession, this.request, this.session.getContext().getUri(), encodedState, getRedirectUri(providerId));

2
services/src/main/java/org/keycloak/social/twitter/TwitterIdentityProvider.java
View file

@ -185,7 +185,7 @@ public class TwitterIdentityProvider extends AbstractIdentityProvider<OAuth2Iden
public Response authResponse(@QueryParam("state") String state,
@QueryParam("denied") String denied,
@QueryParam("oauth_verifier") String verifier) {
IdentityBrokerState idpState = IdentityBrokerState.encoded(state);
IdentityBrokerState idpState = IdentityBrokerState.encoded(state, realm);
String clientId = idpState.getClientId();
String tabId = idpState.getTabId();
if (clientId == null || tabId == null) {