[java-idp-oidc] 01/02: JOIDC-210 - Refactor support for non-URI resource indicators for Nimbus v11

Henri Mikkonen henri.mikkonen at iki.fi
Wed May 22 14:24:27 UTC 2024


This is an automated email from the git hooks/post-receive script.

hjmikkon pushed a commit to branch main
in repository java-idp-oidc.

View the commit online:
http://git.shibboleth.net/view/?p=java-idp-oidc.git;a=commit;h=669edd4b54d5384c7a33c4e60646466e24b2b1ee

commit 669edd4b54d5384c7a33c4e60646466e24b2b1ee
Author: Henri Mikkonen <henri.mikkonen at iki.fi>
AuthorDate: Wed May 22 17:15:16 2024 +0300

    JOIDC-210 - Refactor support for non-URI resource indicators for Nimbus v11
    
    https://shibboleth.atlassian.net/browse/JOIDC-210
    
    Override the query string (GET-parameters) via HttpServletRequestWrapper.
    - It's used for authorization/authentication request parsing.
    The already existing switchIntoCustomResource(..) transforms the resource parameters in body (POST-parameters).
---
 .../impl/OIDCAuthenticationRequestDecoder.java     | 18 +++++---
 .../decoding/impl/BaseOAuth2RequestDecoder.java    | 52 +++++++++++-----------
 .../CustomResourceHttpServletRequestWrapper.java   | 43 ++++++++++++++++++
 .../impl/OAuth2AuthorizationRequestDecoder.java    | 17 ++++---
 .../impl/OIDCAuthenticationRequestDecoderTest.java | 36 +++++++++++++++
 .../OAuth2AuthorizationRequestDecoderTest.java     | 36 +++++++++++++++
 6 files changed, 166 insertions(+), 36 deletions(-)

diff --git a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/decoding/impl/OIDCAuthenticationRequestDecoder.java b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/decoding/impl/OIDCAuthenticationRequestDecoder.java
index 2b17f6a5..ba2567dc 100644
--- a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/decoding/impl/OIDCAuthenticationRequestDecoder.java
+++ b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/decoding/impl/OIDCAuthenticationRequestDecoder.java
@@ -27,7 +27,10 @@ import com.nimbusds.oauth2.sdk.http.HTTPRequest;
 import com.nimbusds.oauth2.sdk.http.JakartaServletUtils;
 import com.nimbusds.openid.connect.sdk.AuthenticationRequest;
 
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletRequestWrapper;
 import net.shibboleth.idp.plugin.oidc.op.oauth2.decoding.impl.BaseOAuth2RequestDecoder;
+import net.shibboleth.idp.plugin.oidc.op.oauth2.decoding.impl.CustomResourceHttpServletRequestWrapper;
 import net.shibboleth.shared.primitive.LoggerFactory;
 
 /**
@@ -43,11 +46,16 @@ public class OIDCAuthenticationRequestDecoder extends BaseOAuth2RequestDecoder<A
     @Override
     protected AuthenticationRequest parseMessage() throws MessageDecodingException {
         try {
-            final HTTPRequest httpReq = JakartaServletUtils.createHTTPRequest(getHttpServletRequest());
-            getProtocolMessageLogger().trace("Inbound request {}", RequestUtil.toString(httpReq));
-            if (httpReq != null) {
-                switchIntoCustomResource(httpReq);
-                return AuthenticationRequest.parse(httpReq);
+            final HttpServletRequest httpServletRequest = getHttpServletRequest();
+            if (httpServletRequest != null) {
+                final HttpServletRequestWrapper httpServletRequestWrapper =
+                        new CustomResourceHttpServletRequestWrapper(httpServletRequest);
+                final HTTPRequest httpReq = JakartaServletUtils.createHTTPRequest(httpServletRequestWrapper);
+                getProtocolMessageLogger().trace("Inbound request {}", RequestUtil.toString(httpReq));
+                if (httpReq != null) {
+                    switchIntoCustomResource(httpReq);
+                    return AuthenticationRequest.parse(httpReq);
+                }
             }
             throw new MessageDecodingException("Could not create HTTPRequest object from the incoming request");
         } catch (final com.nimbusds.oauth2.sdk.ParseException | IOException e) {
diff --git a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/decoding/impl/BaseOAuth2RequestDecoder.java b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/decoding/impl/BaseOAuth2RequestDecoder.java
index f3ec4fbb..3d81d2f2 100644
--- a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/decoding/impl/BaseOAuth2RequestDecoder.java
+++ b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/decoding/impl/BaseOAuth2RequestDecoder.java
@@ -14,11 +14,7 @@
 
 package net.shibboleth.idp.plugin.oidc.op.oauth2.decoding.impl;
 
-import java.io.UnsupportedEncodingException;
 import java.net.URI;
-import java.net.URLEncoder;
-import java.util.List;
-import java.util.Map;
 
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
@@ -43,8 +39,7 @@ import net.shibboleth.shared.primitive.LoggerFactory;
 public abstract class BaseOAuth2RequestDecoder<T extends Request> extends AbstractHttpServletRequestMessageDecoder {
 
     /** Class logger. */
