KEYCLOAK-2802 NPE during identity broker cancelled from account mgmt

This commit is contained in:
mposolda 2016-04-11 23:28:16 +02:00
parent 98ad9b7e7c
commit e4f75409c9
5 changed files with 83 additions and 42 deletions

View file

@ -17,7 +17,6 @@
package org.keycloak.admin.client.resource;
import org.jboss.resteasy.annotations.cache.NoCache;
import org.keycloak.representations.idm.CredentialRepresentation;
import org.keycloak.representations.idm.FederatedIdentityRepresentation;
import org.keycloak.representations.idm.GroupRepresentation;

View file

@ -543,6 +543,11 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal
}
ClientSessionCode clientCode = parsedCode.clientSessionCode;
Response accountManagementFailedLinking = checkAccountManagementFailedLinking(clientCode.getClientSession(), Messages.CONSENT_DENIED);
if (accountManagementFailedLinking != null) {
return accountManagementFailedLinking;
}
return browserAuthentication(clientCode.getClientSession(), null);
}
@ -554,6 +559,11 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal
}
ClientSessionCode clientCode = parsedCode.clientSessionCode;
Response accountManagementFailedLinking = checkAccountManagementFailedLinking(clientCode.getClientSession(), message);
if (accountManagementFailedLinking != null) {
return accountManagementFailedLinking;
}
return browserAuthentication(clientCode.getClientSession(), message);
}
@ -635,20 +645,10 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal
if (!clientCode.isValid(AUTHENTICATE.name(), ClientSessionCode.ActionType.LOGIN)) {
logger.debugf("Authorization code is not valid. Client session ID: %s, Client session's action: %s", clientSession.getId(), clientSession.getAction());
Response staleCodeError;
// Check if error happened during login or during linking from account management
Response accountManagementFailedLinking = checkAccountManagementFailedLinking(clientCode.getClientSession(), Messages.STALE_CODE_ACCOUNT);
Response staleCodeError = (accountManagementFailedLinking != null) ? accountManagementFailedLinking : redirectToErrorPage(Messages.STALE_CODE);
// Linking identityProvider from account mgmt
if (clientSession.getUserSession() != null && client.getClientId().equals(ACCOUNT_MANAGEMENT_CLIENT_ID)) {
this.event.event(EventType.FEDERATED_IDENTITY_LINK);
UserModel user = clientSession.getUserSession().getUser();
this.event.user(user);
this.event.detail(Details.USERNAME, user.getUsername());
staleCodeError = redirectToAccountErrorPage(clientSession, Messages.STALE_CODE_ACCOUNT);
} else {
staleCodeError = redirectToErrorPage(Messages.STALE_CODE);
}
return ParsedCodeContext.response(staleCodeError);
}
@ -666,6 +666,20 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal
return ParsedCodeContext.response(staleCodeError);
}
private Response checkAccountManagementFailedLinking(ClientSessionModel clientSession, String error, Object... parameters) {
if (clientSession.getUserSession() != null && clientSession.getClient() != null && clientSession.getClient().getClientId().equals(ACCOUNT_MANAGEMENT_CLIENT_ID)) {
this.event.event(EventType.FEDERATED_IDENTITY_LINK);
UserModel user = clientSession.getUserSession().getUser();
this.event.user(user);
this.event.detail(Details.USERNAME, user.getUsername());
return redirectToAccountErrorPage(clientSession, error, parameters);
} else {
return null;
}
}
private AuthenticationRequest createAuthenticationRequest(String providerId, ClientSessionCode clientSessionCode) {
ClientSessionModel clientSession = null;
String relayState = null;

View file

@ -17,39 +17,29 @@
package org.keycloak.testsuite.broker;
import org.junit.Assert;
import org.junit.ClassRule;
import org.junit.Test;
import org.keycloak.admin.client.Keycloak;
import org.keycloak.admin.client.resource.RealmResource;
import org.keycloak.common.util.Time;
import org.keycloak.models.IdentityProviderModel;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.RealmModel;
import org.keycloak.models.UserModel;
import org.keycloak.representations.AccessTokenResponse;
import org.keycloak.representations.idm.ClientRepresentation;
import org.keycloak.representations.idm.IdentityProviderRepresentation;
import org.keycloak.representations.idm.RealmRepresentation;
import org.keycloak.services.Urls;
import org.keycloak.services.managers.RealmManager;
import org.keycloak.testsuite.Constants;
import org.keycloak.testsuite.pages.AccountApplicationsPage;
import org.keycloak.testsuite.pages.OAuthGrantPage;
import org.keycloak.testsuite.rule.AbstractKeycloakRule;
import org.keycloak.testsuite.rule.KeycloakRule;
import org.keycloak.testsuite.rule.WebResource;
import org.keycloak.testsuite.KeycloakServer;
import org.keycloak.util.JsonSerialization;
import org.openqa.selenium.NoSuchElementException;
import java.io.IOException;
import java.util.List;
import javax.ws.rs.core.UriBuilder;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
/**

View file

@ -20,15 +20,12 @@ package org.keycloak.testsuite.broker;
import java.util.List;
import org.junit.Assert;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.ClassRule;
import org.junit.Test;
import org.keycloak.admin.client.Keycloak;
import org.keycloak.admin.client.resource.ClientResource;
import org.keycloak.admin.client.resource.RealmResource;
import org.keycloak.common.util.Time;
import org.keycloak.models.IdentityProviderModel;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.RealmModel;
import org.keycloak.representations.idm.ClientRepresentation;
@ -37,9 +34,7 @@ import org.keycloak.representations.idm.RealmRepresentation;
import org.keycloak.representations.idm.UserRepresentation;
import org.keycloak.services.managers.RealmManager;
import org.keycloak.testsuite.KeycloakServer;
import org.keycloak.testsuite.pages.AccountFederatedIdentityPage;
import org.keycloak.testsuite.rule.AbstractKeycloakRule;
import org.keycloak.testsuite.rule.WebResource;
import org.openqa.selenium.NoSuchElementException;
import static org.junit.Assert.assertTrue;
@ -175,11 +170,7 @@ public class OIDCKeycloakServerBrokerWithConsentTest extends AbstractIdentityPro
@Test
public void testAccountManagementLinkingAndExpiredClientSession() throws Exception {
// 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());
loginToAccountManagement("pedroigor");
// Link my "pedroigor" identity with "test-user" from brokered Keycloak
accountFederatedIdentityPage.clickAddProvider(getProviderId());
@ -196,7 +187,7 @@ public class OIDCKeycloakServerBrokerWithConsentTest extends AbstractIdentityPro
// Assert account error page with "staleCodeAccount" error displayed
accountFederatedIdentityPage.assertCurrent();
Assert.assertEquals("The page expired. Please try one more time", accountFederatedIdentityPage.getError());
Assert.assertEquals("The page expired. Please try one more time.", accountFederatedIdentityPage.getError());
// Try to link one more time
@ -213,15 +204,61 @@ public class OIDCKeycloakServerBrokerWithConsentTest extends AbstractIdentityPro
// Assert account error page with "staleCodeAccount" error displayed
accountFederatedIdentityPage.assertCurrent();
Assert.assertEquals("The page expired. Please try one more time", accountFederatedIdentityPage.getError());
Assert.assertEquals("The page expired. Please try one more time.", accountFederatedIdentityPage.getError());
} finally {
Time.setOffset(0);
// Revoke consent
RealmResource brokeredRealm = keycloak2.realm("realm-with-oidc-identity-provider");
List<UserRepresentation> users = brokeredRealm.users().search("test-user", 0, 1);
brokeredRealm.users().get(users.get(0).getId()).revokeConsent("broker-app");
}
// Revoke consent
RealmResource brokeredRealm = keycloak2.realm("realm-with-oidc-identity-provider");
List<UserRepresentation> users = brokeredRealm.users().search("test-user", 0, 1);
brokeredRealm.users().get(users.get(0).getId()).revokeConsent("broker-app");
}
@Test
public void testLoginCancelConsent() throws Exception {
// Try to login
loginIDP("test-user");
// User rejected consent
grantPage.assertCurrent();
grantPage.cancel();
// Assert back on login page
assertTrue(this.driver.getCurrentUrl().startsWith("http://localhost:8081/auth/"));
assertTrue(driver.getTitle().equals("Log in to realm-with-broker"));
}
// KEYCLOAK-2802
@Test
public void testAccountManagementLinkingCancelConsent() throws Exception {
// Login as pedroigor to account management
loginToAccountManagement("pedroigor");
// Link my "pedroigor" identity with "test-user" from brokered Keycloak
accountFederatedIdentityPage.clickAddProvider(getProviderId());
assertTrue(this.driver.getCurrentUrl().startsWith("http://localhost:8082/auth/"));
this.loginPage.login("test-user", "password");
// User rejected consent
grantPage.assertCurrent();
grantPage.cancel();
// Assert account error page with "consentDenied" error displayed
accountFederatedIdentityPage.assertCurrent();
Assert.assertEquals("Consent denied.", accountFederatedIdentityPage.getError());
}
private void loginToAccountManagement(String username) {
accountFederatedIdentityPage.realm("realm-with-broker");
accountFederatedIdentityPage.open();
assertTrue(driver.getTitle().equals("Log in to realm-with-broker"));
loginPage.login(username, "password");
assertTrue(accountFederatedIdentityPage.isCurrent());
}
}

View file

@ -135,7 +135,8 @@ federatedIdentityRemovingLastProviderMessage=You can''t remove last federated id
identityProviderRedirectErrorMessage=Failed to redirect to identity provider.
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
staleCodeAccountMessage=The page expired. Please try one more time.
consentDenied=Consent denied.
accountDisabledMessage=Account is disabled, contact admin.