Merge pull request #3031 from mposolda/master

KEYCLOAK-3220 redirect to client with error if possible

core/src/main/java/org/keycloak/OAuthErrorException.java

@@ -22,11 +22,22 @@ package org.keycloak;
  * @version $Revision: 1 $
  */
 public class OAuthErrorException extends Exception {
+    // OAuth2
     public static final String INVALID_REQUEST = "invalid_request";
+    public static final String INVALID_SCOPE = "invalid_scope";
+    public static final String UNAUTHORIZED_CLIENT = "unauthorized_client";
+    public static final String ACCESS_DENIED = "access_denied";
+    public static final String UNSUPPORTED_RESPONSE_TYPE = "unsupported_response_type";
+    public static final String SERVER_ERROR = "server_error";
+    public static final String TEMPORARILY_UNAVAILABKE = "temporarily_unavailable";
+
+    // OpenID Connect 1
+    public static final String INTERACTION_REQUIRED = "interaction_required";
+    public static final String LOGIN_REQUIRED = "login_required";
+
+    // Others
     public static final String INVALID_CLIENT = "invalid_client";
     public static final String INVALID_GRANT = "invalid_grant";
-    public static final String INVALID_SCOPE = "invalid_grant";
-    public static final String UNAUTHORIZED_CLIENT = "unauthorized_client";
     public static final String UNSUPPORTED_GRANT_TYPE = "unsupported_grant_type";
     public static final String INVALID_TOKEN = "invalid_token";
     public static final String INSUFFICIENT_SCOPE = "insufficient_scope";

services/src/main/java/org/keycloak/protocol/oidc/OIDCLoginProtocol.java

@@ -17,6 +17,7 @@
 package org.keycloak.protocol.oidc;
 
 import org.keycloak.OAuth2Constants;
+import org.keycloak.OAuthErrorException;
 import org.keycloak.events.Details;
 import org.keycloak.events.EventBuilder;
 import org.keycloak.events.EventType;
@@ -193,14 +194,14 @@ public class OIDCLoginProtocol implements LoginProtocol {
         switch (error) {
             case CANCELLED_BY_USER:
             case CONSENT_DENIED:
-                return "access_denied";
+                return OAuthErrorException.ACCESS_DENIED;
             case PASSIVE_INTERACTION_REQUIRED:
-                return "interaction_required";
+                return OAuthErrorException.INTERACTION_REQUIRED;
             case PASSIVE_LOGIN_REQUIRED:
-                return "login_required";
+                return OAuthErrorException.LOGIN_REQUIRED;
             default:
                 logger.untranslatedProtocol(error.name());
-                return "access_denied";
+                return OAuthErrorException.SERVER_ERROR;
         }
     }
 

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

@@ -26,6 +26,8 @@ import javax.ws.rs.GET;
 import javax.ws.rs.core.MultivaluedMap;
 import javax.ws.rs.core.Response;
 
+import org.keycloak.OAuth2Constants;
+import org.keycloak.OAuthErrorException;
 import org.keycloak.authentication.AuthenticationProcessor;
 import org.keycloak.constants.AdapterConstants;
 import org.keycloak.events.Details;
@@ -40,6 +42,7 @@ import org.keycloak.models.IdentityProviderModel;
 import org.keycloak.models.RealmModel;
 import org.keycloak.protocol.AuthorizationEndpointBase;
 import org.keycloak.protocol.oidc.OIDCLoginProtocol;
+import org.keycloak.protocol.oidc.utils.OIDCRedirectUriBuilder;
 import org.keycloak.protocol.oidc.utils.OIDCResponseMode;
 import org.keycloak.protocol.oidc.utils.OIDCResponseType;
 import org.keycloak.protocol.oidc.utils.RedirectUtils;
@@ -146,9 +149,12 @@ public class AuthorizationEndpoint extends AuthorizationEndpointBase {
 
         checkSsl();
         checkRealm();
-        checkResponseType();
         checkClient();
         checkRedirectUri();
+        Response errorResponse = checkResponseType();
+        if (errorResponse != null) {
+            return errorResponse;
+        }
 
         createClientSession();
         // So back button doesn't work
@@ -236,28 +242,25 @@ public class AuthorizationEndpoint extends AuthorizationEndpointBase {
             throw new ErrorPageException(session, Messages.CLIENT_NOT_FOUND);
         }
 
+        if (!client.isEnabled()) {
+            event.error(Errors.CLIENT_DISABLED);
+            throw new ErrorPageException(session, Messages.CLIENT_DISABLED);
+        }
+
         if (client.isBearerOnly()) {
             event.error(Errors.NOT_ALLOWED);
             throw new ErrorPageException(session, Messages.BEARER_ONLY);
         }
 
-        if ((parsedResponseType.hasResponseType(OIDCResponseType.CODE) || parsedResponseType.hasResponseType(OIDCResponseType.NONE)) && !client.isStandardFlowEnabled()) {
-            event.error(Errors.NOT_ALLOWED);
-            throw new ErrorPageException(session, Messages.STANDARD_FLOW_DISABLED);
-        }
-
-        if (parsedResponseType.isImplicitOrHybridFlow() && !client.isImplicitFlowEnabled()) {
-            event.error(Errors.NOT_ALLOWED);
-            throw new ErrorPageException(session, Messages.IMPLICIT_FLOW_DISABLED);
-        }
-
         session.getContext().setClient(client);
     }
 
-    private void checkResponseType() {
+    private Response checkResponseType() {
+        OIDCResponseMode defaultResponseMode = client.isImplicitFlowEnabled() ? OIDCResponseMode.FRAGMENT : OIDCResponseMode.QUERY;
+
         if (responseType == null) {
             event.error(Errors.INVALID_REQUEST);
-            throw new ErrorPageException(session, Messages.MISSING_PARAMETER, OIDCLoginProtocol.RESPONSE_TYPE_PARAM);
+            return redirectErrorToClient(defaultResponseMode, OAuthErrorException.INVALID_REQUEST, "Missing parameter: response_type");
         }
 
         event.detail(Details.RESPONSE_TYPE, responseType);
@@ -270,24 +273,51 @@ public class AuthorizationEndpoint extends AuthorizationEndpointBase {
         } catch (IllegalArgumentException iae) {
             logger.error(iae);
             event.error(Errors.INVALID_REQUEST);
-            throw new ErrorPageException(session, Messages.INVALID_PARAMETER, OIDCLoginProtocol.RESPONSE_TYPE_PARAM);
+            return redirectErrorToClient(defaultResponseMode, OAuthErrorException.UNSUPPORTED_RESPONSE_TYPE, null);
         }
 
+        OIDCResponseMode parsedResponseMode = null;
         try {
-            OIDCResponseMode parsedResponseMode = OIDCResponseMode.parse(responseMode, parsedResponseType);
+            parsedResponseMode = OIDCResponseMode.parse(responseMode, parsedResponseType);
-            event.detail(Details.RESPONSE_MODE, parsedResponseMode.toString().toLowerCase());
-
-            // Disallowed by OIDC specs
-            if (parsedResponseType.isImplicitOrHybridFlow() && parsedResponseMode == OIDCResponseMode.QUERY) {
-                logger.responseModeQueryNotAllowed();
-                event.error(Errors.INVALID_REQUEST);
-                throw new ErrorPageException(session, Messages.INVALID_PARAMETER, OIDCLoginProtocol.RESPONSE_MODE_PARAM);
-            }
-
         } catch (IllegalArgumentException iae) {
             event.error(Errors.INVALID_REQUEST);
-            throw new ErrorPageException(session, Messages.INVALID_PARAMETER, OIDCLoginProtocol.RESPONSE_MODE_PARAM);
+            return redirectErrorToClient(defaultResponseMode, OAuthErrorException.INVALID_REQUEST, "Invalid parameter: response_mode");
         }
+
+        event.detail(Details.RESPONSE_MODE, parsedResponseMode.toString().toLowerCase());
+
+        // Disallowed by OIDC specs
+        if (parsedResponseType.isImplicitOrHybridFlow() && parsedResponseMode == OIDCResponseMode.QUERY) {
+            event.error(Errors.INVALID_REQUEST);
+            return redirectErrorToClient(defaultResponseMode, OAuthErrorException.INVALID_REQUEST, "Response_mode 'query' not allowed for implicit or hybrid flow");
+        }
+
+        if ((parsedResponseType.hasResponseType(OIDCResponseType.CODE) || parsedResponseType.hasResponseType(OIDCResponseType.NONE)) && !client.isStandardFlowEnabled()) {
+            event.error(Errors.NOT_ALLOWED);
+            return redirectErrorToClient(parsedResponseMode, OAuthErrorException.UNSUPPORTED_RESPONSE_TYPE, "Client is not allowed to initiate browser login with given response_type. Standard flow is disabled for the client.");
+        }
+
+        if (parsedResponseType.isImplicitOrHybridFlow() && !client.isImplicitFlowEnabled()) {
+            event.error(Errors.NOT_ALLOWED);
+            return redirectErrorToClient(parsedResponseMode, OAuthErrorException.UNSUPPORTED_RESPONSE_TYPE, "Client is not allowed to initiate browser login with given response_type. Implicit flow is disabled for the client.");
+        }
+
+        return null;
+    }
+
+    private Response redirectErrorToClient(OIDCResponseMode responseMode, String error, String errorDescription) {
+        OIDCRedirectUriBuilder errorResponseBuilder = OIDCRedirectUriBuilder.fromUri(redirectUri, responseMode)
+                .addParam(OAuth2Constants.ERROR, error);
+
+        if (errorDescription != null) {
+            errorResponseBuilder.addParam(OAuth2Constants.ERROR_DESCRIPTION, errorDescription);
+        }
+
+        if (state != null) {
+            errorResponseBuilder.addParam(OAuth2Constants.STATE, state);
+        }
+
+        return errorResponseBuilder.build();
     }
 
     private void checkRedirectUri() {

services/src/main/java/org/keycloak/protocol/oidc/utils/OIDCRedirectUriBuilder.java

@@ -39,6 +39,7 @@ public abstract class OIDCRedirectUriBuilder {
     }
 
     public abstract OIDCRedirectUriBuilder addParam(String paramName, String paramValue);
+
     public abstract Response build();
 
 

services/src/main/java/org/keycloak/protocol/oidc/utils/OIDCResponseType.java

@@ -78,7 +78,7 @@ public class OIDCResponseType {
             throw new IllegalStateException("No responseType provided");
         }
         if (responseTypes.contains(NONE) && responseTypes.size() > 1) {
-            throw new IllegalArgumentException("None not allowed with some other response_type");
+            throw new IllegalArgumentException("'None' not allowed with some other response_type");
         }
 
         // response_type value "token" alone is not mentioned in OIDC specification, however it is supported by OAuth2. We allow it just to be compatible with pure OAuth2 clients like swagger.ui

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

@@ -188,6 +188,8 @@ public class Messages {
 
     public static final String CLIENT_NOT_FOUND = "clientNotFoundMessage";
 
+    public static final String CLIENT_DISABLED = "clientDisabledMessage";
+
     public static final String INVALID_PARAMETER = "invalidParameterMessage";
 
     public static final String IDENTITY_PROVIDER_LOGIN_FAILURE = "identityProviderLoginFailure";

testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/OAuthClient.java

@@ -57,7 +57,9 @@ import java.io.IOException;
 import java.io.UnsupportedEncodingException;
 import java.net.URI;
 import java.net.URISyntaxException;
+import java.nio.charset.Charset;
 import java.security.PublicKey;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.LinkedList;
 import java.util.List;
@@ -449,7 +451,11 @@ public class OAuthClient {
     }
 
     public String getCurrentRequest() {
-        return driver.getCurrentUrl().substring(0, driver.getCurrentUrl().indexOf('?'));
+        int index = driver.getCurrentUrl().indexOf('?');
+        if (index == -1) {
+            index = driver.getCurrentUrl().indexOf('#');
+        }
+        return driver.getCurrentUrl().substring(0, index);
     }
 
     public URI getCurrentUri() {
@@ -469,6 +475,18 @@ public class OAuthClient {
         return m;
     }
 
+    public Map<String, String> getCurrentFragment() {
+        Map<String, String> m = new HashMap<String, String>();
+
+        String fragment = getCurrentUri().getRawFragment();
+        List<NameValuePair> pairs = (fragment == null || fragment.isEmpty()) ? Collections.emptyList() : URLEncodedUtils.parse(fragment, Charset.forName("UTF-8"));
+
+        for (NameValuePair p : pairs) {
+            m.put(p.getName(), p.getValue());
+        }
+        return m;
+    }
+
     public void openLoginForm() {
         driver.navigate().to(getLoginFormUrl());
     }
@@ -624,12 +642,20 @@ public class OAuthClient {
         private String code;
         private String state;
         private String error;
+        private String errorDescription;
 
         public AuthorizationCodeResponse(OAuthClient client) {
+            this(client, false);
+        }
+
+        public AuthorizationCodeResponse(OAuthClient client, boolean fragment) {
             isRedirected = client.getCurrentRequest().equals(client.getRedirectUri());
-            code = client.getCurrentQuery().get(OAuth2Constants.CODE);
+            Map<String, String> params = fragment ? client.getCurrentFragment() : client.getCurrentQuery();
-            state = client.getCurrentQuery().get(OAuth2Constants.STATE);
+
-            error = client.getCurrentQuery().get(OAuth2Constants.ERROR);
+            code = params.get(OAuth2Constants.CODE);
+            state = params.get(OAuth2Constants.STATE);
+            error = params.get(OAuth2Constants.ERROR);
+            errorDescription = params.get(OAuth2Constants.ERROR_DESCRIPTION);
         }
 
         public boolean isRedirected() {
@@ -648,6 +674,9 @@ public class OAuthClient {
             return error;
         }
 
+        public String getErrorDescription() {
+            return errorDescription;
+        }
     }
 
     public static class AccessTokenResponse {

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

@@ -22,6 +22,7 @@ import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.keycloak.OAuth2Constants;
+import org.keycloak.OAuthErrorException;
 import org.keycloak.events.Details;
 import org.keycloak.events.Errors;
 import org.keycloak.models.Constants;
@@ -148,7 +149,12 @@ public class AuthorizationCodeTest extends AbstractKeycloakTest {
         oauth.responseType("token id_token");
         UriBuilder b = UriBuilder.fromUri(oauth.getLoginFormUrl());
         driver.navigate().to(b.build().toURL());
-        assertEquals("Client is not allowed to initiate browser login with given response_type. Implicit flow is disabled for the client.", errorPage.getError());
+
+        OAuthClient.AuthorizationCodeResponse errorResponse = new OAuthClient.AuthorizationCodeResponse(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();
     }
 
@@ -157,8 +163,12 @@ public class AuthorizationCodeTest extends AbstractKeycloakTest {
         oauth.responseType("tokenn");
         UriBuilder b = UriBuilder.fromUri(oauth.getLoginFormUrl());
         driver.navigate().to(b.build().toURL());
-        assertEquals("Invalid parameter: response_type", errorPage.getError());
+
-        events.expectLogin().error(Errors.INVALID_REQUEST).client((String) null).user((String) null).session((String) null).clearDetails().detail(Details.RESPONSE_TYPE, "tokenn").assertEvent();
+        OAuthClient.AuthorizationCodeResponse errorResponse = new OAuthClient.AuthorizationCodeResponse(oauth);
+        Assert.assertTrue(errorResponse.isRedirected());
+        Assert.assertEquals(errorResponse.getError(), OAuthErrorException.UNSUPPORTED_RESPONSE_TYPE);
+
+        events.expectLogin().error(Errors.INVALID_REQUEST).user((String) null).session((String) null).clearDetails().detail(Details.RESPONSE_TYPE, "tokenn").assertEvent();
     }
 
     // KEYCLOAK-3281

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

@@ -221,6 +221,7 @@ locale_ru=\u0420\u0443\u0441\u0441\u043A\u0438\u0439
 backToApplication=« Back to Application
 missingParameterMessage=Missing parameters\: {0}
 clientNotFoundMessage=Client not found.
+clientDisabledMessage=Client disabled.
 invalidParameterMessage=Invalid parameter\: {0}
 alreadyLoggedIn=You are already logged in.