-    @Nonnull
-    private final Logger log = LoggerFactory.getLogger(BaseOAuth2RequestDecoder.class);
+    @Nonnull private final static Logger log = LoggerFactory.getLogger(BaseOAuth2RequestDecoder.class);
 
     /** A flag to remove the IP address from the endpoint URI. */
     private boolean removeIpAddressFromEndpointUri;
@@ -140,32 +135,37 @@ public abstract class BaseOAuth2RequestDecoder<T extends Request> extends Abstra
     }
 
     /**
-     * Switches the 'resource' parameter names in the given HTTP request into a custom parameter name. This allows us
-     * to avoid Nimbus library's control over the values of the parameter.
+     * Switches the 'resource' parameter names in the given HTTP request body into a custom parameter name. This allows
+     * us to avoid Nimbus library's control over the values of the parameter.
      * 
      * @param httpRequest The HTTP request object where the parameter names are switched
      */
     protected void switchIntoCustomResource(@Nonnull final HTTPRequest httpRequest) {
-        final Map<String,List<String>> params = httpRequest.getQueryParameters();
+        final String body = httpRequest.getBody();
+        if (body != null) {
+            httpRequest.setBody(transformResourceParameter(body));
+        }
+    }
+
+    /**
+     * Transforms the 'resource' parameter names in the given input with a custom one defined by {@link
+     * DefaultRequestAudienceLookupFunction#CUSTOM_RESOURCE_PARAM_NAME}.
+     * 
+     * @param string the input
+     * @return the input with resource parameter names transformed into custom ones.
+     */
+    @Nullable public static String transformResourceParameter(@Nullable final String string) {
+        String input = string;
+        if (input == null || !input.contains("resource=")) {
+            return input;
+        }
         final String customParameterName = DefaultRequestAudienceLookupFunction.CUSTOM_RESOURCE_PARAM_NAME;
-        if (params != null && !params.isEmpty() && params.get("resource") != null) {
-            final List<String> resources = params.get("resource");
-            String query = httpRequest.getQuery();
-            for (final String resource : resources) {
-                String encodedResource;
-                try {
-                    encodedResource = URLEncoder.encode(resource, "UTF-8");
-                } catch (UnsupportedEncodingException e) {
-                    return;
-                }
-                query = query.replace("resource=" + encodedResource, customParameterName + "=" + encodedResource);
-                log.debug("Replaced resource={} with {}={} : {}", encodedResource, customParameterName, encodedResource,
-                        query);
-            }
-            httpRequest.setQuery(query);
-        } else {
-            log.trace("No resource parameter(s) found");
+        if (input.startsWith("resource=")) {
+            input = customParameterName + "=" + input.substring("resource=".length());
         }
+        input = input.replace("&resource=", "&" + customParameterName + "=");
+        log.debug("Returning modified string {}", input);
+        return input;
     }
 
 }
