[ENG-12103] ORCiD Rewrite Follow-up

src/main/java/io/cos/cas/osf/authentication/credential/OsfOrcidSsoCredential.java

@@ -16,6 +16,12 @@
 import org.springframework.binding.message.MessageBuilder;
 import org.springframework.binding.validation.ValidationContext;
 
+/**
+ * This is {@link OsfOrcidSsoCredential}.
+ *
+ * @author Longze Chen
+ * @since 26.2.0
+ */
 @AllArgsConstructor
 @NoArgsConstructor
 @Getter
@@ -24,35 +30,57 @@
 @ToString(callSuper = true)
 public class OsfOrcidSsoCredential extends AbstractCredential {
 
+    /** Serial version UID. */
     private static final long serialVersionUID = 7983138918562300147L;
 
+    /** The prefix which is added to {@link #orcidId} in {@link #getId()}. */
     public static final String CREDENTIAL_ID_PREFIX = "OrcidProfile#";
 
+    /** Attribute name for ORCiD ID, which is released to OSF. */
     public static final String AUTHENTICATION_ATTRIBUTE_ORCID_ID = "orcidId";
 
+    /** Attribute name for Access Token, which is released to OSF. */
     public static final String AUTHENTICATION_ATTRIBUTE_ORCID_ACCESS_TOKEN = "orcidAccessToken";
 
+    /** Attribute name for Refresh Token, which is released to OSF. */
     public static final String AUTHENTICATION_ATTRIBUTE_ORCID_REFRESH_TOKEN = "orcidRefreshToken";
 
+    /** ORCiD ID. */
     private String orcidId;
 
+    /**  ORCiD Access Token. */
     private String orcidAccessToken;
 
+    /** ORCiD Refresh Token. */
     private String orcidRefreshToken;
 
+    /**
+     * Get the unique identifier for this credential.
+     *
+     * @return the credential ID, formed as {@code CREDENTIAL_ID_PREFIX + orcidId}
+     */
     @Override
     public String getId() {
         return CREDENTIAL_ID_PREFIX + this.getOrcidId();
     }
 
+    /**
+     * Check if credential is valid. {@link #orcidId} and {@link #orcidAccessToken} must not be null or empty.
+     *
+     * @return {@code true} if both {@code orcidId} and {@code orcidAccessToken} are non-null and non-blank,
+     *         {@code false} otherwise
+     */
     @Override
     @JsonIgnore
     public boolean isValid() {
-        return StringUtils.isNotBlank(getId())
-                && StringUtils.isNotBlank(getOrcidAccessToken())
-                && StringUtils.isNotBlank(getOrcidRefreshToken());
+        return StringUtils.isNoneBlank(this.orcidId, this.orcidAccessToken);
     }
 
+    /**
+     * Validate this credential, adding an error message to the given context if it is not valid.
+     *
+     * @param context the validation context to which any error messages are added
+     */
     @Override
     public void validate(final ValidationContext context) {
         if (!isValid()) {

src/main/java/io/cos/cas/osf/authentication/handler/support/OsfOrcidSsoAuthenticationHandler.java

@@ -18,16 +18,22 @@
 
 import java.security.GeneralSecurityException;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 
+/**
+ * This is {@link OsfOrcidSsoAuthenticationHandler}.
+ *
+ * @author Longze Chen
+ * @since 26.2.0
+ */
 @Getter
 @Setter
 @Slf4j
 public class OsfOrcidSsoAuthenticationHandler extends AbstractPreAndPostProcessingAuthenticationHandler {
 
+    /** Constructor for all required args. */
     public OsfOrcidSsoAuthenticationHandler(
             final String name,
             final ServicesManager servicesManager,
@@ -37,51 +43,60 @@ public OsfOrcidSsoAuthenticationHandler(
         super(name, servicesManager, principalFactory, order);
     }
 
+    /** Authenticate with no-op credential transform. */
     @Override
     protected final AuthenticationHandlerExecutionResult doAuthentication(
             Credential credential
     ) throws GeneralSecurityException {
         OsfOrcidSsoCredential osfOrcidSsoCredential = (OsfOrcidSsoCredential) credential;
-        LOGGER.debug("Attempting authentication internally for transformed credential [{}]", osfOrcidSsoCredential);
+        LOGGER.debug("[ORCiD SSO] Attempting authentication internally for transformed credential [{}]", osfOrcidSsoCredential);
         return authenticateOsfOrcidSsoInternal(osfOrcidSsoCredential);
     }
 
+    /** {@link OsfOrcidSsoAuthenticationHandler} only supports {@link OsfOrcidSsoCredential} */
     @Override
     public boolean supports(final Class<? extends Credential> clazz) {
         return OsfOrcidSsoCredential.class.isAssignableFrom(clazz);
     }
 
+    /** {@link OsfOrcidSsoAuthenticationHandler} only supports {@link OsfOrcidSsoCredential} */
     @Override
     public boolean supports(final Credential credential) {
         return credential instanceof OsfOrcidSsoCredential;
     }
 
+    /** Create {@link AuthenticationHandlerExecutionResult} from {@link OsfOrcidSsoCredential}. */
     protected final AuthenticationHandlerExecutionResult authenticateOsfOrcidSsoInternal(
             final OsfOrcidSsoCredential credential
     ) throws GeneralSecurityException {
 
+        if (credential == null) {
+            LOGGER.error("[ORCiD SSO] Null/Empty ORCiD Credential.");
+            throw new GeneralSecurityException("Null/Empty ORCiD Credential.");
+        }
+
         final String credentialId = credential.getId();
         final String orcidId = credential.getOrcidId();
         final String orcidAccessToken = credential.getOrcidAccessToken();
         final String orcidRefreshToken = credential.getOrcidRefreshToken();
 
-        LOGGER.debug(">>>> credential = {}", credential);
-        LOGGER.debug(">>>> ---- credentialId = {}", credentialId);
-        LOGGER.debug(">>>> ---- orcidId = {}", orcidId);
-        LOGGER.debug(">>>> ---- orcidAccessToken = {}", orcidAccessToken);
-        LOGGER.debug(">>>> ---- orcidRefreshToken = {}", orcidRefreshToken);
+        if (StringUtils.isBlank(orcidId)) {
+            LOGGER.error("[ORCiD SSO] Null/Empty ORCiD ID.");
+            throw new GeneralSecurityException("Null/Empty ORCiD ID.");
+        } else if (StringUtils.isBlank(orcidAccessToken)) {
+            LOGGER.error("[ORCiD SSO] Null/Empty ORCiD Access Token, orcidId=[{}]", orcidId);
+            throw new GeneralSecurityException("Null/Empty ORCiD Access Token.");
+        }
 
-        LOGGER.debug(
-                "Credential metadata: id=[{}], orcidId=[{}], orcidAccessToken=[{}], orcidRefreshToken=[{}]",
+        LOGGER.info(
+                "[ORCiD SSO] Credential metadata: id=[{}], orcidId=[{}], hasAccessToken=[{}], hasRefreshToken=[{}]",
                 credentialId,
                 orcidId,
-                StringUtils.isNoneBlank(orcidAccessToken),
-                StringUtils.isNoneBlank(orcidRefreshToken)
+                StringUtils.isNotBlank(orcidAccessToken),
+                StringUtils.isNotBlank(orcidRefreshToken)
         );
 
         final Map<String, List<Object>> attributes = new LinkedHashMap<>();
-        attributes.put("orcidAccessToken", Collections.singletonList(orcidAccessToken));
-        attributes.put("orcidRefreshToken", Collections.singletonList(orcidRefreshToken));
         final Principal principal = this.principalFactory.createPrincipal(credentialId, attributes);
         final List<MessageDescriptor> warnings = new ArrayList<>();
         return createHandlerResult(credential, principal, warnings);

src/main/java/io/cos/cas/osf/authentication/metadata/OsfOrcidSsoAuthenticationMetaDataPopulator.java

@@ -6,22 +6,31 @@
 import lombok.ToString;
 import lombok.extern.slf4j.Slf4j;
 
+import org.apache.commons.lang3.StringUtils;
+
 import org.apereo.cas.authentication.AuthenticationBuilder;
 import org.apereo.cas.authentication.AuthenticationMetaDataPopulator;
 import org.apereo.cas.authentication.AuthenticationTransaction;
 import org.apereo.cas.authentication.Credential;
 
+/**
+ * This is {@link OsfOrcidSsoAuthenticationMetaDataPopulator}.
+ *
+ * @author Longze Chen
+ * @since 26.1.0
+ */
 @Getter
 @ToString(callSuper = true)
 @Slf4j
 public class OsfOrcidSsoAuthenticationMetaDataPopulator implements AuthenticationMetaDataPopulator {
 
+    /** Add attribute to authentication metadata. */
     @Override
     public void populateAttributes(final AuthenticationBuilder builder, final AuthenticationTransaction transaction) {
         transaction.getPrimaryCredential().ifPresent(r -> {
             final OsfOrcidSsoCredential credential = (OsfOrcidSsoCredential) r;
             LOGGER.info(
-                    "[ORCiD SSO] Credential is of type [{}], thus adding attributes [{}, {}, {}]",
+                    "[ORCiD SSO] Credential is of type [{}], thus adding attributes [{}, {}, and optionally {} if not null/blank)]",
                     OsfOrcidSsoCredential.class.getSimpleName(),
                     OsfOrcidSsoCredential.AUTHENTICATION_ATTRIBUTE_ORCID_ID,
                     OsfOrcidSsoCredential.AUTHENTICATION_ATTRIBUTE_ORCID_ACCESS_TOKEN,
@@ -35,13 +44,17 @@ public void populateAttributes(final AuthenticationBuilder builder, final Authen
                     OsfOrcidSsoCredential.AUTHENTICATION_ATTRIBUTE_ORCID_ACCESS_TOKEN,
                     credential.getOrcidAccessToken()
             );
-            builder.addAttribute(
-                    OsfOrcidSsoCredential.AUTHENTICATION_ATTRIBUTE_ORCID_REFRESH_TOKEN,
-                    credential.getOrcidRefreshToken()
-            );
+            final String refreshToken = credential.getOrcidRefreshToken();
+            if (StringUtils.isNotBlank(refreshToken)) {
+                builder.addAttribute(
+                        OsfOrcidSsoCredential.AUTHENTICATION_ATTRIBUTE_ORCID_REFRESH_TOKEN,
+                        refreshToken
+                );
+            }
         });
     }
 
+    /** {@link OsfOrcidSsoAuthenticationMetaDataPopulator} only supports {@link OsfOrcidSsoCredential} */
     @Override
     public boolean supports(final Credential credential) {
         return credential instanceof OsfOrcidSsoCredential;

src/main/java/io/cos/cas/osf/config/OsfOrcidSsoAuthenticationEventExecutionPlanConfiguration.java

@@ -20,6 +20,12 @@
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 
+/**
+ * This is {@link OsfOrcidSsoAuthenticationEventExecutionPlanConfiguration}.
+ *
+ * @author Longze Chen
+ * @since 26.2.0
+ */
 @Configuration("osfOrcidSsoAuthenticationEventExecutionPlanConfiguration")
 @EnableConfigurationProperties(CasConfigurationProperties.class)
 public class OsfOrcidSsoAuthenticationEventExecutionPlanConfiguration {

src/main/java/io/cos/cas/osf/config/OsfPostgresAuthenticationEventExecutionPlanConfiguration.java

@@ -24,7 +24,7 @@
 /**
  * This is {@link OsfPostgresAuthenticationEventExecutionPlanConfiguration}.
  *
- * Longze Chen
+ * @author Longze Chen
  * @since 20.0.0
  */
 @Configuration("osfPostgresAuthenticationEventExecutionPlanConfiguration")

src/main/java/io/cos/cas/osf/configuration/model/OsfOrcidSsoAuthenticationProperties.java

@@ -8,16 +8,26 @@
 
 import java.io.Serializable;
 
+/**
+ * This is {@link OsfOrcidSsoAuthenticationProperties}.
+ *
+ * @author Longze Chen
+ * @since 26.2.0
+ */
 @Getter
 @Setter
 @Accessors(chain = true)
 public class OsfOrcidSsoAuthenticationProperties implements Serializable {
 
+    /** Serial version UID. */
     private static final long serialVersionUID = 4565930696065100663L;
 
+    /** The name of the authentication handler. */
     private String name = OsfOrcidSsoAuthenticationHandler.class.getSimpleName();
 
+    /** The flag to enable / disable the authentication handler. */
     private boolean enabled = Boolean.TRUE;
 
+    /** The order of the authentication handler. */
     private int order;
 }

src/main/java/io/cos/cas/osf/configuration/model/OsfPostgresAuthenticationProperties.java

@@ -23,36 +23,25 @@
 @Accessors(chain = true)
 public class OsfPostgresAuthenticationProperties implements Serializable {
 
+    /** Serial version UID. */
     private static final long serialVersionUID = -6126944686676618138L;
 
-    /**
-     * The name of the authentication handler.
-     */
+    /** The name of the authentication handler. */
     private String name = OsfPostgresAuthenticationHandler.class.getSimpleName();
 
-    /**
-     * The flag to enable / disable the authentication handler.
-     */
+    /** The flag to enable / disable the authentication handler. */
     private boolean enabled = Boolean.TRUE;
 
-    /**
-     * The order of the authentication handler.
-     */
+    /** The order of the authentication handler. */
     private int order;
 
-    /**
-     * Institution authentication delegation clients.
-     */
+    /** Institution authentication delegation clients. */
     private List<String> institutionClients = new LinkedList<>();
 
-    /**
-     * Non-institution authentication delegation clients.
-     */
+    /** Non-institution authentication delegation clients. */
     private List<String> nonInstitutionClients = new LinkedList<>();
 
-    /**
-     * Nested JPA properties for OSF PostgreSQL database.
-     */
+    /** Nested JPA properties for OSF PostgreSQL database. */
     @NestedConfigurationProperty
     private OsfPostgresJpaProperties jpa = new OsfPostgresJpaProperties();
 }

src/main/java/io/cos/cas/osf/web/flow/login/OsfPrincipalFromNonInteractiveCredentialsAction.java

@@ -184,6 +184,8 @@ public class OsfPrincipalFromNonInteractiveCredentialsAction extends AbstractNon
 
     private static final String LDAP_DN_OU_PREFIX = "ou=";
 
+    private static final String ORCiD_CLIENT_NAME = "orcid";
+
     private static final int OSF_API_RETRY_LIMIT = 3;
 
     private static final List<Integer> OSF_API_RETRY_STATUS = List.of(
@@ -253,21 +255,22 @@ protected Credential constructCredentialsFromRequest(final RequestContext contex
                 final String clientName = ((ClientCredential) credential).getClientName();
                 // Type 1: non-institution SSO (i.e. ORCiD) via pac4j authentication delegation using the OAuth protocol
                 if (authnDelegationClients.get(NON_INSTITUTION_CLIENTS_PARAMETER_NAME).contains(clientName)) {
-                    LOGGER.debug(
-                            "Valid non-institution authn delegation client [{}] found with principal [{}]",
+                    LOGGER.info(
+                            "[PAC4J SSO] Valid non-institution authn delegation client [{}] found with principal [{}]",
                             clientName,
                             credential.getId()
                     );
-                    LOGGER.debug(">>>> credential = {}", ((ClientCredential) credential).getCredentials().toString());
-                    LOGGER.debug(">>>> profile = {}", ((ClientCredential) credential).getUserProfile().toString());
-                    final OrcidProfile orcidUserProfile = (OrcidProfile) ((ClientCredential) credential).getUserProfile();
-                    final String orcidId = orcidUserProfile.getId();
-                    final String orcidAccessToken = (String) orcidUserProfile.getAttribute("access_token");
-                    final String orcidRefreshToken = (String) orcidUserProfile.getAttribute("refresh_token");
-                    LOGGER.debug(">>>> orcidId = {}", orcidId);
-                    LOGGER.debug(">>>> orcidAccessToken = {}", orcidAccessToken);
-                    LOGGER.debug(">>>> orcidRefreshToken = {}", orcidRefreshToken);
-                    return new OsfOrcidSsoCredential(orcidId, orcidAccessToken, orcidRefreshToken);
+                    if (clientName.equalsIgnoreCase(ORCiD_CLIENT_NAME)) {
+                        // Case 1: ORCiD Client will be handled by our customized credential and authn handler
+                        final OrcidProfile orcidUserProfile = (OrcidProfile) ((ClientCredential) credential).getUserProfile();
+                        final String orcidId = orcidUserProfile.getId();
+                        final String orcidAccessToken = (String) orcidUserProfile.getAttribute("access_token");
+                        final String orcidRefreshToken = (String) orcidUserProfile.getAttribute("refresh_token");
+                        return new OsfOrcidSsoCredential(orcidId, orcidAccessToken, orcidRefreshToken);
+                    } else {
+                        // Case 2: Other Client will use built-in credential and authn handler by apereo/pac4j
+                        return credential;
+                    }
                 }
                 // Type 2: institution SSO via pac4j authentication delegation using the CAS protocol
                 if (authnDelegationClients.get(INSTITUTION_CLIENTS_PARAMETER_NAME).contains(clientName)) {

src/main/java/org/pac4j/oauth/profile/creator/OAuth20ProfileCreator.java

@@ -15,7 +15,8 @@
 /**
  * OAuth 2.0 profile creator.
  *
- * OSF CAS Customization: modified {@code addAccessTokenToProfile} to include refresh token in profile attributes.
+ * <p>OSF CAS Customizations: modified {@link #addAccessTokenToProfile(OAuth20Profile, OAuth2AccessToken)} to include
+ * refresh token in profile attributes.</p>
  *
  * @author Jerome Leleu
  * @author Longze Chen
@@ -39,13 +40,19 @@ protected OAuth2AccessToken getAccessToken(final OAuth20Credentials credentials)
     @Override
     protected void addAccessTokenToProfile(final U profile, final OAuth2AccessToken accessToken) {
         if (profile != null) {
-            final String token = accessToken.getAccessToken();
-            logger.debug("add access_token: {} to profile", token);
-            profile.setAccessToken(token);
+            // Add access token
+            final String access_token = accessToken.getAccessToken();
+            logger.debug("[OAuth20 SSO] Add access token to profile: hasAccessToken=[{}]", StringUtils.isNotBlank(access_token));
+            profile.setAccessToken(access_token);
+
+            // Add refresh token manually instead of war-overlaying and customizing org.pac4j.oauth.profile.OAuth20Profile
             final String refreshToken = accessToken.getRefreshToken();
             if (StringUtils.isNoneBlank(refreshToken)) {
-                logger.debug("add refresh_token: {} to profile", token);
+                logger.debug("[OAuth20 SSO] Refresh token found, adding it to profile");
                 profile.addAttribute(REFRESH_TOKEN, refreshToken);
+            } else {
+                logger.debug("[OAuth20 SSO] Refresh token not found, adding empty value to profile");
+                profile.addAttribute(REFRESH_TOKEN, StringUtils.EMPTY);
             }
         }
     }