[java-idp-plugin-oidc-rp] branch main updated: Improve Javadoc. Some code cleanup.

Phil Smart philip.smart at jisc.ac.uk
Wed Oct 19 16:16:39 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=50026c1c36db9c44fe88664cf417722674b4a612

The following commit(s) were added to refs/heads/main by this push:
     new 50026c1  Improve Javadoc. Some code cleanup.
50026c1 is described below

commit 50026c1c36db9c44fe88664cf417722674b4a612
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Wed Oct 19 17:16:33 2022 +0100

    Improve Javadoc. Some code cleanup.
---
 .../impl/AbstractHttpOIDCAuthenticationAction.java |   2 +
 .../AbstractOIDCAuthenticationRequestAction.java   | 200 ---------------------
 .../authn/oidc/rp/impl/BuildRequestObject.java     | 117 ++++++++++--
 .../idp/plugin/authn/oidc/rp/impl/DecryptJWT.java  |   5 +
 .../oidc/rp/impl/ExchangeCodeForAccessToken.java   |   3 +-
 .../oidc/rp/impl/ExtractIDTokenFromResponse.java   |  21 ++-
 .../rp/impl/InitializeAuthorizationRequest.java    |   4 +
 ...nitializeOAuth2ClientAuthenticationContext.java |  21 ++-
 .../rp/impl/InitializeOAuth2ClientContext.java     |  14 +-
 .../authn/oidc/rp/impl/OIDCProxySupport.java       |   5 +-
 .../rp/impl/PopulateJWTDecryptionParameters.java   |   1 +
 .../rp/impl/PopulateJWTEncryptionParameters.java   |   2 +-
 .../rp/impl/PrepareOIDCInboundMessageContext.java  |   4 +-
 .../authn/oidc/rp/impl/ProcessEndUserClaims.java   |  11 +-
 .../rp/impl/UnsupportedResponseTypeAction.java     |   2 +-
 .../authn/oidc/rp/impl/UserInfoEndpointLookup.java |   5 +
 .../ValidateExternalAuthenticationContext.java     |   1 -
 .../rp/impl/ValidateOAuthAccessTokenResponse.java  |   6 +-
 .../authn/oidc/rp/impl/ValidateResponseState.java  |  10 +-
 .../authn/oidc/rp/impl/ValidateTokenClaims.java    |  32 +---
 .../rp/impl/ValidateUserInfoJSONObjectClaims.java  |   4 +
 ...actOIDCAuthenticationRequestMessageHandler.java |  42 ++---
 .../authn/oidc/rp/messaging/impl/EncryptJWT.java   |  25 ++-
 .../authn/oidc/rp/messaging/impl/SignJWT.java      |  14 +-
 .../oidc-relying-party-authn-beans.xml             |   9 +-
 25 files changed, 234 insertions(+), 326 deletions(-)

diff --git a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/AbstractHttpOIDCAuthenticationAction.java b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/AbstractHttpOIDCAuthenticationAction.java
index 3ce3c36..046d13e 100644
--- a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/AbstractHttpOIDCAuthenticationAction.java
+++ b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/AbstractHttpOIDCAuthenticationAction.java
@@ -35,6 +35,7 @@ import org.slf4j.LoggerFactory;
 
 import net.shibboleth.idp.plugin.authn.oidc.rp.OIDCRPException;
 import net.shibboleth.utilities.java.support.annotation.constraint.NonnullAfterInit;
+import net.shibboleth.utilities.java.support.annotation.constraint.ThreadSafeAfterInit;
 import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
 import net.shibboleth.utilities.java.support.component.ComponentSupport;
 import net.shibboleth.utilities.java.support.logic.Constraint;
@@ -44,6 +45,7 @@ import net.shibboleth.utilities.java.support.logic.Constraint;
  * 
  * @param <T> the response type of the object returned as a result of the request.
  */
