keycloak-aplcache

KEYCLOAK-1822 Don't redirect to login theme when error during

11/13/2015 2:37:08 PM

Details

diff --git a/forms/common-themes/src/main/resources/theme/base/account/messages/messages_en.properties b/forms/common-themes/src/main/resources/theme/base/account/messages/messages_en.properties
index 3d2eeea..4980104 100755
--- a/forms/common-themes/src/main/resources/theme/base/account/messages/messages_en.properties
+++ b/forms/common-themes/src/main/resources/theme/base/account/messages/messages_en.properties
@@ -133,6 +133,7 @@ federatedIdentityLinkNotActiveMessage=This identity is not active anymore.
 federatedIdentityRemovingLastProviderMessage=You can''t remove last federated identity as you don''t have password.
 identityProviderRedirectErrorMessage=Failed to redirect to identity provider.
 identityProviderRemovedMessage=Identity provider removed successfully.
+identityProviderAlreadyLinkedMessage=Federated identity returned by {0} is already linked to another user.
 
 accountDisabledMessage=Account is disabled, contact admin.
 
diff --git a/forms/common-themes/src/main/resources/theme/base/login/messages/messages_en.properties b/forms/common-themes/src/main/resources/theme/base/login/messages/messages_en.properties
index 803ef2d..6885308 100644
--- a/forms/common-themes/src/main/resources/theme/base/login/messages/messages_en.properties
+++ b/forms/common-themes/src/main/resources/theme/base/login/messages/messages_en.properties
@@ -185,7 +185,6 @@ resetCredentialNotAllowedMessage=Reset Credential not allowed
 
 permissionNotApprovedMessage=Permission not approved.
 noRelayStateInResponseMessage=No relay state in response from identity provider.
-identityProviderAlreadyLinkedMessage=The identity returned by the identity provider is already linked to another user.
 insufficientPermissionMessage=Insufficient permissions to link identities.
 couldNotProceedWithAuthenticationRequestMessage=Could not proceed with authentication request to identity provider.
 couldNotObtainTokenMessage=Could not obtain token from identity provider.
