Merge pull request #2579 from mposolda/master
Identity brokering error handling fixes
This commit is contained in:
commit
bed8db7466
6 changed files with 308 additions and 93 deletions
|
@ -35,6 +35,7 @@ import javax.ws.rs.Produces;
|
|||
import javax.ws.rs.core.MediaType;
|
||||
import javax.ws.rs.core.Response;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
|
||||
/**
|
||||
* @author rodrigo.sasaki@icarros.com.br
|
||||
|
@ -132,4 +133,13 @@ public interface UserResource {
|
|||
@Path("role-mappings")
|
||||
public RoleMappingResource roles();
|
||||
|
||||
|
||||
@GET
|
||||
@Path("consents")
|
||||
public List<Map<String, Object>> getConsents();
|
||||
|
||||
@DELETE
|
||||
@Path("consents/{client}")
|
||||
public void revokeConsent(@PathParam("client") String clientId);
|
||||
|
||||
}
|
||||
|
|
|
@ -156,6 +156,8 @@ public class Messages {
|
|||
|
||||
public static final String STALE_CODE = "staleCodeMessage";
|
||||
|
||||
public static final String STALE_CODE_ACCOUNT = "staleCodeAccountMessage";
|
||||
|
||||
public static final String IDENTITY_PROVIDER_NOT_UNIQUE = "identityProviderNotUniqueMessage";
|
||||
|
||||
public static final String REALM_SUPPORTS_NO_CREDENTIALS = "realmSupportsNoCredentialsMessage";
|
||||
|
|
|
@ -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);
|
||||
}
|
||||
|
||||
|
@ -634,7 +644,12 @@ 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 = redirectToErrorPage(Messages.STALE_CODE);
|
||||
|
||||
// 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);
|
||||
|
||||
|
||||
return ParsedCodeContext.response(staleCodeError);
|
||||
}
|
||||
|
||||
|
@ -651,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;
|
||||
|
|
|
@ -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;
|
||||
|
||||
/**
|
||||
|
@ -146,88 +136,6 @@ public class OIDCKeyCloakServerBrokerBasicTest extends AbstractKeycloakIdentityP
|
|||
keycloak.realm("realm-with-broker").identityProviders().get("kc-oidc-idp").update(idp);
|
||||
}
|
||||
|
||||
|
||||
@Test
|
||||
public void testConsentDeniedWithExpiredClientSession() throws Exception {
|
||||
// Disable update profile
|
||||
setUpdateProfileFirstLogin(IdentityProviderRepresentation.UPFLM_OFF);
|
||||
|
||||
Keycloak keycloak1 = Keycloak.getInstance("http://localhost:8081/auth", "master", "admin", "admin", org.keycloak.models.Constants.ADMIN_CLI_CLIENT_ID);
|
||||
Keycloak keycloak2 = Keycloak.getInstance("http://localhost:8082/auth", "master", "admin", "admin", org.keycloak.models.Constants.ADMIN_CLI_CLIENT_ID);
|
||||
|
||||
// Require broker to show consent screen
|
||||
RealmResource brokeredRealm = keycloak2.realm("realm-with-oidc-identity-provider");
|
||||
List<ClientRepresentation> clients = brokeredRealm.clients().findByClientId("broker-app");
|
||||
Assert.assertEquals(1, clients.size());
|
||||
ClientRepresentation brokerApp = clients.get(0);
|
||||
brokerApp.setConsentRequired(true);
|
||||
brokeredRealm.clients().get(brokerApp.getId()).update(brokerApp);
|
||||
|
||||
// Change timeouts on realm-with-broker to lower values
|
||||
RealmResource realmWithBroker = keycloak1.realm("realm-with-broker");
|
||||
RealmRepresentation realmBackup = realmWithBroker.toRepresentation();
|
||||
RealmRepresentation realm = realmWithBroker.toRepresentation();
|
||||
realm.setAccessCodeLifespanLogin(30);;
|
||||
realm.setAccessCodeLifespan(30);
|
||||
realm.setAccessCodeLifespanUserAction(30);
|
||||
realmWithBroker.update(realm);
|
||||
|
||||
// Login to broker
|
||||
loginIDP("test-user");
|
||||
|
||||
// Set time offset
|
||||
Time.setOffset(60);
|
||||
try {
|
||||
// User rejected consent
|
||||
grantPage.assertCurrent();
|
||||
grantPage.cancel();
|
||||
|
||||
// Assert error page with backToApplication link displayed
|
||||
errorPage.assertCurrent();
|
||||
errorPage.clickBackToApplication();
|
||||
|
||||
assertTrue(this.driver.getCurrentUrl().startsWith("http://localhost:8081/auth/realms/realm-with-broker/protocol/openid-connect/auth"));
|
||||
|
||||
|
||||
// Login to broker again
|
||||
loginIDP("test-user");
|
||||
|
||||
// Set time offset and manually remove expiredSessions TODO: Will require custom endpoint when migrate to integration-arquillian
|
||||
Time.setOffset(120);
|
||||
|
||||
brokerServerRule.stopSession(this.session, true);
|
||||
this.session = brokerServerRule.startSession();
|
||||
|
||||
session.sessions().removeExpired(getRealm());
|
||||
|
||||
brokerServerRule.stopSession(this.session, true);
|
||||
this.session = brokerServerRule.startSession();
|
||||
|
||||
// User rejected consent
|
||||
grantPage.assertCurrent();
|
||||
grantPage.cancel();
|
||||
|
||||
// Assert error page without backToApplication link (clientSession expired and was removed on the server)
|
||||
errorPage.assertCurrent();
|
||||
try {
|
||||
errorPage.clickBackToApplication();
|
||||
fail("Not expected to have link backToApplication available");
|
||||
} catch (NoSuchElementException nsee) {
|
||||
// Expected;
|
||||
}
|
||||
|
||||
} finally {
|
||||
Time.setOffset(0);
|
||||
}
|
||||
|
||||
// Revert consent
|
||||
brokerApp.setConsentRequired(false);
|
||||
brokeredRealm.clients().get(brokerApp.getId()).update(brokerApp);
|
||||
|
||||
// Revert timeouts
|
||||
realmWithBroker.update(realmBackup);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testSuccessfulAuthenticationWithoutUpdateProfile() {
|
||||
super.testSuccessfulAuthenticationWithoutUpdateProfile();
|
||||
|
|
|
@ -0,0 +1,264 @@
|
|||
/*
|
||||
* Copyright 2016 Red Hat, Inc. and/or its affiliates
|
||||
* and other contributors as indicated by the @author tags.
|
||||
*
|
||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||
* you may not use this file except in compliance with the License.
|
||||
* You may obtain a copy of the License at
|
||||
*
|
||||
* http://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing, software
|
||||
* distributed under the License is distributed on an "AS IS" BASIS,
|
||||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
* See the License for the specific language governing permissions and
|
||||
* limitations under the License.
|
||||
*/
|
||||
|
||||
package org.keycloak.testsuite.broker;
|
||||
|
||||
import java.util.List;
|
||||
|
||||
import org.junit.Assert;
|
||||
import org.junit.BeforeClass;
|
||||
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.KeycloakSession;
|
||||
import org.keycloak.models.RealmModel;
|
||||
import org.keycloak.representations.idm.ClientRepresentation;
|
||||
import org.keycloak.representations.idm.IdentityProviderRepresentation;
|
||||
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.rule.AbstractKeycloakRule;
|
||||
import org.openqa.selenium.NoSuchElementException;
|
||||
|
||||
import static org.junit.Assert.assertTrue;
|
||||
import static org.junit.Assert.fail;
|
||||
|
||||
/**
|
||||
* @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
|
||||
*/
|
||||
public class OIDCKeycloakServerBrokerWithConsentTest extends AbstractIdentityProviderTest {
|
||||
|
||||
private static final int PORT = 8082;
|
||||
|
||||
private static Keycloak keycloak1;
|
||||
private static Keycloak keycloak2;
|
||||
|
||||
@ClassRule
|
||||
public static AbstractKeycloakRule oidcServerRule = new AbstractKeycloakRule() {
|
||||
|
||||
@Override
|
||||
protected void configureServer(KeycloakServer server) {
|
||||
server.getConfig().setPort(PORT);
|
||||
}
|
||||
|
||||
@Override
|
||||
protected void configure(KeycloakSession session, RealmManager manager, RealmModel adminRealm) {
|
||||
server.importRealm(getClass().getResourceAsStream("/broker-test/test-broker-realm-with-kc-oidc.json"));
|
||||
|
||||
// Disable update profile
|
||||
RealmModel realm = getRealm(session);
|
||||
setUpdateProfileFirstLogin(realm, IdentityProviderRepresentation.UPFLM_OFF);
|
||||
}
|
||||
|
||||
@Override
|
||||
protected String[] getTestRealms() {
|
||||
return new String[] { "realm-with-oidc-identity-provider" };
|
||||
}
|
||||
};
|
||||
|
||||
|
||||
@BeforeClass
|
||||
public static void before() {
|
||||
keycloak1 = Keycloak.getInstance("http://localhost:8081/auth", "master", "admin", "admin", org.keycloak.models.Constants.ADMIN_CLI_CLIENT_ID);
|
||||
keycloak2 = Keycloak.getInstance("http://localhost:8082/auth", "master", "admin", "admin", org.keycloak.models.Constants.ADMIN_CLI_CLIENT_ID);
|
||||
|
||||
// Require broker to show consent screen
|
||||
RealmResource brokeredRealm = keycloak2.realm("realm-with-oidc-identity-provider");
|
||||
List<ClientRepresentation> clients = brokeredRealm.clients().findByClientId("broker-app");
|
||||
Assert.assertEquals(1, clients.size());
|
||||
ClientRepresentation brokerApp = clients.get(0);
|
||||
brokerApp.setConsentRequired(true);
|
||||
brokeredRealm.clients().get(brokerApp.getId()).update(brokerApp);
|
||||
|
||||
|
||||
// Change timeouts on realm-with-broker to lower values
|
||||
RealmResource realmWithBroker = keycloak1.realm("realm-with-broker");
|
||||
RealmRepresentation realmRep = realmWithBroker.toRepresentation();
|
||||
realmRep.setAccessCodeLifespanLogin(30);;
|
||||
realmRep.setAccessCodeLifespan(30);
|
||||
realmRep.setAccessCodeLifespanUserAction(30);
|
||||
realmWithBroker.update(realmRep);
|
||||
}
|
||||
|
||||
|
||||
@Override
|
||||
protected String getProviderId() {
|
||||
return "kc-oidc-idp";
|
||||
}
|
||||
|
||||
|
||||
// KEYCLOAK-2769
|
||||
@Test
|
||||
public void testConsentDeniedWithExpiredClientSession() throws Exception {
|
||||
// Login to broker
|
||||
loginIDP("test-user");
|
||||
|
||||
// Set time offset
|
||||
Time.setOffset(60);
|
||||
try {
|
||||
// User rejected consent
|
||||
grantPage.assertCurrent();
|
||||
grantPage.cancel();
|
||||
|
||||
// Assert error page with backToApplication link displayed
|
||||
errorPage.assertCurrent();
|
||||
errorPage.clickBackToApplication();
|
||||
|
||||
assertTrue(this.driver.getCurrentUrl().startsWith("http://localhost:8081/auth/realms/realm-with-broker/protocol/openid-connect/auth"));
|
||||
|
||||
} finally {
|
||||
Time.setOffset(0);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
// KEYCLOAK-2769
|
||||
@Test
|
||||
public void testConsentDeniedWithExpiredAndClearedClientSession() throws Exception {
|
||||
// Login to broker again
|
||||
loginIDP("test-user");
|
||||
|
||||
// Set time offset
|
||||
Time.setOffset(60);
|
||||
try {
|
||||
// Manually remove expiredSessions TODO: Will require custom endpoint when migrate to integration-arquillian
|
||||
brokerServerRule.stopSession(this.session, true);
|
||||
this.session = brokerServerRule.startSession();
|
||||
|
||||
session.sessions().removeExpired(getRealm());
|
||||
|
||||
brokerServerRule.stopSession(this.session, true);
|
||||
this.session = brokerServerRule.startSession();
|
||||
|
||||
// User rejected consent
|
||||
grantPage.assertCurrent();
|
||||
grantPage.cancel();
|
||||
|
||||
// Assert error page without backToApplication link (clientSession expired and was removed on the server)
|
||||
errorPage.assertCurrent();
|
||||
try {
|
||||
errorPage.clickBackToApplication();
|
||||
fail("Not expected to have link backToApplication available");
|
||||
} catch (NoSuchElementException nsee) {
|
||||
// Expected;
|
||||
}
|
||||
|
||||
} finally {
|
||||
Time.setOffset(0);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
// KEYCLOAK-2801
|
||||
@Test
|
||||
public void testAccountManagementLinkingAndExpiredClientSession() 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");
|
||||
|
||||
// Set time offset
|
||||
Time.setOffset(60);
|
||||
try {
|
||||
// User rejected consent
|
||||
grantPage.assertCurrent();
|
||||
grantPage.cancel();
|
||||
|
||||
// Assert account error page with "staleCodeAccount" error displayed
|
||||
accountFederatedIdentityPage.assertCurrent();
|
||||
Assert.assertEquals("The page expired. Please try one more time.", accountFederatedIdentityPage.getError());
|
||||
|
||||
|
||||
// Try to link one more time
|
||||
accountFederatedIdentityPage.clickAddProvider(getProviderId());
|
||||
|
||||
assertTrue(this.driver.getCurrentUrl().startsWith("http://localhost:8082/auth/"));
|
||||
this.loginPage.login("test-user", "password");
|
||||
|
||||
Time.setOffset(120);
|
||||
|
||||
// User granted consent
|
||||
grantPage.assertCurrent();
|
||||
grantPage.accept();
|
||||
|
||||
// Assert account error page with "staleCodeAccount" error displayed
|
||||
accountFederatedIdentityPage.assertCurrent();
|
||||
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");
|
||||
}
|
||||
|
||||
|
||||
@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());
|
||||
}
|
||||
}
|
|
@ -135,6 +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.
|
||||
consentDenied=Consent denied.
|
||||
|
||||
accountDisabledMessage=Account is disabled, contact admin.
|
||||
|
||||
|
|
Loading…
Reference in a new issue