[java-idp-oidc] branch main updated: JOIDC-117 - Take clock skew into account in revocation lifetimes
Henri Mikkonen
henri.mikkonen at iki.fi
Wed Jul 6 12:25:06 UTC 2022
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=5f22b9568d45e123e289f559e27110af642be15f
The following commit(s) were added to refs/heads/main by this push:
new 5f22b956 JOIDC-117 - Take clock skew into account in revocation lifetimes
5f22b956 is described below
commit 5f22b9568d45e123e289f559e27110af642be15f
Author: Henri Mikkonen <henri.mikkonen at iki.fi>
AuthorDate: Wed Jul 6 15:23:43 2022 +0300
JOIDC-117 - Take clock skew into account in revocation lifetimes
https://shibboleth.atlassian.net/browse/JOIDC-117
The default functions for fetching token and chain revocation lifetime now add configurable
clock skew plus additional 5 minutes into the lifetimes. Those functions are now wired to
the SWF actions that performs revocations (RevokeToken, ValidateGrant and
SetRefreshTokenToResponseContext) in a way that the clock skew value is taken from the
idp.policy.clockSkew property.
---
.../oidc/op/oauth2/profile/impl/RevokeToken.java | 4 +-
.../plugin/oidc/op/profile/impl/ValidateGrant.java | 3 +-
...faultChainRevocationLifetimeLookupStrategy.java | 74 +++++++++++++++++
...faultTokenRevocationLifetimeLookupStrategy.java | 33 +++++++-
.../flows/oauth2/revocation/revocation-beans.xml | 11 ++-
.../idp/flows/oidc/token/token-beans.xml | 12 ++-
.../impl/SetRefreshTokenToResponseContextTest.java | 6 ++
...tTokenRevocationLifetimeLookupStrategyTest.java | 94 ++++++++++++++++++++++
8 files changed, 228 insertions(+), 9 deletions(-)
diff --git a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/RevokeToken.java b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/RevokeToken.java
index 65aed21e..d7d884f4 100644
--- a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/RevokeToken.java
+++ b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/RevokeToken.java
@@ -33,12 +33,12 @@ import org.slf4j.LoggerFactory;
import com.nimbusds.jwt.JWTClaimsSet;
import net.shibboleth.idp.plugin.oidc.op.oauth2.messaging.context.OAuth2TokenMgmtResponseContext;
+import net.shibboleth.idp.plugin.oidc.op.profile.logic.DefaultChainRevocationLifetimeLookupStrategy;
import net.shibboleth.idp.plugin.oidc.op.profile.logic.DefaultRootTokenIdentifierLookupStrategy;
import net.shibboleth.idp.plugin.oidc.op.profile.logic.DefaultTokenRevocationLifetimeLookupStrategy;
import net.shibboleth.idp.plugin.oidc.op.storage.RevocationCacheContexts;
import net.shibboleth.idp.profile.AbstractProfileAction;
import net.shibboleth.idp.profile.IdPEventIds;
-import net.shibboleth.oidc.profile.config.navigate.RevocationLifetimeLookupFunction;
import net.shibboleth.oidc.profile.config.navigate.RevocationMethodLookupFunction;
import net.shibboleth.oidc.profile.core.OidcEventIds;
import net.shibboleth.oidc.profile.oauth2.config.OAuth2TokenRevocationConfiguration.OAuth2TokenRevocationMethod;
@@ -95,7 +95,7 @@ public class RevokeToken extends AbstractProfileAction {
*/
public RevokeToken() {
revocationMethodLookupStrategy = new RevocationMethodLookupFunction();
- chainRevocationLifetimeLookupStrategy = new RevocationLifetimeLookupFunction();
+ chainRevocationLifetimeLookupStrategy = new DefaultChainRevocationLifetimeLookupStrategy();
tokenRevocationLifetimeLookupStrategy = new DefaultTokenRevocationLifetimeLookupStrategy();
rootTokenIdentifierLookupStrategy = new DefaultRootTokenIdentifierLookupStrategy();
}
diff --git a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/ValidateGrant.java b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/ValidateGrant.java
index 765f67eb..9f31308d 100644
--- a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/ValidateGrant.java
+++ b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/ValidateGrant.java
@@ -39,6 +39,7 @@ import com.nimbusds.oauth2.sdk.GrantType;
import com.nimbusds.oauth2.sdk.RefreshTokenGrant;
import net.shibboleth.idp.plugin.oidc.op.messaging.context.OIDCAuthenticationResponseContext;
+import net.shibboleth.idp.plugin.oidc.op.profile.logic.DefaultChainRevocationLifetimeLookupStrategy;
import net.shibboleth.idp.plugin.oidc.op.storage.RevocationCacheContexts;
import net.shibboleth.idp.plugin.oidc.op.token.support.AuthorizeCodeClaimsSet;
import net.shibboleth.idp.plugin.oidc.op.token.support.RefreshTokenClaimsSet;
@@ -107,7 +108,7 @@ public class ValidateGrant extends AbstractOIDCTokenResponseAction {
dataSealer = Constraint.isNotNull(sealer, "DataSealer cannot be null");
relyingPartyContextLookupStrategy = new ChildContextLookup<>(RelyingPartyContext.class);
refreshTokensEnabledPredicate = new RefreshTokensEnabledPredicate();
- chainRevocationLifetimeLookupStrategy = new RevocationLifetimeLookupFunction();
+ chainRevocationLifetimeLookupStrategy = new DefaultChainRevocationLifetimeLookupStrategy();
((RevocationLifetimeLookupFunction) chainRevocationLifetimeLookupStrategy).setUseActiveProfileOnly(false);
}
diff --git a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/logic/DefaultChainRevocationLifetimeLookupStrategy.java b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/logic/DefaultChainRevocationLifetimeLookupStrategy.java
new file mode 100644
index 00000000..621babc3
--- /dev/null
+++ b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/logic/DefaultChainRevocationLifetimeLookupStrategy.java
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the University Corporation for Advanced Internet Development,
+ * Inc. (UCAID) under one or more contributor license agreements. See the
+ * NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The UCAID licenses this file to You 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.time.Duration;
+
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+
+import org.opensaml.profile.context.ProfileRequestContext;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import net.shibboleth.oidc.profile.config.navigate.RevocationLifetimeLookupFunction;
+import net.shibboleth.utilities.java.support.logic.Constraint;
+
+/**
+ * Default lookup function for fetching the chain revocation lifetime. This inherits the functionality of
+ * {@link RevocationLifetimeLookupFunction} but also adds the configurable clock skew value and additional 5 minutes
+ * to the returned value.
+ */
+public class DefaultChainRevocationLifetimeLookupStrategy extends RevocationLifetimeLookupFunction {
+
+ /** Class logger. */
+ @Nonnull
+ private final Logger log = LoggerFactory.getLogger(DefaultChainRevocationLifetimeLookupStrategy.class);
+
+ /** Positive clock skew adjustment to consider when calculating revocation lifetime. */
+ @Nonnull private Duration clockSkew;
+
+ /**
+ * Constructor.
+ */
+ public DefaultChainRevocationLifetimeLookupStrategy() {
+ clockSkew = Duration.ofMinutes(5);
+ }
+
+ /**
+ * Set the clock skew.
+ *
+ * @param skew clock skew to set
+ */
+ public void setClockSkew(@Nonnull final Duration skew) {
+ clockSkew = Constraint.isNotNull(skew, "Clock skew cannot be null").abs();
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ @Nullable
+ public Duration apply(@Nullable final ProfileRequestContext input) {
+ final Duration profileDuration = super.apply(input);
+ if (profileDuration == null || profileDuration.isZero()) {
+ log.debug("No chain expiration time could be resolved, returning null");
+ return null;
+ }
+ return profileDuration.plus(Duration.ofMinutes(5)).plus(clockSkew);
+ }
+
+}
\ No newline at end of file
diff --git a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/logic/DefaultTokenRevocationLifetimeLookupStrategy.java b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/logic/DefaultTokenRevocationLifetimeLookupStrategy.java
index 66ba4f52..0d3b6ec0 100644
--- a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/logic/DefaultTokenRevocationLifetimeLookupStrategy.java
+++ b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/logic/DefaultTokenRevocationLifetimeLookupStrategy.java
@@ -30,10 +30,16 @@ import org.slf4j.LoggerFactory;
import com.nimbusds.jwt.JWTClaimsSet;
+import net.shibboleth.utilities.java.support.logic.Constraint;
+
/**
* Default lookup function for fetching the token revocation lifetime from the given claims set. If an expiration
- * time is found from the claims set, a difference between now and it is returned. If the expiration time is in the
- * past, a {@link Duration#ZERO} is returned. If no expiration time is found, null is returned.
+ * time is found from the claims set, a difference between now and it is first calculated and then the configurable
+ * clock skew value and additional 5 minutes are added to the value. The result is returned.
+ *
+ * <p>If the expiration time is in the past with clock skew considered, a {@link Duration#ZERO} is returned.</p>
+ *
+ * <p>If no expiration time is found, null is returned.</p>
*/
public class DefaultTokenRevocationLifetimeLookupStrategy implements Function<JWTClaimsSet, Duration> {
@@ -41,6 +47,25 @@ public class DefaultTokenRevocationLifetimeLookupStrategy implements Function<JW
@Nonnull
private final Logger log = LoggerFactory.getLogger(DefaultTokenRevocationLifetimeLookupStrategy.class);
+ /** Positive clock skew adjustment to consider when calculating revocation lifetime. */
+ @Nonnull private Duration clockSkew;
+
+ /**
+ * Constructor.
+ */
+ public DefaultTokenRevocationLifetimeLookupStrategy() {
+ clockSkew = Duration.ofMinutes(5);
+ }
+
+ /**
+ * Set the clock skew.
+ *
+ * @param skew clock skew to set
+ */
+ public void setClockSkew(@Nonnull final Duration skew) {
+ clockSkew = Constraint.isNotNull(skew, "Clock skew cannot be null").abs();
+ }
+
/** {@inheritDoc} */
@Override
@Nullable
@@ -56,11 +81,11 @@ public class DefaultTokenRevocationLifetimeLookupStrategy implements Function<JW
}
final Instant now = Instant.now();
final Instant exp = expiration.toInstant();
- if (now.isAfter(exp)) {
+ if (now.isAfter(exp.plus(clockSkew))) {
log.debug("Token expiration time was in the past, returning ZERO");
return Duration.ZERO;
}
- return Duration.between(now, exp);
+ return Duration.between(now, exp).abs().plus(Duration.ofMinutes(5)).plus(clockSkew);
}
}
\ No newline at end of file
diff --git a/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oauth2/revocation/revocation-beans.xml b/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oauth2/revocation/revocation-beans.xml
index 334b5849..35e607f1 100644
--- a/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oauth2/revocation/revocation-beans.xml
+++ b/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oauth2/revocation/revocation-beans.xml
@@ -36,7 +36,16 @@
c:_0-ref="shibboleth.RelyingPartyResolverService" />
<bean id="RevokeToken" class="net.shibboleth.idp.plugin.oidc.op.oauth2.profile.impl.RevokeToken" scope="prototype"
- p:revocationCache-ref="shibboleth.oidc.RevocationCache" />
+ p:revocationCache-ref="shibboleth.oidc.RevocationCache">
+ <property name="tokenRevocationLifetimeLookupStrategy">
+ <bean class="net.shibboleth.idp.plugin.oidc.op.profile.logic.DefaultTokenRevocationLifetimeLookupStrategy"
+ p:clockSkew="%{idp.policy.clockSkew:PT5M}" />
+ </property>
+ <property name="chainRevocationLifetimeLookupStrategy">
+ <bean class="net.shibboleth.idp.plugin.oidc.op.profile.logic.DefaultChainRevocationLifetimeLookupStrategy"
+ p:clockSkew="%{idp.policy.clockSkew:PT5M}" />
+ </property>
+ </bean>
<bean id="FormOutboundMessage"
class="net.shibboleth.idp.plugin.oidc.op.oauth2.profile.impl.FormOutboundRevokeTokenResponseMessage"
diff --git a/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/token/token-beans.xml b/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/token/token-beans.xml
index 5b042d44..af045296 100644
--- a/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/token/token-beans.xml
+++ b/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/token/token-beans.xml
@@ -57,7 +57,12 @@
<bean id="ValidateGrant" class="net.shibboleth.idp.plugin.oidc.op.profile.impl.ValidateGrant" scope="prototype"
c:sealer-ref="#{'%{idp.oidc.tokenSealer:shibboleth.oidc.TokenSealer}'.trim()}"
p:replayCache-ref="shibboleth.ReplayCache"
- p:revocationCache-ref="shibboleth.oidc.RevocationCache" />
+ p:revocationCache-ref="shibboleth.oidc.RevocationCache">
+ <property name="chainRevocationLifetimeLookupStrategy">
+ <bean class="net.shibboleth.idp.plugin.oidc.op.profile.logic.DefaultChainRevocationLifetimeLookupStrategy"
+ p:clockSkew="%{idp.policy.clockSkew:PT5M}" />
+ </property>
+ </bean>
<bean id="ValidatePKCE" class="net.shibboleth.idp.plugin.oidc.op.profile.impl.ValidatePKCE" scope="prototype" />
@@ -331,6 +336,11 @@
</constructor-arg>
</bean>
</property>
+
+ <property name="tokenRevocationLifetimeLookupStrategy">
+ <bean class="net.shibboleth.idp.plugin.oidc.op.profile.logic.DefaultTokenRevocationLifetimeLookupStrategy"
+ p:clockSkew="%{idp.policy.clockSkew:PT5M}" />
+ </property>
</bean>
<!-- ID token actions. -->
diff --git a/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/SetRefreshTokenToResponseContextTest.java b/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/SetRefreshTokenToResponseContextTest.java
index 99362102..4dae7dd6 100644
--- a/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/SetRefreshTokenToResponseContextTest.java
+++ b/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/SetRefreshTokenToResponseContextTest.java
@@ -17,6 +17,7 @@
package net.shibboleth.idp.plugin.oidc.op.profile.impl;
+import net.shibboleth.idp.plugin.oidc.op.profile.logic.DefaultTokenRevocationLifetimeLookupStrategy;
import net.shibboleth.idp.plugin.oidc.op.storage.RevocationCacheContexts;
import net.shibboleth.idp.plugin.oidc.op.token.support.AccessTokenClaimsSet;
import net.shibboleth.idp.plugin.oidc.op.token.support.AuthorizeCodeClaimsSet;
@@ -97,6 +98,11 @@ public class SetRefreshTokenToResponseContextTest extends BaseOIDCResponseAction
if (manipulationStrategy != null) {
action.setTokenClaimsSetManipulationStrategyLookupStrategy(manipulationStrategy);
}
+ final DefaultTokenRevocationLifetimeLookupStrategy revocationLifetimeLookup
+ = new DefaultTokenRevocationLifetimeLookupStrategy();
+ revocationLifetimeLookup.setClockSkew(Duration.ZERO);
+ action.setTokenRevocationLifetimeLookupStrategy(revocationLifetimeLookup);
+
action.setEnforceRefreshTokenRotationCondition(prc -> enforceRotation);
action.initialize();
return action;
diff --git a/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/logic/DefaultTokenRevocationLifetimeLookupStrategyTest.java b/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/logic/DefaultTokenRevocationLifetimeLookupStrategyTest.java
new file mode 100644
index 00000000..a8396c39
--- /dev/null
+++ b/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/logic/DefaultTokenRevocationLifetimeLookupStrategyTest.java
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the University Corporation for Advanced Internet Development,
+ * Inc. (UCAID) under one or more contributor license agreements. See the
+ * NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The UCAID licenses this file to You 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.time.Duration;
+import java.time.Instant;
+import java.util.Date;
+
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+import com.nimbusds.jwt.JWTClaimsSet;
+
+/**
+ * Unit tests for {@link DefaultTokenRevocationLifetimeLookupStrategy}.
+ */
+public class DefaultTokenRevocationLifetimeLookupStrategyTest {
+
+ DefaultTokenRevocationLifetimeLookupStrategy function;
+
+ protected void initFunction(final Duration clockSkew) {
+ function = new DefaultTokenRevocationLifetimeLookupStrategy();
+ function.setClockSkew(clockSkew);
+ }
+
+ protected JWTClaimsSet buildClaimsSet(final Instant exp) {
+ return new JWTClaimsSet.Builder()
+ .issuer("mockIssuer")
+ .expirationTime(exp == null ? null : Date.from(exp))
+ .build();
+ }
+
+ @Test
+ public void testNullReturnsNull() {
+ initFunction(Duration.ZERO);
+ Assert.assertNull(function.apply(null));
+ }
+
+ @Test
+ public void testNullExpReturnsNull() {
+ initFunction(Duration.ZERO);
+ Assert.assertNull(function.apply(buildClaimsSet(null)));
+ }
+
+ @Test
+ public void testExpiredReturnsZero() {
+ initFunction(Duration.ZERO);
+ Assert.assertEquals(function.apply(buildClaimsSet(Instant.now().minusSeconds(5))), Duration.ZERO);
+ }
+
+ @Test
+ public void testExpiredWithClockSkewReturnsZero() {
+ initFunction(Duration.ofSeconds(5));
+ Assert.assertEquals(function.apply(buildClaimsSet(Instant.now().minusSeconds(10))), Duration.ZERO);
+ }
+
+ @Test
+ public void testExpiredButWithinClockSkewReturnsNonZero() {
+ initFunction(Duration.ofSeconds(10));
+ final Duration result = function.apply(buildClaimsSet(Instant.now().minusSeconds(5)));
+ Assert.assertFalse(result.isZero() || result.isNegative());
+ }
+
+ @Test
+ public void testUnExpiredButWithoutClockSkewReturnsNonZero() {
+ initFunction(Duration.ZERO);
+ final Duration result = function.apply(buildClaimsSet(Instant.now().plusSeconds(5)));
+ Assert.assertFalse(result.isZero() || result.isNegative());
+ }
+
+ @Test
+ public void testUnExpiredButWithClockSkewReturnsNonZeroWithClockSkewPlusFiveMins() {
+ final Instant now = Instant.now();
+ initFunction(Duration.ofMinutes(5));
+ final Duration result = function.apply(buildClaimsSet(now.plusSeconds(5)));
+ Assert.assertFalse(result.minus(Duration.ofMinutes(10)).isNegative());
+ }
+
+}
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list