Redirect loop with authentication success but access denied at default identity provider

closes #17441

server-spi-private/src/main/java/org/keycloak/broker/provider/IdentityProvider.java

@@ -60,9 +60,10 @@ public interface IdentityProvider<C extends IdentityProviderModel> extends Provi
          * 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
          *
+         * @param idpConfig identity provider config
          * @return see description
          */
-        Response cancelled();
+        Response cancelled(IdentityProviderModel idpConfig);
 
         /**
          * Called when error happened on the IDP side.

services/src/main/java/org/keycloak/broker/oidc/AbstractOAuth2IdentityProvider.java

@@ -491,7 +491,7 @@ public abstract class AbstractOAuth2IdentityProvider<C extends OAuth2IdentityPro
                 if (error != null) {
                     logger.error(error + " for broker login " + providerConfig.getProviderId());
                     if (error.equals(ACCESS_DENIED)) {
-                        return callback.cancelled();
+                        return callback.cancelled(providerConfig);
                     } else if (error.equals(OAuthErrorException.LOGIN_REQUIRED) || error.equals(OAuthErrorException.INTERACTION_REQUIRED)) {
                         return callback.error(error);
                     } else {

services/src/main/java/org/keycloak/services/messages/Messages.java

@@ -285,6 +285,8 @@ public class Messages {
     // Conditions in Conditional Flow
     public static final String ACCESS_DENIED = "access-denied";
 
+    public static final String ACCESS_DENIED_WHEN_IDP_AUTH = "access-denied-when-idp-auth";
+
     public static final String DELETE_ACCOUNT_LACK_PRIVILEDGES = "deletingAccountForbidden";
     public static final String DELETE_ACCOUNT_ERROR = "errorDeletingAccount";
 

services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java

@@ -62,6 +62,7 @@ import org.keycloak.models.UserModel;
 import org.keycloak.models.UserSessionModel;
 import org.keycloak.models.utils.AuthenticationFlowResolver;
 import org.keycloak.models.utils.FormMessage;
+import org.keycloak.models.utils.KeycloakModelUtils;
 import org.keycloak.protocol.LoginProtocol;
 import org.keycloak.protocol.oidc.OIDCLoginProtocol;
 import org.keycloak.protocol.oidc.TokenManager;
@@ -860,15 +861,16 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal
 
 
     @Override
-    public Response cancelled() {
+    public Response cancelled(IdentityProviderModel idpConfig) {
         AuthenticationSessionModel authSession = session.getContext().getAuthenticationSession();
 
-        Response accountManagementFailedLinking = checkAccountManagementFailedLinking(authSession, Messages.CONSENT_DENIED);
+        String idpDisplayName = KeycloakModelUtils.getIdentityProviderDisplayName(session, idpConfig);
+        Response accountManagementFailedLinking = checkAccountManagementFailedLinking(authSession, Messages.ACCESS_DENIED_WHEN_IDP_AUTH, idpDisplayName);
         if (accountManagementFailedLinking != null) {
             return accountManagementFailedLinking;
         }
 
-        return browserAuthentication(authSession, null);
+        return browserAuthentication(authSession, Messages.ACCESS_DENIED_WHEN_IDP_AUTH, idpDisplayName);
     }
 
     @Override
@@ -1192,7 +1194,7 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal
     }
 
 
-    protected Response browserAuthentication(AuthenticationSessionModel authSession, String errorMessage) {
+    protected Response browserAuthentication(AuthenticationSessionModel authSession, String errorMessage, Object... parameters) {
         this.event.event(EventType.LOGIN);
         AuthenticationFlowModel flow = AuthenticationFlowResolver.resolveBrowserFlow(authSession);
         String flowId = flow.getId();
@@ -1207,7 +1209,7 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal
                 .setSession(session)
                 .setUriInfo(session.getContext().getUri())
                 .setRequest(request);
-        if (errorMessage != null) processor.setForwardedErrorMessage(new FormMessage(null, errorMessage));
+        if (errorMessage != null) processor.setForwardedErrorMessage(new FormMessage(null, errorMessage, parameters));
 
         try {
             CacheControlUtil.noBackButtonCacheControlHeader(session);

services/src/main/java/org/keycloak/social/twitter/TwitterIdentityProvider.java

@@ -202,7 +202,7 @@ public class TwitterIdentityProvider extends AbstractIdentityProvider<OAuth2Iden
             AuthenticationSessionModel authSession = ClientSessionCode.getClientSession(state, tabId, session, realm, client, event, AuthenticationSessionModel.class);
 
             if (denied != null) {
-                return callback.cancelled();
+                return callback.cancelled(provider.getConfig());
             }
 
             OAuth2IdentityProviderConfig providerConfig = provider.getConfig();

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractDefaultIdpTest.java

@@ -0,0 +1,167 @@
+/*
+ * Copyright 2023 Red Hat, Inc. and/or its affiliates
+ *  and other contributors as indicated by the @author tags.
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ *
+ */
+
+package org.keycloak.testsuite.broker;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.UUID;
+import java.util.stream.IntStream;
+
+import org.junit.Test;
+import org.keycloak.admin.client.resource.RealmResource;
+import org.keycloak.authentication.authenticators.browser.IdentityProviderAuthenticatorFactory;
+import org.keycloak.models.AuthenticationExecutionModel;
+import org.keycloak.models.AuthenticatorConfigModel;
+import org.keycloak.representations.idm.ClientRepresentation;
+import org.keycloak.testsuite.Assert;
+import org.keycloak.testsuite.util.FlowUtil;
+import org.keycloak.testsuite.util.UIUtils;
+import org.openqa.selenium.By;
+import org.openqa.selenium.WebElement;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+import static org.keycloak.testsuite.broker.BrokerTestTools.getConsumerRoot;
+import static org.keycloak.testsuite.broker.BrokerTestTools.waitForPage;
+
+/**
+ * Test for scenarios where "Identity Provider Authenticator" is set with "default identity provider" directly redirecting to provider realm
+ */
+public abstract class AbstractDefaultIdpTest extends AbstractInitializedBaseBrokerTest {
+
+    @Override
+    public void beforeBrokerTest() {
+        super.beforeBrokerTest();
+        // Require broker to show consent screen
+        RealmResource brokeredRealm = adminClient.realm(bc.providerRealmName());
+        List<ClientRepresentation> clients = brokeredRealm.clients().findByClientId(bc.getIDPClientIdInProviderRealm());
+        org.junit.Assert.assertEquals(1, clients.size());
+        ClientRepresentation brokerApp = clients.get(0);
+        brokerApp.setConsentRequired(true);
+        brokeredRealm.clients().get(brokerApp.getId()).update(brokerApp);
+    }
+
+    @Test
+    public void testDefaultIdpNotSet() {
+        // Set the Default Identity Provider option for the Identity Provider Redirector to null
+        configureFlow(null);
+
+        // Navigate to the auth page
+        driver.navigate().to(getAccountUrl(getConsumerRoot(), bc.consumerRealmName()));
+        waitForPage(driver, "sign in to", true);
+
+        Assert.assertTrue("Driver should be on the initial page and nothing should have happened",
+                driver.getCurrentUrl().contains("/auth/realms/" + bc.consumerRealmName() + "/"));
+    }
+
+    @Test
+    public void testDefaultIdpSet() {
+        // Set the Default Identity Provider option to the remote IdP name
+        configureFlow(getBrokerConfiguration().getIDPAlias());
+
+        String username = "all-info-set@localhost.com";
+        createUser(bc.providerRealmName(), username, "password", "FirstName");
+
+        // Navigate to the auth page
+        driver.navigate().to(getAccountUrl(getConsumerRoot(), bc.consumerRealmName()));
+        waitForPage(driver, "sign in to", true);
+
+        // Make sure we got redirected to the remote IdP automatically
+        Assert.assertTrue("Driver should be on the provider realm page right now",
+                driver.getCurrentUrl().contains("/auth/realms/" + bc.providerRealmName() + "/"));
+    }
+
+    protected void testDefaultIdpSetTriedAndReturnedError(String expectedErrorMessageOnLoginScreen) {
+        // Set the Default Identity Provider option to the remote IdP name
+        configureFlow(getBrokerConfiguration().getIDPAlias());
+
+        String username = "all-info-set@localhost.com";
+        createUser(bc.providerRealmName(), username, "password", "FirstName");
+
+        // Navigate to the auth page
+        driver.navigate().to(getAccountUrl(getConsumerRoot(), bc.consumerRealmName()));
+        waitForPage(driver, "sign in to", true);
+
+        // Make sure we got redirected to the remote IdP automatically
+        Assert.assertTrue("Driver should be on the provider realm page right now",
+                driver.getCurrentUrl().contains("/auth/realms/" + bc.providerRealmName() + "/"));
+
+        // Attempt login
+        log.debug("Logging in");
+        loginPage.login(bc.getUserLogin(), bc.getUserPassword());
+
+        // Deny user consent
+        grantPage.assertCurrent();
+        grantPage.cancel();
+
+        waitForPage(driver, "sign in to", true);
+
+        WebElement errorElement = driver.findElement(By.className("alert-error"));
+        assertNotNull("Page should show an error message but it's missing", errorElement);
+
+        // Login to IDP failed due consent denied. Error message is displayed on the username/password screen of the consumer realm
+        assertEquals(expectedErrorMessageOnLoginScreen, UIUtils.getTextFromElement(errorElement));
+    }
+
+    protected void configureFlow(String defaultIdpValue) {
+        String newFlowAlias;
+
+        HashMap<String, String> defaultIdpConfig = new HashMap<String, String>();
+        if (defaultIdpValue != null && !defaultIdpValue.isEmpty()) {
+            defaultIdpConfig.put(IdentityProviderAuthenticatorFactory.DEFAULT_PROVIDER, defaultIdpValue);
+            newFlowAlias = "Browser - Default IdP " + defaultIdpValue;
+        }
+        else
+            newFlowAlias = "Browser - Default IdP OFF";
+
+        testingClient.server("consumer").run(session -> FlowUtil.inCurrentRealm(session).copyBrowserFlow(newFlowAlias));
+        testingClient.server("consumer").run(session ->
+                {
+                    List<AuthenticationExecutionModel> executions = FlowUtil.inCurrentRealm(session)
+                            .selectFlow(newFlowAlias)
+                            .getExecutions();
+
+                    int index = IntStream.range(0, executions.size())
+                            .filter(t -> IdentityProviderAuthenticatorFactory.PROVIDER_ID.equals(executions.get(t).getAuthenticator()))
+                            .findFirst()
+                            .orElse(-1);
+
+                    assertTrue("Identity Provider Redirector execution not found", index >= 0);
+
+                    FlowUtil.inCurrentRealm(session)
+                            .selectFlow(newFlowAlias)
+                            .updateExecution(index,
+                                    config -> {
+                                        AuthenticatorConfigModel authConfig = new AuthenticatorConfigModel();
+                                        authConfig.setId(UUID.randomUUID().toString());
+                                        authConfig.setAlias("cfg" + authConfig.getId().hashCode());
+                                        authConfig.setConfig(defaultIdpConfig);
+
+                                        session.getContext().getRealm().addAuthenticatorConfig(authConfig);
+
+                                        config.setAuthenticatorConfig(authConfig.getId());
+                                    }
+                            )
+                            .defineAsBrowserFlow();
+                }
+        );
+    }
+}

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerWithConsentTest.java