diff --git a/model/api/src/main/java/org/keycloak/models/utils/FormMessage.java b/model/api/src/main/java/org/keycloak/models/utils/FormMessage.java
index b840de6..9f5c5de 100755
--- a/model/api/src/main/java/org/keycloak/models/utils/FormMessage.java
+++ b/model/api/src/main/java/org/keycloak/models/utils/FormMessage.java
@@ -23,6 +23,9 @@ public class FormMessage {
 	private String message;
 	private Object[] parameters;
 
+	public FormMessage() {
+	}
+
 	/**
 	 * Create message.
 	 * 
diff --git a/services/src/main/java/org/keycloak/services/resources/AccountService.java b/services/src/main/java/org/keycloak/services/resources/AccountService.java
index 2bebec3..af24034 100755
--- a/services/src/main/java/org/keycloak/services/resources/AccountService.java
+++ b/services/src/main/java/org/keycloak/services/resources/AccountService.java
@@ -61,6 +61,7 @@ import org.keycloak.services.messages.Messages;
 import org.keycloak.services.util.ResolveRelative;
 import org.keycloak.services.validation.Validation;
 import org.keycloak.common.util.UriUtils;
+import org.keycloak.util.JsonSerialization;
 
 import javax.ws.rs.Consumes;
 import javax.ws.rs.GET;
@@ -74,6 +75,8 @@ import javax.ws.rs.core.Response;
 import javax.ws.rs.core.UriBuilder;
 import javax.ws.rs.core.UriInfo;
 import javax.ws.rs.core.Variant;
+
+import java.io.IOException;
 import java.lang.reflect.Method;
 import java.net.URI;
 import java.util.HashSet;
@@ -116,6 +119,9 @@ public class AccountService extends AbstractSecuredLocalService {
 
     public static final String KEYCLOAK_STATE_CHECKER = "KEYCLOAK_STATE_CHECKER";
 
+    // Used when some other context (ie. IdentityBrokerService) wants to forward error to account management and display it here
+    public static final String ACCOUNT_MGMT_FORWARDED_ERROR_NOTE = "ACCOUNT_MGMT_FORWARDED_ERROR";
+
     private final AppAuthManager authManager;
     private EventBuilder event;
     private AccountProvider account;
@@ -217,6 +223,17 @@ public class AccountService extends AbstractSecuredLocalService {
 
             setReferrerOnPage();
 
+            String forwardedError = auth.getClientSession().getNote(ACCOUNT_MGMT_FORWARDED_ERROR_NOTE);
+            if (forwardedError != null) {
+                try {
+                    FormMessage errorMessage = JsonSerialization.readValue(forwardedError, FormMessage.class);
+                    account.setError(errorMessage.getMessage(), errorMessage.getParameters());
+                    auth.getClientSession().removeNote(ACCOUNT_MGMT_FORWARDED_ERROR_NOTE);
+                } catch (IOException ioe) {
+                    throw new RuntimeException(ioe);
+                }
+            }
+
             return account.createResponse(page);
         } else {
             return login(path);
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 c8784bd..5912536 100755
--- a/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java
+++ b/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java
@@ -65,6 +65,7 @@ import org.keycloak.services.Urls;
 import org.keycloak.services.validation.Validation;
 import org.keycloak.social.SocialIdentityProvider;
 import org.keycloak.common.util.ObjectUtil;
+import org.keycloak.util.JsonSerialization;
 
 import javax.ws.rs.*;
 import javax.ws.rs.core.Context;
@@ -74,6 +75,7 @@ import javax.ws.rs.core.Response.Status;
 import javax.ws.rs.core.UriBuilder;
 import javax.ws.rs.core.UriInfo;
 
+import java.io.IOException;
 import java.net.URI;
 import java.util.ArrayList;
 import java.util.HashMap;
@@ -419,7 +421,6 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal
                 return finishBrokerAuthentication(context, federatedUser, clientSession, providerId);
             }
         }  catch (Exception e) {
-            // TODO?
             return redirectToErrorPage(Messages.IDENTITY_PROVIDER_UNEXPECTED_ERROR, e);
         }
     }
@@ -465,7 +466,7 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal
         this.event.event(EventType.FEDERATED_IDENTITY_LINK);
 
         if (federatedUser != null) {
-            return redirectToErrorPage(Messages.IDENTITY_PROVIDER_ALREADY_LINKED, context.getIdpConfig().getAlias());
+            return redirectToAccountErrorPage(clientSession, Messages.IDENTITY_PROVIDER_ALREADY_LINKED, context.getIdpConfig().getAlias());
         }
 
         UserModel authenticatedUser = clientSession.getUserSession().getUser();
@@ -475,12 +476,10 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal
         }
 
         if (!authenticatedUser.isEnabled()) {
-            fireErrorEvent(Errors.USER_DISABLED);
-            return redirectToErrorPage(Messages.ACCOUNT_DISABLED);
+            return redirectToAccountErrorPage(clientSession, Messages.ACCOUNT_DISABLED);
         }
 
         if (!authenticatedUser.hasRole(this.realmModel.getClientByClientId(ACCOUNT_MANAGEMENT_CLIENT_ID).getRole(MANAGE_ACCOUNT))) {
-            fireErrorEvent(Errors.NOT_ALLOWED);
             return redirectToErrorPage(Messages.INSUFFICIENT_PERMISSION);
         }
 
@@ -581,6 +580,20 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal
         return ErrorPage.error(this.session, message, parameters);
     }
 
+    private Response redirectToAccountErrorPage(ClientSessionModel clientSession, String message, Object ... parameters) {
+        fireErrorEvent(message);
+
+        FormMessage errorMessage = new FormMessage(message, parameters);
+        try {
+            String serializedError = JsonSerialization.writeValueAsString(errorMessage);
+            clientSession.setNote(AccountService.ACCOUNT_MGMT_FORWARDED_ERROR_NOTE, serializedError);
+        } catch (IOException ioe) {
+            throw new RuntimeException(ioe);
+        }
+
+        return Response.status(302).location(UriBuilder.fromUri(clientSession.getRedirectUri()).build()).build();
+    }
+
     private Response redirectToLoginPage(Throwable t, ClientSessionCode clientCode) {
         String message = t.getMessage();
 
diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractKeycloakIdentityProviderTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractKeycloakIdentityProviderTest.java
index 904caf6..1d2e5b6 100644
--- a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractKeycloakIdentityProviderTest.java
+++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractKeycloakIdentityProviderTest.java
@@ -389,6 +389,34 @@ public abstract class AbstractKeycloakIdentityProviderTest extends AbstractIdent
         this.updateProfilePage.assertCurrent();
     }
 
+
+    // KEYCLOAK-1822
+    @Test
+    public void testAccountManagementLinkedIdentityAlreadyExists() {
+        // Login as "test-user" through broker
+        IdentityProviderModel identityProvider = getIdentityProviderModel();
+        assertSuccessfulAuthentication(identityProvider, "test-user", "test-user@localhost", false);
+
+        // Login as pedroigor to account management
+        accountFederatedIdentityPage.realm("realm-with-broker");
+        accountFederatedIdentityPage.open();
+        assertTrue(driver.getTitle().equals("Log in to realm-with-broker"));
+        loginPage.login("pedroigor", "password");
+        assertTrue(accountFederatedIdentityPage.isCurrent());
+
+        // Try to link my "pedroigor" identity with "test-user" from brokered Keycloak.
+        accountFederatedIdentityPage.clickAddProvider(identityProvider.getAlias());
+
+        assertTrue(this.driver.getCurrentUrl().startsWith("http://localhost:8082/auth/"));
+        this.loginPage.login("test-user", "password");
+        doAfterProviderAuthentication();
+
+        // Error is displayed in account management because federated identity"test-user" already linked to local account "test-user"
+        assertTrue(accountFederatedIdentityPage.isCurrent());
+        assertEquals("Federated identity returned by " + getProviderId() + " is already linked to another user.", accountFederatedIdentityPage.getError());
+    }
+
+
     @Test(expected = NoSuchElementException.class)
     public void testIdentityProviderNotAllowed() {
         this.driver.navigate().to("http://localhost:8081/test-app/");
diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/pages/AccountFederatedIdentityPage.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/pages/AccountFederatedIdentityPage.java
index 4c02798..54a5cbb 100644
--- a/testsuite/integration/src/test/java/org/keycloak/testsuite/pages/AccountFederatedIdentityPage.java
+++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/pages/AccountFederatedIdentityPage.java
@@ -5,12 +5,17 @@ import javax.ws.rs.core.UriBuilder;
 import org.keycloak.services.Urls;
 import org.keycloak.testsuite.Constants;
 import org.openqa.selenium.By;
+import org.openqa.selenium.WebElement;
+import org.openqa.selenium.support.FindBy;
 
 /**
  * @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
  */
 public class AccountFederatedIdentityPage extends AbstractAccountPage {
 
+    @FindBy(className = "alert-error")
+    private WebElement errorMessage;
+
     public AccountFederatedIdentityPage() {};
 
     private String realmName = "test";
@@ -39,4 +44,8 @@ public class AccountFederatedIdentityPage extends AbstractAccountPage {
     public void clickRemoveProvider(String providerId) {
         driver.findElement(By.id("remove-" + providerId)).click();
     }
+
+    public String getError() {
+        return errorMessage.getText();
+    }
 }