[java-idp-plugin-oidc-rp] branch main updated: Add condition for checking if UserInfo claims should be requested
Phil Smart
philip.smart at jisc.ac.uk
Wed Feb 16 16:49:46 UTC 2022
This is an automated email from the git hooks/post-receive script.
philsmart pushed a commit to branch main
in repository java-idp-plugin-oidc-rp.
View the commit online:
http://git.shibboleth.net/view/?p=java-idp-plugin-oidc-rp.git;a=commit;h=1240adcd12f137013ea5248caee372f5f76be865
The following commit(s) were added to refs/heads/main by this push:
new 1240adc Add condition for checking if UserInfo claims should be requested
1240adc is described below
commit 1240adcd12f137013ea5248caee372f5f76be865
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Wed Feb 16 16:49:39 2022 +0000
Add condition for checking if UserInfo claims should be requested
As taken from the profile configuration. That might need to change.
---
...DTokenClaims.java => ProcessEndUserClaims.java} | 70 +++++++++-------------
.../oidc/rp/impl/UserInfoLookupCondition.java | 54 +++++++++++++++++
.../oidc-relying-party-authn-beans.xml | 8 ++-
.../oidc-relying-party-authn-flow.xml | 4 +-
.../rp/impl/MergeUserInfoAndIDTokenClaimsTest.java | 6 +-
5 files changed, 93 insertions(+), 49 deletions(-)
diff --git a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ProcessUserInfoAndIDTokenClaims.java b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ProcessEndUserClaims.java
similarity index 82%
rename from idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ProcessUserInfoAndIDTokenClaims.java
rename to idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ProcessEndUserClaims.java
index 425e0fc..cd65321 100644
--- a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ProcessUserInfoAndIDTokenClaims.java
+++ b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ProcessEndUserClaims.java
@@ -45,17 +45,19 @@ import net.shibboleth.utilities.java.support.component.ComponentSupport;
import net.shibboleth.utilities.java.support.logic.Constraint;
/**
- * Merge the claims in the id_token with the claims from the UserInfo response. Before merge,
- * both id_token and UserInfo claim sets are sanitized by a replaceable strategy. For example,
- * by default to remove 'validation claims' that should not be exposed further by the system.
- * This can be turned off by setting a no-op sanitizer, or setting
+ * Process the end-user claims from the id_token and possible UserInfo claims sets.
+ *
+ * <p>Sanitized both claims sets using a replaceable strategy. For example,
+ * by default to remove 'validation claims' that should not be exposed further by the system.</p>
+ *
+ * <p>Merge the claims sets together to produce an aggregate claims set. The UserInfo claims can
+ * be empty i.e. claims from the UserInfo endpoint were not requested.</p>
*/
//TODO similar too ValidateUserInfoClaims, do we need to extend OIDC action
-//TODO ensure everything passes through this, even if no UserInfo
-public class ProcessUserInfoAndIDTokenClaims extends AbstractOIDCAuthenticationAction {
+public class ProcessEndUserClaims extends AbstractOIDCAuthenticationAction {
/** Class logger.*/
- @Nonnull private final Logger log = LoggerFactory.getLogger(ProcessUserInfoAndIDTokenClaims.class);
+ @Nonnull private final Logger log = LoggerFactory.getLogger(ProcessEndUserClaims.class);
/** Strategy used to look up the {@link UserInfoResponseContext}. */
@Nonnull private Function<ProfileRequestContext, UserInfoResponseContext>
@@ -79,20 +81,18 @@ public class ProcessUserInfoAndIDTokenClaims extends AbstractOIDCAuthenticationA
*/
@Nonnull private UnaryOperator<ClaimsSet> claimSanitizationStrategy;
- /** The stashed user info claims.*/
+ /** The stashed UserInfo claims. This is allowably {@literal empty} if the UserInfo endpoint was not used.*/
@Nullable private ClaimsSet userInfoClaims;
- /** The stashed id_token claims.*/
+ /**
+ * The stashed id_token claims. This should never be {@literal null} or {@literal empty} once
+ * {@link #doPreExecute(ProfileRequestContext)} has run.
+ */
@Nullable private JWTClaimsSet idTokenClaims;
- /** The subject identifier taken from the id_token claims.*/
- @Nullable private String idTokenSubject;
-
- /** The issuer of the id_token response taken from the id_token claims.*/
- @Nullable private String idTokenIssuer;
/** Constructor.*/
- public ProcessUserInfoAndIDTokenClaims() {
+ public ProcessEndUserClaims() {
userInfoResponseContextLookupStrategy =
new ChildContextLookup<>(UserInfoResponseContext.class).compose(
new InboundMessageContextLookup());
@@ -202,17 +202,13 @@ public class ProcessUserInfoAndIDTokenClaims extends AbstractOIDCAuthenticationA
final UserInfoResponseContext userInfoCtx =
userInfoResponseContextLookupStrategy.apply(profileRequestContext);
- if (userInfoCtx == null) {
- log.debug("{} No UserInfo response context returned by lookup strategy", getLogPrefix());
- ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_PROFILE_CTX);
- return false;
+ if (userInfoCtx == null || userInfoCtx.getUserInfo() == null) {
+ log.trace("{} No UserInfo response context returned by lookup strategy, creating empty "
+ + "UserInfo claims", getLogPrefix());
+ userInfoClaims = new ClaimsSet();
+ } else{
+ userInfoClaims = userInfoCtx.getUserInfo().getClaimsSet();
}
- if (userInfoCtx.getUserInfo() == null) {
- log.debug("{} No UserInfo returned by lookup strategy", getLogPrefix());
- ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_PROFILE_CTX);
- return false;
- }
- userInfoClaims = userInfoCtx.getUserInfo().getClaimsSet();
final AccessTokenResponseContext tokenResponseCtx =
accessTokenResponseContextLookupStrategy.apply(profileRequestContext);
@@ -238,28 +234,18 @@ public class ProcessUserInfoAndIDTokenClaims extends AbstractOIDCAuthenticationA
} catch (final ParseException e) {
log.debug("{} Unable to parse claims from id_token", getLogPrefix());
ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_PROFILE_CTX);
- }
-
- idTokenSubject = idTokenClaims.getSubject();
- if (idTokenSubject == null) {
- log.error("{} No subject found in id_token", getLogPrefix());
- ActionSupport.buildEvent(profileRequestContext, OidcEventIds.INVALID_ID_TOKEN);
- return false;
- }
- idTokenIssuer = idTokenClaims.getIssuer();
- if (idTokenIssuer == null) {
- log.error("{} No issuer found in id_token", getLogPrefix());
- ActionSupport.buildEvent(profileRequestContext, OidcEventIds.INVALID_ID_TOKEN);
- return false;
- }
-
+ }
+
return true;
}
@Override protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext,
@Nonnull final AuthenticationContext authenticationContext) {
- log.trace("{} Merging UserInfo and id_token claims", getLogPrefix());
+ if (log.isTraceEnabled()) {
+ log.trace("{} Processing {} claims", getLogPrefix(),
+ userInfoClaims.toJSONObject().size() > 0 ? "UserInfo and id_token" : "id_token");
+ }
final ClaimsSet idToken = new ClaimsSet();
idToken.putAll(idTokenClaims.toJSONObject());
@@ -276,7 +262,7 @@ public class ProcessUserInfoAndIDTokenClaims extends AbstractOIDCAuthenticationA
.setUnprocessedIdTokenClaims(idTokenClaims);
if (log.isTraceEnabled()) {
- log.trace("{} Merged UserInfo and id_token claims to produce a claims set containing '{}'",
+ log.trace("{} Merged and sanitized claims to produce the claims set '{}'",
getLogPrefix(), mergedClaims.toJSONString());
}
}
diff --git a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/UserInfoLookupCondition.java b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/UserInfoLookupCondition.java
new file mode 100644
index 0000000..e2e0e24
--- /dev/null
+++ b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/UserInfoLookupCondition.java
@@ -0,0 +1,54 @@
+/*
+ * 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.authn.oidc.rp.impl;
+
+import java.util.function.Predicate;
+
+import javax.annotation.Nullable;
+
+import org.opensaml.profile.context.ProfileRequestContext;
+
+import net.shibboleth.idp.profile.context.RelyingPartyContext;
+import net.shibboleth.oidc.profile.config.OIDCAuthorizationConfiguration;
+
+/**
+ * Checks whether the UserInfo endpoint should be accessed to retrieve claims about the
+ * authenticated end-user. Defaults to true, unless overridden in the profile configuration.
+ */
+public class UserInfoLookupCondition implements Predicate<ProfileRequestContext> {
+
+ @Override
+ public boolean test(@Nullable final ProfileRequestContext prc) {
+ if (prc == null) {
+ return true;
+ }
+ final RelyingPartyContext rpCtx = prc.getSubcontext(RelyingPartyContext.class);
+ if (rpCtx != null && rpCtx.getProfileConfig() != null &&
+ rpCtx.getProfileConfig() instanceof OIDCAuthorizationConfiguration) {
+ final OIDCAuthorizationConfiguration profileConfiguration =
+ (OIDCAuthorizationConfiguration) rpCtx.getProfileConfig();
+ return profileConfiguration.isRetrieveUserInfoEndpointClaims(prc);
+ }
+
+ // Perform user input validation by default if no config found
+ return true;
+
+
+ }
+
+}
diff --git a/idp-oidc-rp-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/OIDCRelyingParty/oidc-relying-party-authn-beans.xml b/idp-oidc-rp-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/OIDCRelyingParty/oidc-relying-party-authn-beans.xml
index 0dc6a1f..b6492b9 100644
--- a/idp-oidc-rp-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/OIDCRelyingParty/oidc-relying-party-authn-beans.xml
+++ b/idp-oidc-rp-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/OIDCRelyingParty/oidc-relying-party-authn-beans.xml
@@ -399,6 +399,10 @@
</util:list>
+ <bean id="CheckUserInfoRequiredCondition"
+ class="net.shibboleth.idp.plugin.authn.oidc.rp.impl.UserInfoLookupCondition"/>
+
+
<!-- UserInfo endpoint beans -->
<bean id="UserInfoEndpointLookup" class="net.shibboleth.idp.plugin.authn.oidc.rp.impl.UserInfoEndpointLookup"
@@ -424,8 +428,8 @@
p:authenticationContextLookupStrategy-ref="ParentAuthenticiationContextLookup"/>
- <bean id="ProcessUserInfoAndIDTokenClaims"
- class="net.shibboleth.idp.plugin.authn.oidc.rp.impl.ProcessUserInfoAndIDTokenClaims" scope="prototype"
+ <bean id="ProcessEndUserClaims"
+ class="net.shibboleth.idp.plugin.authn.oidc.rp.impl.ProcessEndUserClaims" scope="prototype"
p:profileContextLookupStrategy-ref="shibboleth.ChildLookup.ProxyProfileRequestContext"
p:authenticationContextLookupStrategy-ref="ParentAuthenticiationContextLookup"/>
diff --git a/idp-oidc-rp-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/OIDCRelyingParty/oidc-relying-party-authn-flow.xml b/idp-oidc-rp-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/OIDCRelyingParty/oidc-relying-party-authn-flow.xml
index 99b96f4..8b31a5d 100644
--- a/idp-oidc-rp-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/OIDCRelyingParty/oidc-relying-party-authn-flow.xml
+++ b/idp-oidc-rp-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/OIDCRelyingParty/oidc-relying-party-authn-flow.xml
@@ -107,7 +107,7 @@
<!-- Should we request information from the UserInfo endpoint based on profile config -->
<decision-state id="CheckUserInfoRequired">
- <if test="true"
+ <if test="CheckUserInfoRequiredCondition.test(opensamlProfileRequestContext.getSubcontext('net.shibboleth.idp.authn.context.AuthenticationContext').getSubcontext('org.opensaml.profile.context.ProfileRequestContext'))"
then="UserInfoRequest"
else="FinalizeResponse" />
<!-- if else here, we need to set id_token claims into EndUserClaimsContext -->
@@ -157,7 +157,7 @@
</action-state>
<action-state id="FinalizeResponse">
- <evaluate expression="ProcessUserInfoAndIDTokenClaims" /> <!-- check this works if no userInfo -->
+ <evaluate expression="ProcessEndUserClaims" /> <!-- check this works if no userInfo -->
<evaluate expression="ValidateOIDCAuthentication" />
<evaluate expression="'proceed'" />
<transition on="proceed" to="proceed" />
diff --git a/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/MergeUserInfoAndIDTokenClaimsTest.java b/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/MergeUserInfoAndIDTokenClaimsTest.java
index b974b9e..b106ffc 100644
--- a/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/MergeUserInfoAndIDTokenClaimsTest.java
+++ b/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/MergeUserInfoAndIDTokenClaimsTest.java
@@ -47,17 +47,17 @@ import net.shibboleth.idp.profile.context.navigate.WebflowRequestContextProfileR
import net.shibboleth.oidc.security.jwt.claims.impl.JWTClaims;
import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
-/** Tests for {@link ProcessUserInfoAndIDTokenClaims}.*/
+/** Tests for {@link ProcessEndUserClaims}.*/
public class MergeUserInfoAndIDTokenClaimsTest extends AbstractOIDCTest {
/** Action to test.*/
- private ProcessUserInfoAndIDTokenClaims action;
+ private ProcessEndUserClaims action;
@BeforeMethod
public void setup() throws Exception {
super.setup();
- action = new ProcessUserInfoAndIDTokenClaims();
+ action = new ProcessEndUserClaims();
final AccessTokenResponseContext trc = new AccessTokenResponseContext();
final PlainJWT jwt = new PlainJWT(new JWTClaimsSet.Builder()
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list