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

Issue:
[oauth2] authorization server must throw invalid request exception if request includes a parameter more than once
```
invalid_request
               The request is missing a required parameter, includes an
               invalid parameter value, includes a parameter more than
               once, or is otherwise malformed.
```

Pull request:
fix(oauth2): handle duplicate parameters
fixes #363

Patch:
diff --git a/gravitee-am-gateway/gravitee-am-gateway-handler/src/main/java/io/gravitee/am/gateway/handler/vertx/handler/oauth2/endpoint/authorization/AuthorizationRequestParseHandler.java b/gravitee-am-gateway/gravitee-am-gateway-handler/src/main/java/io/gravitee/am/gateway/handler/vertx/handler/oauth2/endpoint/authorization/AuthorizationRequestParseHandler.java
index ae02987eb7..47a7b0b34f 100644
--- a/gravitee-am-gateway/gravitee-am-gateway-handler/src/main/java/io/gravitee/am/gateway/handler/vertx/handler/oauth2/endpoint/authorization/AuthorizationRequestParseHandler.java
+++ b/gravitee-am-gateway/gravitee-am-gateway-handler/src/main/java/io/gravitee/am/gateway/handler/vertx/handler/oauth2/endpoint/authorization/AuthorizationRequestParseHandler.java
@@ -32,6 +32,7 @@ import io.gravitee.am.model.Domain;
 import io.gravitee.common.http.HttpHeaders;
 import io.vertx.core.Handler;
 import io.vertx.core.json.Json;
+import io.vertx.reactivex.core.MultiMap;
 import io.vertx.reactivex.ext.auth.User;
 import io.vertx.reactivex.ext.web.RoutingContext;
 import org.slf4j.Logger;
@@ -41,6 +42,7 @@ import java.net.URISyntaxException;
 import java.util.Arrays;
 import java.util.Date;
 import java.util.List;
+import java.util.Set;
 
 /**
  * The authorization server validates the request to ensure that all required parameters are present and valid.
@@ -66,6 +68,9 @@ public class AuthorizationRequestParseHandler implements Handler<RoutingContext>
 
     @Override
     public void handle(RoutingContext context) {
+        // proceed request parameters
+        parseRequestParameters(context);
+
         // proceed response type parameter
         parseResponseTypeParameter(context);
 
@@ -93,6 +98,19 @@ public class AuthorizationRequestParseHandler implements Handler<RoutingContext>
         context.next();
     }
 
+    private void parseRequestParameters(RoutingContext context) {
+        // invalid_request if the request is missing a required parameter, includes an
+        // invalid parameter value, includes a parameter more than once, or is otherwise malformed.
+        MultiMap requestParameters = context.request().params();
+        Set<String> requestParametersNames = requestParameters.names();
+        requestParametersNames.forEach(requestParameterName -> {
+            List<String> requestParameterValue = requestParameters.getAll(requestParameterName);
+            if (requestParameterValue.size() > 1) {
+                throw new InvalidRequestException("Parameter [" + requestParameterName + "] is included more than once");
+            }
+        });
+    }
+
     private void parseScopeParameter(RoutingContext context) {
         // Check scope parameter
         String scopes = context.request().params().get(OAuth2Constants.SCOPE);
diff --git a/gravitee-am-gateway/gravitee-am-gateway-handler/src/main/java/io/gravitee/am/gateway/handler/vertx/handler/oauth2/endpoint/token/TokenRequestParseHandler.java b/gravitee-am-gateway/gravitee-am-gateway-handler/src/main/java/io/gravitee/am/gateway/handler/vertx/handler/oauth2/endpoint/token/TokenRequestParseHandler.java
index a13196fa1b..172b09bb74 100644
--- a/gravitee-am-gateway/gravitee-am-gateway-handler/src/main/java/io/gravitee/am/gateway/handler/vertx/handler/oauth2/endpoint/token/TokenRequestParseHandler.java
+++ b/gravitee-am-gateway/gravitee-am-gateway-handler/src/main/java/io/gravitee/am/gateway/handler/vertx/handler/oauth2/endpoint/token/TokenRequestParseHandler.java
@@ -17,12 +17,14 @@ package io.gravitee.am.gateway.handler.vertx.handler.oauth2.endpoint.token;
 
 import io.gravitee.am.gateway.handler.oauth2.exception.InvalidRequestException;
 import io.gravitee.am.gateway.handler.oauth2.exception.InvalidScopeException;
-import io.gravitee.am.gateway.handler.oauth2.request.TokenRequest;
 import io.gravitee.am.gateway.handler.oauth2.utils.OAuth2Constants;
-import io.gravitee.am.gateway.handler.vertx.handler.oauth2.request.TokenRequestFactory;
 import io.vertx.core.Handler;
+import io.vertx.reactivex.core.MultiMap;
 import io.vertx.reactivex.ext.web.RoutingContext;
 
+import java.util.List;
+import java.util.Set;
+
 /**
  * The authorization server validates the Access Token Request to ensure that all required parameters are present and valid.
  * If the request is valid, the authorization server authenticates the client and obtains
@@ -33,23 +35,46 @@ import io.vertx.reactivex.ext.web.RoutingContext;
  */
 public class TokenRequestParseHandler implements Handler<RoutingContext> {
 
-    private final TokenRequestFactory tokenRequestFactory = new TokenRequestFactory();
-
     @Override
     public void handle(RoutingContext context) {
-        TokenRequest tokenRequest = tokenRequestFactory.create(context.request());
+        // proceed request parameters
+        parseRequestParameters(context);
+
+        // proceed grant_type parameter
+        parseGrantTypeParameter(context);
+
+        // proceed scope parameter
+        parseScopeParameter(context);
+
+        context.next();
+    }
 
-        // Check if a grant_type is defined
-        if (tokenRequest.getGrantType() == null) {
+    private void parseRequestParameters(RoutingContext context) {
+        // invalid_request if the request is missing a required parameter, includes an
+        // invalid parameter value, includes a parameter more than once, or is otherwise malformed.
+        MultiMap requestParameters = context.request().params();
+        Set<String> requestParametersNames = requestParameters.names();
+        requestParametersNames.forEach(requestParameterName -> {
+            List<String> requestParameterValue = requestParameters.getAll(requestParameterName);
+            if (requestParameterValue.size() > 1) {
+                throw new InvalidRequestException("Parameter [" + requestParameterName + "] is included more than once");
+            }
+        });
+    }
+
+    private void parseGrantTypeParameter(RoutingContext context) {
+        String grantType = context.request().getParam(OAuth2Constants.GRANT_TYPE);
+
+        if (grantType == null) {
             throw new InvalidRequestException("Missing parameter: grant_type");
         }
+    }
 
+    private void parseScopeParameter(RoutingContext context) {
         // Check scope parameter
         String scopes = context.request().params().get(OAuth2Constants.SCOPE);
         if (scopes != null && scopes.isEmpty()) {
             throw new InvalidScopeException("Invalid parameter: scope must not be empty");
         }
-
-        context.next();
     }
 }


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 ad2ffbbca7..345cdb6ffc 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
@@ -260,6 +260,22 @@ public class AuthorizationEndpointHandlerTest  extends RxWebTestBase {
                 HttpStatusCode.FOUND_302, "Found", null);
     }
 
