[java-idp-plugin-oidc-rp] branch main updated: Fix failing tests

Phil Smart philip.smart at jisc.ac.uk
Mon Aug 1 12:37:18 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=cdb212d538882295939cc987a8510ca2c8f2eb55

The following commit(s) were added to refs/heads/main by this push:
     new cdb212d  Fix failing tests
cdb212d is described below

commit cdb212d538882295939cc987a8510ca2c8f2eb55
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Mon Aug 1 13:37:13 2022 +0100

    Fix failing tests
---
 .../impl/DefaultAuthCodeTokenRequestEncoder.java   |   3 +-
 .../DefaultAuthCodeTokenRequestEncoderTest.java    |  14 ++-
 .../plugin/authn/oidc/rp/impl/OIDCRPFlowTest.java  | 120 +++++++++++----------
 .../rp/messaging/impl/SignRequestObjectTest.java   |  69 +++++++++---
 4 files changed, 135 insertions(+), 71 deletions(-)

diff --git a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/encoding/impl/DefaultAuthCodeTokenRequestEncoder.java b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/encoding/impl/DefaultAuthCodeTokenRequestEncoder.java
index 25d5120..c5c4b33 100644
--- a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/encoding/impl/DefaultAuthCodeTokenRequestEncoder.java
+++ b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/encoding/impl/DefaultAuthCodeTokenRequestEncoder.java
@@ -53,7 +53,8 @@ public class DefaultAuthCodeTokenRequestEncoder extends AbstractRequestEncoderFu
     @Nullable public HttpUriRequest doApply(@Nonnull final ProfileRequestContext profileRequestContext) {
 
         try {
-            if (getClientAuthenticationContext() == null) {
+            if (getClientAuthenticationContext() == null || getClientAuthenticationContext().getClientAuthentication()
+                    == null) {
                 log.warn("No client authentication context to base token request off");
                 return null;
             }
diff --git a/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/encoding/impl/DefaultAuthCodeTokenRequestEncoderTest.java b/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/encoding/impl/DefaultAuthCodeTokenRequestEncoderTest.java
index ccbaaf1..d0cd2f5 100644
--- a/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/encoding/impl/DefaultAuthCodeTokenRequestEncoderTest.java
+++ b/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/encoding/impl/DefaultAuthCodeTokenRequestEncoderTest.java
@@ -27,7 +27,15 @@ import org.apache.http.client.methods.HttpUriRequest;
 import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
+import com.nimbusds.oauth2.sdk.auth.ClientAuthentication;
+import com.nimbusds.oauth2.sdk.auth.ClientSecretBasic;
+import com.nimbusds.oauth2.sdk.auth.Secret;
+import com.nimbusds.oauth2.sdk.http.HTTPRequest;
+import com.nimbusds.oauth2.sdk.id.ClientID;
+
+import net.shibboleth.idp.plugin.authn.oidc.rp.context.OIDCPeerEntityContext;
 import net.shibboleth.idp.plugin.authn.oidc.rp.impl.AbstractOIDCTest;
+import net.shibboleth.oidc.authn.context.OAuth2ClientAuthenticationContext;
 
 /** Tests for the DefaultTokenRequestEncoder.*/
 public class DefaultAuthCodeTokenRequestEncoderTest extends AbstractOIDCTest {
@@ -35,10 +43,14 @@ public class DefaultAuthCodeTokenRequestEncoderTest extends AbstractOIDCTest {
     /** The encoder to test.*/
     private DefaultAuthCodeTokenRequestEncoder encoder;
     
+    @Override
     @BeforeMethod
     public void setup() throws Exception {
         super.setup();
         encoder = new DefaultAuthCodeTokenRequestEncoder();
+        final var authContext = prc.getOutboundMessageContext().getSubcontext(OIDCPeerEntityContext.class, true)
+            .getSubcontext(OAuth2ClientAuthenticationContext.class, true);
+        authContext.setClientAuthentication(new ClientSecretBasic(new ClientID("client_id"), new Secret("secret")));
     }
     
     
@@ -50,7 +62,7 @@ public class DefaultAuthCodeTokenRequestEncoderTest extends AbstractOIDCTest {
         assertTrue(request instanceof HttpEntityEnclosingRequest);
         assertNotNull(request.getFirstHeader("Authorization"));
         assertNotNull(((HttpEntityEnclosingRequest)request).getEntity().getContent());
-        String content = new String(
+        final String content = new String(
                 ((HttpEntityEnclosingRequest)request).getEntity().getContent().readAllBytes(), StandardCharsets.UTF_8);
         assertTrue("grant_type expected in request", content.contains("grant_type"));
         assertTrue("authorization_code expected in request", content.contains("authorization_code"));
diff --git a/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/OIDCRPFlowTest.java b/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/OIDCRPFlowTest.java
index e0d794f..9f47937 100644
--- a/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/OIDCRPFlowTest.java
+++ b/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/OIDCRPFlowTest.java
@@ -390,6 +390,56 @@ public class OIDCRPFlowTest extends AbstractAuthnXmlFlowExecutionTests {
         context.setRedirectUriOverride(redirectOverride);
         return context;
     }
+    
+    /**
+     * Create a basic security configuration, which can be overriden per test if required.
+     * 
+
+     * @return the basic security configuration.
+     */
+    private OIDCSecurityConfiguration createBasicSecurityConfigAndValidationParams() {
+
+        final var securityConfig = new OIDCSecurityConfiguration();
+        
+        final var idTokenSigValConfig = new BasicSignatureValidationConfiguration<SignedJWT>();
+        idTokenSigValConfig.setSignatureTrustEngine(
+                new ExplicitKeySignedJWTTrustEngine(new CriterionCredentialResolver(), 
+                        new BasicJOSEObjectCredentialResolver()));
+        securityConfig.setIdTokenJwtSignatureValidationConfiguration(idTokenSigValConfig);
+        
+        final var userInfoTokenSigValConfig = new BasicSignatureValidationConfiguration<SignedJWT>();
+        userInfoTokenSigValConfig.setSignatureTrustEngine(
+                new ExplicitKeySignedJWTTrustEngine(new CriterionCredentialResolver(), 
+                        new BasicJOSEObjectCredentialResolver()));
+        securityConfig.setUserInfoTokenJwtSignatureValidationConfiguration(userInfoTokenSigValConfig);
+        
+        //The CEK resolver just resolves keys from the criteria set.
+        final var idTokenDecryptConfig = new BasicJWTDecryptionConfiguration();        
+        idTokenDecryptConfig.setContentEncryptionKeyCredentialResolver(new CriterionCredentialResolver());
+        securityConfig.setIdTokenJwtDecryptionConfiguration(idTokenDecryptConfig);
+        
+        final var userInfoDecryptConfig = new BasicJWTDecryptionConfiguration();        
+        userInfoDecryptConfig.setContentEncryptionKeyCredentialResolver(new CriterionCredentialResolver());
+        securityConfig.setUserInfoJwtDecryptionConfiguration(userInfoDecryptConfig);
+        
+        
+        return securityConfig;
+    }
+    
+    private void assertStandardSuccessConditions(final ProfileRequestContext prc) {
+        //assert success conditions. 
+        assertFlowExecutionEnded();
+        assertNotNull(prc.getSubcontext(AuthenticationContext.class));
+        assertNotNull(prc.getSubcontext(SubjectCanonicalizationContext.class));
+        assertNotNull(prc.getSubcontext(SubjectCanonicalizationContext.class).getSubject().getPrincipals());
+        //As SimpleSubjectCanonicalization has not been run, we pull out the subject
+        final UsernamePrincipal usernamePrincipal = 
+                prc.getSubcontext(SubjectCanonicalizationContext.class).getSubject()
+                .getPrincipals(UsernamePrincipal.class).iterator().next();
+        assertNotNull(usernamePrincipal);
+        assertEquals(usernamePrincipal.getName(),"jdoe");
+    }
+    
    
     
     /** 
@@ -444,6 +494,8 @@ public class OIDCRPFlowTest extends AbstractAuthnXmlFlowExecutionTests {
         updateFlowExecution(flowExecution);
         flowExecution.start(inputMap, externalContext);    
         assertCurrentStateEquals("AuthnRequest");
+        
+        mockOPServer.shutdown();
     }
     
     /**
@@ -482,6 +534,8 @@ public class OIDCRPFlowTest extends AbstractAuthnXmlFlowExecutionTests {
         updateFlowExecution(flowExecution);
         flowExecution.start(inputMap, externalContext);    
         assertCurrentStateEquals("AuthnRequest");
+        
+        mockOPServer.shutdown();
     }
     
     /**
@@ -526,6 +580,8 @@ public class OIDCRPFlowTest extends AbstractAuthnXmlFlowExecutionTests {
         updateFlowExecution(flowExecution);
         flowExecution.start(inputMap, externalContext);    
         assertCurrentStateEquals("AuthnRequest");
+        
+        mockOPServer.shutdown();
     }
     
     /**
@@ -564,6 +620,8 @@ public class OIDCRPFlowTest extends AbstractAuthnXmlFlowExecutionTests {
         updateFlowExecution(flowExecution);
         flowExecution.start(inputMap, externalContext);    
         assertCurrentStateEquals("AuthnRequest");
+        
+        mockOPServer.shutdown();
     }
     
     @Test
@@ -604,55 +662,8 @@ public class OIDCRPFlowTest extends AbstractAuthnXmlFlowExecutionTests {
         updateFlowExecution(flowExecution);
         flowExecution.start(inputMap, externalContext);    
         assertCurrentStateEquals("AuthnRequest");
-    }
-    
-    /**
-     * Create a basic security configuration, which can be overriden per test if required.
-     * 
-
-     * @return the basic security configuration.
-     */
-    private OIDCSecurityConfiguration createBasicSecurityConfigAndValidationParams() {
-
-        final var securityConfig = new OIDCSecurityConfiguration();
-        
-        final var idTokenSigValConfig = new BasicSignatureValidationConfiguration<SignedJWT>();
-        idTokenSigValConfig.setSignatureTrustEngine(
-                new ExplicitKeySignedJWTTrustEngine(new CriterionCredentialResolver(), 
-                        new BasicJOSEObjectCredentialResolver()));
-        securityConfig.setIdTokenJwtSignatureValidationConfiguration(idTokenSigValConfig);
-        
-        final var userInfoTokenSigValConfig = new BasicSignatureValidationConfiguration<SignedJWT>();
-        userInfoTokenSigValConfig.setSignatureTrustEngine(
-                new ExplicitKeySignedJWTTrustEngine(new CriterionCredentialResolver(), 
-                        new BasicJOSEObjectCredentialResolver()));
-        securityConfig.setUserInfoTokenJwtSignatureValidationConfiguration(userInfoTokenSigValConfig);
-        
-        //The CEK resolver just resolves keys from the criteria set.
-        final var idTokenDecryptConfig = new BasicJWTDecryptionConfiguration();        
-        idTokenDecryptConfig.setContentEncryptionKeyCredentialResolver(new CriterionCredentialResolver());
-        securityConfig.setIdTokenJwtDecryptionConfiguration(idTokenDecryptConfig);
-        
-        final var userInfoDecryptConfig = new BasicJWTDecryptionConfiguration();        
-        userInfoDecryptConfig.setContentEncryptionKeyCredentialResolver(new CriterionCredentialResolver());
-        securityConfig.setUserInfoJwtDecryptionConfiguration(userInfoDecryptConfig);
         
-        
-        return securityConfig;
-    }
-    
-    private void assertStandardSuccessConditions(final ProfileRequestContext prc) {
-        //assert success conditions. 
-        assertFlowExecutionEnded();
-        assertNotNull(prc.getSubcontext(AuthenticationContext.class));
-        assertNotNull(prc.getSubcontext(SubjectCanonicalizationContext.class));
-        assertNotNull(prc.getSubcontext(SubjectCanonicalizationContext.class).getSubject().getPrincipals());
-        //As SimpleSubjectCanonicalization has not been run, we pull out the subject
-        final UsernamePrincipal usernamePrincipal = 
-                prc.getSubcontext(SubjectCanonicalizationContext.class).getSubject()
-                .getPrincipals(UsernamePrincipal.class).iterator().next();
-        assertNotNull(usernamePrincipal);
-        assertEquals(usernamePrincipal.getName(),"jdoe");
+        mockOPServer.shutdown();
     }
     
     
@@ -703,8 +714,9 @@ public class OIDCRPFlowTest extends AbstractAuthnXmlFlowExecutionTests {
         mockOPServer.shutdown();
         
         assertStandardSuccessConditions(prc);
-      
-       
+        
+        mockOPServer.shutdown();
+           
     }
     
     /** 
@@ -758,6 +770,7 @@ public class OIDCRPFlowTest extends AbstractAuthnXmlFlowExecutionTests {
         assertNotNull(prc.getSubcontext(AuthenticationContext.class));
         assertNull(prc.getSubcontext(SubjectCanonicalizationContext.class));      
        
+        mockOPServer.shutdown();
     }
     
     
@@ -804,8 +817,8 @@ public class OIDCRPFlowTest extends AbstractAuthnXmlFlowExecutionTests {
         
         mockOPServer.shutdown();
         
-        assertStandardSuccessConditions(prc);    
-       
+        assertStandardSuccessConditions(prc);   
+        
     }
     
     @Test 
@@ -930,8 +943,7 @@ public class OIDCRPFlowTest extends AbstractAuthnXmlFlowExecutionTests {
         mockOPServer.shutdown();
         
         assertStandardSuccessConditions(prc);
-      
-       
+               
     }
     
     /**
diff --git a/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/messaging/impl/SignRequestObjectTest.java b/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/messaging/impl/SignRequestObjectTest.java
index 7ed4488..a19fc61 100644
--- a/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/messaging/impl/SignRequestObjectTest.java
+++ b/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/messaging/impl/SignRequestObjectTest.java
@@ -1,17 +1,34 @@
+/*
+ * 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.messaging.impl;
 
-import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.fail;
 
+import java.text.ParseException;
 import java.util.Date;
 
 import javax.annotation.Nonnull;
 
-import org.opensaml.messaging.handler.MessageHandlerException;
 import org.opensaml.xmlsec.SignatureSigningParameters;
+import org.testng.AssertJUnit;
 import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
-import com.nimbusds.jose.JOSEException;
 import com.nimbusds.jose.JWSAlgorithm;
 import com.nimbusds.jose.jwk.Curve;
 import com.nimbusds.jose.jwk.ECKey;
@@ -30,11 +47,15 @@ import net.shibboleth.idp.plugin.authn.oidc.rp.impl.TestCredentialHelper;
 import net.shibboleth.oidc.profile.core.OIDCAuthenticationRequest;
 import net.shibboleth.oidc.security.context.JWTSecurityParametersContext;
 
-/** Tests for the SignRequestObject message handler.*/
+/** 
+ * Tests for the SignRequestObject message handler. 
+ * 
+ * <p>Note, These tests sign a RequestObject. </p>
+ */
 public class SignRequestObjectTest extends AbstractOIDCTest {
     
     /** A client_secret to use.*/
-    @Nonnull private final String CLIENT_SECRET = "Xp2s5v8y/B?E(H+MbQeThWmYq3t6w9z$";
+    @Nonnull private static final String CLIENT_SECRET = "Xp2s5v8y/B?E(H+MbQeThWmYq3t6w9z$";
     
     /** The signer to test.*/
     private SignRequestObject signer;
@@ -48,6 +69,20 @@ public class SignRequestObjectTest extends AbstractOIDCTest {
         super.setup();
         signer = new SignRequestObject();
         
+        signer.setClaimsToSignLookupStrategy(mc -> {
+            final OIDCAuthenticationRequest ar = (OIDCAuthenticationRequest)mc.getMessage();
+            try {
+                return ar.getRequestObject().getJWTClaimsSet();
+            } catch (final ParseException e) {
+                fail();                
+            }
+            return null;
+        });
+        signer.setJwtUpdateConsumer((jwt, mc) -> {
+            final OIDCAuthenticationRequest ar = (OIDCAuthenticationRequest)mc.getMessage();
+            ar.setRequestObject(jwt);
+        });
+        
         request = new OIDCAuthenticationRequest(new ClientID("test-client"));
         final JWTClaimsSet claims = new JWTClaimsSet.Builder()
                 .issuer("test-client")
@@ -69,11 +104,12 @@ public class SignRequestObjectTest extends AbstractOIDCTest {
         secParamCtx.setSignatureSigningParameters(params);        
         prc.getOutboundMessageContext().addSubcontext(secParamCtx);
         
+        signer.initialize();
         signer.invoke(prc.getOutboundMessageContext());
         final JWT jwt = request.getRequestObject();
-        assertTrue(jwt instanceof SignedJWT);
+        AssertJUnit.assertTrue(jwt instanceof SignedJWT);
         final var signedJWT = (SignedJWT)jwt;
-        assertTrue(JWSAlgorithm.Family.HMAC_SHA.contains(signedJWT.getHeader().getAlgorithm()));
+        AssertJUnit.assertTrue(JWSAlgorithm.Family.HMAC_SHA.contains(signedJWT.getHeader().getAlgorithm()));
     }
     
     @Test(expectedExceptions = Exception.class)
@@ -86,15 +122,16 @@ public class SignRequestObjectTest extends AbstractOIDCTest {
         secParamCtx.setSignatureSigningParameters(params);        
         prc.getOutboundMessageContext().addSubcontext(secParamCtx);
         
+        signer.initialize();
         signer.invoke(prc.getOutboundMessageContext());
         final JWT jwt = request.getRequestObject();
-        assertTrue(jwt instanceof SignedJWT);
+        AssertJUnit.assertTrue(jwt instanceof SignedJWT);
         final var signedJWT = (SignedJWT)jwt;
-        assertTrue(JWSAlgorithm.Family.HMAC_SHA.contains(signedJWT.getHeader().getAlgorithm()));
+        AssertJUnit.assertTrue(JWSAlgorithm.Family.HMAC_SHA.contains(signedJWT.getHeader().getAlgorithm()));
     }
     
     @Test
-    public void testSignRS256_Success() throws MessageHandlerException, JOSEException {
+    public void testSignRS256_Success() throws Exception {
         
         final JWTSecurityParametersContext secParamCtx = new JWTSecurityParametersContext();
         final var params = new SignatureSigningParameters();
@@ -107,15 +144,16 @@ public class SignRequestObjectTest extends AbstractOIDCTest {
         secParamCtx.setSignatureSigningParameters(params);        
         prc.getOutboundMessageContext().addSubcontext(secParamCtx);
         
+        signer.initialize();
         signer.invoke(prc.getOutboundMessageContext());
         final JWT jwt = request.getRequestObject();
-        assertTrue(jwt instanceof SignedJWT);
+        AssertJUnit.assertTrue(jwt instanceof SignedJWT);
         final var signedJWT = (SignedJWT)jwt;
-        assertTrue(JWSAlgorithm.Family.RSA.contains(signedJWT.getHeader().getAlgorithm()));
+        AssertJUnit.assertTrue(JWSAlgorithm.Family.RSA.contains(signedJWT.getHeader().getAlgorithm()));
     }
     
     @Test
-    public void testSignES256_Success() throws MessageHandlerException, JOSEException {
+    public void testSignES256_Success() throws Exception {
         
         final JWTSecurityParametersContext secParamCtx = new JWTSecurityParametersContext();
         final var params = new SignatureSigningParameters();
@@ -128,11 +166,12 @@ public class SignRequestObjectTest extends AbstractOIDCTest {
         secParamCtx.setSignatureSigningParameters(params);        
         prc.getOutboundMessageContext().addSubcontext(secParamCtx);
         
+        signer.initialize();
         signer.invoke(prc.getOutboundMessageContext());
         final JWT jwt = request.getRequestObject();
-        assertTrue(jwt instanceof SignedJWT);
+        AssertJUnit.assertTrue(jwt instanceof SignedJWT);
         final var signedJWT = (SignedJWT)jwt;
-        assertTrue(JWSAlgorithm.Family.EC.contains(signedJWT.getHeader().getAlgorithm()));
+        AssertJUnit.assertTrue(JWSAlgorithm.Family.EC.contains(signedJWT.getHeader().getAlgorithm()));
     }
 
 }

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


More information about the commits mailing list