@@ -185,6 +185,6 @@ public class KcOidcBrokerWithConsentTest extends AbstractInitializedBaseBrokerTe
 
         // Assert account error page with "consentDenied" error displayed
         accountFederatedIdentityPage.assertCurrent();
-        Assert.assertEquals("Consent denied.", accountFederatedIdentityPage.getError());
+        Assert.assertEquals("Access denied when authenticating with kc-oidc-idp", accountFederatedIdentityPage.getError());
     }
 }

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcDefaultIdpTest.java

@@ -0,0 +1,40 @@
+/*
+ * Copyright 2023 Red Hat, Inc. and/or its affiliates
+ *  and other contributors as indicated by the @author tags.
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ *
+ */
+
+package org.keycloak.testsuite.broker;
+
+import org.junit.Test;
+
+/**
+ * @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
+ */
+public class KcOidcDefaultIdpTest extends AbstractDefaultIdpTest {
+
+
+    // Issue 17441
+    @Test
+    public void testDefaultIdpSetTriedAndReturnedError() {
+        testDefaultIdpSetTriedAndReturnedError("Access denied when authenticating with kc-oidc-idp");
+    }
+
+    @Override
+    protected BrokerConfiguration getBrokerConfiguration() {
+        return new KcOidcBrokerConfiguration();
+    }
+}

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlDefaultIdpTest.java

