[java-oidc-common] 12/35: Improve OIDC message encoders to support validation hooks

Phil Smart philip.smart at jisc.ac.uk
Tue Sep 20 14:19:12 UTC 2022


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

philsmart pushed a commit to branch dev/JCOMOIDC-41
in repository java-oidc-common.

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

commit a7ca91715b7acec76756797c4b02289fb542c9ff
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Wed Jul 6 10:50:58 2022 +0100

    Improve OIDC message encoders to support validation hooks
---
 .../encoder/impl/AbstractOIDCMessageEncoder.java   | 128 ++++++++++++++++++++-
 1 file changed, 122 insertions(+), 6 deletions(-)

diff --git a/oidc-common-profile-impl/src/main/java/net/shibboleth/oidc/profile/encoder/impl/AbstractOIDCMessageEncoder.java b/oidc-common-profile-impl/src/main/java/net/shibboleth/oidc/profile/encoder/impl/AbstractOIDCMessageEncoder.java
index e790640..db4813e 100644
--- a/oidc-common-profile-impl/src/main/java/net/shibboleth/oidc/profile/encoder/impl/AbstractOIDCMessageEncoder.java
+++ b/oidc-common-profile-impl/src/main/java/net/shibboleth/oidc/profile/encoder/impl/AbstractOIDCMessageEncoder.java
@@ -19,26 +19,60 @@ package net.shibboleth.oidc.profile.encoder.impl;
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Optional;
+import java.util.function.Predicate;
 import java.util.stream.Collectors;
 
 import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 
+import org.opensaml.messaging.encoder.MessageEncodingException;
 import org.opensaml.messaging.encoder.servlet.AbstractHttpServletResponseMessageEncoder;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
+import com.google.common.base.Predicates;
 import com.nimbusds.openid.connect.sdk.claims.ACR;
 
 import net.shibboleth.oidc.profile.core.OIDCAuthenticationRequest;
 import net.shibboleth.oidc.profile.encoder.OIDCMessageEncoder;
 import net.shibboleth.utilities.java.support.collection.Pair;
