[java-idp-oidc] branch main updated: JOIDC-191 - Harmonise the use of identifier generation strategies

Henri Mikkonen henri.mikkonen at iki.fi
Thu Mar 21 13:01:02 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=09402fe984adc18eb9d46b7b485cfc2c324d4f70

The following commit(s) were added to refs/heads/main by this push:
     new 09402fe9 JOIDC-191 - Harmonise the use of identifier generation strategies
09402fe9 is described below

commit 09402fe984adc18eb9d46b7b485cfc2c324d4f70
Author: Henri Mikkonen <henri.mikkonen at iki.fi>
AuthorDate: Thu Mar 21 15:00:36 2024 +0200

    JOIDC-191 - Harmonise the use of identifier generation strategies
    
    https://shibboleth.atlassian.net/browse/JOIDC-191
    
    Wired new property 'idp.oidc.xmlSafeIdentifiers' to control whether to generate
    XML ID safe identifiers. Defaults to true for compatibility.
    
    In general, the bean shibboleth.oidc.DefaultIdentifierGenerationStrategy is used
    for generating identifiers. That bean exploits security configuration.
    
    In dynamic client registration, custom beans can be wired for client ID and secret
    generation:
    - shibboleth.oidc.dynreg.ClientIDGenerationStrategy
    - shibboleth.oidc.dynreg.ClientSecretGenerationStrategy
---
 .../op/token/support/AccessTokenClaimsSet.java     |  3 --
 .../op/token/support/AuthorizeCodeClaimsSet.java   |  5 --
 .../oidc/op/token/support/TokenClaimsSet.java      | 16 ++++++
 .../admin/impl/IssueRegistrationAccessToken.java   | 21 ++++++--
 .../impl/PrepareBackChannelLogoutRequest.java      | 16 +++++-
 .../op/oauth2/profile/impl/BuildAccessToken.java   | 19 ++++++-
 .../SetAuthorizationCodeToResponseContext.java     | 17 +++++-
 .../oidc/op/profile/impl/GenerateClientID.java     | 60 ++++++++++++++++++----
 .../oidc/op/profile/impl/GenerateClientSecret.java | 19 ++++++-
 .../impl/SetRefreshTokenToResponseContext.java     | 17 +++++-
 .../META-INF/net.shibboleth.idp/postconfig.xml     |  5 ++
 .../issue-registration-access-token-beans.xml      |  4 +-
 .../oidc/oidc-logout-propagation-beans.xml         |  4 +-
 .../flows/oidc/abstract/oidc-abstract-beans.xml    |  2 +-
 .../idp/flows/oidc/authorize/authorize-beans.xml   | 18 ++++---
 .../idp/flows/oidc/register/register-beans.xml     |  8 ++-
 .../idp/flows/oidc/token/token-beans.xml           | 17 +++---
 .../idp/plugin/oidc/op/conf/oidc.properties        |  3 ++
 18 files changed, 205 insertions(+), 49 deletions(-)

diff --git a/idp-oidc-extension-api/src/main/java/net/shibboleth/idp/plugin/oidc/op/token/support/AccessTokenClaimsSet.java b/idp-oidc-extension-api/src/main/java/net/shibboleth/idp/plugin/oidc/op/token/support/AccessTokenClaimsSet.java
index e6a1f8b5..a23bd3cd 100644
--- a/idp-oidc-extension-api/src/main/java/net/shibboleth/idp/plugin/oidc/op/token/support/AccessTokenClaimsSet.java
+++ b/idp-oidc-extension-api/src/main/java/net/shibboleth/idp/plugin/oidc/op/token/support/AccessTokenClaimsSet.java
@@ -20,16 +20,13 @@ import javax.annotation.Nullable;
 import com.nimbusds.jwt.JWT;
 import com.nimbusds.jwt.JWTClaimsSet;
 import com.nimbusds.oauth2.sdk.Scope;
-import com.nimbusds.oauth2.sdk.id.ClientID;
 import com.nimbusds.openid.connect.sdk.claims.ACR;
 import com.nimbusds.openid.connect.sdk.claims.ClaimsSet;
 
 import net.shibboleth.shared.annotation.constraint.NotEmpty;
 import net.shibboleth.shared.security.DataSealer;
 import net.shibboleth.shared.security.DataSealerException;
-import net.shibboleth.shared.security.IdentifierGenerationStrategy;
 
-import java.net.URI;
 import java.text.ParseException;
 import java.time.Instant;
 import java.util.Map;
diff --git a/idp-oidc-extension-api/src/main/java/net/shibboleth/idp/plugin/oidc/op/token/support/AuthorizeCodeClaimsSet.java b/idp-oidc-extension-api/src/main/java/net/shibboleth/idp/plugin/oidc/op/token/support/AuthorizeCodeClaimsSet.java
index 060d9d4e..a205c8c4 100644
--- a/idp-oidc-extension-api/src/main/java/net/shibboleth/idp/plugin/oidc/op/token/support/AuthorizeCodeClaimsSet.java
+++ b/idp-oidc-extension-api/src/main/java/net/shibboleth/idp/plugin/oidc/op/token/support/AuthorizeCodeClaimsSet.java
@@ -17,17 +17,12 @@ package net.shibboleth.idp.plugin.oidc.op.token.support;
 import javax.annotation.Nonnull;
 
 import com.nimbusds.jwt.JWTClaimsSet;
