[KEYCLOAK-14432] Unhandled NPE in identity broker auth response

services/src/main/java/org/keycloak/broker/oidc/AbstractOAuth2IdentityProvider.java

@@ -76,7 +76,6 @@ import java.io.IOException;
 import java.net.URI;
 import java.util.Arrays;
 import java.util.List;
-import java.util.UUID;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -453,6 +452,10 @@ public abstract class AbstractOAuth2IdentityProvider<C extends OAuth2IdentityPro
         public Response authResponse(@QueryParam(AbstractOAuth2IdentityProvider.OAUTH2_PARAMETER_STATE) String state,
                                      @QueryParam(AbstractOAuth2IdentityProvider.OAUTH2_PARAMETER_CODE) String authorizationCode,
                                      @QueryParam(OAuth2Constants.ERROR) String error) {
+            if (state == null) {
+                return errorIdentityProviderLogin(Messages.IDENTITY_PROVIDER_MISSING_STATE_ERROR);
+            }
+
             if (error != null) {
                 logger.error(error + " for broker login " + getConfig().getProviderId());
                 if (error.equals(ACCESS_DENIED)) {
@@ -488,9 +491,13 @@ public abstract class AbstractOAuth2IdentityProvider<C extends OAuth2IdentityPro
             } catch (Exception e) {
                 logger.error("Failed to make identity provider oauth callback", e);
             }
+            return errorIdentityProviderLogin(Messages.IDENTITY_PROVIDER_UNEXPECTED_ERROR);
+        }
+
+        private Response errorIdentityProviderLogin(String message) {
             event.event(EventType.LOGIN);
             event.error(Errors.IDENTITY_PROVIDER_LOGIN_FAILURE);
-            return ErrorPage.error(session, null, Response.Status.BAD_GATEWAY, Messages.IDENTITY_PROVIDER_UNEXPECTED_ERROR);
+            return ErrorPage.error(session, null, Response.Status.BAD_GATEWAY, message);
         }
 
         public SimpleHttp generateTokenRequest(String authorizationCode) {

services/src/main/java/org/keycloak/services/messages/Messages.java

@@ -167,6 +167,8 @@ public class Messages {
 
     public static final String IDENTITY_PROVIDER_UNEXPECTED_ERROR = "identityProviderUnexpectedErrorMessage";
 
+    public static final String IDENTITY_PROVIDER_MISSING_STATE_ERROR = "identityProviderMissingStateMessage";
+
     public static final String IDENTITY_PROVIDER_NOT_FOUND = "identityProviderNotFoundMessage";
 
     public static final String IDENTITY_PROVIDER_LINK_SUCCESS = "identityProviderLinkSuccess";

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerTest.java

@@ -2,6 +2,7 @@ package org.keycloak.testsuite.broker;
 
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Lists;
+import org.hamcrest.Matchers;
 import org.junit.Test;
 import org.keycloak.admin.client.resource.ClientResource;
 import org.keycloak.admin.client.resource.ClientsResource;
@@ -24,9 +25,12 @@ import org.keycloak.representations.idm.ProtocolMapperRepresentation;
 import org.keycloak.representations.idm.RoleRepresentation;
 import org.keycloak.representations.idm.UserRepresentation;
 import org.keycloak.testsuite.Assert;
+import org.keycloak.testsuite.admin.ApiUtil;
 
+import javax.ws.rs.core.Response;
 import java.util.Collections;
 import java.util.List;
+import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
 
@@ -395,6 +399,52 @@ public final class KcOidcBrokerTest extends AbstractAdvancedBrokerTest {
         errorPage.assertCurrent();
     }
 
+    @Test
+    public void testMissingStateParameter() {
+        final String IDP_NAME = "github";
+
+        RealmResource realmResource = Optional.ofNullable(realmsResouce().realm(bc.providerRealmName())).orElse(null);
+        assertThat(realmResource, Matchers.notNullValue());
+        final int COUNT_PROVIDERS = Optional.of(realmResource.identityProviders().findAll().size()).orElse(0);
+
+        try {
+            IdentityProviderRepresentation idp = new IdentityProviderRepresentation();
+            idp.setAlias(IDP_NAME);
+            idp.setDisplayName(IDP_NAME);
+            idp.setProviderId(IDP_NAME);
+            idp.setEnabled(true);
+
+            Response response = realmResource.identityProviders().create(idp);
+            assertThat(response, Matchers.notNullValue());
+            assertThat(response.getStatus(), Matchers.is(Response.Status.CREATED.getStatusCode()));
+            assertThat(realmResource.identityProviders().findAll().size(), Matchers.is(COUNT_PROVIDERS + 1));
+            assertThat(ApiUtil.getCreatedId(response), Matchers.notNullValue());
+
+            IdentityProviderRepresentation provider = Optional.ofNullable(realmResource.identityProviders().get(IDP_NAME).toRepresentation()).orElse(null);
+            assertThat(provider, Matchers.notNullValue());
+            assertThat(provider.getProviderId(), Matchers.is(IDP_NAME));
+            assertThat(provider.getAlias(), Matchers.is(IDP_NAME));
+            assertThat(provider.getDisplayName(), Matchers.is(IDP_NAME));
+
+            final String REALM_NAME = Optional.ofNullable(realmResource.toRepresentation().getRealm()).orElse(null);
+            assertThat(REALM_NAME, Matchers.notNullValue());
+
+            final String LINK = oauth.AUTH_SERVER_ROOT + "/realms/" + REALM_NAME + "/broker/" + IDP_NAME + "/endpoint?code=foo123";
+
+            driver.navigate().to(LINK);
+            waitForPage(driver, "log in to provider", true);
+
+            errorPage.assertCurrent();
+            assertThat(errorPage.getError(), Matchers.is("Missing state parameter in response from identity provider."));
+
+        } finally {
+            IdentityProviderResource resource = Optional.ofNullable(realmResource.identityProviders().get(IDP_NAME)).orElse(null);
+            assertThat(resource, Matchers.notNullValue());
+            resource.remove();
+            assertThat(Optional.of(realmResource.identityProviders().findAll().size()).orElse(0), Matchers.is(COUNT_PROVIDERS));
+        }
+    }
+
     private UserRepresentation getFederatedIdentity() {
         List<UserRepresentation> users = realmsResouce().realm(bc.consumerRealmName()).users().search(bc.getUserLogin());
 

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

@@ -263,6 +263,7 @@ invalidAccessCodeMessage=Invalid access code.
 sessionNotActiveMessage=Session not active.
 invalidCodeMessage=An error occurred, please login again through your application.
 identityProviderUnexpectedErrorMessage=Unexpected error when authenticating with identity provider
+identityProviderMissingStateMessage=Missing state parameter in response from identity provider.
 identityProviderNotFoundMessage=Could not find an identity provider with the identifier.
 identityProviderLinkSuccess=You successfully verified your email. Please go back to your original browser and continue there with the login.
 staleCodeMessage=This page is no longer valid, please go back to your application and log in again