[java-idp-oidc] branch main updated: JOIDC-142 - Improve Request Object handling and configuration
Henri Mikkonen
henri.mikkonen at iki.fi
Fri Apr 7 05:29:54 UTC 2023
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=74a4b5d3e229909e76fb9641e53c47380c937459
The following commit(s) were added to refs/heads/main by this push:
new 74a4b5d3 JOIDC-142 - Improve Request Object handling and configuration
74a4b5d3 is described below
commit 74a4b5d3e229909e76fb9641e53c47380c937459
Author: Henri Mikkonen <henri.mikkonen at iki.fi>
AuthorDate: Fri Apr 7 08:29:04 2023 +0300
JOIDC-142 - Improve Request Object handling and configuration
https://shibboleth.atlassian.net/browse/JOIDC-142
Added support for useRequestObject -profile configuration parameter in the
authorize-flow.
---
.../impl/SetRequestObjectToResponseContext.java | 22 +++-
.../idp/flows/oidc/authorize/authorize-beans.xml | 3 +-
.../SetRequestObjectToResponseContextTest.java | 15 +++
.../oidc/op/profile/flow/AuthorizeFlowTest.java | 123 +++++++++++++++++++++
.../src/test/resources/conf/relying-party.xml | 9 ++
5 files changed, 170 insertions(+), 2 deletions(-)
diff --git a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/SetRequestObjectToResponseContext.java b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/SetRequestObjectToResponseContext.java
index 11f9ebd9..eec618c6 100644
--- a/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/SetRequestObjectToResponseContext.java
+++ b/idp-oidc-extension-impl/src/main/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/SetRequestObjectToResponseContext.java
@@ -21,6 +21,7 @@ import java.io.IOException;
import java.net.URI;
import java.text.ParseException;
import java.util.Set;
+import java.util.function.Predicate;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
@@ -60,6 +61,9 @@ public class SetRequestObjectToResponseContext extends AbstractOAuthAuthorizatio
/** HTTP client security parameters. */
@Nullable private HttpClientSecurityParameters httpClientSecurityParameters;
+
+ /** Predicate for enforcing the use of request objects. */
+ @NonnullAfterInit private Predicate<ProfileRequestContext> requestObjectEnforcedPredicate;
/**
* Set the {@link HttpClient} to use.
@@ -81,6 +85,15 @@ public class SetRequestObjectToResponseContext extends AbstractOAuthAuthorizatio
httpClientSecurityParameters = params;
}
+ /**
+ * Set the predicate for enforcing the use of request objects.
+ *
+ * @param predicate the predicate for enforcing the use of request objects
+ */
+ public void setRequestObjectEnforcedPredicate(@Nonnull final Predicate<ProfileRequestContext> predicate) {
+ requestObjectEnforcedPredicate = Constraint.isNotNull(predicate,
+ "Request object enforced predicate annot be null");
+ }
/**
* Build the {@link HttpClientContext} instance to be used by the HttpClient.
*
@@ -101,6 +114,7 @@ public class SetRequestObjectToResponseContext extends AbstractOAuthAuthorizatio
protected void doInitialize() throws ComponentInitializationException {
super.doInitialize();
Constraint.isNotNull(httpClient, "Httpclient cannot be null");
+ Constraint.isNotNull(requestObjectEnforcedPredicate, "Request object enforced predicate annot be null");
}
/** {@inheritDoc} */
@@ -111,7 +125,13 @@ public class SetRequestObjectToResponseContext extends AbstractOAuthAuthorizatio
}
if (!getAuthorizationRequest().specifiesRequestObject()) {
- log.debug("{} No request_uri or request by value, nothing to do", getLogPrefix());
+ if (requestObjectEnforcedPredicate.test(profileRequestContext)) {
+ log.warn("{} No request_uri or request by value, even though it's enforced for {}", getLogPrefix(),
+ getMetadataContext().getClientInformation().getID().getValue());
+ ActionSupport.buildEvent(profileRequestContext, OidcEventIds.MISSING_MANDATORY_REQUEST_OBJECT);
+ } else {
+ log.debug("{} No request_uri or request by value, nothing to do", getLogPrefix());
+ }
return false;
}
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 c3d2edf5..ddc50316 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
@@ -110,7 +110,8 @@
<bean id="SetRequestObjectToResponseContext"
class="net.shibboleth.idp.plugin.oidc.op.oauth2.profile.impl.SetRequestObjectToResponseContext" scope="prototype"
p:httpClient="#{getObject('shibboleth.oidc.NonBrowser.HttpClient') ?: getObject('shibboleth.InternalHttpClient')}"
- p:httpClientSecurityParameters="#{getObject('shibboleth.oidc.NonBrowser.HttpClientSecurityParameters')}" />
+ p:httpClientSecurityParameters="#{getObject('shibboleth.oidc.NonBrowser.HttpClientSecurityParameters')}"
+ p:requestObjectEnforcedPredicate-ref="UseRequestObjectPredicate"/>
<bean id="RequestObjectEncryptedCondition" parent="shibboleth.Conditions.Expression"
c:expression="#input.getOutboundMessageContext().getSubcontext(T(net.shibboleth.idp.plugin.oidc.op.messaging.context.OIDCAuthenticationResponseContext)).getRequestObject() instanceof T(com.nimbusds.jwt.EncryptedJWT)" />
diff --git a/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/SetRequestObjectToResponseContextTest.java b/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/SetRequestObjectToResponseContextTest.java
index 23cdd3a5..5e550497 100644
--- a/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/SetRequestObjectToResponseContextTest.java
+++ b/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/oauth2/profile/impl/SetRequestObjectToResponseContextTest.java
@@ -27,7 +27,10 @@ import net.shibboleth.utilities.java.support.test.repository.RepositorySupport;
import java.net.URI;
import java.net.URISyntaxException;
import java.util.Set;
+import java.util.function.Predicate;
+import org.mockito.Mockito;
+import org.opensaml.profile.context.ProfileRequestContext;
import org.springframework.webflow.execution.Event;
import org.testng.Assert;
import org.testng.annotations.BeforeMethod;
@@ -45,6 +48,8 @@ public class SetRequestObjectToResponseContextTest extends BaseOIDCResponseActio
/** Action. */
private SetRequestObjectToResponseContext action;
+
+ private Predicate<ProfileRequestContext> enforceRequestObjects = Mockito.mock(Predicate.class);
/** Init. */
@BeforeMethod
@@ -55,16 +60,26 @@ public class SetRequestObjectToResponseContextTest extends BaseOIDCResponseActio
final HttpClientBuilder builder = new HttpClientBuilder();
builder.setTLSSocketFactory(HttpClientSupport.buildNoTrustTLSSocketFactory());
action.setHttpClient(builder.buildClient());
+ action.setRequestObjectEnforcedPredicate(enforceRequestObjects);
action.initialize();
}
/** Test when no request object or URI is used. */
@Test
public void testNothingToDo() {
+ Mockito.when(enforceRequestObjects.test(Mockito.any())).thenReturn(false);
final Event event = action.execute(requestCtx);
ActionTestingSupport.assertProceedEvent(event);
}
+ /** Test when no request object or URI is used when configured mandatory */
+ @Test
+ public void testMandatoryMissing() {
+ Mockito.when(enforceRequestObjects.test(Mockito.any())).thenReturn(true);
+ final Event event = action.execute(requestCtx);
+ ActionTestingSupport.assertEvent(event, OidcEventIds.MISSING_MANDATORY_REQUEST_OBJECT);
+ }
+
/**
* Test on request object.
*
diff --git a/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/flow/AuthorizeFlowTest.java b/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/flow/AuthorizeFlowTest.java
index 5d133c17..a7a1bde5 100644
--- a/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/flow/AuthorizeFlowTest.java
+++ b/idp-oidc-extension-impl/src/test/java/net/shibboleth/idp/plugin/oidc/op/profile/flow/AuthorizeFlowTest.java
@@ -119,6 +119,21 @@ public class AuthorizeFlowTest extends AbstractOidcFlowTest {
Assert.assertNull(successResponse.getIssuer());
}
+ @Test
+ public void testWithAuthorizationCodeFlowRequestObjectEnforcedNoRO() throws IOException, SessionException {
+ setRequestParameters(List.of(new Pair<>("client_id", "mockClientIdRequestObjectEnforced"),
+ new Pair<>("response_type", "code"),
+ new Pair<>("scope", "openid profile"),
+ new Pair<>("redirect_uri", redirectUri)));
+ request.setMethod("GET");
+ storeMetadata(storageService, "mockClientIdRequestObjectEnforced", clientSecret, scope, redirectUri);
+
+ initializeThreadLocals();
+
+ final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+ Assert.assertEquals("ErrorView", result.getOutcome().getId());
+ }
+
@Test
public void testWithAuthorizationCodeFlowAndResource() throws IOException, SessionException {
request.setMethod("GET");
@@ -254,6 +269,22 @@ public class AuthorizeFlowTest extends AbstractOidcFlowTest {
Assert.assertNull(successResponse.getIssuer());
}
+ @Test
+ public void testWithImplicitFlowRequestObjectEnforcedNoRO() throws IOException, SessionException {
+ request.setMethod("GET");
+ setRequestParameters(List.of(new Pair<>("client_id", "mockClientIdRequestObjectEnforced"),
+ new Pair<>("response_type", "id_token"),
+ new Pair<>("scope", "openid profile"),
+ new Pair<>("redirect_uri", redirectUri),
+ new Pair<>("nonce", "idhas3h23hi13h1o2i32")));
+ storeMetadata(storageService, "mockClientIdRequestObjectEnforced", clientSecret, scope, redirectUri);
+
+ initializeThreadLocals();
+
+ final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+ Assert.assertEquals("ErrorView", result.getOutcome().getId());
+ }
+
@Test
public void testWithImplicitFlowIssInResponse() throws IOException, SessionException, ParseException {
request.setMethod("GET");
@@ -337,6 +368,22 @@ public class AuthorizeFlowTest extends AbstractOidcFlowTest {
Assert.assertNull(successResponse.getIssuer());
}
+ @Test
+ public void testWithImplicitTokenFlowRequestObjectEnforcedNoRO() throws IOException, SessionException {
+ request.setMethod("GET");
+ setRequestParameters(List.of(new Pair<>("client_id", "mockClientIdRequestObjectEnforced"),
+ new Pair<>("response_type", "id_token token"),
+ new Pair<>("scope", "openid profile"),
+ new Pair<>("redirect_uri", redirectUri),
+ new Pair<>("nonce", "idhas3h23hi13h1o2i32")));
+ storeMetadata(storageService, "mockClientIdRequestObjectEnforced", clientSecret, scope, redirectUri);
+
+ initializeThreadLocals();
+
+ final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+ Assert.assertEquals("ErrorView", result.getOutcome().getId());
+ }
+
@Test
public void testWithImplicitTokenFlowIssInResponse() throws IOException, SessionException, DataSealerException, ParseException {
request.setMethod("GET");
@@ -492,6 +539,22 @@ public class AuthorizeFlowTest extends AbstractOidcFlowTest {
Assert.assertNull(successResponse.getIssuer());
}
+ @Test
+ public void testWithHybridIdTokenFlowRequestObjectEnforcedNoRO() throws IOException, SessionException {
+ request.setMethod("GET");
+ setRequestParameters(List.of(new Pair<>("client_id", "mockClientIdRequestObjectEnforced"),
+ new Pair<>("response_type", "code id_token"),
+ new Pair<>("scope", "openid profile"),
+ new Pair<>("redirect_uri", redirectUri),
+ new Pair<>("nonce", "idhas3h23hi13h1o2i32")));
+ storeMetadata(storageService, "mockClientIdRequestObjectEnforced", clientSecret, scope, redirectUri);
+
+ initializeThreadLocals();
+
+ final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+ Assert.assertEquals("ErrorView", result.getOutcome().getId());
+ }
+
@Test
public void testWithHybridIdTokenFlowIssInResponse() throws IOException, SessionException {
request.setMethod("GET");
@@ -591,6 +654,22 @@ public class AuthorizeFlowTest extends AbstractOidcFlowTest {
Assert.assertNull(successResponse.getIssuer());
}
+ @Test
+ public void testWithHybridTokenFlowRequestObjectEnforcedNoRO() throws IOException, SessionException {
+ request.setMethod("GET");
+ setRequestParameters(List.of(new Pair<>("client_id", "mockClientIdRequestObjectEnforced"),
+ new Pair<>("response_type", "code token"),
+ new Pair<>("scope", "openid profile"),
+ new Pair<>("redirect_uri", redirectUri),
+ new Pair<>("nonce", "idhas3h23hi13h1o2i32")));
+ storeMetadata(storageService, "mockClientIdRequestObjectEnforced", clientSecret, scope, redirectUri);
+
+ initializeThreadLocals();
+
+ final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+ Assert.assertEquals("ErrorView", result.getOutcome().getId());
+ }
+
@Test
public void testWithHybridTokenFlowNoOpenidScopeRequested() throws IOException, SessionException, ParseException, DataSealerException {
request.setMethod("GET");
@@ -688,6 +767,22 @@ public class AuthorizeFlowTest extends AbstractOidcFlowTest {
Assert.assertNull(token.getClaimsSet().getStringClaim("eduPersonScopedAffiliation"));
}
+ @Test
+ public void testWithHybridIdTokenTokenFlowRequestObjectEnforcedNoRO() throws IOException, SessionException {
+ request.setMethod("GET");
+ setRequestParameters(List.of(new Pair<>("client_id", "mockClientIdRequestObjectEnforced"),
+ new Pair<>("response_type", "code id_token token"),
+ new Pair<>("scope", "openid profile"),
+ new Pair<>("redirect_uri", redirectUri),
+ new Pair<>("nonce", "idhas3h23hi13h1o2i32")));
+ storeMetadata(storageService, "mockClientIdRequestObjectEnforced", clientSecret, scope, redirectUri);
+
+ initializeThreadLocals();
+
+ final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+ Assert.assertEquals("ErrorView", result.getOutcome().getId());
+ }
+
@Test
public void testWithHybridIdTokenTokenFlowNoOpenidScopeRequested() throws IOException, SessionException, ParseException, DataSealerException {
request.setMethod("GET");
@@ -1156,6 +1251,33 @@ public class AuthorizeFlowTest extends AbstractOidcFlowTest {
Assert.assertNull(successResponse.getIssuer());
}
+ @Test
+ public void testWithPlainReqObjectOverwriteRedirectUriROEnforced() throws IOException, SessionException {
+ final JWTClaimsSet ro = new JWTClaimsSet.Builder()
+ .claim("redirect_uri", redirectUri)
+ .build();
+ final PlainJWT requestObject = new PlainJWT(ro);
+ request.setMethod("GET");
+ setRequestParameters(List.of(new Pair<>("client_id", "mockClientIdRequestObjectEnforced"),
+ new Pair<>("response_type", "code"),
+ new Pair<>("scope", "openid profile"),
+ new Pair<>("redirect_uri", "https://invalid.org/cb"),
+ new Pair<>("request", requestObject.serialize())));
+ storeMetadata(storageService, "mockClientIdRequestObjectEnforced", clientSecret, scope, redirectUri);
+
+ initializeThreadLocals();
+
+ final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+ final AuthenticationResponse responseMessage = parseSuccessResponse(result, AuthenticationResponse.class);
+ final AuthenticationSuccessResponse successResponse = responseMessage.toSuccessResponse();
+ Assert.assertEquals(successResponse.getRedirectionURI().toString(), redirectUri);
+ Assert.assertNull(successResponse.getIDToken());
+ Assert.assertNull(successResponse.getAccessToken());
+ Assert.assertNotNull(successResponse.getAuthorizationCode());
+ Assert.assertNotNull(getSidFromAuthorizeCodeClaimsSet(successResponse));
+ Assert.assertNull(successResponse.getIssuer());
+ }
+
@Test
public void testWithPlainReqObjectClaimsRequest() throws IOException, SessionException,
DataSealerException, ParseException {
@@ -1536,6 +1658,7 @@ public class AuthorizeFlowTest extends AbstractOidcFlowTest {
removeMetadata(storageService, "mockClientIdPKCEPlain");
removeMetadata(storageService, "mockClientIdPKCES256");
removeMetadata(storageService, "mockClientIdCustomTokens");
+ removeMetadata(storageService, "mockClientIdRequestObjectEnforced");
removeMetadata(storageService, clientIdIssInResponse);
}
diff --git a/idp-oidc-extension-impl/src/test/resources/conf/relying-party.xml b/idp-oidc-extension-impl/src/test/resources/conf/relying-party.xml
index 0af0bd12..d451d60c 100644
--- a/idp-oidc-extension-impl/src/test/resources/conf/relying-party.xml
+++ b/idp-oidc-extension-impl/src/test/resources/conf/relying-party.xml
@@ -80,6 +80,15 @@
</list>
</property>
</bean>
+ <bean parent="RelyingPartyByName" c:relyingPartyIds="mockClientIdRequestObjectEnforced">
+ <property name="profileConfigurations">
+ <list>
+ <bean parent="OIDC.SSO.MDDriven" p:encryptionOptional="true" p:useRequestObject="true" p:signRequestObject="false"/>
+ <bean parent="OAUTH2.Token.MDDriven" p:encryptionOptional="false" />
+ <bean parent="OIDC.UserInfo.MDDriven" p:encryptionOptional="false" />
+ </list>
+ </property>
+ </bean>
<bean parent="RelyingPartyByName" c:relyingPartyIds="mockClientIdRequestObjectEncryptionEnforced">
<property name="profileConfigurations">
<list>
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list