-import com.nimbusds.oauth2.sdk.Scope;
-import com.nimbusds.oauth2.sdk.id.ClientID;
 
 import net.shibboleth.shared.annotation.constraint.NotEmpty;
 import net.shibboleth.shared.security.DataSealer;
 import net.shibboleth.shared.security.DataSealerException;
-import net.shibboleth.shared.security.IdentifierGenerationStrategy;
 
-import java.net.URI;
 import java.text.ParseException;
-import java.time.Instant;
 
 /** Class wrapping claims set for authorize code. */
 public final class AuthorizeCodeClaimsSet extends TokenClaimsSet {
diff --git a/idp-oidc-extension-api/src/main/java/net/shibboleth/idp/plugin/oidc/op/token/support/TokenClaimsSet.java b/idp-oidc-extension-api/src/main/java/net/shibboleth/idp/plugin/oidc/op/token/support/TokenClaimsSet.java
index 2b3f7f8c..2f13339c 100644
--- a/idp-oidc-extension-api/src/main/java/net/shibboleth/idp/plugin/oidc/op/token/support/TokenClaimsSet.java
+++ b/idp-oidc-extension-api/src/main/java/net/shibboleth/idp/plugin/oidc/op/token/support/TokenClaimsSet.java
@@ -799,6 +799,22 @@ public class TokenClaimsSet {
             return this;
         }
 
+        /**
+         * Set JWT ID via generator.
+         * 
+         * @param generator ID generator
+         * @param xmlSafe true iff the result must be XML ID safe
+         * 
+         * @return the builder
+         * 
+         * @since 4.1.0
+         */
+        public Builder<T> setJWTID(@Nonnull final IdentifierGenerationStrategy generator, final boolean xmlSafe) {
+            jwtid = Constraint.isNotNull(generator, "IdentifierGenerationStrategy cannot be null")
+                    .generateIdentifier(xmlSafe);
+            return this;
+        }
+
         /**
          * Set JWT ID.
          * 
diff --git a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/admin/impl/IssueRegistrationAccessToken.java b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/admin/impl/IssueRegistrationAccessToken.java
index 486f6094..abe90401 100644
--- a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/admin/impl/IssueRegistrationAccessToken.java
+++ b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/admin/impl/IssueRegistrationAccessToken.java
@@ -121,7 +121,7 @@ public class IssueRegistrationAccessToken extends AbstractAdminApiProfileAction
 
     /** The resolved metadata policy. */
     @Nullable private Map<String,MetadataPolicy> metadataPolicy;
-    
+
     /** The token issuer. */
     @Nonnull private String issuer;
     
@@ -139,7 +139,10 @@ public class IssueRegistrationAccessToken extends AbstractAdminApiProfileAction
 
     /** The token lifetime. */
     @Nullable private Duration tokenLifetime;
-    
+
+    /** The xmlSafe-flag passed to the identifier generator. */
+    private boolean xmlSafeIdentifier;
+
     /**
      * Constructor.
      */
@@ -158,6 +161,7 @@ public class IssueRegistrationAccessToken extends AbstractAdminApiProfileAction
                 new SpringFlowScopeLookupFunction(IssueRegistrationAccessTokenArguments.URL_PARAM_REPLACEMENT);
         
         defaultTokenLifetime = Duration.ofDays(1);
+        xmlSafeIdentifier = true;
     }
     
     /**
@@ -318,6 +322,17 @@ public class IssueRegistrationAccessToken extends AbstractAdminApiProfileAction
         defaultTokenLifetime = Constraint.isNotNull(lifetime, "Default token lifetime cannot be null");
     }
 
+    /**
+     * Set the xmlSafe-flag passed to the identifier generator
+     * 
+     * @param flag xmlSafe-flag
+     */
+    public void setXmlSafeIdentifier(final boolean flag) {
+        checkSetterPreconditions();
+        
+        xmlSafeIdentifier = flag;
+    }
+
     /** {@inheritDoc} */
     @Override
     protected void doInitialize() throws ComponentInitializationException {
@@ -417,7 +432,7 @@ public class IssueRegistrationAccessToken extends AbstractAdminApiProfileAction
             return;
         }
         
-        final String id = idGenerator.generateIdentifier();
+        final String id = idGenerator.generateIdentifier(xmlSafeIdentifier);
         
         final Instant now = Instant.now();
         final Instant exp = now.plus(tokenLifetime);
diff --git a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/logout/profile/impl/PrepareBackChannelLogoutRequest.java b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/logout/profile/impl/PrepareBackChannelLogoutRequest.java
index 4fe46c9d..843d3aa4 100644
--- a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/logout/profile/impl/PrepareBackChannelLogoutRequest.java
+++ b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/logout/profile/impl/PrepareBackChannelLogoutRequest.java
@@ -57,11 +57,15 @@ public class PrepareBackChannelLogoutRequest extends AbstractOIDCBackChannelLogo
     /** The generator to use. */
     @Nullable private IdentifierGenerationStrategy idGenerator;
 
