KEYCLOAK-2802 NPE during identity broker cancelled from account mgmt

integration/admin-client/src/main/java/org/keycloak/admin/client/resource/UserResource.java

@@ -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;

services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java

@@ -543,6 +543,11 @@ public Response cancelled(String code) {
         }
         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 Response error(String code, String message) {
         }
         ClientSessionCode clientCode = parsedCode.clientSessionCode;
 
+        Response accountManagementFailedLinking = checkAccountManagementFailedLinking(clientCode.getClientSession(), message);
+        if (accountManagementFailedLinking != null) {
+            return accountManagementFailedLinking;
+        }
+
         return browserAuthentication(clientCode.getClientSession(), message);
     }
 
@@ -635,20 +645,10 @@ private ParsedCodeContext parseClientSessionCode(String code) {
                 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 @@ private ParsedCodeContext parseClientSessionCode(String code) {
         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;

testsuite/integration/src/test/java/org/keycloak/testsuite/broker/OIDCKeyCloakServerBrokerBasicTest.java

@@ -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;
 
 /**

testsuite/integration/src/test/java/org/keycloak/testsuite/broker/OIDCKeycloakServerBrokerWithConsentTest.java

@@ -20,15 +20,12 @@
 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.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 void testConsentDeniedWithExpiredAndClearedClientSession() throws Excepti
     @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 void testAccountManagementLinkingAndExpiredClientSession() throws Excepti
 
             // 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 void testAccountManagementLinkingAndExpiredClientSession() throws Excepti
 
             // 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());
     }
 }

themes/src/main/resources/theme/base/account/messages/messages_en.properties

@@ -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.