diff --git a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/decoding/impl/CustomResourceHttpServletRequestWrapper.java b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/decoding/impl/CustomResourceHttpServletRequestWrapper.java
new file mode 100644
index 00000000..65baa75e
--- /dev/null
+++ b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/decoding/impl/CustomResourceHttpServletRequestWrapper.java
@@ -0,0 +1,43 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package net.shibboleth.idp.plugin.oidc.op.oauth2.decoding.impl;
+
+import javax.annotation.Nonnull;
+
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletRequestWrapper;
+
+/**
+ * A custom extension to {@link HttpServletRequestWrapper} overriding the resource parameter with a custom one in the
+ * query string.
+ */
+public class CustomResourceHttpServletRequestWrapper extends HttpServletRequestWrapper {
+
+    /**
+     * Constructor.
+     *
+     * @param httpServletRequest The wrapped HTTP servlet request
+     */
+    public CustomResourceHttpServletRequestWrapper(@Nonnull final HttpServletRequest httpServletRequest) {
+        super(httpServletRequest);
+    }
+
+    /** {@inheritDoc} */
+    @Override
+    public String getQueryString() {
+        return BaseOAuth2RequestDecoder.transformResourceParameter(super.getQueryString());
+    }
+
+}
diff --git a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/decoding/impl/OAuth2AuthorizationRequestDecoder.java b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/decoding/impl/OAuth2AuthorizationRequestDecoder.java
index d85ccd1c..1e852f55 100644
--- a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/decoding/impl/OAuth2AuthorizationRequestDecoder.java
+++ b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/decoding/impl/OAuth2AuthorizationRequestDecoder.java
@@ -27,6 +27,8 @@ import com.nimbusds.oauth2.sdk.AuthorizationRequest;
 import com.nimbusds.oauth2.sdk.http.HTTPRequest;
 import com.nimbusds.oauth2.sdk.http.JakartaServletUtils;
 
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletRequestWrapper;
 import net.shibboleth.idp.plugin.oidc.op.decoding.impl.RequestUtil;
 import net.shibboleth.shared.primitive.LoggerFactory;
 
