[java-idp-oidc] branch main updated: Repurpose registration token ontime flag as a replacement signal.
Scott Cantor
cantor.2 at osu.edu
Thu Mar 10 17:20:38 UTC 2022
This is an automated email from the git hooks/post-receive script.
scantor 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=a87dc7c8731b523a5d293ea6b1d0981a9229a49e
The following commit(s) were added to refs/heads/main by this push:
new a87dc7c8 Repurpose registration token ontime flag as a replacement signal.
a87dc7c8 is described below
commit a87dc7c8731b523a5d293ea6b1d0981a9229a49e
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Thu Mar 10 12:20:35 2022 -0500
Repurpose registration token ontime flag as a replacement signal.
---
.../oidc/op/profile/impl/GenerateClientID.java | 1 -
.../op/profile/impl/StoreClientInformation.java | 43 +++++++++++-
.../impl/ValidateRegistrationAccessToken.java | 27 ++------
.../oidc/op/profile/flow/RegistrationFlowTest.java | 76 +++++++++++++++++-----
4 files changed, 109 insertions(+), 38 deletions(-)
diff --git a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/GenerateClientID.java b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/GenerateClientID.java
index 081e553f..5fc84d61 100644
--- a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/GenerateClientID.java
+++ b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/GenerateClientID.java
@@ -115,7 +115,6 @@ public class GenerateClientID extends AbstractProfileAction {
"OIDCClientRegistrationResponseContext lookup strategy cannot be null");
}
-
/**
* Set the strategy used to locate the {@link OIDCClientRegistrationTokenClaimsContext} associated with a given
* request.
diff --git a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/StoreClientInformation.java b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/StoreClientInformation.java
index 2076f5b3..70d9f9dc 100644
--- a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/StoreClientInformation.java
+++ b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/StoreClientInformation.java
@@ -37,6 +37,8 @@ import com.nimbusds.openid.connect.sdk.rp.OIDCClientInformation;
import com.nimbusds.openid.connect.sdk.rp.OIDCClientInformationResponse;
import net.shibboleth.idp.plugin.oidc.op.messaging.context.OIDCClientRegistrationResponseContext;
+import net.shibboleth.idp.plugin.oidc.op.messaging.context.OIDCClientRegistrationTokenClaimsContext;
+import net.shibboleth.idp.plugin.oidc.op.profile.context.navigate.DefaultOIDCClientRegistrationTokenClaimsContextLookupFunction;
import net.shibboleth.idp.profile.AbstractProfileAction;
import net.shibboleth.oidc.metadata.ClientInformationManager;
import net.shibboleth.oidc.metadata.ClientInformationManagerException;
@@ -60,6 +62,13 @@ public class StoreClientInformation extends AbstractProfileAction {
/** Strategy to obtain registration validity period policy. */
@Nullable private Function<ProfileRequestContext,Duration> registrationValidityPeriodStrategy;
+ /** Strategy used to locate the {@link OIDCClientRegistrationTokenClaimsContext} associated with the request. */
+ @Nonnull private Function<ProfileRequestContext,OIDCClientRegistrationTokenClaimsContext>
+ registrationTokenContextLookupStrategy;
+
+ /** The OIDCClientRegistrationTokenClaimsContext from which to optionally obtain client ID. */
+ @Nullable private OIDCClientRegistrationTokenClaimsContext registrationTokenCtx;
+
/** The response message. */
@Nullable private OIDCClientInformationResponse response;
@@ -72,6 +81,7 @@ public class StoreClientInformation extends AbstractProfileAction {
/** Constructor. */
public StoreClientInformation() {
oidcResponseContextLookupStrategy = new ChildContextLookup<>(OIDCClientRegistrationResponseContext.class);
+ registrationTokenContextLookupStrategy = new DefaultOIDCClientRegistrationTokenClaimsContextLookupFunction();
}
/**
@@ -119,6 +129,20 @@ public class StoreClientInformation extends AbstractProfileAction {
"OIDCClientRegistrationResponseContext lookup strategy cannot be null");
}
+ /**
+ * Set the strategy used to locate the {@link OIDCClientRegistrationTokenClaimsContext} associated with a given
+ * request.
+ *
+ * @param strategy lookup strategy
+ */
+ public void setRegistrationTokenContextLookupStrategy(
+ @Nonnull final Function<ProfileRequestContext,OIDCClientRegistrationTokenClaimsContext> strategy) {
+ ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+
+ registrationTokenContextLookupStrategy = Constraint.isNotNull(strategy,
+ "OIDCClientRegistrationTokenClaimsContext lookup strategy cannot be null");
+ }
+
/** {@inheritDoc} */
@Override
protected void doInitialize() throws ComponentInitializationException {
@@ -148,6 +172,12 @@ public class StoreClientInformation extends AbstractProfileAction {
ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_MSG_CTX);
return false;
}
+
+ registrationTokenCtx = registrationTokenContextLookupStrategy.apply(profileRequestContext);
+ if (registrationTokenCtx != null && registrationTokenCtx.getClaimsSet() == null) {
+ registrationTokenCtx = null;
+ }
+
response = (OIDCClientInformationResponse) message;
return true;
}
@@ -160,10 +190,19 @@ public class StoreClientInformation extends AbstractProfileAction {
Duration lifetime = registrationValidityPeriodStrategy != null ?
registrationValidityPeriodStrategy.apply(profileRequestContext) : null;
+ final boolean replace;
+ if (registrationTokenCtx != null) {
+ replace = !registrationTokenCtx.getClaimsSet().isOnetime();
+ } else {
+ replace = false;
+ }
+
+ log.debug("{} Storing client information (replace = {})", getLogPrefix(), replace);
+
try {
if (lifetime != null && lifetime.isZero()) {
log.debug("{} Registration won't expire, lifetime set to 0", getLogPrefix());
- clientInformationManager.storeClientInformation(clientInformation, null);
+ clientInformationManager.storeClientInformation(clientInformation, null, replace);
} else {
if (lifetime == null) {
log.debug("{} No registration validity period supplied, using default", getLogPrefix());
@@ -171,7 +210,7 @@ public class StoreClientInformation extends AbstractProfileAction {
}
final Instant expiration = Instant.now().plus(lifetime);
log.debug("{} Registration will expire on {}", getLogPrefix(), expiration);
- clientInformationManager.storeClientInformation(clientInformation, expiration);
+ clientInformationManager.storeClientInformation(clientInformation, expiration, replace);
}
} catch (final ClientInformationManagerException e) {
log.error("{} Could not store the client information", getLogPrefix(), e);
diff --git a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/ValidateRegistrationAccessToken.java b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/ValidateRegistrationAccessToken.java
index 083f908a..e30f802f 100644
--- a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/ValidateRegistrationAccessToken.java
+++ b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/ValidateRegistrationAccessToken.java
@@ -17,7 +17,6 @@
package net.shibboleth.idp.plugin.oidc.op.profile.impl;
-import java.time.Duration;
import java.time.Instant;
import java.util.function.Function;
@@ -196,7 +195,7 @@ public class ValidateRegistrationAccessToken extends AbstractOIDCRequestAction<O
final RegistrationClaimsSet claimsSet;
try {
final String unwrapped = dataSealer.unwrap(accessToken);
- log.debug("{} access token unwrapped into {}", getLogPrefix(), unwrapped);
+ log.debug("{} Access token unwrapped into {}", getLogPrefix(), unwrapped);
claimsSet = objectMapper.readValue(unwrapped, RegistrationClaimsSet.class);
} catch (final DataSealerException | JsonProcessingException e) {
log.error("{} Decoding access token failed: {}", getLogPrefix(), e);
@@ -207,42 +206,30 @@ public class ValidateRegistrationAccessToken extends AbstractOIDCRequestAction<O
log.debug("{} registration access token decoded into {}", getLogPrefix(), claimsSet);
if (Instant.now().isAfter(claimsSet.getExpiration())) {
- log.error("{} registration access token exp is in the past {}", getLogPrefix(), claimsSet.getExpiration());
+ log.error("{} Registration access token exp is in the past {}", getLogPrefix(), claimsSet.getExpiration());
ActionSupport.buildEvent(profileRequestContext, OidcEventIds.INVALID_GRANT);
return;
}
if (revocationCache.isRevoked(RevocationCacheContexts.REGISTRATION_ACCESS_TOKEN, claimsSet.getJti())) {
- log.error("{} registration access token {} has been revoked", getLogPrefix(),
+ log.error("{} Registration access token {} has been revoked", getLogPrefix(),
claimsSet.getJti());
ActionSupport.buildEvent(profileRequestContext, OidcEventIds.INVALID_GRANT);
return;
}
final String relyingPartyId = claimsSet.getRelyingPartyId();
if (relyingPartyId == null) {
- log.error("{} registration access token {} didn't contain relying party identifier", getLogPrefix(),
+ log.error("{} Registration access token {} didn't contain relying party identifier", getLogPrefix(),
claimsSet.getJti());
ActionSupport.buildEvent(profileRequestContext, OidcEventIds.INVALID_GRANT);
return;
}
- log.debug("{} registration access token {} successfully validated", getLogPrefix(), claimsSet.getJti());
- if (claimsSet.isOnetime()) {
- //TODO clockskew
- final boolean revoked = revocationCache.revoke(RevocationCacheContexts.REGISTRATION_ACCESS_TOKEN,
- claimsSet.getJti(), Duration.between(Instant.now(), claimsSet.getExpiration()));
- if (revoked) {
- log.debug("{} Successfully revoked the one-time token with jti {}", getLogPrefix(),
- claimsSet.getJti());
- } else {
- log.error("{} Could not revoke a one-time token with jti {}", getLogPrefix(), claimsSet.getJti());
- ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_PROFILE_CTX);
- return;
- }
- }
+
+ log.debug("{} Registration access token {} successfully validated", getLogPrefix(), claimsSet.getJti());
final OIDCClientRegistrationTokenClaimsContext registrationClaimsContext =
registrationClaimsContextCreationStrategy.apply(profileRequestContext);
if (registrationClaimsContext == null) {
- log.error("{} The registration token claims context could not be created, invalid profile context",
+ log.error("{} Registration token claims context could not be created, invalid profile context",
getLogPrefix());
ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_PROFILE_CTX);
return;
diff --git a/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/flow/RegistrationFlowTest.java b/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/flow/RegistrationFlowTest.java
index 69e5292a..fca9528e 100644
--- a/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/flow/RegistrationFlowTest.java
+++ b/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/flow/RegistrationFlowTest.java
@@ -25,6 +25,8 @@ import java.time.Instant;
import org.opensaml.storage.StorageService;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
+import org.springframework.mock.web.MockHttpServletRequest;
+import org.springframework.mock.web.MockHttpServletResponse;
import org.springframework.webflow.executor.FlowExecutionResult;
import org.testng.Assert;
import org.testng.annotations.AfterMethod;
@@ -34,6 +36,8 @@ import org.testng.annotations.Test;
import com.nimbusds.langtag.LangTag;
import com.nimbusds.langtag.LangTagException;
import com.nimbusds.oauth2.sdk.GrantType;
+import com.nimbusds.oauth2.sdk.OAuth2Error;
+import com.nimbusds.oauth2.sdk.client.RegistrationError;
import com.nimbusds.oauth2.sdk.token.BearerAccessToken;
import com.nimbusds.openid.connect.sdk.rp.OIDCClientInformation;
import com.nimbusds.openid.connect.sdk.rp.OIDCClientInformationResponse;
@@ -120,42 +124,82 @@ public class RegistrationFlowTest extends AbstractOidcFlowTest {
@Test
public void testAccessToken_nonCompliantWithProfilePolicy1() throws Exception {
setJsonRequest("POST", "{ \"redirect_uris\":[\"" + redirectUri + "\"] }");
- request.addHeader("Authorization", buildRegistrationAccessToken("[\"https://invalid.domain.org/cb\"]")
+ request.addHeader("Authorization", buildRegistrationAccessToken(false, "[\"https://invalid.domain.org/cb\"]")
.toAuthorizationHeader());
final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
- assertErrorCode(result, "invalid_client_metadata");
+ assertErrorCode(result, RegistrationError.INVALID_CLIENT_METADATA.getCode());
}
@Test
public void testAccessToken_nonCompliantWithProfilePolicy2() throws Exception {
setJsonRequest("POST", "{ \"redirect_uris\":[\"" + redirectUri + "\"], \"id_token_signed_response_alg\":\"HS256\" }");
- request.addHeader("Authorization", buildRegistrationAccessToken("[\"https://example.org/cb\"]")
+ request.addHeader("Authorization", buildRegistrationAccessToken(false, "[\"https://example.org/cb\"]")
.toAuthorizationHeader());
final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
- assertErrorCode(result, "invalid_client_metadata");
+ assertErrorCode(result, RegistrationError.INVALID_CLIENT_METADATA.getCode());
}
@Test
public void testAccessToken_success() throws Exception {
setJsonRequest("POST", buildRequestMessage(redirectUri));
- request.addHeader("Authorization", buildRegistrationAccessToken("[\"https://example.org/cb\"]")
+ request.addHeader("Authorization", buildRegistrationAccessToken(false, "[\"https://example.org/cb\"]")
.toAuthorizationHeader());
assertSuccessfulResponse(flowExecutor.launchExecution(FLOW_ID, null, externalContext), null);
}
@Test
public void testAccessToken_success_withClientID() throws Exception {
- setJsonRequest("POST", buildRequestMessage(redirectUri));
clientId = "https://example.org";
- request.addHeader("Authorization", buildRegistrationAccessToken("[\"https://example.org/cb\"]")
+
+ setJsonRequest("POST", buildRequestMessage(redirectUri));
+ request.addHeader("Authorization", buildRegistrationAccessToken(false, "[\"https://example.org/cb\"]")
.toAuthorizationHeader());
assertSuccessfulResponse(flowExecutor.launchExecution(FLOW_ID, null, externalContext), clientId);
}
+
+ @Test
+ public void testAccessToken_successReplace() throws Exception {
+ clientId = "https://example.org";
+
+ setJsonRequest("POST", buildRequestMessage(redirectUri));
+ request.addHeader("Authorization", buildRegistrationAccessToken(false, "[\"https://example.org/cb\"]")
+ .toAuthorizationHeader());
+ assertSuccessfulResponse(flowExecutor.launchExecution(FLOW_ID, null, externalContext), null);
+
+ // Repeat should work.
+ initializeMocks();
+ initializeThreadLocals();
+
+ setJsonRequest("POST", buildRequestMessage(redirectUri));
+ request.addHeader("Authorization", buildRegistrationAccessToken(false, "[\"https://example.org/cb\"]")
+ .toAuthorizationHeader());
+ assertSuccessfulResponse(flowExecutor.launchExecution(FLOW_ID, null, externalContext), null);
+ }
+
+ @Test
+ public void testAccessToken_NoReplace() throws Exception {
+ clientId = "https://example.org";
+
+ setJsonRequest("POST", buildRequestMessage(redirectUri));
+ request.addHeader("Authorization", buildRegistrationAccessToken(false, "[\"https://example.org/cb\"]")
+ .toAuthorizationHeader());
+ assertSuccessfulResponse(flowExecutor.launchExecution(FLOW_ID, null, externalContext), null);
+
+ // Repeat should not work.
+ initializeMocks();
+ initializeThreadLocals();
+
+ setJsonRequest("POST", buildRequestMessage(redirectUri));
+ request.addHeader("Authorization", buildRegistrationAccessToken(true, "[\"https://example.org/cb\"]")
+ .toAuthorizationHeader());
+ final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+ assertErrorCode(result, OAuth2Error.SERVER_ERROR_CODE);
+ }
@Test
public void testAccessToken_successCustomClaimIgnored() throws Exception {
setJsonRequest("POST", buildRequestMessage(redirectUri, "\"customClaim\":\"customValue\""));
- request.addHeader("Authorization", buildRegistrationAccessToken("[\"https://example.org/cb\"]")
+ request.addHeader("Authorization", buildRegistrationAccessToken(false, "[\"https://example.org/cb\"]")
.toAuthorizationHeader());
final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
assertSuccessfulResponse(result, null);
@@ -167,7 +211,7 @@ public class RegistrationFlowTest extends AbstractOidcFlowTest {
@Test
public void testAccessToken_successCustomClaimInPolicyAdded() throws Exception {
setJsonRequest("POST", buildRequestMessage(redirectUri, "\"customClaim\":\"customValue\""));
- request.addHeader("Authorization", buildRegistrationAccessToken("[\"https://example.org/cb\"]", new String[] {"customClaim"})
+ request.addHeader("Authorization", buildRegistrationAccessToken(false, "[\"https://example.org/cb\"]", new String[] {"customClaim"})
.toAuthorizationHeader());
final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
assertSuccessfulResponse(result, null);
@@ -180,10 +224,10 @@ public class RegistrationFlowTest extends AbstractOidcFlowTest {
public void testAccessToken_noPolicyNoRedirectUri() throws Exception {
setJsonRequest("POST", "{ \"test\":false }");
rpId = "mockDynRegClientNoProfilePolicy";
- request.addHeader("Authorization", buildRegistrationAccessToken((String) null,
+ request.addHeader("Authorization", buildRegistrationAccessToken(false, (String) null,
(String[]) null).toAuthorizationHeader());
final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
- assertErrorCode(result, "invalid_redirect_uri");
+ assertErrorCode(result, RegistrationError.INVALID_REDIRECT_URI.getCode());
}
@Test
@@ -191,7 +235,7 @@ public class RegistrationFlowTest extends AbstractOidcFlowTest {
// the policy for mockDynRegClientAnotherProfilePolicy shouldn't accept other grant_types than implicit
setJsonRequest("POST", buildRequestMessage(redirectUri, "\"grant_types\":[\"authorization_code\"]"));
rpId = "mockDynRegClientAnotherProfilePolicy";
- request.addHeader("Authorization", buildRegistrationAccessToken((String) null,
+ request.addHeader("Authorization", buildRegistrationAccessToken(false, (String) null,
(String[]) null).toAuthorizationHeader());
final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
assertErrorCode(result, "invalid_client_metadata");
@@ -201,7 +245,7 @@ public class RegistrationFlowTest extends AbstractOidcFlowTest {
public void testAccessToken_nonDefaultPolicyActive_successWithCompatibleRequest() throws Exception {
setJsonRequest("POST", buildRequestMessage(redirectUri, "\"grant_types\":[\"implicit\"]"));
rpId = "mockDynRegClientAnotherProfilePolicy";
- request.addHeader("Authorization", buildRegistrationAccessToken((String) null,
+ request.addHeader("Authorization", buildRegistrationAccessToken(false, (String) null,
(String[]) null).toAuthorizationHeader());
final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
assertSuccessfulResponse(result, null);
@@ -241,7 +285,8 @@ public class RegistrationFlowTest extends AbstractOidcFlowTest {
Assert.assertEquals(storedMetadata.getPolicyURIEntries(), metadata.getPolicyURIEntries());
}
- protected BearerAccessToken buildRegistrationAccessToken(final String redirectUriSubset, final String... additionalPolicyClaims) throws Exception {
+ protected BearerAccessToken buildRegistrationAccessToken(final boolean onetime, final String redirectUriSubset,
+ final String... additionalPolicyClaims) throws Exception {
final StringBuilder metadata = new StringBuilder();
if (additionalPolicyClaims != null) {
for (int i = 0; i < additionalPolicyClaims.length; i++) {
@@ -265,7 +310,8 @@ public class RegistrationFlowTest extends AbstractOidcFlowTest {
"\"iat\":" + Instant.now().getEpochSecond() + "," +
"\"jti\":\"" + idGenerator.generateIdentifier() + "\"," +
"\"rp_id\":\"" + rpId + "\"," +
- (clientId != null ? ("\"client_id\":\"" + clientId + "\",") : "") +
+ (clientId != null ? ("\"client_id\":\"" + clientId + "\",") : "") +
+ "\"onetime\":" + Boolean.toString(onetime) + "," +
"\"metadata\":" + (metadata.length() == 0 ? "null" : "{" + metadata.toString()) + "}" +
"}";
return new BearerAccessToken(BaseOIDCResponseActionTest.initializeDataSealer().wrap(json,
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list