KEYCLOAK-1035 Brokered identity linked by account management may not be recognized during login
This commit is contained in:
parent
d59a28f1cb
commit
90496f62ac
6 changed files with 122 additions and 31 deletions
|
@ -49,7 +49,7 @@ public class AccountFederatedIdentityBean {
|
|||
.queryParam("stateChecker", stateChecker)
|
||||
.build().toString();
|
||||
|
||||
FederatedIdentityEntry entry = new FederatedIdentityEntry(identity, provider.getName(), actionUrl);
|
||||
FederatedIdentityEntry entry = new FederatedIdentityEntry(identity, provider.getId(), provider.getName(), actionUrl);
|
||||
this.identities.add(entry);
|
||||
}
|
||||
}
|
||||
|
@ -78,17 +78,19 @@ public class AccountFederatedIdentityBean {
|
|||
public class FederatedIdentityEntry {
|
||||
|
||||
private FederatedIdentityModel federatedIdentityModel;
|
||||
private final String providerId;
|
||||
private final String providerName;
|
||||
private final String actionUrl;
|
||||
|
||||
public FederatedIdentityEntry(FederatedIdentityModel federatedIdentityModel, String providerName, String actionUrl) {
|
||||
public FederatedIdentityEntry(FederatedIdentityModel federatedIdentityModel, String providerId, String providerName, String actionUrl) {
|
||||
this.federatedIdentityModel = federatedIdentityModel;
|
||||
this.providerId = providerId;
|
||||
this.providerName = providerName;
|
||||
this.actionUrl = actionUrl;
|
||||
}
|
||||
|
||||
public String getProviderId() {
|
||||
return federatedIdentityModel != null ? federatedIdentityModel.getIdentityProvider() : null;
|
||||
return providerId;
|
||||
}
|
||||
|
||||
public String getProviderName() {
|
||||
|
|
|
@ -19,10 +19,10 @@
|
|||
<div class="col-sm-5 col-md-5">
|
||||
<#if identity.connected>
|
||||
<#if federatedIdentity.removeLinkPossible>
|
||||
<a href="${identity.actionUrl}" type="submit" class="btn btn-primary btn-lg">Remove ${identity.providerName!}</a>
|
||||
<a href="${identity.actionUrl}" type="submit" id="remove-${identity.providerId!}" class="btn btn-primary btn-lg">Remove ${identity.providerName!}</a>
|
||||
</#if>
|
||||
<#else>
|
||||
<a href="${identity.actionUrl}" type="submit" class="btn btn-primary btn-lg">Add ${identity.providerName!}</a>
|
||||
<a href="${identity.actionUrl}" type="submit" id="add-${identity.providerId!}" class="btn btn-primary btn-lg">Add ${identity.providerName!}</a>
|
||||
</#if>
|
||||
</div>
|
||||
</div>
|
||||
|
|
|
@ -17,6 +17,7 @@
|
|||
*/
|
||||
package org.keycloak.services.resources;
|
||||
|
||||
import org.jboss.logging.Logger;
|
||||
import org.jboss.resteasy.spi.HttpRequest;
|
||||
import org.keycloak.ClientConnection;
|
||||
import org.keycloak.broker.provider.AuthenticationRequest;
|
||||
|
@ -77,6 +78,8 @@ import static org.keycloak.models.UserModel.RequiredAction.UPDATE_PROFILE;
|
|||
@Path("/broker")
|
||||
public class AuthenticationBrokerResource {
|
||||
|
||||
private static final Logger logger = Logger.getLogger(AuthenticationBrokerResource.class);
|
||||
|
||||
@Context
|
||||
private UriInfo uriInfo;
|
||||
|
||||
|
@ -132,6 +135,7 @@ public class AuthenticationBrokerResource {
|
|||
return response;
|
||||
}
|
||||
} catch (Exception e) {
|
||||
logger.error("Could not send authentication request to identity provider " + providerId, e);
|
||||
String message = "Could not send authentication request to identity provider";
|
||||
event.error(message);
|
||||
return redirectToErrorPage(realm, message);
|
||||
|
@ -278,6 +282,7 @@ public class AuthenticationBrokerResource {
|
|||
|
||||
return performLocalAuthentication(realm, providerId, identity, clientCode);
|
||||
} catch (Exception e) {
|
||||
logger.error("Could not authenticate user against provider " + providerId, e);
|
||||
if (session.getTransaction().isActive()) {
|
||||
session.getTransaction().rollback();
|
||||
}
|
||||
|
@ -333,34 +338,36 @@ public class AuthenticationBrokerResource {
|
|||
return Response.status(302).location(UriBuilder.fromUri(clientSession.getRedirectUri()).build()).build();
|
||||
}
|
||||
|
||||
UserModel user = session.users().getUserByEmail(updatedIdentity.getEmail(), realm);
|
||||
String errorMessage = "federatedIdentityEmailExists";
|
||||
if (federatedUser == null) {
|
||||
|
||||
if (user == null) {
|
||||
user = session.users().getUserByUsername(updatedIdentity.getUsername(), realm);
|
||||
errorMessage = "federatedIdentityUsernameExists";
|
||||
}
|
||||
UserModel existingUser = session.users().getUserByEmail(updatedIdentity.getEmail(), realm);
|
||||
String errorMessage = "federatedIdentityEmailExists";
|
||||
|
||||
if (user == null) {
|
||||
federatedUser = session.users().addUser(realm, updatedIdentity.getUsername());
|
||||
federatedUser.setEnabled(true);
|
||||
federatedUser.setFirstName(updatedIdentity.getFirstName());
|
||||
federatedUser.setLastName(updatedIdentity.getLastName());
|
||||
federatedUser.setEmail(updatedIdentity.getEmail());
|
||||
|
||||
session.users().addFederatedIdentity(realm, federatedUser, federatedIdentityModel);
|
||||
|
||||
event.clone().user(federatedUser).event(EventType.REGISTER)
|
||||
.detail(Details.REGISTER_METHOD, authMethod)
|
||||
.detail(Details.EMAIL, federatedUser.getEmail())
|
||||
.removeDetail("auth_method")
|
||||
.success();
|
||||
|
||||
if (identityProviderConfig.isUpdateProfileFirstLogin()) {
|
||||
federatedUser.addRequiredAction(UPDATE_PROFILE);
|
||||
if (existingUser == null) {
|
||||
existingUser = session.users().getUserByUsername(updatedIdentity.getUsername(), realm);
|
||||
errorMessage = "federatedIdentityUsernameExists";
|
||||
}
|
||||
} else {
|
||||
if (federatedUser == null) {
|
||||
|
||||
if (existingUser == null) {
|
||||
logger.debug("Creating user " + updatedIdentity.getUsername() + " and linking to federation provider " + providerId);
|
||||
federatedUser = session.users().addUser(realm, updatedIdentity.getUsername());
|
||||
federatedUser.setEnabled(true);
|
||||
federatedUser.setFirstName(updatedIdentity.getFirstName());
|
||||
federatedUser.setLastName(updatedIdentity.getLastName());
|
||||
federatedUser.setEmail(updatedIdentity.getEmail());
|
||||
|
||||
session.users().addFederatedIdentity(realm, federatedUser, federatedIdentityModel);
|
||||
|
||||
event.clone().user(federatedUser).event(EventType.REGISTER)
|
||||
.detail(Details.REGISTER_METHOD, authMethod)
|
||||
.detail(Details.EMAIL, federatedUser.getEmail())
|
||||
.removeDetail("auth_method")
|
||||
.success();
|
||||
|
||||
if (identityProviderConfig.isUpdateProfileFirstLogin()) {
|
||||
federatedUser.addRequiredAction(UPDATE_PROFILE);
|
||||
}
|
||||
} else {
|
||||
return Flows.forms(session, realm, clientSession.getClient(), uriInfo)
|
||||
.setClientSessionCode(clientCode.getCode())
|
||||
.setError(errorMessage)
|
||||
|
|
|
@ -33,6 +33,7 @@ import org.keycloak.representations.IDToken;
|
|||
import org.keycloak.testsuite.OAuthClient;
|
||||
import org.keycloak.testsuite.OAuthClient.AccessTokenResponse;
|
||||
import org.keycloak.testsuite.broker.util.UserSessionStatusServlet.UserSessionStatus;
|
||||
import org.keycloak.testsuite.pages.AccountFederatedIdentityPage;
|
||||
import org.keycloak.testsuite.pages.AccountPasswordPage;
|
||||
import org.keycloak.testsuite.pages.LoginPage;
|
||||
import org.keycloak.testsuite.pages.LoginUpdateProfilePage;
|
||||
|
@ -92,6 +93,9 @@ public abstract class AbstractIdentityProviderTest {
|
|||
@WebResource
|
||||
protected AccountPasswordPage changePasswordPage;
|
||||
|
||||
@WebResource
|
||||
protected AccountFederatedIdentityPage accountFederatedIdentityPage;
|
||||
|
||||
private KeycloakSession session;
|
||||
|
||||
@Before
|
||||
|
@ -118,6 +122,7 @@ public abstract class AbstractIdentityProviderTest {
|
|||
@Test
|
||||
public void testSuccessfulAuthenticationWithoutUpdateProfile() {
|
||||
IdentityProviderModel identityProviderModel = getIdentityProviderModel();
|
||||
identityProviderModel.setUpdateProfileFirstLogin(false);
|
||||
|
||||
assertSuccessfulAuthentication(identityProviderModel);
|
||||
}
|
||||
|
@ -202,6 +207,38 @@ public abstract class AbstractIdentityProviderTest {
|
|||
assertEquals("User with email already exists. Please login to account management to link the account.", element.getText());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testAccountManagementLinkIdentity() {
|
||||
// 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());
|
||||
|
||||
// Link my "pedroigor" identity with "test-user" from brokered Keycloak
|
||||
IdentityProviderModel identityProviderModel = getIdentityProviderModel();
|
||||
accountFederatedIdentityPage.clickAddProvider(identityProviderModel.getId());
|
||||
|
||||
assertTrue(this.driver.getCurrentUrl().startsWith("http://localhost:8082/auth/"));
|
||||
this.loginPage.login("test-user", "password");
|
||||
doAfterProviderAuthentication();
|
||||
|
||||
// Assert identity linked in account management
|
||||
assertTrue(accountFederatedIdentityPage.isCurrent());
|
||||
assertTrue(driver.getPageSource().contains("id=\"remove-" + identityProviderModel.getId() + "\""));
|
||||
|
||||
// Logout from account management
|
||||
accountFederatedIdentityPage.logout();
|
||||
assertTrue(driver.getTitle().equals("Log in to realm-with-broker"));
|
||||
|
||||
// Assert I am logged immediately to account management
|
||||
loginPage.clickSocial(identityProviderModel.getId());
|
||||
doAfterProviderAuthentication();
|
||||
assertTrue(accountFederatedIdentityPage.isCurrent());
|
||||
assertTrue(driver.getPageSource().contains("id=\"remove-" + identityProviderModel.getId() + "\""));
|
||||
}
|
||||
|
||||
@Test(expected = NoSuchElementException.class)
|
||||
public void testIdentityProviderNotAllowed() {
|
||||
this.driver.navigate().to("http://localhost:8081/test-app/");
|
||||
|
|
|
@ -0,0 +1,42 @@
|
|||
package org.keycloak.testsuite.pages;
|
||||
|
||||
import javax.ws.rs.core.UriBuilder;
|
||||
|
||||
import org.keycloak.services.resources.flows.Urls;
|
||||
import org.keycloak.testsuite.Constants;
|
||||
import org.openqa.selenium.By;
|
||||
|
||||
/**
|
||||
* @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
|
||||
*/
|
||||
public class AccountFederatedIdentityPage extends AbstractAccountPage {
|
||||
|
||||
public AccountFederatedIdentityPage() {};
|
||||
|
||||
private String realmName = "test";
|
||||
|
||||
public void open() {
|
||||
driver.navigate().to(getPath());
|
||||
}
|
||||
|
||||
public void realm(String realmName) {
|
||||
this.realmName = realmName;
|
||||
}
|
||||
|
||||
public String getPath() {
|
||||
return Urls.accountFederatedIdentityPage(UriBuilder.fromUri(Constants.AUTH_SERVER_ROOT).build(), realmName).toString();
|
||||
}
|
||||
|
||||
@Override
|
||||
public boolean isCurrent() {
|
||||
return driver.getTitle().contains("Account Management") && driver.getPageSource().contains("Federated Identities");
|
||||
}
|
||||
|
||||
public void clickAddProvider(String providerId) {
|
||||
driver.findElement(By.id("add-" + providerId)).click();
|
||||
}
|
||||
|
||||
public void clickRemoveProvider(String providerId) {
|
||||
driver.findElement(By.id("remove-" + providerId)).click();
|
||||
}
|
||||
}
|
|
@ -163,7 +163,10 @@
|
|||
{ "type" : "password",
|
||||
"value" : "password" }
|
||||
],
|
||||
"realmRoles": ["manager"]
|
||||
"realmRoles": ["manager"],
|
||||
"applicationRoles": {
|
||||
"account": [ "manage-account" ]
|
||||
}
|
||||
}
|
||||
],
|
||||
"applications": [
|
||||
|
|
Loading…
Reference in a new issue