From 3c44e6c3773d708796b8abee6891d7ae98ac0cf8 Mon Sep 17 00:00:00 2001 From: rmartinc Date: Wed, 12 Dec 2018 09:26:46 +0100 Subject: [PATCH] KEYCLOAK-9068: IDP-initiated-flow is not working with REDIRECT binding --- .../keycloak/protocol/saml/SamlService.java | 58 ++++++++++++------- .../resources/IdentityBrokerService.java | 4 ++ .../updaters/ClientAttributeUpdater.java | 4 ++ .../testsuite/saml/IdpInitiatedLoginTest.java | 54 ++++++++++++++++- 4 files changed, 97 insertions(+), 23 deletions(-) diff --git a/services/src/main/java/org/keycloak/protocol/saml/SamlService.java b/services/src/main/java/org/keycloak/protocol/saml/SamlService.java index fbc2aa2ec1..a9e189a49f 100755 --- a/services/src/main/java/org/keycloak/protocol/saml/SamlService.java +++ b/services/src/main/java/org/keycloak/protocol/saml/SamlService.java @@ -655,19 +655,45 @@ public class SamlService extends AuthorizationEndpointBase { event.error(Errors.INVALID_CLIENT); return ErrorPage.error(session, null, Response.Status.BAD_REQUEST, "Wrong client protocol."); } - if (client.getManagementUrl() == null && client.getAttribute(SamlProtocol.SAML_ASSERTION_CONSUMER_URL_POST_ATTRIBUTE) == null && client.getAttribute(SamlProtocol.SAML_ASSERTION_CONSUMER_URL_REDIRECT_ATTRIBUTE) == null) { + + session.getContext().setClient(client); + + AuthenticationSessionModel authSession = getOrCreateLoginSessionForIdpInitiatedSso(this.session, this.realm, client, relayState); + if (authSession == null) { logger.error("SAML assertion consumer url not set up"); event.error(Errors.INVALID_REDIRECT_URI); return ErrorPage.error(session, null, Response.Status.BAD_REQUEST, Messages.INVALID_REDIRECT_URI); } - session.getContext().setClient(client); - - AuthenticationSessionModel authSession = getOrCreateLoginSessionForIdpInitiatedSso(this.session, this.realm, client, relayState); - return newBrowserAuthentication(authSession, false, false); } + /** + * Checks the client configuration to return the redirect URL and the binding type. + * POST is preferred, only if the SAML_ASSERTION_CONSUMER_URL_POST_ATTRIBUTE + * and management URL are empty REDIRECT is chosen. + * + * @param client Client to create client session for + * @return a two string array [samlUrl, bindingType] or null if error + */ + private String[] getUrlAndBindingForIdpInitiatedSso(ClientModel client) { + String postUrl = client.getAttribute(SamlProtocol.SAML_ASSERTION_CONSUMER_URL_POST_ATTRIBUTE); + String getUrl = client.getAttribute(SamlProtocol.SAML_ASSERTION_CONSUMER_URL_REDIRECT_ATTRIBUTE); + if (postUrl != null && !postUrl.trim().isEmpty()) { + // first the POST binding URL + return new String[] {postUrl.trim(), SamlProtocol.SAML_POST_BINDING}; + } else if (client.getManagementUrl() != null && !client.getManagementUrl().trim().isEmpty()) { + // second the management URL and POST + return new String[] {client.getManagementUrl().trim(), SamlProtocol.SAML_POST_BINDING}; + } else if (getUrl != null && !getUrl.trim().isEmpty()){ + // last option REDIRECT binding and URL + return new String[] {getUrl.trim(), SamlProtocol.SAML_REDIRECT_BINDING}; + } else { + // error + return null; + } + } + /** * Creates a client session object for SAML IdP-initiated SSO session. * The session takes the parameters from from client definition, @@ -677,29 +703,21 @@ public class SamlService extends AuthorizationEndpointBase { * @param realm Realm to create client session in * @param client Client to create client session for * @param relayState Optional relay state - free field as per SAML specification - * @return + * @return The auth session model or null if there is no SAML url is found */ public AuthenticationSessionModel getOrCreateLoginSessionForIdpInitiatedSso(KeycloakSession session, RealmModel realm, ClientModel client, String relayState) { - String bindingType = SamlProtocol.SAML_POST_BINDING; - if (client.getManagementUrl() == null && client.getAttribute(SamlProtocol.SAML_ASSERTION_CONSUMER_URL_POST_ATTRIBUTE) == null && client.getAttribute(SamlProtocol.SAML_ASSERTION_CONSUMER_URL_REDIRECT_ATTRIBUTE) != null) { - bindingType = SamlProtocol.SAML_REDIRECT_BINDING; - } - - String redirect; - if (bindingType.equals(SamlProtocol.SAML_REDIRECT_BINDING)) { - redirect = client.getAttribute(SamlProtocol.SAML_ASSERTION_CONSUMER_URL_REDIRECT_ATTRIBUTE); - } else { - redirect = client.getAttribute(SamlProtocol.SAML_ASSERTION_CONSUMER_URL_POST_ATTRIBUTE); - } - if (redirect == null) { - redirect = client.getManagementUrl(); + String[] bindingProperties = getUrlAndBindingForIdpInitiatedSso(client); + if (bindingProperties == null) { + return null; } + String redirect = bindingProperties[0]; + String bindingType = bindingProperties[1]; AuthenticationSessionModel authSession = createAuthenticationSession(client, null); authSession.setProtocol(SamlProtocol.LOGIN_PROTOCOL); authSession.setAction(AuthenticationSessionModel.Action.AUTHENTICATE.name()); - authSession.setClientNote(SamlProtocol.SAML_BINDING, SamlProtocol.SAML_POST_BINDING); + authSession.setClientNote(SamlProtocol.SAML_BINDING, bindingType); authSession.setClientNote(SamlProtocol.SAML_IDP_INITIATED_LOGIN, "true"); authSession.setRedirectUri(redirect); diff --git a/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java b/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java index 66278a703d..0b2dc11891 100755 --- a/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java +++ b/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java @@ -1052,6 +1052,10 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal SamlService samlService = (SamlService) factory.createProtocolEndpoint(realmModel, event); ResteasyProviderFactory.getInstance().injectProperties(samlService); AuthenticationSessionModel authSession = samlService.getOrCreateLoginSessionForIdpInitiatedSso(session, realmModel, oClient.get(), null); + if (authSession == null) { + event.error(Errors.INVALID_REDIRECT_URI); + return ParsedCodeContext.response(redirectToErrorPage(Response.Status.BAD_REQUEST, Messages.INVALID_REDIRECT_URI)); + } return ParsedCodeContext.clientSessionCode(new ClientSessionCode<>(session, this.realmModel, authSession)); } diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/ClientAttributeUpdater.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/ClientAttributeUpdater.java index 0445b5eb1b..81d5e9f125 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/ClientAttributeUpdater.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/ClientAttributeUpdater.java @@ -102,4 +102,8 @@ public class ClientAttributeUpdater extends ServerResourceUpdater { + assertThat(ob, Matchers.isSamlResponse(JBossSAMLURIConstants.STATUS_SUCCESS)); + ResponseType resp = (ResponseType) ob; + assertThat(resp.getDestination(), is(SAML_ASSERTION_CONSUMER_URL_SALES_POST)); + return null; + }) + .build() + .execute(); + } + } + + @Test + public void testIdpInitiatedLoginRedirect() throws IOException { + String url = adminClient.realm(REALM_NAME).clients().findByClientId(SAML_CLIENT_ID_SALES_POST).get(0) + .getAttributes().get(SamlProtocol.SAML_ASSERTION_CONSUMER_URL_POST_ATTRIBUTE); + try (Closeable c = ClientAttributeUpdater.forClient(adminClient, REALM_NAME, SAML_CLIENT_ID_SALES_POST) + .setAttribute(SamlProtocol.SAML_ASSERTION_CONSUMER_URL_POST_ATTRIBUTE, null) + .setAttribute(SamlProtocol.SAML_ASSERTION_CONSUMER_URL_REDIRECT_ATTRIBUTE, url) + .update()) { + new SamlClientBuilder() + .idpInitiatedLogin(getAuthServerSamlEndpoint(REALM_NAME), "sales-post").build() + .login().user(bburkeUser).build() + .processSamlResponse(Binding.REDIRECT) + .transformObject(ob -> { + assertThat(ob, Matchers.isSamlResponse(JBossSAMLURIConstants.STATUS_SUCCESS)); + ResponseType resp = (ResponseType) ob; + assertThat(resp.getDestination(), is(SAML_ASSERTION_CONSUMER_URL_SALES_POST)); + return null; + }) + .build() + .execute(); + } + } + @Test public void testTwoConsequentIdpInitiatedLogins() { new SamlClientBuilder()