libre.sh/keycloak-scim
2
0
Fork 0
Code Issues 14 Pull requests Releases Packages Activity Actions 6

KEYCLOAK-2769 Better error handling of expired code in IdentityBrokerService

Browse source
This commit is contained in:
mposolda 2016-04-11 18:08:10 +02:00
parent 23e261244c
commit 3e9ba71baa
6 changed files with 108 additions and 35 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
services/src/main/java/org/keycloak/services/messages/Messages.java
View file

@ -154,6 +154,8 @@ public class Messages {
public static final String IDENTITY_PROVIDER_LINK_SUCCESS = "identityProviderLinkSuccess";
public static final String STALE_CODE = "staleCodeMessage";
public static final String IDENTITY_PROVIDER_NOT_UNIQUE = "identityProviderNotUniqueMessage";
public static final String REALM_SUPPORTS_NO_CREDENTIALS = "realmSupportsNoCredentialsMessage";

99
services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java
View file

@ -142,7 +142,12 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal
}
try {
ClientSessionCode clientSessionCode = parseClientSessionCode(code);
ParsedCodeContext parsedCode = parseClientSessionCode(code);
if (parsedCode.response != null) {
return parsedCode.response;
}
ClientSessionCode clientSessionCode = parsedCode.clientSessionCode;
IdentityProvider identityProvider = getIdentityProvider(session, realmModel, providerId);
Response response = identityProvider.performLogin(createAuthenticationRequest(providerId, clientSessionCode));
@ -241,14 +246,14 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal
}
public Response authenticated(BrokeredIdentityContext context) {
ClientSessionCode clientCode = null;
IdentityProviderModel identityProviderConfig = context.getIdpConfig();
try {
clientCode = parseClientSessionCode(context.getCode());
} catch (Exception e) {
return redirectToErrorPage(Messages.IDENTITY_PROVIDER_AUTHENTICATION_FAILED, e, identityProviderConfig.getProviderId());
ParsedCodeContext parsedCode = parseClientSessionCode(context.getCode());
if (parsedCode.response != null) {
return parsedCode.response;
}
ClientSessionCode clientCode = parsedCode.clientSessionCode;
String providerId = identityProviderConfig.getAlias();
if (!identityProviderConfig.isStoreToken()) {
if (isDebugEnabled()) {
@ -325,8 +330,11 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal
@GET
@Path("/after-first-broker-login")
public Response afterFirstBrokerLogin(@QueryParam("code") String code) {
ClientSessionCode clientCode = parseClientSessionCode(code);
ClientSessionModel clientSession = clientCode.getClientSession();
ParsedCodeContext parsedCode = parseClientSessionCode(code);
if (parsedCode.response != null) {
return parsedCode.response;
}
ClientSessionModel clientSession = parsedCode.clientSessionCode.getClientSession();
try {
this.event.detail(Details.CODE_ID, clientSession.getId())
@ -439,8 +447,11 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal
@GET
@Path("/after-post-broker-login")
public Response afterPostBrokerLoginFlow(@QueryParam("code") String code) {
ClientSessionCode clientCode = parseClientSessionCode(code);
ClientSessionModel clientSession = clientCode.getClientSession();
ParsedCodeContext parsedCode = parseClientSessionCode(code);
if (parsedCode.response != null) {
return parsedCode.response;
}
ClientSessionModel clientSession = parsedCode.clientSessionCode.getClientSession();
try {
SerializedBrokeredIdentityContext serializedCtx = SerializedBrokeredIdentityContext.readFromClientSession(clientSession, PostBrokerLoginConstants.PBL_BROKERED_IDENTITY_CONTEXT);
@ -526,20 +537,23 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal
@Override
public Response cancelled(String code) {
ClientSessionCode clientCode = ClientSessionCode.parse(code, this.session, this.realmModel);
if (clientCode == null || !clientCode.isValid(AUTHENTICATE.name(), ClientSessionCode.ActionType.LOGIN)) {
return redirectToErrorPage(Messages.INVALID_CODE);
ParsedCodeContext parsedCode = parseClientSessionCode(code);
if (parsedCode.response != null) {
return parsedCode.response;
}
ClientSessionCode clientCode = parsedCode.clientSessionCode;
return browserAuthentication(clientCode.getClientSession(), null);
}
@Override
public Response error(String code, String message) {
ClientSessionCode clientCode = ClientSessionCode.parse(code, this.session, this.realmModel);
if (clientCode == null || !clientCode.isValid(AUTHENTICATE.name(), ClientSessionCode.ActionType.LOGIN)) {
return redirectToErrorPage(Messages.INVALID_CODE);
ParsedCodeContext parsedCode = parseClientSessionCode(code);
if (parsedCode.response != null) {
return parsedCode.response;
}
ClientSessionCode clientCode = parsedCode.clientSessionCode;
return browserAuthentication(clientCode.getClientSession(), message);
}
@ -600,36 +614,41 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal
}
private ClientSessionCode parseClientSessionCode(String code) {
private ParsedCodeContext parseClientSessionCode(String code) {
ClientSessionCode clientCode = ClientSessionCode.parse(code, this.session, this.realmModel);
if (clientCode != null && clientCode.isValid(AUTHENTICATE.name(), ClientSessionCode.ActionType.LOGIN)) {
if (clientCode != null) {
ClientSessionModel clientSession = clientCode.getClientSession();
if (clientSession != null) {
ClientModel client = clientSession.getClient();
if (clientSession.getUserSession() != null) {
this.event.session(clientSession.getUserSession());
}
if (client == null) {
throw new IdentityBrokerException("Invalid client");
}
ClientModel client = clientSession.getClient();
if (client != null) {
logger.debugf("Got authorization code from client [%s].", client.getClientId());
this.event.client(client);
this.session.getContext().setClient(client);
if (clientSession.getUserSession() != null) {
this.event.session(clientSession.getUserSession());
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);
return ParsedCodeContext.response(staleCodeError);
}
}
if (isDebugEnabled()) {
logger.debugf("Authorization code is valid.");
}
if (isDebugEnabled()) {
logger.debugf("Authorization code is valid.");
}
return clientCode;
return ParsedCodeContext.clientSessionCode(clientCode);
}
}
throw new IdentityBrokerException("Invalid code, please login again through your client.");
logger.debugf("Authorization code is not valid. Code: %s", code);
Response staleCodeError = redirectToErrorPage(Messages.STALE_CODE);
return ParsedCodeContext.response(staleCodeError);
}
private AuthenticationRequest createAuthenticationRequest(String providerId, ClientSessionCode clientSessionCode) {
@ -804,4 +823,22 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal
this.session.getTransaction().rollback();
}
}
private static class ParsedCodeContext {
private ClientSessionCode clientSessionCode;
private Response response;
public static ParsedCodeContext clientSessionCode(ClientSessionCode clientSessionCode) {
ParsedCodeContext ctx = new ParsedCodeContext();
ctx.clientSessionCode = clientSessionCode;
return ctx;
}
public static ParsedCodeContext response(Response response) {
ParsedCodeContext ctx = new ParsedCodeContext();
ctx.response = response;
return ctx;
}
}
}

30
testsuite/integration/src/test/java/org/keycloak/testsuite/broker/OIDCKeyCloakServerBrokerBasicTest.java
View file

@ -49,6 +49,7 @@ 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;
/**
@ -147,7 +148,7 @@ public class OIDCKeyCloakServerBrokerBasicTest extends AbstractKeycloakIdentityP
@Test
public void testAccessDeniedError() throws Exception {
public void testConsentDeniedWithExpiredClientSession() throws Exception {
// Disable update profile
setUpdateProfileFirstLogin(IdentityProviderRepresentation.UPFLM_OFF);
@ -174,9 +175,26 @@ public class OIDCKeyCloakServerBrokerBasicTest extends AbstractKeycloakIdentityP
// Login to broker
loginIDP("test-user");
// Set time offset and manually enforce removeExpired sessions TODO: Will need custom REST endpoints for that on integration-arquillian
// 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();
@ -189,8 +207,14 @@ public class OIDCKeyCloakServerBrokerBasicTest extends AbstractKeycloakIdentityP
grantPage.assertCurrent();
grantPage.cancel();
// Assert classic error page ( We're sorry ) displayed. TODO: Should be improved? See KEYCLOAK-2740
// 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);

9
testsuite/integration/src/test/java/org/keycloak/testsuite/pages/ErrorPage.java
View file

@ -16,8 +16,10 @@
*/
package org.keycloak.testsuite.pages;
import org.junit.Assert;
import org.keycloak.testsuite.OAuthClient;
import org.keycloak.testsuite.rule.WebResource;
import org.openqa.selenium.By;
import org.openqa.selenium.WebElement;
import org.openqa.selenium.support.FindBy;
@ -32,10 +34,17 @@ public class ErrorPage extends AbstractPage {
@FindBy(className = "instruction")
private WebElement errorMessage;
@FindBy(id = "backToApplication")
private WebElement backToApplicationLink;
public String getError() {
return errorMessage.getText();
}
public void clickBackToApplication() {
backToApplicationLink.click();
}
public boolean isCurrent() {
return driver.getTitle().equals("We're sorry...");
}

2
themes/src/main/resources/theme/base/login/error.ftl
View file

@ -8,7 +8,7 @@
<div id="kc-error-message">
<p class="instruction">${message.summary}</p>
<#if client?? && client.baseUrl?has_content>
<p><a href="${client.baseUrl}">${msg("backToApplication")}</a></p>
<p><a id="backToApplication" href="${client.baseUrl}">${msg("backToApplication")}</a></p>
</#if>
</div>
</#if>

1
themes/src/main/resources/theme/base/login/messages/messages_en.properties
View file

@ -202,6 +202,7 @@ invalidCodeMessage=An error occurred, please login again through your applicatio
identityProviderUnexpectedErrorMessage=Unexpected error when authenticating with identity provider
identityProviderNotFoundMessage=Could not find an identity provider with the identifier.
identityProviderLinkSuccess=Your account was successfully linked with {0} account {1} .
staleCodeMessage=This page is no longer valid, please go back to your application and login again
realmSupportsNoCredentialsMessage=Realm does not support any credential type.
identityProviderNotUniqueMessage=Realm supports multiple identity providers. Could not determine which identity provider should be used to authenticate with.
emailVerifiedMessage=Your email address has been verified.