diff --git a/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java b/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java index 27d47b5e1e..4a9d99383c 100755 --- a/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java +++ b/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java @@ -531,7 +531,7 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal @Override public Response cancelled(String code) { ClientSessionCode clientCode = ClientSessionCode.parse(code, this.session, this.realmModel); - if (clientCode.getClientSession() == null || !clientCode.isValid(AUTHENTICATE.name(), ClientSessionCode.ActionType.LOGIN)) { + if (clientCode == null || !clientCode.isValid(AUTHENTICATE.name(), ClientSessionCode.ActionType.LOGIN)) { return redirectToErrorPage(Messages.INVALID_CODE); } @@ -541,7 +541,7 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal @Override public Response error(String code, String message) { ClientSessionCode clientCode = ClientSessionCode.parse(code, this.session, this.realmModel); - if (clientCode.getClientSession() == null || !clientCode.isValid(AUTHENTICATE.name(), ClientSessionCode.ActionType.LOGIN)) { + if (clientCode == null || !clientCode.isValid(AUTHENTICATE.name(), ClientSessionCode.ActionType.LOGIN)) { return redirectToErrorPage(Messages.INVALID_CODE); } return browserAuthentication(clientCode.getClientSession(), message); diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractIdentityProviderTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractIdentityProviderTest.java index b45213aa26..f5f80f5011 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractIdentityProviderTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractIdentityProviderTest.java @@ -40,6 +40,7 @@ import org.keycloak.testsuite.broker.util.UserSessionStatusServlet.UserSessionSt import org.keycloak.testsuite.pages.AccountFederatedIdentityPage; import org.keycloak.testsuite.pages.AccountPasswordPage; import org.keycloak.testsuite.pages.AccountUpdateProfilePage; +import org.keycloak.testsuite.pages.ErrorPage; import org.keycloak.testsuite.pages.LoginPage; import org.keycloak.testsuite.pages.LoginUpdateProfilePage; import org.keycloak.testsuite.pages.OAuthGrantPage; @@ -109,6 +110,9 @@ public abstract class AbstractIdentityProviderTest { @WebResource protected AccountFederatedIdentityPage accountFederatedIdentityPage; + @WebResource + protected ErrorPage errorPage; + protected KeycloakSession session; protected int logoutTimeOffset = 0; diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/OIDCKeyCloakServerBrokerBasicTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/OIDCKeyCloakServerBrokerBasicTest.java index ad3e9ffeeb..68d903cafc 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/OIDCKeyCloakServerBrokerBasicTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/OIDCKeyCloakServerBrokerBasicTest.java @@ -17,12 +17,16 @@ 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.KeycloakSession; import org.keycloak.models.RealmModel; 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; @@ -30,7 +34,6 @@ import org.keycloak.services.managers.RealmManager; import org.keycloak.testsuite.Constants; import org.keycloak.testsuite.KeycloakServer; import org.keycloak.testsuite.pages.AccountApplicationsPage; -import org.keycloak.testsuite.pages.OAuthGrantPage; import org.keycloak.testsuite.rule.AbstractKeycloakRule; import org.keycloak.testsuite.rule.WebResource; import org.keycloak.util.JsonSerialization; @@ -38,6 +41,7 @@ import org.openqa.selenium.NoSuchElementException; import javax.ws.rs.core.UriBuilder; import java.io.IOException; +import java.util.List; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.fail; @@ -68,9 +72,6 @@ public class OIDCKeyCloakServerBrokerBasicTest extends AbstractKeycloakIdentityP } }; - @WebResource - private OAuthGrantPage grantPage; - @WebResource protected AccountApplicationsPage accountApplicationsPage; @@ -139,6 +140,65 @@ public class OIDCKeyCloakServerBrokerBasicTest extends AbstractKeycloakIdentityP keycloak.realm("realm-with-broker").identityProviders().get("kc-oidc-idp").update(idp); } + + @Test + public void testAccessDeniedError() 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 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 and manually enforce removeExpired sessions TODO: Will need custom REST endpoints for that on integration-arquillian + Time.setOffset(60); + try { + 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 classic error page ( We're sorry ) displayed. TODO: Should be improved? See KEYCLOAK-2740 + errorPage.assertCurrent(); + + } 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();