+ at ThreadSafeAfterInit
 public abstract class AbstractHttpOIDCAuthenticationAction<T> extends AbstractOIDCAuthenticationResponseAction {
     
     /** Class logger.*/
diff --git a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/AbstractOIDCAuthenticationRequestAction.java b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/AbstractOIDCAuthenticationRequestAction.java
deleted file mode 100644
index 6727aa2..0000000
--- a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/AbstractOIDCAuthenticationRequestAction.java
+++ /dev/null
@@ -1,200 +0,0 @@
-/*
- * 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.Function;
-
-import javax.annotation.Nonnull;
-import javax.annotation.Nullable;
-
-import org.opensaml.messaging.context.MessageContext;
-import org.opensaml.messaging.context.navigate.ChildContextLookup;
-import org.opensaml.profile.action.ActionSupport;
-import org.opensaml.profile.action.EventIds;
-import org.opensaml.profile.context.ProfileRequestContext;
-import org.opensaml.profile.context.navigate.OutboundMessageContextLookup;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import com.nimbusds.openid.connect.sdk.op.OIDCProviderMetadata;
-
-import net.shibboleth.idp.authn.AbstractAuthenticationAction;
-import net.shibboleth.idp.authn.context.AuthenticationContext;
-import net.shibboleth.idp.plugin.authn.oidc.rp.context.OIDCPeerEntityContext;
-import net.shibboleth.idp.profile.IdPEventIds;
-import net.shibboleth.idp.profile.context.RelyingPartyContext;
-import net.shibboleth.oidc.metadata.context.OIDCProviderMetadataContext;
-import net.shibboleth.oidc.profile.config.OIDCAuthorizationConfiguration;
-import net.shibboleth.oidc.profile.core.OIDCAuthenticationRequest;
-import net.shibboleth.utilities.java.support.component.ComponentSupport;
-import net.shibboleth.utilities.java.support.logic.Constraint;
-
-/**
- * 
- * Abstract class for actions performing operations on a {@link OIDCAuthenticationRequest} located under
- * the outbound message context.
- * 
- * <p>Makes available the OpenID Provider metadata context, the applicable profile configuration, and the
- * in-building authentication request.</p> 
- * 
- */
- at Deprecated 
-//because we message handlers to handle the authn request?
-public abstract class AbstractOIDCAuthenticationRequestAction extends AbstractAuthenticationAction {
-
-    /** Class logger. */
-    @Nonnull private final Logger log = LoggerFactory.getLogger(AbstractOIDCAuthenticationRequestAction.class);
-    
-    /** Lookup strategy to locate the OpenID Provider metadata to use.*/
-    @Nonnull private Function<ProfileRequestContext, OIDCProviderMetadataContext> providerMetadataLookupStrategy;
-    
-    /** Lookup function for relying party context. */
-    @Nonnull private Function<ProfileRequestContext,RelyingPartyContext> relyingPartyContextLookupStrategy;
-        
-    /** OIDC authentication request built by the IdP. */
-    @Nullable private OIDCAuthenticationRequest authnRequest;
-    
-    /** OpenID Provider metadata .*/
-    @Nullable private OIDCProviderMetadata providerMetadata;
-    
-    /** Applicable profile configuration. */
-    @Nullable private OIDCAuthorizationConfiguration profileConfiguration;
-    
-    /** Constructor.*/
-    protected AbstractOIDCAuthenticationRequestAction() {
-        providerMetadataLookupStrategy = new ChildContextLookup<>(OIDCProviderMetadataContext.class).compose(
-                new ChildContextLookup<>(OIDCPeerEntityContext.class).compose(
-                        new OutboundMessageContextLookup()));
-        
-        relyingPartyContextLookupStrategy = new ChildContextLookup<>(RelyingPartyContext.class);
-    }
-    
-    /**
-     * Set the strategy used to locate the {@link RelyingPartyContext} associated with a given
-     * {@link ProfileRequestContext}.
-     * 
-     * @param strategy lookup strategy
-     */
-    public void setRelyingPartyContextLookupStrategy(
-            @Nonnull final Function<ProfileRequestContext,RelyingPartyContext> strategy) {
-        ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
-        
-        relyingPartyContextLookupStrategy =
-                Constraint.isNotNull(strategy, "RelyingPartyContext lookup strategy cannot be null");
-    }
-    
-    /**
-     * Set the lookup strategy to locate the OpenID providers metadata.
-     * 
-     * @param strategy the strategy.
-     */
-    public void setProviderMetadataLookupStrategy(
-            @Nonnull final Function<ProfileRequestContext, OIDCProviderMetadataContext> strategy) {
-        ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
-        
-        providerMetadataLookupStrategy = 
-                Constraint.isNotNull(strategy,"Provider metadata lookup strategy can not be null");
-    }
-
-
-    /**
-     * Returns the authentication request build by this IdP. Should never be 
-     * {@code null} after {@link #doPreExecute(ProfileRequestContext)} has been called.
-     * 
-     * @return the authentication request.
-     */
-    @Nullable protected OIDCAuthenticationRequest getAuthenticationRequest() {
-        return authnRequest;
-    }
-    
-    /**
-     * Returns the profile configuration associated with this request.Should never be 
-     * {@code null} after {@link #doPreExecute(ProfileRequestContext)} has been called.
-     * 
-     * @return the profile configuration
-     */
-    @Nullable protected OIDCAuthorizationConfiguration getProfileConfiguration() {
-        return profileConfiguration;
-    }
-
-    
-    /**
-     * Returns the OpenID Provider metadata. Should never be {@code null} after
-     * {@link #doPreExecute(ProfileRequestContext)} has been called.
-     * 
-     * @return The provider metadata context.
-     */
-    @Nullable protected OIDCProviderMetadata getProviderMetadata() {
-        return providerMetadata;
-    }
-
-    @Override
-    protected boolean doPreExecute(@Nonnull final ProfileRequestContext profileRequestContext,
-            @Nonnull final AuthenticationContext authenticationContext) {
-
-//
-//        final MessageContext inboundMessageCtx = profileRequestContext.getInboundMessageContext();
-//        if (inboundMessageCtx == null) {
-//            log.error("{} No inbound message context", getLogPrefix());
-//            ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_MSG_CTX);
-//            return false;
-//        }
-       
-        final MessageContext outboundMsgContext = profileRequestContext.getOutboundMessageContext();
-        if (outboundMsgContext == null) {
-            log.error("{} Outbound message context was null", getLogPrefix());
-            ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_MSG_CTX);
-            return false;
-        } 
-        if (!(outboundMsgContext.getMessage() instanceof OIDCAuthenticationRequest)) {
-            log.error("{} Outbound message was not an authentication request", getLogPrefix());
-            ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_MSG_CTX);
-            return false;
-        }
-        authnRequest = (OIDCAuthenticationRequest) outboundMsgContext.getMessage();
-        
-        final OIDCProviderMetadataContext providerMetadataContext = 
-                providerMetadataLookupStrategy.apply(profileRequestContext);
-        if (providerMetadataContext == null) {
-            log.error("{} No provider metadata context found for peer", getLogPrefix());
-            ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_PROFILE_CTX);
-            return false;
-        }
-        
-        providerMetadata = providerMetadataContext.getProviderInformation();
-        if (providerMetadata == null) {
-            log.error("{} No provider metadata found for peer", getLogPrefix());
-            ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_PROFILE_CTX);
-            return false;
-        }
-        
-        final RelyingPartyContext rpCtx = relyingPartyContextLookupStrategy.apply(profileRequestContext);
-        if (rpCtx != null && rpCtx.getConfiguration() != null &&
-                rpCtx.getProfileConfig() instanceof OIDCAuthorizationConfiguration) {
-            profileConfiguration = (OIDCAuthorizationConfiguration) rpCtx.getProfileConfig();
-        }
-        if (profileConfiguration == null) {
-            log.error("{} OIDCCoreProtocolConfiguration not found", getLogPrefix());
-            ActionSupport.buildEvent(profileRequestContext, IdPEventIds.INVALID_PROFILE_CONFIG);
-            return false;
-        }
-        
-        return true;
-    }
-
-}
diff --git a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/BuildRequestObject.java b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/BuildRequestObject.java
index 37ee5f6..474becf 100644
--- a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/BuildRequestObject.java
+++ b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/BuildRequestObject.java
@@ -17,13 +17,18 @@
 
 package net.shibboleth.idp.plugin.authn.oidc.rp.impl;
 
+import java.util.function.Function;
 import java.util.function.Predicate;
 
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
+import org.opensaml.messaging.context.MessageContext;
+import org.opensaml.messaging.context.navigate.ChildContextLookup;
 import org.opensaml.profile.action.ActionSupport;
+import org.opensaml.profile.action.EventIds;
 import org.opensaml.profile.context.ProfileRequestContext;
+import org.opensaml.profile.context.navigate.OutboundMessageContextLookup;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -32,27 +37,46 @@ import com.nimbusds.jwt.JWT;
 import com.nimbusds.oauth2.sdk.id.Audience;
 import com.nimbusds.oauth2.sdk.id.Issuer;
 import com.nimbusds.openid.connect.sdk.claims.ClaimsSet;
+import com.nimbusds.openid.connect.sdk.op.OIDCProviderMetadata;
 
+import net.shibboleth.idp.authn.AbstractAuthenticationAction;
 import net.shibboleth.idp.authn.AuthnEventIds;
 import net.shibboleth.idp.authn.context.AuthenticationContext;
+import net.shibboleth.idp.plugin.authn.oidc.rp.context.OIDCPeerEntityContext;
 import net.shibboleth.idp.profile.context.RelyingPartyContext;
+import net.shibboleth.oidc.metadata.context.OIDCProviderMetadataContext;
 import net.shibboleth.oidc.profile.core.OIDCAuthenticationRequest;
 import net.shibboleth.utilities.java.support.component.ComponentSupport;
+import net.shibboleth.utilities.java.support.logic.Constraint;
 
 
 /**
- * Action that creates a Request Object {@link JWT}, and sets it to work context
+ * Action that creates a Request Object {@link JWT}, and sets it to the work context
  * {@link OIDCAuthenticationRequest} located under {@link ProfileRequestContext#getOutboundMessageContext()}.
  * 
- * <p>Note, some parameters are set downstream before the request object is signed and or encrypted. These
- * parameters are only available to the Http Controller e.g. state, and must be set during the external
+ * <p>Note, some parameters are set downstream in the flow before the request object is signed and or encrypted. 
+ * These parameters are only available to the HTTP Controller e.g. state, and must be set during the external
  * authentication redirect.</p>
+ * 
+ * @event {@link org.opensaml.profile.action.EventIds#PROCEED_EVENT_ID}
+ * @event {@link EventIds#INVALID_MSG_CTX}
+ * @event {@link EventIds#INVALID_PROFILE_CTX}
+ * @event {@link AuthnEventIds#INVALID_AUTHN_CTX}
+ * @pre <pre>ProfileRequestContext.getOutboundMessageContext().getMessage() 
+ * instance of OIDCAuthenticationRequest.class</pre>
+ * @post Add a JWT request object to the in-flight authentication request 
  */
