Repository: gravitee-io/graviteeio-access-management (java)

Issue:
[oidc] Hybrid flow : Authorization Error Response must be return in the fragment component of the Redirection URI
https://openid.net/specs/openid-connect-core-1_0.html#HybridAuthError

Pull request:
fix(oidc): Authorization Error Response must be return in the fragmen…
…t component of the Redirection URI

fixes #413

Patch:
diff --git a/gravitee-am-gateway/gravitee-am-gateway-handler/src/main/java/io/gravitee/am/gateway/handler/vertx/handler/oauth2/endpoint/authorization/AuthorizationEndpointFailureHandler.java b/gravitee-am-gateway/gravitee-am-gateway-handler/src/main/java/io/gravitee/am/gateway/handler/vertx/handler/oauth2/endpoint/authorization/AuthorizationEndpointFailureHandler.java
index fa7f0735e1..9ba0e9b314 100644
--- a/gravitee-am-gateway/gravitee-am-gateway-handler/src/main/java/io/gravitee/am/gateway/handler/vertx/handler/oauth2/endpoint/authorization/AuthorizationEndpointFailureHandler.java
+++ b/gravitee-am-gateway/gravitee-am-gateway-handler/src/main/java/io/gravitee/am/gateway/handler/vertx/handler/oauth2/endpoint/authorization/AuthorizationEndpointFailureHandler.java
@@ -144,7 +144,7 @@ public class AuthorizationEndpointFailureHandler implements Handler<RoutingConte
             query.put(OAuth2Constants.STATE, authorizationRequest.getState());
         }
 
-        boolean fragment = !isDefaultErrorPage(authorizationRequest.getRedirectUri()) && isImplicitFlow(authorizationRequest.getResponseType());
+        boolean fragment = !isDefaultErrorPage(authorizationRequest.getRedirectUri()) && (isImplicitFlow(authorizationRequest.getResponseType()) || isHybridFlow(authorizationRequest.getResponseType()));
         return append(authorizationRequest.getRedirectUri(), query, fragment);
     }
 
@@ -155,6 +155,12 @@ public class AuthorizationEndpointFailureHandler implements Handler<RoutingConte
                         || ResponseType.ID_TOKEN_TOKEN.equals(responseType));
     }
 
+    private boolean isHybridFlow(String responseType) {
+        return responseType != null && (ResponseType.CODE_ID_TOKEN.equals(responseType)
+                || ResponseType.CODE_TOKEN.equals(responseType)
+                || ResponseType.CODE_ID_TOKEN_TOKEN.equals(responseType));
+    }
+
     private boolean isDefaultErrorPage(String redirectUri) {
         return redirectUri.contains(defaultErrorPagePath);
     }
diff --git a/gravitee-am-gateway/gravitee-am-gateway-handler/src/main/java/io/gravitee/am/gateway/handler/vertx/handler/oauth2/endpoint/authorization/AuthorizationEndpointHandler.java b/gravitee-am-gateway/gravitee-am-gateway-handler/src/main/java/io/gravitee/am/gateway/handler/vertx/handler/oauth2/endpoint/authorization/AuthorizationEndpointHandler.java
index d035930644..8d70e1058a 100644
--- a/gravitee-am-gateway/gravitee-am-gateway-handler/src/main/java/io/gravitee/am/gateway/handler/vertx/handler/oauth2/endpoint/authorization/AuthorizationEndpointHandler.java
+++ b/gravitee-am-gateway/gravitee-am-gateway-handler/src/main/java/io/gravitee/am/gateway/handler/vertx/handler/oauth2/endpoint/authorization/AuthorizationEndpointHandler.java
@@ -95,7 +95,7 @@ public class AuthorizationEndpointHandler implements Handler<RoutingContext> {
                         // else redirect to consent user approval page
                         String prompt = request.getRequestParameters().getFirst(OIDCParameters.PROMPT);
                         if (prompt != null && Arrays.asList(prompt.split("\\s+")).contains("none")) {
-                            context.fail(new InteractionRequiredException());
+                            context.fail(new InteractionRequiredException("Interaction required"));
                         } else {
                             // TODO should we put this data inside repository to handle cluster environment ?
                             context.session().put(OAuth2Constants.AUTHORIZATION_REQUEST, request);
diff --git a/gravitee-am-gateway/gravitee-am-gateway-handler/src/main/java/io/gravitee/am/gateway/handler/vertx/handler/oauth2/endpoint/authorization/AuthorizationRequestParseParametersHandler.java b/gravitee-am-gateway/gravitee-am-gateway-handler/src/main/java/io/gravitee/am/gateway/handler/vertx/handler/oauth2/endpoint/authorization/AuthorizationRequestParseParametersHandler.java
index 95594bfa8d..90ad875eb5 100644
--- a/gravitee-am-gateway/gravitee-am-gateway-handler/src/main/java/io/gravitee/am/gateway/handler/vertx/handler/oauth2/endpoint/authorization/AuthorizationRequestParseParametersHandler.java
+++ b/gravitee-am-gateway/gravitee-am-gateway-handler/src/main/java/io/gravitee/am/gateway/handler/vertx/handler/oauth2/endpoint/authorization/AuthorizationRequestParseParametersHandler.java
@@ -99,7 +99,7 @@ public class AuthorizationRequestParseParametersHandler implements Handler<Routi
             // The Authorization Server MUST NOT display any authentication or consent user interface pages.
             // An error is returned if an End-User is not already authenticated.
             if (promptValues.contains("none") && context.user() == null) {
-                throw new LoginRequiredException();
+                throw new LoginRequiredException("Login required");
             }
 
             // The Authentication Request contains the prompt parameter with the value login.


Test patch:
diff --git a/gravitee-am-gateway/gravitee-am-gateway-handler/src/test/java/io/gravitee/am/gateway/handler/vertx/endpoint/AuthorizationEndpointHandlerTest.java b/gravitee-am-gateway/gravitee-am-gateway-handler/src/test/java/io/gravitee/am/gateway/handler/vertx/endpoint/AuthorizationEndpointHandlerTest.java
index 4e6086f35a..c74c88077d 100644
--- a/gravitee-am-gateway/gravitee-am-gateway-handler/src/test/java/io/gravitee/am/gateway/handler/vertx/endpoint/AuthorizationEndpointHandlerTest.java
+++ b/gravitee-am-gateway/gravitee-am-gateway-handler/src/test/java/io/gravitee/am/gateway/handler/vertx/endpoint/AuthorizationEndpointHandlerTest.java
@@ -433,7 +433,7 @@ public class AuthorizationEndpointHandlerTest  extends RxWebTestBase {
                 resp -> {
                     String location = resp.headers().get("location");
                     assertNotNull(location);
-                    assertEquals("http://localhost:9999/callback?error=login_required", location);
+                    assertEquals("http://localhost:9999/callback?error=login_required&error_description=Login+required", location);
                 },
                 HttpStatusCode.FOUND_302, "Found", null);
     }
@@ -500,7 +500,7 @@ public class AuthorizationEndpointHandlerTest  extends RxWebTestBase {
                 resp -> {
                     String location = resp.headers().get("location");
                     assertNotNull(location);
-                    assertEquals("http://localhost:9999/callback?error=login_required", location);
+                    assertEquals("http://localhost:9999/callback?error=login_required&error_description=Login+required", location);
                 },
                 HttpStatusCode.FOUND_302, "Found", null);
     }