@@ -43,11 +45,16 @@ public class OAuth2AuthorizationRequestDecoder extends BaseOAuth2RequestDecoder<
     @Override
     protected AuthorizationRequest parseMessage() throws MessageDecodingException {
         try {
-            final HTTPRequest httpReq = JakartaServletUtils.createHTTPRequest(getHttpServletRequest());
-            getProtocolMessageLogger().trace("Inbound request {}", RequestUtil.toString(httpReq));
-            if (httpReq != null) {
-                switchIntoCustomResource(httpReq);
-                return AuthorizationRequest.parse(httpReq);
+            final HttpServletRequest httpServletRequest = getHttpServletRequest();
+            if (httpServletRequest != null) {
+                final HttpServletRequestWrapper httpServletRequestWrapper =
+                        new CustomResourceHttpServletRequestWrapper(httpServletRequest);
+                final HTTPRequest httpReq = JakartaServletUtils.createHTTPRequest(httpServletRequestWrapper);
+                getProtocolMessageLogger().trace("Inbound request {}", RequestUtil.toString(httpReq));
+                if (httpReq != null) {
+                    switchIntoCustomResource(httpReq);
+                    return AuthorizationRequest.parse(httpReq);
+                }
             }
             throw new MessageDecodingException("Could not create HTTPRequest object from the incoming request");
         } catch (final com.nimbusds.oauth2.sdk.ParseException | IOException e) {
diff --git a/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/decoding/impl/OIDCAuthenticationRequestDecoderTest.java b/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/decoding/impl/OIDCAuthenticationRequestDecoderTest.java
index 179f4edc..30de0973 100644
--- a/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/decoding/impl/OIDCAuthenticationRequestDecoderTest.java
+++ b/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/decoding/impl/OIDCAuthenticationRequestDecoderTest.java
@@ -82,6 +82,24 @@ public class OIDCAuthenticationRequestDecoderTest {
                 .contains("https://resource.example.org/"));
     }
 
+    @Test
+    public void testRequestDecodingWithURIResourceFirst() throws MessageDecodingException, URISyntaxException {
+        httpRequest
+                .setQueryString("resource=https%3A%2F%2Fresource.example.org%2F&response_type=code&client_id=s6BhdRkqt3&redirect_uri=https%3A%2F%2Fclient.example.org%2Fcb&scope=openid%20profile&state=af0ifjsldkj&nonce=n-0S6_WzA2Mj");
+        decoder.decode();
+        final MessageContext messageContext = decoder.getMessageContext();
+        // We are not testing nimbus itself here, i.e. we are happy to decode
+        // one parameter successfully
+        assert messageContext != null;
+        final AuthenticationRequest request = (AuthenticationRequest) messageContext.getMessage();
+        assert request != null;
+        final ResponseType responseType = request.getResponseType();
+        assert responseType != null;
+        Assert.assertEquals(responseType.toString(), ResponseType.Value.CODE.toString());
+        Assert.assertTrue(request.getCustomParameter(DefaultRequestAudienceLookupFunction.CUSTOM_RESOURCE_PARAM_NAME)
+                .contains("https://resource.example.org/"));
+    }
+
     @Test
     public void testRequestDecodingWithNonURIResource() throws MessageDecodingException {
         httpRequest
@@ -100,6 +118,24 @@ public class OIDCAuthenticationRequestDecoderTest {
                 .contains("mockResourceId"));
     }
 
+    @Test
+    public void testRequestDecodingWithNonURIResourceFirst() throws MessageDecodingException {
+        httpRequest
+                .setQueryString("resource=mockResourceId&response_type=code&client_id=s6BhdRkqt3&redirect_uri=https%3A%2F%2Fclient.example.org%2Fcb&scope=openid%20profile&state=af0ifjsldkj&nonce=n-0S6_WzA2Mj");
+        decoder.decode();
+        final MessageContext messageContext = decoder.getMessageContext();
+        // We are not testing nimbus itself here, i.e. we are happy to decode
+        // one parameter successfully
+        assert messageContext != null;
+        final AuthenticationRequest request = (AuthenticationRequest) messageContext.getMessage();
+        assert request != null;
+        final ResponseType responseType = request.getResponseType();
+        assert responseType != null;
+        Assert.assertEquals(responseType.toString(), ResponseType.Value.CODE.toString());
+        Assert.assertTrue(request.getCustomParameter(DefaultRequestAudienceLookupFunction.CUSTOM_RESOURCE_PARAM_NAME)
+                .contains("mockResourceId"));
+    }
+
     @Test(expectedExceptions = MessageDecodingException.class)
     public void testInvalidRequestDecoding() throws MessageDecodingException {
         // Mandatory response_type parameter removed, decoding should fail
diff --git a/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/oauth2/decoding/impl/OAuth2AuthorizationRequestDecoderTest.java b/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/oauth2/decoding/impl/OAuth2AuthorizationRequestDecoderTest.java
index 887f9b2c..7b4c9394 100644
--- a/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/oauth2/decoding/impl/OAuth2AuthorizationRequestDecoderTest.java
+++ b/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/oauth2/decoding/impl/OAuth2AuthorizationRequestDecoderTest.java
@@ -84,6 +84,24 @@ public class OAuth2AuthorizationRequestDecoderTest {
         }
     }
 
+    @Test
+    public void testRequestDecodingWithURIResourceFirst() throws MessageDecodingException, URISyntaxException {
+        httpRequest
+                .setQueryString("resource=https%3A%2F%2Fresource.example.org%2F&response_type=code&client_id=s6BhdRkqt3&redirect_uri=https%3A%2F%2Fclient.example.org%2Fcb&scope=profile&state=af0ifjsldkj&nonce=n-0S6_WzA2Mj");
+        decoder.decode();
+        final MessageContext messageContext = decoder.getMessageContext();
+        // We are not testing nimbus itself here, i.e. we are happy to decode
+        // one parameter successfully
+        assert messageContext != null;
+        if (messageContext.getMessage() instanceof AuthorizationRequest authzRequest) {
+            Assert.assertEquals(authzRequest.getResponseType().toString(), ResponseType.Value.CODE.toString());
+            Assert.assertTrue(authzRequest.getCustomParameter(DefaultRequestAudienceLookupFunction.CUSTOM_RESOURCE_PARAM_NAME)
+                .contains("https://resource.example.org/"));
+        } else {
+            Assert.fail();
+        }
+    }
+
     @Test
     public void testRequestDecodingWithNonURIResource() throws MessageDecodingException {
         httpRequest
@@ -102,6 +120,24 @@ public class OAuth2AuthorizationRequestDecoderTest {
         }
     }
 
+    @Test
+    public void testRequestDecodingWithNonURIResourceFirst() throws MessageDecodingException {
+        httpRequest
+                .setQueryString("resource=mockResourceId&response_type=code&client_id=s6BhdRkqt3&redirect_uri=https%3A%2F%2Fclient.example.org%2Fcb&scope=profile&state=af0ifjsldkj&nonce=n-0S6_WzA2Mj");
+        decoder.decode();
+        final MessageContext messageContext = decoder.getMessageContext();
+        // We are not testing nimbus itself here, i.e. we are happy to decode
+        // one parameter successfully
+        assert messageContext != null;
+        if (messageContext.getMessage() instanceof AuthorizationRequest authzRequest) {
+            Assert.assertEquals(authzRequest.getResponseType().toString(), ResponseType.Value.CODE.toString());
+            Assert.assertTrue(authzRequest.getCustomParameter(DefaultRequestAudienceLookupFunction.CUSTOM_RESOURCE_PARAM_NAME)
+                    .contains("mockResourceId"));
+        } else {
+            Assert.fail();
+        }
+    }
+
     @Test(expectedExceptions = MessageDecodingException.class)
     public void testInvalidRequestDecoding() throws MessageDecodingException {
         // Mandatory response_type parameter removed, decoding should fail

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list