@@ -1,153 +1,17 @@
 package org.keycloak.testsuite.broker;
 
 import org.junit.Test;
-import org.keycloak.admin.client.resource.RealmResource;
-import org.keycloak.authentication.authenticators.browser.IdentityProviderAuthenticatorFactory;
-import org.keycloak.models.AuthenticationExecutionModel;
-import org.keycloak.models.AuthenticatorConfigModel;
-import org.keycloak.representations.idm.ClientRepresentation;
-import org.keycloak.testsuite.Assert;
-import org.keycloak.testsuite.util.FlowUtil;
-import org.keycloak.testsuite.util.UIUtils;
-import org.openqa.selenium.By;
-import org.openqa.selenium.WebElement;
-
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
-import static org.keycloak.testsuite.broker.BrokerTestTools.getConsumerRoot;
-import static org.keycloak.testsuite.broker.BrokerTestTools.waitForPage;
-
-import java.util.HashMap;
-import java.util.List;
-import java.util.stream.IntStream;
-import java.util.UUID;
 
 /**
  * Test of various scenarios related to the use of default IdP option
  * in the Identity Provider Redirector authenticator
  */
-public class KcSamlDefaultIdpTest extends AbstractInitializedBaseBrokerTest {
+public class KcSamlDefaultIdpTest extends AbstractDefaultIdpTest {
-
-    @Override
-    public void beforeBrokerTest() {
-        super.beforeBrokerTest();
-        // Require broker to show consent screen
-        RealmResource brokeredRealm = adminClient.realm(bc.providerRealmName());
-        List<ClientRepresentation> clients = brokeredRealm.clients().findByClientId(bc.getIDPClientIdInProviderRealm());
-        org.junit.Assert.assertEquals(1, clients.size());
-        ClientRepresentation brokerApp = clients.get(0);
-        brokerApp.setConsentRequired(true);
-        brokeredRealm.clients().get(brokerApp.getId()).update(brokerApp);
-    }
-
-    @Test
-    public void testDefaultIdpNotSet() {
-        // Set the Default Identity Provider option for the Identity Provider Redirector to null
-        configureFlow(null);
-
-        // Navigate to the auth page
-        driver.navigate().to(getAccountUrl(getConsumerRoot(), bc.consumerRealmName()));
-        waitForPage(driver, "sign in to", true);
-
-        Assert.assertTrue("Driver should be on the initial page and nothing should have happened",
-                driver.getCurrentUrl().contains("/auth/realms/" + bc.consumerRealmName() + "/"));
-    }
-
-    @Test
-    public void testDefaultIdpSet() {
-        // Set the Default Identity Provider option to the remote IdP name
-        configureFlow("kc-saml-idp");
-
-        String username = "all-info-set@localhost.com";
-        createUser(bc.providerRealmName(), username, "password", "FirstName");
-
-        // Navigate to the auth page
-        driver.navigate().to(getAccountUrl(getConsumerRoot(), bc.consumerRealmName()));
-        waitForPage(driver, "sign in to", true);
-
-        // Make sure we got redirected to the remote IdP automatically
-        Assert.assertTrue("Driver should be on the provider realm page right now",
-                driver.getCurrentUrl().contains("/auth/realms/" + bc.providerRealmName() + "/"));
-    }
 
     // KEYCLOAK-17368
     @Test
     public void testDefaultIdpSetTriedAndReturnedError() {
-        // Set the Default Identity Provider option to the remote IdP name
+        testDefaultIdpSetTriedAndReturnedError("Unexpected error when authenticating with identity provider");
-        configureFlow("kc-saml-idp");
-
-        String username = "all-info-set@localhost.com";
-        createUser(bc.providerRealmName(), username, "password", "FirstName");
-
-        // Navigate to the auth page
-        driver.navigate().to(getAccountUrl(getConsumerRoot(), bc.consumerRealmName()));
-        waitForPage(driver, "sign in to", true);
-
-        // Make sure we got redirected to the remote IdP automatically
-        Assert.assertTrue("Driver should be on the provider realm page right now",
-                driver.getCurrentUrl().contains("/auth/realms/" + bc.providerRealmName() + "/"));
-
-        // Attempt login
-        log.debug("Logging in");
-        loginPage.login(bc.getUserLogin(), bc.getUserPassword());
-
-        // Deny user consent
-        grantPage.assertCurrent();
-        grantPage.cancel();
-
-        waitForPage(driver, "sign in to", true);
-
-        WebElement errorElement = driver.findElement(By.className("alert-error"));
-        assertNotNull("Page should show an error message but it's missing", errorElement);
-
-        assertEquals("Unexpected error when authenticating with identity provider", UIUtils.getTextFromElement(errorElement));
-    }
-
-    private void configureFlow(String defaultIdpValue)
-    {
-        String newFlowAlias;
-
-        HashMap<String, String> defaultIdpConfig = new HashMap<String, String>();
-        if (defaultIdpValue != null && !defaultIdpValue.isEmpty())
-        {
-            defaultIdpConfig.put(IdentityProviderAuthenticatorFactory.DEFAULT_PROVIDER, defaultIdpValue);
-            newFlowAlias = "Browser - Default IdP " + defaultIdpValue;
-        }
-        else
-            newFlowAlias = "Browser - Default IdP OFF";
-
-        testingClient.server("consumer").run(session -> FlowUtil.inCurrentRealm(session).copyBrowserFlow(newFlowAlias));
-        testingClient.server("consumer").run(session ->
-            {
-                List<AuthenticationExecutionModel> executions = FlowUtil.inCurrentRealm(session)
-                    .selectFlow(newFlowAlias)
-                    .getExecutions();
-
-                int index = IntStream.range(0, executions.size())
-                    .filter(t -> IdentityProviderAuthenticatorFactory.PROVIDER_ID.equals(executions.get(t).getAuthenticator()))
-                    .findFirst()
-                    .orElse(-1);
-
-                assertTrue("Identity Provider Redirector execution not found", index >= 0);
-
-                FlowUtil.inCurrentRealm(session)
-                    .selectFlow(newFlowAlias)
-                    .updateExecution(index,
-                        config -> {
-                            AuthenticatorConfigModel authConfig = new AuthenticatorConfigModel();
-                            authConfig.setId(UUID.randomUUID().toString());
-                            authConfig.setAlias("cfg" + authConfig.getId().hashCode());
-                            authConfig.setConfig(defaultIdpConfig);
-
-                            session.getContext().getRealm().addAuthenticatorConfig(authConfig);
-
-                            config.setAuthenticatorConfig(authConfig.getId());
-                        }
-                    )
-                .defineAsBrowserFlow();
-            }
-        );
     }
 
     @Override

themes/src/main/resources/theme/base/account/messages/messages_en.properties

@@ -214,6 +214,7 @@ identityProviderRemovedMessage=Identity provider removed successfully.
 identityProviderAlreadyLinkedMessage=Federated identity returned by {0} is already linked to another user.
 staleCodeAccountMessage=The page expired. Please try one more time.
 consentDenied=Consent denied.
+access-denied-when-idp-auth=Access denied when authenticating with {0}
 
 accountDisabledMessage=Account is disabled, contact your administrator.
 

themes/src/main/resources/theme/base/login/messages/messages_en.properties

@@ -500,6 +500,7 @@ accountUnusable=Any subsequent use of the application will not be possible with
 userDeletedSuccessfully=User deleted successfully
 
 access-denied=Access denied
+access-denied-when-idp-auth=Access denied when authenticating with {0}
 
 frontchannel-logout.title=Logging out
 frontchannel-logout.message=You are logging out from following apps