diff --git a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpoint.java b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpoint.java index b08d3d8298..931b04cfba 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpoint.java +++ b/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()) { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/AuthorizationCodeTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/AuthorizationCodeTest.java index cf69f74883..99ee7c6462 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/AuthorizationCodeTest.java +++ b/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"); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/flows/AbstractOIDCResponseTypeTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/flows/AbstractOIDCResponseTypeTest.java index f4d806a986..28c6ff5077 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/flows/AbstractOIDCResponseTypeTest.java +++ b/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 retrieveIDTokens(EventRepresentation loginEvent); + + protected ClientManager.ClientManagerBuilder clientManagerBuilder() { + return ClientManager.realm(adminClient.realm("test")).clientId("test-app"); + } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/flows/OIDCBasicResponseTypeCodeTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/flows/OIDCBasicResponseTypeCodeTest.java index 7f042f5dc6..355aa42bab 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/flows/OIDCBasicResponseTypeCodeTest.java +++ b/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(); + } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/flows/OIDCHybridResponseTypeCodeIDTokenTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/flows/OIDCHybridResponseTypeCodeIDTokenTest.java index f210b715d5..ebad58aff3 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/flows/OIDCHybridResponseTypeCodeIDTokenTest.java +++ b/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(); + } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/flows/OIDCHybridResponseTypeCodeIDTokenTokenTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/flows/OIDCHybridResponseTypeCodeIDTokenTokenTest.java index 354db5a46d..dd56c7d956 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/flows/OIDCHybridResponseTypeCodeIDTokenTokenTest.java +++ b/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(); + } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/flows/OIDCHybridResponseTypeCodeTokenTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/flows/OIDCHybridResponseTypeCodeTokenTest.java index 9490a7ecf6..4ce4a28bbc 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/flows/OIDCHybridResponseTypeCodeTokenTest.java +++ b/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(); + } + } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/flows/OIDCImplicitResponseTypeIDTokenTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/flows/OIDCImplicitResponseTypeIDTokenTest.java index f5284f353c..2a82cd9057 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/flows/OIDCImplicitResponseTypeIDTokenTest.java +++ b/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(); + } + + } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/flows/OIDCImplicitResponseTypeIDTokenTokenTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/flows/OIDCImplicitResponseTypeIDTokenTokenTest.java index 1ce2e666eb..899d406d96 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/flows/OIDCImplicitResponseTypeIDTokenTokenTest.java +++ b/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(); + } }