diff --git a/server-spi-private/src/main/java/org/keycloak/broker/provider/BrokeredIdentityContext.java b/server-spi-private/src/main/java/org/keycloak/broker/provider/BrokeredIdentityContext.java index a4dfa6f612..d3bb8948e8 100755 --- a/server-spi-private/src/main/java/org/keycloak/broker/provider/BrokeredIdentityContext.java +++ b/server-spi-private/src/main/java/org/keycloak/broker/provider/BrokeredIdentityContext.java @@ -42,7 +42,6 @@ public class BrokeredIdentityContext { private String lastName; private String brokerSessionId; private String brokerUserId; - private String code; private String token; private IdentityProviderModel idpConfig; private IdentityProvider idp; @@ -136,14 +135,6 @@ public class BrokeredIdentityContext { this.token = token; } - public String getCode() { - return code; - } - - public void setCode(String code) { - this.code = code; - } - public IdentityProviderModel getIdpConfig() { return idpConfig; } diff --git a/server-spi-private/src/main/java/org/keycloak/broker/provider/IdentityProvider.java b/server-spi-private/src/main/java/org/keycloak/broker/provider/IdentityProvider.java index 6564118269..75405ba784 100755 --- a/server-spi-private/src/main/java/org/keycloak/broker/provider/IdentityProvider.java +++ b/server-spi-private/src/main/java/org/keycloak/broker/provider/IdentityProvider.java @@ -38,18 +38,39 @@ public interface IdentityProvider extends Provi String FEDERATED_ACCESS_TOKEN = "FEDERATED_ACCESS_TOKEN"; interface AuthenticationCallback { + + /** + * Common method to return current authenticationSession and verify if it is not expired + * + * @param encodedCode + * @return see description + */ + AuthenticationSessionModel getAndVerifyAuthenticationSession(String encodedCode); + /** * This method should be called by provider after the JAXRS callback endpoint has finished authentication - * with the remote IDP + * with the remote IDP. There is an assumption that authenticationSession is set in the context when this method is called * * @param context - * @return + * @return see description */ Response authenticated(BrokeredIdentityContext context); - Response cancelled(String code); + /** + * Called when user cancelled authentication on the IDP side - for example user didn't approve consent page on the IDP side. + * Assumption is that authenticationSession is set in the {@link org.keycloak.models.KeycloakContext} when this method is called + * + * @return see description + */ + Response cancelled(); - Response error(String code, String message); + /** + * Called when error happened on the IDP side. + * Assumption is that authenticationSession is set in the {@link org.keycloak.models.KeycloakContext} when this method is called + * + * @return see description + */ + Response error(String message); } diff --git a/services/src/main/java/org/keycloak/broker/oidc/AbstractOAuth2IdentityProvider.java b/services/src/main/java/org/keycloak/broker/oidc/AbstractOAuth2IdentityProvider.java index c75e39c8fe..07c38d1f86 100755 --- a/services/src/main/java/org/keycloak/broker/oidc/AbstractOAuth2IdentityProvider.java +++ b/services/src/main/java/org/keycloak/broker/oidc/AbstractOAuth2IdentityProvider.java @@ -479,18 +479,20 @@ public abstract class AbstractOAuth2IdentityProvider notes = new HashMap<>(); BrokeredIdentityContext identity = new BrokeredIdentityContext(principal); identity.getContextData().put(SAML_LOGIN_RESPONSE, responseType); identity.getContextData().put(SAML_ASSERTION, assertion); - if (clientId != null && ! clientId.trim().isEmpty()) { - identity.getContextData().put(SAML_IDP_INITIATED_CLIENT_ID, clientId); - } + identity.setAuthenticationSession(authSession); identity.setUsername(principal); @@ -478,7 +492,7 @@ public class SAMLEndpoint { logger.error("Assertion expired."); event.event(EventType.IDENTITY_PROVIDER_RESPONSE); event.error(Errors.INVALID_SAML_RESPONSE); - return ErrorPage.error(session, null, Response.Status.BAD_REQUEST, Messages.EXPIRED_CODE); + return ErrorPage.error(session, authSession, Response.Status.BAD_REQUEST, Messages.EXPIRED_CODE); } AuthnStatementType authn = null; @@ -502,7 +516,6 @@ public class SAMLEndpoint { if (authn != null && authn.getSessionIndex() != null) { identity.setBrokerSessionId(identity.getBrokerUserId() + "." + authn.getSessionIndex()); } - identity.setCode(relayState); return callback.authenticated(identity); @@ -514,6 +527,42 @@ public class SAMLEndpoint { } + /** + * If there is a client whose SAML IDP-initiated SSO URL name is set to the + * given {@code clientUrlName}, creates a fresh authentication session for that + * client and returns a {@link AuthenticationSessionModel} object with that session. + * Otherwise returns "client not found" response. + * + * @param clientUrlName + * @return see description + */ + private AuthenticationSessionModel samlIdpInitiatedSSO(final String clientUrlName) { + event.event(EventType.LOGIN); + CacheControlUtil.noBackButtonCacheControlHeader(); + Optional oClient = SAMLEndpoint.this.realm.getClientsStream() + .filter(c -> Objects.equals(c.getAttribute(SamlProtocol.SAML_IDP_INITIATED_SSO_URL_NAME), clientUrlName)) + .findFirst(); + + if (! oClient.isPresent()) { + event.error(Errors.CLIENT_NOT_FOUND); + Response response = ErrorPage.error(session, null, Response.Status.BAD_REQUEST, Messages.CLIENT_NOT_FOUND); + throw new WebApplicationException(response); + } + + LoginProtocolFactory factory = (LoginProtocolFactory) session.getKeycloakSessionFactory().getProviderFactory(LoginProtocol.class, SamlProtocol.LOGIN_PROTOCOL); + SamlService samlService = (SamlService) factory.createProtocolEndpoint(SAMLEndpoint.this.realm, event); + ResteasyProviderFactory.getInstance().injectProperties(samlService); + AuthenticationSessionModel authSession = samlService.getOrCreateLoginSessionForIdpInitiatedSso(session, SAMLEndpoint.this.realm, oClient.get(), null); + if (authSession == null) { + event.error(Errors.INVALID_REDIRECT_URI); + Response response = ErrorPage.error(session, null, Response.Status.BAD_REQUEST, Messages.INVALID_REDIRECT_URI); + throw new WebApplicationException(response); + } + + return authSession; + } + + private boolean isSuccessfulSamlResponse(ResponseType responseType) { return responseType != null && responseType.getStatus() != null 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 25588ff745..49a046d9d0 100755 --- a/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java +++ b/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java @@ -372,12 +372,10 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal } try { - ParsedCodeContext parsedCode = parseSessionCode(code, clientId, tabId); - if (parsedCode.response != null) { - return parsedCode.response; - } + AuthenticationSessionModel authSession = parseSessionCode(code, clientId, tabId); - ClientSessionCode clientSessionCode = parsedCode.clientSessionCode; + ClientSessionCode clientSessionCode = new ClientSessionCode<>(session, realmModel, authSession); + clientSessionCode.setAction(AuthenticationSessionModel.Action.AUTHENTICATE.name()); IdentityProviderModel identityProviderModel = realmModel.getIdentityProviderByAlias(providerId); if (identityProviderModel == null) { throw new IdentityBrokerException("Identity Provider [" + providerId + "] not found."); @@ -506,17 +504,7 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal public Response authenticated(BrokeredIdentityContext context) { IdentityProviderModel identityProviderConfig = context.getIdpConfig(); - - final ParsedCodeContext parsedCode; - if (context.getContextData().get(SAMLEndpoint.SAML_IDP_INITIATED_CLIENT_ID) != null) { - parsedCode = samlIdpInitiatedSSO((String) context.getContextData().get(SAMLEndpoint.SAML_IDP_INITIATED_CLIENT_ID)); - } else { - parsedCode = parseEncodedSessionCode(context.getCode()); - } - if (parsedCode.response != null) { - return parsedCode.response; - } - ClientSessionCode clientCode = parsedCode.clientSessionCode; + AuthenticationSessionModel authenticationSession = context.getAuthenticationSession(); String providerId = identityProviderConfig.getAlias(); if (!identityProviderConfig.isStoreToken()) { @@ -525,9 +513,6 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal } context.setToken(null); } - - AuthenticationSessionModel authenticationSession = clientCode.getClientSession(); - context.setAuthenticationSession(authenticationSession); StatusResponseType loginResponse = (StatusResponseType) context.getContextData().get(SAMLEndpoint.SAML_LOGIN_RESPONSE); if (loginResponse != null) { @@ -629,7 +614,7 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal } authenticationSession.setAuthenticatedUser(federatedUser); - return finishOrRedirectToPostBrokerLogin(authenticationSession, context, false, parsedCode.clientSessionCode); + return finishOrRedirectToPostBrokerLogin(authenticationSession, context, false); } } @@ -655,15 +640,11 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal public Response afterFirstBrokerLogin(@QueryParam(LoginActionsService.SESSION_CODE) String code, @QueryParam("client_id") String clientId, @QueryParam(Constants.TAB_ID) String tabId) { - ParsedCodeContext parsedCode = parseSessionCode(code, clientId, tabId); - if (parsedCode.response != null) { - return parsedCode.response; - } - return afterFirstBrokerLogin(parsedCode.clientSessionCode); + AuthenticationSessionModel authSession = parseSessionCode(code, clientId, tabId); + return afterFirstBrokerLogin(authSession); } - private Response afterFirstBrokerLogin(ClientSessionCode clientSessionCode) { - AuthenticationSessionModel authSession = clientSessionCode.getClientSession(); + private Response afterFirstBrokerLogin(AuthenticationSessionModel authSession) { try { this.event.detail(Details.CODE_ID, authSession.getParentSession().getId()) .removeDetail("auth_method"); @@ -742,7 +723,7 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal updateFederatedIdentity(context, federatedUser); } - return finishOrRedirectToPostBrokerLogin(authSession, context, true, clientSessionCode); + return finishOrRedirectToPostBrokerLogin(authSession, context, true); } catch (Exception e) { return redirectToErrorPage(authSession, Response.Status.INTERNAL_SERVER_ERROR, Messages.IDENTITY_PROVIDER_UNEXPECTED_ERROR, e); @@ -750,12 +731,12 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal } - private Response finishOrRedirectToPostBrokerLogin(AuthenticationSessionModel authSession, BrokeredIdentityContext context, boolean wasFirstBrokerLogin, ClientSessionCode clientSessionCode) { + private Response finishOrRedirectToPostBrokerLogin(AuthenticationSessionModel authSession, BrokeredIdentityContext context, boolean wasFirstBrokerLogin) { String postBrokerLoginFlowId = context.getIdpConfig().getPostBrokerLoginFlowId(); if (postBrokerLoginFlowId == null) { logger.debugf("Skip redirect to postBrokerLogin flow. PostBrokerLogin flow not set for identityProvider '%s'.", context.getIdpConfig().getAlias()); - return afterPostBrokerLoginFlowSuccess(authSession, context, wasFirstBrokerLogin, clientSessionCode); + return afterPostBrokerLoginFlowSuccess(authSession, context, wasFirstBrokerLogin); } else { logger.debugf("Redirect to postBrokerLogin flow after authentication with identityProvider '%s'.", context.getIdpConfig().getAlias()); @@ -783,11 +764,7 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal public Response afterPostBrokerLoginFlow(@QueryParam(LoginActionsService.SESSION_CODE) String code, @QueryParam("client_id") String clientId, @QueryParam(Constants.TAB_ID) String tabId) { - ParsedCodeContext parsedCode = parseSessionCode(code, clientId, tabId); - if (parsedCode.response != null) { - return parsedCode.response; - } - AuthenticationSessionModel authenticationSession = parsedCode.clientSessionCode.getClientSession(); + AuthenticationSessionModel authenticationSession = parseSessionCode(code, clientId, tabId); try { SerializedBrokeredIdentityContext serializedCtx = SerializedBrokeredIdentityContext.readFromAuthenticationSession(authenticationSession, PostBrokerLoginConstants.PBL_BROKERED_IDENTITY_CONTEXT); @@ -810,13 +787,13 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal authenticationSession.removeAuthNote(PostBrokerLoginConstants.PBL_BROKERED_IDENTITY_CONTEXT); authenticationSession.removeAuthNote(PostBrokerLoginConstants.PBL_AFTER_FIRST_BROKER_LOGIN); - return afterPostBrokerLoginFlowSuccess(authenticationSession, context, wasFirstBrokerLogin, parsedCode.clientSessionCode); + return afterPostBrokerLoginFlowSuccess(authenticationSession, context, wasFirstBrokerLogin); } catch (IdentityBrokerException e) { return redirectToErrorPage(authenticationSession, Response.Status.INTERNAL_SERVER_ERROR, Messages.IDENTITY_PROVIDER_UNEXPECTED_ERROR, e); } } - private Response afterPostBrokerLoginFlowSuccess(AuthenticationSessionModel authSession, BrokeredIdentityContext context, boolean wasFirstBrokerLogin, ClientSessionCode clientSessionCode) { + private Response afterPostBrokerLoginFlowSuccess(AuthenticationSessionModel authSession, BrokeredIdentityContext context, boolean wasFirstBrokerLogin) { String providerId = context.getIdpConfig().getAlias(); UserModel federatedUser = authSession.getAuthenticatedUser(); @@ -831,7 +808,7 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal SerializedBrokeredIdentityContext serializedCtx = SerializedBrokeredIdentityContext.readFromAuthenticationSession(authSession, AbstractIdpAuthenticator.BROKERED_CONTEXT_NOTE); authSession.setAuthNote(AbstractIdpAuthenticator.FIRST_BROKER_LOGIN_SUCCESS, serializedCtx.getIdentityProviderId()); - return afterFirstBrokerLogin(clientSessionCode); + return afterFirstBrokerLogin(authSession); } else { return finishBrokerAuthentication(context, federatedUser, authSession, providerId); } @@ -873,40 +850,32 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal @Override - public Response cancelled(String code) { - ParsedCodeContext parsedCode = parseEncodedSessionCode(code); - if (parsedCode.response != null) { - return parsedCode.response; - } - ClientSessionCode clientCode = parsedCode.clientSessionCode; + public Response cancelled() { + AuthenticationSessionModel authSession = session.getContext().getAuthenticationSession(); - Response accountManagementFailedLinking = checkAccountManagementFailedLinking(clientCode.getClientSession(), Messages.CONSENT_DENIED); + Response accountManagementFailedLinking = checkAccountManagementFailedLinking(authSession, Messages.CONSENT_DENIED); if (accountManagementFailedLinking != null) { return accountManagementFailedLinking; } - return browserAuthentication(clientCode.getClientSession(), null); + return browserAuthentication(authSession, null); } @Override - public Response error(String code, String message) { - ParsedCodeContext parsedCode = parseEncodedSessionCode(code); - if (parsedCode.response != null) { - return parsedCode.response; - } - ClientSessionCode clientCode = parsedCode.clientSessionCode; + public Response error(String message) { + AuthenticationSessionModel authSession = session.getContext().getAuthenticationSession(); - Response accountManagementFailedLinking = checkAccountManagementFailedLinking(clientCode.getClientSession(), message); + Response accountManagementFailedLinking = checkAccountManagementFailedLinking(authSession, message); if (accountManagementFailedLinking != null) { return accountManagementFailedLinking; } - Response passiveLoginErrorReturned = checkPassiveLoginError(clientCode.getClientSession(), message); + Response passiveLoginErrorReturned = checkPassiveLoginError(authSession, message); if (passiveLoginErrorReturned != null) { return passiveLoginErrorReturned; } - return browserAuthentication(clientCode.getClientSession(), message); + return browserAuthentication(authSession, message); } @@ -1059,7 +1028,8 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal } } - private ParsedCodeContext parseEncodedSessionCode(String encodedCode) { + @Override + public AuthenticationSessionModel getAndVerifyAuthenticationSession(String encodedCode) { IdentityBrokerState state = IdentityBrokerState.encoded(encodedCode); String code = state.getDecodedState(); String clientId = state.getClientId(); @@ -1067,11 +1037,14 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal return parseSessionCode(code, clientId, tabId); } - private ParsedCodeContext parseSessionCode(String code, String clientId, String tabId) { + /** + * This method will throw JAX-RS exception in case it is not able to retrieve AuthenticationSessionModel. It never returns null + */ + private AuthenticationSessionModel parseSessionCode(String code, String clientId, String tabId) { if (code == null || clientId == null || tabId == null) { logger.debugf("Invalid request. Authorization code, clientId or tabId was null. Code=%s, clientId=%s, tabID=%s", code, clientId, tabId); Response staleCodeError = redirectToErrorPage(Response.Status.BAD_REQUEST, Messages.INVALID_REQUEST); - return ParsedCodeContext.response(staleCodeError); + throw new WebApplicationException(staleCodeError); } SessionCodeChecks checks = new SessionCodeChecks(realmModel, session.getContext().getUri(), request, clientConnection, session, event, null, code, null, clientId, tabId, LoginActionsService.AUTHENTICATE_PATH); @@ -1083,59 +1056,26 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal // Check if error happened during login or during linking from account management Response accountManagementFailedLinking = checkAccountManagementFailedLinking(authSession, Messages.STALE_CODE_ACCOUNT); if (accountManagementFailedLinking != null) { - return ParsedCodeContext.response(accountManagementFailedLinking); + throw new WebApplicationException(accountManagementFailedLinking); } else { Response errorResponse = checks.getResponse(); // Remove "code" from browser history errorResponse = BrowserHistoryHelper.getInstance().saveResponseAndRedirect(session, authSession, errorResponse, true, request); - return ParsedCodeContext.response(errorResponse); + throw new WebApplicationException(errorResponse); } } else { - return ParsedCodeContext.response(checks.getResponse()); + throw new WebApplicationException(checks.getResponse()); } } else { if (isDebugEnabled()) { logger.debugf("Authorization code is valid."); } - return ParsedCodeContext.clientSessionCode(checks.getClientCode()); + return checks.getClientCode().getClientSession(); } } - /** - * If there is a client whose SAML IDP-initiated SSO URL name is set to the - * given {@code clientUrlName}, creates a fresh client session for that - * client and returns a {@link ParsedCodeContext} object with that session. - * Otherwise returns "client not found" response. - * - * @param clientUrlName - * @return see description - */ - private ParsedCodeContext samlIdpInitiatedSSO(final String clientUrlName) { - event.event(EventType.LOGIN); - CacheControlUtil.noBackButtonCacheControlHeader(); - Optional oClient = this.realmModel.getClientsStream() - .filter(c -> Objects.equals(c.getAttribute(SamlProtocol.SAML_IDP_INITIATED_SSO_URL_NAME), clientUrlName)) - .findFirst(); - - if (! oClient.isPresent()) { - event.error(Errors.CLIENT_NOT_FOUND); - return ParsedCodeContext.response(redirectToErrorPage(Response.Status.BAD_REQUEST, Messages.CLIENT_NOT_FOUND)); - } - - LoginProtocolFactory factory = (LoginProtocolFactory) session.getKeycloakSessionFactory().getProviderFactory(LoginProtocol.class, SamlProtocol.LOGIN_PROTOCOL); - 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)); - } - private Response checkAccountManagementFailedLinking(AuthenticationSessionModel authSession, String error, Object... parameters) { UserSessionModel userSession = new AuthenticationSessionManager(session).getUserSession(authSession); if (userSession != null && authSession.getClient() != null && authSession.getClient().getClientId().equals(Constants.ACCOUNT_MANAGEMENT_CLIENT_ID)) { @@ -1348,21 +1288,4 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal } } - private static class ParsedCodeContext { - private ClientSessionCode clientSessionCode; - private Response response; - - public static ParsedCodeContext clientSessionCode(ClientSessionCode clientSessionCode) { - ParsedCodeContext ctx = new ParsedCodeContext(); - ctx.clientSessionCode = clientSessionCode; - return ctx; - } - - public static ParsedCodeContext response(Response response) { - ParsedCodeContext ctx = new ParsedCodeContext(); - ctx.response = response; - return ctx; - } - } - } diff --git a/services/src/main/java/org/keycloak/social/twitter/TwitterIdentityProvider.java b/services/src/main/java/org/keycloak/social/twitter/TwitterIdentityProvider.java index dba58ae7b7..9434d33797 100755 --- a/services/src/main/java/org/keycloak/social/twitter/TwitterIdentityProvider.java +++ b/services/src/main/java/org/keycloak/social/twitter/TwitterIdentityProvider.java @@ -184,28 +184,27 @@ public class TwitterIdentityProvider extends AbstractIdentityProviderMarek Posolda + */ +public class KcOidcBrokerStateParameterTest extends AbstractInitializedBaseBrokerTest { + + @Page + protected AppPage appPage; + + @Page + protected LoginExpiredPage loginExpiredPage; + + @Rule + public AssertEvents events = new AssertEvents(this); + + @Override + protected BrokerConfiguration getBrokerConfiguration() { + return KcOidcBrokerConfiguration.INSTANCE; + } + + @Test + public void testMissingStateParameter() { + final String consumerEndpointUrl = getURLOfOIDCIdpEndpointOnConsumerSide() + "?code=foo123"; + + events.clear(); + + // Manually open the consumer endpoint URL + driver.navigate().to(consumerEndpointUrl); + waitForPage(driver, "sign in to consumer", true); + + errorPage.assertCurrent(); + assertThat(errorPage.getError(), Matchers.is("Missing state parameter in response from identity provider.")); + + // Test that only loginEvent happened on consumer side. There should *not* be request sent to provider realm codeToToken endpoint (Assert that event is not there) + String consumerRealmId = realmsResouce().realm(bc.consumerRealmName()).toRepresentation().getId(); + events.expect(EventType.IDENTITY_PROVIDER_LOGIN_ERROR) + .clearDetails() + .session((String) null) + .realm(consumerRealmId) + .user((String) null) + .client((String) null) + .error("identity_provider_login_failure") + .assertEvent(); + + events.assertEmpty(); + } + + + @Test + public void testIncorrectStateParameter() throws Exception { + final String consumerEndpointUrl = KeycloakUriBuilder.fromUri(getURLOfOIDCIdpEndpointOnConsumerSide()) + .queryParam(OAuth2Constants.CODE, "foo456") + .queryParam(OAuth2Constants.STATE, "someIncorrectState") + .build().toString(); + + events.clear(); + + // Manually open the consumer endpoint URL + String consumerRealmId = realmsResouce().realm(bc.consumerRealmName()).toRepresentation().getId(); + driver.navigate().to(consumerEndpointUrl); + + // Test that only loginEvent happened on consumer side. There should *not* be request sent to provider realm codeToToken endpoint (Assert that event is not there) + events.expect(EventType.IDENTITY_PROVIDER_LOGIN_ERROR) + .clearDetails() + .session((String) null) + .realm(consumerRealmId) + .user((String) null) + .client((String) null) + .error("invalidRequestMessage") + .assertEvent(); + + events.assertEmpty(); + } + + + @Test + public void testCorrectStateParameterButIncorrectCode() throws Exception { + driver.navigate().to(getAccountUrl(getConsumerRoot(), bc.consumerRealmName())); + + waitForPage(driver, "sign in to", true); + loginPage.clickSocial(bc.getIDPAlias()); + waitForPage(driver, "sign in to", true); + + // Get the "state", which was generated by "consumer" before sending OIDC AuthenticationRequest to "provider" + String url = driver.getCurrentUrl(); + String stateParamValue = UriUtils.decodeQueryString(url).getFirst(OAuth2Constants.STATE); + + final String consumerEndpointUrl = KeycloakUriBuilder.fromUri(getURLOfOIDCIdpEndpointOnConsumerSide()) + .queryParam(OAuth2Constants.CODE, "foo123") + .queryParam(OAuth2Constants.STATE, stateParamValue) + .build().toString(); + + events.clear(); + + // Manually open the consumer endpoint URL + String providerRealmId = realmsResouce().realm(bc.providerRealmName()).toRepresentation().getId(); + String consumerRealmId = realmsResouce().realm(bc.consumerRealmName()).toRepresentation().getId(); + driver.navigate().to(consumerEndpointUrl); + + // Check that loginError on consumer side was triggered. Also CodeToToken request was sent to the "provider", but failed due the incorrect code + events.expect(EventType.CODE_TO_TOKEN_ERROR) + .clearDetails() + .session((String) null) + .realm(providerRealmId) + .user((String) null) + .client("brokerapp") + .error("invalid_code") + .assertEvent(); + + events.expect(EventType.IDENTITY_PROVIDER_LOGIN_ERROR) + .clearDetails() + .session((String) null) + .realm(consumerRealmId) + .user((String) null) + .client(Constants.ACCOUNT_MANAGEMENT_CLIENT_ID) + .error("identity_provider_login_failure") + .assertEvent(); + + // Re-send the request to same URL. There should *not* be additional + // request sent to provider realm codeToToken endpoint due the "state" already used on consumer side (Assert that CodeToToken event is not there) + // The consumer should display "Page has expired" error + driver.navigate().to(consumerEndpointUrl); + loginExpiredPage.assertCurrent(); + + events.assertEmpty(); + + } + + // Return the endpoint on consumer side where the IDentity Provider redirects the browser after successful authentication on IDP side. + private String getURLOfOIDCIdpEndpointOnConsumerSide() { + BrokerConfiguration brokerConfig = getBrokerConfiguration(); + return oauth.AUTH_SERVER_ROOT + "/realms/" + brokerConfig.consumerRealmName() + "/broker/" + brokerConfig.getIDPAlias() + "/endpoint"; + } + + +} diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerTest.java index 2d770b3dc7..1a48c604d8 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerTest.java @@ -413,52 +413,6 @@ public final class KcOidcBrokerTest extends AbstractAdvancedBrokerTest { errorPage.assertCurrent(); } - @Test - public void testMissingStateParameter() { - final String IDP_NAME = "github"; - - RealmResource realmResource = Optional.ofNullable(realmsResouce().realm(bc.providerRealmName())).orElse(null); - assertThat(realmResource, Matchers.notNullValue()); - final int COUNT_PROVIDERS = Optional.of(realmResource.identityProviders().findAll().size()).orElse(0); - - try { - IdentityProviderRepresentation idp = new IdentityProviderRepresentation(); - idp.setAlias(IDP_NAME); - idp.setDisplayName(IDP_NAME); - idp.setProviderId(IDP_NAME); - idp.setEnabled(true); - - Response response = realmResource.identityProviders().create(idp); - assertThat(response, Matchers.notNullValue()); - assertThat(response.getStatus(), Matchers.is(Response.Status.CREATED.getStatusCode())); - assertThat(realmResource.identityProviders().findAll().size(), Matchers.is(COUNT_PROVIDERS + 1)); - assertThat(ApiUtil.getCreatedId(response), Matchers.notNullValue()); - - IdentityProviderRepresentation provider = Optional.ofNullable(realmResource.identityProviders().get(IDP_NAME).toRepresentation()).orElse(null); - assertThat(provider, Matchers.notNullValue()); - assertThat(provider.getProviderId(), Matchers.is(IDP_NAME)); - assertThat(provider.getAlias(), Matchers.is(IDP_NAME)); - assertThat(provider.getDisplayName(), Matchers.is(IDP_NAME)); - - final String REALM_NAME = Optional.ofNullable(realmResource.toRepresentation().getRealm()).orElse(null); - assertThat(REALM_NAME, Matchers.notNullValue()); - - final String LINK = oauth.AUTH_SERVER_ROOT + "/realms/" + REALM_NAME + "/broker/" + IDP_NAME + "/endpoint?code=foo123"; - - driver.navigate().to(LINK); - waitForPage(driver, "sign in to provider", true); - - errorPage.assertCurrent(); - assertThat(errorPage.getError(), Matchers.is("Missing state parameter in response from identity provider.")); - - } finally { - IdentityProviderResource resource = Optional.ofNullable(realmResource.identityProviders().get(IDP_NAME)).orElse(null); - assertThat(resource, Matchers.notNullValue()); - resource.remove(); - assertThat(Optional.of(realmResource.identityProviders().findAll().size()).orElse(0), Matchers.is(COUNT_PROVIDERS)); - } - } - @Test public void testIdPNotFound() { final String notExistingIdP = "not-exists";