From 8f221bb21ea625b5799f2693e6f58bfa637603bf Mon Sep 17 00:00:00 2001 From: Marek Posolda Date: Tue, 11 Jan 2022 11:19:15 +0100 Subject: [PATCH] Validation for CIBA binding_message parameter (#9470) closes #9469 --- .../org/keycloak/OAuthErrorException.java | 3 ++ .../BackchannelAuthenticationEndpoint.java | 15 ++++++- .../keycloak/testsuite/client/CIBATest.java | 40 +++++++++++++++++++ 3 files changed, 57 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/keycloak/OAuthErrorException.java b/core/src/main/java/org/keycloak/OAuthErrorException.java index a246b383fe..df31c6781f 100755 --- a/core/src/main/java/org/keycloak/OAuthErrorException.java +++ b/core/src/main/java/org/keycloak/OAuthErrorException.java @@ -53,6 +53,9 @@ public class OAuthErrorException extends Exception { public static final String SLOW_DOWN = "slow_down"; public static final String EXPIRED_TOKEN = "expired_token"; + // CIBA + public static final String INVALID_BINDING_MESSAGE = "invalid_binding_message"; + // Others public static final String INVALID_CLIENT = "invalid_client"; public static final String INVALID_GRANT = "invalid_grant"; diff --git a/services/src/main/java/org/keycloak/protocol/oidc/grants/ciba/endpoints/BackchannelAuthenticationEndpoint.java b/services/src/main/java/org/keycloak/protocol/oidc/grants/ciba/endpoints/BackchannelAuthenticationEndpoint.java index 7ab0dfc4f6..b678efbdbe 100644 --- a/services/src/main/java/org/keycloak/protocol/oidc/grants/ciba/endpoints/BackchannelAuthenticationEndpoint.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/grants/ciba/endpoints/BackchannelAuthenticationEndpoint.java @@ -54,6 +54,7 @@ import javax.ws.rs.core.Response; import java.util.Collections; import java.util.Optional; +import java.util.regex.Pattern; import static org.keycloak.protocol.oidc.OIDCLoginProtocol.ID_TOKEN_HINT; import static org.keycloak.protocol.oidc.OIDCLoginProtocol.LOGIN_HINT_PARAM; @@ -62,6 +63,8 @@ public class BackchannelAuthenticationEndpoint extends AbstractCibaEndpoint { private final RealmModel realm; + private static final Pattern BINDING_MESSAGE_VALIDATION = Pattern.compile("^[a-zA-Z0-9-._+/!?#]{1,50}$"); + public BackchannelAuthenticationEndpoint(KeycloakSession session, EventBuilder event) { super(session, event); this.realm = session.getContext().getRealm(); @@ -172,7 +175,10 @@ public class BackchannelAuthenticationEndpoint extends AbstractCibaEndpoint { request.setScope(scope); // optional parameters - if (endpointRequest.getBindingMessage() != null) request.setBindingMessage(endpointRequest.getBindingMessage()); + if (endpointRequest.getBindingMessage() != null) { + validateBindingMessage(endpointRequest.getBindingMessage()); + request.setBindingMessage(endpointRequest.getBindingMessage()); + } if (endpointRequest.getAcr() != null) request.setAcrValues(endpointRequest.getAcr()); CibaConfig policy = realm.getCibaPolicy(); @@ -228,6 +234,13 @@ public class BackchannelAuthenticationEndpoint extends AbstractCibaEndpoint { } } + protected void validateBindingMessage(String bindingMessage) { + if (!BINDING_MESSAGE_VALIDATION.matcher(bindingMessage).matches()) { + throw new ErrorResponseException(OAuthErrorException.INVALID_BINDING_MESSAGE, "the binding_message value has to be max 50 characters in length and must contain only basic plain-text characters without spaces", + Response.Status.BAD_REQUEST); + } + } + private UserModel resolveUser(BackchannelAuthenticationEndpointRequest endpointRequest, String authRequestedUserHint) { CIBALoginUserResolver resolver = session.getProvider(CIBALoginUserResolver.class); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/CIBATest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/CIBATest.java index 1771b623b5..b0b2051f15 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/CIBATest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/CIBATest.java @@ -28,6 +28,7 @@ import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.impl.client.CloseableHttpClient; import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.nullValue; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -397,6 +398,45 @@ public class CIBATest extends AbstractClientPoliciesTest { } } + // Corresponds to the test fapi-ciba-id1-ensure-authorization-request-with-potentially-bad-binding-message from the FAPI CIBA conformance testsuite + @Test + public void testBadBindingMessage() throws Exception { + ClientResource clientResource = null; + ClientRepresentation clientRep = null; + try { + final String username = "nutzername-schwarz"; + clientResource = ApiUtil.findClientByClientId(adminClient.realm(TEST_REALM_NAME), TEST_CLIENT_NAME); + clientRep = clientResource.toRepresentation(); + prepareCIBASettings(clientResource, clientRep); + + // Binding message with non plain-text characters + String bindingMessage = "1234 \uD83D\uDC4D\uD83C\uDFFF 品川 Lor"; + AuthenticationRequestAcknowledgement response = oauth.doBackchannelAuthenticationRequest(TEST_CLIENT_NAME, TEST_CLIENT_PASSWORD, username, bindingMessage, null); + assertThat(response.getStatusCode(), is(equalTo(400))); + assertThat(response.getError(), is(OAuthErrorException.INVALID_BINDING_MESSAGE)); + + // Long binding message + bindingMessage = "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud."; + response = oauth.doBackchannelAuthenticationRequest(TEST_CLIENT_NAME, TEST_CLIENT_PASSWORD, username, bindingMessage, null); + assertThat(response.getStatusCode(), is(equalTo(400))); + assertThat(response.getError(), is(OAuthErrorException.INVALID_BINDING_MESSAGE)); + + // Empty binding message + bindingMessage = ""; + response = oauth.doBackchannelAuthenticationRequest(TEST_CLIENT_NAME, TEST_CLIENT_PASSWORD, username, bindingMessage, null); + assertThat(response.getStatusCode(), is(equalTo(400))); + assertThat(response.getError(), is(OAuthErrorException.INVALID_BINDING_MESSAGE)); + + // Valid binding message + bindingMessage = "Lorem_ipsum"; + response = oauth.doBackchannelAuthenticationRequest(TEST_CLIENT_NAME, TEST_CLIENT_PASSWORD, username, bindingMessage, null); + assertThat(response.getStatusCode(), is(equalTo(200))); + assertThat(response.getError(), is(nullValue())); + } finally { + revertCIBASettings(clientResource, clientRep); + } + } + @Test @Ignore("Should never happen because the AD does not send any information about the user but only the status of the authentication") public void testDifferentUserAuthenticated() throws Exception {