[java-idp-oidc] branch main updated: JOIDC-178 - Allow redirection URI validation via custom function
Henri Mikkonen
henri.mikkonen at iki.fi
Tue Jun 11 14:42:37 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=5ca9a1a673fc1fe47aadab2a31a9dd1331f38335
The following commit(s) were added to refs/heads/main by this push:
new 5ca9a1a6 JOIDC-178 - Allow redirection URI validation via custom function
5ca9a1a6 is described below
commit 5ca9a1a673fc1fe47aadab2a31a9dd1331f38335
Author: Henri Mikkonen <henri.mikkonen at iki.fi>
AuthorDate: Tue Jun 11 17:41:09 2024 +0300
JOIDC-178 - Allow redirection URI validation via custom function
https://shibboleth.atlassian.net/browse/JOIDC-178
- Modified ValidateRedirectURI to exploit custom validation lookup strategy
- if bi-predicate is found (via profile configuration), it's used for both registered and unregistered clients
- New 'shibboleth.oidc.MetadataPolicyRedirectUriValidator' abstract bean can be used for wiring metadata policy -based custom validators
- By default shibboleth.oidc.DefaultMetadataPolicyCustomOperators is used for custom operator wiring
- may be overridden via 'shibboleth.oidc.RedirectUriValidator.MetadataPolicyCustomOperators'
---
.../oauth2/profile/impl/ValidateRedirectURI.java | 44 ++++++++-
.../logic/MetadataPolicyRedirectUriValidator.java | 108 +++++++++++++++++++++
.../META-INF/net.shibboleth.idp/postconfig.xml | 11 +++
.../profile/impl/ValidateRedirectURITest.java | 42 ++++++++
.../MetadataPolicyRedirectUriValidatorTest.java | 89 +++++++++++++++++
5 files changed, 290 insertions(+), 4 deletions(-)
diff --git a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/ValidateRedirectURI.java b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/ValidateRedirectURI.java
index 929eff93..1e4a5c34 100644
--- a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/ValidateRedirectURI.java
+++ b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/ValidateRedirectURI.java
@@ -18,6 +18,7 @@ import java.net.URI;
import java.util.Map;
import java.util.Set;
import java.util.function.BiFunction;
+import java.util.function.BiPredicate;
import java.util.function.Function;
import javax.annotation.Nonnull;
@@ -31,6 +32,7 @@ import net.shibboleth.idp.plugin.oidc.op.profile.context.navigate.DefaultRequest
import net.shibboleth.idp.plugin.oidc.op.profile.context.navigate.DefaultValidRedirectUrisLookupFunction;
import net.shibboleth.oidc.metadata.policy.MetadataPolicy;
import net.shibboleth.oidc.metadata.policy.UnregisteredClientPolicy;
+import net.shibboleth.oidc.profile.config.navigate.CustomRedirectUriValidationStrategyLookupFunction;
import net.shibboleth.oidc.profile.config.navigate.UnregisteredClientPolicyLookupFunction;
import net.shibboleth.oidc.profile.core.OidcEventIds;
import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
@@ -68,6 +70,10 @@ public class ValidateRedirectURI extends AbstractOAuthAuthorizationResponseActio
/** Whether to require redirect uri value in the request also when only single value is registered. */
private boolean requireRequestedValue = true;
+ /** Strategy to obtain custom redirect URI validation strategy. */
+ @Nonnull private Function<ProfileRequestContext,BiPredicate<URI, ProfileRequestContext>>
+ customRedirectUriValidationStrategyLookupStrategy;
+
/**
* Constructor.
*/
@@ -76,6 +82,7 @@ public class ValidateRedirectURI extends AbstractOAuthAuthorizationResponseActio
validRedirectURIsLookupStrategy = new DefaultValidRedirectUrisLookupFunction();
registeredRedirectURIsLookupStrategy = new DefaultValidRedirectUrisLookupFunction();
unregisteredClientPolicyLookupStrategy = new UnregisteredClientPolicyLookupFunction();
+ customRedirectUriValidationStrategyLookupStrategy = new CustomRedirectUriValidationStrategyLookupFunction();
}
/**
@@ -127,7 +134,7 @@ public class ValidateRedirectURI extends AbstractOAuthAuthorizationResponseActio
*
* @param strategy lookup strategy
*/
- public void setUnregisteredClientPolicyLookupStrategy(
+ public void setUnregisteredClientPolicyLookupStrategy(@Nonnull
final Function<ProfileRequestContext, Map<String, UnregisteredClientPolicy>> strategy) {
checkSetterPreconditions();
@@ -140,13 +147,29 @@ public class ValidateRedirectURI extends AbstractOAuthAuthorizationResponseActio
*
* @param enforcer policy enforcer
*/
- public void setUnregisteredClientPolicyEnforcer(
+ public void setUnregisteredClientPolicyEnforcer(@Nonnull
final BiFunction<Object, MetadataPolicy, Pair<Object, Boolean>> enforcer) {
checkSetterPreconditions();
unregisteredClientPolicyEnforcer = Constraint.isNotNull(enforcer, "Unregistered client policy cannot be null");
}
+ /**
+ * Set the strategy to obtain custom redirect URI validation strategy. If a non-null value is resolved via strategy,
+ * the bi-predicate will be used for validating the incoming request URI value.
+ *
+ * @param strategy lookup strategy
+ *
+ * @since 4.2.0
+ */
+ public void setCustomRedirectUriValidationStrategyLookupStrategy(@Nonnull
+ final Function<ProfileRequestContext,BiPredicate<URI, ProfileRequestContext>> strategy) {
+ checkSetterPreconditions();
+
+ customRedirectUriValidationStrategyLookupStrategy =
+ Constraint.isNotNull(strategy, "Custom redirect URI validation lookup strategy cannot be null");
+ }
+
/** {@inheritDoc} */
@Override
protected void doInitialize() throws ComponentInitializationException {
@@ -164,6 +187,19 @@ public class ValidateRedirectURI extends AbstractOAuthAuthorizationResponseActio
final OIDCAuthenticationResponseContext oidcResponseContext = getOidcResponseContext();
assert oidcResponseContext != null;
+ final BiPredicate<URI, ProfileRequestContext> customRedirectUriValidationStrategy =
+ customRedirectUriValidationStrategyLookupStrategy.apply(profileRequestContext);
+ if (customRedirectUriValidationStrategy != null) {
+ if (!customRedirectUriValidationStrategy.test(requestRedirectURI, profileRequestContext)) {
+ log.warn("{} Custom redirect URI validation failed for {}", getLogPrefix(), requestRedirectURI);
+ ActionSupport.buildEvent(profileRequestContext, OidcEventIds.INVALID_REDIRECT_URI);
+ } else {
+ oidcResponseContext.setRedirectURI(requestRedirectURI);
+ log.debug("{} Custom redirect URI validation successful for {}", getLogPrefix(), requestRedirectURI);
+ }
+ return;
+ }
+
if (requestRedirectURI != null && getMetadataContext() == null) {
final Map<String, UnregisteredClientPolicy> policies =
unregisteredClientPolicyLookupStrategy.apply(profileRequestContext);
@@ -175,8 +211,8 @@ public class ValidateRedirectURI extends AbstractOAuthAuthorizationResponseActio
final Boolean enforcerResult = result.getSecond();
if (enforcerResult != null && enforcerResult.booleanValue()
&& requestRedirectURI.toString().equals(result.getFirst())) {
- log.debug("{} Redirection URI {} accepted by the policy for unregistered clients", getLogPrefix(),
- requestRedirectURI);
+ log.debug("{} Redirection URI {} accepted by the policy for unregistered clients",
+ getLogPrefix(), requestRedirectURI);
oidcResponseContext.setRedirectURI(requestRedirectURI);
return;
}
diff --git a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/logic/MetadataPolicyRedirectUriValidator.java b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/logic/MetadataPolicyRedirectUriValidator.java
new file mode 100644
index 00000000..8dbd8d57
--- /dev/null
+++ b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/logic/MetadataPolicyRedirectUriValidator.java
@@ -0,0 +1,108 @@
+/*
+ * 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.profile.logic;
+
+import java.net.URI;
+import java.util.Map;
+import java.util.Optional;
+import java.util.function.BiFunction;
+import java.util.function.BiPredicate;
+import java.util.function.Predicate;
+
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+
+import org.opensaml.profile.context.ProfileRequestContext;
+
+import net.shibboleth.oidc.metadata.policy.MetadataPolicy;
+import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
+import net.shibboleth.shared.collection.Pair;
+import net.shibboleth.shared.component.AbstractIdentifiableInitializableComponent;
+import net.shibboleth.shared.component.ComponentInitializationException;
+import net.shibboleth.shared.logic.Constraint;
+
+/**
+ * A custom bi-predicate for redirect URI validation exploiting metadata policy.
+ */
+public class MetadataPolicyRedirectUriValidator extends AbstractIdentifiableInitializableComponent
+ implements BiPredicate<URI, ProfileRequestContext> {
+
+ /** The metadata policy enforcer function used against the {@link URI} given as input. */
+ @NonnullAfterInit private BiFunction<Object,MetadataPolicy,Pair<Object,Boolean>> metadataPolicyEnforcer;
+
+ /** The metadata policy validator for verifying the given metadata policy. */
+ @NonnullAfterInit private Predicate<Map<String, MetadataPolicy>> metadataPolicyValidator;
+
+ /** The metadata policy used for the enforcer function for validating the redirect URI. */
+ @NonnullAfterInit private MetadataPolicy metadataPolicy;
+
+ /**
+ * Set the metadata policy enforcer function used against the {@link URI} given as input.
+ *
+ * @param enforcer What to set.
+ */
+ public void setMetadataPolicyEnforcer(
+ @Nonnull final BiFunction<Object,MetadataPolicy,Pair<Object,Boolean>> enforcer) {
+ checkSetterPreconditions();
+ metadataPolicyEnforcer = Constraint.isNotNull(enforcer, "Metadata policy enforcer cannot be null");
+ }
+
+ /**
+ * Set the metadata policy validator function for verifying the given metadata policy.
+ *
+ * @param validator What to set.
+ */
+ public void setMetadataPolicyValidator(@Nonnull final Predicate<Map<String, MetadataPolicy>> validator) {
+ checkSetterPreconditions();
+ metadataPolicyValidator = Constraint.isNotNull(validator, "Metadata policy validator cannot be null");
+ }
+
+ /**
+ * Set the metadata policy used for the enforcer function for validating the redirect URI.
+ *
+ * @param policy What to set.
+ */
+ public void setMetadataPolicy(@Nonnull final MetadataPolicy policy) {
+ checkSetterPreconditions();
+ metadataPolicy = Constraint.isNotNull(policy, "Metadata policy cannot be null");
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public void doInitialize() throws ComponentInitializationException {
+ super.doInitialize();
+ if (metadataPolicyEnforcer == null) {
+ throw new ComponentInitializationException("Metadata policy enforcer cannot be null");
+ }
+ if (metadataPolicyValidator == null) {
+ throw new ComponentInitializationException("Metadata policy validator cannot be null");
+ }
+ if (metadataPolicy == null) {
+ throw new ComponentInitializationException("Metadata policy cannot be null");
+ }
+ if (!metadataPolicyValidator.test(Map.of("redirect_uri", metadataPolicy))) {
+ throw new ComponentInitializationException("Metadata policy didn't pass the validation");
+ }
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public boolean test(@Nullable final URI uri, @Nullable final ProfileRequestContext profileRequestContext) {
+ final Pair<Object,Boolean> enforcerResult =
+ metadataPolicyEnforcer.apply(uri == null ? null : uri.toString(), metadataPolicy);
+ return enforcerResult != null && Optional.of(enforcerResult.getSecond()).orElse(Boolean.FALSE);
+ }
+
+}
diff --git a/idp-oidc-extension-impl/src/main/resources/META-INF/net.shibboleth.idp/postconfig.xml b/idp-oidc-extension-impl/src/main/resources/META-INF/net.shibboleth.idp/postconfig.xml
index 0ea9dd1b..7196b647 100644
--- a/idp-oidc-extension-impl/src/main/resources/META-INF/net.shibboleth.idp/postconfig.xml
+++ b/idp-oidc-extension-impl/src/main/resources/META-INF/net.shibboleth.idp/postconfig.xml
@@ -859,4 +859,15 @@
</constructor-arg>
</bean>
+ <bean id="shibboleth.oidc.MetadataPolicyRedirectUriValidator" abstract="true"
+ class="net.shibboleth.idp.plugin.oidc.op.profile.logic.MetadataPolicyRedirectUriValidator">
+ <property name="metadataPolicyEnforcer">
+ <bean class="net.shibboleth.oidc.metadata.policy.impl.DefaultMetadataPolicyEnforcer"
+ p:customMetadataPolicyOperators="#{getObject('shibboleth.oidc.RedirectUriValidator.MetadataPolicyCustomOperators') ?: getObject('shibboleth.oidc.DefaultMetadataPolicyCustomOperators')}"/>
+ </property>
+ <property name="metadataPolicyValidator">
+ <bean class="net.shibboleth.oidc.metadata.policy.impl.DefaultMetadataPolicyValidator"
+ p:customMetadataPolicyOperators="#{getObject('shibboleth.oidc.RedirectUriValidator.MetadataPolicyCustomOperators') ?: getObject('shibboleth.oidc.DefaultMetadataPolicyCustomOperators')}"/>
+ </property>
+ </bean>
</beans>
diff --git a/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/ValidateRedirectURITest.java b/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/ValidateRedirectURITest.java
index ee79116d..9a3c93cb 100644
--- a/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/ValidateRedirectURITest.java
+++ b/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/ValidateRedirectURITest.java
@@ -19,6 +19,7 @@ import java.net.URISyntaxException;
import java.util.List;
import java.util.Map;
import java.util.Set;
+import java.util.function.BiPredicate;
import net.shibboleth.idp.plugin.oidc.op.profile.impl.BaseOIDCResponseActionTest;
import net.shibboleth.idp.profile.testing.ActionTestingSupport;
@@ -29,6 +30,7 @@ import net.shibboleth.oidc.metadata.policy.impl.DefaultMetadataPolicyEnforcer;
import net.shibboleth.oidc.profile.core.OidcEventIds;
import net.shibboleth.shared.component.ComponentInitializationException;
+import org.opensaml.profile.context.ProfileRequestContext;
import org.springframework.webflow.execution.Event;
import org.testng.Assert;
import org.testng.annotations.Test;
@@ -55,6 +57,12 @@ public class ValidateRedirectURITest extends BaseOIDCResponseActionTest {
private void init(final boolean requireRequestedValue, final URI requestedUri, final Set<URI> validUris,
final Set<URI> registeredUris, final Map<String, UnregisteredClientPolicy> policies)
throws ComponentInitializationException {
+ init(requireRequestedValue, requestedUri, validUris, registeredUris, policies, null);
+ }
+
+ private void init(final boolean requireRequestedValue, final URI requestedUri, final Set<URI> validUris,
+ final Set<URI> registeredUris, final Map<String, UnregisteredClientPolicy> policies,
+ final BiPredicate<URI,ProfileRequestContext> customValidator) throws ComponentInitializationException {
action = new ValidateRedirectURI();
action.setRequireRequestedValue(requireRequestedValue);
action.setRedirectURILookupStrategy(prc -> requestedUri);
@@ -68,6 +76,9 @@ public class ValidateRedirectURITest extends BaseOIDCResponseActionTest {
action.setUnregisteredClientPolicyLookupStrategy(prc -> policies);
}
action.setUnregisteredClientPolicyEnforcer(new DefaultMetadataPolicyEnforcer());
+ if (customValidator != null) {
+ action.setCustomRedirectUriValidationStrategyLookupStrategy(prc -> customValidator);
+ }
action.initialize();
}
@@ -106,6 +117,22 @@ public class ValidateRedirectURITest extends BaseOIDCResponseActionTest {
Assert.assertNull(respCtx.getRedirectURI());
}
+ @Test
+ public void testNoMatchViaMetadata_overrideViaCustomPredicate()
+ throws ComponentInitializationException, URISyntaxException {
+ init(true, new URI(requestUri), null, null, null, (uri, prc) -> true);
+ OIDCMetadataContext oidcCtx =
+ profileRequestCtx.ensureInboundMessageContext().ensureSubcontext(OIDCMetadataContext.class);
+ OIDCClientMetadata metaData = new OIDCClientMetadata();
+ metaData.setRedirectionURI(new URI("https://notmatching.org"));
+ OIDCClientInformation information =
+ new OIDCClientInformation(new ClientID("test"), null, metaData, null, null, null);
+ oidcCtx.setClientInformation(information);
+ final Event event = action.execute(requestCtx);
+ ActionTestingSupport.assertProceedEvent(event);
+ Assert.assertNotNull(respCtx.getRedirectURI());
+ }
+
/**
* Test case of having a match for redirect uri.
*
@@ -127,6 +154,21 @@ public class ValidateRedirectURITest extends BaseOIDCResponseActionTest {
Assert.assertNotNull(respCtx.getRedirectURI());
}
+ @Test
+ public void testMatch_overrideViaCustomPredicate() throws ComponentInitializationException, URISyntaxException {
+ init(true, new URI(requestUri), null, null, null, (uri, prc) -> false);
+ OIDCMetadataContext oidcCtx =
+ profileRequestCtx.ensureInboundMessageContext().ensureSubcontext(OIDCMetadataContext.class);
+ OIDCClientMetadata metaData = new OIDCClientMetadata();
+ metaData.setRedirectionURI(new URI("https://client.example.org/cb"));
+ OIDCClientInformation information =
+ new OIDCClientInformation(new ClientID("test"), null, metaData, null, null, null);
+ oidcCtx.setClientInformation(information);
+ final Event event = action.execute(requestCtx);
+ ActionTestingSupport.assertEvent(event, OidcEventIds.INVALID_REDIRECT_URI);
+ Assert.assertNull(respCtx.getRedirectURI());
+ }
+
@Test
public void testNoMatchViaPolicy() throws ComponentInitializationException, URISyntaxException {
init(true, new URI(requestUri), null, null, Map.of("redirect_uri", new UnregisteredClientPolicy(
diff --git a/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/logic/MetadataPolicyRedirectUriValidatorTest.java b/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/logic/MetadataPolicyRedirectUriValidatorTest.java
new file mode 100644
index 00000000..528f77a1
--- /dev/null
+++ b/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/logic/MetadataPolicyRedirectUriValidatorTest.java
@@ -0,0 +1,89 @@
+/*
+ * 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.profile.logic;
+
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.util.List;
+
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+import net.shibboleth.oidc.metadata.policy.MetadataPolicy;
+import net.shibboleth.oidc.metadata.policy.impl.DefaultMetadataPolicyEnforcer;
+import net.shibboleth.oidc.metadata.policy.impl.DefaultMetadataPolicyValidator;
+import net.shibboleth.shared.component.ComponentInitializationException;
+
+/**
+ * Unit tests for {@link MetadataPolicyRedirectUriValidator}.
+ */
+public class MetadataPolicyRedirectUriValidatorTest {
+
+ MetadataPolicyRedirectUriValidator predicate;
+
+ @SuppressWarnings("null")
+ public void init(final MetadataPolicy metadataPolicy) throws ComponentInitializationException {
+ predicate = new MetadataPolicyRedirectUriValidator();
+ predicate.setId("mockId");
+ predicate.setMetadataPolicyEnforcer(new DefaultMetadataPolicyEnforcer());
+ predicate.setMetadataPolicyValidator(new DefaultMetadataPolicyValidator());
+ predicate.setMetadataPolicy(metadataPolicy);
+ predicate.initialize();
+ }
+
+ @Test(expectedExceptions = ComponentInitializationException.class)
+ public void testInvalidPolicy() throws ComponentInitializationException {
+ init(new MetadataPolicy.Builder()
+ .withOneOfValues(List.of("mock"))
+ .withSubsetOfValues(List.of("mock")) // not compatible with oneOfValues
+ .build());
+ }
+
+ @Test
+ public void testSuccess() throws URISyntaxException, ComponentInitializationException {
+ init(new MetadataPolicy.Builder()
+ .withOneOfValues(List.of("https://example.org/cb1", "https://example.org/cb2"))
+ .build());
+ Assert.assertTrue(predicate.test(new URI("https://example.org/cb1"), null));
+ }
+
+ @Test
+ public void testFail() throws URISyntaxException, ComponentInitializationException {
+ init(new MetadataPolicy.Builder()
+ .withOneOfValues(List.of("https://example.org/cb1", "https://example.org/cb2"))
+ .build());
+ Assert.assertFalse(predicate.test(new URI("https://example.org/noMatch"), null));
+ }
+
+ @Test
+ public void testFail_nullAcceptedWithNonEssentialPolicy() throws URISyntaxException,
+ ComponentInitializationException {
+ init(new MetadataPolicy.Builder()
+ .withOneOfValues(List.of("https://example.org/cb1", "https://example.org/cb2"))
+ .build());
+ Assert.assertTrue(predicate.test(null, null));
+ }
+
+ @Test
+ public void testFail_nullNotAcceptedWithEssentialPolicy() throws URISyntaxException,
+ ComponentInitializationException {
+ init(new MetadataPolicy.Builder()
+ .withOneOfValues(List.of("https://example.org/cb1", "https://example.org/cb2"))
+ .withEssential(true)
+ .build());
+ Assert.assertFalse(predicate.test(null, null));
+ }
+
+}
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list