From ffea32539d8b2fdf995463eafc31c917b029a060 Mon Sep 17 00:00:00 2001 From: cryptalith Date: Thu, 24 Sep 2026 17:32:39 -0600 Subject: [PATCH 1/4] PD-5692 read and clear the mandatory password reset flag --- .../v3/impl/ProfileEntityManagerImpl.java | 4 ++ .../ProfileEntityManagerReadOnly.java | 7 +++ .../ProfileEntityManagerReadOnlyImpl.java | 8 +++ .../java/org/orcid/core/togglz/Features.java | 5 +- .../v3/impl/ProfileEntityManagerImplTest.java | 59 +++++++++++++++++++ .../org/orcid/persistence/dao/ProfileDao.java | 11 ++++ .../persistence/dao/impl/ProfileDaoImpl.java | 12 +++- .../orcid/persistence/dao/ProfileDaoTest.java | 40 +++++++++++++ 8 files changed, 144 insertions(+), 2 deletions(-) diff --git a/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/ProfileEntityManagerImpl.java b/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/ProfileEntityManagerImpl.java index 02b16d46b30..a362e377654 100644 --- a/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/ProfileEntityManagerImpl.java +++ b/orcid-core/src/main/java/org/orcid/core/manager/v3/impl/ProfileEntityManagerImpl.java @@ -394,6 +394,8 @@ public boolean claimProfileAndUpdatePreferences(String orcid, String email, Avai profile.setClaimed(true); profile.setCompletedDate(new Date()); profile.setEncryptedPassword(encryptionManager.hashForInternalUse(claim.getPassword().getValue())); + // A new password satisfies a mandatory password reset + profile.setForcePasswordReset(null); if (locale != null) { profile.setLocale(locale.name()); } @@ -505,6 +507,8 @@ public Boolean doInTransaction(TransactionStatus status) { profileEntity.setIndexingStatus(IndexingStatus.PENDING); if (reactivation != null) { profileEntity.setEncryptedPassword(encryptionManager.hashForInternalUse(reactivation.getPassword().getValue())); + // A new password satisfies a mandatory password reset + profileEntity.setForcePasswordReset(null); profileEntity.setActivitiesVisibilityDefault(reactivation.getActivitiesVisibilityDefault().getVisibility().name()); } profileDao.merge(profileEntity); diff --git a/orcid-core/src/main/java/org/orcid/core/manager/v3/read_only/ProfileEntityManagerReadOnly.java b/orcid-core/src/main/java/org/orcid/core/manager/v3/read_only/ProfileEntityManagerReadOnly.java index 8235234bc4a..469f342d46c 100644 --- a/orcid-core/src/main/java/org/orcid/core/manager/v3/read_only/ProfileEntityManagerReadOnly.java +++ b/orcid-core/src/main/java/org/orcid/core/manager/v3/read_only/ProfileEntityManagerReadOnly.java @@ -15,4 +15,11 @@ public interface ProfileEntityManagerReadOnly extends ManagerReadOnlyBase { Boolean haveMemberPushedWorksOrAffiliationsToRecord(String orcid, String clientId); Boolean hasToken(String userName, long lastModified); + + /** + * Whether the record has to reset its password before it can sign in with + * one. Never cached: which database answers depends on the bean, the + * writable manager reads the primary and the read-only one the replica. + */ + boolean isPasswordResetRequired(String orcid); } \ No newline at end of file diff --git a/orcid-core/src/main/java/org/orcid/core/manager/v3/read_only/impl/ProfileEntityManagerReadOnlyImpl.java b/orcid-core/src/main/java/org/orcid/core/manager/v3/read_only/impl/ProfileEntityManagerReadOnlyImpl.java index 76593c005a1..455f6fcf38d 100644 --- a/orcid-core/src/main/java/org/orcid/core/manager/v3/read_only/impl/ProfileEntityManagerReadOnlyImpl.java +++ b/orcid-core/src/main/java/org/orcid/core/manager/v3/read_only/impl/ProfileEntityManagerReadOnlyImpl.java @@ -76,6 +76,14 @@ public Boolean haveMemberPushedWorksOrAffiliationsToRecord(String orcid, String return profileDao.haveMemberPushedWorksOrAffiliationsToRecord(orcid, clientId); } + @Override + public boolean isPasswordResetRequired(String orcid) { + if (PojoUtil.isEmpty(orcid)) { + return false; + } + return profileDao.isPasswordResetRequired(orcid); + } + @Override @Cacheable(value = "count-tokens", key = "#userName.concat('-').concat(#lastModified)") public Boolean hasToken(String userName, long lastModified) { diff --git a/orcid-core/src/main/java/org/orcid/core/togglz/Features.java b/orcid-core/src/main/java/org/orcid/core/togglz/Features.java index 16536948ff1..50c6d70b202 100644 --- a/orcid-core/src/main/java/org/orcid/core/togglz/Features.java +++ b/orcid-core/src/main/java/org/orcid/core/togglz/Features.java @@ -119,7 +119,10 @@ public enum Features implements Feature { TWO_FACTOR_RECOVERY_PHONE, @Label("Login - recovery phone interstitial") - LOGIN_RECOVERY_PHONE_INTERSTITIAL; + LOGIN_RECOVERY_PHONE_INTERSTITIAL, + + @Label("Mandatory password reset: block password sign in and show the reset notice for records flagged with force_password_reset") + FORCE_PASSWORD_RESET; public boolean isActive() { return FeatureContext.getFeatureManager().isActive(this); } diff --git a/orcid-core/src/test/java/org/orcid/core/manager/v3/impl/ProfileEntityManagerImplTest.java b/orcid-core/src/test/java/org/orcid/core/manager/v3/impl/ProfileEntityManagerImplTest.java index 8c36e9f564d..a9f343f7a5c 100644 --- a/orcid-core/src/test/java/org/orcid/core/manager/v3/impl/ProfileEntityManagerImplTest.java +++ b/orcid-core/src/test/java/org/orcid/core/manager/v3/impl/ProfileEntityManagerImplTest.java @@ -574,6 +574,22 @@ public void claimProfileAndUpdatePreferencesUpdatesExistingFrequencySetting() { verify(emailFrequencyManager, never()).createOnClaim(anyString(), anyBoolean()); } + @Test + public void claimProfileAndUpdatePreferencesClearsForcePasswordReset() { + when(emailManager.verifySetCurrentAndPrimary("orcid", "a@b.com")).thenReturn(true); + ProfileEntity profile = new ProfileEntity(); + profile.setForcePasswordReset(new Date()); + when(profileDao.find("orcid")).thenReturn(profile); + when(encryptionManager.hashForInternalUse("password#1")).thenReturn("enc"); + when(biographyManager.exists("orcid")).thenReturn(false); + when(emailFrequencyManager.emailFrequencyExists("orcid")).thenReturn(true); + + manager.claimProfileAndUpdatePreferences("orcid", "a@b.com", null, claim(false)); + + assertNull(profile.getForcePasswordReset()); + verify(profileDao).merge(profile); + } + @Test public void updateLocaleDelegates() { manager.updateLocale("orcid", AvailableLocales.EN); @@ -631,6 +647,49 @@ public void reactivateFromUserPathRecreatesAdditionalEmailsUpdatesNameAndReturns verify(emailManager).clearEmailsAfterReactivation("orcid"); } + @Test + public void reactivateFromUserPathClearsForcePasswordReset() { + ProfileEntity profile = new ProfileEntity(); + profile.setDeactivationDate(new Date()); + profile.setForcePasswordReset(new Date()); + when(profileDao.find("orcid")).thenReturn(profile); + when(encryptionManager.hashForInternalUse("reactivation#1")).thenReturn("encrypted"); + when(recordNameManagerReadOnlyV3.getRecordName("orcid")).thenReturn(new Name()); + + manager.reactivate("orcid", "primary@test.org", reactivation()); + + assertEquals("encrypted", profile.getEncryptedPassword()); + assertNull(profile.getForcePasswordReset()); + verify(profileDao).merge(profile); + } + + @Test + public void reactivateFromAdminPathKeepsForcePasswordReset() { + // An admin reactivation keeps the old password, so the reset is still owed + ProfileEntity profile = new ProfileEntity(); + profile.setDeactivationDate(new Date()); + Date flagged = new Date(); + profile.setForcePasswordReset(flagged); + when(profileDao.find("orcid")).thenReturn(profile); + + manager.reactivate("orcid", "primary@test.org", null); + + assertEquals(flagged, profile.getForcePasswordReset()); + } + + @Test + public void isPasswordResetRequiredDelegates() { + when(profileDao.isPasswordResetRequired("orcid")).thenReturn(true); + assertTrue(manager.isPasswordResetRequired("orcid")); + } + + @Test + public void isPasswordResetRequiredIsFalseForAnEmptyOrcid() { + assertFalse(manager.isPasswordResetRequired("")); + assertFalse(manager.isPasswordResetRequired(null)); + verify(profileDao, never()).isPasswordResetRequired(anyString()); + } + @Test public void updatePasswordHashesAndPersists() { when(encryptionManager.hashForInternalUse("password")).thenReturn("encrypted"); diff --git a/orcid-persistence/src/main/java/org/orcid/persistence/dao/ProfileDao.java b/orcid-persistence/src/main/java/org/orcid/persistence/dao/ProfileDao.java index fd502b149eb..bc3291c441c 100644 --- a/orcid-persistence/src/main/java/org/orcid/persistence/dao/ProfileDao.java +++ b/orcid-persistence/src/main/java/org/orcid/persistence/dao/ProfileDao.java @@ -209,6 +209,17 @@ public interface ProfileDao extends GenericDao { @Transactional(propagation = Propagation.REQUIRED) int updateForcePasswordReset(List ids, Date forcePasswordResetDate); + /** + * Whether the record must reset its password before it can sign in with + * one: the force_password_reset date is set on a claimed record that is + * neither deactivated nor deprecated. A narrow, uncached read, because the + * profile entity cache is keyed on last_modified and setting or clearing + * the flag does not change it. Which database answers depends on the bean: + * profileDao reads the primary, profileDaoReadOnly the replica. + */ + @Transactional(value = "transactionManagerReadOnly", readOnly = true) + boolean isPasswordResetRequired(String orcid); + @Transactional(value = "transactionManagerReadOnly", readOnly = true) public List registeredBetween(Date startDate, Date endDate); diff --git a/orcid-persistence/src/main/java/org/orcid/persistence/dao/impl/ProfileDaoImpl.java b/orcid-persistence/src/main/java/org/orcid/persistence/dao/impl/ProfileDaoImpl.java index ed553e0f094..ed0286bd610 100644 --- a/orcid-persistence/src/main/java/org/orcid/persistence/dao/impl/ProfileDaoImpl.java +++ b/orcid-persistence/src/main/java/org/orcid/persistence/dao/impl/ProfileDaoImpl.java @@ -494,7 +494,8 @@ public String retrievePrimaryAccountOrcid(String deprecatedOrcid) { @Override @Transactional public void changeEncryptedPassword(String orcid, String encryptedPassword) { - Query updateQuery = entityManager.createQuery("update ProfileEntity p set p.encryptedPassword = :encryptedPassword where p.id = :orcid"); + // A new password satisfies a mandatory password reset, whichever path set it + Query updateQuery = entityManager.createQuery("update ProfileEntity p set p.encryptedPassword = :encryptedPassword, p.forcePasswordReset = null where p.id = :orcid"); updateQuery.setParameter("orcid", orcid); updateQuery.setParameter("encryptedPassword", encryptedPassword); updateQuery.executeUpdate(); @@ -848,6 +849,15 @@ public int updateForcePasswordReset(List ids, Date forcePasswordResetDat return query.executeUpdate(); } + @Override + @Transactional(value = "transactionManagerReadOnly", readOnly = true) + public boolean isPasswordResetRequired(String orcid) { + Query query = entityManager.createNativeQuery("SELECT count(*) FROM profile WHERE orcid = :orcid AND force_password_reset IS NOT NULL " + + "AND claimed = true AND profile_deactivation_date IS NULL AND primary_record IS NULL"); + query.setParameter("orcid", orcid); + return ((Number) query.getSingleResult()).longValue() > 0; + } + @SuppressWarnings("unchecked") @Override @Transactional(value = "transactionManagerReadOnly", readOnly = true) diff --git a/orcid-persistence/src/test/java/org/orcid/persistence/dao/ProfileDaoTest.java b/orcid-persistence/src/test/java/org/orcid/persistence/dao/ProfileDaoTest.java index f60fbd1b28d..6e75f87996e 100644 --- a/orcid-persistence/src/test/java/org/orcid/persistence/dao/ProfileDaoTest.java +++ b/orcid-persistence/src/test/java/org/orcid/persistence/dao/ProfileDaoTest.java @@ -297,6 +297,46 @@ public void testUpdateForcePasswordReset() { assertNull(untouchedProfile.getForcePasswordReset()); } + @Test + @Rollback(true) + @Transactional(propagation = Propagation.REQUIRES_NEW) + public void testIsPasswordResetRequired() { + // 4442 is claimed and active, 4447 is unclaimed, 444X is deprecated and + // 0000-0000-0000-0007 is deactivated: only the first one must answer true + List flagged = Arrays.asList("4444-4444-4444-4442", "4444-4444-4444-4447", "4444-4444-4444-444X", "0000-0000-0000-0007"); + assertEquals(4, profileDao.updateForcePasswordReset(flagged, new Date())); + entityManager.clear(); + + assertTrue(profileDao.isPasswordResetRequired("4444-4444-4444-4442")); + assertFalse(profileDao.isPasswordResetRequired("4444-4444-4444-4447")); + assertFalse(profileDao.isPasswordResetRequired("4444-4444-4444-444X")); + assertFalse(profileDao.isPasswordResetRequired("0000-0000-0000-0007")); + // Not flagged, and not a record at all + assertFalse(profileDao.isPasswordResetRequired("4444-4444-4444-4443")); + assertFalse(profileDao.isPasswordResetRequired("9999-9999-9999-9999")); + } + + @Test + @Rollback(true) + @Transactional(propagation = Propagation.REQUIRES_NEW) + public void testChangeEncryptedPasswordClearsForcePasswordReset() { + String orcid = "4444-4444-4444-4442"; + String untouched = "4444-4444-4444-4443"; + assertEquals(2, profileDao.updateForcePasswordReset(Arrays.asList(orcid, untouched), new Date())); + entityManager.clear(); + assertTrue(profileDao.isPasswordResetRequired(orcid)); + + profileDao.changeEncryptedPassword(orcid, "new-encrypted-password"); + entityManager.clear(); + + ProfileEntity profile = profileDao.find(orcid); + assertEquals("new-encrypted-password", profile.getEncryptedPassword()); + assertNull(profile.getForcePasswordReset()); + assertFalse(profileDao.isPasswordResetRequired(orcid)); + // Only the record whose password changed is released + assertNotNull(profileDao.find(untouched).getForcePasswordReset()); + } + @Test @Rollback(true) @Transactional(propagation = Propagation.REQUIRES_NEW) From 6ca06a97a2c73915713eecb8d9b316728dc68828 Mon Sep 17 00:00:00 2001 From: cryptalith Date: Thu, 24 Sep 2026 17:32:47 -0600 Subject: [PATCH 2/4] PD-5692 refuse password sign in while a reset is owed --- .../AjaxAuthenticationFailureHandler.java | 5 + .../spring/OrcidAuthenticationProvider.java | 12 ++ .../RecoveryPhoneSigninController.java | 12 ++ .../PasswordResetRequiredException.java | 22 +++ .../AjaxAuthenticationFailureHandlerTest.java | 51 ++++++ .../OrcidAuthenticationProviderTest.java | 151 +++++++++++++++++- .../RecoveryPhoneSigninControllerTest.java | 34 ++++ 7 files changed, 286 insertions(+), 1 deletion(-) create mode 100644 orcid-web/src/main/java/org/orcid/frontend/web/exception/PasswordResetRequiredException.java create mode 100644 orcid-web/src/test/java/org/orcid/frontend/spring/AjaxAuthenticationFailureHandlerTest.java diff --git a/orcid-web/src/main/java/org/orcid/frontend/spring/AjaxAuthenticationFailureHandler.java b/orcid-web/src/main/java/org/orcid/frontend/spring/AjaxAuthenticationFailureHandler.java index f466167f6cc..4508fd90930 100644 --- a/orcid-web/src/main/java/org/orcid/frontend/spring/AjaxAuthenticationFailureHandler.java +++ b/orcid-web/src/main/java/org/orcid/frontend/spring/AjaxAuthenticationFailureHandler.java @@ -12,6 +12,7 @@ import org.orcid.core.security.UnclaimedProfileExistsException; import org.orcid.frontend.web.exception.Bad2FARecoveryCodeException; import org.orcid.frontend.web.exception.Bad2FAVerificationCodeException; +import org.orcid.frontend.web.exception.PasswordResetRequiredException; import org.orcid.frontend.web.exception.VerificationCodeFor2FARequiredException; import org.springframework.security.authentication.DisabledException; import org.springframework.security.core.AuthenticationException; @@ -60,6 +61,10 @@ public void onAuthenticationFailure(HttpServletRequest request, HttpServletRespo } else if (exception.getCause() instanceof InvalidUserTypeException) { writer.println(","); writer.println("\"invalidUserType\": true"); + } else if (exception instanceof PasswordResetRequiredException) { + // Thrown by OrcidAuthenticationProvider after the password matched + writer.println(","); + writer.println("\"passwordResetRequired\": true"); } writer.println("}"); } diff --git a/orcid-web/src/main/java/org/orcid/frontend/spring/OrcidAuthenticationProvider.java b/orcid-web/src/main/java/org/orcid/frontend/spring/OrcidAuthenticationProvider.java index 6690fda7c2c..f77b3580024 100644 --- a/orcid-web/src/main/java/org/orcid/frontend/spring/OrcidAuthenticationProvider.java +++ b/orcid-web/src/main/java/org/orcid/frontend/spring/OrcidAuthenticationProvider.java @@ -17,6 +17,7 @@ import org.orcid.core.togglz.Features; import org.orcid.frontend.web.exception.Bad2FARecoveryCodeException; import org.orcid.frontend.web.exception.Bad2FAVerificationCodeException; +import org.orcid.frontend.web.exception.PasswordResetRequiredException; import org.orcid.frontend.web.exception.VerificationCodeFor2FARequiredException; import org.orcid.persistence.jpa.entities.ProfileEntity; import org.orcid.utils.OrcidStringUtils; @@ -129,6 +130,17 @@ public Authentication authenticate(Authentication auth) throws AuthenticationExc } } + // A correct password on a record flagged for a mandatory password reset + // does not sign in. Checked after the password and the lock, so a wrong + // password still counts as a failed attempt, and before 2FA, so the user + // is not asked for a code first. The status is read from the primary, + // not the profile cache: the cache is keyed on last_modified, which + // setting or clearing the flag does not change. + if (Features.FORCE_PASSWORD_RESET.isActive() && profileEntityManager.isPasswordResetRequired(profile.getId())) { + LOGGER.info("Correct sign in refused, password reset required for: " + profile.getId()); + throw new PasswordResetRequiredException(); + } + if (profile.getUsing2FA()) { String recoveryCode = ((MFAWebAuthenticationDetails) auth.getDetails()).getRecoveryCode(); if (recoveryCode != null && !recoveryCode.isEmpty()) { diff --git a/orcid-web/src/main/java/org/orcid/frontend/web/controllers/RecoveryPhoneSigninController.java b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/RecoveryPhoneSigninController.java index 0bdda042513..418dbfb5e4f 100644 --- a/orcid-web/src/main/java/org/orcid/frontend/web/controllers/RecoveryPhoneSigninController.java +++ b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/RecoveryPhoneSigninController.java @@ -20,6 +20,7 @@ import org.orcid.frontend.recoveryphone.RecoveryPhoneSigninVerifyRequest; import org.orcid.frontend.recoveryphone.RecoveryPhoneSigninVerifyResponse; import org.orcid.frontend.recoveryphone.RecoveryPhoneVerificationService; +import org.orcid.frontend.web.exception.PasswordResetRequiredException; import org.orcid.frontend.web.exception.VerificationCodeFor2FARequiredException; import org.orcid.utils.OrcidStringUtils; import org.slf4j.Logger; @@ -246,6 +247,17 @@ private String verifyCredentials(HttpServletRequest request, String username, St // The password was accepted and the account is using 2FA: the one // state these endpoints exist to serve return null; + } catch (PasswordResetRequiredException e) { + // The password was accepted, but the record has to reset it before + // it can sign in, which the provider checks ahead of 2FA. A 2FA + // account must still be able to recover here: the password reset + // asks for a 2FA code too, so refusing would leave a user without + // their authenticator locked out of both + String orcid = resolveOrcid(username); + if (orcid == null) { + return BAD_CREDENTIALS; + } + return twoFactorAuthenticationManager.userUsing2FA(orcid) ? null : TWO_FACTOR_DISABLED; } catch (AuthenticationException e) { // Covers a wrong password, a locked account and the bad 2FA code // exceptions, none of which we distinguish for an anonymous caller diff --git a/orcid-web/src/main/java/org/orcid/frontend/web/exception/PasswordResetRequiredException.java b/orcid-web/src/main/java/org/orcid/frontend/web/exception/PasswordResetRequiredException.java new file mode 100644 index 00000000000..5c2eeaf5f94 --- /dev/null +++ b/orcid-web/src/main/java/org/orcid/frontend/web/exception/PasswordResetRequiredException.java @@ -0,0 +1,22 @@ +package org.orcid.frontend.web.exception; + +import org.springframework.security.authentication.AccountStatusException; + +/** + * The password was correct, but the record is flagged for a mandatory password + * reset (profile.force_password_reset), so it cannot sign in with a password + * until the password is changed. + * + * An AccountStatusException rather than a BadCredentialsException: the sign in + * lock must not count it as a failed attempt, and the provider manager must not + * fall through to another provider. + */ +public class PasswordResetRequiredException extends AccountStatusException { + + private static final long serialVersionUID = 1L; + + public PasswordResetRequiredException() { + super("Password reset required"); + } + +} diff --git a/orcid-web/src/test/java/org/orcid/frontend/spring/AjaxAuthenticationFailureHandlerTest.java b/orcid-web/src/test/java/org/orcid/frontend/spring/AjaxAuthenticationFailureHandlerTest.java new file mode 100644 index 00000000000..b7349cdcd79 --- /dev/null +++ b/orcid-web/src/test/java/org/orcid/frontend/spring/AjaxAuthenticationFailureHandlerTest.java @@ -0,0 +1,51 @@ +package org.orcid.frontend.spring; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import org.junit.Test; +import org.orcid.frontend.web.exception.PasswordResetRequiredException; +import org.orcid.frontend.web.exception.VerificationCodeFor2FARequiredException; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; +import org.springframework.security.authentication.BadCredentialsException; +import org.springframework.security.core.AuthenticationException; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; + +public class AjaxAuthenticationFailureHandlerTest { + + private final AjaxAuthenticationFailureHandler handler = new AjaxAuthenticationFailureHandler(); + + @Test + public void passwordResetRequiredIsReported() throws Exception { + JsonNode body = fail(new PasswordResetRequiredException()); + + assertFalse(body.get("success").asBoolean()); + assertTrue(body.get("passwordResetRequired").asBoolean()); + assertFalse(body.has("verificationCodeRequired")); + } + + @Test + public void badCredentialsDoNotReportPasswordResetRequired() throws Exception { + JsonNode body = fail(new BadCredentialsException("Bad credentials")); + + assertFalse(body.get("success").asBoolean()); + assertFalse(body.has("passwordResetRequired")); + } + + @Test + public void aMissing2FACodeDoesNotReportPasswordResetRequired() throws Exception { + JsonNode body = fail(new VerificationCodeFor2FARequiredException()); + + assertTrue(body.get("verificationCodeRequired").asBoolean()); + assertFalse(body.has("passwordResetRequired")); + } + + private JsonNode fail(AuthenticationException exception) throws Exception { + MockHttpServletResponse response = new MockHttpServletResponse(); + handler.onAuthenticationFailure(new MockHttpServletRequest(), response, exception); + return new ObjectMapper().readTree(response.getContentAsString()); + } +} diff --git a/orcid-web/src/test/java/org/orcid/frontend/spring/OrcidAuthenticationProviderTest.java b/orcid-web/src/test/java/org/orcid/frontend/spring/OrcidAuthenticationProviderTest.java index 3967ea78595..e9c50c34aaa 100644 --- a/orcid-web/src/test/java/org/orcid/frontend/spring/OrcidAuthenticationProviderTest.java +++ b/orcid-web/src/test/java/org/orcid/frontend/spring/OrcidAuthenticationProviderTest.java @@ -1,23 +1,39 @@ package org.orcid.frontend.spring; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThrows; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import java.util.Date; import java.util.HashSet; import java.util.List; import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; import org.mockito.Mock; import org.mockito.MockitoAnnotations; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; +import org.orcid.authorization.authentication.MFAWebAuthenticationDetails; import org.orcid.core.manager.ProfileEntityCacheManager; +import org.orcid.core.manager.v3.ProfileEntityManager; import org.orcid.core.manager.v3.read_only.EmailManagerReadOnly; import org.orcid.core.security.OrcidUserDetailsService; import org.orcid.core.security.OrcidRoles; +import org.orcid.core.togglz.Features; +import org.orcid.frontend.web.exception.PasswordResetRequiredException; +import org.orcid.frontend.web.exception.VerificationCodeFor2FARequiredException; import org.orcid.persistence.jpa.entities.ProfileEntity; import org.orcid.test.TargetProxyHelper; +import org.springframework.security.authentication.BadCredentialsException; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.authority.SimpleGrantedAuthority; import org.springframework.security.core.authority.mapping.GrantedAuthoritiesMapper; @@ -25,9 +41,28 @@ import org.springframework.security.core.userdetails.UserCache; import org.springframework.security.core.userdetails.UserDetails; import org.springframework.security.core.userdetails.UserDetailsChecker; +import org.springframework.security.core.userdetails.UserDetailsService; +import org.springframework.security.crypto.password.PasswordEncoder; +import org.togglz.junit.TogglzRule; public class OrcidAuthenticationProviderTest { + private static final String ORCID = "0000-0000-0000-0001"; + + private static final String PASSWORD = "password"; + + @Rule + public TogglzRule togglzRule = TogglzRule.allDisabled(Features.class); + + @Mock + PasswordEncoder passwordEncoderMock; + + @Mock + UserDetailsService userDetailsServiceMock; + + @Mock + ProfileEntityManager profileEntityManagerMock; + @Mock UserCache userCacheMock; @@ -58,12 +93,20 @@ public void before() { TargetProxyHelper.injectIntoProxy(orcidAuthenticationProvider, "profileEntityCacheManager", profileEntityCacheManagerMock); TargetProxyHelper.injectIntoProxy(orcidAuthenticationProvider, "emailManagerReadOnly", emailManagerReadOnlyMock); TargetProxyHelper.injectIntoProxy(orcidAuthenticationProvider, "orcidUserDetailsService", orcidUserDetailsServiceMock); + TargetProxyHelper.injectIntoProxy(orcidAuthenticationProvider, "profileEntityManager", profileEntityManagerMock); + TargetProxyHelper.injectIntoProxy(orcidAuthenticationProvider, "lockoutWindow", 10); + TargetProxyHelper.injectIntoProxy(orcidAuthenticationProvider, "lockoutThreshhold", 10); + orcidAuthenticationProvider.setPasswordEncoder(passwordEncoderMock); + orcidAuthenticationProvider.setUserDetailsService(userDetailsServiceMock); + orcidAuthenticationProvider.setAuthoritiesMapper(grantedAuthoritiesMapperMock); + when(passwordEncoderMock.matches(PASSWORD, "encrypted")).thenReturn(true); + when(userDetailsServiceMock.loadUserByUsername(anyString())).thenAnswer(invocation -> new User((String) invocation.getArgument(0), "encrypted", List.of())); when(userCacheMock.getUserFromCache(anyString())).then(new Answer(){ @Override public UserDetails answer(InvocationOnMock invocation) throws Throwable { - return new User((String) invocation.getArgument(0), "password", List.of()); + return new User((String) invocation.getArgument(0), "encrypted", List.of()); } }); @@ -90,6 +133,112 @@ public UserDetails answer(InvocationOnMock invocation) throws Throwable { }); } + @Test + public void authenticatesAnUnflaggedRecordWithTheFeatureOn() { + togglzRule.enable(Features.FORCE_PASSWORD_RESET); + profile(false); + + UsernamePasswordAuthenticationToken token = (UsernamePasswordAuthenticationToken) orcidAuthenticationProvider.authenticate(signIn(PASSWORD, null)); + + assertNotNull(token); + assertEquals(ORCID, ((User) token.getPrincipal()).getUsername()); + verify(profileEntityManagerMock).isPasswordResetRequired(ORCID); + } + + @Test + public void refusesACorrectPasswordOnAFlaggedRecordWithTheFeatureOn() { + togglzRule.enable(Features.FORCE_PASSWORD_RESET); + // The cached entity carries no date: the gate must ask the manager, which reads the primary + profile(false); + when(profileEntityManagerMock.isPasswordResetRequired(ORCID)).thenReturn(true); + + assertThrows(PasswordResetRequiredException.class, () -> orcidAuthenticationProvider.authenticate(signIn(PASSWORD, null))); + + // A correct password is not a failed attempt, and no authentication is built + verify(profileEntityManagerMock, never()).updateSigninLock(anyString(), anyInt()); + verify(profileEntityManagerMock, never()).startSigninLock(anyString()); + verify(orcidUserDetailsServiceMock, never()).loadUserByProfile(any()); + } + + @Test + public void aCorrectPasswordOnAFlaggedRecordIsNotAFailedAttemptWithTheLockoutOn() { + togglzRule.enable(Features.FORCE_PASSWORD_RESET); + togglzRule.enable(Features.ENABLE_ACCOUNT_LOCKOUT); + // Two earlier failures, and no lock yet + ProfileEntity p = profile(false); + p.setSigninLockCount(2); + when(profileEntityManagerMock.isPasswordResetRequired(ORCID)).thenReturn(true); + + assertThrows(PasswordResetRequiredException.class, () -> orcidAuthenticationProvider.authenticate(signIn(PASSWORD, null))); + + // Cleared, as a correct password clears it for any record, and nothing is counted + verify(profileEntityManagerMock).resetSigninLock(ORCID); + verify(profileEntityManagerMock, never()).updateSigninLock(anyString(), anyInt()); + verify(profileEntityManagerMock, never()).startSigninLock(anyString()); + } + + @Test + public void authenticatesAFlaggedRecordWithTheFeatureOff() { + profile(false); + when(profileEntityManagerMock.isPasswordResetRequired(ORCID)).thenReturn(true); + + assertNotNull(orcidAuthenticationProvider.authenticate(signIn(PASSWORD, null))); + verify(profileEntityManagerMock, never()).isPasswordResetRequired(anyString()); + } + + @Test + public void refusesAFlaggedRecordBeforeAskingForA2FACode() { + togglzRule.enable(Features.FORCE_PASSWORD_RESET); + profile(true); + when(profileEntityManagerMock.isPasswordResetRequired(ORCID)).thenReturn(true); + + assertThrows(PasswordResetRequiredException.class, () -> orcidAuthenticationProvider.authenticate(signIn(PASSWORD, null))); + } + + @Test + public void asksAnUnflagged2FARecordForItsCode() { + togglzRule.enable(Features.FORCE_PASSWORD_RESET); + profile(true); + + assertThrows(VerificationCodeFor2FARequiredException.class, () -> orcidAuthenticationProvider.authenticate(signIn(PASSWORD, null))); + } + + @Test + public void aWrongPasswordOnAFlaggedRecordIsBadCredentials() { + togglzRule.enable(Features.FORCE_PASSWORD_RESET); + profile(false); + when(profileEntityManagerMock.isPasswordResetRequired(ORCID)).thenReturn(true); + + assertThrows(BadCredentialsException.class, () -> orcidAuthenticationProvider.authenticate(signIn("wrong", null))); + verify(profileEntityManagerMock, never()).isPasswordResetRequired(anyString()); + } + + @Test + public void aLockedOutFlaggedRecordGetsBadCredentialsBeforeTheResetCheck() { + togglzRule.enable(Features.FORCE_PASSWORD_RESET); + ProfileEntity p = profile(false); + p.setSigninLockCount(12); + p.setSigninLockStart(new Date()); + when(profileEntityManagerMock.isPasswordResetRequired(ORCID)).thenReturn(true); + + assertThrows(BadCredentialsException.class, () -> orcidAuthenticationProvider.authenticate(signIn(PASSWORD, null))); + verify(profileEntityManagerMock, never()).isPasswordResetRequired(anyString()); + } + + private ProfileEntity profile(boolean using2FA) { + ProfileEntity p = new ProfileEntity(ORCID); + p.setUsing2FA(using2FA); + p.setEncryptedPassword("encrypted"); + when(profileEntityCacheManagerMock.retrieve(ORCID)).thenReturn(p); + return p; + } + + private UsernamePasswordAuthenticationToken signIn(String password, String verificationCode) { + UsernamePasswordAuthenticationToken authRequest = new UsernamePasswordAuthenticationToken(ORCID, password); + authRequest.setDetails(new MFAWebAuthenticationDetails("127.0.0.1", null, verificationCode, null)); + return authRequest; + } + /* @Test public void authenticateOrcidTest() { diff --git a/orcid-web/src/test/java/org/orcid/frontend/web/controllers/RecoveryPhoneSigninControllerTest.java b/orcid-web/src/test/java/org/orcid/frontend/web/controllers/RecoveryPhoneSigninControllerTest.java index 50755d50dd6..33e4ffcbc68 100644 --- a/orcid-web/src/test/java/org/orcid/frontend/web/controllers/RecoveryPhoneSigninControllerTest.java +++ b/orcid-web/src/test/java/org/orcid/frontend/web/controllers/RecoveryPhoneSigninControllerTest.java @@ -26,6 +26,7 @@ import org.orcid.frontend.recoveryphone.RecoveryPhoneSigninVerifyRequest; import org.orcid.frontend.recoveryphone.RecoveryPhoneSigninVerifyResponse; import org.orcid.frontend.recoveryphone.RecoveryPhoneVerificationService; +import org.orcid.frontend.web.exception.PasswordResetRequiredException; import org.orcid.frontend.web.exception.VerificationCodeFor2FARequiredException; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.security.authentication.AuthenticationManager; @@ -196,6 +197,39 @@ public void testTheAuthenticationTokenCarriesNo2FACodes() { assertEquals(REMOTE_ADDRESS, details.getRemoteAddress()); } + /** + * A record flagged for a mandatory password reset is refused by the provider + * before its 2FA check. The recovery still has to work for a 2FA account, + * because the password reset that clears the flag asks for a 2FA code too. + */ + @Test + public void testSendCodeStillServesAFlagged2FAAccount() { + togglzRule.enable(Features.TWO_FACTOR_RECOVERY_PHONE); + when(authenticationProvider.authenticate(any(Authentication.class))).thenThrow(new PasswordResetRequiredException()); + when(twoFactorAuthenticationManager.userUsing2FA(ORCID)).thenReturn(true); + aRecoveryPhoneIsStored(); + when(recoveryPhoneVerificationService.sendCode(eq(ORCID), any(RecoveryPhoneSendCodeRequest.class))) + .thenReturn(RecoveryPhoneSendCodeResponse.success(30)); + + RecoveryPhoneSigninSendCodeResponse response = controller.sendCode(request, sendCodeRequest(ORCID)); + + assertTrue(response.isSuccess()); + verify(recoveryPhoneVerificationService).sendCode(eq(ORCID), any(RecoveryPhoneSendCodeRequest.class)); + } + + @Test + public void testSendCodeRefusesAFlaggedAccountThatIsNotUsing2FA() { + togglzRule.enable(Features.TWO_FACTOR_RECOVERY_PHONE); + when(authenticationProvider.authenticate(any(Authentication.class))).thenThrow(new PasswordResetRequiredException()); + when(twoFactorAuthenticationManager.userUsing2FA(ORCID)).thenReturn(false); + + RecoveryPhoneSigninSendCodeResponse response = controller.sendCode(request, sendCodeRequest(ORCID)); + + assertFalse(response.isSuccess()); + assertEquals(RecoveryPhoneSigninController.TWO_FACTOR_DISABLED, response.getErrorCode()); + verify(recoveryPhoneVerificationService, never()).sendCode(anyString(), any(RecoveryPhoneSendCodeRequest.class)); + } + @Test public void testSendCodeRefusesAnAccountThatIsNotUsing2FA() { togglzRule.enable(Features.TWO_FACTOR_RECOVERY_PHONE); From bae1e7e4c84a3eb34e1fe7342b53a68eee594e8e Mon Sep 17 00:00:00 2001 From: cryptalith Date: Thu, 24 Sep 2026 17:33:01 -0600 Subject: [PATCH 3/4] PD-5692 reset status lookup, reset by iD and reuse check --- .../resources/i18n/messages_en.properties | 1 + .../controllers/PasswordResetController.java | 65 ++++++++ .../PasswordResetStatusController.java | 106 +++++++++++++ .../resources/orcid-frontend-security.xml | 2 + .../PasswordResetControllerTest.java | 141 ++++++++++++++++++ .../PasswordResetStatusControllerTest.java | 117 +++++++++++++++ 6 files changed, 432 insertions(+) create mode 100644 orcid-web/src/main/java/org/orcid/frontend/web/controllers/PasswordResetStatusController.java create mode 100644 orcid-web/src/test/java/org/orcid/frontend/web/controllers/PasswordResetStatusControllerTest.java diff --git a/orcid-core/src/main/resources/i18n/messages_en.properties b/orcid-core/src/main/resources/i18n/messages_en.properties index 03ec5745e4a..f4e8fee2c0d 100644 --- a/orcid-core/src/main/resources/i18n/messages_en.properties +++ b/orcid-core/src/main/resources/i18n/messages_en.properties @@ -90,6 +90,7 @@ Pattern.registrationForm.password.eigthCharacters=8 or more characters Pattern.registrationForm.password.oneNumber=1 number Pattern.registrationForm.password.letterOrSymbol=1 letter or symbol Pattern.registrationForm.password.containsEmail=Password must not be the same as your email address +Pattern.registrationForm.password.previouslyUsed=Please enter a new password. Previous passwords cannot be reused. StringMatchIgnoreCase.registrationForm=The email and confirmed email must match. admin.profile_deprecation.deprecate_account.success_message=Profile {0} has been successfully deprecated to profile {1}. change_password.confirmnewpassword=Confirm new password diff --git a/orcid-web/src/main/java/org/orcid/frontend/web/controllers/PasswordResetController.java b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/PasswordResetController.java index cdba57ca6a7..5a01114d72c 100644 --- a/orcid-web/src/main/java/org/orcid/frontend/web/controllers/PasswordResetController.java +++ b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/PasswordResetController.java @@ -165,6 +165,9 @@ public class PasswordResetController extends BaseController { List errors = new ArrayList<>(); passwordResetRequest.setErrors(errors); passwordResetRequest.setEmail(passwordResetRequest.getEmail().trim()); + if (OrcidStringUtils.isValidOrcid(passwordResetRequest.getEmail())) { + return issuePasswordResetRequestForOrcid(passwordResetRequest); + } if (!validateEmailAddress(passwordResetRequest.getEmail())) { errors.add(getMessage("Email.resetPasswordForm.invalidEmail")); return new ResponseEntity<>(passwordResetRequest, HttpStatus.OK); @@ -198,6 +201,60 @@ public class PasswordResetController extends BaseController { } } + /** + * A password reset requested with an ORCID iD, which only the mandatory + * password reset notice on the sign in forms sends (PD-5692). The email + * goes to the record's primary address, fanned out like any other reset. + * + * Only a record that has to reset its password gets one: an iD is public, + * and every reset email replaces the record's previous reset link, so an + * open iD path would let anyone invalidate anyone's link. The answer is the + * same whatever happened, and it names only what the caller sent, never the + * address the email went to. + */ + private ResponseEntity issuePasswordResetRequestForOrcid(EmailRequest passwordResetRequest) { + String orcid = passwordResetRequest.getEmail(); + try { + if (Features.FORCE_PASSWORD_RESET.isActive() && profileEntityManager.isPasswordResetRequired(orcid)) { + String primaryEmail = findPrimaryEmail(orcid); + if (primaryEmail != null) { + LOGGER.info("Password reset: Reset password requested by ORCID iD for '{}'", orcid); + recordEmailSender.sendPasswordResetEmail(primaryEmail, orcid); + } else { + LOGGER.warn("Password reset: No primary email to send the reset link to for '{}'", orcid); + } + } else { + LOGGER.info("Password reset: ORCID iD request ignored for '{}', no mandatory password reset", orcid); + } + passwordResetRequest.setSuccessMessage(getMessage("orcid.frontend.reset.password.successfulReset") + " " + orcid); + return new ResponseEntity<>(passwordResetRequest, HttpStatus.OK); + } catch (Exception e) { + LOGGER.error("Password reset: Unable to reset password for " + orcid, e); + passwordResetRequest.getErrors().add(getMessage("Email.resetPasswordForm.error")); + return new ResponseEntity<>(passwordResetRequest, HttpStatus.BAD_REQUEST); + } + } + + private String findPrimaryEmail(String orcid) { + try { + org.orcid.jaxb.model.v3.release.record.Email primary = emailManagerReadOnly.findPrimaryEmail(orcid); + return primary == null || PojoUtil.isEmpty(primary.getEmail()) ? null : primary.getEmail(); + } catch (NoResultException e) { + return null; + } + } + + /** + * Whether the given password is the record's current one. Read from the + * primary rather than the profile cache, so a password changed a moment ago + * on another node is the one compared. + */ + private boolean isCurrentPassword(String orcid, String password) { + ProfileEntity profile = profileEntityManager.findByOrcid(orcid); + return profile != null && profile.getEncryptedPassword() != null && password != null + && encryptionManager.hashMatches(password, profile.getEncryptedPassword()); + } + @RequestMapping(value = "/reset-password-email/{encryptedEmail}", method = RequestMethod.GET) public ModelAndView resetPasswordEmail(HttpServletRequest request, @PathVariable("encryptedEmail") String encryptedEmail) { PasswordResetToken passwordResetToken = buildResetTokenFromEncryptedLink(encryptedEmail); @@ -327,6 +384,14 @@ else if (oneTimeResetPasswordForm.getTwoFactorCode() != null && !oneTimeResetPas } } + // Previous passwords cannot be reused (PD-5692). Checked only after 2FA + // has passed, so the reset link alone cannot be used to test guesses + // against the current password; the token is left unused for the retry + if (isCurrentPassword(orcid, oneTimeResetPasswordForm.getNewPassword().getValue())) { + oneTimeResetPasswordForm.getNewPassword().getErrors().add("Pattern.registrationForm.password.previouslyUsed"); + return oneTimeResetPasswordForm; + } + profileEntityManager.updatePassword(orcid, oneTimeResetPasswordForm.getNewPassword().getValue()); //send the security notification email on change password if(Features.SEND_EMAIL_ON_RESET_PASSWORD.isActive()) { diff --git a/orcid-web/src/main/java/org/orcid/frontend/web/controllers/PasswordResetStatusController.java b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/PasswordResetStatusController.java new file mode 100644 index 00000000000..63fd81f946a --- /dev/null +++ b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/PasswordResetStatusController.java @@ -0,0 +1,106 @@ +package org.orcid.frontend.web.controllers; + +import jakarta.annotation.Resource; +import jakarta.persistence.NoResultException; + +import org.apache.commons.lang3.StringUtils; +import org.orcid.core.manager.v3.read_only.EmailManagerReadOnly; +import org.orcid.core.manager.v3.read_only.ProfileEntityManagerReadOnly; +import org.orcid.core.togglz.Features; +import org.orcid.utils.OrcidStringUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.stereotype.Controller; +import org.springframework.web.bind.annotation.RequestBody; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RequestMethod; +import org.springframework.web.bind.annotation.ResponseBody; + +/** + * Tells the sign in forms, as soon as an email address or ORCID iD has been + * entered, that the record behind it has to reset its password before it can + * sign in with one (mandatory password reset, PD-5692). + * + * Anonymous by design: disclosing this state from the identifier alone is an + * accepted product decision. The answer is identical for an unknown + * identifier, a record that is not flagged, and the feature being off, so the + * endpoint says nothing about whether an address or iD is registered. + * + * Reads go to the read only database and are never cached, because the flag + * does not change the record's last modified date, which every profile cache + * is keyed on. The sign in itself re-checks against the primary. + */ +@Controller +public class PasswordResetStatusController { + + private static final Logger LOGGER = LoggerFactory.getLogger(PasswordResetStatusController.class); + + /** Longer than any email address the registry accepts, so nothing real is refused */ + private static final int MAX_USERNAME_LENGTH = 320; + + @Resource(name = "profileEntityManagerReadOnlyV3") + private ProfileEntityManagerReadOnly profileEntityManagerReadOnly; + + @Resource(name = "emailManagerReadOnlyV3") + private EmailManagerReadOnly emailManagerReadOnly; + + @RequestMapping(value = "/signin/password-reset-status.json", method = RequestMethod.POST) + public @ResponseBody PasswordResetStatus getPasswordResetStatus(@RequestBody PasswordResetStatusRequest request) { + if (!Features.FORCE_PASSWORD_RESET.isActive() || request == null) { + return new PasswordResetStatus(false); + } + String orcid = resolveOrcid(request.getUsername()); + if (orcid == null) { + return new PasswordResetStatus(false); + } + boolean required = profileEntityManagerReadOnly.isPasswordResetRequired(orcid); + if (required) { + LOGGER.debug("Password reset required reported for {}", orcid); + } + return new PasswordResetStatus(required); + } + + private String resolveOrcid(String username) { + String value = StringUtils.trimToNull(username); + if (value == null || value.length() > MAX_USERNAME_LENGTH) { + return null; + } + if (OrcidStringUtils.isValidOrcid(value)) { + return value; + } + if (!value.contains("@")) { + return null; + } + try { + return emailManagerReadOnly.findOrcidIdByEmail(value); + } catch (NoResultException e) { + return null; + } + } + + public static class PasswordResetStatusRequest { + + private String username; + + public String getUsername() { + return username; + } + + public void setUsername(String username) { + this.username = username; + } + } + + public static class PasswordResetStatus { + + private final boolean passwordResetRequired; + + public PasswordResetStatus(boolean passwordResetRequired) { + this.passwordResetRequired = passwordResetRequired; + } + + public boolean isPasswordResetRequired() { + return passwordResetRequired; + } + } +} diff --git a/orcid-web/src/main/resources/orcid-frontend-security.xml b/orcid-web/src/main/resources/orcid-frontend-security.xml index db3190690ed..7473d1ed365 100644 --- a/orcid-web/src/main/resources/orcid-frontend-security.xml +++ b/orcid-web/src/main/resources/orcid-frontend-security.xml @@ -255,6 +255,8 @@ access="IS_AUTHENTICATED_ANONYMOUSLY" /> + response = controller.issuePasswordResetRequest(newRequest(), orcidIdRequest()); + + assertEquals(HttpStatus.OK, response.getStatusCode()); + verify(recordEmailSender).sendPasswordResetEmail(EMAIL, ORCID); + assertTrue(response.getBody().getErrors().isEmpty()); + assertTrue(response.getBody().getSuccessMessage().endsWith(ORCID)); + assertFalse(response.getBody().getSuccessMessage().contains(EMAIL)); + verify(emailManager, never()).emailExists(anyString()); + } + + @Test + public void issuePasswordResetRequestWithAnOrcidIdThatIsNotFlaggedSendsNothing() { + togglzRule.enable(Features.FORCE_PASSWORD_RESET); + when(profileEntityManager.isPasswordResetRequired(ORCID)).thenReturn(false); + + ResponseEntity response = controller.issuePasswordResetRequest(newRequest(), orcidIdRequest()); + + assertEquals(HttpStatus.OK, response.getStatusCode()); + assertTrue(response.getBody().getSuccessMessage().endsWith(ORCID)); + verifyNoInteractions(recordEmailSender); + verify(emailManagerReadOnly, never()).findPrimaryEmail(anyString()); + } + + @Test + public void issuePasswordResetRequestWithAnOrcidIdSendsNothingWhileTheFeatureIsOff() { + // Flagged, but never asked: the feature switch is checked first + lenient().when(profileEntityManager.isPasswordResetRequired(ORCID)).thenReturn(true); + + ResponseEntity response = controller.issuePasswordResetRequest(newRequest(), orcidIdRequest()); + + assertEquals(HttpStatus.OK, response.getStatusCode()); + verifyNoInteractions(recordEmailSender); + verify(profileEntityManager, never()).isPasswordResetRequired(anyString()); + } + + @Test + public void issuePasswordResetRequestWithAFlaggedOrcidIdAndNoPrimaryAddressSendsNothing() { + togglzRule.enable(Features.FORCE_PASSWORD_RESET); + when(profileEntityManager.isPasswordResetRequired(ORCID)).thenReturn(true); + when(emailManagerReadOnly.findPrimaryEmail(ORCID)).thenThrow(new NoResultException()); + + ResponseEntity response = controller.issuePasswordResetRequest(newRequest(), orcidIdRequest()); + + assertEquals(HttpStatus.OK, response.getStatusCode()); + verifyNoInteractions(recordEmailSender); + } + @Test public void submitPasswordResetV2LegacyTokenWithInvalidOrcidFails() { when(encryptionManager.decryptForExternalUse(anyString())).thenReturn("email=0000-0000-0000-9999&issueDate=2070-05-29T17:04:27"); @@ -824,6 +952,19 @@ private Reactivation validReactivation() { return reg; } + private void currentPasswordIs(String currentPassword) { + ProfileEntity profile = new ProfileEntity(ORCID); + profile.setEncryptedPassword("current-hash"); + when(profileEntityManager.findByOrcid(ORCID)).thenReturn(profile); + when(encryptionManager.hashMatches(anyString(), eq("current-hash"))).thenAnswer(invocation -> currentPassword.equals(invocation.getArgument(0))); + } + + private EmailRequest orcidIdRequest() { + EmailRequest body = new EmailRequest(); + body.setEmail(" " + ORCID + " "); + return body; + } + private MockHttpServletRequest newRequest() { MockHttpServletRequest request = new MockHttpServletRequest(); request.setRemoteAddr("127.0.0.1"); diff --git a/orcid-web/src/test/java/org/orcid/frontend/web/controllers/PasswordResetStatusControllerTest.java b/orcid-web/src/test/java/org/orcid/frontend/web/controllers/PasswordResetStatusControllerTest.java new file mode 100644 index 00000000000..bd92a67c57d --- /dev/null +++ b/orcid-web/src/test/java/org/orcid/frontend/web/controllers/PasswordResetStatusControllerTest.java @@ -0,0 +1,117 @@ +package org.orcid.frontend.web.controllers; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import jakarta.persistence.NoResultException; + +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.orcid.core.manager.v3.read_only.EmailManagerReadOnly; +import org.orcid.core.manager.v3.read_only.ProfileEntityManagerReadOnly; +import org.orcid.core.togglz.Features; +import org.orcid.frontend.web.controllers.PasswordResetStatusController.PasswordResetStatusRequest; +import org.togglz.junit.TogglzRule; + +public class PasswordResetStatusControllerTest { + + private static final String ORCID = "0000-0000-0000-0001"; + + private static final String EMAIL = "user@orcid.org"; + + @Rule + public TogglzRule togglzRule = TogglzRule.allDisabled(Features.class); + + @Mock + private ProfileEntityManagerReadOnly profileEntityManagerReadOnly; + + @Mock + private EmailManagerReadOnly emailManagerReadOnly; + + @InjectMocks + private PasswordResetStatusController controller; + + @Before + public void setUp() { + MockitoAnnotations.initMocks(this); + } + + @Test + public void isInertWhileTheFeatureIsOff() { + when(profileEntityManagerReadOnly.isPasswordResetRequired(ORCID)).thenReturn(true); + + assertFalse(status(ORCID)); + verify(profileEntityManagerReadOnly, never()).isPasswordResetRequired(anyString()); + verify(emailManagerReadOnly, never()).findOrcidIdByEmail(anyString()); + } + + @Test + public void reportsAFlaggedOrcidId() { + togglzRule.enable(Features.FORCE_PASSWORD_RESET); + when(profileEntityManagerReadOnly.isPasswordResetRequired(ORCID)).thenReturn(true); + + assertTrue(status(ORCID)); + assertTrue(status(" " + ORCID + " ")); + verify(emailManagerReadOnly, never()).findOrcidIdByEmail(anyString()); + } + + @Test + public void reportsAFlaggedEmailAddress() { + togglzRule.enable(Features.FORCE_PASSWORD_RESET); + when(emailManagerReadOnly.findOrcidIdByEmail(EMAIL)).thenReturn(ORCID); + when(profileEntityManagerReadOnly.isPasswordResetRequired(ORCID)).thenReturn(true); + + assertTrue(status(EMAIL)); + } + + @Test + public void anUnflaggedRecordIsNotReported() { + togglzRule.enable(Features.FORCE_PASSWORD_RESET); + when(emailManagerReadOnly.findOrcidIdByEmail(EMAIL)).thenReturn(ORCID); + when(profileEntityManagerReadOnly.isPasswordResetRequired(ORCID)).thenReturn(false); + + assertFalse(status(EMAIL)); + assertFalse(status(ORCID)); + } + + @Test + public void anUnknownEmailAddressAnswersLikeAnUnflaggedRecord() { + togglzRule.enable(Features.FORCE_PASSWORD_RESET); + when(emailManagerReadOnly.findOrcidIdByEmail(EMAIL)).thenThrow(new NoResultException()); + + assertFalse(status(EMAIL)); + verify(profileEntityManagerReadOnly, never()).isPasswordResetRequired(anyString()); + } + + @Test + public void somethingThatIsNeitherAnIdNorAnAddressIsNotLookedUp() { + togglzRule.enable(Features.FORCE_PASSWORD_RESET); + + assertFalse(status("not-an-identifier")); + assertFalse(status("0000-0000-0000")); + assertFalse(status("")); + assertFalse(status(null)); + assertFalse(status(repeat('a', 400) + "@orcid.org")); + assertFalse(controller.getPasswordResetStatus(null).isPasswordResetRequired()); + verify(emailManagerReadOnly, never()).findOrcidIdByEmail(anyString()); + verify(profileEntityManagerReadOnly, never()).isPasswordResetRequired(anyString()); + } + + private boolean status(String username) { + PasswordResetStatusRequest request = new PasswordResetStatusRequest(); + request.setUsername(username); + return controller.getPasswordResetStatus(request).isPasswordResetRequired(); + } + + private static String repeat(char c, int times) { + return String.valueOf(c).repeat(times); + } +} From 77f6be698927ca25bb599151105f2cbf25da4d6c Mon Sep 17 00:00:00 2001 From: cryptalith Date: Thu, 24 Sep 2026 17:33:01 -0600 Subject: [PATCH 4/4] PD-5692 report the reset flag in userInfo.json --- .../web/controllers/HomeController.java | 12 ++- .../web/controllers/HomeControllerTest.java | 88 +++++++++++++++++++ 2 files changed, 99 insertions(+), 1 deletion(-) diff --git a/orcid-web/src/main/java/org/orcid/frontend/web/controllers/HomeController.java b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/HomeController.java index dbe6630fa32..751ecfddf86 100644 --- a/orcid-web/src/main/java/org/orcid/frontend/web/controllers/HomeController.java +++ b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/HomeController.java @@ -20,6 +20,7 @@ import org.orcid.core.manager.ProfileEntityCacheManager; import org.orcid.core.manager.v3.ProfileEntityManager; import org.orcid.core.manager.v3.read_only.EmailManagerReadOnly; +import org.orcid.core.manager.v3.read_only.ProfileEntityManagerReadOnly; import org.orcid.core.security.OrcidRoles; import org.orcid.core.stats.StatisticsManager; import org.orcid.core.togglz.Features; @@ -88,6 +89,9 @@ public class HomeController extends BaseController { @Resource(name = "emailManagerReadOnlyV3") protected EmailManagerReadOnly emailManagerReadOnly; + @Resource(name = "profileEntityManagerReadOnlyV3") + private ProfileEntityManagerReadOnly profileEntityManagerReadOnly; + @RequestMapping(value = "/") public ModelAndView homeHandler(HttpServletRequest request) { ModelAndView mav = new ModelAndView("home"); @@ -227,7 +231,13 @@ private void removeSessionIdCookie(HttpServletRequest request, HttpServletRespon } if(!PojoUtil.isEmpty(p.getGroupType())) { info.put("MEMBER_TYPE", p.getGroupType()); - } + } + // Mandatory password reset: only for the record's own user, never a + // delegate or an admin acting as them. Read uncached, because the + // flag does not change last_modified and so never evicts p above + if (Features.FORCE_PASSWORD_RESET.isActive() && effectiveOrcid.equals(realUserOrcid)) { + info.put("FORCE_PASSWORD_RESET", String.valueOf(profileEntityManagerReadOnly.isPasswordResetRequired(effectiveOrcid))); + } } return info; } diff --git a/orcid-web/src/test/java/org/orcid/frontend/web/controllers/HomeControllerTest.java b/orcid-web/src/test/java/org/orcid/frontend/web/controllers/HomeControllerTest.java index 08925fde38f..14baa2a19c0 100644 --- a/orcid-web/src/test/java/org/orcid/frontend/web/controllers/HomeControllerTest.java +++ b/orcid-web/src/test/java/org/orcid/frontend/web/controllers/HomeControllerTest.java @@ -1,14 +1,34 @@ package org.orcid.frontend.web.controllers; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import java.util.List; +import java.util.Map; + import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.orcid.core.manager.ProfileEntityCacheManager; +import org.orcid.core.manager.v3.SourceManager; +import org.orcid.core.manager.v3.read_only.EmailManagerReadOnly; +import org.orcid.core.manager.v3.read_only.ProfileEntityManagerReadOnly; import org.orcid.core.togglz.Features; +import org.orcid.persistence.jpa.entities.ProfileEntity; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.security.core.authority.SimpleGrantedAuthority; +import org.springframework.security.core.userdetails.User; +import org.springframework.test.util.ReflectionTestUtils; import org.togglz.core.manager.FeatureManager; import org.togglz.core.repository.FeatureState; +import org.togglz.junit.TogglzRule; /** * Unit tests for HomeController config.json / togglz feature value logic. @@ -105,6 +125,74 @@ public void getFeatureValue_invalidPercentageFallsBackToTrue() { assertEquals("true", controller.getFeatureValue(featureManager, TEST_FEATURE)); } + @Rule + public TogglzRule togglzRule = TogglzRule.allDisabled(Features.class); + + private static final String ORCID = "0000-0000-0000-0001"; + + private static final String DELEGATE_ORCID = "0000-0000-0000-0002"; + + private final ProfileEntityManagerReadOnly profileEntityManagerReadOnly = mock(ProfileEntityManagerReadOnly.class); + + @Test + public void getUserInfo_reportsAMandatoryPasswordResetToTheRecordsOwnUser() { + togglzRule.enable(Features.FORCE_PASSWORD_RESET); + when(profileEntityManagerReadOnly.isPasswordResetRequired(ORCID)).thenReturn(true); + + Map info = signedIn(ORCID).getUserInfo(new MockHttpServletRequest()); + + assertEquals("true", info.get("FORCE_PASSWORD_RESET")); + } + + @Test + public void getUserInfo_reportsFalseForARecordThatIsNotFlagged() { + togglzRule.enable(Features.FORCE_PASSWORD_RESET); + + Map info = signedIn(ORCID).getUserInfo(new MockHttpServletRequest()); + + assertEquals("false", info.get("FORCE_PASSWORD_RESET")); + } + + @Test + public void getUserInfo_saysNothingWhileTheFeatureIsOff() { + when(profileEntityManagerReadOnly.isPasswordResetRequired(ORCID)).thenReturn(true); + + Map info = signedIn(ORCID).getUserInfo(new MockHttpServletRequest()); + + assertFalse(info.containsKey("FORCE_PASSWORD_RESET")); + verify(profileEntityManagerReadOnly, never()).isPasswordResetRequired(anyString()); + } + + @Test + public void getUserInfo_saysNothingToADelegate() { + togglzRule.enable(Features.FORCE_PASSWORD_RESET); + when(profileEntityManagerReadOnly.isPasswordResetRequired(ORCID)).thenReturn(true); + + Map info = signedIn(DELEGATE_ORCID).getUserInfo(new MockHttpServletRequest()); + + assertEquals("true", info.get("IN_DELEGATION_MODE")); + assertFalse(info.containsKey("FORCE_PASSWORD_RESET")); + verify(profileEntityManagerReadOnly, never()).isPasswordResetRequired(anyString()); + } + + /** A controller signed in as ORCID's record, by its own user or by a delegate. */ + private HomeController signedIn(String realUserOrcid) { + HomeController controller = spy(new HomeController()); + doReturn(new User(ORCID, "password", List.of(new SimpleGrantedAuthority("ROLE_USER")))).when(controller).getCurrentUser(); + doReturn(realUserOrcid).when(controller).getRealUserOrcid(); + + ProfileEntity profile = new ProfileEntity(ORCID); + profile.setClaimed(true); + ProfileEntityCacheManager profileEntityCacheManager = mock(ProfileEntityCacheManager.class); + when(profileEntityCacheManager.retrieve(ORCID)).thenReturn(profile); + + ReflectionTestUtils.setField(controller, "profileEntityCacheManager", profileEntityCacheManager); + ReflectionTestUtils.setField(controller, "emailManagerReadOnly", mock(EmailManagerReadOnly.class)); + ReflectionTestUtils.setField(controller, "sourceManager", mock(SourceManager.class)); + ReflectionTestUtils.setField(controller, "profileEntityManagerReadOnly", profileEntityManagerReadOnly); + return controller; + } + /** * Subclass to expose protected getFeatureValue for testing. */