keycloak-aplcache

Details

diff --git a/services/src/main/java/org/keycloak/broker/saml/SAMLEndpoint.java b/services/src/main/java/org/keycloak/broker/saml/SAMLEndpoint.java
index 39a9d2e..38e57cb 100755
--- a/services/src/main/java/org/keycloak/broker/saml/SAMLEndpoint.java
+++ b/services/src/main/java/org/keycloak/broker/saml/SAMLEndpoint.java
@@ -19,6 +19,7 @@ package org.keycloak.broker.saml;
 
 import org.jboss.logging.Logger;
 import org.jboss.resteasy.annotations.cache.NoCache;
+
 import org.keycloak.broker.provider.BrokeredIdentityContext;
 import org.keycloak.broker.provider.IdentityBrokerException;
 import org.keycloak.broker.provider.IdentityProvider;
@@ -78,10 +79,13 @@ import java.security.Key;
 import java.security.cert.X509Certificate;
 import java.util.LinkedList;
 import java.util.List;
+
 import org.keycloak.rotation.HardcodedKeyLocator;
 import org.keycloak.rotation.KeyLocator;
 import org.keycloak.saml.processing.core.util.KeycloakKeySamlExtensionGenerator;
 
+import java.util.*;
+
 /**
  * @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
  * @version $Revision: 1 $
@@ -333,6 +337,13 @@ public class SAMLEndpoint {
 
             try {
                 KeyManager.ActiveRsaKey keys = session.keys().getActiveRsaKey(realm);
+                if (! isSuccessfulSamlResponse(responseType)) {
+                    String statusMessage = responseType.getStatus() == null ? Messages.IDENTITY_PROVIDER_UNEXPECTED_ERROR : responseType.getStatus().getStatusMessage();
+                    return callback.error(relayState, statusMessage);
+                }
+                if (responseType.getAssertions() == null || responseType.getAssertions().isEmpty()) {
+                    return callback.error(relayState, Messages.IDENTITY_PROVIDER_UNEXPECTED_ERROR);
+                }
                 AssertionType assertion = AssertionUtil.getAssertion(responseType, keys.getPrivateKey());
                 SubjectType subject = assertion.getSubject();
                 SubjectType.STSubType subType = subject.getSubType();
@@ -395,6 +406,13 @@ public class SAMLEndpoint {
         }
 
 
+        private boolean isSuccessfulSamlResponse(ResponseType responseType) {
+            return responseType != null
+              && responseType.getStatus() != null
+              && responseType.getStatus().getStatusCode() != null
+              && responseType.getStatus().getStatusCode().getValue() != null
+              && Objects.equals(responseType.getStatus().getStatusCode().getValue().toString(), JBossSAMLURIConstants.STATUS_SUCCESS.get());
+        }
 
 
         public Response handleSamlResponse(String samlResponse, String relayState, String clientId) {
diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/ConsentPage.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/ConsentPage.java
index b8b244d..5110b32 100644
--- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/ConsentPage.java
+++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/ConsentPage.java
@@ -28,10 +28,17 @@ public class ConsentPage extends AbstractPage {
     @FindBy(id = "kc-login")
     private WebElement submitButton;
 
+    @FindBy(id = "kc-cancel")
+    private WebElement cancelButton;
+
     public void confirm() {
         submitButton.click();
     }
 
+    public void cancel() {
+        cancelButton.click();
+    }
+
     @Override
     public boolean isCurrent() {
         return driver.getTitle().equalsIgnoreCase("grant access");
diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractBaseBrokerTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractBaseBrokerTest.java
index 217a4e7..c2c628d 100644
--- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractBaseBrokerTest.java
+++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractBaseBrokerTest.java
@@ -20,10 +20,7 @@ package org.keycloak.testsuite.broker;
 import java.util.List;
 
 import org.jboss.arquillian.graphene.page.Page;
-import org.junit.Before;
-import org.keycloak.admin.client.resource.RealmResource;
 import org.keycloak.representations.idm.RealmRepresentation;
-import org.keycloak.representations.idm.UserRepresentation;
 import org.keycloak.testsuite.AbstractKeycloakTest;
 import org.keycloak.testsuite.Assert;
 import org.keycloak.testsuite.Retry;
@@ -35,8 +32,6 @@ import org.keycloak.testsuite.pages.LoginPage;
 import org.keycloak.testsuite.pages.UpdateAccountInformationPage;
 import org.openqa.selenium.TimeoutException;
 
-import static org.keycloak.testsuite.admin.ApiUtil.createUserWithAdminClient;
-import static org.keycloak.testsuite.admin.ApiUtil.resetUserPassword;
 import static org.keycloak.testsuite.broker.BrokerTestTools.encodeUrl;
 import static org.keycloak.testsuite.broker.BrokerTestTools.waitForPage;
 
diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractBrokerTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractBrokerTest.java
index 8950d1b..6f3314f 100644
--- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractBrokerTest.java
+++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractBrokerTest.java
@@ -2,28 +2,31 @@ package org.keycloak.testsuite.broker;
 
 import org.junit.Before;
 import org.junit.Test;
+
 import org.keycloak.admin.client.resource.RealmResource;
 import org.keycloak.admin.client.resource.UsersResource;
 import org.keycloak.representations.idm.ClientRepresentation;
 import org.keycloak.representations.idm.RealmRepresentation;
 import org.keycloak.representations.idm.UserRepresentation;
 import org.keycloak.testsuite.Assert;
-import org.keycloak.testsuite.util.RealmBuilder;
+import org.keycloak.testsuite.pages.ConsentPage;
+import org.keycloak.testsuite.util.*;
 
 import org.openqa.selenium.TimeoutException;
 
 import java.util.List;
+import java.util.concurrent.TimeUnit;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 import static org.keycloak.testsuite.admin.ApiUtil.createUserWithAdminClient;
 import static org.keycloak.testsuite.admin.ApiUtil.resetUserPassword;
 import static org.keycloak.testsuite.broker.BrokerTestConstants.USER_EMAIL;
-import static org.keycloak.testsuite.broker.BrokerTestTools.*;
 import static org.keycloak.testsuite.util.MailAssert.assertEmailAndGetUrl;
-import org.keycloak.testsuite.util.MailServer;
-import org.keycloak.testsuite.util.MailServerConfiguration;
-import org.keycloak.testsuite.util.UserBuilder;
+
+import org.jboss.arquillian.graphene.page.Page;
+
+import static org.keycloak.testsuite.broker.BrokerTestTools.*;
 
 public abstract class AbstractBrokerTest extends AbstractBaseBrokerTest {
 
@@ -256,6 +259,44 @@ public abstract class AbstractBrokerTest extends AbstractBaseBrokerTest {
         assertEquals("Account is disabled, contact admin.", errorPage.getError());
     }
 
+    @Page
+    ConsentPage consentPage;
+
+    // KEYCLOAK-4181
+    @Test
+    public void loginWithExistingUserWithErrorFromProviderIdP() {
+        ClientRepresentation client = adminClient.realm(bc.providerRealmName())
+          .clients()
+          .findByClientId(bc.getIDPClientIdInProviderRealm(suiteContext))
+          .get(0);
+
+        adminClient.realm(bc.providerRealmName())
+          .clients()
+          .get(client.getId())
+          .update(ClientBuilder.edit(client).consentRequired(true).build());
+
+        driver.navigate().to(getAccountUrl(bc.consumerRealmName()));
+
+        log.debug("Clicking social " + bc.getIDPAlias());
+        accountLoginPage.clickSocial(bc.getIDPAlias());
+
+        waitForPage(driver, "log in to");
+
+        Assert.assertTrue("Driver should be on the provider realm page right now",
+                driver.getCurrentUrl().contains("/auth/realms/" + bc.providerRealmName() + "/"));
+
+        log.debug("Logging in");
+        accountLoginPage.login(bc.getUserLogin(), bc.getUserPassword());
+
+        driver.manage().timeouts().pageLoadTimeout(30, TimeUnit.MINUTES);
+
+        waitForPage(driver, "grant access");
+        consentPage.cancel();
+
+        waitForPage(driver, "log in to");
+    }
+
+
 
     protected void testSingleLogout() {
         log.debug("Testing single log out");
diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/BrokerConfiguration.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/BrokerConfiguration.java
index 0e00a70..5605cf6 100644
--- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/BrokerConfiguration.java
+++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/BrokerConfiguration.java
@@ -43,6 +43,11 @@ public interface BrokerConfiguration {
     String consumerRealmName();
 
     /**
+     * @return Client ID of the identity provider as set in provider realm.
+     */
+    String getIDPClientIdInProviderRealm(SuiteContext suiteContext);
+
+    /**
      * @return User login name of the brokered user
      */
     String getUserLogin();
diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerConfiguration.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerConfiguration.java
index aa3e56e..61664a9 100644
--- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerConfiguration.java
+++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerConfiguration.java
@@ -50,6 +50,7 @@ public class KcOidcBrokerConfiguration implements BrokerConfiguration {
     public List<ClientRepresentation> createProviderClients(SuiteContext suiteContext) {
         ClientRepresentation client = new ClientRepresentation();
         client.setId(CLIENT_ID);
+        client.setClientId(getIDPClientIdInProviderRealm(suiteContext));
         client.setName(CLIENT_ID);
         client.setSecret(CLIENT_SECRET);
         client.setEnabled(true);
@@ -124,6 +125,11 @@ public class KcOidcBrokerConfiguration implements BrokerConfiguration {
     }
 
     @Override
+    public String getIDPClientIdInProviderRealm(SuiteContext suiteContext) {
+        return CLIENT_ID;
+    }
+
+    @Override
     public String getUserPassword() {
         return USER_PASSWORD;
     }
diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlBrokerConfiguration.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlBrokerConfiguration.java
index 7da71ad..e5f5b8d 100644
--- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlBrokerConfiguration.java
+++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlBrokerConfiguration.java
@@ -54,7 +54,7 @@ public class KcSamlBrokerConfiguration implements BrokerConfiguration {
     public List<ClientRepresentation> createProviderClients(SuiteContext suiteContext) {
         ClientRepresentation client = new ClientRepresentation();
 
-        client.setClientId(getAuthRoot(suiteContext) + "/auth/realms/" + REALM_CONS_NAME);
+        client.setClientId(getIDPClientIdInProviderRealm(suiteContext));
         client.setEnabled(true);
         client.setProtocol(IDP_SAML_PROVIDER_ID);
         client.setRedirectUris(Collections.singletonList(
@@ -157,6 +157,11 @@ public class KcSamlBrokerConfiguration implements BrokerConfiguration {
     }
 
     @Override
+    public String getIDPClientIdInProviderRealm(SuiteContext suiteContext) {
+        return getAuthRoot(suiteContext) + "/auth/realms/" + consumerRealmName();
+    }
+
+    @Override
     public String getUserLogin() {
         return USER_LOGIN;
     }
diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlSignedBrokerTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlSignedBrokerTest.java
index cd315f3..b4825cd 100644
--- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlSignedBrokerTest.java
+++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlSignedBrokerTest.java
@@ -15,8 +15,6 @@ public class KcSamlSignedBrokerTest extends KcSamlBrokerTest {
 
     public static class KcSamlSignedBrokerConfiguration extends KcSamlBrokerConfiguration {
 
-        public static final KcSamlSignedBrokerConfiguration INSTANCE = new KcSamlSignedBrokerConfiguration();
-
         @Override
         public RealmRepresentation createProviderRealm() {
             RealmRepresentation realm = super.createProviderRealm();
diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/util/ClientBuilder.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/util/ClientBuilder.java
index 9de5ae2..12fe4a1 100644
--- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/util/ClientBuilder.java
+++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/util/ClientBuilder.java
@@ -61,6 +61,11 @@ public class ClientBuilder {
         return this;
     }
 
+    public ClientBuilder consentRequired(boolean consentRequired) {
+        rep.setConsentRequired(consentRequired);
+        return this;
+    }
+
     public ClientBuilder publicClient() {
         rep.setPublicClient(true);
         return this;