[java-idp-plugin-oidc-rp] branch main updated: Cleanup properties. Fix redirect_uri creation

Phil Smart philip.smart at jisc.ac.uk
Tue Oct 11 15:29:23 UTC 2022


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

philsmart pushed a commit to branch main
in repository java-idp-plugin-oidc-rp.

View the commit online:
http://git.shibboleth.net/view/?p=java-idp-plugin-oidc-rp.git;a=commit;h=683e0333ae73d73977ada512bc6df8556f8a7a37

The following commit(s) were added to refs/heads/main by this push:
     new 683e033  Cleanup properties. Fix redirect_uri creation
683e033 is described below

commit 683e0333ae73d73977ada512bc6df8556f8a7a37
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Tue Oct 11 16:29:18 2022 +0100

    Cleanup properties. Fix redirect_uri creation
---
 .../oidc/rp/config/CredentialsListFactory.java     |  2 +-
 .../rp/messaging/impl/AddRedirectURIHandler.java   | 42 ++++++++++++------
 .../oidc-relying-party-authn-beans.xml             |  2 +-
 .../authn/oidc/rp/conf/authn/oidc-rp.properties    | 50 ++++++++--------------
 .../messaging/impl/AddRedirectURIHandlerTest.java  | 30 +++++++++++++
 5 files changed, 78 insertions(+), 48 deletions(-)

