KEYCLOAK-1822 Don't redirect to login theme when error during link identity in account mgmt.

This commit is contained in:
mposolda 2015-11-13 17:37:08 +01:00
parent 600d1f8a56
commit 4288260aa6
7 changed files with 76 additions and 6 deletions

View file

@ -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. federatedIdentityRemovingLastProviderMessage=You can''t remove last federated identity as you don''t have password.
identityProviderRedirectErrorMessage=Failed to redirect to identity provider. identityProviderRedirectErrorMessage=Failed to redirect to identity provider.
identityProviderRemovedMessage=Identity provider removed successfully. identityProviderRemovedMessage=Identity provider removed successfully.
identityProviderAlreadyLinkedMessage=Federated identity returned by {0} is already linked to another user.
accountDisabledMessage=Account is disabled, contact admin. accountDisabledMessage=Account is disabled, contact admin.

View file

@ -185,7 +185,6 @@ resetCredentialNotAllowedMessage=Reset Credential not allowed
permissionNotApprovedMessage=Permission not approved. permissionNotApprovedMessage=Permission not approved.
noRelayStateInResponseMessage=No relay state in response from identity provider. 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. insufficientPermissionMessage=Insufficient permissions to link identities.
couldNotProceedWithAuthenticationRequestMessage=Could not proceed with authentication request to identity provider. couldNotProceedWithAuthenticationRequestMessage=Could not proceed with authentication request to identity provider.
couldNotObtainTokenMessage=Could not obtain token from identity provider. couldNotObtainTokenMessage=Could not obtain token from identity provider.

View file

@ -23,6 +23,9 @@ public class FormMessage {
private String message; private String message;
private Object[] parameters; private Object[] parameters;
public FormMessage() {
}
/** /**
* Create message. * Create message.
* *

View file

@ -61,6 +61,7 @@ import org.keycloak.services.messages.Messages;
import org.keycloak.services.util.ResolveRelative; import org.keycloak.services.util.ResolveRelative;
import org.keycloak.services.validation.Validation; import org.keycloak.services.validation.Validation;
import org.keycloak.common.util.UriUtils; import org.keycloak.common.util.UriUtils;
import org.keycloak.util.JsonSerialization;
import javax.ws.rs.Consumes; import javax.ws.rs.Consumes;
import javax.ws.rs.GET; 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.UriBuilder;
import javax.ws.rs.core.UriInfo; import javax.ws.rs.core.UriInfo;
import javax.ws.rs.core.Variant; import javax.ws.rs.core.Variant;
import java.io.IOException;
import java.lang.reflect.Method; import java.lang.reflect.Method;
import java.net.URI; import java.net.URI;
import java.util.HashSet; import java.util.HashSet;
@ -116,6 +119,9 @@ public class AccountService extends AbstractSecuredLocalService {
public static final String KEYCLOAK_STATE_CHECKER = "KEYCLOAK_STATE_CHECKER"; 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 final AppAuthManager authManager;
private EventBuilder event; private EventBuilder event;
private AccountProvider account; private AccountProvider account;
@ -217,6 +223,17 @@ public class AccountService extends AbstractSecuredLocalService {
setReferrerOnPage(); 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); return account.createResponse(page);
} else { } else {
return login(path); return login(path);

View file

@ -65,6 +65,7 @@ import org.keycloak.services.Urls;
import org.keycloak.services.validation.Validation; import org.keycloak.services.validation.Validation;
import org.keycloak.social.SocialIdentityProvider; import org.keycloak.social.SocialIdentityProvider;
import org.keycloak.common.util.ObjectUtil; import org.keycloak.common.util.ObjectUtil;
import org.keycloak.util.JsonSerialization;
import javax.ws.rs.*; import javax.ws.rs.*;
import javax.ws.rs.core.Context; 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.UriBuilder;
import javax.ws.rs.core.UriInfo; import javax.ws.rs.core.UriInfo;
import java.io.IOException;
import java.net.URI; import java.net.URI;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.HashMap; import java.util.HashMap;
@ -419,7 +421,6 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal
return finishBrokerAuthentication(context, federatedUser, clientSession, providerId); return finishBrokerAuthentication(context, federatedUser, clientSession, providerId);
} }
} catch (Exception e) { } catch (Exception e) {
// TODO?
return redirectToErrorPage(Messages.IDENTITY_PROVIDER_UNEXPECTED_ERROR, e); 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); this.event.event(EventType.FEDERATED_IDENTITY_LINK);
if (federatedUser != null) { 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(); UserModel authenticatedUser = clientSession.getUserSession().getUser();
@ -475,12 +476,10 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal
} }
if (!authenticatedUser.isEnabled()) { if (!authenticatedUser.isEnabled()) {
fireErrorEvent(Errors.USER_DISABLED); return redirectToAccountErrorPage(clientSession, Messages.ACCOUNT_DISABLED);
return redirectToErrorPage(Messages.ACCOUNT_DISABLED);
} }
if (!authenticatedUser.hasRole(this.realmModel.getClientByClientId(ACCOUNT_MANAGEMENT_CLIENT_ID).getRole(MANAGE_ACCOUNT))) { if (!authenticatedUser.hasRole(this.realmModel.getClientByClientId(ACCOUNT_MANAGEMENT_CLIENT_ID).getRole(MANAGE_ACCOUNT))) {
fireErrorEvent(Errors.NOT_ALLOWED);
return redirectToErrorPage(Messages.INSUFFICIENT_PERMISSION); return redirectToErrorPage(Messages.INSUFFICIENT_PERMISSION);
} }
@ -581,6 +580,20 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal
return ErrorPage.error(this.session, message, parameters); 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) { private Response redirectToLoginPage(Throwable t, ClientSessionCode clientCode) {
String message = t.getMessage(); String message = t.getMessage();

View file

@ -389,6 +389,34 @@ public abstract class AbstractKeycloakIdentityProviderTest extends AbstractIdent
this.updateProfilePage.assertCurrent(); 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) @Test(expected = NoSuchElementException.class)
public void testIdentityProviderNotAllowed() { public void testIdentityProviderNotAllowed() {
this.driver.navigate().to("http://localhost:8081/test-app/"); this.driver.navigate().to("http://localhost:8081/test-app/");

View file

@ -5,12 +5,17 @@ import javax.ws.rs.core.UriBuilder;
import org.keycloak.services.Urls; import org.keycloak.services.Urls;
import org.keycloak.testsuite.Constants; import org.keycloak.testsuite.Constants;
import org.openqa.selenium.By; 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> * @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
*/ */
public class AccountFederatedIdentityPage extends AbstractAccountPage { public class AccountFederatedIdentityPage extends AbstractAccountPage {
@FindBy(className = "alert-error")
private WebElement errorMessage;
public AccountFederatedIdentityPage() {}; public AccountFederatedIdentityPage() {};
private String realmName = "test"; private String realmName = "test";
@ -39,4 +44,8 @@ public class AccountFederatedIdentityPage extends AbstractAccountPage {
public void clickRemoveProvider(String providerId) { public void clickRemoveProvider(String providerId) {
driver.findElement(By.id("remove-" + providerId)).click(); driver.findElement(By.id("remove-" + providerId)).click();
} }
public String getError() {
return errorMessage.getText();
}
} }