fix: check PKCE is used for FAPI using PAR authentication request

fixes gravitee-io/issues#5973
gravitee-io · Sep 1, 2021 · 5ad536d · 5ad536d
1 parent 19be0db
commit 5ad536d
Show file tree

Hide file tree

Showing 12 changed files with 29 additions and 18 deletions.
diff --git a/...andler-common/src/main/java/io/gravitee/am/gateway/handler/common/utils/ConstantKeys.java b/...andler-common/src/main/java/io/gravitee/am/gateway/handler/common/utils/ConstantKeys.java
@@ -99,4 +99,6 @@ public interface ConstantKeys {
     String REQUEST_OBJECT_KEY = "requestObject";
     // identifier of the Pushed Authorization Parameters
     String REQUEST_URI_ID_KEY = "requestUriId";
+    String REQUEST_OBJECT_FROM_URI = "request-object-from-uri";
+
 }
diff --git a/...eway-handler-oidc/src/main/java/io/gravitee/am/gateway/handler/oauth2/OAuth2Provider.java b/...eway-handler-oidc/src/main/java/io/gravitee/am/gateway/handler/oauth2/OAuth2Provider.java
@@ -213,7 +213,7 @@ private void initRouter() {
                 .handler(authenticationFlowHandler.create())
                 .handler(new AuthorizationRequestResolveHandler())
                 .handler(new AuthorizationRequestEndUserConsentHandler(userConsentService))
-                .handler(new AuthorizationEndpoint(flow, thymeleafTemplateEngine, parService))
+                .handler(new AuthorizationEndpoint(flow, thymeleafTemplateEngine, parService, domain))
                 .failureHandler(new AuthorizationRequestFailureHandler(openIDDiscoveryService, jwtService, jweService));
 
         // Authorization consent endpoint

diff --git a/...tee/am/gateway/handler/oauth2/resources/endpoint/authorization/AuthorizationEndpoint.java b/...tee/am/gateway/handler/oauth2/resources/endpoint/authorization/AuthorizationEndpoint.java
@@ -24,6 +24,7 @@
 import io.gravitee.am.gateway.handler.oauth2.service.request.AuthorizationRequest;
 import io.gravitee.am.gateway.handler.oauth2.service.response.AuthorizationResponse;
 import io.gravitee.am.gateway.handler.oidc.service.flow.Flow;
+import io.gravitee.am.model.Domain;
 import io.gravitee.am.model.oidc.Client;
 import io.gravitee.common.http.HttpHeaders;
 import io.gravitee.common.http.MediaType;
@@ -55,10 +56,12 @@ public class AuthorizationEndpoint implements Handler<RoutingContext> {
     private final Flow flow;
     private final ThymeleafTemplateEngine engine;
     private final PushedAuthorizationRequestService parService;
+    private final Domain domain;
 
-    public AuthorizationEndpoint(Flow flow, ThymeleafTemplateEngine engine, PushedAuthorizationRequestService parService) {
+    public AuthorizationEndpoint(Flow flow, ThymeleafTemplateEngine engine, PushedAuthorizationRequestService parService, Domain domain) {
         this.flow = flow;
         this.engine = engine;
+        this.domain = domain;
         this.parService = parService;
     }
 

diff --git a/...er/oauth2/resources/handler/authorization/AuthorizationRequestParseParametersHandler.java b/...er/oauth2/resources/handler/authorization/AuthorizationRequestParseParametersHandler.java
@@ -46,12 +46,10 @@
 import java.net.MalformedURLException;
 import java.net.URISyntaxException;
 import java.net.URL;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.Date;
-import java.util.List;
+import java.util.*;
 
 import static io.gravitee.am.gateway.handler.common.utils.ConstantKeys.PROVIDER_METADATA_CONTEXT_KEY;
+import static io.gravitee.am.gateway.handler.common.utils.ConstantKeys.REQUEST_OBJECT_FROM_URI;
 import static io.gravitee.am.gateway.handler.common.vertx.utils.UriBuilderRequest.CONTEXT_PATH;
 import static io.gravitee.am.gateway.handler.oauth2.resources.handler.authorization.ParamUtils.getOAuthParameter;
 import static io.gravitee.am.service.utils.ResponseTypeUtils.requireNonce;
@@ -145,16 +143,20 @@ private void parsePKCEParameter(RoutingContext context, Client client) {
             throw new InvalidRequestException("Missing parameter: code_challenge");
         }
 
-        if (codeChallenge == null && client.isForcePKCE()) {
+        final boolean pkceRequiredByFapi = this.domain.usePlainFapiProfile() && Optional.ofNullable((Boolean)context.get(REQUEST_OBJECT_FROM_URI)).orElse(Boolean.FALSE);
+        if (codeChallenge == null && (client.isForcePKCE() || pkceRequiredByFapi)) {
             throw new InvalidRequestException("Missing parameter: code_challenge");
         }
 
         if (codeChallenge != null) {
             if (codeChallengeMethod != null) {
                 // https://tools.ietf.org/html/rfc7636#section-4.2
                 // It must be plain or S256
-                if (!CodeChallengeMethod.S256.equalsIgnoreCase(codeChallengeMethod) &&
-                        !CodeChallengeMethod.PLAIN.equalsIgnoreCase(codeChallengeMethod)) {
+                // For FAPI, only S256 is allowed for PKCE
+                // https://openid.net/specs/openid-financial-api-part-2-1_0.html#authorization-server (point 18)
+                if ((this.domain.usePlainFapiProfile() && !CodeChallengeMethod.S256.equalsIgnoreCase(codeChallengeMethod)) ||
+                        (!CodeChallengeMethod.S256.equalsIgnoreCase(codeChallengeMethod) &&
+                        !CodeChallengeMethod.PLAIN.equalsIgnoreCase(codeChallengeMethod))) {
                     throw new InvalidRequestException("Invalid parameter: code_challenge_method");
                 }
             } else {

diff --git a/...oauth2/resources/handler/authorization/AuthorizationRequestParseRequestObjectHandler.java b/...oauth2/resources/handler/authorization/AuthorizationRequestParseRequestObjectHandler.java
@@ -52,8 +52,7 @@
  * @author David BRASSELY (david.brassely at graviteesource.com)
  * @author GraviteeSource Team
  */
-public class AuthorizationRequestParseRequestObjectHandler implements Handler<RoutingContext> {
-    private static final String REQUEST_OBJECT_FROM_URI = "ro-from-uri";
+public class AuthorizationRequestParseRequestObjectHandler extends AbstractAuthorizationRequestHandler implements Handler<RoutingContext> {
     private static final String HTTPS_SCHEME  = "https";
 
     // When the request parameter is used, the OpenID Connect request parameter values contained in the JWT supersede those passed using the OAuth 2.0 request syntax.
@@ -67,7 +66,9 @@ public class AuthorizationRequestParseRequestObjectHandler implements Handler<Ro
                             io.gravitee.am.common.oauth2.Parameters.REDIRECT_URI,
                             io.gravitee.am.common.oauth2.Parameters.SCOPE,
                             io.gravitee.am.common.oauth2.Parameters.RESPONSE_MODE,
-                            io.gravitee.am.common.oauth2.Parameters.STATE
+                            io.gravitee.am.common.oauth2.Parameters.STATE,
+                            io.gravitee.am.common.oauth2.Parameters.CODE_CHALLENGE,
+                            io.gravitee.am.common.oauth2.Parameters.CODE_CHALLENGE_METHOD
                             )).flatMap(Collection::stream).collect(Collectors.toList());
     public static final int ONE_HOUR_IN_MILLIS = 3600 * 1000;
 

diff --git a/...ain/java/io/gravitee/am/gateway/handler/oauth2/resources/request/TokenRequestFactory.java b/...ain/java/io/gravitee/am/gateway/handler/oauth2/resources/request/TokenRequestFactory.java
@@ -77,6 +77,7 @@ public TokenRequest create(RoutingContext context) {
         String scope = request.params().get(Parameters.SCOPE);
         tokenRequest.setScopes(scope != null && !scope.isEmpty() ? new HashSet<>(Arrays.asList(scope.split("\\s+"))) : null);
         tokenRequest.setAdditionalParameters(extractAdditionalParameters(request));
+
         return tokenRequest;
     }
 

diff --git a/...ravitee/am/gateway/handler/oauth2/service/granter/code/AuthorizationCodeTokenGranter.java b/...ravitee/am/gateway/handler/oauth2/service/granter/code/AuthorizationCodeTokenGranter.java
@@ -29,8 +29,9 @@
 import io.gravitee.am.gateway.handler.oauth2.service.request.TokenRequestResolver;
 import io.gravitee.am.gateway.handler.oauth2.service.token.TokenService;
 import io.gravitee.am.model.AuthenticationFlowContext;
-import io.gravitee.am.model.oidc.Client;
+import io.gravitee.am.model.Domain;
 import io.gravitee.am.model.User;
+import io.gravitee.am.model.oidc.Client;
 import io.gravitee.am.repository.oauth2.model.AuthorizationCode;
 import io.gravitee.am.service.AuthenticationFlowContextService;
 import io.gravitee.common.util.MultiValueMap;
@@ -92,7 +93,7 @@ protected Single<TokenRequest> parseRequest(TokenRequest tokenRequest, Client cl
                                     .onErrorResumeNext(error -> (exitOnError) ? Maybe.error(error) : Maybe.just(new AuthenticationFlowContext()))
                                     .map(ctx -> {
                                         checkRedirectUris(tokenRequest1, authorizationCode);
-                                        checkPKCE(tokenRequest1, authorizationCode);
+                                        checkPKCE( tokenRequest1, authorizationCode);
                                         // set resource owner
                                         tokenRequest1.setSubject(authorizationCode.getSubject());
                                         // set original scopes

diff --git a/...idc/src/main/java/io/gravitee/am/gateway/handler/oauth2/service/request/TokenRequest.java b/...idc/src/main/java/io/gravitee/am/gateway/handler/oauth2/service/request/TokenRequest.java
@@ -65,7 +65,6 @@ public class TokenRequest extends OAuth2Request {
      */
     private String requestingPartyToken;
 
-
     public String getUsername() {
         return username;
     }

diff --git a/...a/io/gravitee/am/gateway/handler/oauth2/resources/endpoint/AuthorizationEndpointTest.java b/...a/io/gravitee/am/gateway/handler/oauth2/resources/endpoint/AuthorizationEndpointTest.java
@@ -101,7 +101,7 @@ public class AuthorizationEndpointTest extends RxWebTestBase {
     public void setUp() throws Exception {
         super.setUp();
 
-        AuthorizationEndpoint authorizationEndpointHandler = new AuthorizationEndpoint(flow, thymeleafTemplateEngine, parService);
+        AuthorizationEndpoint authorizationEndpointHandler = new AuthorizationEndpoint(flow, thymeleafTemplateEngine, parService, domain);
 
         // set openid provider service
         OpenIDProviderMetadata openIDProviderMetadata = new OpenIDProviderMetadata();

diff --git a/.../src/main/java/io/gravitee/am/repository/jdbc/oauth2/api/model/JdbcAuthorizationCode.java b/.../src/main/java/io/gravitee/am/repository/jdbc/oauth2/api/model/JdbcAuthorizationCode.java
@@ -135,4 +135,5 @@ public String getRequestParameters() {
     public void setRequestParameters(String requestParameters) {
         this.requestParameters = requestParameters;
     }
+
 }
diff --git a/...ravitee-am-repository-jdbc/src/main/resources/liquibase/changelogs/v3_11_0/schema-par.yml b/...ravitee-am-repository-jdbc/src/main/resources/liquibase/changelogs/v3_11_0/schema-par.yml
@@ -1,6 +1,6 @@
 databaseChangeLog:
   - changeSet:
-      id: 3.11.0
+      id: 3.11.0-PAR-Table
       author: GraviteeSource Team
       changes:
 

diff --git a/...s/src/test/java/io/gravitee/am/repository/oauth2/api/AuthorizationCodeRepositoryTest.java b/...s/src/test/java/io/gravitee/am/repository/oauth2/api/AuthorizationCodeRepositoryTest.java
@@ -44,7 +44,8 @@ public void shouldStoreCode() {
 
         testObserver.assertComplete();
         testObserver.assertNoErrors();
-        testObserver.assertValue(authorizationCode1 -> authorizationCode1.getCode().equals(code) && authorizationCode1.getContextVersion() == 1);
+        testObserver.assertValue(authorizationCode1 -> authorizationCode1.getCode().equals(code)
+                && authorizationCode1.getContextVersion() == 1);
     }
 
     @Test