-public class BuildRequestObject extends AbstractOIDCAuthenticationRequestAction {
+public class BuildRequestObject extends AbstractAuthenticationAction {
     
     /** Class logger. */
     @Nonnull private final Logger log = LoggerFactory.getLogger(BuildRequestObject.class);
     
+    /** Lookup strategy to locate the OpenID Provider metadata to use.*/
+    @Nonnull private Function<ProfileRequestContext, OIDCProviderMetadataContext> providerMetadataLookupStrategy;
+    
+    /** Lookup function for relying party context. */
+    @Nonnull private Function<ProfileRequestContext,RelyingPartyContext> relyingPartyContextLookupStrategy;
+    
     /** A hook to allow additional checking of the request object claims after it is built.*/
     @Nonnull private Predicate<ClaimsSet> claimsSetIsValidPredicate;
     
@@ -63,13 +87,50 @@ public class BuildRequestObject extends AbstractOIDCAuthenticationRequestAction
      */
     @Nonnull private Predicate<ProfileRequestContext> requestObjectToBeSignedPredicate;
     
-    /** The RelyingPartyContext to operate on. */
-    @Nullable private RelyingPartyContext rpCtx;
+    /** OIDC authentication request built by the IdP. */
+    @Nullable private OIDCAuthenticationRequest authnRequest;
+    
+    /** OpenID Provider metadata .*/
+    @Nullable private OIDCProviderMetadata providerMetadata;
     
     /** Constructor.*/
     public BuildRequestObject() {
         claimsSetIsValidPredicate = Predicates.alwaysTrue();
         requestObjectToBeSignedPredicate = Predicates.alwaysTrue();
+        
+        providerMetadataLookupStrategy = new ChildContextLookup<>(OIDCProviderMetadataContext.class).compose(
+                new ChildContextLookup<>(OIDCPeerEntityContext.class).compose(
+                        new OutboundMessageContextLookup()));
+        
+        relyingPartyContextLookupStrategy = new ChildContextLookup<>(RelyingPartyContext.class);
+    }
+    
+    
+    /**
+     * Set the strategy used to locate the {@link RelyingPartyContext} associated with a given
+     * {@link ProfileRequestContext}.
+     * 
+     * @param strategy lookup strategy
+     */
+    public void setRelyingPartyContextLookupStrategy(
+            @Nonnull final Function<ProfileRequestContext,RelyingPartyContext> strategy) {
+        ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+        
+        relyingPartyContextLookupStrategy =
+                Constraint.isNotNull(strategy, "RelyingPartyContext lookup strategy cannot be null");
+    }
+    
+    /**
+     * Set the lookup strategy to locate the OpenID providers metadata.
+     * 
+     * @param strategy the strategy.
+     */
+    public void setProviderMetadataLookupStrategy(
+            @Nonnull final Function<ProfileRequestContext, OIDCProviderMetadataContext> strategy) {
+        ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+        
+        providerMetadataLookupStrategy = 
+                Constraint.isNotNull(strategy,"Provider metadata lookup strategy can not be null");
     }
     
     /**
@@ -102,21 +163,55 @@ public class BuildRequestObject extends AbstractOIDCAuthenticationRequestAction
             requestObjectToBeSignedPredicate = predicate;
         }
     }
+    
+    @Override
+    protected boolean doPreExecute(@Nonnull final ProfileRequestContext profileRequestContext,
+            @Nonnull final AuthenticationContext authenticationContext) {
+       
+        final MessageContext outboundMsgContext = profileRequestContext.getOutboundMessageContext();
+        if (outboundMsgContext == null) {
+            log.error("{} Outbound message context was null", getLogPrefix());
+            ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_MSG_CTX);
+            return false;
+        } 
+        if (!(outboundMsgContext.getMessage() instanceof OIDCAuthenticationRequest)) {
+            log.error("{} Outbound message was not an authentication request", getLogPrefix());
+            ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_MSG_CTX);
+            return false;
+        }
+        authnRequest = (OIDCAuthenticationRequest) outboundMsgContext.getMessage();
+        
+        final OIDCProviderMetadataContext providerMetadataContext = 
+                providerMetadataLookupStrategy.apply(profileRequestContext);
+        if (providerMetadataContext == null) {
+            log.error("{} No provider metadata context found for peer", getLogPrefix());
+            ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_PROFILE_CTX);
+            return false;
+        }
+        
+        providerMetadata = providerMetadataContext.getProviderInformation();
+        if (providerMetadata == null) {
+            log.error("{} No provider metadata found for peer", getLogPrefix());
+            ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_PROFILE_CTX);
+            return false;
+        }
+        
+        return true;
+    }
 
     /** {@inheritDoc} */
     @Override
     protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext,
             @Nonnull final AuthenticationContext authenticationContext) {
         
-        log.debug("{} Building a plain RequestObject JWT", getLogPrefix());
+        log.debug("{} Building a plain RequestObject JWT", getLogPrefix());        
         
-        final OIDCAuthenticationRequest authnRequest = getAuthenticationRequest();        
         final ClaimsSet requestObjectClaims = new ClaimsSet();     
         
         if (requestObjectToBeSignedPredicate.test(profileRequestContext)) {
-            if (getProviderMetadata().getIssuer() != null) {
+            if (providerMetadata.getIssuer() != null) {
                 requestObjectClaims.setAudience(
-                        new Audience(getProviderMetadata().getIssuer().getValue()));                
+                        new Audience(providerMetadata.getIssuer().getValue()));                
             } else {
                 // Should never happen
                 log.error("{} Signed RequestObject requires 'iss' claim, which is currently null",
@@ -127,7 +222,7 @@ public class BuildRequestObject extends AbstractOIDCAuthenticationRequestAction
             requestObjectClaims.setIssuer(new Issuer(authnRequest.getClientID().getValue())); 
         }        
         
-        requestObjectClaims.setClaim("client_id", getAuthenticationRequest().getClientID().toString());  
+        requestObjectClaims.setClaim("client_id", authnRequest.getClientID().toString());  
 
         setClaimIfPresent(requestObjectClaims, "nonce", authnRequest.getNonce());        
         setClaimIfPresent(requestObjectClaims, "response_type", authnRequest.getResponseType());
diff --git a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/DecryptJWT.java b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/DecryptJWT.java
index 5afc792..982e706 100644
--- a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/DecryptJWT.java
+++ b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/DecryptJWT.java
@@ -48,6 +48,11 @@ import net.shibboleth.utilities.java.support.logic.Constraint;
 
 /** 
  * Decrypt the located JWE using the decryption parameters stored in the security context.
+ *
+ * @event {@link org.opensaml.profile.action.EventIds#PROCEED_EVENT_ID}
+ * @event {@link EventIds#INVALID_SEC_CFG}
+ * @event {@link OidcEventIds#INVALID_ID_TOKEN}
+ * @post Decrypt a JWT and add it back to via an update strategy.
  */
 public class DecryptJWT extends AbstractProfileAction {
     
diff --git a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ExchangeCodeForAccessToken.java b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ExchangeCodeForAccessToken.java
index 1879970..98c33ce 100644
--- a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ExchangeCodeForAccessToken.java
+++ b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ExchangeCodeForAccessToken.java
@@ -40,8 +40,7 @@ import net.shibboleth.utilities.java.support.logic.Constraint;
 
 /**
  * Action to exchange the authorization code in the authentication response for an OAuth access token which 
- * contains an OIDC id_token. Once obtained, adds the token to the {@link AccessTokenResponseContext}.
- * 
+ * contains an OIDC id_token. Once obtained, adds the token to the {@link AccessTokenResponseContext}. * 
  * 
  * @event {@link org.opensaml.profile.action.EventIds#PROCEED_EVENT_ID}
  * @event {@link EventIds#INVALID_PROFILE_CTX}
diff --git a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ExtractIDTokenFromResponse.java b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ExtractIDTokenFromResponse.java
index cb88676..b6d7557 100644
--- a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ExtractIDTokenFromResponse.java
+++ b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ExtractIDTokenFromResponse.java
@@ -46,8 +46,15 @@ import net.shibboleth.utilities.java.support.component.ComponentInitializationEx
 import net.shibboleth.utilities.java.support.component.ComponentSupport;
 import net.shibboleth.utilities.java.support.logic.Constraint;
 
-/** Action that extracts an id_token from the access token response from the
- *  inbound message context and sets it onto a TODO context.*/
+/** 
+ * Action that extracts an id_token from the access token response and sets it onto the 
+ * {@link AccessTokenResponseContext}.
+ * 
+ * @event {@link org.opensaml.profile.action.EventIds#PROCEED_EVENT_ID}
+ * @event {@link EventIds#INVALID_PROFILE_CTX}
+ * @event {@link OidcEventIds#INVALID_ID_TOKEN}
+ * @post id_token added to {@link AccessTokenResponseContext}.
+ */
 public class ExtractIDTokenFromResponse extends AbstractProfileAction {
 
     /** Class logger. */
@@ -92,7 +99,8 @@ public class ExtractIDTokenFromResponse extends AbstractProfileAction {
         }
     }
     
-    @Override protected boolean doPreExecute(@Nonnull final ProfileRequestContext profileRequestContext) {
+    @Override 
+    protected boolean doPreExecute(@Nonnull final ProfileRequestContext profileRequestContext) {
         
         responseCtx = tokenResponseContextLookupStrategy.apply(profileRequestContext);
         if (responseCtx == null) {
@@ -119,20 +127,23 @@ public class ExtractIDTokenFromResponse extends AbstractProfileAction {
         
         final String rawIdTokenValue = rawIdTokenLookupStrategy.apply(profileRequestContext);
         if (rawIdTokenValue == null) {
-            log.warn("{} Unable to lookup id_token",getLogPrefix());
+            log.warn("{} Unable to extract id_token from access token response",getLogPrefix());
             ActionSupport.buildEvent(profileRequestContext,OidcEventIds.INVALID_ID_TOKEN);
             return;
         }
-        log.trace("{} (Assumed) Base64 encoded id_token is '{}'",getLogPrefix(), rawIdTokenValue);
+        log.trace("{} Encoded id_token is: '{}'",getLogPrefix(), rawIdTokenValue);
         try {
             final JOSEObject joseObject = JOSEObject.parse(rawIdTokenValue);
             if (joseObject instanceof PlainObject) {
+                log.trace("{} Plain id_token found", getLogPrefix());
                 responseCtx.setIdToken(PlainJWT.parse(rawIdTokenValue));
                 
             } else if (joseObject instanceof JWSObject) {
+                log.trace("{} Signed id_token found", getLogPrefix());
                 responseCtx.setIdToken(SignedJWT.parse(rawIdTokenValue));
                 
             } else if (joseObject instanceof JWEObject) {
+                log.trace("{} Encrypted id_token found", getLogPrefix());
                 responseCtx.setIdToken(EncryptedJWT.parse(rawIdTokenValue));
             }            
         } catch (final ParseException e) {
diff --git a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/InitializeAuthorizationRequest.java b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/InitializeAuthorizationRequest.java
index 3585ec6..72c1ef3 100644
--- a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/InitializeAuthorizationRequest.java
+++ b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/InitializeAuthorizationRequest.java
@@ -42,6 +42,10 @@ import net.shibboleth.utilities.java.support.logic.Constraint;
 /**
  * And action that creates an {@link OIDCAuthenticationRequest} shell to populate in future steps,
  * and sets it to the outbound message context.
+ * 
+ * @event {@link org.opensaml.profile.action.EventIds#PROCEED_EVENT_ID}
+ * @event {@link IdPEventIds.INVALID_PROFILE_CONFIG}
+ * @post Add an {@link OIDCAuthenticationRequest} as the message of the outbound context.
  */
 public class InitializeAuthorizationRequest extends AbstractProfileAction {
     
diff --git a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/InitializeOAuth2ClientAuthenticationContext.java b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/InitializeOAuth2ClientAuthenticationContext.java
index fbdb63c..2e888e0 100644
--- a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/InitializeOAuth2ClientAuthenticationContext.java
+++ b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/InitializeOAuth2ClientAuthenticationContext.java
@@ -56,8 +56,14 @@ import net.shibboleth.utilities.java.support.component.ComponentSupport;
 import net.shibboleth.utilities.java.support.logic.Constraint;
 
 /**
- * An {@link AbstractProfileAction action} that resolves the client authentication method for the chosen 
- * downstream provider (issuer). 
+ * An {@link AbstractProfileAction action} that resolves the Client Authentication method for the chosen 
+ * upstream OpenID Provider (issuer) from the profile configuration.
+ * 
+ * @event {@link org.opensaml.profile.action.EventIds#PROCEED_EVENT_ID}
+ * @event {@link IdPEventIds.INVALID_PROFILE_CONFIG}
+ * @event {@link EventIds#INVALID_PROFILE_CTX}
+ * @event {@link IdPEventIds#INVALID_RELYING_PARTY_CONFIG}
+ * @post Add the {@link ClientAuthenticationMethod} to the {@link OAuth2ClientAuthenticationContext}
  */
 //TODO simply some of these with a base class e.g. client context lookups.
 public class InitializeOAuth2ClientAuthenticationContext extends AbstractProfileAction {
@@ -166,8 +172,8 @@ public class InitializeOAuth2ClientAuthenticationContext extends AbstractProfile
         
         final Set<String> clientAuthMethods = 
                 profileConfiguration.getTokenEndpointAuthMethods(profileRequestContext);
-        if (clientAuthMethods == null || clientAuthMethods.isEmpty()) {
-            log.error("{} No client authentication mode found from profile configuration", getLogPrefix());
+        if (clientAuthMethods.isEmpty()) {
+            log.error("{} No client authentication method found from profile configuration", getLogPrefix());
             ActionSupport.buildEvent(profileRequestContext, IdPEventIds.INVALID_RELYING_PARTY_CONFIG);
             return;
         }
@@ -221,10 +227,11 @@ public class InitializeOAuth2ClientAuthenticationContext extends AbstractProfile
             @Nonnull final String clientId, @Nonnull final String tokenEndpointAuthMethod,
             @Nonnull final Credential clientCredential) {
         
-        // TODO support private key jwt
+        // TODO support JWT types
+        
         if (clientCredential.getSecretKey() == null) {
-            log.warn("{} Client credential is not a symmetric key, only client_secret currently supported to"
-                    + "construct client authentication",getLogPrefix());
+            log.warn("{} Client credential is not a symmetric key, only client_secret methods currently supported "
+                    + "to construct client authentication",getLogPrefix());
             return null;
         }
         Duration secretExpiresAt = Duration.ZERO;
diff --git a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/InitializeOAuth2ClientContext.java b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/InitializeOAuth2ClientContext.java
index fab32ee..50d6bf2 100644
--- a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/InitializeOAuth2ClientContext.java
+++ b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/InitializeOAuth2ClientContext.java
@@ -39,14 +39,18 @@ import net.shibboleth.idp.profile.IdPEventIds;
 import net.shibboleth.idp.profile.context.RelyingPartyContext;
 import net.shibboleth.oidc.profile.config.OIDCAuthorizationConfiguration;
 import net.shibboleth.utilities.java.support.annotation.constraint.NonnullAfterInit;
-import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
 import net.shibboleth.utilities.java.support.component.ComponentSupport;
 import net.shibboleth.utilities.java.support.logic.Constraint;
 import net.shibboleth.utilities.java.support.primitive.StringSupport;
 
 /**
  * An {@link AbstractProfileAction action} that resolves the client identifier for the chosen 
- * downstream provider (issuer). 
+ * upstream provider (issuer). 
+ * 
+ * @event {@link org.opensaml.profile.action.EventIds#PROCEED_EVENT_ID}
+ * @event {@link IdPEventIds.INVALID_PROFILE_CONFIG}
+ * @event {@link EventIds#INVALID_PROFILE_CTX}
+ * @post Add the clientId to the {@link OAuth2ClientContext}
  */
 public class InitializeOAuth2ClientContext extends AbstractProfileAction {
 
@@ -105,12 +109,6 @@ public class InitializeOAuth2ClientContext extends AbstractProfileAction {
                 Constraint.isNotNull(strategy, "Redirect URI lookup strategy can not be null");
     }
     
-    
-    @Override
-    protected void doInitialize() throws ComponentInitializationException {
-        super.doInitialize();
-    }
-    
     /**
      * Set the strategy to lookup the {@link OAuth2ClientContext} 
      * from the {@link ProfileRequestContext}.
diff --git a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/OIDCProxySupport.java b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/OIDCProxySupport.java
index 8028995..a8ea7b0 100644
--- a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/OIDCProxySupport.java
+++ b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/OIDCProxySupport.java
@@ -27,10 +27,7 @@ import org.apache.commons.codec.binary.Hex;
 import net.shibboleth.idp.plugin.authn.oidc.rp.OIDCProxyException;
 import net.shibboleth.utilities.java.support.logic.Constraint;
 
-/**
- * Support class for OIDC proxy related implementations.
- */
-//TODO Duo has similar support class methods, maybe merge into commons.
+/** Support class for OIDC proxy related implementations.*/
 public final class OIDCProxySupport {
     
     /** Private constructor.*/
diff --git a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/PopulateJWTDecryptionParameters.java b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/PopulateJWTDecryptionParameters.java
index b238d66..8827d9a 100644
--- a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/PopulateJWTDecryptionParameters.java
+++ b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/PopulateJWTDecryptionParameters.java
@@ -59,6 +59,7 @@ import net.shibboleth.utilities.java.support.resolver.ResolverException;
  * @event {@link EventIds#PROCEED_EVENT_ID}
  * @event {@link EventIds#INVALID_PROFILE_CTX}
  * @event {@link EventIds#INVALID_SEC_CFG}
+ * @post set the decryption parameters onto the security parameters context
  */
 public class PopulateJWTDecryptionParameters extends AbstractProfileAction {
     
diff --git a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/PopulateJWTEncryptionParameters.java b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/PopulateJWTEncryptionParameters.java
index a776348..5b50b43 100644
--- a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/PopulateJWTEncryptionParameters.java
+++ b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/PopulateJWTEncryptionParameters.java
@@ -69,10 +69,10 @@ import net.shibboleth.utilities.java.support.resolver.ResolverException;
  * @event {@link EventIds#PROCEED_EVENT_ID}
  * @event {@link EventIds#INVALID_PROFILE_CTX}
  * @event {@link EventIds#INVALID_SEC_CFG}
+ * @post set the encryption parameters onto the security parameters context
  */
 //TODO similar to PopulateOIDCEncryptionParameters? shall we merge into commons, adds the OP metadata from downstream
 // If exists, useful for proxy.
-// TODO move to commons?
 public class PopulateJWTEncryptionParameters extends AbstractProfileAction {
 
     /** Class logger. */
diff --git a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/PrepareOIDCInboundMessageContext.java b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/PrepareOIDCInboundMessageContext.java
index 14da715..9e53a9e 100644
--- a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/PrepareOIDCInboundMessageContext.java
+++ b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/PrepareOIDCInboundMessageContext.java
@@ -50,6 +50,7 @@ import net.shibboleth.utilities.java.support.logic.Constraint;
  * 
  * @event {@link EventIds#PROCEED_EVENT_ID}
  * @event {@link EventIds#INVALID_PROFILE_CTX}
+ * @post Add an {@link OIDCPeerEntityContext} to the inbound message context.
  */
 public class PrepareOIDCInboundMessageContext extends AbstractProfileAction {
     
@@ -146,8 +147,7 @@ public class PrepareOIDCInboundMessageContext extends AbstractProfileAction {
         } else {
             msgCtx = new MessageContext();
             profileRequestContext.setInboundMessageContext(msgCtx);
-        }
-        
+        }        
 
         final OIDCPeerEntityContext peerContext = msgCtx.getSubcontext(OIDCPeerEntityContext.class, true);
         peerContext.setIdentifier(identifier);
diff --git a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ProcessEndUserClaims.java b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ProcessEndUserClaims.java
index 224d721..bbcbdb9 100644
--- a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ProcessEndUserClaims.java
+++ b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ProcessEndUserClaims.java
@@ -51,8 +51,11 @@ import net.shibboleth.utilities.java.support.logic.Constraint;
  * 
  * <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>
+ * 
+ * @event {@link org.opensaml.profile.action.EventIds#PROCEED_EVENT_ID}
+ * @event {@link EventIds#INVALID_PROFILE_CTX}
+ * @post Add a set of merged and sanatized claims to the {@link EndUserClaimsContext}.
  */
-//TODO similar too ValidateUserInfoClaims, do we need to extend OIDC action
 public class ProcessEndUserClaims extends AbstractOIDCAuthenticationResponseAction {
     
     /** Class logger.*/
@@ -115,8 +118,7 @@ public class ProcessEndUserClaims extends AbstractOIDCAuthenticationResponseActi
      * 
      * @param strategy the strategy to use.
      */
-    public void setClaimMergingStrategy(
-            @Nonnull final BinaryOperator<ClaimsSet> strategy) {
+    public void setClaimMergingStrategy(@Nonnull final BinaryOperator<ClaimsSet> strategy) {
         ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
         
         claimMergingStrategy =  Constraint.isNotNull(strategy,
@@ -129,8 +131,7 @@ public class ProcessEndUserClaims extends AbstractOIDCAuthenticationResponseActi
      * 
      * @param strategy the strategy to use.
      */
-    public void setClaimSanitizationStrategy(
-            @Nonnull final UnaryOperator<ClaimsSet> strategy) {
+    public void setClaimSanitizationStrategy(@Nonnull final UnaryOperator<ClaimsSet> strategy) {
         claimSanitizationStrategy = Constraint.isNotNull(strategy,
                 "ClaimSanatizationStrategy cannot be null");
     }
diff --git a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/UnsupportedResponseTypeAction.java b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/UnsupportedResponseTypeAction.java
index feee9c7..55f3c3e 100644
--- a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/UnsupportedResponseTypeAction.java
+++ b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/UnsupportedResponseTypeAction.java
@@ -27,7 +27,7 @@ import org.slf4j.LoggerFactory;
 import net.shibboleth.idp.profile.AbstractProfileAction;
 import net.shibboleth.oidc.profile.core.OidcEventIds;
 
-/** Simple action to log the response was to an unsupported grant type.*/
+/** Simple action to log the response from an unsupported grant type.*/
 public class UnsupportedResponseTypeAction extends AbstractProfileAction {
     
     /** Class logger. */
diff --git a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/UserInfoEndpointLookup.java b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/UserInfoEndpointLookup.java
index 462c9b8..cd688a7 100644
--- a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/UserInfoEndpointLookup.java
+++ b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/UserInfoEndpointLookup.java
@@ -41,6 +41,11 @@ import net.shibboleth.utilities.java.support.logic.Constraint;
 /**
  * Request information from the UserInfo OAuth2.0 endpoint using the access_token already present
  * in the context. Return consented claims about the subject. 
+ * 
+ * @event {@link org.opensaml.profile.action.EventIds#PROCEED_EVENT_ID}
+ * @event {@link EventIds#INVALID_PROFILE_CTX}
+ * @event {@link AuthnEventIds#AUTHN_EXCEPTION}
+ * @post Add a claims returned from the UserInfo endpoint to the {@link UserInfoResponseContext}.
  */
 public class UserInfoEndpointLookup extends AbstractHttpOIDCAuthenticationAction<UserInfoResponse> {
     
diff --git a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ValidateExternalAuthenticationContext.java b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ValidateExternalAuthenticationContext.java
index fbee66d..6bf0cc9 100644
--- a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ValidateExternalAuthenticationContext.java
+++ b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ValidateExternalAuthenticationContext.java
@@ -37,7 +37,6 @@ import org.slf4j.LoggerFactory;
  *  
  * @event {@link EventIds#PROCEED_EVENT_ID}
  * @event {@link AuthnEventIds#INVALID_AUTHN_CTX}
- * @event various
  */
 public class ValidateExternalAuthenticationContext extends AbstractAuthenticationAction {
 
diff --git a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ValidateOAuthAccessTokenResponse.java b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ValidateOAuthAccessTokenResponse.java
index 014f3f3..6b820ad 100644
--- a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ValidateOAuthAccessTokenResponse.java
+++ b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ValidateOAuthAccessTokenResponse.java
@@ -40,6 +40,10 @@ import net.shibboleth.utilities.java.support.logic.Constraint;
 /** 
  * Validation action that validates the OAuth Access Token Response against RFC 6749 section 5.1
  * and OpenID Connect Core 1.0 section 3.1.3.3.
+ * 
+ * @event {@link org.opensaml.profile.action.EventIds#PROCEED_EVENT_ID}
+ * @event {@link EventIds#INVALID_PROFILE_CTX}
+ * @event {@link OidcEventIds#INVALID_ACCESS_TOKEN}
  */
 public class ValidateOAuthAccessTokenResponse extends AbstractOIDCAuthenticationResponseAction {    
     
@@ -123,7 +127,7 @@ public class ValidateOAuthAccessTokenResponse extends AbstractOIDCAuthentication
                 return;
             }
         }
-        // Is recomended, but we do not use it
+        // Is recommended, but we do not use it
         if (!rawTokenResponse.containsKey("expires_in")) {
             log.trace("{} Expires_in not found in access token, is recomended ", getLogPrefix());
         }
diff --git a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ValidateResponseState.java b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ValidateResponseState.java
index 67fd511..8095b8b 100644
--- a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ValidateResponseState.java
+++ b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ValidateResponseState.java
@@ -26,6 +26,7 @@ import org.slf4j.LoggerFactory;
 
 import net.shibboleth.idp.authn.AuthnEventIds;
 import net.shibboleth.idp.authn.context.AuthenticationContext;
+import net.shibboleth.oidc.profile.core.OidcEventIds;
 import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
 
 /**
@@ -37,14 +38,7 @@ import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
  * 
  * @event {@link org.opensaml.profile.action.EventIds#PROCEED_EVENT_ID}
  * @event {@link AuthnEventIds#NO_CREDENTIALS}
- * FIXME: condiditions
- * @pre <pre>
- *      ProfileRequestContext.getSubcontext(AuthenticationContext.class, false) != null
- *      </pre>
- * 
- * @pre <pre>
- *      AuthenticationContext.getSubcontext(DuoOIDCAuthenticationContext.class, false) != null
- *      </pre>
+ * @pre <pre>ProfileRequestContext.getSubcontext(AuthenticationContext.class, false) != null</pre>
  * 
  */
 public class ValidateResponseState extends AbstractOIDCAuthenticationResponseAction {    
diff --git a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ValidateTokenClaims.java b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ValidateTokenClaims.java
index af4a64b..aa7e972 100644
--- a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ValidateTokenClaims.java
+++ b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ValidateTokenClaims.java
@@ -25,6 +25,7 @@ import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
 import org.opensaml.profile.action.ActionSupport;
+import org.opensaml.profile.action.EventIds;
 import org.opensaml.profile.context.ProfileRequestContext;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -48,34 +49,13 @@ import net.shibboleth.utilities.java.support.logic.Constraint;
  * {@link ClaimsValidator claims validator}. The verifier <b>must</b> be thread-safe and validate, at
  * minimum the claims set against the OpenID Connect core 1.0 section 3.1.3.7 specification. 
  * 
- * TODO: check these conditions
  * 
- * @pre
+ * @pre <pre>ProfileRequestContext.getSubcontext(AuthenticationContext.class, false) != null</pre> 
+ * @pre <pre>JWT.getJWTClaimsSet() != null</pre>
  * 
- *      <pre>
- *      ProfileRequestContext.getSubcontext(AuthenticationContext.class, false) != null
- *      </pre>
- * 
- * @pre
- * 
- *      <pre>
- *      AuthenticationContext.getSubcontext(DuoOIDCAuthenticationContext.class, false) != null
- *      </pre>
- * 
- * @pre
- * 
- *      <pre>
- *      DuoOIDCAuthenticationContext.getAuthToken() != null
- *      </pre>
- * @pre
- * 
- *      <pre>
- *      DuoOIDCAuthenticationContext.getIntegration() != null
- *      </pre>
- * 
- * @event {@link org.opensaml.profile.action.EventIds#PROCEED_EVENT_ID}
- * @event {@link net.shibboleth.idp.authn.AuthnEventIds#AUTHN_EXCEPTION}
- * @event {@link net.shibboleth.idp.authn.AuthnEventIds#NO_CREDENTIALS}
+ * @event {@link EventIds#PROCEED_EVENT_ID}
+ * @event {@link AuthnEventIds#INVALID_AUTHN_CTX}
+ * @event {@link OidcEventIds.INVALID_TOKEN}
  */
 public class ValidateTokenClaims extends AbstractOIDCAuthenticationResponseAction {
     
diff --git a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ValidateUserInfoJSONObjectClaims.java b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ValidateUserInfoJSONObjectClaims.java
index 889cb59..ad654f5 100644
--- a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ValidateUserInfoJSONObjectClaims.java
+++ b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ValidateUserInfoJSONObjectClaims.java
@@ -44,6 +44,10 @@ import net.shibboleth.utilities.java.support.logic.Constraint;
 
 /**
  * Validate a successful UserInfo JSON Object Response according to section 5.3.2 of OpenID Connect Core 1.0. 
+ * 
+ * @event {@link org.opensaml.profile.action.EventIds#PROCEED_EVENT_ID}
+ * @event {@link OidcEventIsd#INVALID_USERINFO_CLAIMS}
+ * @event {@link EventIds#INVALID_PROFILE_CTX}
  */
 public class ValidateUserInfoJSONObjectClaims extends AbstractAuthenticationAction {
     
diff --git a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/messaging/impl/AbstractOIDCAuthenticationRequestMessageHandler.java b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/messaging/impl/AbstractOIDCAuthenticationRequestMessageHandler.java
index cda0193..ae024ae 100644
--- a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/messaging/impl/AbstractOIDCAuthenticationRequestMessageHandler.java
+++ b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/messaging/impl/AbstractOIDCAuthenticationRequestMessageHandler.java
@@ -36,8 +36,8 @@ import net.shibboleth.utilities.java.support.component.ComponentSupport;
 import net.shibboleth.utilities.java.support.logic.Constraint;
 
 /** 
- * An abstract message handler that does *not* run inside an {@link WebFlowMessageHandlerAdaptor}
- * that pulls out the {@link OutboundMessageHandlerContext} from the message context.
+ * An abstract message handler that does *not* run inside an {@link WebFlowMessageHandlerAdaptor}. Makes available the
+ * {@link OIDCAuthenticationRequest}.
  */
 public abstract class AbstractOIDCAuthenticationRequestMessageHandler extends AbstractMessageHandler {
 
@@ -68,6 +68,25 @@ public abstract class AbstractOIDCAuthenticationRequestMessageHandler extends Ab
             return null;
         };
     }
+    
+    
+    /**
+     * Get the outbound message context.
+     * 
+     * @return the outbound message context.
+     */
+    @Nullable protected OutboundMessageHandlerContext getOutboundMessageContext() {
+        return outboundMessageContext;
+    }
+    
+    /**
+     * Get the authentication request.
+     * 
+     * @return the authentication request
+     */
+    @Nullable protected OIDCAuthenticationRequest getAuthenticationRequest() {
+        return authnRequest;
+    }
 
     /**
      * Set the strategy used to locate the {@link OIDCAuthenticationRequest} to use.
@@ -112,24 +131,7 @@ public abstract class AbstractOIDCAuthenticationRequestMessageHandler extends Ab
         
         return super.doPreInvoke(messageContext);
     }
-    
-    /**
-     * Get the outbound message context.
-     * 
-     * @return the outbound message context.
-     */
-    @Nullable protected OutboundMessageHandlerContext getOutboundMessageContext() {
-        return outboundMessageContext;
-    }
-    
-    /**
-     * Get the authentication request.
-     * 
-     * @return the authentication request
-     */
-    @Nullable protected OIDCAuthenticationRequest getAuthenticationRequest() {
-        return authnRequest;
-    }
+
     
     
 
diff --git a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/messaging/impl/EncryptJWT.java b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/messaging/impl/EncryptJWT.java
index 2bc0030..9ff9921 100644
--- a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/messaging/impl/EncryptJWT.java
+++ b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/messaging/impl/EncryptJWT.java
@@ -83,12 +83,12 @@ public class EncryptJWT extends AbstractMessageHandler {
     @Nullable private JWTEncryptionParameters encryptionParameters;
     
     /** A friendly name to log as the subject of encryption.*/
-    @Nonnull private String forFriendlyName;
+    @Nonnull private String logName;
     
     
     /** Constructor.*/
     public EncryptJWT() { 
-        forFriendlyName = "not-specified";
+        logName = "not-specified";
         securityParametersLookupStrategy = new ChildContextLookup<>(JWTSecurityParametersContext.class);
     }
     
@@ -97,11 +97,11 @@ public class EncryptJWT extends AbstractMessageHandler {
      * 
      * @param name the friendly name
      */
-    public void setForFriendlyName(@Nonnull @NotEmpty final String name) {
+    public void setLogName(@Nonnull @NotEmpty final String name) {
         ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
         ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
 
-        forFriendlyName = Constraint.isNotEmpty(name, "ForFriendlyName can not be null or empty");
+        logName = Constraint.isNotEmpty(name, "ForFriendlyName can not be null or empty");
     }
     
     /**
@@ -221,7 +221,7 @@ public class EncryptJWT extends AbstractMessageHandler {
                 
                 jweObject = new JWEObject(new JWEHeader.Builder(encAlg, encEnc).contentType("JWT")
                                 .keyID(keyTransportKid).build(), payload);
-                logEncryption(forFriendlyName, keyTransportKid, encAlg.getName(), encEnc.getName());
+                logEncryption(keyTransportKid, encAlg.getName(), encEnc.getName());
                 jweObject.encrypt(new RSAEncrypter((RSAPublicKey) keyTransportCredential.getPublicKey()));
                 
             } else if (JWEAlgorithm.Family.ECDH_ES.contains(encAlg) && keyTransportCredential != null &&
@@ -229,7 +229,7 @@ public class EncryptJWT extends AbstractMessageHandler {
                 
                 jweObject = new JWEObject(new JWEHeader.Builder(encAlg, encEnc).contentType("JWT")
                         .keyID(keyTransportKid).build(), payload);                
-                logEncryption(forFriendlyName, keyTransportKid, encAlg.getName(), encEnc.getName());
+                logEncryption(keyTransportKid, encAlg.getName(), encEnc.getName());
                 jweObject.encrypt(new ECDHEncrypter((ECPublicKey) keyTransportCredential.getPublicKey()));
                 
             } else if ((JWEAlgorithm.Family.AES_KW.contains(encAlg) || JWEAlgorithm.Family.AES_GCM_KW.contains(encAlg)) 
@@ -237,7 +237,7 @@ public class EncryptJWT extends AbstractMessageHandler {
                 
                 jweObject = new JWEObject(new JWEHeader.Builder(encAlg, encEnc).contentType("JWT")
                         .keyID(keyTransportKid).build(), payload);
-                logEncryption(forFriendlyName, keyTransportKid, encAlg.getName(), encEnc.getName());              
+                logEncryption(keyTransportKid, encAlg.getName(), encEnc.getName());              
                 jweObject.encrypt(new AESEncrypter(keyTransportCredential.getSecretKey()));
                 
             } else if (JWEAlgorithm.DIR.equals(encAlg) && dataEncryptionCredential != null &&
@@ -245,7 +245,7 @@ public class EncryptJWT extends AbstractMessageHandler {
                 
                 jweObject = new JWEObject(new JWEHeader.Builder(encAlg, encEnc).contentType("JWT")
                         .keyID(dataEncryptionKid).build(), payload);
-                logEncryption(forFriendlyName, dataEncryptionKid, encAlg.getName(), encEnc.getName());      
+                logEncryption(dataEncryptionKid, encAlg.getName(), encEnc.getName());      
                 jweObject.encrypt(new DirectEncrypter(dataEncryptionCredential.getSecretKey()));
                 
             } else {
@@ -258,9 +258,9 @@ public class EncryptJWT extends AbstractMessageHandler {
             jwtUpdateConsumer.accept(encryptedJWT, messageContext);
             
             if (log.isDebugEnabled() && !log.isTraceEnabled()) {
-                log.debug("{} Encrypted '{}' JWT", getLogPrefix(), forFriendlyName);
+                log.debug("{} Encrypted '{}' JWT", getLogPrefix(), logName);
             } else if (log.isTraceEnabled()) {
-                log.debug("{} Encrypted '{}' JWT: {}", getLogPrefix(), forFriendlyName, encryptedJWT.serialize());
+                log.debug("{} Encrypted '{}' JWT: {}", getLogPrefix(), logName, encryptedJWT.serialize());
             }
             
         } catch (final Exception e) {
@@ -274,15 +274,14 @@ public class EncryptJWT extends AbstractMessageHandler {
      * A convince method to log encryption parameters. Avoids some of the clutter in the calling
      * methods.
      * 
-     * @param friendlyName the friendly name of the subject of encryption
      * @param keyID the keyID
      * @param enc the content encryption algorithm
      * @param alg the key management algorithm
      */
-    private void logEncryption(@Nullable final String friendlyName, @Nullable final String keyID, 
+    private void logEncryption(@Nullable final String keyID, 
             @Nullable final String enc, @Nullable final String alg) {
         log.debug("{} Encrypting '{}' with kid '{}' and params alg: {} enc: {}", 
-                getLogPrefix(), friendlyName, keyID, alg, enc); 
+                getLogPrefix(), logName, keyID, alg, enc); 
     }
 
 }
diff --git a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/messaging/impl/SignJWT.java b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/messaging/impl/SignJWT.java
index f0b30aa..7654400 100644
--- a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/messaging/impl/SignJWT.java
+++ b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/messaging/impl/SignJWT.java
@@ -89,11 +89,11 @@ public class SignJWT extends AbstractMessageHandler {
     @Nullable @NotEmpty private String typeHeader;
     
     /** A friendly name to log as the subject of encryption parameter resolution.*/
-    @Nonnull private String forFriendlyName;
+    @Nonnull private String logName;
     
     /** Constructor.*/
     public SignJWT() {
-        forFriendlyName = "not-specified";
+        logName = "not-specified";
         securityParametersLookupStrategy = new ChildContextLookup<>(JWTSecurityParametersContext.class);        
     }
     
@@ -102,11 +102,11 @@ public class SignJWT extends AbstractMessageHandler {
      * 
      * @param name the friendly name
      */
-    public void setForFriendlyName(@Nonnull @NotEmpty final String name) {
+    public void setLogName(@Nonnull @NotEmpty final String name) {
         ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
         ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
 
-        forFriendlyName = Constraint.isNotEmpty(name, "ForFriendlyName can not be null or empty");
+        logName = Constraint.isNotEmpty(name, "ForFriendlyName can not be null or empty");
     }
     
     @Override
@@ -218,14 +218,14 @@ public class SignJWT extends AbstractMessageHandler {
             jwt = new SignedJWT(headerBuilder.build(), jwtClaimSetToSign);
             jwt.sign(signer);
             if (log.isDebugEnabled() && !log.isTraceEnabled()) {
-                log.debug("{} Signed JWT '{}'", getLogPrefix(), forFriendlyName);
+                log.debug("{} Signed JWT '{}'", getLogPrefix(), logName);
             } else if (log.isTraceEnabled()) {
-                log.debug("{} Signed JWT '{}': {}", getLogPrefix(), forFriendlyName, jwt.serialize());
+                log.debug("{} Signed JWT '{}': {}", getLogPrefix(), logName, jwt.serialize());
             }            
             
             if (jwt.getState() != State.SIGNED) {
                 // Should not really happen, as JOSEException should be thrown
-                log.error("{} JWT '{}' was not signed", getLogPrefix(), forFriendlyName);
+                log.error("{} JWT '{}' was not signed", getLogPrefix(), logName);
                 throw new MessageHandlerException("JWT was not signed, unknown cause");
             }
             
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 704417e..ab3b8db 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
@@ -273,7 +273,7 @@
                     scope="prototype" />
 
                 <bean id="SignRequestObject" class="net.shibboleth.idp.plugin.authn.oidc.rp.messaging.impl.SignJWT"
-                    scope="prototype" p:forFriendlyName="RequestObject">
+                    scope="prototype" p:logName="RequestObject">
                     <property name="claimsToSignLookupStrategy">
                         <bean
                             class="net.shibboleth.idp.plugin.authn.oidc.rp.messaging.context.logic.JWTClaimsSetFromRequestObjectLookupFunction" />
@@ -286,7 +286,7 @@
 
                 <bean id="EncryptRequestObject"
                     class="net.shibboleth.idp.plugin.authn.oidc.rp.messaging.impl.EncryptJWT" scope="prototype"
-                    p:forFriendlyName="RequestObject">
+                    p:logName="RequestObject">
                     <property name="payloadToEncryptLookupStrategy">
                         <bean
                             class="net.shibboleth.idp.plugin.authn.oidc.rp.messaging.context.logic.PayloadFromRequestObjectLookupFunction" />
@@ -401,6 +401,7 @@
         class="net.shibboleth.idp.plugin.authn.oidc.rp.encoding.impl.NimbusAuthCodeTokenRequestEncoder" />
 
     <!-- Create a default object mapper. Setup should not change once injected -->
+    <!-- TODO can we not use more global versions? -->
     <bean id="shibboleth.authn.oidc.rp.JSONObjectMapper" class="com.fasterxml.jackson.databind.ObjectMapper" />
 
     <bean class="org.springframework.beans.factory.config.MethodInvokingBean"
@@ -425,7 +426,7 @@
     </bean>
 
 
-    <!-- ID_TOKEN Decryption -->
+    <!-- ID TOKEN Decryption -->
 
     <!-- TODO should we use an activation condition to decide if decryption params are needed e.g. JWT is not a JWE to stop 
         redundent resolution of credentials -->
@@ -459,7 +460,7 @@
     </bean>
 
 
-    <!-- ID_TOKEN Signature Validation -->
+    <!-- ID TOKEN Signature Validation -->
 
     <bean id="PopulateIDTokenSignatureValidationParameters" parent="NestedWebFlowProfileActionAdaptor"
         scope="prototype">

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


More information about the commits mailing list