KEYCLOAK-3400 OIDC request with missing response_type should respond with error

services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpoint.java

@@ -271,12 +271,10 @@ public class AuthorizationEndpoint extends AuthorizationEndpointBase {
     }
 
     private Response checkResponseType() {
-        OIDCResponseMode defaultResponseMode = client.isImplicitFlowEnabled() ? OIDCResponseMode.FRAGMENT : OIDCResponseMode.QUERY;
-
         if (responseType == null) {
             logger.missingParameter(OAuth2Constants.RESPONSE_TYPE);
             event.error(Errors.INVALID_REQUEST);
-            return redirectErrorToClient(defaultResponseMode, OAuthErrorException.INVALID_REQUEST, "Missing parameter: response_type");
+            return redirectErrorToClient(OIDCResponseMode.QUERY, OAuthErrorException.INVALID_REQUEST, "Missing parameter: response_type");
         }
 
         event.detail(Details.RESPONSE_TYPE, responseType);
@@ -289,7 +287,7 @@ public class AuthorizationEndpoint extends AuthorizationEndpointBase {
         } catch (IllegalArgumentException iae) {
             logger.error(iae.getMessage());
             event.error(Errors.INVALID_REQUEST);
-            return redirectErrorToClient(defaultResponseMode, OAuthErrorException.UNSUPPORTED_RESPONSE_TYPE, null);
+            return redirectErrorToClient(OIDCResponseMode.QUERY, OAuthErrorException.UNSUPPORTED_RESPONSE_TYPE, null);
         }
 
         OIDCResponseMode parsedResponseMode = null;
@@ -298,7 +296,7 @@ public class AuthorizationEndpoint extends AuthorizationEndpointBase {
         } catch (IllegalArgumentException iae) {
             logger.invalidParameter(OIDCLoginProtocol.RESPONSE_MODE_PARAM);
             event.error(Errors.INVALID_REQUEST);
-            return redirectErrorToClient(defaultResponseMode, OAuthErrorException.INVALID_REQUEST, "Invalid parameter: response_mode");
+            return redirectErrorToClient(OIDCResponseMode.QUERY, OAuthErrorException.INVALID_REQUEST, "Invalid parameter: response_mode");
         }
 
         event.detail(Details.RESPONSE_MODE, parsedResponseMode.toString().toLowerCase());
@@ -307,7 +305,7 @@ public class AuthorizationEndpoint extends AuthorizationEndpointBase {
         if (parsedResponseType.isImplicitOrHybridFlow() && parsedResponseMode == OIDCResponseMode.QUERY) {
             logger.responseModeQueryNotAllowed();
             event.error(Errors.INVALID_REQUEST);
-            return redirectErrorToClient(defaultResponseMode, OAuthErrorException.INVALID_REQUEST, "Response_mode 'query' not allowed for implicit or hybrid flow");
+            return redirectErrorToClient(OIDCResponseMode.QUERY, OAuthErrorException.INVALID_REQUEST, "Response_mode 'query' not allowed for implicit or hybrid flow");
         }
 
         if ((parsedResponseType.hasResponseType(OIDCResponseType.CODE) || parsedResponseType.hasResponseType(OIDCResponseType.NONE)) && !client.isStandardFlowEnabled()) {

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/AuthorizationCodeTest.java

@@ -144,33 +144,6 @@ public class AuthorizationCodeTest extends AbstractKeycloakTest {
         assertCode(codeId, response.getCode());
     }
 
-    @Test
-    public void authorizationRequestImplicitFlowDisabled() throws IOException {
-        oauth.responseType("token id_token");
-        UriBuilder b = UriBuilder.fromUri(oauth.getLoginFormUrl());
-        driver.navigate().to(b.build().toURL());
-
-        OAuthClient.AuthorizationEndpointResponse errorResponse = new OAuthClient.AuthorizationEndpointResponse(oauth, true);
-        Assert.assertTrue(errorResponse.isRedirected());
-        Assert.assertEquals(errorResponse.getError(), OAuthErrorException.UNSUPPORTED_RESPONSE_TYPE);
-        Assert.assertEquals(errorResponse.getErrorDescription(), "Client is not allowed to initiate browser login with given response_type. Implicit flow is disabled for the client.");
-
-        events.expectLogin().error(Errors.NOT_ALLOWED).user((String) null).session((String) null).clearDetails().detail(Details.RESPONSE_TYPE, "token id_token").assertEvent();
-    }
-
-    @Test
-    public void authorizationRequestMissingResponseType() throws IOException {
-        oauth.responseType(null);
-        UriBuilder b = UriBuilder.fromUri(oauth.getLoginFormUrl());
-        driver.navigate().to(b.build().toURL());
-
-        OAuthClient.AuthorizationEndpointResponse errorResponse = new OAuthClient.AuthorizationEndpointResponse(oauth);
-        Assert.assertTrue(errorResponse.isRedirected());
-        Assert.assertEquals(errorResponse.getError(), OAuthErrorException.INVALID_REQUEST);
-
-        events.expectLogin().error(Errors.INVALID_REQUEST).user((String) null).session((String) null).clearDetails().assertEvent();
-    }
-
     @Test
     public void authorizationRequestInvalidResponseType() throws IOException {
         oauth.responseType("tokenn");

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/flows/AbstractOIDCResponseTypeTest.java

@@ -17,13 +17,18 @@
 
 package org.keycloak.testsuite.oidc.flows;
 
+import java.io.IOException;
+import java.net.MalformedURLException;
 import java.util.List;
 
+import javax.ws.rs.core.UriBuilder;
+
 import org.jboss.arquillian.graphene.page.Page;
 import org.junit.Rule;
 import org.junit.Test;
 import org.keycloak.OAuthErrorException;
 import org.keycloak.events.Details;
+import org.keycloak.events.Errors;
 import org.keycloak.jose.jws.Algorithm;
 import org.keycloak.representations.IDToken;
 import org.keycloak.representations.idm.EventRepresentation;
@@ -34,6 +39,7 @@ import org.keycloak.testsuite.TestRealmKeycloakTest;
 import org.keycloak.testsuite.admin.AbstractAdminTest;
 import org.keycloak.testsuite.pages.AppPage;
 import org.keycloak.testsuite.pages.LoginPage;
+import org.keycloak.testsuite.util.ClientManager;
 import org.keycloak.testsuite.util.OAuthClient;
 
 import static org.junit.Assert.assertFalse;
@@ -81,6 +87,21 @@ public abstract class AbstractOIDCResponseTypeTest extends TestRealmKeycloakTest
     }
 
 
+    @Test
+    public void authorizationRequestMissingResponseType() throws IOException {
+        oauth.responseType(null);
+        UriBuilder b = UriBuilder.fromUri(oauth.getLoginFormUrl());
+        driver.navigate().to(b.build().toURL());
+
+        // Always read error from the "query"
+        OAuthClient.AuthorizationEndpointResponse errorResponse = new OAuthClient.AuthorizationEndpointResponse(oauth, false);
+        org.junit.Assert.assertTrue(errorResponse.isRedirected());
+        org.junit.Assert.assertEquals(errorResponse.getError(), OAuthErrorException.INVALID_REQUEST);
+
+        events.expectLogin().error(Errors.INVALID_REQUEST).user((String) null).session((String) null).clearDetails().assertEvent();
+    }
+
+
     protected void validateNonceNotUsedErrorExpected() {
         oauth.nonce(null);
         driver.navigate().to(oauth.getLoginFormUrl());
@@ -97,6 +118,45 @@ public abstract class AbstractOIDCResponseTypeTest extends TestRealmKeycloakTest
     }
 
 
+    protected void validateErrorImplicitFlowNotAllowed() throws Exception {
+        // Disable implicit flow for client
+        clientManagerBuilder().implicitFlow(false);
+
+        UriBuilder b = UriBuilder.fromUri(oauth.getLoginFormUrl());
+        driver.navigate().to(b.build().toURL());
+
+        OAuthClient.AuthorizationEndpointResponse errorResponse = new OAuthClient.AuthorizationEndpointResponse(oauth);
+        Assert.assertTrue(errorResponse.isRedirected());
+        Assert.assertEquals(errorResponse.getError(), OAuthErrorException.UNSUPPORTED_RESPONSE_TYPE);
+        Assert.assertEquals(errorResponse.getErrorDescription(), "Client is not allowed to initiate browser login with given response_type. Implicit flow is disabled for the client.");
+
+        events.expectLogin().error(Errors.NOT_ALLOWED).user((String) null).session((String) null).clearDetails().assertEvent();
+
+        // Revert
+        clientManagerBuilder().implicitFlow(true);
+    }
+
+
+    protected void validateErrorStandardFlowNotAllowed() throws Exception {
+        // Disable standard flow for client
+        clientManagerBuilder().standardFlow(false);
+
+        UriBuilder b = UriBuilder.fromUri(oauth.getLoginFormUrl());
+        driver.navigate().to(b.build().toURL());
+
+        OAuthClient.AuthorizationEndpointResponse errorResponse = new OAuthClient.AuthorizationEndpointResponse(oauth);
+        Assert.assertTrue(errorResponse.isRedirected());
+        Assert.assertEquals(errorResponse.getError(), OAuthErrorException.UNSUPPORTED_RESPONSE_TYPE);
+        Assert.assertEquals(errorResponse.getErrorDescription(), "Client is not allowed to initiate browser login with given response_type. Standard flow is disabled for the client.");
+
+        events.expectLogin().error(Errors.NOT_ALLOWED).user((String) null).session((String) null).clearDetails().assertEvent();
+
+        // Revert
+        clientManagerBuilder().standardFlow(true);
+    }
+
+
+
     protected EventRepresentation loginUser(String nonce) {
         if (nonce != null) {
             oauth.nonce(nonce);
@@ -112,4 +172,8 @@ public abstract class AbstractOIDCResponseTypeTest extends TestRealmKeycloakTest
     }
 
     protected abstract List<IDToken> retrieveIDTokens(EventRepresentation loginEvent);
+
+    protected ClientManager.ClientManagerBuilder clientManagerBuilder() {
+        return ClientManager.realm(adminClient.realm("test")).clientId("test-app");
+    }
 }

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/flows/OIDCBasicResponseTypeCodeTest.java

@@ -39,7 +39,7 @@ public class OIDCBasicResponseTypeCodeTest extends AbstractOIDCResponseTypeTest
 
     @Before
     public void clientConfiguration() {
-        ClientManager.realm(adminClient.realm("test")).clientId("test-app").standardFlow(true).implicitFlow(false);
+        clientManagerBuilder().standardFlow(true).implicitFlow(false);
 
         oauth.clientId("test-app");
         oauth.responseType(OIDCResponseType.CODE);
@@ -68,4 +68,9 @@ public class OIDCBasicResponseTypeCodeTest extends AbstractOIDCResponseTypeTest
             Assert.assertNull(idToken.getNonce());
         }
     }
+
+    @Test
+    public void errorStandardFlowNotAllowed() throws Exception {
+        super.validateErrorStandardFlowNotAllowed();
+    }
 }

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/flows/OIDCHybridResponseTypeCodeIDTokenTest.java

@@ -28,7 +28,6 @@ import org.keycloak.protocol.oidc.utils.OIDCResponseType;
 import org.keycloak.representations.IDToken;
 import org.keycloak.representations.idm.EventRepresentation;
 import org.keycloak.testsuite.Assert;
-import org.keycloak.testsuite.util.ClientManager;
 import org.keycloak.testsuite.util.OAuthClient;
 
 /**
@@ -40,7 +39,7 @@ public class OIDCHybridResponseTypeCodeIDTokenTest extends AbstractOIDCResponseT
 
     @Before
     public void clientConfiguration() {
-        ClientManager.realm(adminClient.realm("test")).clientId("test-app").standardFlow(true).implicitFlow(true);
+        clientManagerBuilder().standardFlow(true).implicitFlow(true);
 
         oauth.clientId("test-app");
         oauth.responseType(OIDCResponseType.CODE + " " + OIDCResponseType.ID_TOKEN);
@@ -72,4 +71,14 @@ public class OIDCHybridResponseTypeCodeIDTokenTest extends AbstractOIDCResponseT
     public void nonceNotUsedErrorExpected() {
         super.validateNonceNotUsedErrorExpected();
     }
+
+    @Test
+    public void errorStandardFlowNotAllowed() throws Exception {
+        super.validateErrorStandardFlowNotAllowed();
+    }
+
+    @Test
+    public void errorImplicitFlowNotAllowed() throws Exception {
+        super.validateErrorImplicitFlowNotAllowed();
+    }
 }

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/flows/OIDCHybridResponseTypeCodeIDTokenTokenTest.java

@@ -28,7 +28,6 @@ import org.keycloak.protocol.oidc.utils.OIDCResponseType;
 import org.keycloak.representations.IDToken;
 import org.keycloak.representations.idm.EventRepresentation;
 import org.keycloak.testsuite.Assert;
-import org.keycloak.testsuite.util.ClientManager;
 import org.keycloak.testsuite.util.OAuthClient;
 
 /**
@@ -40,7 +39,7 @@ public class OIDCHybridResponseTypeCodeIDTokenTokenTest extends AbstractOIDCResp
 
     @Before
     public void clientConfiguration() {
-        ClientManager.realm(adminClient.realm("test")).clientId("test-app").standardFlow(true).implicitFlow(true);
+        clientManagerBuilder().standardFlow(true).implicitFlow(true);
 
         oauth.clientId("test-app");
         oauth.responseType(OIDCResponseType.CODE + " " + OIDCResponseType.ID_TOKEN + " "  + OIDCResponseType.TOKEN);
@@ -73,4 +72,14 @@ public class OIDCHybridResponseTypeCodeIDTokenTokenTest extends AbstractOIDCResp
     public void nonceNotUsedErrorExpected() {
         super.validateNonceNotUsedErrorExpected();
     }
+
+    @Test
+    public void errorStandardFlowNotAllowed() throws Exception {
+        super.validateErrorStandardFlowNotAllowed();
+    }
+
+    @Test
+    public void errorImplicitFlowNotAllowed() throws Exception {
+        super.validateErrorImplicitFlowNotAllowed();
+    }
 }

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/flows/OIDCHybridResponseTypeCodeTokenTest.java

@@ -17,7 +17,6 @@
 
 package org.keycloak.testsuite.oidc.flows;
 
-import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 
@@ -28,7 +27,6 @@ import org.keycloak.protocol.oidc.utils.OIDCResponseType;
 import org.keycloak.representations.IDToken;
 import org.keycloak.representations.idm.EventRepresentation;
 import org.keycloak.testsuite.Assert;
-import org.keycloak.testsuite.util.ClientManager;
 import org.keycloak.testsuite.util.OAuthClient;
 
 /**
@@ -40,7 +38,7 @@ public class OIDCHybridResponseTypeCodeTokenTest extends AbstractOIDCResponseTyp
 
     @Before
     public void clientConfiguration() {
-        ClientManager.realm(adminClient.realm("test")).clientId("test-app").standardFlow(true).implicitFlow(true);
+        clientManagerBuilder().standardFlow(true).implicitFlow(true);
 
         oauth.clientId("test-app");
         oauth.responseType(OIDCResponseType.CODE + " " + OIDCResponseType.TOKEN);
@@ -66,4 +64,14 @@ public class OIDCHybridResponseTypeCodeTokenTest extends AbstractOIDCResponseTyp
         super.validateNonceNotUsedErrorExpected();
     }
 
+    @Test
+    public void errorStandardFlowNotAllowed() throws Exception {
+        super.validateErrorStandardFlowNotAllowed();
+    }
+
+    @Test
+    public void errorImplicitFlowNotAllowed() throws Exception {
+        super.validateErrorImplicitFlowNotAllowed();
+    }
+
 }

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/flows/OIDCImplicitResponseTypeIDTokenTest.java

@@ -27,7 +27,6 @@ import org.keycloak.protocol.oidc.utils.OIDCResponseType;
 import org.keycloak.representations.IDToken;
 import org.keycloak.representations.idm.EventRepresentation;
 import org.keycloak.testsuite.Assert;
-import org.keycloak.testsuite.util.ClientManager;
 import org.keycloak.testsuite.util.OAuthClient;
 
 /**
@@ -39,7 +38,7 @@ public class OIDCImplicitResponseTypeIDTokenTest extends AbstractOIDCResponseTyp
 
     @Before
     public void clientConfiguration() {
-        ClientManager.realm(adminClient.realm("test")).clientId("test-app").standardFlow(false).implicitFlow(true);
+        clientManagerBuilder().standardFlow(false).implicitFlow(true);
 
         oauth.clientId("test-app");
         oauth.responseType(OIDCResponseType.ID_TOKEN);
@@ -66,4 +65,10 @@ public class OIDCImplicitResponseTypeIDTokenTest extends AbstractOIDCResponseTyp
         super.validateNonceNotUsedErrorExpected();
     }
 
+    @Test
+    public void errorImplicitFlowNotAllowed() throws Exception {
+        super.validateErrorImplicitFlowNotAllowed();
+    }
+
+
 }

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/flows/OIDCImplicitResponseTypeIDTokenTokenTest.java

@@ -28,7 +28,6 @@ import org.keycloak.protocol.oidc.utils.OIDCResponseType;
 import org.keycloak.representations.IDToken;
 import org.keycloak.representations.idm.EventRepresentation;
 import org.keycloak.testsuite.Assert;
-import org.keycloak.testsuite.util.ClientManager;
 import org.keycloak.testsuite.util.OAuthClient;
 
 /**
@@ -40,7 +39,7 @@ public class OIDCImplicitResponseTypeIDTokenTokenTest extends AbstractOIDCRespon
 
     @Before
     public void clientConfiguration() {
-        ClientManager.realm(adminClient.realm("test")).clientId("test-app").standardFlow(false).implicitFlow(true);
+        clientManagerBuilder().standardFlow(false).implicitFlow(true);
 
         oauth.clientId("test-app");
         oauth.responseType(OIDCResponseType.ID_TOKEN + " " + OIDCResponseType.TOKEN);
@@ -68,4 +67,9 @@ public class OIDCImplicitResponseTypeIDTokenTokenTest extends AbstractOIDCRespon
     public void nonceNotUsedErrorExpected() {
         super.validateNonceNotUsedErrorExpected();
     }
+
+    @Test
+    public void errorImplicitFlowNotAllowed() throws Exception {
+        super.validateErrorImplicitFlowNotAllowed();
+    }
 }