+    /** The xmlSafe-flag passed to the identifier generator. */
+    private boolean xmlSafeIdentifier;
+
     /**
      * Constructor.
      */
     public PrepareBackChannelLogoutRequest() {
         idGeneratorLookupStrategy = FunctionSupport.constant(new SecureRandomIdentifierGenerationStrategy());
+        xmlSafeIdentifier = true;
     }
 
     /**
@@ -76,6 +80,16 @@ public class PrepareBackChannelLogoutRequest extends AbstractOIDCBackChannelLogo
                 Constraint.isNotNull(strategy, "Identifier generation strategy cannot be null");
     }
 
+    /**
+     * Set the xmlSafe-flag passed to the identifier generator
+     * 
+     * @param flag xmlSafe-flag
+     */
+    public void setXmlSafeIdentifier(final boolean flag) {
+        checkSetterPreconditions();
+        xmlSafeIdentifier = flag;
+    }
+
     /** {@inheritDoc} */
     @Override
     protected boolean doPreExecute(@Nonnull ProfileRequestContext profileRequestContext) {
@@ -100,7 +114,7 @@ public class PrepareBackChannelLogoutRequest extends AbstractOIDCBackChannelLogo
         final Subject sub = new Subject(getOidcRPSession().getSubject());
         final List<Audience> aud = Collections.singletonList(new Audience(getOidcRPSession().getId()));
         final Date iat = Calendar.getInstance().getTime();
-        final JWTID jti = new JWTID(idGenerator.generateIdentifier());
+        final JWTID jti = new JWTID(idGenerator.generateIdentifier(xmlSafeIdentifier));
         final SessionID sid = new SessionID(getOidcRPSession().getSessionIdentifier());
 
         final LogoutTokenClaimsSet logoutTokenClaimsSet = new LogoutTokenClaimsSet(iss, sub, aud, iat, jti, sid);
diff --git a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/BuildAccessToken.java b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/BuildAccessToken.java
index a63530dd..bb304a64 100644
--- a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/BuildAccessToken.java
+++ b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/BuildAccessToken.java
@@ -157,6 +157,9 @@ public class BuildAccessToken extends AbstractOIDCResponseAction {
     /** Access token context. */
     @Nullable private AccessTokenContext accessTokenCtx;
 
+    /** The xmlSafe-flag passed to the identifier generator. */
+    private boolean xmlSafeIdentifier;
+
     /** Constructor. */
     public BuildAccessToken() {
         accessTokenTypeLookupStrategy = new AccessTokenTypeLookupFunction();
@@ -183,6 +186,7 @@ public class BuildAccessToken extends AbstractOIDCResponseAction {
                         new OutboundMessageContextLookup()));
         tokenClaimsSetManipulationStrategyLookupStrategy =
                 new AccessTokenClaimsSetManipulationStrategyLookupFunction();
+        xmlSafeIdentifier = true;
     }
     
     /**
@@ -320,6 +324,17 @@ public class BuildAccessToken extends AbstractOIDCResponseAction {
                 Constraint.isNotNull(strategy, "Manipulation strategy lookup strategy cannot be null");
     }
 
+    /**
+     * Set the xmlSafe-flag passed to the identifier generator
+     * 
+     * @param flag xmlSafe-flag
+     */
+    public void setXmlSafeIdentifier(final boolean flag) {
+        checkSetterPreconditions();
+
+        xmlSafeIdentifier = flag;
+    }
+
     /** {@inheritDoc} */
     @Override
     protected void doInitialize() throws ComponentInitializationException {
@@ -453,7 +468,7 @@ public class BuildAccessToken extends AbstractOIDCResponseAction {
                     dateExp);
             // Add additional bits.
             builder.setAudience(responseCtx.getAudience());
-            builder.setJWTID(idGenerator);
+            builder.setJWTID(idGenerator, xmlSafeIdentifier);
             builder.setSessionIdentifier(responseCtx.getSessionId());
             // Set root token identifier to contain jit from the claims set used for building the new token
             if (StringSupport.trimOrNull(tokenClaimsSet.getRootTokenIdentifier()) == null) {
@@ -465,7 +480,7 @@ public class BuildAccessToken extends AbstractOIDCResponseAction {
             final JSONArray consented = consentCtx != null ? consentCtx.getConsentedAttributes() : null;
             
             builder = (Builder) new AccessTokenClaimsSet.Builder()
-                    .setJWTID(idGenerator)
+                    .setJWTID(idGenerator, xmlSafeIdentifier)
                     .setClientID(clientID)
                     .setIssuer(issuer)
                     .setPrincipal(subjectCtx.getPrincipalName())
diff --git a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/SetAuthorizationCodeToResponseContext.java b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/SetAuthorizationCodeToResponseContext.java
index e52c1d01..8559122d 100644
--- a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/SetAuthorizationCodeToResponseContext.java
+++ b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/SetAuthorizationCodeToResponseContext.java
@@ -120,6 +120,9 @@ public class SetAuthorizationCodeToResponseContext extends AbstractOAuthAuthoriz
     /** Code challenge and the code challenge method stored to authz code.*/
     @Nullable private String codeChallenge;
 
+    /** The xmlSafe-flag passed to the identifier generator. */
+    private boolean xmlSafeIdentifier;
+
     /**
      * Constructor.
      */
@@ -138,6 +141,7 @@ public class SetAuthorizationCodeToResponseContext extends AbstractOAuthAuthoriz
         idGeneratorLookupStrategy = FunctionSupport.constant(new SecureRandomIdentifierGenerationStrategy());
         tokenClaimsSetManipulationStrategyLookupStrategy =
                 new AuthorizationCodeClaimsSetManipulationStrategyLookupFunction();
+        xmlSafeIdentifier = true;
     }
 
     /**
@@ -259,6 +263,17 @@ public class SetAuthorizationCodeToResponseContext extends AbstractOAuthAuthoriz
                 Constraint.isNotNull(strategy, "Manipulation strategy lookup strategy cannot be null");
     }
 
+    /**
+     * Set the xmlSafe-flag passed to the identifier generator
+     * 
+     * @param flag xmlSafe-flag
+     */
+    public void setXmlSafeIdentifier(final boolean flag) {
+        checkSetterPreconditions();
+        
+        xmlSafeIdentifier = flag;
+    }
+
     /** {@inheritDoc} */
     @Override
     protected void doInitialize() throws ComponentInitializationException {
@@ -336,7 +351,7 @@ public class SetAuthorizationCodeToResponseContext extends AbstractOAuthAuthoriz
         final Instant dateExp = Instant.now().plus(authzCodeLifetime);
         final Scope scope = responseCtx.getScope();
         final AuthorizeCodeClaimsSet claimsSet = new AuthorizeCodeClaimsSet.Builder()
-                .setJWTID(idGenerator)
+                .setJWTID(idGenerator, xmlSafeIdentifier)
                 .setClientID(getAuthorizationRequest().getClientID())
                 .setIssuer(issuerLookupStrategy.apply(profileRequestContext))
                 .setPrincipal(subjectCtx.getPrincipalName())
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 cff47052..d84a7a1c 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
@@ -36,7 +36,9 @@ import net.shibboleth.idp.profile.IdPEventIds;
 import net.shibboleth.profile.config.ProfileConfiguration;
 import net.shibboleth.profile.context.RelyingPartyContext;
 import net.shibboleth.shared.logic.Constraint;
+import net.shibboleth.shared.logic.FunctionSupport;
 import net.shibboleth.shared.security.IdentifierGenerationStrategy;
+import net.shibboleth.shared.security.impl.SecureRandomIdentifierGenerationStrategy;
 
 /**
  * Creates the client ID for the registration.
@@ -62,7 +64,10 @@ public class GenerateClientID extends AbstractProfileAction {
     /** Strategy used to locate the {@link OIDCClientRegistrationTokenClaimsContext} associated with the request. */
     @Nonnull private Function<ProfileRequestContext,OIDCClientRegistrationTokenClaimsContext>
         registrationTokenContextLookupStrategy;
-    
+
+    /** Strategy used to locate the {@link IdentifierGenerationStrategy} to use. */
+    @Nonnull private Function<ProfileRequestContext, IdentifierGenerationStrategy> idGeneratorLookupStrategy;
+
     /** The RelyingPartyContext to operate on. */
     @Nullable private RelyingPartyContext rpCtx;
 
@@ -71,7 +76,13 @@ public class GenerateClientID extends AbstractProfileAction {
 
     /** The OIDCClientRegistrationTokenClaimsContext from which to optionally obtain client ID. */
     @Nullable private OIDCClientRegistrationTokenClaimsContext registrationTokenCtx;
-    
+
+    /** The client ID generator to use. */
+    @Nullable private IdentifierGenerationStrategy idGenerator;
+
+    /** The xmlSafe-flag passed to the identifier generator. */
+    private boolean xmlSafeIdentifier;
+
     /** Constructor. */
     public GenerateClientID() {
         relyingPartyContextLookupStrategy = new ChildContextLookup<>(RelyingPartyContext.class);
@@ -79,6 +90,8 @@ public class GenerateClientID extends AbstractProfileAction {
                 OIDCClientRegistrationResponseContext.class).compose(
                         new OutboundMessageContextLookup());
         registrationTokenContextLookupStrategy = new DefaultOIDCClientRegistrationTokenClaimsContextLookupFunction();
+        idGeneratorLookupStrategy = FunctionSupport.constant(new SecureRandomIdentifierGenerationStrategy());
+        xmlSafeIdentifier = true;
     }
 
     /**
@@ -90,7 +103,7 @@ public class GenerateClientID extends AbstractProfileAction {
      */
     public void setRelyingPartyContextLookupStrategy(
             @Nonnull final Function<ProfileRequestContext,RelyingPartyContext> strategy) {
-        ifInitializedThrowUnmodifiabledComponentException();
+        checkSetterPreconditions();
         
         relyingPartyContextLookupStrategy = Constraint.isNotNull(strategy,
                 "RelyingPartyContext lookup strategy cannot be null");
@@ -105,7 +118,7 @@ public class GenerateClientID extends AbstractProfileAction {
      */
     public void setOidcResponseContextLookupStrategy(
             @Nonnull final Function<ProfileRequestContext,OIDCClientRegistrationResponseContext> strategy) {
-        ifInitializedThrowUnmodifiabledComponentException();
+        checkSetterPreconditions();
         
         oidcResponseContextLookupStrategy = Constraint.isNotNull(strategy,
                 "OIDCClientRegistrationResponseContext lookup strategy cannot be null");
@@ -119,12 +132,36 @@ public class GenerateClientID extends AbstractProfileAction {
      */
     public void setRegistrationTokenContextLookupStrategy(
             @Nonnull final Function<ProfileRequestContext,OIDCClientRegistrationTokenClaimsContext> strategy) {
-        ifInitializedThrowUnmodifiabledComponentException();
+        checkSetterPreconditions();
         
         registrationTokenContextLookupStrategy = Constraint.isNotNull(strategy,
                 "OIDCClientRegistrationTokenClaimsContext lookup strategy cannot be null");
     }
 
+    /**
+     * Set the strategy used to locate the {@link IdentifierGenerationStrategy} to use.
+     * 
+     * @param strategy What to set.
+     */
+    public void setIdentifierGeneratorLookupStrategy(
+            @Nonnull final Function<ProfileRequestContext, IdentifierGenerationStrategy> strategy) {
+        checkSetterPreconditions();
+
+        idGeneratorLookupStrategy =
+                Constraint.isNotNull(strategy, "IdentifierGenerationStrategy lookup strategy cannot be null");
+    }
+
+    /**
+     * Set the xmlSafe-flag passed to the identifier generator
+     * 
+     * @param flag xmlSafe-flag
+     */
+    public void setXmlSafeIdentifier(final boolean flag) {
+        checkSetterPreconditions();
+
+        xmlSafeIdentifier = flag;
+    }
+
     /** {@inheritDoc} */
     @Override
     protected boolean doPreExecute(@Nonnull final ProfileRequestContext profileRequestContext) {
@@ -165,16 +202,19 @@ public class GenerateClientID extends AbstractProfileAction {
             registrationTokenCtx = null;
         }
 
+        idGenerator = idGeneratorLookupStrategy.apply(profileRequestContext);
+        if (idGenerator == null) {
+            log.debug("{} No identifier generation strategy", getLogPrefix());
+            ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_PROFILE_CTX);
+            return false;
+        }
+
         return true;
     }
 
     /** {@inheritDoc} */
     @Override
     protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext) {
-        final ProfileConfiguration profileConfig = rpCtx.getProfileConfig();
-        final IdentifierGenerationStrategy idGenerator =
-                profileConfig.getSecurityConfiguration(profileRequestContext).getIdGenerator();
-        
         String clientId = null;
         if (registrationTokenCtx != null) {
             clientId = registrationTokenCtx.getClaimsSet().getClientId();
@@ -183,7 +223,7 @@ public class GenerateClientID extends AbstractProfileAction {
         if (clientId != null) {
             log.debug("{} Using client_id supplied by access token: {}", getLogPrefix(), clientId);
         } else {
-            clientId = idGenerator.generateIdentifier();
+            clientId = idGenerator.generateIdentifier(xmlSafeIdentifier);
             log.debug("{} Created a new client ID: {}", getLogPrefix(), clientId);
         }
         
diff --git a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/GenerateClientSecret.java b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/GenerateClientSecret.java
index 1e40d4df..434f9476 100644
--- a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/GenerateClientSecret.java
+++ b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/GenerateClientSecret.java
@@ -64,10 +64,14 @@ public class GenerateClientSecret extends AbstractProfileAction {
     /** Strategy to obtain client secret validity period policy. */
     @Nullable private Function<ProfileRequestContext,Duration> secretExpirationPeriodStrategy;
 
+    /** The xmlSafe-flag passed to the identifier generator. */
+    private boolean xmlSafeIdentifier;
+
     /** Constructor. */
     public GenerateClientSecret() {
         oidcResponseContextLookupStrategy = new ChildContextLookup<>(OIDCClientRegistrationResponseContext.class);
         idGeneratorLookupStrategy = FunctionSupport.constant(new SecureRandomIdentifierGenerationStrategy());
+        xmlSafeIdentifier = true;
     }
     
     /**
@@ -102,12 +106,23 @@ public class GenerateClientSecret extends AbstractProfileAction {
      */
     public void setIdentifierGeneratorLookupStrategy(
             @Nonnull final Function<ProfileRequestContext, IdentifierGenerationStrategy> strategy) {
-        ifInitializedThrowUnmodifiabledComponentException();
+        checkSetterPreconditions();
 
         idGeneratorLookupStrategy =
                 Constraint.isNotNull(strategy, "IdentifierGenerationStrategy lookup strategy cannot be null");
     }
 
+    /**
+     * Set the xmlSafe-flag passed to the identifier generator
+     * 
+     * @param flag xmlSafe-flag
+     */
+    public void setXmlSafeIdentifier(final boolean flag) {
+        checkSetterPreconditions();
+
+        xmlSafeIdentifier = flag;
+    }
+
     /** {@inheritDoc} */
     @Override
     protected boolean doPreExecute(@Nonnull final ProfileRequestContext profileRequestContext) {
@@ -152,7 +167,7 @@ public class GenerateClientSecret extends AbstractProfileAction {
         final Instant now = Instant.now();
         final Instant expiration = now.plus(lifetime);
 
-        final String clientSecret = idGenerator.generateIdentifier();
+        final String clientSecret = idGenerator.generateIdentifier(xmlSafeIdentifier);
         oidcResponseCtx.setClientSecret(clientSecret);
         if (expiration.isAfter(now)) {
             oidcResponseCtx.setClientSecretExpiresAt(expiration);
diff --git a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/SetRefreshTokenToResponseContext.java b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/SetRefreshTokenToResponseContext.java
index e3cf0379..4299089e 100644
--- a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/SetRefreshTokenToResponseContext.java
+++ b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/profile/impl/SetRefreshTokenToResponseContext.java
@@ -121,6 +121,9 @@ public class SetRefreshTokenToResponseContext extends AbstractOIDCResponseAction
     /** Refresh Token type. */
     @Nullable private String refreshTokenType;
 
+    /** The xmlSafe-flag passed to the identifier generator. */
+    private boolean xmlSafeIdentifier;
+
     /**
      * Constructor.
      * 
@@ -137,6 +140,7 @@ public class SetRefreshTokenToResponseContext extends AbstractOIDCResponseAction
         tokenRevocationLifetimeLookupStrategy = new DefaultTokenRevocationLifetimeLookupStrategy();
         refreshTokenTypeLookupStrategy = new RefreshTokenTypeLookupFunction();
         refreshTokenSerializationStrategies = CollectionSupport.emptyMap();
+        xmlSafeIdentifier = true;
     }
 
     /**
@@ -249,6 +253,17 @@ public class SetRefreshTokenToResponseContext extends AbstractOIDCResponseAction
         refreshTokenSerializationStrategies = Constraint.isNotNull(strategies, "Strategies cannot be null");
     }
 
+    /**
+     * Set the xmlSafe-flag passed to the identifier generator
+     * 
+     * @param flag xmlSafe-flag
+     */
+    public void setXmlSafeIdentifier(final boolean flag) {
+        checkSetterPreconditions();
+        
+        xmlSafeIdentifier = flag;
+    }
+
     /** {@inheritDoc} */
     @Override
     protected void doInitialize() throws ComponentInitializationException {
@@ -316,7 +331,7 @@ public class SetRefreshTokenToResponseContext extends AbstractOIDCResponseAction
         final RefreshTokenClaimsSet claimsSet =
                 new RefreshTokenClaimsSet.Builder(tokenClaimsSet, Instant.now(),
                         chainExp.isBefore(tokenExp) ? chainExp : tokenExp, chainExp)
-                .setJWTID(idGenerator)
+                .setJWTID(idGenerator, xmlSafeIdentifier)
                 .setRootTokenIdentifier(rootTokenId)
                 .build();
         
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 8db4a7bd..29a16fb8 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
@@ -782,6 +782,11 @@
         p:validator-ref="shibboleth.Conditions.TRUE"
         abstract="true" />
 
+    <bean id="shibboleth.oidc.DefaultIdentifierGenerationStrategy"
+        class="net.shibboleth.profile.config.navigate.IdentifierGenerationStrategyLookupFunction"
+        p:defaultIdentifierGenerationStrategy-ref="shibboleth.DefaultIdentifierGenerationStrategy" />
+
+
     <!-- TODO: OPCSP-prefixed beans temporarily defined here and used in views to calculate CSP hashes and nonces.
          Switch into shibboleth.CSP -prefixed ones once we depend on 5.1+ -->
     
diff --git a/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/admin/oidc/issue-registration-access-token/issue-registration-access-token-beans.xml b/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/admin/oidc/issue-registration-access-token/issue-registration-access-token-beans.xml
index 6d3d64ee..4de46bde 100644
--- a/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/admin/oidc/issue-registration-access-token/issue-registration-access-token-beans.xml
+++ b/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/admin/oidc/issue-registration-access-token/issue-registration-access-token-beans.xml
@@ -39,7 +39,9 @@
         p:defaultTokenLifetime="%{idp.oidc.admin.registration.defaultTokenLifetime:P1D}"
         p:policyLocationPolicyName="%{idp.oidc.admin.registration.policyLocationPolicy:AccessByAdmin}"
         p:policyIdPolicyName="%{idp.oidc.admin.registration.policyIdPolicy:AccessByAdmin}"
-        p:clientIdPolicyName="%{idp.oidc.admin.registration.clientIdPolicy:AccessByAdmin}" />
+        p:clientIdPolicyName="%{idp.oidc.admin.registration.clientIdPolicy:AccessByAdmin}"
+        p:xmlSafeIdentifier="%{idp.oidc.xmlSafeIdentifiers:true}"
+        p:identifierGeneratorLookupStrategy-ref="shibboleth.oidc.DefaultIdentifierGenerationStrategy"/>
 
     <bean id="shibboleth.oidc.admin.DefaultMetadataPolicyLookupStrategy"
         class="net.shibboleth.oidc.profile.config.navigate.ResolverBasedRegistrationMetadataPolicyLookupFunction"
diff --git a/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/logoutprop/oidc/oidc-logout-propagation-beans.xml b/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/logoutprop/oidc/oidc-logout-propagation-beans.xml
index c8f835e9..cfa85a7d 100644
--- a/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/logoutprop/oidc/oidc-logout-propagation-beans.xml
+++ b/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/logoutprop/oidc/oidc-logout-propagation-beans.xml
@@ -80,7 +80,9 @@
 
     <bean id="PrepareBackChannelLogoutRequest"
           class="net.shibboleth.idp.plugin.oidc.op.logout.profile.impl.PrepareBackChannelLogoutRequest"
-          scope="prototype"/>
+          scope="prototype"
+          p:xmlSafeIdentifier="%{idp.oidc.xmlSafeIdentifiers:true}"
+          p:identifierGeneratorLookupStrategy-ref="shibboleth.oidc.DefaultIdentifierGenerationStrategy"/>
 
     <bean id="SignBackChannelLogoutToken"
           class="net.shibboleth.idp.profile.impl.WebFlowMessageHandlerAdaptor"
diff --git a/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/abstract/oidc-abstract-beans.xml b/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/abstract/oidc-abstract-beans.xml
index 55b31ae4..b67702ec 100644
--- a/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/abstract/oidc-abstract-beans.xml
+++ b/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/abstract/oidc-abstract-beans.xml
@@ -98,7 +98,7 @@
 
     <bean id="SessionIdGenerationStrategy" parent="shibboleth.Functions.Expression" 
         p:customObject="#{getObject('%{idp.oidc.SessionIdentifierGenerationStrategy:shibboleth.DefaultIdentifierGenerationStrategy}'.trim())}"
-        c:expression="#custom.generateIdentifier()"/>
+        c:expression="#custom.generateIdentifier(%{idp.oidc.xmlSafeIdentifiers:true})"/>
 
     <bean id="shibboleth.oidc.ChildLookup.JWTSecurityParameters"
         class="org.opensaml.messaging.context.navigate.ChildContextLookup"
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 7f01a35e..6ff3ab6e 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
@@ -531,11 +531,9 @@
 
     <bean id="SetAuthorizationCodeToResponseContext"
         class="net.shibboleth.idp.plugin.oidc.op.oauth2.profile.impl.SetAuthorizationCodeToResponseContext" scope="prototype"
-        p:dataSealer-ref="#{'%{idp.oidc.tokenSealer:shibboleth.oidc.TokenSealer}'.trim()}">
-        <property name="identifierGeneratorLookupStrategy">
-            <bean class="net.shibboleth.profile.config.navigate.IdentifierGenerationStrategyLookupFunction"
-                p:defaultIdentifierGenerationStrategy-ref="shibboleth.DefaultIdentifierGenerationStrategy" />
-        </property>
+        p:dataSealer-ref="#{'%{idp.oidc.tokenSealer:shibboleth.oidc.TokenSealer}'.trim()}"
+        p:xmlSafeIdentifier="%{idp.oidc.xmlSafeIdentifiers:true}"
+        p:identifierGeneratorLookupStrategy-ref="shibboleth.oidc.DefaultIdentifierGenerationStrategy">
         <property name="activationCondition">
             <ref bean="AuthorizeCodeRequested" />
         </property>
@@ -563,8 +561,10 @@
     <bean id="BuildOIDCAccessToken"
         class="net.shibboleth.idp.plugin.oidc.op.oauth2.profile.impl.BuildAccessToken" scope="prototype"
         p:dataSealer="#{getObject('%{idp.oidc.tokenSealer:shibboleth.oidc.TokenSealer}'.trim())}"
-        p:clientIDLookupStrategy-ref="RequestClientIDLookup" />
-        
+        p:clientIDLookupStrategy-ref="RequestClientIDLookup"
+        p:xmlSafeIdentifier="%{idp.oidc.xmlSafeIdentifiers:true}"
+        p:identifierGeneratorLookupStrategy-ref="shibboleth.oidc.DefaultIdentifierGenerationStrategy" />
+
     <bean id="RequestClientIDLookup" parent="shibboleth.Functions.Compose"
         c:g-ref="shibboleth.ClientIDLookupStrategy"
         c:f-ref="shibboleth.MessageContextLookup.Inbound" />
@@ -683,7 +683,9 @@
         p:issuerLookupStrategy-ref="AudienceIssuerLookupFunction"
         p:clientIDLookupStrategy-ref="RequestClientIDLookup"
         p:accessTokenTypeLookupStrategy-ref="AccessTokenTypeLookupFunction"
-        p:accessTokenLifetimeLookupStrategy-ref="AccessTokenLifetimeLookupFunction" />
+        p:accessTokenLifetimeLookupStrategy-ref="AccessTokenLifetimeLookupFunction"
+        p:xmlSafeIdentifier="%{idp.oidc.xmlSafeIdentifiers:true}"
+        p:identifierGeneratorLookupStrategy-ref="shibboleth.oidc.DefaultIdentifierGenerationStrategy" />
 
     <bean id="AccessTokenTypeLookupFunction"
         class="net.shibboleth.oidc.profile.config.navigate.AccessTokenTypeLookupFunction"
diff --git a/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/register/register-beans.xml b/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/register/register-beans.xml
index e4cabc80..a7969f1c 100644
--- a/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/register/register-beans.xml
+++ b/idp-oidc-extension-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/oidc/register/register-beans.xml
@@ -71,10 +71,14 @@
 
     <bean id="GenerateClientID"
         class="net.shibboleth.idp.plugin.oidc.op.profile.impl.GenerateClientID"
-        scope="prototype" />
+        scope="prototype"
+        p:xmlSafeIdentifier="%{idp.oidc.xmlSafeIdentifiers:true}"
+        p:identifierGeneratorLookupStrategy="#{getObject('shibboleth.oidc.dynreg.ClientIDGenerationStrategy') ?: getObject('shibboleth.oidc.DefaultIdentifierGenerationStrategy')}"/>
 
     <bean id="GenerateClientSecret"
-            class="net.shibboleth.idp.plugin.oidc.op.profile.impl.GenerateClientSecret" scope="prototype">
+        class="net.shibboleth.idp.plugin.oidc.op.profile.impl.GenerateClientSecret" scope="prototype"
+        p:xmlSafeIdentifier="%{idp.oidc.xmlSafeIdentifiers:true}"
+        p:identifierGeneratorLookupStrategy="#{getObject('shibboleth.oidc.dynreg.ClientSecretGenerationStrategy') ?: getObject('shibboleth.oidc.DefaultIdentifierGenerationStrategy')}">
         <property name="secretExpirationPeriodStrategy">
             <bean class="net.shibboleth.oidc.profile.config.navigate.SecretExpirationPeriodLookupFunction" />
         </property>
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 a6667518..f36bef14 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
@@ -367,7 +367,9 @@
 
     <bean id="BuildOIDCAccessToken"
         class="net.shibboleth.idp.plugin.oidc.op.oauth2.profile.impl.BuildAccessToken" scope="prototype"
-        p:dataSealer="#{getObject('%{idp.oidc.tokenSealer:shibboleth.oidc.TokenSealer}'.trim())}" />
+        p:dataSealer="#{getObject('%{idp.oidc.tokenSealer:shibboleth.oidc.TokenSealer}'.trim())}"
+        p:xmlSafeIdentifier="%{idp.oidc.xmlSafeIdentifiers:true}"
+        p:identifierGeneratorLookupStrategy-ref="shibboleth.oidc.DefaultIdentifierGenerationStrategy" />
 
     <bean id="SignOIDCAccessToken" class="net.shibboleth.idp.profile.impl.WebFlowMessageHandlerAdaptor"
             scope="prototype" c:executionDirection="OUTBOUND">
@@ -408,16 +410,13 @@
             c:sealer-ref="#{'%{idp.oidc.tokenSealer:shibboleth.oidc.TokenSealer}'.trim()}"
             p:revocationCache-ref="shibboleth.oidc.RevocationCache"
             p:activationCondition-ref="#{'%{idp.oauth2.refreshToken.activation:DefaultRefreshTokenActivationCondition}'.trim()}"
-            p:refreshTokenSerializationStrategies-ref="#{'%{idp.oauth2.refreshToken.serializationStrategies:shibboleth.oidc.DefaultRefreshTokenSerializationStrategies}'.trim()}">
-            
+            p:refreshTokenSerializationStrategies-ref="#{'%{idp.oauth2.refreshToken.serializationStrategies:shibboleth.oidc.DefaultRefreshTokenSerializationStrategies}'.trim()}"
+            p:xmlSafeIdentifier="%{idp.oidc.xmlSafeIdentifiers:true}"
+            p:identifierGeneratorLookupStrategy-ref="shibboleth.oidc.DefaultIdentifierGenerationStrategy">
         <property name="tokenRevocationLifetimeLookupStrategy">
             <bean class="net.shibboleth.idp.plugin.oidc.op.profile.logic.DefaultTokenRevocationLifetimeLookupStrategy"
                 p:clockSkew="%{idp.policy.clockSkew:PT5M}" />
         </property>
-        <property name="identifierGeneratorLookupStrategy">
-            <bean class="net.shibboleth.profile.config.navigate.IdentifierGenerationStrategyLookupFunction"
-                p:defaultIdentifierGenerationStrategy-ref="shibboleth.DefaultIdentifierGenerationStrategy" />
-        </property>
     </bean>
 
     <util:map id="shibboleth.oidc.DefaultRefreshTokenSerializationStrategies" scope="prototype">
@@ -737,7 +736,9 @@
         p:dataSealer="#{getObject('%{idp.oidc.tokenSealer:shibboleth.oidc.TokenSealer}'.trim())}"
         p:issuerLookupStrategy-ref="AudienceIssuerLookupFunction"
         p:accessTokenTypeLookupStrategy-ref="AccessTokenTypeLookupFunction"
-        p:accessTokenLifetimeLookupStrategy-ref="AccessTokenLifetimeLookupFunction" />
+        p:accessTokenLifetimeLookupStrategy-ref="AccessTokenLifetimeLookupFunction"
+        p:xmlSafeIdentifier="%{idp.oidc.xmlSafeIdentifiers:true}"
+        p:identifierGeneratorLookupStrategy-ref="shibboleth.oidc.DefaultIdentifierGenerationStrategy" />
 
     <bean id="AccessTokenTypeLookupFunction"
         class="net.shibboleth.oidc.profile.config.navigate.AccessTokenTypeLookupFunction"
diff --git a/idp-oidc-extension-impl/src/main/resources/net/shibboleth/idp/plugin/oidc/op/conf/oidc.properties b/idp-oidc-extension-impl/src/main/resources/net/shibboleth/idp/plugin/oidc/op/conf/oidc.properties
index 055bec09..cb7e822e 100644
--- a/idp-oidc-extension-impl/src/main/resources/net/shibboleth/idp/plugin/oidc/op/conf/oidc.properties
+++ b/idp-oidc-extension-impl/src/main/resources/net/shibboleth/idp/plugin/oidc/op/conf/oidc.properties
@@ -66,6 +66,9 @@ idp.signing.oidc.rsa.enc.key = %{idp.home}/credentials/idp-encryption-rsa.jwk
 # Store user consent to authorization code & access/refresh tokens instead of exploiting consent storage
 #idp.oidc.encodeConsentInTokens = false
 
+# Use xmlSafe -option for the generated identifiers, usually adds '_' prefix. Defaults to true.
+#idp.oidc.xmlSafeIdentifiers = false
+
 # The location for the policy JSON file for unregistered clients (when no client metadata is registered
 # and shibboleth.UnverifiedRelyingParty is enabled
 # Related to OIDC.SSO, OAUTH2.Token, OIDC.UserInfo, OAUTH2.Introspection, OAUTH2.Revocation configurations

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


More information about the commits mailing list