[java-idp-oidc] branch main updated: JOIDC-188 - Make offline_access consent handling configurable
Henri Mikkonen
henri.mikkonen at iki.fi
Fri Mar 15 07:31:29 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=ff878926156d53e875e79a68dcd3097315df5ec2
The following commit(s) were added to refs/heads/main by this push:
new ff878926 JOIDC-188 - Make offline_access consent handling configurable
ff878926 is described below
commit ff878926156d53e875e79a68dcd3097315df5ec2
Author: Henri Mikkonen <henri.mikkonen at iki.fi>
AuthorDate: Fri Mar 15 09:31:08 2024 +0200
JOIDC-188 - Make offline_access consent handling configurable
https://shibboleth.atlassian.net/browse/JOIDC-188
Refactored RevokeConsent action to use a configurable predicate for deciding whether or not to revoke pre-existing consent. By default
the functionality remains the same: consent is revoked if offline_access scope is involved, or prompt contains consent.
The default behaviour is provided by shibboleth.oidc.DefaultRevokeConsentPredicate. It is wired with two Predicate<ProfileRequestContext>):
- shibboleth.oidc.RevokeWithOfflineAccessScopePredicate: whether to revoke with offline_access scope, defaults to shibboleth.Conditions.TRUE
- shibboleth.oidc.RevokeWithConsentPromptPredicate: whether to revoke when prompt contains consent, defaults to shibboleth.Conditions.TRUE
The default behaviour may be overriden with shibboleth.oidc.RevokeConsentPredicate (Predicate<ProfileRequestContext>) bean.
---
.../plugin/oidc/op/profile/impl/RevokeConsent.java | 43 +++----
.../logic/DefaultRevokeConsentPredicate.java | 130 +++++++++++++++++++++
.../idp/flows/oidc/authorize/authorize-beans.xml | 8 +-
.../oidc/op/profile/impl/RevokeConsentTest.java | 9 +-
.../logic/DefaultRevokeConsentPredicateTest.java | 129 ++++++++++++++++++++
5 files changed, 289 insertions(+), 30 deletions(-)
diff --git a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/RevokeConsent.java b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/RevokeConsent.java
index ca1fb53b..f66777e5 100644
--- a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/RevokeConsent.java
+++ b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/RevokeConsent.java
@@ -14,65 +14,58 @@
package net.shibboleth.idp.plugin.oidc.op.profile.impl;
-import java.util.function.Function;
+import java.util.function.Predicate;
import javax.annotation.Nonnull;
import org.opensaml.profile.context.ProfileRequestContext;
import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import com.nimbusds.oauth2.sdk.Scope;
-import com.nimbusds.openid.connect.sdk.OIDCScopeValue;
-import com.nimbusds.openid.connect.sdk.Prompt;
import net.shibboleth.idp.consent.context.ConsentManagementContext;
-import net.shibboleth.idp.plugin.oidc.op.profile.context.navigate.DefaultRequestedPromptLookupFunction;
+import net.shibboleth.idp.plugin.oidc.op.profile.logic.DefaultRevokeConsentPredicate;
import net.shibboleth.shared.logic.Constraint;
+import net.shibboleth.shared.primitive.LoggerFactory;
/**
- * Action that revokes consent if offline_access scope or prompt with consent is requested.
+ * Action that revokes consent if the configurable predicate returns true. The revocation is signaled by setting the
+ * flag {@link ConsentManagementContext#setRevokeConsent(boolean)}.
*/
public class RevokeConsent extends AbstractOIDCResponseAction {
/** Class logger. */
@Nonnull private Logger log = LoggerFactory.getLogger(RevokeConsent.class);
- /** Strategy used to obtain the requested prompt value. */
+ /** Predicate used to decide if pre-existing consent is to be revoked. */
@Nonnull
- private Function<ProfileRequestContext, Prompt> promptLookupStrategy;
+ private Predicate<ProfileRequestContext> revokePredicate;
/**
* Constructor.
*/
public RevokeConsent() {
- promptLookupStrategy = new DefaultRequestedPromptLookupFunction();
+ revokePredicate = new DefaultRevokeConsentPredicate();
}
/**
- * Set the strategy used to locate the requested prompt.
+ * Set the predicate used to decide if pre-existing consent is to be revoked.
+ *
+ * @param predicate What to set.
*
- * @param strategy lookup strategy
+ * @since 4.1.0
*/
- public void setPromptLookupStrategy(@Nonnull final Function<ProfileRequestContext, Prompt> strategy) {
- ifInitializedThrowUnmodifiabledComponentException();
- promptLookupStrategy = Constraint.isNotNull(strategy, "PromptLookupStrategy lookup strategy cannot be null");
+ public void setRevokeConsentPredicate(@Nonnull final Predicate<ProfileRequestContext> predicate) {
+ checkSetterPreconditions();
+ revokePredicate = Constraint.isNotNull(predicate, "Predicate cannot be null");
}
/** {@inheritDoc} */
@Override
protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext) {
-
- final Scope scope = getOidcResponseContext().getScope();
- if (scope != null && getOidcResponseContext().getScope().contains(OIDCScopeValue.OFFLINE_ACCESS)) {
- log.debug("{} Pre-existing consent revoked as offline_access scope is requested", getLogPrefix());
+ if (revokePredicate.test(profileRequestContext)) {
+ log.debug("{} Pre-existing consent revoked as predicate returned true", getLogPrefix());
profileRequestContext.ensureSubcontext(ConsentManagementContext.class).setRevokeConsent(true);
} else {
- final Prompt prompt = promptLookupStrategy.apply(profileRequestContext);
- if (prompt != null && prompt.contains(Prompt.Type.CONSENT)) {
- log.debug("{} Pre-existing consent revoked as user consent is requested", getLogPrefix());
- profileRequestContext.ensureSubcontext(ConsentManagementContext.class).setRevokeConsent(true);
- }
+ log.debug("{} Pre-existing consent revoked as predicate returned false", getLogPrefix());
}
}
}
\ No newline at end of file
diff --git a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/logic/DefaultRevokeConsentPredicate.java b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/logic/DefaultRevokeConsentPredicate.java
new file mode 100644
index 00000000..82aa5e83
--- /dev/null
+++ b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/logic/DefaultRevokeConsentPredicate.java
@@ -0,0 +1,130 @@
+/*
+ * 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.util.function.Function;
+import java.util.function.Predicate;
+
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+
+import net.shibboleth.idp.plugin.oidc.op.profile.context.navigate.DefaultRequestedPromptLookupFunction;
+import net.shibboleth.idp.plugin.oidc.op.profile.context.navigate.ValidatedScopeLookupFunction;
+import net.shibboleth.shared.component.AbstractInitializableComponent;
+import net.shibboleth.shared.logic.Constraint;
+import net.shibboleth.shared.logic.PredicateSupport;
+import net.shibboleth.shared.primitive.LoggerFactory;
+
+import org.opensaml.profile.context.ProfileRequestContext;
+import org.slf4j.Logger;
+
+import com.nimbusds.oauth2.sdk.Scope;
+import com.nimbusds.openid.connect.sdk.OIDCScopeValue;
+import com.nimbusds.openid.connect.sdk.Prompt;
+
+/**
+ * Default predicate to decide if the pre-existing consent should be revoked. By default the revocation is done if the
+ * scope offline_access was requested (and granted) or if prompt contained consent value.
+ *
+ * @since 4.1.0
+ */
+public class DefaultRevokeConsentPredicate extends AbstractInitializableComponent implements Predicate<ProfileRequestContext> {
+
+ /** Class logger. */
+ @Nonnull
+ private final Logger log = LoggerFactory.getLogger(DefaultRevokeConsentPredicate.class);
+
+ /** Strategy used to obtain the validated scope value. */
+ @Nonnull private Function<ProfileRequestContext,Scope> scopeLookupStrategy;
+
+ /** Strategy used to obtain the requested prompt value. */
+ @Nonnull private Function<ProfileRequestContext, Prompt> promptLookupStrategy;
+
+ /** Predicate used to determine if consent should be revoked with offline_access scope. */
+ @Nonnull private Predicate<ProfileRequestContext> revokeWithOfflineAccessScopePredicate;
+
+ /** Predicate used to determine if consent should be revoked with consent prompt. */
+ @Nonnull private Predicate<ProfileRequestContext> revokeWithConsentPromptPredicate;
+
+ /**
+ * Constructor.
+ */
+ public DefaultRevokeConsentPredicate() {
+ scopeLookupStrategy = new ValidatedScopeLookupFunction();
+ promptLookupStrategy = new DefaultRequestedPromptLookupFunction();
+ revokeWithOfflineAccessScopePredicate = PredicateSupport.alwaysTrue();
+ revokeWithConsentPromptPredicate = PredicateSupport.alwaysTrue();
+ }
+
+ /**
+ * Set the strategy used to locate the validated scope.
+ *
+ * @param strategy What to set.
+ */
+ public void setScopeLookupStrategy(@Nonnull final Function<ProfileRequestContext, Scope> strategy) {
+ checkSetterPreconditions();
+ scopeLookupStrategy = Constraint.isNotNull(strategy, "ScopeLookupStrategy lookup strategy cannot be null");
+ }
+
+ /**
+ * Set the strategy used to locate the requested prompt.
+ *
+ * @param strategy What to set.
+ */
+ public void setPromptLookupStrategy(@Nonnull final Function<ProfileRequestContext, Prompt> strategy) {
+ checkSetterPreconditions();
+ promptLookupStrategy = Constraint.isNotNull(strategy, "PromptLookupStrategy lookup strategy cannot be null");
+ }
+
+ /**
+ * Set the predicate used to determine if consent should be revoked with offline_access scope.
+ *
+ * @param predicate What to set.
+ */
+ public void setRevokeWithOfflineAccessScopePredicate(@Nonnull final Predicate<ProfileRequestContext> predicate) {
+ checkSetterPreconditions();
+ revokeWithOfflineAccessScopePredicate = Constraint.isNotNull(predicate,
+ "RevokeWithOfflineAccessScopePredicate cannot be null");
+ }
+
+ /**
+ * Set the predicate used to determine if consent should be revoked with consent prompt.
+ *
+ * @param predicate What to set.
+ */
+ public void setRevokeWithConsentPromptPredicate(@Nonnull final Predicate<ProfileRequestContext> predicate) {
+ checkSetterPreconditions();
+ revokeWithConsentPromptPredicate = Constraint.isNotNull(predicate,
+ "RevokeWithConsentPromptPredicate cannot be null");
+ }
+ /** {@inheritDoc} */
+ @Override
+ public boolean test(@Nullable final ProfileRequestContext input) {
+ final Scope scope = scopeLookupStrategy.apply(input);
+ if (scope != null && scope.contains(OIDCScopeValue.OFFLINE_ACCESS)
+ && revokeWithOfflineAccessScopePredicate.test(input)) {
+ log.debug("Pre-existing consent to be revoked as offline_access scope is requested");
+ return true;
+ } else {
+ final Prompt prompt = promptLookupStrategy.apply(input);
+ if (prompt != null && prompt.contains(Prompt.Type.CONSENT)
+ && revokeWithConsentPromptPredicate.test(input)) {
+ log.debug("Pre-existing consent to be revoked as user consent is requested");
+ return true;
+ }
+ }
+ return false;
+ }
+}
\ No newline at end of file
diff --git a/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/authorize/authorize-beans.xml b/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/authorize/authorize-beans.xml
index e2ce0c4f..847a9b79 100644
--- a/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/authorize/authorize-beans.xml
+++ b/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/authorize/authorize-beans.xml
@@ -337,7 +337,13 @@
class="org.opensaml.storage.impl.client.PopulateClientStorageLoadContext" scope="prototype"
p:storageServices="#{ getObject('shibboleth.ClientStorageServices') ?: getObject('shibboleth.DefaultClientStorageServices') }" />
- <bean id="RevokeConsent" class="net.shibboleth.idp.plugin.oidc.op.profile.impl.RevokeConsent" scope="prototype" />
+ <bean id="RevokeConsent" class="net.shibboleth.idp.plugin.oidc.op.profile.impl.RevokeConsent" scope="prototype"
+ p:revokeConsentPredicate="#{getObject('shibboleth.oidc.RevokeConsentPredicate') ?: getObject('shibboleth.oidc.DefaultRevokeConsentPredicate')}" />
+
+ <bean id="shibboleth.oidc.DefaultRevokeConsentPredicate"
+ class="net.shibboleth.idp.plugin.oidc.op.profile.logic.DefaultRevokeConsentPredicate"
+ p:revokeWithOfflineAccessScopePredicate="#{getObject('shibboleth.oidc.RevokeWithOfflineAccessScopePredicate') ?: getObject('shibboleth.Conditions.TRUE')}"
+ p:revokeWithConsentPromptPredicate="#{getObject('shibboleth.oidc.RevokeWithConsentPromptPredicate') ?: getObject('shibboleth.Conditions.TRUE')}" />
<bean id="SetAuthenticationTimeToResponseContext"
class="net.shibboleth.idp.plugin.oidc.op.profile.impl.SetAuthenticationTimeToResponseContext" scope="prototype" />
diff --git a/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/RevokeConsentTest.java b/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/RevokeConsentTest.java
index cb20ce3c..fec4ff41 100644
--- a/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/RevokeConsentTest.java
+++ b/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/RevokeConsentTest.java
@@ -64,30 +64,31 @@ public class RevokeConsentTest extends BaseOIDCResponseActionTest {
scope.add(OIDCScopeValue.OFFLINE_ACCESS);
respCtx.setScope(scope);
ActionTestingSupport.assertProceedEvent(action.execute(requestCtx));
- Assert.assertTrue(profileRequestCtx.getSubcontext(ConsentManagementContext.class).getRevokeConsent());
+ Assert.assertTrue(profileRequestCtx.ensureSubcontext(ConsentManagementContext.class).getRevokeConsent());
}
/**
* Test that action revokes consent for prompt = consent.
*/
@Test
- public void testNoRevocationCache() throws NoSuchAlgorithmException, ComponentInitializationException {
+ public void testRevokeViaPrompt() throws NoSuchAlgorithmException, ComponentInitializationException {
AuthenticationRequest req = new AuthenticationRequest.Builder(new ResponseType("code"), new Scope("openid"),
new ClientID("000123"), URI.create("https://example.com/callback")).prompt(new Prompt("consent"))
.state(new State()).build();
setAuthenticationRequest(req);
action.initialize();
ActionTestingSupport.assertProceedEvent(action.execute(requestCtx));
- Assert.assertTrue(profileRequestCtx.getSubcontext(ConsentManagementContext.class).getRevokeConsent());
+ Assert.assertTrue(profileRequestCtx.ensureSubcontext(ConsentManagementContext.class).getRevokeConsent());
}
/**
* Test that action does not accept null strategy
*/
+ @SuppressWarnings("null")
@Test(expectedExceptions = ConstraintViolationException.class)
public void testNullStrategy() throws NoSuchAlgorithmException, ComponentInitializationException {
action = new RevokeConsent();
- action.setPromptLookupStrategy(null);
+ action.setRevokeConsentPredicate(null);
}
}
\ No newline at end of file
diff --git a/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/logic/DefaultRevokeConsentPredicateTest.java b/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/logic/DefaultRevokeConsentPredicateTest.java
new file mode 100644
index 00000000..8a91362a
--- /dev/null
+++ b/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/logic/DefaultRevokeConsentPredicateTest.java
@@ -0,0 +1,129 @@
+/*
+ * 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 static org.mockito.Mockito.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.util.function.Function;
+import java.util.function.Predicate;
+
+import org.opensaml.profile.context.ProfileRequestContext;
+import org.testng.Assert;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import com.nimbusds.oauth2.sdk.Scope;
+import com.nimbusds.openid.connect.sdk.Prompt;
+
+import net.shibboleth.shared.component.ComponentInitializationException;
+
+/**
+ * Unit tests for {@link DefaultRevokeConsentPredicate}.
+ */
+ at SuppressWarnings("unchecked")
+public class DefaultRevokeConsentPredicateTest {
+
+ DefaultRevokeConsentPredicate predicate;
+ Function<ProfileRequestContext, Scope> scopeLookup = mock(Function.class);
+ Function<ProfileRequestContext, Prompt> promptLookup = mock(Function.class);
+ Predicate<ProfileRequestContext> offlineAccessPredicate = mock(Predicate.class);
+ Predicate<ProfileRequestContext> consentPromptPredicate = mock(Predicate.class);
+
+ @BeforeMethod
+ public void setup() throws ComponentInitializationException {
+ predicate = new DefaultRevokeConsentPredicate();
+ assert scopeLookup != null;
+ predicate.setScopeLookupStrategy(scopeLookup);
+ assert promptLookup != null;
+ predicate.setPromptLookupStrategy(promptLookup);
+ assert offlineAccessPredicate != null;
+ predicate.setRevokeWithOfflineAccessScopePredicate(offlineAccessPredicate);
+ assert consentPromptPredicate != null;
+ predicate.setRevokeWithConsentPromptPredicate(consentPromptPredicate);
+ predicate.initialize();
+ }
+
+ @Test
+ public void noScopeNoPrompt_shouldReturnFalse() {
+ when(scopeLookup.apply(any())).thenReturn(null);
+ when(promptLookup.apply(any())).thenReturn(null);
+ Assert.assertFalse(predicate.test(new ProfileRequestContext()));
+ }
+
+ @Test
+ public void scopeNoOfflineAccess_shouldReturnFalse() {
+ when(scopeLookup.apply(any())).thenReturn(new Scope("openid"));
+ when(promptLookup.apply(any())).thenReturn(null);
+ Assert.assertFalse(predicate.test(new ProfileRequestContext()));
+ }
+
+ @Test
+ public void scopeWithOfflineAccessAndPredicate_shouldReturnTrue() {
+ when(scopeLookup.apply(any())).thenReturn(new Scope("openid", "offline_access"));
+ when(offlineAccessPredicate.test(any())).thenReturn(true);
+ when(promptLookup.apply(any())).thenReturn(null);
+ Assert.assertTrue(predicate.test(new ProfileRequestContext()));
+ }
+
+ @Test
+ public void scopeWithOfflineAccessAndNoPredicate_shouldReturnFalse() {
+ when(scopeLookup.apply(any())).thenReturn(new Scope("openid", "offline_access"));
+ when(offlineAccessPredicate.test(any())).thenReturn(false);
+ when(promptLookup.apply(any())).thenReturn(null);
+ Assert.assertFalse(predicate.test(new ProfileRequestContext()));
+ }
+
+ @Test
+ public void promptNoConsent_shouldReturnFalse() {
+ when(scopeLookup.apply(any())).thenReturn(null);
+ when(promptLookup.apply(any())).thenReturn(new Prompt());
+ Assert.assertFalse(predicate.test(new ProfileRequestContext()));
+ }
+
+ @Test
+ public void promptConsentAndPredicate_shouldReturnTrue() {
+ when(scopeLookup.apply(any())).thenReturn(null);
+ when(promptLookup.apply(any())).thenReturn(new Prompt("consent"));
+ when(consentPromptPredicate.test(any())).thenReturn(true);
+ Assert.assertTrue(predicate.test(new ProfileRequestContext()));
+ }
+
+ @Test
+ public void promptConsentAndNoPredicate_shouldReturnFalse() {
+ when(scopeLookup.apply(any())).thenReturn(null);
+ when(promptLookup.apply(any())).thenReturn(new Prompt("consent"));
+ when(consentPromptPredicate.test(any())).thenReturn(false);
+ Assert.assertFalse(predicate.test(new ProfileRequestContext()));
+ }
+
+ @Test
+ public void promptContainsConsentAndPredicate_shouldReturnTrue() {
+ when(scopeLookup.apply(any())).thenReturn(null);
+ when(promptLookup.apply(any())).thenReturn(new Prompt("login", "consent"));
+ when(consentPromptPredicate.test(any())).thenReturn(true);
+ Assert.assertTrue(predicate.test(new ProfileRequestContext()));
+ }
+
+ @Test
+ public void promptContainsConsentAndNoPredicate_shouldReturnFalse() {
+ when(scopeLookup.apply(any())).thenReturn(null);
+ when(promptLookup.apply(any())).thenReturn(new Prompt("login", "consent"));
+ when(consentPromptPredicate.test(any())).thenReturn(false);
+ Assert.assertFalse(predicate.test(new ProfileRequestContext()));
+ }
+
+}
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list