+import net.shibboleth.utilities.java.support.component.ComponentSupport;
 import net.shibboleth.utilities.java.support.net.URLBuilder;
 
 /**
  * Base class for OIDC message encoders.
- */
-//TODO: add a method or strategy for testing the request covers all the required parameters for a given response type 
+ */ 
 public abstract class AbstractOIDCMessageEncoder extends AbstractHttpServletResponseMessageEncoder 
             implements OIDCMessageEncoder {
     
+    /** Class logger. */
+    @Nonnull private final Logger log = LoggerFactory.getLogger(AbstractOIDCMessageEncoder.class);
+    
+    /** A hook to allow additional checking of the authorization parameters after it is build.*/
+    @Nonnull private Predicate<List<Pair<String, String>>> authorizationParamsAreValidPredicate;
+    
+    protected AbstractOIDCMessageEncoder() {
+        authorizationParamsAreValidPredicate = Predicates.alwaysTrue();
+    }
+    
+    /**
+     * Set a hook that allows the built authorization parameters to be validated before they are used.
+     * This is run in addition too, but before, the built in validation taken from the specification.
+     * If this returns false, the built in validation is not run, and validation fails.
+     * 
+     * @param predicate the hook to run
+     */
+    public void setAuthorizationParamsAreValidPredicate(
+            @Nullable final Predicate<List<Pair<String, String>>> predicate) {
+        ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+        ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
+
+        if (predicate != null) {
+            authorizationParamsAreValidPredicate = predicate;
+        }
+    }
+    
     
     /**
      * Serialize OAuth 2.0 authorization parameters from the authentication request to the query string 
@@ -46,9 +80,11 @@ public abstract class AbstractOIDCMessageEncoder extends AbstractHttpServletResp
      * 
      * @param request the authentication request.
      * @param builder the URL builder to add the query parameters to.
+     * 
+     * @throws MessageEncodingException on error building the parameters
      */
     protected void serializeAuthorizationParamsToUrl(@Nonnull final OIDCAuthenticationRequest request,
-            @Nonnull final URLBuilder builder) {
+            @Nonnull final URLBuilder builder) throws MessageEncodingException {
         
         final List<Pair<String, String>> params = createParametersFromRequest(request);
         params.forEach(param -> builder.getQueryParams().add(param));
@@ -60,8 +96,11 @@ public abstract class AbstractOIDCMessageEncoder extends AbstractHttpServletResp
      * @param request the authentication request query string.
      * 
      * @return the query string.
+     * 
+     * @throws MessageEncodingException on error building the parameters
      */
-    protected String serializeAuthorizationParamsToQueryString(@Nonnull final OIDCAuthenticationRequest request) {
+    protected String serializeAuthorizationParamsToQueryString(@Nonnull final OIDCAuthenticationRequest request)
+            throws MessageEncodingException {
         //TODO maybe a better way to do this than the full URL builder?
         final URLBuilder builder = new URLBuilder();
         final List<Pair<String, String>> params = createParametersFromRequest(request);
@@ -75,8 +114,11 @@ public abstract class AbstractOIDCMessageEncoder extends AbstractHttpServletResp
      * @param req the authentication request
      * 
      * @return a list of authorization parameters.
+     * 
+     * @throws MessageEncodingException on error building the parameters
      */
-    protected List<Pair<String, String>> createParametersFromRequest(@Nonnull final OIDCAuthenticationRequest req) {
+    protected List<Pair<String, String>> createParametersFromRequest(@Nonnull final OIDCAuthenticationRequest req) 
+                                                                                    throws MessageEncodingException {
         
         final List<Pair<String, String>> params = new ArrayList<>();
         
@@ -115,8 +157,82 @@ public abstract class AbstractOIDCMessageEncoder extends AbstractHttpServletResp
             params.add(new Pair<>("acr_values", acrString));          
         }
         //TODO: requestURI, includedGrantedScopes?, resource_uris?
+        
+        if (!validateParams(params)) {            
+            throw new MessageEncodingException("Authorization parameters are not valid");
+        }
+        
         return params;
-
+    }
+    
+    /**
+     * Ensure the authorization parameters are valid.
+     * 
+     * @param params the parameters
+     * 
+     * @return true if the authorization parameters are valid, false otherwise
+     */
+    protected boolean validateParams(final List<Pair<String, String>> params) {
+        
+        if (!authorizationParamsAreValidPredicate.test(params)) {
+            return false;
+        }        
+        if (!pairFirstEquals("response_type", params)){
+            log.error("Authorization request parameters are invalid, no response_type");
+            return false;
+        }
+        if (!pairFirstEquals("client_id", params)){
+            log.error("Authorization request parameters are invalid, no client_id");
+            return false;
+        }
+        if (!pairFirstEquals("scope", params)){
+            log.error("Authorization request parameters are invalid, no scope");
+            return false;
+        }
+        if (!pairSecondContains("scope", "openid", params)) {
+            log.error("Authorization request parameters are invalid, scope does not contain 'openid'");
+            return false;
+        }
+        
+        return true;
+    }
+    
+    /** 
+     * Check if the value is contained (exactly) in the parameter map as the first item of any pair.
+     * 
+     * @param value the value to check exists
+     * 
+     * @param params the parameters to check
+     * 
+     * @return true iff the value exists as the first item of any pair, false otherwise
+     */
+    private boolean pairFirstEquals(@Nonnull final String value, final List<Pair<String, String>> params) {
+        return params.stream().map(Pair::getFirst).anyMatch(key -> key.equals(value));
+    }
+    
+    /** 
+     * Check if the value is contained (string containment) in the parameter map as the second item of a pair. 
+     * The pair is chosen using the first parameter as a key. If they key does not exist in the list, false 
+     * is returned.
+     * 
+     * @param key the key used to find the pair that should contain the value
+     * @param value the value to check exists
+     * 
+     * @param params the parameters to check
+     * 
+     * @return true iff the value is contained in the second item of the pair referenced by the key, false otherwise
+     */
+    private boolean pairSecondContains(@Nonnull final String key,
+            @Nonnull final String value, final List<Pair<String, String>> params) {
+        
+        final Optional<Pair<String, String>> pairByKey = 
+                params.stream().filter(p -> p.getFirst().equals(key)).findFirst();
+        
+        if (pairByKey.isEmpty() || pairByKey.get().getSecond() == null) {
+            return false;
+        }
+        
+        return pairByKey.get().getSecond().contains(value);
     }
 
 }

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


More information about the commits mailing list