+    @Test
+    public void shouldNotInvokeAuthorizationEndpoint_duplicateParameters() throws Exception {
+        when(domain.getPath()).thenReturn("test");
+
+        testRequest(
+                HttpMethod.GET,
+                "/oauth/authorize?response_type=code&response_type=code&client_id=client-id&redirect_uri=http://localhost:9999/callback",
+                null,
+                resp -> {
+                    String location = resp.headers().get("location");
+                    assertNotNull(location);
+                    assertEquals("/test/oauth/error?error=invalid_request&error_description=Parameter+%255Bresponse_type%255D+is+included+more+than+once", location);
+                },
+                HttpStatusCode.FOUND_302, "Found", null);
+    }
+
     @Test
     public void shouldInvokeAuthorizationEndpoint_approvalPage() throws Exception {
         AuthorizationRequest authorizationRequest = new AuthorizationRequest();
diff --git a/gravitee-am-gateway/gravitee-am-gateway-handler/src/test/java/io/gravitee/am/gateway/handler/vertx/endpoint/TokenEndpointHandlerTest.java b/gravitee-am-gateway/gravitee-am-gateway-handler/src/test/java/io/gravitee/am/gateway/handler/vertx/endpoint/TokenEndpointHandlerTest.java
index 0c09952b5c..0cd81e5674 100644
--- a/gravitee-am-gateway/gravitee-am-gateway-handler/src/test/java/io/gravitee/am/gateway/handler/vertx/endpoint/TokenEndpointHandlerTest.java
+++ b/gravitee-am-gateway/gravitee-am-gateway-handler/src/test/java/io/gravitee/am/gateway/handler/vertx/endpoint/TokenEndpointHandlerTest.java
@@ -117,6 +117,13 @@ public class TokenEndpointHandlerTest extends RxWebTestBase {
                 HttpStatusCode.UNAUTHORIZED_401, "Unauthorized");
     }
 
+    @Test
+    public void shouldNotInvokeAuthorizationEndpoint_duplicateParameters() throws Exception {
+        testRequest(
+                HttpMethod.POST, "/oauth/token?grant_type=client_credentials&grant_type=client_credentials",
+                HttpStatusCode.BAD_REQUEST_400, "Bad Request");
+    }
+
     @Test
     public void shouldInvokeTokenEndpoint_withValidClientCredentials_noGrantType() throws Exception {
         io.gravitee.am.model.Client client = new io.gravitee.am.model.Client();