diff --git a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/config/CredentialsListFactory.java b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/config/CredentialsListFactory.java
index ec7d214..04a40b6 100644
--- a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/config/CredentialsListFactory.java
+++ b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/config/CredentialsListFactory.java
@@ -32,7 +32,7 @@ import org.springframework.beans.factory.config.AbstractFactoryBean;
 /** 
  * A factory that returns a list of credentials which does not contain any {@literal null} elements.
  * 
- * <p>Primarily created to support signature credential injection into the {@link BasicSignatureSigningConfiguration}
+ * <p>Primarily created to support signature credential injection into the {@link BasicSignatureSigningConfiguration} sdrfgrfgergerg
  * when the credential list may contain, in its raw state, {@literal null} elements.</p>
  */
 public class CredentialsListFactory extends AbstractFactoryBean<List<Credential>> {
diff --git a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/messaging/impl/AddRedirectURIHandler.java b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/messaging/impl/AddRedirectURIHandler.java
index 188bfb5..1e3a4c5 100644
--- a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/messaging/impl/AddRedirectURIHandler.java
+++ b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/messaging/impl/AddRedirectURIHandler.java
@@ -18,29 +18,29 @@
 package net.shibboleth.idp.plugin.authn.oidc.rp.messaging.impl;
 
 import java.net.URI;
+import java.net.URISyntaxException;
 import java.util.function.BiFunction;
-import java.util.function.Function;
 
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import javax.servlet.http.HttpServletRequest;
 
 import org.opensaml.messaging.context.MessageContext;
-import org.opensaml.messaging.context.navigate.RecursiveTypedParentContextLookup;
 import org.opensaml.messaging.handler.MessageHandlerException;
-import org.opensaml.profile.action.ActionSupport;
 import org.opensaml.profile.context.ProfileRequestContext;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import net.shibboleth.oidc.profile.core.OidcEventIds;
 import net.shibboleth.utilities.java.support.annotation.constraint.NonnullAfterInit;
 import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
 import net.shibboleth.utilities.java.support.component.ComponentSupport;
 import net.shibboleth.utilities.java.support.logic.Constraint;
+import net.shibboleth.utilities.java.support.primitive.StringSupport;
 
 /** 
- * A message handler that adds a redirect_uri to the authentication request.
+ * A message handler that adds a redirect_uri to the authentication request. The redirect_uri is either
+ * taken directly from the override set on the RP profile configuration, or if none exist, is computed using 
+ * the creation strategy. 
  */
 public class AddRedirectURIHandler extends AbstractOIDCAuthenticationRequestActionMessageHandler {
 
@@ -77,14 +77,30 @@ public class AddRedirectURIHandler extends AbstractOIDCAuthenticationRequestActi
 
     @Override protected void doInvoke(@Nonnull final MessageContext messageContext) 
             throws MessageHandlerException {   
-       
-        final URI redirectUri = 
-                redirectUriCreationStrategy.apply(getHttpServletRequest(), lookupProfileRequestContext(messageContext));
-        if (redirectUri == null) {
-            throw new MessageHandlerException("Redirect URI could not be located or created using the strategy");
-        }
-        log.trace("{} Created redirect_uri '{}'", getLogPrefix(), redirectUri);
-        getAuthenticationRequest().setRedirectURI(redirectUri);
+        
+        final String redirectOverride = 
+                getProfileConfiguration().getRedirectUriOverride(lookupProfileRequestContext(messageContext));        
+        
+        
+        if (StringSupport.trimOrNull(redirectOverride) != null) {
+            log.debug("{} Redirect_uri override in profile configuration is: '{}'", getLogPrefix(), 
+                    redirectOverride);
+            try {
+                getAuthenticationRequest().setRedirectURI(new URI(redirectOverride));
+            } catch (final URISyntaxException e) {
+                throw new MessageHandlerException("Redirect URI override was not a valid URI", e);
+            }
+        } else {       
+            final URI redirectUri = 
+                    redirectUriCreationStrategy.apply(
+                            getHttpServletRequest(), lookupProfileRequestContext(messageContext));            
+            if (redirectUri == null) {
+                throw new MessageHandlerException("Redirect URI could not be located or created using the strategy");
+            }
+            getAuthenticationRequest().setRedirectURI(redirectUri);
+        }        
+        
+        log.trace("{} Created redirect_uri '{}'", getLogPrefix(), getAuthenticationRequest().getRedirectURI());
   
     }
 
diff --git a/idp-oidc-rp-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/OIDCRelyingParty/oidc-relying-party-authn-beans.xml b/idp-oidc-rp-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/OIDCRelyingParty/oidc-relying-party-authn-beans.xml
index 71a860d..e21343a 100644
--- a/idp-oidc-rp-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/OIDCRelyingParty/oidc-relying-party-authn-beans.xml
+++ b/idp-oidc-rp-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/OIDCRelyingParty/oidc-relying-party-authn-beans.xml
@@ -18,7 +18,7 @@
 
     <!-- Initial discovery step -->
     <bean id="PropertyDrivenDiscovery" parent="shibboleth.Functions.Constant"
-        c:target="#{'%{idp.authn.oidc.rp.proxyIssuer:}'.trim()}" />
+        c:target="#{'%{idp.authn.oidc.rp.provider.proxyIssuer:}'.trim()}" />
 
 
     <!-- Parent beans for indirecting into nested PRC. -->
diff --git a/idp-oidc-rp-impl/src/main/resources/net/shibboleth/idp/plugin/authn/oidc/rp/conf/authn/oidc-rp.properties b/idp-oidc-rp-impl/src/main/resources/net/shibboleth/idp/plugin/authn/oidc/rp/conf/authn/oidc-rp.properties
index 33b61f7..f2bd3e9 100644
--- a/idp-oidc-rp-impl/src/main/resources/net/shibboleth/idp/plugin/authn/oidc/rp/conf/authn/oidc-rp.properties
+++ b/idp-oidc-rp-impl/src/main/resources/net/shibboleth/idp/plugin/authn/oidc/rp/conf/authn/oidc-rp.properties
@@ -1,47 +1,31 @@
-##does not need the .well-known/openid-configuration path.
-#idp.oidc.rp.providerConfigurationDocument=https://www.certification.openid.net/test/a/test_rp_proxy/
-#idp.oidc.rp.redirectURI=https://localhost:8443/idp/profile/Authn/OIDC/RP/callback
-## openid is defaulted. Other scopes could be; profile etc.
-#idp.oidc.rp.scope=email
-#idp.authn.OIDC.RP.discoveryRequired=true
-## normally in authn.properties
-#idp.authn.discoveryURL=http://phil.com
-
-## find metadata from the well-known location
-#idp.authn.oidc.rp.proxyIssuer=https://www.certification.openid.net/test/a/test_rp_proxy/
-idp.authn.oidc.rp.proxyIssuer=https://accounts.google.com
-#idp.authn.oidc.rp.proxyIssuer=https://testop.funet.fi
-idp.authn.oidc.rp.client.redirecturl.allowedOrigins = https://localhost:8443
+idp.authn.oidc.rp.provider.proxyIssuer=https://issuer.com
 ## If discovery is required, these default properties will not be set, you must manage per OP using overrides
-idp.authn.oidc.rp.client.clientId = mytestclient
-#idp.authn.oidc.rp.client.clientSecret = !A%D*F-JaNdRgUkXp2s5v8y/B?E(H+Kb
-#idp.authn.oidc.rp.client.clientSecret=UjWnZr4u7x!A%D*G-KaPdSgVkYp2s5v8
-idp.authn.oidc.rp.client.clientSecret=
+idp.authn.oidc.rp.client.clientId = client_id
+idp.authn.oidc.rp.client.clientSecret= client_secret
 #idp.authn.oidc.rp.client.clientSecretExpiresAt = PT0S
-
-## Use small fetch interval so we can re-run tests against the certification OP
-idp.authn.oidc.rp.provider.keyfetch.interval = PT5S
+idp.authn.oidc.rp.client.redirecturl.allowedOrigins = https://localhost:8443
 
 ## Uncomment this if you want to enable RSA decryption (where the public key is registered with the OP)
-idp.authn.oidc.rp.client.enc.key=%{idp.home}/credentials/idp-encryption-rsa.jwk
-idp.authn.oidc.rp.client.sig.key=%{idp.home}/credentials/idp-signing-rsa.jwk
-## Example key wrap key (oct)
-#idp.authn.oidc.rp.client.requestobject.enc.symmetric.kw.key=%{idp.home}/credentials/oidc-rp-requestobject-encryption.jwk
-#idp.authn.oidc.rp.client.requestobject.enc.symmetric.cek.key=%{idp.home}/credentials/...
+#idp.authn.oidc.rp.client.enc.key=%{idp.home}/credentials/idp-encryption-rsa.jwk
+#idp.authn.oidc.rp.client.sig.key=%{idp.home}/credentials/idp-signing-rsa.jwk
 
-idp.authn.oidc.rp.discoveryRequired=false
+#idp.authn.oidc.rp.provider.discoveryRequired=false
+#idp.authn.discoveryURL=http://opdisco.com
 
 idp.authn.oidc.rp.client.requestobject.supported= false
 idp.authn.oidc.rp.client.requestobject.encrypted = false
 idp.authn.oidc.rp.client.requestobject.signed = true
 
-idp.authn.oidc.rp.useUserInfoEndpoint = true
+idp.authn.oidc.rp.client.userinfo.enabled = true
 
-## Override the default response_mode for the given response_type
-idp.authn.oidc.rp.responseMode = form_post
+## Use small fetch interval so we can re-run tests against the certification OP
+idp.authn.oidc.rp.provider.keyfetch.interval = PT5S
 
-## Comma seperated list of additional scopes e.g. PROFILE or EMAIL. The openid scope is added by default
-idp.authn.oidc.rp.scopes = profile,email
+## Override the default response_mode for the given response_type
+#idp.authn.oidc.rp.client.responseMode = query
+#idp.authn.oidc.rp.client.authenticationMethod = client_secret_basic
+## Comma seperated list of additional scopes e.g. profile or email. The openid scope is added by default
+#idp.authn.oidc.rp.client.scopes =
 
 idp.authn.oidc.rp.supportedPrincipals = saml2/http://example.org/ac/classes/mfa
 
@@ -51,5 +35,5 @@ idp.authn.oidc.rp.c14n.subjectidentifier.uppercase = false
 idp.authn.oidc.rp.c14n.subjectidentifier.trim = true
 idp.authn.oidc.rp.c14n.subjectidentifier.disabled = false
 
-#idp.authn.oidc.rp.clientAuthenticationMethod = client_secret_basic
+
 
diff --git a/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/messaging/impl/AddRedirectURIHandlerTest.java b/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/messaging/impl/AddRedirectURIHandlerTest.java
index 7e5db27..4e22420 100644
--- a/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/messaging/impl/AddRedirectURIHandlerTest.java
+++ b/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/messaging/impl/AddRedirectURIHandlerTest.java
@@ -77,6 +77,36 @@ public class AddRedirectURIHandlerTest extends AbstractOIDCTest {
         handler.initialize();
         handler.invoke(prc.getOutboundMessageContext());
     }
+    
+    @Test
+    public void testRedirectURIOverride() throws Exception {        
+        
+        final String redirectUriOverride = "https://rp.example.com/callback";
+        oidcAuthzConfig.setRedirectUriOverride(redirectUriOverride);
+        
+        //Should take the override and not this
+        final URI redirectUriFromFunction = new URI("https://rp.example.com/not-this-callback");
+        handler.setRedirectUriCreationStrategy((http, prc) -> redirectUriFromFunction);
+        
+        handler.initialize();
+        handler.invoke(prc.getOutboundMessageContext());
+        assertEquals(authnRequest.getRedirectURI().toString(),redirectUriOverride);
+    }
+    
+    @Test(expectedExceptions = MessageHandlerException.class)
+    public void testRedirectURIOverrideInvalidURI() throws Exception {        
+        
+        final String redirectUriOverride = "http://rp.example.com/fail?id={1}";
+        oidcAuthzConfig.setRedirectUriOverride(redirectUriOverride);
+        
+        //Should fail on the override and not this
+        final URI redirectUriFromFunction = new URI("https://rp.example.com/not-this-callback");
+        handler.setRedirectUriCreationStrategy((http, prc) -> redirectUriFromFunction);
+        
+        handler.initialize();
+        handler.invoke(prc.getOutboundMessageContext());
+        
+    }
 
 
 }

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


More information about the commits mailing list