From 6bee6f05342d37fbc348a1db5c5e2986c65f4c0a Mon Sep 17 00:00:00 2001 From: gerlach Date: Tue, 5 May 2026 15:30:22 +0200 Subject: [PATCH] feat(auth): sync OIDC user properties --- doc/release-notes/10266-oidc-property-sync.md | 1 + .../source/installation/config.rst | 25 ++-- docker-compose-dev.yml | 1 + .../harvard/iq/dataverse/UserServiceBean.java | 15 ++ .../AuthenticationServiceBean.java | 91 +++++++++++- .../providers/builtin/DataverseUserPage.java | 15 +- .../AbstractOAuth2AuthenticationProvider.java | 4 +- .../oauth2/OAuth2FirstLoginPage.java | 2 +- .../providers/oauth2/OAuth2UserRecord.java | 17 ++- .../providers/oauth2/impl/OrcidOAuth2AP.java | 3 +- .../oauth2/oidc/OIDCAuthProvider.java | 11 +- .../iq/dataverse/settings/FeatureFlags.java | 18 ++- .../AuthenticationServiceBeanTest.java | 129 ++++++++++++++++++ .../oauth2/impl/GitHubOAuth2APTest.java | 2 +- 14 files changed, 307 insertions(+), 27 deletions(-) create mode 100644 doc/release-notes/10266-oidc-property-sync.md diff --git a/doc/release-notes/10266-oidc-property-sync.md b/doc/release-notes/10266-oidc-property-sync.md new file mode 100644 index 00000000000..907eaf6f68c --- /dev/null +++ b/doc/release-notes/10266-oidc-property-sync.md @@ -0,0 +1 @@ +OIDC user properties can now be synchronized with the Dataverse user account during authentication when the dataverse.feature.oidc-user-property-sync feature flag is enabled. This includes first name, last name, email address, and email verification status (mapped from the email_verified claim). \ No newline at end of file diff --git a/doc/sphinx-guides/source/installation/config.rst b/doc/sphinx-guides/source/installation/config.rst index e5ed52acb83..0e7f69e11d4 100644 --- a/doc/sphinx-guides/source/installation/config.rst +++ b/doc/sphinx-guides/source/installation/config.rst @@ -100,9 +100,9 @@ Using X-Forwarded-Proto for Signed URLs +++++++++++++++++++++++++++++++++++++++ If you use a proxy such as Apache or Nginx, or have a firewall such as Anubis, and they are configured to forward traffic to Dataverse over HTTP -(i.e. your proxy receives user calls over HTTPS but forwards locally to Dataverse over HTTP), signed URLs, used by external tools and +(i.e. your proxy receives user calls over HTTPS but forwards locally to Dataverse over HTTP), signed URLs, used by external tools and upload apps (such as DVWebloader), are likely to fail unless you configure your proxy to send an X-Forwarded-Proto HTTP Header. -This allows Dataverse to recognize that the communication from the user was over HTTPS and that validation of signed URLs should assume +This allows Dataverse to recognize that the communication from the user was over HTTPS and that validation of signed URLs should assume they started with https:// (rather than http:// as received from the proxy). .. _PrivacyConsiderations: @@ -2259,19 +2259,19 @@ These archival Bags include all of the files and metadata in a given dataset ver The Dataverse Software offers an internal archive workflow which may be configured as a PostPublication workflow via an admin API call to manually submit previously published Datasets and prior versions to a configured archive such as Chronopolis. The workflow creates a `JSON-LD `_ serialized `OAI-ORE `_ map file, which is also available as a metadata export format in the Dataverse Software web interface. -The size of the zipped archival Bag can be limited, and files that don't fit within that limit can either be transferred separately (placed so that they are correctly positioned according to the BagIt specification when the zipped bag in unzipped in place) or just referenced for later download (using the BagIt concept of a 'holey' bag with a list of files in a ``fetch.txt`` file) can now be configured for all archivers. These settings allow for managing large datasets by excluding files over a certain size or total data size, which can be useful for archivers with size limitations or to reduce transfer times. See the :ref:`dataverse.bagit.zip.max-file-size`, :ref:`dataverse.bagit.zip.max-data-size`, and :ref:`dataverse.bagit.zip.holey` JVM options for more details. +The size of the zipped archival Bag can be limited, and files that don't fit within that limit can either be transferred separately (placed so that they are correctly positioned according to the BagIt specification when the zipped bag in unzipped in place) or just referenced for later download (using the BagIt concept of a 'holey' bag with a list of files in a ``fetch.txt`` file) can now be configured for all archivers. These settings allow for managing large datasets by excluding files over a certain size or total data size, which can be useful for archivers with size limitations or to reduce transfer times. See the :ref:`dataverse.bagit.zip.max-file-size`, :ref:`dataverse.bagit.zip.max-data-size`, and :ref:`dataverse.bagit.zip.holey` JVM options for more details. -At present, archiving classes include the DuraCloudSubmitToArchiveCommand, LocalSubmitToArchiveCommand, GoogleCloudSubmitToArchive, and S3SubmitToArchiveCommand , which all extend the AbstractSubmitToArchiveCommand and use the configurable mechanisms discussed below. (A DRSSubmitToArchiveCommand, which works with Harvard's DRS also exists and, while specific to DRS, is a useful example of how Archivers can support single-version-only semantics and support archiving only from specified collections (with collection specific parameters)). +At present, archiving classes include the DuraCloudSubmitToArchiveCommand, LocalSubmitToArchiveCommand, GoogleCloudSubmitToArchive, and S3SubmitToArchiveCommand , which all extend the AbstractSubmitToArchiveCommand and use the configurable mechanisms discussed below. (A DRSSubmitToArchiveCommand, which works with Harvard's DRS also exists and, while specific to DRS, is a useful example of how Archivers can support single-version-only semantics and support archiving only from specified collections (with collection specific parameters)). All current options support the :ref:`Archival Status API` calls and the same status is available in the dataset page version table (for contributors/those who could view the unpublished dataset, with more detail available to superusers). Two settings that can be used with all current Archivers are: - \:BagGeneratorThreads - the number of threads to use when adding data files to the zipped bag. The default is 2. Values of 4 or more may increase performance on larger machines but may cause problems if file access is throttled -- \:ArchiveOnlyIfEarlierVersionsAreArchived - when true, requires dataset versions to be archived in order by confirming that all prior versions have been successfully archived before allowing a new version to be archived. Default is false +- \:ArchiveOnlyIfEarlierVersionsAreArchived - when true, requires dataset versions to be archived in order by confirming that all prior versions have been successfully archived before allowing a new version to be archived. Default is false These must be included in the \:ArchiverSettings for the Archiver to work - + Archival Bags are created per dataset version. By default, if a version is republished (via the superuser-only 'Update Current Version' publication option in the UI/API), a new archival bag is not created for the version. If the archiver used is capable of deleting existing bags (Google, S3, and File Archivers) superusers can trigger a manual update of the archival bag, and, if the :ref:`dataverse.bagit.archive-on-version-update` flag is set to true, this will be done automatically when 'Update Current Version' is used. @@ -3740,7 +3740,7 @@ i.e via the Update-Current-Version publication option. Setting the flag true onl dataverse.files.globus-monitoring-server ++++++++++++++++++++++++++++++++++++++++ -This setting is required in conjunction with the :ref:`dataverse.feature.globus-use-experimental-async-framework` feature flag. Setting it to true designates the Dataverse instance to serve as the dedicated polling server. It is needed so that the new framework can be used in a multi-node installation. +This setting is required in conjunction with the :ref:`dataverse.feature.globus-use-experimental-async-framework` feature flag. Setting it to true designates the Dataverse instance to serve as the dedicated polling server. It is needed so that the new framework can be used in a multi-node installation. .. _dataverse.csl.common-styles: @@ -3949,6 +3949,13 @@ dataverse.feature.api-bearer-auth-provide-missing-claims Enables sending missing user claims in the request JSON provided during OIDC user registration, when these claims are not returned by the identity provider and are required for registration. This feature only works when the feature flag ``api-bearer-auth`` is also enabled. **Caution: Enabling this feature flag exposes the installation to potential user impersonation issues.** +.. _dataverse.feature.oidc-user-property-sync: + +dataverse.feature.oidc-user-property-sync ++++++++++++++++++++++++++++++++++++++++++ + +Enables synchronization of user properties from the OIDC identity provider to the Dataverse user during authentication. When enabled, first name, last name, email address, and email verification state are updated from OIDC claims on each authenticated request. Updates are only applied if values have changed. The email verification state is mapped from the optional ``email_verified`` claim to Dataverse's internal ``emailConfirmed`` timestamp. + .. _dataverse.feature.api-bearer-auth-handle-tos-acceptance-in-idp: dataverse.feature.api-bearer-auth-handle-tos-acceptance-in-idp @@ -4725,7 +4732,7 @@ In the following example, the harvester is instructed to sleep for 900 milliseco ``curl -X PUT -d "{\"harvarddv\": 0.9, \"default\": 0}" "http://localhost:8080/api/admin/settings/:HarvestingClientCallRateLimit"`` -Please note that the default in the example above is there for illustrative purposes and is otherwise redundant, since no sleep interval is the default behavior anyway. +Please note that the default in the example above is there for illustrative purposes and is otherwise redundant, since no sleep interval is the default behavior anyway. .. _:ZipUploadFilesLimit: @@ -5417,7 +5424,7 @@ For examples, see the specific configuration above in :ref:`BagIt Export`. ++++++++++++++++++++++++++++++++++++++++ This setting, if true, only allows creation of an archival Bag for a dataset version if all prior versions have been successfully archived. The default is false (any version can be archived independently as long as other settings allow it) - + :ArchiverSettings +++++++++++++++++ diff --git a/docker-compose-dev.yml b/docker-compose-dev.yml index 88b902dfc7f..f16332eb5fc 100644 --- a/docker-compose-dev.yml +++ b/docker-compose-dev.yml @@ -19,6 +19,7 @@ services: DATAVERSE_FEATURE_API_BEARER_AUTH: "1" DATAVERSE_FEATURE_INDEX_HARVESTED_METADATA_SOURCE: "1" DATAVERSE_FEATURE_API_BEARER_AUTH_PROVIDE_MISSING_CLAIMS: "1" + DATAVERSE_FEATURE_OIDC_USER_PROPERTY_SYNC: "1" DATAVERSE_MAIL_SYSTEM_EMAIL: "dataverse@localhost" DATAVERSE_MAIL_MTA_HOST: "smtp" DATAVERSE_AUTH_OIDC_ENABLED: "1" diff --git a/src/main/java/edu/harvard/iq/dataverse/UserServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/UserServiceBean.java index d63fcfa3e34..d69b8d7f9f5 100644 --- a/src/main/java/edu/harvard/iq/dataverse/UserServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/UserServiceBean.java @@ -18,6 +18,7 @@ import jakarta.ejb.TransactionAttributeType; import jakarta.inject.Named; import jakarta.persistence.EntityManager; +import jakarta.persistence.NoResultException; import jakarta.persistence.PersistenceContext; import jakarta.persistence.Query; import org.apache.commons.lang3.StringUtils; @@ -537,4 +538,18 @@ public AuthenticatedUser updateLastApiUseTime(AuthenticatedUser user) { user.setLastApiUseTime(new Timestamp(new Date().getTime())); return save(user); } + + public AuthenticatedUser findFresh(Long id) { + try { + return em.createQuery( + "SELECT u FROM AuthenticatedUser u " + + "LEFT JOIN FETCH u.authenticatedUserLookup " + + "WHERE u.id = :id", AuthenticatedUser.class) + .setParameter("id", id) + .setHint("jakarta.persistence.cache.retrieveMode", "BYPASS") + .getSingleResult(); + } catch (NoResultException e) { + return null; + } + } } diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/AuthenticationServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/authorization/AuthenticationServiceBean.java index 20060faba90..0e70b35b9d3 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/AuthenticationServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/AuthenticationServiceBean.java @@ -66,6 +66,7 @@ import jakarta.validation.Validation; import jakarta.validation.Validator; import jakarta.validation.ValidatorFactory; +import org.apache.commons.lang3.StringUtils; /** * AuthenticationService is for general authentication-related operations. @@ -1024,24 +1025,106 @@ public AuthenticatedUser lookupUserByOIDCBearerToken(String bearerToken) throws authenticatedUser = lookupUser(ShibAuthenticationProvider.PROVIDER_ID, userPersistentId); if (authenticatedUser != null) { logger.log(Level.FINE, "Shibboleth user found for the given bearer token"); - return authenticatedUser; + return syncUserProperties(authenticatedUser, oAuth2UserRecord); } } else if (FeatureFlags.API_BEARER_AUTH_USE_OAUTH_USER_ON_ID_MATCH.enabled() && oAuth2UserRecord.hasOAuthAttributes()) { OAuthUserLookupParams userLookupParams = OAuthUserLookupParamsFactory.getOAuthUserLookupParams(oAuth2UserRecord.getIdp(), oAuth2UserRecord.getOidcUserId()); authenticatedUser = lookupUser(userLookupParams.getProviderId(), userLookupParams.getLookupUserId()); if (authenticatedUser != null) { logger.log(Level.FINE, "OAuth user found for the given bearer token"); - return authenticatedUser; + return syncUserProperties(authenticatedUser, oAuth2UserRecord); } } else if (FeatureFlags.API_BEARER_AUTH_USE_BUILTIN_USER_ON_ID_MATCH.enabled() && oAuth2UserRecord.hasBuiltinAttributes()) { authenticatedUser = lookupUser(BuiltinAuthenticationProvider.PROVIDER_ID, oAuth2UserRecord.getUsername()); if (authenticatedUser != null) { logger.log(Level.FINE, "Builtin user found for the given bearer token"); - return authenticatedUser; + return syncUserProperties(authenticatedUser, oAuth2UserRecord); } } - return lookupUser(oAuth2UserRecord.getUserRecordIdentifier()); + authenticatedUser = lookupUser(oAuth2UserRecord.getUserRecordIdentifier()); + // fallback + if (authenticatedUser != null) { + authenticatedUser = syncUserProperties(authenticatedUser, oAuth2UserRecord); + } + return authenticatedUser; + } + + private AuthenticatedUser syncUserProperties(AuthenticatedUser user, OAuth2UserRecord record) { + if (!FeatureFlags.OIDC_USER_PROPERTY_SYNC.enabled()) { + logger.fine("OIDC user property sync is disabled (feature flag not raised)"); + return user; + } + + boolean changed = false; + + if (record.getDisplayInfo() != null) { + String newFirstName = StringUtils.trimToNull(record.getDisplayInfo().getFirstName()); + String newLastName = StringUtils.trimToNull(record.getDisplayInfo().getLastName()); + + if (newFirstName != null && !newFirstName.equals(user.getFirstName())) { + user.setFirstName(newFirstName); + changed = true; + } + + if (newLastName != null && !newLastName.equals(user.getLastName())) { + user.setLastName(newLastName); + changed = true; + } + } + + String newEmail = StringUtils.trimToNull(resolveEmail(record)); + if (newEmail != null && !equalsIgnoreCaseSafe(newEmail, user.getEmail())) { + user.setEmail(newEmail); + changed = true; + } + + Boolean emailVerified = record.getEmailVerified(); + if (emailVerified != null) { + if (emailVerified && user.getEmailConfirmed() == null) { + user.setEmailConfirmed(new Timestamp(System.currentTimeMillis())); + changed = true; + } else if (!emailVerified && user.getEmailConfirmed() != null) { + user.setEmailConfirmed(null); + changed = true; + } + } + + if (changed) { + return userService.save(user); + } + + return user; + } + + private String resolveEmail(OAuth2UserRecord record) { + if (record.getDisplayInfo() != null) { + String displayEmail = StringUtils.trimToNull(record.getDisplayInfo().getEmailAddress()); + if (displayEmail != null) { + return displayEmail; + } + } + // fallback + return StringUtils.trimToNull(getFirstEmail(record)); + } + + + private String getFirstEmail(OAuth2UserRecord record) { + List emails = record.getAvailableEmailAddresses(); + if (emails == null || emails.isEmpty()) { + return null; + } + return emails.get(0); + } + + /** + * Null-safe, case-insensitive comparison for email addresses. + */ + private boolean equalsIgnoreCaseSafe(String a, String b) { + if (a == null || b == null) { + return a == b; + } + return a.equalsIgnoreCase(b); } /** diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/DataverseUserPage.java b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/DataverseUserPage.java index 3de54cd6983..87fc95416fb 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/DataverseUserPage.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/DataverseUserPage.java @@ -179,8 +179,17 @@ public String init() { } if (session.getUser(true).isAuthenticated()) { - setCurrentUser((AuthenticatedUser) session.getUser()); - userAuthProvider = authenticationService.lookupProvider(currentUser); + AuthenticatedUser sessionUser = (AuthenticatedUser) session.getUser(); + AuthenticatedUser freshUser = userService.findFresh(sessionUser.getId()); + if (freshUser != null) { + // Update properties on the session instance directly instead of replacing it + // (via using Session.setUser()) to avoid session ID change which would invalidate the ViewScoped bean. + sessionUser.setFirstName(freshUser.getFirstName()); + sessionUser.setLastName(freshUser.getLastName()); + sessionUser.setEmail(freshUser.getEmail()); + } + setCurrentUser(freshUser != null ? freshUser : sessionUser); + userAuthProvider = authenticationService.lookupProvider(sessionUser); notificationsList = userNotificationService.findByUser(currentUser.getId()); notificationTypeList = Arrays.asList(Type.values()).stream() .filter(x -> !Type.CONFIRMEMAIL.equals(x) && x.hasDescription() && !settingsWrapper.isAlwaysMuted(x)) @@ -609,7 +618,7 @@ public boolean isEmailGrandfathered() { } public AuthenticationProvider getUserAuthProvider() { - if ( userAuthProvider == null ) { + if (userAuthProvider == null && currentUser != null) { userAuthProvider = authenticationService.lookupProvider(currentUser); } return userAuthProvider; diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/AbstractOAuth2AuthenticationProvider.java b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/AbstractOAuth2AuthenticationProvider.java index 89e264c936b..d0aee120d10 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/AbstractOAuth2AuthenticationProvider.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/AbstractOAuth2AuthenticationProvider.java @@ -197,7 +197,9 @@ protected OAuth2UserRecord getUserRecord(@NotNull String responseBody, @NotNull parsed.username, OAuth2TokenData.from(accessToken), parsed.displayInfo, - parsed.emails); + parsed.emails, + null + ); } @Override diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2FirstLoginPage.java b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2FirstLoginPage.java index b8cb297ada4..102f4d374d1 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2FirstLoginPage.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2FirstLoginPage.java @@ -144,7 +144,7 @@ public void init() throws IOException { accessToken.setAccessToken("qwe-addssd-iiiiie"); setNewUser(new OAuth2UserRecord(authProviderId, eppn, randomUsername, accessToken, new AuthenticatedUserDisplayInfo(firstName, lastName, email, "myAffiliation", "myPosition"), - extraEmails)); + extraEmails, null)); } } diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2UserRecord.java b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2UserRecord.java index 6667d8551ca..81861b3ab1d 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2UserRecord.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2UserRecord.java @@ -34,6 +34,7 @@ public class OAuth2UserRecord implements Serializable { */ public static final String OIDC_USER_ID_CLAIM_NAME = "oidc"; public static final String IDP_CLAIM_NAME = "idp"; + public static final String EMAIL_VERIFIED_CLAIM_NAME = "email_verified"; private final String serviceId; @@ -61,6 +62,7 @@ public class OAuth2UserRecord implements Serializable { private final AuthenticatedUserDisplayInfo displayInfo; private final List availableEmailAddresses; private final OAuth2TokenData tokenData; + private final Boolean emailVerified; /** * Constructor for users without Shibboleth attributes. @@ -71,9 +73,10 @@ public OAuth2UserRecord( String username, OAuth2TokenData tokenData, AuthenticatedUserDisplayInfo displayInfo, - List availableEmailAddresses - ) { - this(serviceId, idInService, username, null, null, null, tokenData, displayInfo, availableEmailAddresses); + List availableEmailAddresses, + Boolean emailVerified + ) { + this(serviceId, idInService, username, null, null, null, tokenData, displayInfo, availableEmailAddresses, emailVerified); } /** @@ -88,7 +91,8 @@ public OAuth2UserRecord( String oidcUserId, OAuth2TokenData tokenData, AuthenticatedUserDisplayInfo displayInfo, - List availableEmailAddresses + List availableEmailAddresses, + Boolean emailVerified ) { this.serviceId = serviceId; this.idInService = idInService; @@ -99,6 +103,7 @@ public OAuth2UserRecord( this.tokenData = tokenData; this.displayInfo = displayInfo; this.availableEmailAddresses = availableEmailAddresses; + this.emailVerified = emailVerified; } public String getServiceId() { @@ -129,6 +134,10 @@ public List getAvailableEmailAddresses() { return availableEmailAddresses; } + public Boolean getEmailVerified() { + return emailVerified; + } + public AuthenticatedUserDisplayInfo getDisplayInfo() { return displayInfo; } diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/impl/OrcidOAuth2AP.java b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/impl/OrcidOAuth2AP.java index f32ff2b18e5..37f965d44ba 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/impl/OrcidOAuth2AP.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/impl/OrcidOAuth2AP.java @@ -108,7 +108,8 @@ final protected OAuth2UserRecord getUserRecord(@NotNull String responseBody, @No parsed.username, OAuth2TokenData.from(accessToken), parsed.displayInfo, - parsed.emails); + parsed.emails, + null); } @Override diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/oidc/OIDCAuthProvider.java b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/oidc/OIDCAuthProvider.java index 3e62598ee79..1453a047a8c 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/oidc/OIDCAuthProvider.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/oidc/OIDCAuthProvider.java @@ -38,6 +38,7 @@ import edu.harvard.iq.dataverse.authorization.providers.oauth2.OAuth2Exception; import edu.harvard.iq.dataverse.authorization.providers.oauth2.OAuth2UserRecord; import edu.harvard.iq.dataverse.authorization.providers.shib.ShibUtil; +import edu.harvard.iq.dataverse.settings.FeatureFlags; import edu.harvard.iq.dataverse.settings.JvmSettings; import edu.harvard.iq.dataverse.util.BundleUtil; @@ -241,6 +242,13 @@ public OAuth2UserRecord getUserRecord(UserInfo userInfo) { String idp = (idpObj != null) ? idpObj.toString() : null; String oidcUserId = (oidcUserIdObj != null) ? oidcUserIdObj.toString() : null; + // Only extract email_verified claim if sync feature is enabled + Boolean emailVerified = null; + if (FeatureFlags.OIDC_USER_PROPERTY_SYNC.enabled()) { + Object emailVerifiedObj = userInfo.getClaim(OAuth2UserRecord.EMAIL_VERIFIED_CLAIM_NAME); + emailVerified = emailVerifiedObj instanceof Boolean ? (Boolean) emailVerifiedObj : null; + } + // Build display info from user attributes AuthenticatedUserDisplayInfo displayInfo = new AuthenticatedUserDisplayInfo( userInfo.getGivenName(), @@ -259,7 +267,8 @@ public OAuth2UserRecord getUserRecord(UserInfo userInfo) { oidcUserId, null, displayInfo, - null + null, + emailVerified ); } diff --git a/src/main/java/edu/harvard/iq/dataverse/settings/FeatureFlags.java b/src/main/java/edu/harvard/iq/dataverse/settings/FeatureFlags.java index e1c7e69f7db..1019483bd83 100644 --- a/src/main/java/edu/harvard/iq/dataverse/settings/FeatureFlags.java +++ b/src/main/java/edu/harvard/iq/dataverse/settings/FeatureFlags.java @@ -249,11 +249,25 @@ public enum FeatureFlags { * @since Dataverse 6.9 */ ONLY_UPDATE_DATACITE_WHEN_NEEDED("only-update-datacite-when-needed"), - - /** Require Embargo Reason. By default, adding a reason when embargoing is optional. This + + /** Require Embargo Reason. By default, adding a reason when embargoing is optional. This * flag makes a reason required, both in the UI and API. */ REQUIRE_EMBARGO_REASON("require-embargo-reason"), + + /** + * Enables OIDC user property synchronization from the IdP to Dataverse. + * When enabled, user properties (first name, last name, email, email_verified) + * are updated from OIDC UserInfo claims every time the user makes an authenticated request, + * keeping Dataverse data in sync with the IdP. + * When disabled (default), user properties are only set during initial account + * creation and are never updated afterward, even if they change at the IdP. + + * @apiNote Raise flag by setting "dataverse.feature.oidc-user-property-sync" + * @since Dataverse 6.10 + */ + OIDC_USER_PROPERTY_SYNC("oidc-user-property-sync"), + ; final String flag; diff --git a/src/test/java/edu/harvard/iq/dataverse/authorization/AuthenticationServiceBeanTest.java b/src/test/java/edu/harvard/iq/dataverse/authorization/AuthenticationServiceBeanTest.java index 56a46b4f772..a425d445765 100644 --- a/src/test/java/edu/harvard/iq/dataverse/authorization/AuthenticationServiceBeanTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/authorization/AuthenticationServiceBeanTest.java @@ -3,6 +3,7 @@ import com.nimbusds.oauth2.sdk.ParseException; import com.nimbusds.oauth2.sdk.token.BearerAccessToken; import com.nimbusds.openid.connect.sdk.claims.UserInfo; +import edu.harvard.iq.dataverse.UserServiceBean; import edu.harvard.iq.dataverse.authorization.exceptions.AuthorizationException; import edu.harvard.iq.dataverse.authorization.providers.builtin.BuiltinAuthenticationProvider; import edu.harvard.iq.dataverse.authorization.providers.oauth2.OAuth2Exception; @@ -30,6 +31,7 @@ import org.mockito.Mockito; import java.io.IOException; +import java.util.List; import java.util.Map; import java.util.Optional; import java.util.stream.Stream; @@ -50,6 +52,9 @@ public void setUp() { sut = new AuthenticationServiceBean(); sut.authProvidersRegistrationService = Mockito.mock(AuthenticationProvidersRegistrationServiceBean.class); sut.em = Mockito.mock(EntityManager.class); + sut.userService = Mockito.mock(UserServiceBean.class); + Mockito.when(sut.userService.save(Mockito.any(AuthenticatedUser.class))) + .thenAnswer(invocation -> invocation.getArgument(0)); } @Test @@ -125,6 +130,130 @@ void testLookupUserByOIDCBearerToken_oneProvider_validToken_noAccount() throws P assertNull(actualUser); } + @Test + @JvmSetting(key = JvmSettings.FEATURE_FLAG, value = "true", varArgs = "oidc-user-property-sync") + void testLookupUserByOIDCBearerToken_oneProvider_validToken_updatesUserProperties() throws ParseException, IOException, AuthorizationException, OAuth2Exception { + // Given a single OIDC provider that returns a valid user identifier with updated properties + OAuth2UserRecord record = setupOidcUserRecordBasics(); + AuthenticatedUserDisplayInfo displayInfo = new AuthenticatedUserDisplayInfo("NewFirst", "NewLast", "new@example.org", "", ""); + Mockito.when(record.getDisplayInfo()).thenReturn(displayInfo); + Mockito.when(record.getAvailableEmailAddresses()).thenReturn(List.of("new@example.org")); + + // Setting up an authenticated user with outdated properties + AuthenticatedUser oldAuthenticatedUser = new AuthenticatedUser(); + oldAuthenticatedUser.setFirstName("OldFirst"); + oldAuthenticatedUser.setLastName("OldLast"); + oldAuthenticatedUser.setEmail("old@example.org"); + setupAuthenticatedUserByAuthPrvIDQueryWithResult(oldAuthenticatedUser); + + // When invoking lookupUserByOIDCBearerToken + AuthenticatedUser actualUser = sut.lookupUserByOIDCBearerToken(TEST_BEARER_TOKEN); + + // Verify that save() was called with the updated user + Mockito.verify(sut.userService, Mockito.times(1)).save(Mockito.any()); + + // Then the user properties should be updated to match the OIDC token + assertEquals("NewFirst", actualUser.getFirstName()); + assertEquals("NewLast", actualUser.getLastName()); + assertEquals("new@example.org", actualUser.getEmail()); + } + + @Test + @JvmSetting(key = JvmSettings.FEATURE_FLAG, value = "true", varArgs = "oidc-user-property-sync") + void testLookupUserByOIDCBearerToken_oneProvider_validToken_noPropertyChanges_doesNotSave() throws ParseException, IOException, AuthorizationException, OAuth2Exception { + // Given a single OIDC provider that returns a valid user identifier with same properties + OAuth2UserRecord record = setupOidcUserRecordBasics(); + AuthenticatedUserDisplayInfo displayInfo = new AuthenticatedUserDisplayInfo("SameFirst", "SameLast", "same@example.org", "", ""); + Mockito.when(record.getDisplayInfo()).thenReturn(displayInfo); + Mockito.when(record.getAvailableEmailAddresses()).thenReturn(List.of("same@example.org")); + + // Setting up an authenticated user with identical properties + AuthenticatedUser authenticatedUser = new AuthenticatedUser(); + authenticatedUser.setFirstName("SameFirst"); + authenticatedUser.setLastName("SameLast"); + authenticatedUser.setEmail("same@example.org"); + setupAuthenticatedUserByAuthPrvIDQueryWithResult(authenticatedUser); + + // When invoking lookupUserByOIDCBearerToken + sut.lookupUserByOIDCBearerToken(TEST_BEARER_TOKEN); + + // Then save should never be called since nothing changed + Mockito.verify(sut.userService, Mockito.never()).save(Mockito.any()); + } + + @Test + @JvmSetting(key = JvmSettings.FEATURE_FLAG, value = "true", varArgs = "oidc-user-property-sync") + void testLookupUserByOIDCBearerToken_oneProvider_validToken_onlyEmailChanges() throws ParseException, IOException, AuthorizationException, OAuth2Exception { + // Given a single OIDC provider that returns a valid user identifier with updated email only + OAuth2UserRecord record = setupOidcUserRecordBasics(); + AuthenticatedUserDisplayInfo displayInfo = new AuthenticatedUserDisplayInfo("SameFirst", "SameLast", "new@example.org", "", ""); + Mockito.when(record.getDisplayInfo()).thenReturn(displayInfo); + Mockito.when(record.getAvailableEmailAddresses()).thenReturn(List.of("new@example.org")); + + // Setting up an authenticated user with the same name but different email + AuthenticatedUser authenticatedUser = new AuthenticatedUser(); + authenticatedUser.setFirstName("SameFirst"); + authenticatedUser.setLastName("SameLast"); + authenticatedUser.setEmail("old@example.org"); + setupAuthenticatedUserByAuthPrvIDQueryWithResult(authenticatedUser); + + // When invoking lookupUserByOIDCBearerToken + AuthenticatedUser actualUser = sut.lookupUserByOIDCBearerToken(TEST_BEARER_TOKEN); + + // Then only the email should be updated + assertEquals("SameFirst", actualUser.getFirstName()); + assertEquals("SameLast", actualUser.getLastName()); + assertEquals("new@example.org", actualUser.getEmail()); + } + + @Test + @JvmSetting(key = JvmSettings.FEATURE_FLAG, value = "true", varArgs = "oidc-user-property-sync") + void testLookupUserByOIDCBearerToken_oneProvider_validToken_displayInfoNull_doesNotUpdateNames() throws ParseException, IOException, AuthorizationException, OAuth2Exception { + // Given a single OIDC provider that returns no display info but a valid email + OAuth2UserRecord record = setupOidcUserRecordBasics(); + Mockito.when(record.getDisplayInfo()).thenReturn(null); + Mockito.when(record.getAvailableEmailAddresses()).thenReturn(List.of("new@example.org")); + + // Setting up an authenticated user with existing properties + AuthenticatedUser authenticatedUser = new AuthenticatedUser(); + authenticatedUser.setFirstName("ExistingFirst"); + authenticatedUser.setLastName("ExistingLast"); + authenticatedUser.setEmail("old@example.org"); + setupAuthenticatedUserByAuthPrvIDQueryWithResult(authenticatedUser); + + // When invoking lookupUserByOIDCBearerToken + AuthenticatedUser actualUser = sut.lookupUserByOIDCBearerToken(TEST_BEARER_TOKEN); + + // Then names should remain unchanged since displayInfo is null + assertEquals("ExistingFirst", actualUser.getFirstName()); + assertEquals("ExistingLast", actualUser.getLastName()); + // But email should still be updated + assertEquals("new@example.org", actualUser.getEmail()); + } + + @Test + @JvmSetting(key = JvmSettings.FEATURE_FLAG, value = "true", varArgs = "oidc-user-property-sync") + void testLookupUserByOIDCBearerToken_oneProvider_validToken_emailNullInRecord_doesNotUpdateEmail() throws ParseException, IOException, AuthorizationException, OAuth2Exception { + // Given a single OIDC provider that returns a valid user identifier with no email + OAuth2UserRecord record = setupOidcUserRecordBasics(); + AuthenticatedUserDisplayInfo displayInfo = new AuthenticatedUserDisplayInfo("NewFirst", "NewLast", "", "", ""); + Mockito.when(record.getDisplayInfo()).thenReturn(displayInfo); + Mockito.when(record.getAvailableEmailAddresses()).thenReturn(null); + + // Setting up an authenticated user with an existing email + AuthenticatedUser authenticatedUser = new AuthenticatedUser(); + authenticatedUser.setFirstName("OldFirst"); + authenticatedUser.setLastName("OldLast"); + authenticatedUser.setEmail("existing@example.org"); + setupAuthenticatedUserByAuthPrvIDQueryWithResult(authenticatedUser); + + // When invoking lookupUserByOIDCBearerToken + AuthenticatedUser actualUser = sut.lookupUserByOIDCBearerToken(TEST_BEARER_TOKEN); + + // Then email should remain unchanged + assertEquals("existing@example.org", actualUser.getEmail()); + } + @Test @JvmSetting(key = JvmSettings.FEATURE_FLAG, value = "true", varArgs = "api-bearer-auth-use-builtin-user-on-id-match") void testLookupUserByOIDCBearerToken_oneProvider_validToken_userNotPresentAsBuiltin_useBuiltinUserOnIdMatchFeatureFlagEnabled() diff --git a/src/test/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/impl/GitHubOAuth2APTest.java b/src/test/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/impl/GitHubOAuth2APTest.java index ed6b9789848..affe02bce5d 100644 --- a/src/test/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/impl/GitHubOAuth2APTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/impl/GitHubOAuth2APTest.java @@ -62,7 +62,7 @@ public void testParseUserResponse() { public OAuth2UserRecord getExampleUserRecord() { ParsedUserResponse res = parseUserResponse(GITHUB_RESPONSE); - return new OAuth2UserRecord(this.getId(), res.userIdInProvider, res.username, null, res.displayInfo, res.emails); + return new OAuth2UserRecord(this.getId(), res.userIdInProvider, res.username, null, res.displayInfo, res.emails, null); } }