diff --git a/orcid-api-common/src/main/resources/orcid-oauth2-api-common-config.xml b/orcid-api-common/src/main/resources/orcid-oauth2-api-common-config.xml
index fea74d3b950..77aff8a541b 100644
--- a/orcid-api-common/src/main/resources/orcid-oauth2-api-common-config.xml
+++ b/orcid-api-common/src/main/resources/orcid-oauth2-api-common-config.xml
@@ -55,6 +55,7 @@
+
diff --git a/orcid-core/src/main/java/org/orcid/core/manager/RecoveryPhone.java b/orcid-core/src/main/java/org/orcid/core/manager/RecoveryPhone.java
new file mode 100644
index 00000000000..87c25198097
--- /dev/null
+++ b/orcid-core/src/main/java/org/orcid/core/manager/RecoveryPhone.java
@@ -0,0 +1,39 @@
+package org.orcid.core.manager;
+
+import java.util.Date;
+
+/**
+ * The stored recovery phone for a record, as far as anything outside the
+ * persistence layer is allowed to see it: the last four digits and the dates.
+ * The number itself never leaves the database in a readable form through this
+ * object. The one path that reads it is
+ * {@link RecoveryPhoneManager#getDecryptedPhoneNumber(String)}, kept separate
+ * so that the callers who only want the mask and the dates never decrypt.
+ */
+public class RecoveryPhone {
+
+ private final String lastFour;
+
+ private final Date dateCreated;
+
+ private final Date lastModified;
+
+ public RecoveryPhone(String lastFour, Date dateCreated, Date lastModified) {
+ this.lastFour = lastFour;
+ this.dateCreated = dateCreated;
+ this.lastModified = lastModified;
+ }
+
+ public String getLastFour() {
+ return lastFour;
+ }
+
+ public Date getDateCreated() {
+ return dateCreated;
+ }
+
+ public Date getLastModified() {
+ return lastModified;
+ }
+
+}
diff --git a/orcid-core/src/main/java/org/orcid/core/manager/RecoveryPhoneManager.java b/orcid-core/src/main/java/org/orcid/core/manager/RecoveryPhoneManager.java
new file mode 100644
index 00000000000..68dc6958801
--- /dev/null
+++ b/orcid-core/src/main/java/org/orcid/core/manager/RecoveryPhoneManager.java
@@ -0,0 +1,42 @@
+package org.orcid.core.manager;
+
+public interface RecoveryPhoneManager {
+
+ /**
+ * The stored recovery phone for the record, or null if there isn't one.
+ * It carries the last four digits and the dates only, so it costs no
+ * decryption: the Account settings panel polls this on every refresh.
+ */
+ RecoveryPhone getRecoveryPhone(String orcid);
+
+ /**
+ * The stored number in E.164 form, or null when there is none.
+ *
+ * This is the only path that decrypts the stored number, and it exists for
+ * the flows that have to send a text to it without the user re-typing it.
+ * The value must never be written to a log line, put in a RUM attribute key
+ * or value, or returned over HTTP (R1.2): anything that reaches the user
+ * carries the last four digits only.
+ */
+ String getDecryptedPhoneNumber(String orcid);
+
+ /**
+ * Stores the given E.164 number as the record's recovery phone, replacing
+ * any existing one. The number is persisted reversibly encrypted, together
+ * with the last four digits.
+ *
+ * @return the stored recovery phone as it now stands, from the row this
+ * call wrote. Callers answer from it rather than calling
+ * {@link #getRecoveryPhone(String)} straight after: that read goes
+ * to the read-only pool, which is a replica in a deployed
+ * environment and may not yet carry what was just written.
+ */
+ RecoveryPhone saveRecoveryPhone(String orcid, String e164PhoneNumber);
+
+ /**
+ * Removes the recovery phone, if any. Called when 2FA is disabled, since
+ * turning 2FA off resets every 2FA backup option.
+ */
+ void removeRecoveryPhone(String orcid);
+
+}
diff --git a/orcid-core/src/main/java/org/orcid/core/manager/TwoFactorAuthenticationManager.java b/orcid-core/src/main/java/org/orcid/core/manager/TwoFactorAuthenticationManager.java
index 229bc486429..b343469eee9 100644
--- a/orcid-core/src/main/java/org/orcid/core/manager/TwoFactorAuthenticationManager.java
+++ b/orcid-core/src/main/java/org/orcid/core/manager/TwoFactorAuthenticationManager.java
@@ -11,6 +11,17 @@ public interface TwoFactorAuthenticationManager {
void disable2FA(String orcid);
+ /**
+ * The R3.5 transaction: turns 2FA off, deletes the stored recovery phone number and invalidates the unused backup
+ * codes, recording a distinct profile event so support can tell a recovery phone disable from a self service one.
+ * Used by the sign in and the authentication challenge recovery phone flows, where the user proved possession of
+ * the recovery number instead of a 2FA code, and the number is consumed by that single use.
+ *
+ * @param orcid
+ * the ORCID iD of the record whose 2FA is being disabled
+ */
+ void disable2FAByRecoveryPhone(String orcid);
+
void adminDisable2FA(String orcid, String adminOrcidId);
boolean userUsing2FA(String orcid);
diff --git a/orcid-core/src/main/java/org/orcid/core/manager/impl/RecoveryPhoneManagerImpl.java b/orcid-core/src/main/java/org/orcid/core/manager/impl/RecoveryPhoneManagerImpl.java
new file mode 100644
index 00000000000..26c29c4c494
--- /dev/null
+++ b/orcid-core/src/main/java/org/orcid/core/manager/impl/RecoveryPhoneManagerImpl.java
@@ -0,0 +1,87 @@
+package org.orcid.core.manager.impl;
+
+import jakarta.annotation.Resource;
+
+import org.orcid.core.manager.EncryptionManager;
+import org.orcid.core.manager.RecoveryPhone;
+import org.orcid.core.manager.RecoveryPhoneManager;
+import org.orcid.persistence.dao.ProfileEventDao;
+import org.orcid.persistence.dao.ProfileRecoveryPhoneDao;
+import org.orcid.persistence.jpa.entities.ProfileEventEntity;
+import org.orcid.persistence.jpa.entities.ProfileEventType;
+import org.orcid.persistence.jpa.entities.ProfileRecoveryPhoneEntity;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.transaction.annotation.Transactional;
+
+public class RecoveryPhoneManagerImpl implements RecoveryPhoneManager {
+
+ private static final Logger LOG = LoggerFactory.getLogger(RecoveryPhoneManagerImpl.class);
+
+ private static final int LAST_FOUR_LENGTH = 4;
+
+ @Resource
+ private ProfileRecoveryPhoneDao profileRecoveryPhoneDao;
+
+ @Resource
+ private EncryptionManager encryptionManager;
+
+ @Resource
+ private ProfileEventDao profileEventDao;
+
+ @Override
+ public RecoveryPhone getRecoveryPhone(String orcid) {
+ ProfileRecoveryPhoneEntity entity = profileRecoveryPhoneDao.findByOrcid(orcid);
+ if (entity == null) {
+ return null;
+ }
+ return toRecoveryPhone(entity);
+ }
+
+ @Override
+ public String getDecryptedPhoneNumber(String orcid) {
+ ProfileRecoveryPhoneEntity entity = profileRecoveryPhoneDao.findByOrcid(orcid);
+ if (entity == null) {
+ return null;
+ }
+ // Decrypting lives here and not in getRecoveryPhone because the Account settings panel polls
+ // that one for the mask and the dates alone; only the flows that have to text the number pay
+ // for the crypto, and the result must not reach a log line, a RUM attribute or an HTTP response.
+ return encryptionManager.decryptForInternalUse(entity.getEncryptedPhoneNumber());
+ }
+
+ @Override
+ @Transactional
+ public RecoveryPhone saveRecoveryPhone(String orcid, String e164PhoneNumber) {
+ // Whether this is a first number or a replacement is the DAO's to say: it decides
+ // inside its own write transaction. Asking findByOrcid here would open a second,
+ // read-only connection in the middle of this write and, on a deployed environment,
+ // ask a replica that may still be missing a row this record wrote seconds ago
+ ProfileRecoveryPhoneDao.UpsertResult result = profileRecoveryPhoneDao.upsert(orcid,
+ encryptionManager.encryptForInternalUse(e164PhoneNumber), lastFour(e164PhoneNumber));
+ boolean isNew = result.isInserted();
+ profileEventDao.persist(new ProfileEventEntity(orcid,
+ isNew ? ProfileEventType.PROFILE_RECOVERY_PHONE_ADDED : ProfileEventType.PROFILE_RECOVERY_PHONE_UPDATED));
+ LOG.info("Recovery phone {} for {}", isNew ? "added" : "updated", orcid);
+ return toRecoveryPhone(result.getEntity());
+ }
+
+ @Override
+ @Transactional
+ public void removeRecoveryPhone(String orcid) {
+ if (profileRecoveryPhoneDao.deleteByOrcid(orcid)) {
+ profileEventDao.persist(new ProfileEventEntity(orcid, ProfileEventType.PROFILE_RECOVERY_PHONE_REMOVED));
+ LOG.info("Recovery phone removed for {}", orcid);
+ }
+ }
+
+ private static RecoveryPhone toRecoveryPhone(ProfileRecoveryPhoneEntity entity) {
+ return new RecoveryPhone(entity.getLastFour(), entity.getDateCreated(), entity.getLastModified());
+ }
+
+ private String lastFour(String e164PhoneNumber) {
+ String digits = e164PhoneNumber.replaceAll("\\D", "");
+ return digits.length() <= LAST_FOUR_LENGTH ? digits : digits.substring(digits.length() - LAST_FOUR_LENGTH);
+ }
+
+}
diff --git a/orcid-core/src/main/java/org/orcid/core/manager/impl/TwoFactorAuthenticationManagerImpl.java b/orcid-core/src/main/java/org/orcid/core/manager/impl/TwoFactorAuthenticationManagerImpl.java
index 4d8fa7d40d8..85109b16f29 100644
--- a/orcid-core/src/main/java/org/orcid/core/manager/impl/TwoFactorAuthenticationManagerImpl.java
+++ b/orcid-core/src/main/java/org/orcid/core/manager/impl/TwoFactorAuthenticationManagerImpl.java
@@ -10,6 +10,7 @@
import org.orcid.core.manager.BackupCodeManager;
import org.orcid.core.manager.EncryptionManager;
import org.orcid.core.manager.ProfileEntityCacheManager;
+import org.orcid.core.manager.RecoveryPhoneManager;
import org.orcid.core.manager.TwoFactorAuthenticationManager;
import org.orcid.core.manager.read_only.EmailManagerReadOnly;
import org.orcid.jaxb.model.record_v2.Email;
@@ -43,6 +44,9 @@ public class TwoFactorAuthenticationManagerImpl implements TwoFactorAuthenticati
@Resource
private BackupCodeManager backupCodeManager;
+ @Resource
+ private RecoveryPhoneManager recoveryPhoneManager;
+
@Resource
private ProfileEventDao profileEventDao;
@@ -98,12 +102,31 @@ public void disable2FA(String orcid) {
public Boolean doInTransaction(TransactionStatus status) {
profileDao.disable2FA(orcid);
backupCodeManager.removeUnusedBackupCodes(orcid);
+ recoveryPhoneManager.removeRecoveryPhone(orcid);
profileEventDao.persist(new ProfileEventEntity(orcid, ProfileEventType.PROFILE_2FA_DISABLED));
return true;
}
});
}
+ @Override
+ public void disable2FAByRecoveryPhone(String orcid) {
+ // {}, not %s: slf4j takes its placeholder that way. The line above uses %s and has therefore
+ // been printing the literal characters instead of the iD for as long as it has existed; that one
+ // is not this ticket's to change, but there is no reason to copy it.
+ LOG.warn("2FA disabled by recovery phone for {}", orcid);
+ transactionTemplate.execute(new TransactionCallback() {
+ @Override
+ public Boolean doInTransaction(TransactionStatus status) {
+ profileDao.disable2FA(orcid);
+ backupCodeManager.removeUnusedBackupCodes(orcid);
+ recoveryPhoneManager.removeRecoveryPhone(orcid);
+ profileEventDao.persist(new ProfileEventEntity(orcid, ProfileEventType.PROFILE_2FA_DISABLED_BY_RECOVERY_PHONE));
+ return true;
+ }
+ });
+ }
+
@Override
public void adminDisable2FA(String orcid, String adminOrcidId) {
String message = String.format("Admin %s have disabled 2FA for %s", adminOrcidId, orcid);
@@ -113,6 +136,7 @@ public void adminDisable2FA(String orcid, String adminOrcidId) {
public Boolean doInTransaction(TransactionStatus status) {
profileDao.disable2FA(orcid);
backupCodeManager.removeUnusedBackupCodes(orcid);
+ recoveryPhoneManager.removeRecoveryPhone(orcid);
profileEventDao.persist(new ProfileEventEntity(orcid, ProfileEventType.PROFILE_2FA_DISABLED_BY_ADMIN, message));
return true;
}
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 1257f31ffc8..f01b07db624 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
@@ -18,6 +18,7 @@
import org.orcid.core.manager.ClientDetailsEntityCacheManager;
import org.orcid.core.manager.EncryptionManager;
import org.orcid.core.manager.ProfileEntityCacheManager;
+import org.orcid.core.manager.RecoveryPhoneManager;
import org.orcid.core.manager.impl.OrcidUrlManager;
import org.orcid.core.manager.v3.*;
import org.orcid.core.manager.v3.read_only.RecordNameManagerReadOnly;
@@ -129,6 +130,9 @@ public class ProfileEntityManagerImpl extends ProfileEntityManagerReadOnlyImpl i
@Resource
protected BackupCodeDao backupCodeDao;
+
+ @Resource
+ private RecoveryPhoneManager recoveryPhoneManager;
@Resource
private ProfileLastModifiedDao profileLastModifiedDao;
@@ -678,6 +682,10 @@ private void clearRecord(String orcid, Boolean disableTokens) {
// Admin disabling 2FA, so, we should not notify the user
profileDao.disable2FA(orcid);
backupCodeDao.removedUsedBackupCodes(orcid);
+ // The recovery phone number is 2FA backup state too, and it is personal data that must
+ // not outlive the record: clearing 2FA at the DAO leaves the encrypted number behind,
+ // so it goes the same way it does when 2FA is turned off through the manager.
+ recoveryPhoneManager.removeRecoveryPhone(orcid);
// delete notifications
notificationManager.deleteNotificationsForRecord(orcid);
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 997407fdc9d..16536948ff1 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
@@ -113,7 +113,13 @@ public enum Features implements Feature {
SEND_EMAIL_ON_DEPRECATE_RECORD,
@Label("Send email on reset password")
- SEND_EMAIL_ON_RESET_PASSWORD;
+ SEND_EMAIL_ON_RESET_PASSWORD,
+
+ @Label("2FA recovery phone number (add/manage from account settings)")
+ TWO_FACTOR_RECOVERY_PHONE,
+
+ @Label("Login - recovery phone interstitial")
+ LOGIN_RECOVERY_PHONE_INTERSTITIAL;
public boolean isActive() {
return FeatureContext.getFeatureManager().isActive(this);
}
diff --git a/orcid-core/src/main/java/org/orcid/core/utils/cache/redis/RedisClient.java b/orcid-core/src/main/java/org/orcid/core/utils/cache/redis/RedisClient.java
index bf4551b2c0a..2c4f3ee8f39 100644
--- a/orcid-core/src/main/java/org/orcid/core/utils/cache/redis/RedisClient.java
+++ b/orcid-core/src/main/java/org/orcid/core/utils/cache/redis/RedisClient.java
@@ -37,6 +37,7 @@ public class RedisClient {
private static final int DEFAULT_CACHE_EXPIRY = 60;
private static final int DEFAULT_TIMEOUT = 10000;
+ private static final boolean DEFAULT_USE_SSL = true;
public static final int MACH_KEY_BATCH_SIZE = 1000;
private final String redisHost;
@@ -44,6 +45,10 @@ public class RedisClient {
private final String redisPassword;
private final int cacheExpiryInSecs;
private final int clientTimeoutInMillis;
+ // TLS is on unless a deployment says otherwise. A plaintext Redis with TLS forced on does not
+ // reject the handshake, it simply never answers, so the connect blocks until the socket read
+ // times out -- once per client, per Spring context.
+ private final boolean useSsl;
public JedisPool pool;
private SetParams defaultSetParams;
@@ -54,34 +59,47 @@ public class RedisClient {
private boolean enabled = false;
public RedisClient(String redisHost, int redisPort, String password) {
- this.redisHost = redisHost;
- this.redisPort = redisPort;
- this.redisPassword = password;
- this.cacheExpiryInSecs = DEFAULT_CACHE_EXPIRY;
- this.clientTimeoutInMillis = DEFAULT_TIMEOUT;
+ this(redisHost, redisPort, password, DEFAULT_CACHE_EXPIRY, DEFAULT_TIMEOUT, DEFAULT_USE_SSL);
}
public RedisClient(String redisHost, int redisPort, String password, int cacheExpiryInSecs) {
- this.redisHost = redisHost;
- this.redisPort = redisPort;
- this.redisPassword = password;
- this.cacheExpiryInSecs = cacheExpiryInSecs;
- this.clientTimeoutInMillis = DEFAULT_TIMEOUT;
+ this(redisHost, redisPort, password, cacheExpiryInSecs, DEFAULT_TIMEOUT, DEFAULT_USE_SSL);
}
public RedisClient(String redisHost, int redisPort, String password, int cacheExpiryInSecs, int clientTimeoutInMillis) {
+ this(redisHost, redisPort, password, cacheExpiryInSecs, clientTimeoutInMillis, DEFAULT_USE_SSL);
+ }
+
+ public RedisClient(String redisHost, int redisPort, String password, int cacheExpiryInSecs, int clientTimeoutInMillis, boolean useSsl) {
this.redisHost = redisHost;
this.redisPort = redisPort;
this.redisPassword = password;
this.cacheExpiryInSecs = cacheExpiryInSecs;
this.clientTimeoutInMillis = clientTimeoutInMillis;
+ this.useSsl = useSsl;
+ }
+
+ /** The Jedis configuration this client would connect with. Package-private so it can be
+ * asserted on without opening a connection. */
+ JedisClientConfig buildClientConfig() {
+ DefaultJedisClientConfig.Builder builder = DefaultJedisClientConfig.builder()
+ .connectionTimeoutMillis(this.clientTimeoutInMillis)
+ .socketTimeoutMillis(this.clientTimeoutInMillis)
+ .ssl(this.useSsl);
+ // A blank password means "no authentication". Setting it anyway makes Jedis send
+ // AUTH "", which a Redis with no requirepass rejects outright:
+ // ERR AUTH called without any password configured for the default user
+ // so the client would fail to connect against an unauthenticated server.
+ if (this.redisPassword != null && !this.redisPassword.trim().isEmpty()) {
+ builder.password(this.redisPassword);
+ }
+ return builder.build();
}
@PostConstruct
private void init() {
try {
- JedisClientConfig config = DefaultJedisClientConfig.builder().connectionTimeoutMillis(this.clientTimeoutInMillis)
- .socketTimeoutMillis(this.clientTimeoutInMillis).password(this.redisPassword).ssl(true).build();
+ JedisClientConfig config = buildClientConfig();
pool = new JedisPool(new HostAndPort(this.redisHost, this.redisPort), config);
defaultSetParams = new SetParams();
defaultSetParams.ex(Long.valueOf(this.cacheExpiryInSecs));
diff --git a/orcid-core/src/main/java/org/orcid/pojo/TwoFactorAuthStatus.java b/orcid-core/src/main/java/org/orcid/pojo/TwoFactorAuthStatus.java
index 5c7fc896e59..29030c6c063 100644
--- a/orcid-core/src/main/java/org/orcid/pojo/TwoFactorAuthStatus.java
+++ b/orcid-core/src/main/java/org/orcid/pojo/TwoFactorAuthStatus.java
@@ -10,6 +10,14 @@ public class TwoFactorAuthStatus extends AuthChallenge {
private Date recoveryCodeCreationDate;
+ private String maskedRecoveryPhoneNumber;
+
+ private Date recoveryPhoneCreationDate;
+
+ private Date recoveryPhoneLastModifiedDate;
+
+ private boolean recoveryPhoneModified;
+
public boolean isEnabled() {
return enabled;
}
@@ -33,4 +41,36 @@ public Date getRecoveryCodeCreationDate() {
public void setRecoveryCodeCreationDate(Date recoveryCodeCreationDate) {
this.recoveryCodeCreationDate = recoveryCodeCreationDate;
}
+
+ public String getMaskedRecoveryPhoneNumber() {
+ return maskedRecoveryPhoneNumber;
+ }
+
+ public void setMaskedRecoveryPhoneNumber(String maskedRecoveryPhoneNumber) {
+ this.maskedRecoveryPhoneNumber = maskedRecoveryPhoneNumber;
+ }
+
+ public Date getRecoveryPhoneCreationDate() {
+ return recoveryPhoneCreationDate;
+ }
+
+ public void setRecoveryPhoneCreationDate(Date recoveryPhoneCreationDate) {
+ this.recoveryPhoneCreationDate = recoveryPhoneCreationDate;
+ }
+
+ public Date getRecoveryPhoneLastModifiedDate() {
+ return recoveryPhoneLastModifiedDate;
+ }
+
+ public void setRecoveryPhoneLastModifiedDate(Date recoveryPhoneLastModifiedDate) {
+ this.recoveryPhoneLastModifiedDate = recoveryPhoneLastModifiedDate;
+ }
+
+ public boolean isRecoveryPhoneModified() {
+ return recoveryPhoneModified;
+ }
+
+ public void setRecoveryPhoneModified(boolean recoveryPhoneModified) {
+ this.recoveryPhoneModified = recoveryPhoneModified;
+ }
}
diff --git a/orcid-core/src/main/resources/orcid-core-context.xml b/orcid-core/src/main/resources/orcid-core-context.xml
index c2fd0a2958a..2a2e288e4b7 100644
--- a/orcid-core/src/main/resources/orcid-core-context.xml
+++ b/orcid-core/src/main/resources/orcid-core-context.xml
@@ -729,6 +729,8 @@
+
+
@@ -1039,6 +1041,7 @@
+
@@ -1048,6 +1051,7 @@
+
diff --git a/orcid-core/src/test/java/org/orcid/core/manager/RecoveryPhoneManagerTest.java b/orcid-core/src/test/java/org/orcid/core/manager/RecoveryPhoneManagerTest.java
new file mode 100644
index 00000000000..c4ecac956b8
--- /dev/null
+++ b/orcid-core/src/test/java/org/orcid/core/manager/RecoveryPhoneManagerTest.java
@@ -0,0 +1,206 @@
+package org.orcid.core.manager;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoInteractions;
+import static org.mockito.Mockito.when;
+
+import java.util.Date;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.ArgumentCaptor;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+import org.orcid.core.manager.impl.RecoveryPhoneManagerImpl;
+import org.orcid.persistence.dao.ProfileEventDao;
+import org.orcid.persistence.dao.ProfileRecoveryPhoneDao;
+import org.orcid.persistence.jpa.entities.ProfileEventEntity;
+import org.orcid.persistence.jpa.entities.ProfileEventType;
+import org.orcid.persistence.jpa.entities.ProfileRecoveryPhoneEntity;
+import org.orcid.test.OrcidJUnit4ClassRunner;
+import org.springframework.test.context.ContextConfiguration;
+
+@RunWith(OrcidJUnit4ClassRunner.class)
+@ContextConfiguration(locations = { "classpath:test-orcid-core-context.xml" })
+public class RecoveryPhoneManagerTest {
+
+ private static final String ORCID = "0000-0000-0000-0001";
+
+ private static final String PHONE = "+441234567890";
+
+ private static final String ENCRYPTED = "encrypted-number";
+
+ @Mock
+ private ProfileRecoveryPhoneDao profileRecoveryPhoneDao;
+
+ @Mock
+ private EncryptionManager encryptionManager;
+
+ @Mock
+ private ProfileEventDao profileEventDao;
+
+ @InjectMocks
+ private RecoveryPhoneManagerImpl recoveryPhoneManager;
+
+ @Before
+ public void init() {
+ MockitoAnnotations.initMocks(this);
+ }
+
+ @Test
+ public void getRecoveryPhoneReturnsNullWhenNoneStored() {
+ when(profileRecoveryPhoneDao.findByOrcid(ORCID)).thenReturn(null);
+ assertNull(recoveryPhoneManager.getRecoveryPhone(ORCID));
+ }
+
+ @Test
+ public void getRecoveryPhoneMapsTheLastFourAndTheDates() {
+ Date created = new Date(1000L);
+ Date modified = new Date(2000L);
+ when(profileRecoveryPhoneDao.findByOrcid(ORCID)).thenReturn(entity(ENCRYPTED, "7890", created, modified));
+
+ RecoveryPhone recoveryPhone = recoveryPhoneManager.getRecoveryPhone(ORCID);
+
+ assertEquals("7890", recoveryPhone.getLastFour());
+ assertEquals(created, recoveryPhone.getDateCreated());
+ assertEquals(modified, recoveryPhone.getLastModified());
+ }
+
+ @Test
+ public void getRecoveryPhoneNeverDecrypts() {
+ when(profileRecoveryPhoneDao.findByOrcid(ORCID)).thenReturn(entity(ENCRYPTED, "7890", new Date(), new Date()));
+
+ recoveryPhoneManager.getRecoveryPhone(ORCID);
+
+ // The point of keeping the number on its own accessor: the Account settings panel polls this
+ // one for the mask and the dates, and no poll should cost a decryption.
+ verifyNoInteractions(encryptionManager);
+ }
+
+ @Test
+ public void getDecryptedPhoneNumberDecryptsTheStoredNumber() {
+ when(profileRecoveryPhoneDao.findByOrcid(ORCID)).thenReturn(entity(ENCRYPTED, "7890", new Date(), new Date()));
+ when(encryptionManager.decryptForInternalUse(ENCRYPTED)).thenReturn(PHONE);
+
+ String phoneNumber = recoveryPhoneManager.getDecryptedPhoneNumber(ORCID);
+
+ // The stored column is what gets decrypted, and the caller gets the number back in full:
+ // the recovery flows text it without the user re-typing it.
+ verify(encryptionManager).decryptForInternalUse(ENCRYPTED);
+ assertEquals(PHONE, phoneNumber);
+ }
+
+ @Test
+ public void getDecryptedPhoneNumberReturnsNullWhenNoneStored() {
+ when(profileRecoveryPhoneDao.findByOrcid(ORCID)).thenReturn(null);
+ assertNull(recoveryPhoneManager.getDecryptedPhoneNumber(ORCID));
+ }
+
+ @Test
+ public void saveStoresTheEncryptedNumberAndTheLastFour() {
+ Date now = new Date();
+ when(encryptionManager.encryptForInternalUse(PHONE)).thenReturn(ENCRYPTED);
+ when(profileRecoveryPhoneDao.upsert(ORCID, ENCRYPTED, "7890"))
+ .thenReturn(new ProfileRecoveryPhoneDao.UpsertResult(entity(ENCRYPTED, "7890", now, now), true));
+
+ RecoveryPhone saved = recoveryPhoneManager.saveRecoveryPhone(ORCID, PHONE);
+
+ // Answered from the row the upsert wrote, never from a second lookup
+ assertEquals("7890", saved.getLastFour());
+ assertEquals(now, saved.getDateCreated());
+ verify(profileRecoveryPhoneDao, never()).findByOrcid(anyString());
+ verify(profileRecoveryPhoneDao).upsert(ORCID, ENCRYPTED, "7890");
+ // Reversibly encrypted, never hashed, and the plain number never reaches the DAO.
+ verify(encryptionManager, never()).hashForInternalUse(anyString());
+ verify(profileRecoveryPhoneDao, never()).upsert(eq(ORCID), eq(PHONE), anyString());
+ assertEquals(ProfileEventType.PROFILE_RECOVERY_PHONE_ADDED, capturedEventType());
+ }
+
+ /*
+ * findByOrcid runs on the read-only pool. Here it says there is no row - the
+ * answer a lagging replica gives seconds after the primary stored one - while
+ * the upsert, on the primary, found the row and replaced it. The event follows
+ * the upsert: recording this as an "added" would be wrong in the audit trail.
+ */
+ @Test
+ public void savingOverAnExistingNumberIsRecordedAsAnUpdate() {
+ Date created = new Date(System.currentTimeMillis() - 60_000L);
+ Date modified = new Date();
+ when(profileRecoveryPhoneDao.findByOrcid(ORCID)).thenReturn(null);
+ when(encryptionManager.encryptForInternalUse(anyString())).thenReturn("new-encrypted");
+ when(profileRecoveryPhoneDao.upsert(ORCID, "new-encrypted", "7890"))
+ .thenReturn(new ProfileRecoveryPhoneDao.UpsertResult(entity("new-encrypted", "7890", created, modified), false));
+
+ RecoveryPhone saved = recoveryPhoneManager.saveRecoveryPhone(ORCID, PHONE);
+
+ assertEquals(created, saved.getDateCreated());
+ assertEquals(modified, saved.getLastModified());
+ verify(profileRecoveryPhoneDao).upsert(ORCID, "new-encrypted", "7890");
+ assertEquals(ProfileEventType.PROFILE_RECOVERY_PHONE_UPDATED, capturedEventType());
+ }
+
+ @Test
+ public void lastFourIsTakenFromTheDigitsOnly() {
+ when(encryptionManager.encryptForInternalUse(anyString())).thenReturn(ENCRYPTED);
+ when(profileRecoveryPhoneDao.upsert(eq(ORCID), anyString(), anyString()))
+ .thenReturn(new ProfileRecoveryPhoneDao.UpsertResult(entity(ENCRYPTED, "9876", new Date(), new Date()), true));
+
+ recoveryPhoneManager.saveRecoveryPhone(ORCID, "+1 (555) 010-9876");
+
+ verify(profileRecoveryPhoneDao).upsert(eq(ORCID), anyString(), eq("9876"));
+ }
+
+ @Test
+ public void removeIsSilentWhenThereWasNothingToRemove() {
+ when(profileRecoveryPhoneDao.deleteByOrcid(ORCID)).thenReturn(false);
+
+ recoveryPhoneManager.removeRecoveryPhone(ORCID);
+
+ verify(profileEventDao, never()).persist(any(ProfileEventEntity.class));
+ }
+
+ @Test
+ public void removeRecordsAnEventWhenANumberWasDeleted() {
+ when(profileRecoveryPhoneDao.deleteByOrcid(ORCID)).thenReturn(true);
+
+ recoveryPhoneManager.removeRecoveryPhone(ORCID);
+
+ assertEquals(ProfileEventType.PROFILE_RECOVERY_PHONE_REMOVED, capturedEventType());
+ }
+
+ private ProfileEventType capturedEventType() {
+ ArgumentCaptor captor = ArgumentCaptor.forClass(ProfileEventEntity.class);
+ verify(profileEventDao).persist(captor.capture());
+ return captor.getValue().getType();
+ }
+
+ private static ProfileRecoveryPhoneEntity entity(String encrypted, String lastFour, Date created, Date modified) {
+ ProfileRecoveryPhoneEntity entity = new ProfileRecoveryPhoneEntity() {
+ private static final long serialVersionUID = 1L;
+
+ @Override
+ public Date getDateCreated() {
+ return created;
+ }
+
+ @Override
+ public Date getLastModified() {
+ return modified;
+ }
+ };
+ entity.setEncryptedPhoneNumber(encrypted);
+ entity.setLastFour(lastFour);
+ return entity;
+ }
+
+}
diff --git a/orcid-core/src/test/java/org/orcid/core/manager/TwoFactorAuthenticationManagerTest.java b/orcid-core/src/test/java/org/orcid/core/manager/TwoFactorAuthenticationManagerTest.java
index d58ebd18b84..f25c85122b9 100644
--- a/orcid-core/src/test/java/org/orcid/core/manager/TwoFactorAuthenticationManagerTest.java
+++ b/orcid-core/src/test/java/org/orcid/core/manager/TwoFactorAuthenticationManagerTest.java
@@ -2,11 +2,14 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -17,6 +20,7 @@
import org.jboss.aerogear.security.otp.api.Base32;
import org.junit.Before;
import org.junit.Test;
+import org.mockito.ArgumentCaptor;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
@@ -30,6 +34,7 @@
import org.orcid.persistence.dao.ProfileEventDao;
import org.orcid.persistence.jpa.entities.ProfileEntity;
import org.orcid.persistence.jpa.entities.ProfileEventEntity;
+import org.orcid.persistence.jpa.entities.ProfileEventType;
import org.orcid.pojo.AuthChallenge;
import org.springframework.transaction.support.TransactionCallback;
import org.springframework.transaction.support.TransactionTemplate;
@@ -48,6 +53,9 @@ public class TwoFactorAuthenticationManagerTest {
@Mock
private BackupCodeManager backupCodeManager;
+ @Mock
+ private RecoveryPhoneManager recoveryPhoneManager;
+
@Mock
private ProfileDao profileDao;
@@ -128,6 +136,35 @@ public void testDisable2FA() {
verify(profileDao).disable2FA(anyString());
verify(profileEventDao).persist(any(ProfileEventEntity.class));
verify(backupCodeManager).removeUnusedBackupCodes(anyString());
+ // turning 2FA off resets every 2FA backup option
+ verify(recoveryPhoneManager).removeRecoveryPhone(anyString());
+ }
+
+ @Test
+ public void testDisable2FAByRecoveryPhone() {
+ twoFactorAuthenticationManager.disable2FAByRecoveryPhone("orcid");
+
+ // the whole R3.5 transaction: 2FA off, unused backup codes invalidated, the recovery number consumed
+ verify(profileDao, times(1)).disable2FA(eq("orcid"));
+ verify(backupCodeManager, times(1)).removeUnusedBackupCodes(eq("orcid"));
+ verify(recoveryPhoneManager, times(1)).removeRecoveryPhone(eq("orcid"));
+
+ ArgumentCaptor captor = ArgumentCaptor.forClass(ProfileEventEntity.class);
+ verify(profileEventDao, times(1)).persist(captor.capture());
+ assertEquals("orcid", captor.getValue().getOrcid());
+ assertEquals(ProfileEventType.PROFILE_2FA_DISABLED_BY_RECOVERY_PHONE, captor.getValue().getType());
+ }
+
+ @Test
+ public void testDisable2FAByRecoveryPhoneDoesNotRecordASelfServiceDisable() {
+ twoFactorAuthenticationManager.disable2FAByRecoveryPhone("orcid");
+
+ // support tells a recovery phone disable from a self service one by the event type alone, so exactly one
+ // event is recorded and it is not the self service or the admin one
+ ArgumentCaptor captor = ArgumentCaptor.forClass(ProfileEventEntity.class);
+ verify(profileEventDao, times(1)).persist(captor.capture());
+ assertNotEquals(ProfileEventType.PROFILE_2FA_DISABLED, captor.getValue().getType());
+ assertNotEquals(ProfileEventType.PROFILE_2FA_DISABLED_BY_ADMIN, captor.getValue().getType());
}
@Test
@@ -136,6 +173,7 @@ public void testAdminDisable2FA() {
verify(profileDao).disable2FA(anyString());
verify(profileEventDao).persist(any(ProfileEventEntity.class));
verify(backupCodeManager).removeUnusedBackupCodes(anyString());
+ verify(recoveryPhoneManager).removeRecoveryPhone(anyString());
}
@Test
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 03821873d5d..06dbf925acf 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
@@ -44,6 +44,7 @@
import org.orcid.core.locale.LocaleManager;
import org.orcid.core.manager.ClientDetailsEntityCacheManager;
import org.orcid.core.manager.EncryptionManager;
+import org.orcid.core.manager.RecoveryPhoneManager;
import org.orcid.core.manager.v3.AddressManager;
import org.orcid.core.manager.v3.AffiliationsManager;
import org.orcid.core.manager.v3.BiographyManager;
@@ -179,6 +180,8 @@ public class ProfileEntityManagerImplTest {
private ResearcherUrlManager researcherUrlManager;
@Mock
private RedisClient redisClient;
+ @Mock
+ private RecoveryPhoneManager recoveryPhoneManager;
@Before
public void setUp() {
@@ -217,6 +220,7 @@ public void setUp() {
inject(ProfileEntityManagerImpl.class, "biographyManager", biographyManager);
inject(ProfileEntityManagerImpl.class, "emailFrequencyManager", emailFrequencyManager);
inject(ProfileEntityManagerImpl.class, "redisClient", redisClient);
+ inject(ProfileEntityManagerImpl.class, "recoveryPhoneManager", recoveryPhoneManager);
doAnswer(invocation -> {
TransactionCallback callback = (TransactionCallback) invocation.getArguments()[0];
@@ -367,6 +371,25 @@ public void deactivateRecordClearsRecordHidesEmailsAndDisablesTokens() {
verify(profileHistoryEventManager).recordEvent(ProfileHistoryEventType.SET_DEFAULT_VIS_TO_PRIVATE, "orcid", "deactivated/deprecated");
}
+ /*
+ * Deactivation clears 2FA at the DAO, which used to leave the recovery phone
+ * number - encrypted, but personal data about a named person - behind in
+ * profile_recovery_phone. It has to go the same way it does when 2FA is turned
+ * off through the manager. Deprecation runs the same clearRecord, so this one
+ * test covers both doors.
+ */
+ @Test
+ public void deactivateRecordRemovesTheRecoveryPhoneNumber() {
+ when(profileDao.updateDefaultVisibility("orcid", org.orcid.jaxb.model.common_v2.Visibility.PRIVATE.name())).thenReturn(true);
+ when(recordNameManagerV3.exists("orcid")).thenReturn(false);
+ when(biographyManager.exists("orcid")).thenReturn(false);
+
+ assertTrue(manager.deactivateRecord("orcid"));
+
+ verify(profileDao).disable2FA("orcid");
+ verify(recoveryPhoneManager).removeRecoveryPhone("orcid");
+ }
+
@Test
public void enableDeveloperToolsDelegates() {
when(profileDao.updateDeveloperTools("orcid", true)).thenReturn(true);
diff --git a/orcid-core/src/test/java/org/orcid/core/utils/cache/redis/RedisClientTest.java b/orcid-core/src/test/java/org/orcid/core/utils/cache/redis/RedisClientTest.java
new file mode 100644
index 00000000000..306fb440f91
--- /dev/null
+++ b/orcid-core/src/test/java/org/orcid/core/utils/cache/redis/RedisClientTest.java
@@ -0,0 +1,75 @@
+package org.orcid.core.utils.cache.redis;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+
+import org.junit.Test;
+
+import redis.clients.jedis.JedisClientConfig;
+
+/**
+ * TLS was hardcoded on, which meant a plaintext Redis never answered the handshake and every
+ * connect blocked for the full socket timeout -- once per client, per Spring context. These tests
+ * pin the flag and, more importantly, pin the default: production must keep getting TLS.
+ *
+ * Deliberately a plain unit test with no Spring context. RedisClient carries no
+ * {@code @Profile("!unitTests")} (unlike SessionCacheConfig and JedisPoolBuilder), so any
+ * context-loading test really attempts a TLS connect and waits out the timeout.
+ */
+public class RedisClientTest {
+
+ private static final String HOST = "localhost";
+ private static final int PORT = 6379;
+ private static final String PASSWORD = "";
+ private static final int EXPIRY = 600;
+ private static final int TIMEOUT = 10000;
+
+ @Test
+ public void sslIsEnabledByDefaultOnEveryConstructor() {
+ assertTrue("3-arg constructor must default to TLS",
+ new RedisClient(HOST, PORT, PASSWORD).buildClientConfig().isSsl());
+ assertTrue("4-arg constructor must default to TLS",
+ new RedisClient(HOST, PORT, PASSWORD, EXPIRY).buildClientConfig().isSsl());
+ assertTrue("5-arg constructor must default to TLS",
+ new RedisClient(HOST, PORT, PASSWORD, EXPIRY, TIMEOUT).buildClientConfig().isSsl());
+ }
+
+ @Test
+ public void sslCanBeTurnedOff() {
+ JedisClientConfig config = new RedisClient(HOST, PORT, PASSWORD, EXPIRY, TIMEOUT, false).buildClientConfig();
+ assertFalse("ssl=false must reach the Jedis config", config.isSsl());
+ }
+
+ @Test
+ public void sslCanBeTurnedOnExplicitly() {
+ JedisClientConfig config = new RedisClient(HOST, PORT, PASSWORD, EXPIRY, TIMEOUT, true).buildClientConfig();
+ assertTrue(config.isSsl());
+ }
+
+ @Test
+ public void aBlankPasswordMeansNoAuthentication() {
+ // Jedis sends AUTH "" if a password is set at all, and a Redis without requirepass
+ // rejects that outright, so a blank password must not reach the config.
+ assertNull("empty password must not be sent",
+ new RedisClient(HOST, PORT, "", EXPIRY, TIMEOUT, false).buildClientConfig().getPassword());
+ assertNull("blank password must not be sent",
+ new RedisClient(HOST, PORT, " ", EXPIRY, TIMEOUT, false).buildClientConfig().getPassword());
+ assertNull("null password must not be sent",
+ new RedisClient(HOST, PORT, null, EXPIRY, TIMEOUT, false).buildClientConfig().getPassword());
+ }
+
+ @Test
+ public void arealPasswordIsStillSent() {
+ assertEquals("s3cret",
+ new RedisClient(HOST, PORT, "s3cret", EXPIRY, TIMEOUT, true).buildClientConfig().getPassword());
+ }
+
+ @Test
+ public void theOtherSettingsStillReachTheConfig() {
+ JedisClientConfig config = new RedisClient(HOST, PORT, PASSWORD, EXPIRY, 1234, false).buildClientConfig();
+ assertEquals(1234, config.getConnectionTimeoutMillis());
+ assertEquals(1234, config.getSocketTimeoutMillis());
+ }
+}
diff --git a/orcid-persistence/src/main/java/org/orcid/persistence/dao/ProfileRecoveryPhoneDao.java b/orcid-persistence/src/main/java/org/orcid/persistence/dao/ProfileRecoveryPhoneDao.java
new file mode 100644
index 00000000000..b04a76b89a9
--- /dev/null
+++ b/orcid-persistence/src/main/java/org/orcid/persistence/dao/ProfileRecoveryPhoneDao.java
@@ -0,0 +1,44 @@
+package org.orcid.persistence.dao;
+
+import org.orcid.persistence.jpa.entities.ProfileRecoveryPhoneEntity;
+
+public interface ProfileRecoveryPhoneDao extends GenericDao {
+
+ ProfileRecoveryPhoneEntity findByOrcid(String orcid);
+
+ /**
+ * Stores the recovery phone for a record, replacing any existing one. A
+ * record can only ever have a single recovery phone number.
+ *
+ * @return the row as it now stands, flushed so its dates are the stored
+ * ones, and whether this call created it. A caller that needs to
+ * know what was written reads it from here rather than looking the
+ * row up again: {@link #findByOrcid} runs on the read-only
+ * transaction manager, which is a second connection inside a write
+ * and, in a deployed environment, a replica that may not yet carry
+ * the row.
+ */
+ UpsertResult upsert(String orcid, String encryptedPhoneNumber, String lastFour);
+
+ boolean deleteByOrcid(String orcid);
+
+ /** What an upsert did: the stored row and whether it was created by that call. */
+ final class UpsertResult {
+ private final ProfileRecoveryPhoneEntity entity;
+ private final boolean inserted;
+
+ public UpsertResult(ProfileRecoveryPhoneEntity entity, boolean inserted) {
+ this.entity = entity;
+ this.inserted = inserted;
+ }
+
+ public ProfileRecoveryPhoneEntity getEntity() {
+ return entity;
+ }
+
+ public boolean isInserted() {
+ return inserted;
+ }
+ }
+
+}
diff --git a/orcid-persistence/src/main/java/org/orcid/persistence/dao/impl/ProfileRecoveryPhoneDaoImpl.java b/orcid-persistence/src/main/java/org/orcid/persistence/dao/impl/ProfileRecoveryPhoneDaoImpl.java
new file mode 100644
index 00000000000..9a16ee99a0b
--- /dev/null
+++ b/orcid-persistence/src/main/java/org/orcid/persistence/dao/impl/ProfileRecoveryPhoneDaoImpl.java
@@ -0,0 +1,66 @@
+package org.orcid.persistence.dao.impl;
+
+import java.util.List;
+
+import jakarta.persistence.Query;
+
+import org.orcid.persistence.dao.ProfileRecoveryPhoneDao;
+import org.orcid.persistence.jpa.entities.ProfileRecoveryPhoneEntity;
+import org.springframework.transaction.annotation.Transactional;
+
+public class ProfileRecoveryPhoneDaoImpl extends GenericDaoImpl implements ProfileRecoveryPhoneDao {
+
+ public ProfileRecoveryPhoneDaoImpl() {
+ super(ProfileRecoveryPhoneEntity.class);
+ }
+
+ /**
+ * Reads go to the read-only pool, as every DAO read does since PD-13463. Inside
+ * {@link #upsert} this method is called on {@code this}, not through the proxy, so
+ * that lookup stays in the write transaction: an existence check that decides
+ * between insert and update must not be answered by a replica.
+ */
+ @Override
+ @SuppressWarnings("unchecked")
+ @Transactional(value = "transactionManagerReadOnly", readOnly = true)
+ public ProfileRecoveryPhoneEntity findByOrcid(String orcid) {
+ Query query = entityManager.createQuery("FROM ProfileRecoveryPhoneEntity WHERE orcid = :orcid");
+ query.setParameter("orcid", orcid);
+ List results = query.getResultList();
+ return results.isEmpty() ? null : results.get(0);
+ }
+
+ @Override
+ @Transactional
+ public UpsertResult upsert(String orcid, String encryptedPhoneNumber, String lastFour) {
+ // The lookup is a call on this, not on the proxy, so it runs inside this write
+ // transaction on the primary: the existence check that decides insert or update
+ // has to see the row this same connection may have written a moment ago
+ ProfileRecoveryPhoneEntity existing = findByOrcid(orcid);
+ if (existing == null) {
+ ProfileRecoveryPhoneEntity entity = new ProfileRecoveryPhoneEntity();
+ entity.setOrcid(orcid);
+ entity.setEncryptedPhoneNumber(encryptedPhoneNumber);
+ entity.setLastFour(lastFour);
+ this.persist(entity);
+ // dateCreated and lastModified were stamped by the BaseEntity lifecycle callback
+ return new UpsertResult(entity, true);
+ }
+ existing.setEncryptedPhoneNumber(encryptedPhoneNumber);
+ existing.setLastFour(lastFour);
+ this.merge(existing);
+ // The update callback that stamps lastModified runs at flush, and the transaction
+ // this joined commits after the caller has already built its answer from the row
+ this.flush();
+ return new UpsertResult(existing, false);
+ }
+
+ @Override
+ @Transactional
+ public boolean deleteByOrcid(String orcid) {
+ Query query = entityManager.createQuery("DELETE FROM ProfileRecoveryPhoneEntity WHERE orcid = :orcid");
+ query.setParameter("orcid", orcid);
+ return query.executeUpdate() > 0;
+ }
+
+}
diff --git a/orcid-persistence/src/main/java/org/orcid/persistence/jpa/entities/ProfileEventType.java b/orcid-persistence/src/main/java/org/orcid/persistence/jpa/entities/ProfileEventType.java
index 0251efbca88..d2d99d6d5ed 100644
--- a/orcid-persistence/src/main/java/org/orcid/persistence/jpa/entities/ProfileEventType.java
+++ b/orcid-persistence/src/main/java/org/orcid/persistence/jpa/entities/ProfileEventType.java
@@ -36,7 +36,10 @@ public enum ProfileEventType {
EMAIL_VIS_2019_SENT, EMAIL_VIS_2019_SKIPPED, EMAIL_VIS_2019_FAILED,
// 2FA enable/disable events
- PROFILE_2FA_ENABLED, PROFILE_2FA_DISABLED, PROFILE_2FA_DISABLED_BY_ADMIN,
+ PROFILE_2FA_ENABLED, PROFILE_2FA_DISABLED, PROFILE_2FA_DISABLED_BY_ADMIN, PROFILE_2FA_DISABLED_BY_RECOVERY_PHONE,
+
+ // 2FA recovery phone number events
+ PROFILE_RECOVERY_PHONE_ADDED, PROFILE_RECOVERY_PHONE_UPDATED, PROFILE_RECOVERY_PHONE_REMOVED,
//Send email to encourage users to add works to their record
ADD_WORKS_FIRST_REMINDER_SENT,
diff --git a/orcid-persistence/src/main/java/org/orcid/persistence/jpa/entities/ProfileRecoveryPhoneEntity.java b/orcid-persistence/src/main/java/org/orcid/persistence/jpa/entities/ProfileRecoveryPhoneEntity.java
new file mode 100644
index 00000000000..9104bd36ae1
--- /dev/null
+++ b/orcid-persistence/src/main/java/org/orcid/persistence/jpa/entities/ProfileRecoveryPhoneEntity.java
@@ -0,0 +1,75 @@
+package org.orcid.persistence.jpa.entities;
+
+import java.io.Serializable;
+
+import jakarta.persistence.Column;
+import jakarta.persistence.Entity;
+import jakarta.persistence.GeneratedValue;
+import jakarta.persistence.GenerationType;
+import jakarta.persistence.Id;
+import jakarta.persistence.SequenceGenerator;
+import jakarta.persistence.Table;
+
+/**
+ * The 2FA recovery phone number for a record.
+ *
+ * The number is held in E.164 form, reversibly encrypted with the same
+ * mechanism as the 2FA secret, alongside the last four digits, which are all
+ * that is ever displayed back to the user. The encryption has to be reversible
+ * because the Registry sends a text to the stored number without the user
+ * re-typing it; the encrypted column is the only place the full number exists.
+ */
+@Entity
+@Table(name = "profile_recovery_phone")
+public class ProfileRecoveryPhoneEntity extends BaseEntity implements Serializable {
+
+ private static final long serialVersionUID = 1L;
+
+ private Long id;
+
+ private String orcid;
+
+ private String encryptedPhoneNumber;
+
+ private String lastFour;
+
+ @Id
+ @Column(name = "id")
+ @GeneratedValue(strategy = GenerationType.AUTO, generator = "profile_recovery_phone_seq")
+ @SequenceGenerator(name = "profile_recovery_phone_seq", sequenceName = "profile_recovery_phone_seq", allocationSize = 1)
+ public Long getId() {
+ return id;
+ }
+
+ public void setId(Long id) {
+ this.id = id;
+ }
+
+ @Column(name = "orcid", length = 19)
+ public String getOrcid() {
+ return orcid;
+ }
+
+ public void setOrcid(String orcid) {
+ this.orcid = orcid;
+ }
+
+ @Column(name = "encrypted_phone_number", nullable = false)
+ public String getEncryptedPhoneNumber() {
+ return encryptedPhoneNumber;
+ }
+
+ public void setEncryptedPhoneNumber(String encryptedPhoneNumber) {
+ this.encryptedPhoneNumber = encryptedPhoneNumber;
+ }
+
+ @Column(name = "last_four", length = 4)
+ public String getLastFour() {
+ return lastFour;
+ }
+
+ public void setLastFour(String lastFour) {
+ this.lastFour = lastFour;
+ }
+
+}
diff --git a/orcid-persistence/src/main/resources/META-INF/persistence.xml b/orcid-persistence/src/main/resources/META-INF/persistence.xml
index 1a1cc1b847e..86953a6f711 100644
--- a/orcid-persistence/src/main/resources/META-INF/persistence.xml
+++ b/orcid-persistence/src/main/resources/META-INF/persistence.xml
@@ -87,6 +87,7 @@
org.orcid.persistence.jpa.entities.InvalidRecordDataChangeEntity
org.orcid.persistence.jpa.entities.BackupCodeEntity
+ org.orcid.persistence.jpa.entities.ProfileRecoveryPhoneEntity
org.orcid.persistence.jpa.entities.ProfileHistoryEventEntity
org.orcid.persistence.jpa.entities.RejectedGroupingSuggestionEntity
org.orcid.persistence.jpa.entities.ValidatedPublicProfileEntity
diff --git a/orcid-persistence/src/main/resources/db-master.xml b/orcid-persistence/src/main/resources/db-master.xml
index add9b5c1f45..8a2821f2ada 100644
--- a/orcid-persistence/src/main/resources/db-master.xml
+++ b/orcid-persistence/src/main/resources/db-master.xml
@@ -431,4 +431,6 @@
+
+
\ No newline at end of file
diff --git a/orcid-persistence/src/main/resources/db/updates/create_profile_recovery_phone_table.xml b/orcid-persistence/src/main/resources/db/updates/create_profile_recovery_phone_table.xml
new file mode 100644
index 00000000000..31b58ec3585
--- /dev/null
+++ b/orcid-persistence/src/main/resources/db/updates/create_profile_recovery_phone_table.xml
@@ -0,0 +1,59 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+ create unique index profile_recovery_phone_orcid_index on profile_recovery_phone(orcid);
+
+
+
+
+ SELECT 1 FROM pg_roles WHERE rolname='orcidro'
+
+ GRANT SELECT ON profile_recovery_phone to orcidro;
+
+
+
diff --git a/orcid-persistence/src/main/resources/db/updates/recovery_phone_encrypted_number.xml b/orcid-persistence/src/main/resources/db/updates/recovery_phone_encrypted_number.xml
new file mode 100644
index 00000000000..dbf85fe79f4
--- /dev/null
+++ b/orcid-persistence/src/main/resources/db/updates/recovery_phone_encrypted_number.xml
@@ -0,0 +1,62 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/orcid-persistence/src/main/resources/orcid-persistence-context.xml b/orcid-persistence/src/main/resources/orcid-persistence-context.xml
index c5d2f719e12..d6dd324cb00 100644
--- a/orcid-persistence/src/main/resources/orcid-persistence-context.xml
+++ b/orcid-persistence/src/main/resources/orcid-persistence-context.xml
@@ -258,6 +258,8 @@
+
+
diff --git a/orcid-persistence/src/test/java/org/orcid/persistence/dao/ProfileRecoveryPhoneDaoTest.java b/orcid-persistence/src/test/java/org/orcid/persistence/dao/ProfileRecoveryPhoneDaoTest.java
new file mode 100644
index 00000000000..59890e0bd4b
--- /dev/null
+++ b/orcid-persistence/src/test/java/org/orcid/persistence/dao/ProfileRecoveryPhoneDaoTest.java
@@ -0,0 +1,128 @@
+package org.orcid.persistence.dao;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+
+import java.util.Arrays;
+import java.util.Date;
+
+import jakarta.annotation.Resource;
+
+import org.junit.After;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.orcid.persistence.dao.ProfileRecoveryPhoneDao.UpsertResult;
+import org.orcid.persistence.jpa.entities.ProfileRecoveryPhoneEntity;
+import org.orcid.test.DBUnitTest;
+import org.orcid.test.OrcidJUnit4ClassRunner;
+import org.springframework.test.context.ContextConfiguration;
+import org.springframework.transaction.support.TransactionTemplate;
+
+/**
+ * Drives the DAO through its Spring proxy against the test database, so the
+ * read-only transaction manager on findByOrcid and the write transaction on
+ * upsert are the ones that run, not a mock's idea of them.
+ */
+@RunWith(OrcidJUnit4ClassRunner.class)
+@ContextConfiguration(locations = { "classpath:test-orcid-persistence-context.xml" })
+public class ProfileRecoveryPhoneDaoTest extends DBUnitTest {
+
+ private static final String ORCID = "4444-4444-4444-4441";
+
+ @Resource(name = "profileRecoveryPhoneDao")
+ private ProfileRecoveryPhoneDao dao;
+
+ @Resource(name = "transactionTemplate")
+ private TransactionTemplate transactionTemplate;
+
+ @BeforeClass
+ public static void initDBUnitData() throws Exception {
+ initDBUnitData(Arrays.asList("/data/SubjectEntityData.xml", "/data/SourceClientDetailsEntityData.xml", "/data/ProfileEntityData.xml"));
+ }
+
+ @AfterClass
+ public static void removeDBUnitData() throws Exception {
+ removeDBUnitData(Arrays.asList("/data/ProfileEntityData.xml", "/data/SubjectEntityData.xml"));
+ }
+
+ @After
+ public void removeTheRow() {
+ dao.deleteByOrcid(ORCID);
+ }
+
+ @Test
+ public void findByOrcidAnswersNullWhenNothingIsStored() {
+ assertNull(dao.findByOrcid(ORCID));
+ }
+
+ @Test
+ public void upsertCreatesTheRowAndSaysSo() {
+ UpsertResult result = dao.upsert(ORCID, "encrypted-one", "7890");
+
+ assertTrue(result.isInserted());
+ ProfileRecoveryPhoneEntity written = result.getEntity();
+ assertNotNull(written.getId());
+ assertNotNull(written.getDateCreated());
+ assertEquals(written.getDateCreated(), written.getLastModified());
+
+ ProfileRecoveryPhoneEntity read = dao.findByOrcid(ORCID);
+ assertEquals("7890", read.getLastFour());
+ assertEquals("encrypted-one", read.getEncryptedPhoneNumber());
+ assertEquals(written.getId(), read.getId());
+ }
+
+ @Test
+ public void upsertReplacesTheRowAndSaysSo() {
+ UpsertResult first = dao.upsert(ORCID, "encrypted-one", "7890");
+ UpsertResult second = dao.upsert(ORCID, "encrypted-two", "4321");
+
+ assertFalse(second.isInserted());
+ assertEquals(first.getEntity().getId(), second.getEntity().getId());
+ assertEquals(first.getEntity().getDateCreated(), second.getEntity().getDateCreated());
+
+ ProfileRecoveryPhoneEntity read = dao.findByOrcid(ORCID);
+ assertEquals("4321", read.getLastFour());
+ assertEquals("encrypted-two", read.getEncryptedPhoneNumber());
+ }
+
+ /*
+ * The manager builds its answer from the returned row while its own
+ * transaction is still open. The entity callback that stamps lastModified
+ * only runs at flush, so upsert has to flush before returning or the row
+ * it hands back still carries the previous date. Asserting inside an outer
+ * transaction is what makes that visible: without the flush the commit
+ * would stamp the date later and a plain call could never tell.
+ */
+ @Test
+ public void upsertAnswersWithTheFlushedRowInsideAnOuterTransaction() throws Exception {
+ Date before = dao.upsert(ORCID, "encrypted-one", "7890").getEntity().getLastModified();
+ // The stamp has millisecond resolution; a replacement inside the same
+ // millisecond would be indistinguishable from no stamp at all
+ Thread.sleep(5);
+
+ // Read while the outer transaction is still open: execute() commits on the
+ // way out, and a commit stamps the date too, which would hide a missing flush
+ Date stampedBeforeCommit = transactionTemplate.execute(status -> {
+ UpsertResult second = dao.upsert(ORCID, "encrypted-two", "4321");
+ assertFalse(second.isInserted());
+ return second.getEntity().getLastModified();
+ });
+
+ assertTrue("lastModified must already be stamped when upsert returns", stampedBeforeCommit.after(before));
+ assertEquals(stampedBeforeCommit, dao.findByOrcid(ORCID).getLastModified());
+ }
+
+ @Test
+ public void deleteByOrcidReportsWhetherARowWent() {
+ assertFalse(dao.deleteByOrcid(ORCID));
+ dao.upsert(ORCID, "encrypted-one", "7890");
+ assertTrue(dao.deleteByOrcid(ORCID));
+ assertFalse(dao.deleteByOrcid(ORCID));
+ assertNull(dao.findByOrcid(ORCID));
+ }
+}
diff --git a/orcid-utils/src/main/java/org/orcid/utils/phone/PhoneNumberValidationResult.java b/orcid-utils/src/main/java/org/orcid/utils/phone/PhoneNumberValidationResult.java
index 46a9e1ab5df..2e1b45cd850 100644
--- a/orcid-utils/src/main/java/org/orcid/utils/phone/PhoneNumberValidationResult.java
+++ b/orcid-utils/src/main/java/org/orcid/utils/phone/PhoneNumberValidationResult.java
@@ -2,22 +2,35 @@
public class PhoneNumberValidationResult {
+ /** Generic reason, used when nothing more specific can be determined. */
+ public static final String INVALID = "INVALID_PHONE_NUMBER";
+
+ public static final String TOO_SHORT = "PHONE_TOO_SHORT";
+
+ public static final String TOO_LONG = "PHONE_TOO_LONG";
+
private final boolean valid;
private final String e164Number;
+ private final String errorCode;
private final String errorMessage;
- private PhoneNumberValidationResult(boolean valid, String e164Number, String errorMessage) {
+ private PhoneNumberValidationResult(boolean valid, String e164Number, String errorCode, String errorMessage) {
this.valid = valid;
this.e164Number = e164Number;
+ this.errorCode = errorCode;
this.errorMessage = errorMessage;
}
public static PhoneNumberValidationResult valid(String e164Number) {
- return new PhoneNumberValidationResult(true, e164Number, null);
+ return new PhoneNumberValidationResult(true, e164Number, null, null);
}
public static PhoneNumberValidationResult invalid(String errorMessage) {
- return new PhoneNumberValidationResult(false, null, errorMessage);
+ return new PhoneNumberValidationResult(false, null, INVALID, errorMessage);
+ }
+
+ public static PhoneNumberValidationResult invalid(String errorCode, String errorMessage) {
+ return new PhoneNumberValidationResult(false, null, errorCode, errorMessage);
}
public boolean isValid() {
@@ -28,6 +41,10 @@ public String getE164Number() {
return e164Number;
}
+ public String getErrorCode() {
+ return errorCode;
+ }
+
public String getErrorMessage() {
return errorMessage;
}
diff --git a/orcid-utils/src/main/java/org/orcid/utils/phone/PhoneNumberValidator.java b/orcid-utils/src/main/java/org/orcid/utils/phone/PhoneNumberValidator.java
index 76ff4728492..cdd6d1c4d66 100644
--- a/orcid-utils/src/main/java/org/orcid/utils/phone/PhoneNumberValidator.java
+++ b/orcid-utils/src/main/java/org/orcid/utils/phone/PhoneNumberValidator.java
@@ -22,11 +22,30 @@ public PhoneNumberValidationResult validate(String rawPhoneNumber, String defaul
try {
PhoneNumber phoneNumber = phoneNumberUtil.parse(rawPhoneNumber, StringUtils.defaultIfBlank(defaultRegion, DEFAULT_REGION));
if (!phoneNumberUtil.isValidNumber(phoneNumber)) {
- return PhoneNumberValidationResult.invalid("Phone number is invalid");
+ // Length problems get their own reason so the UI can tell the user what to fix
+ switch (phoneNumberUtil.isPossibleNumberWithReason(phoneNumber)) {
+ case TOO_SHORT:
+ case INVALID_LENGTH:
+ // Only long enough to dial locally, which is short of what we need
+ case IS_POSSIBLE_LOCAL_ONLY:
+ return PhoneNumberValidationResult.invalid(PhoneNumberValidationResult.TOO_SHORT, "Phone number is too short");
+ case TOO_LONG:
+ return PhoneNumberValidationResult.invalid(PhoneNumberValidationResult.TOO_LONG, "Phone number is too long");
+ default:
+ return PhoneNumberValidationResult.invalid("Phone number is invalid");
+ }
}
return PhoneNumberValidationResult.valid(phoneNumberUtil.format(phoneNumber, PhoneNumberUtil.PhoneNumberFormat.E164));
} catch (NumberParseException e) {
- return PhoneNumberValidationResult.invalid(e.getMessage());
+ switch (e.getErrorType()) {
+ case TOO_LONG:
+ return PhoneNumberValidationResult.invalid(PhoneNumberValidationResult.TOO_LONG, "Phone number is too long");
+ case TOO_SHORT_AFTER_IDD:
+ case TOO_SHORT_NSN:
+ return PhoneNumberValidationResult.invalid(PhoneNumberValidationResult.TOO_SHORT, "Phone number is too short");
+ default:
+ return PhoneNumberValidationResult.invalid(e.getMessage());
+ }
}
}
}
diff --git a/orcid-utils/src/main/java/org/orcid/utils/sms/LogVerificationCodeSender.java b/orcid-utils/src/main/java/org/orcid/utils/sms/LogVerificationCodeSender.java
new file mode 100644
index 00000000000..329840e7e00
--- /dev/null
+++ b/orcid-utils/src/main/java/org/orcid/utils/sms/LogVerificationCodeSender.java
@@ -0,0 +1,104 @@
+package org.orcid.utils.sms;
+
+import java.util.concurrent.atomic.AtomicLong;
+
+import org.apache.commons.lang3.StringUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.beans.factory.annotation.Value;
+import org.springframework.context.annotation.Condition;
+import org.springframework.context.annotation.ConditionContext;
+import org.springframework.context.annotation.Conditional;
+import org.springframework.core.type.AnnotatedTypeMetadata;
+import org.springframework.stereotype.Component;
+
+/**
+ * Writes the verification code to the application log instead of texting it.
+ *
+ * This sender is for local and development use only, and it is gated so that it cannot exist anywhere else: the bean
+ * is registered only where the environment selects {@code org.orcid.sms.provider=log}, and {@link #sendCode} refuses
+ * unless the configuration it reads says the same. The gate is what keeps it local: senders are looked up by name,
+ * and a caller can ask for a name, so a bean that merely expects not to be chosen is not safe enough.
+ *
+ * It sends no message to anyone. It exists so that an automated end to end run can read a code it could never
+ * receive by text.
+ *
+ * Even here the recipient is masked: the full number must never appear in a log line, not even from the local sender.
+ */
+@Component
+@Conditional(LogVerificationCodeSender.LogProviderConfiguredCondition.class)
+public class LogVerificationCodeSender implements VerificationCodeSender {
+
+ public static final String PROVIDER = "log";
+
+ static final String PROVIDER_PROPERTY = "org.orcid.sms.provider";
+
+ private static final Logger LOG = LoggerFactory.getLogger(LogVerificationCodeSender.class);
+
+ /** Fixed length mask, so the mask does not leak how long the number is. The same mask the Registry serves over HTTP. */
+ private static final String MASK = "***********";
+
+ private static final int VISIBLE_CHARACTERS = 4;
+
+ /** A counter, not a random value: the id only has to be non-null and distinct within the run. */
+ private static final AtomicLong MESSAGE_ID_SEQUENCE = new AtomicLong();
+
+ @Value("${org.orcid.sms.provider:aws}")
+ private String configuredProvider;
+
+ @Override
+ public String getProvider() {
+ return PROVIDER;
+ }
+
+ @Override
+ public SmsSendResult sendCode(String toE164Number, String code, String locale) {
+ // Belt and braces behind the bean gate, so that a wiring mistake fails closed: a sender that puts live
+ // verification codes in the log refuses outright wherever the log provider was not the configured one, and
+ // says so without writing the code first.
+ if (!isLogProviderSelected(configuredProvider)) {
+ return SmsSendResult.failure(PROVIDER, "SMS_PROVIDER_NOT_CONFIGURED",
+ "The log sender only sends when org.orcid.sms.provider=log");
+ }
+ LOG.info("RECOVERY_PHONE_CODE to={} code={} locale={}", mask(toE164Number), code, locale);
+ return SmsSendResult.success(PROVIDER, PROVIDER + "-" + MESSAGE_ID_SEQUENCE.incrementAndGet(), "logged");
+ }
+
+ /**
+ * Keeps only the last four characters of the number, behind the fixed length mask. A null, blank or short value
+ * is masked whole rather than printed, so nothing that is too short to spare four characters can be read back out
+ * of the log.
+ */
+ static String mask(String toE164Number) {
+ if (StringUtils.length(toE164Number) <= VISIBLE_CHARACTERS) {
+ return MASK;
+ }
+ return MASK + toE164Number.substring(toE164Number.length() - VISIBLE_CHARACTERS);
+ }
+
+ private static boolean isLogProviderSelected(String provider) {
+ return PROVIDER.equalsIgnoreCase(StringUtils.trim(provider));
+ }
+
+ void setConfiguredProvider(String configuredProvider) {
+ this.configuredProvider = configuredProvider;
+ }
+
+ /**
+ * Keeps the bean out of every context that has not asked for it. The classpath scanner evaluates
+ * {@code @Conditional} itself, so it applies to the plain {@code } this application is
+ * wired with; there is no Spring Boot here, so {@code @ConditionalOnProperty} is not available.
+ *
+ * The scan runs long before the property placeholder merges the ORCID configuration file, so the Spring
+ * Environment is the only source a condition can read: select this sender with
+ * {@code -Dorg.orcid.sms.provider=log} on the JVM. A system property also wins over the properties file when the
+ * {@code @Value} above is resolved, so one setting satisfies both gates and neither opens on its own.
+ */
+ static class LogProviderConfiguredCondition implements Condition {
+
+ @Override
+ public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata) {
+ return isLogProviderSelected(context.getEnvironment().getProperty(PROVIDER_PROPERTY));
+ }
+ }
+}
diff --git a/orcid-utils/src/test/java/org/orcid/utils/sms/LogVerificationCodeSenderTest.java b/orcid-utils/src/test/java/org/orcid/utils/sms/LogVerificationCodeSenderTest.java
new file mode 100644
index 00000000000..0f59fb3c134
--- /dev/null
+++ b/orcid-utils/src/test/java/org/orcid/utils/sms/LogVerificationCodeSenderTest.java
@@ -0,0 +1,167 @@
+package org.orcid.utils.sms;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.orcid.utils.sms.LogVerificationCodeSender.LogProviderConfiguredCondition;
+import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
+import org.springframework.beans.factory.support.BeanDefinitionRegistry;
+import org.springframework.context.annotation.ConditionContext;
+import org.springframework.core.env.Environment;
+import org.springframework.core.io.ResourceLoader;
+import org.springframework.mock.env.MockEnvironment;
+
+public class LogVerificationCodeSenderTest {
+
+ private static final String PHONE_NUMBER = "+50688887777";
+
+ private LogVerificationCodeSender sender = new LogVerificationCodeSender();
+
+ @Before
+ public void selectTheLogProvider() {
+ sender.setConfiguredProvider(LogVerificationCodeSender.PROVIDER);
+ }
+
+ @Test
+ public void providerIsLog() {
+ assertEquals("log", LogVerificationCodeSender.PROVIDER);
+ assertEquals(LogVerificationCodeSender.PROVIDER, sender.getProvider());
+ }
+
+ @Test
+ public void theBeanIsOnlyRegisteredWhereTheEnvironmentSelectsTheLogProvider() {
+ LogProviderConfiguredCondition condition = new LogProviderConfiguredCondition();
+
+ assertTrue(condition.matches(contextConfiguredWith("log"), null));
+ assertFalse(condition.matches(contextConfiguredWith("aws"), null));
+ assertFalse(condition.matches(contextConfiguredWith(null), null));
+ }
+
+ @Test
+ public void sendCodeReportsSuccessForTheLogProvider() {
+ SmsSendResult result = sender.sendCode(PHONE_NUMBER, "123456", "en");
+
+ assertTrue(result.isSuccess());
+ assertEquals(LogVerificationCodeSender.PROVIDER, result.getProvider());
+ assertTrue(result.getProviderMessageId() != null && result.getProviderMessageId().startsWith("log-"));
+ }
+
+ /**
+ * What this test can and cannot prove.
+ *
+ * It cannot assert that the line reaches a log, because on this module's unit test classpath it does not reach
+ * one at all: slf4j-api is 1.7.24 while the only binding present is log4j-slf4j2-impl, which implements the
+ * slf4j 2.x service interface. A 1.7 api looks for the old StaticLoggerBinder, finds nothing, and falls back to
+ * a no-op logger, so every LOG call in this module is silently discarded under test. Attaching a log4j2 appender
+ * captures nothing for the same reason.
+ *
+ * So the masking rule (R1.2) is proved here against the helper that produces the value, and that the line really
+ * is written, in the format the end to end suite reads, is proved against a running Registry instead.
+ */
+ @Test
+ public void sendCodeRefusesWhenAnotherProviderIsConfigured() {
+ sender.setConfiguredProvider("aws");
+
+ SmsSendResult result = sender.sendCode(PHONE_NUMBER, "123456", "en");
+
+ assertFalse(result.isSuccess());
+ assertEquals(LogVerificationCodeSender.PROVIDER, result.getProvider());
+ assertEquals("SMS_PROVIDER_NOT_CONFIGURED", result.getErrorCode());
+ assertEquals(null, result.getProviderMessageId());
+ // Deliberately no assertion about the log here: see the note above -- an appender on this classpath
+ // captures nothing whatever the sender does, so "nothing was logged" would pass for the wrong reason.
+ }
+
+ @Test
+ public void sendCodeGivesEachMessageItsOwnId() {
+ String firstId = sender.sendCode(PHONE_NUMBER, "123456", "en").getProviderMessageId();
+ String secondId = sender.sendCode(PHONE_NUMBER, "654321", "en").getProviderMessageId();
+
+ assertNotNull(firstId);
+ assertNotNull(secondId);
+ assertFalse(firstId.equals(secondId));
+ }
+
+ @Test
+ public void maskKeepsOnlyTheLastFourCharacters() {
+ String masked = LogVerificationCodeSender.mask(PHONE_NUMBER);
+
+ assertEquals("***********7777", masked);
+ assertFalse(masked.contains(PHONE_NUMBER));
+ assertFalse(masked.contains("5068"));
+ }
+
+ @Test
+ public void maskIsFixedLengthSoItDoesNotLeakHowLongTheNumberIs() {
+ assertEquals(15, LogVerificationCodeSender.mask("+15550001111").length());
+ assertEquals(15, LogVerificationCodeSender.mask("+5068888777766").length());
+ }
+
+ @Test
+ public void maskHidesANumberTooShortToSpareFourCharacters() {
+ assertEquals("***********", LogVerificationCodeSender.mask("123"));
+ assertEquals("***********", LogVerificationCodeSender.mask("1234"));
+ assertEquals("***********", LogVerificationCodeSender.mask(""));
+ assertEquals("***********", LogVerificationCodeSender.mask(null));
+ }
+
+ @Test
+ public void sendCodeDoesNotThrowForANullOrShortNumber() {
+ SmsSendResult nullNumber = sender.sendCode(null, "123456", "en");
+ SmsSendResult shortNumber = sender.sendCode("123", "123456", null);
+
+ assertTrue(nullNumber.isSuccess());
+ assertNotNull(nullNumber.getProviderMessageId());
+ assertTrue(shortNumber.isSuccess());
+ assertNotNull(shortNumber.getProviderMessageId());
+ }
+
+ private static ConditionContext contextConfiguredWith(String provider) {
+ MockEnvironment environment = new MockEnvironment();
+ if (provider != null) {
+ environment.setProperty(LogVerificationCodeSender.PROVIDER_PROPERTY, provider);
+ }
+ return new StubConditionContext(environment);
+ }
+
+ /** The condition reads nothing but the environment, so the rest of the context is not part of what is tested. */
+ private static class StubConditionContext implements ConditionContext {
+
+ private final Environment environment;
+
+ StubConditionContext(Environment environment) {
+ this.environment = environment;
+ }
+
+ @Override
+ public BeanDefinitionRegistry getRegistry() {
+ return null;
+ }
+
+ @Override
+ public ConfigurableListableBeanFactory getBeanFactory() {
+ return null;
+ }
+
+ @Override
+ public Environment getEnvironment() {
+ return environment;
+ }
+
+ @Override
+ public ResourceLoader getResourceLoader() {
+ return null;
+ }
+
+ @Override
+ public ClassLoader getClassLoader() {
+ return null;
+ }
+ }
+
+ /** Collects what the sender writes, so a test can assert on what did, and what did not, reach the log. */
+}
diff --git a/orcid-web/src/main/java/org/orcid/frontend/recoveryphone/RecoveryPhoneChallengeSendCodeResponse.java b/orcid-web/src/main/java/org/orcid/frontend/recoveryphone/RecoveryPhoneChallengeSendCodeResponse.java
new file mode 100644
index 00000000000..4119319f452
--- /dev/null
+++ b/orcid-web/src/main/java/org/orcid/frontend/recoveryphone/RecoveryPhoneChallengeSendCodeResponse.java
@@ -0,0 +1,77 @@
+package org.orcid.frontend.recoveryphone;
+
+/**
+ * The outcome of asking for a code during an authentication challenge, where
+ * the user never types the number: it is the one stored on the record.
+ *
+ * It carries the masked number as well as the outcome, so the panel can tell
+ * the user where the code went. The full number is never returned.
+ */
+public class RecoveryPhoneChallengeSendCodeResponse {
+
+ private boolean success;
+
+ private String errorCode;
+
+ /**
+ * Seconds the user has to wait before another code can be sent. Drives the
+ * countdown in the UI so the client and the throttle cannot disagree.
+ */
+ private int resendAfterSeconds;
+
+ private String maskedRecoveryPhoneNumber;
+
+ public static RecoveryPhoneChallengeSendCodeResponse failure(String errorCode) {
+ RecoveryPhoneChallengeSendCodeResponse response = new RecoveryPhoneChallengeSendCodeResponse();
+ response.setSuccess(false);
+ response.setErrorCode(errorCode);
+ return response;
+ }
+
+ /**
+ * Restates the outcome of the underlying send, adding the masked number.
+ * A failed send keeps its resend countdown, since a refused resend is the
+ * one failure the countdown belongs to.
+ */
+ public static RecoveryPhoneChallengeSendCodeResponse from(RecoveryPhoneSendCodeResponse sendCodeResponse, String maskedRecoveryPhoneNumber) {
+ RecoveryPhoneChallengeSendCodeResponse response = new RecoveryPhoneChallengeSendCodeResponse();
+ response.setSuccess(sendCodeResponse.isSuccess());
+ response.setErrorCode(sendCodeResponse.getErrorCode());
+ response.setResendAfterSeconds(sendCodeResponse.getResendAfterSeconds());
+ response.setMaskedRecoveryPhoneNumber(maskedRecoveryPhoneNumber);
+ return response;
+ }
+
+ public boolean isSuccess() {
+ return success;
+ }
+
+ public void setSuccess(boolean success) {
+ this.success = success;
+ }
+
+ public String getErrorCode() {
+ return errorCode;
+ }
+
+ public void setErrorCode(String errorCode) {
+ this.errorCode = errorCode;
+ }
+
+ public int getResendAfterSeconds() {
+ return resendAfterSeconds;
+ }
+
+ public void setResendAfterSeconds(int resendAfterSeconds) {
+ this.resendAfterSeconds = resendAfterSeconds;
+ }
+
+ public String getMaskedRecoveryPhoneNumber() {
+ return maskedRecoveryPhoneNumber;
+ }
+
+ public void setMaskedRecoveryPhoneNumber(String maskedRecoveryPhoneNumber) {
+ this.maskedRecoveryPhoneNumber = maskedRecoveryPhoneNumber;
+ }
+
+}
diff --git a/orcid-web/src/main/java/org/orcid/frontend/recoveryphone/RecoveryPhoneChallengeVerifyRequest.java b/orcid-web/src/main/java/org/orcid/frontend/recoveryphone/RecoveryPhoneChallengeVerifyRequest.java
new file mode 100644
index 00000000000..d24a752f9d9
--- /dev/null
+++ b/orcid-web/src/main/java/org/orcid/frontend/recoveryphone/RecoveryPhoneChallengeVerifyRequest.java
@@ -0,0 +1,33 @@
+package org.orcid.frontend.recoveryphone;
+
+/**
+ * What the user sends to pass an authentication challenge with their recovery
+ * phone number: their password, and the code we texted to the stored number.
+ *
+ * The number itself is never part of this request. The Registry sent the code
+ * to the number it holds, and checks the code against that same number, so a
+ * client cannot point the challenge at a number of its own.
+ */
+public class RecoveryPhoneChallengeVerifyRequest {
+
+ private String password;
+
+ private String verificationCode;
+
+ public String getPassword() {
+ return password;
+ }
+
+ public void setPassword(String password) {
+ this.password = password;
+ }
+
+ public String getVerificationCode() {
+ return verificationCode;
+ }
+
+ public void setVerificationCode(String verificationCode) {
+ this.verificationCode = verificationCode;
+ }
+
+}
diff --git a/orcid-web/src/main/java/org/orcid/frontend/recoveryphone/RecoveryPhoneCodeEntry.java b/orcid-web/src/main/java/org/orcid/frontend/recoveryphone/RecoveryPhoneCodeEntry.java
new file mode 100644
index 00000000000..5a9392dd2aa
--- /dev/null
+++ b/orcid-web/src/main/java/org/orcid/frontend/recoveryphone/RecoveryPhoneCodeEntry.java
@@ -0,0 +1,118 @@
+package org.orcid.frontend.recoveryphone;
+
+import org.apache.commons.lang3.StringUtils;
+import org.codehaus.jettison.json.JSONException;
+import org.codehaus.jettison.json.JSONObject;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * The single pending recovery phone verification code per record.
+ *
+ * Sending a code overwrites the entry, so a code sent to a previous number
+ * stops working as soon as the user asks for a new one. The number the code was
+ * sent to is kept alongside it so the save step can refuse a code that was
+ * issued for a different number.
+ */
+public class RecoveryPhoneCodeEntry {
+
+ private static final Logger LOG = LoggerFactory.getLogger(RecoveryPhoneCodeEntry.class);
+
+ private static final String KEY_PREFIX = "recovery-phone-code-";
+
+ private static final String CODE_FIELD = "code";
+ private static final String PHONE_FIELD = "phone";
+ private static final String PROVIDER_FIELD = "provider";
+ private static final String MESSAGE_ID_FIELD = "messageId";
+ private static final String ATTEMPTS_FIELD = "attempts";
+ private static final String SENT_AT_FIELD = "sentAt";
+
+ private final String code;
+
+ private final String phoneE164;
+
+ private final String provider;
+
+ private final String providerMessageId;
+
+ private int attempts;
+
+ private final long sentAt;
+
+ public RecoveryPhoneCodeEntry(String code, String phoneE164, String provider, String providerMessageId, int attempts, long sentAt) {
+ this.code = code;
+ this.phoneE164 = phoneE164;
+ this.provider = provider;
+ this.providerMessageId = providerMessageId;
+ this.attempts = attempts;
+ this.sentAt = sentAt;
+ }
+
+ public static String redisKey(String orcid) {
+ return KEY_PREFIX + orcid;
+ }
+
+ public String getCode() {
+ return code;
+ }
+
+ public String getPhoneE164() {
+ return phoneE164;
+ }
+
+ public String getProvider() {
+ return provider;
+ }
+
+ public String getProviderMessageId() {
+ return providerMessageId;
+ }
+
+ public int getAttempts() {
+ return attempts;
+ }
+
+ public int incrementAttempts() {
+ return ++attempts;
+ }
+
+ public long getSentAt() {
+ return sentAt;
+ }
+
+ public String serialize() {
+ try {
+ JSONObject json = new JSONObject();
+ json.put(CODE_FIELD, code);
+ json.put(PHONE_FIELD, phoneE164);
+ json.put(PROVIDER_FIELD, provider);
+ json.put(MESSAGE_ID_FIELD, providerMessageId == null ? JSONObject.NULL : providerMessageId);
+ json.put(ATTEMPTS_FIELD, attempts);
+ json.put(SENT_AT_FIELD, sentAt);
+ return json.toString();
+ } catch (JSONException e) {
+ throw new IllegalStateException("Unable to serialize recovery phone code entry", e);
+ }
+ }
+
+ /**
+ * @param value
+ * the raw stored value, or null when there is no pending code
+ * @return the parsed entry, or null when there is nothing usable stored
+ */
+ public static RecoveryPhoneCodeEntry parse(String value) {
+ if (StringUtils.isBlank(value)) {
+ return null;
+ }
+ try {
+ JSONObject json = new JSONObject(value);
+ return new RecoveryPhoneCodeEntry(json.getString(CODE_FIELD), json.getString(PHONE_FIELD), json.getString(PROVIDER_FIELD),
+ json.isNull(MESSAGE_ID_FIELD) ? null : json.getString(MESSAGE_ID_FIELD), json.getInt(ATTEMPTS_FIELD),
+ json.getLong(SENT_AT_FIELD));
+ } catch (JSONException e) {
+ LOG.error("Unable to parse the recovery phone code entry, treating it as missing", e);
+ return null;
+ }
+ }
+
+}
diff --git a/orcid-web/src/main/java/org/orcid/frontend/recoveryphone/RecoveryPhoneCodeStore.java b/orcid-web/src/main/java/org/orcid/frontend/recoveryphone/RecoveryPhoneCodeStore.java
new file mode 100644
index 00000000000..5e0ce7795e6
--- /dev/null
+++ b/orcid-web/src/main/java/org/orcid/frontend/recoveryphone/RecoveryPhoneCodeStore.java
@@ -0,0 +1,112 @@
+package org.orcid.frontend.recoveryphone;
+
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+import jakarta.annotation.Resource;
+
+import org.orcid.core.utils.cache.redis.RedisClient;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.beans.factory.annotation.Value;
+import org.springframework.stereotype.Component;
+
+/**
+ * Holds the pending recovery phone verification code for a record, keyed by
+ * ORCID iD so one record can never consume or interfere with another's code.
+ *
+ * Redis is the real store: the registry runs on several nodes and the request
+ * that verifies a code is not necessarily the one that sent it. When Redis is
+ * unavailable {@link #save} reports failure so the caller can refuse to send a
+ * code nobody would be able to confirm.
+ *
+ * The in-memory map is a local development convenience only. It is off unless
+ * {@code org.orcid.sms.code.allowInMemoryStore} is set, and it is never correct
+ * on a multi-node deployment.
+ */
+@Component
+public class RecoveryPhoneCodeStore {
+
+ private static final Logger LOG = LoggerFactory.getLogger(RecoveryPhoneCodeStore.class);
+
+ @Resource
+ private RedisClient redisClient;
+
+ @Value("${org.orcid.sms.code.allowInMemoryStore:false}")
+ private boolean allowInMemoryStore;
+
+ private final Map inMemoryEntries = new ConcurrentHashMap<>();
+
+ /**
+ * @return true when the code was stored and can be confirmed later
+ */
+ public boolean save(String orcid, RecoveryPhoneCodeEntry entry, int ttlSeconds) {
+ String key = RecoveryPhoneCodeEntry.redisKey(orcid);
+ if (redisClient.set(key, entry.serialize(), ttlSeconds)) {
+ return true;
+ }
+ if (allowInMemoryStore) {
+ LOG.warn("Redis unavailable, storing the recovery phone code in memory. This is only valid for local development.");
+ inMemoryEntries.put(key, new InMemoryEntry(entry.serialize(), System.currentTimeMillis() + (ttlSeconds * 1000L)));
+ return true;
+ }
+ LOG.error("Unable to store the recovery phone verification code, Redis is unavailable");
+ return false;
+ }
+
+ public RecoveryPhoneCodeEntry get(String orcid) {
+ String key = RecoveryPhoneCodeEntry.redisKey(orcid);
+ RecoveryPhoneCodeEntry entry = RecoveryPhoneCodeEntry.parse(redisClient.get(key));
+ if (entry != null) {
+ return entry;
+ }
+ if (allowInMemoryStore) {
+ InMemoryEntry inMemory = inMemoryEntries.get(key);
+ if (inMemory != null) {
+ if (inMemory.isExpired()) {
+ inMemoryEntries.remove(key);
+ return null;
+ }
+ return RecoveryPhoneCodeEntry.parse(inMemory.getValue());
+ }
+ }
+ return null;
+ }
+
+ public void remove(String orcid) {
+ String key = RecoveryPhoneCodeEntry.redisKey(orcid);
+ redisClient.remove(key);
+ if (allowInMemoryStore) {
+ inMemoryEntries.remove(key);
+ }
+ }
+
+ void setRedisClient(RedisClient redisClient) {
+ this.redisClient = redisClient;
+ }
+
+ void setAllowInMemoryStore(boolean allowInMemoryStore) {
+ this.allowInMemoryStore = allowInMemoryStore;
+ }
+
+ private static class InMemoryEntry {
+
+ private final String value;
+
+ private final long expiresAt;
+
+ InMemoryEntry(String value, long expiresAt) {
+ this.value = value;
+ this.expiresAt = expiresAt;
+ }
+
+ String getValue() {
+ return value;
+ }
+
+ boolean isExpired() {
+ return System.currentTimeMillis() > expiresAt;
+ }
+ }
+
+}
diff --git a/orcid-web/src/main/java/org/orcid/frontend/recoveryphone/RecoveryPhoneSaveRequest.java b/orcid-web/src/main/java/org/orcid/frontend/recoveryphone/RecoveryPhoneSaveRequest.java
new file mode 100644
index 00000000000..22495e930dd
--- /dev/null
+++ b/orcid-web/src/main/java/org/orcid/frontend/recoveryphone/RecoveryPhoneSaveRequest.java
@@ -0,0 +1,40 @@
+package org.orcid.frontend.recoveryphone;
+
+public class RecoveryPhoneSaveRequest {
+
+ private String phoneNumber;
+
+ private String verificationCode;
+
+ /**
+ * Where the form is being shown: SETTINGS, ONBOARDING or INTERSTITIAL. It
+ * decides which proof of identity the request is accepted on, never what
+ * the request is allowed to do.
+ */
+ private String context;
+
+ public String getPhoneNumber() {
+ return phoneNumber;
+ }
+
+ public void setPhoneNumber(String phoneNumber) {
+ this.phoneNumber = phoneNumber;
+ }
+
+ public String getVerificationCode() {
+ return verificationCode;
+ }
+
+ public void setVerificationCode(String verificationCode) {
+ this.verificationCode = verificationCode;
+ }
+
+ public String getContext() {
+ return context;
+ }
+
+ public void setContext(String context) {
+ this.context = context;
+ }
+
+}
diff --git a/orcid-web/src/main/java/org/orcid/frontend/recoveryphone/RecoveryPhoneSaveResponse.java b/orcid-web/src/main/java/org/orcid/frontend/recoveryphone/RecoveryPhoneSaveResponse.java
new file mode 100644
index 00000000000..13c813ce3d3
--- /dev/null
+++ b/orcid-web/src/main/java/org/orcid/frontend/recoveryphone/RecoveryPhoneSaveResponse.java
@@ -0,0 +1,78 @@
+package org.orcid.frontend.recoveryphone;
+
+import org.orcid.pojo.ajaxForm.Date;
+
+/**
+ * The outcome of saving a recovery phone number, carrying the refreshed panel
+ * state so the settings page does not have to re-read the status straight away.
+ */
+public class RecoveryPhoneSaveResponse {
+
+ private boolean success;
+
+ private String errorCode;
+
+ private String maskedRecoveryPhoneNumber;
+
+ private Date recoveryPhoneCreationDate;
+
+ private Date recoveryPhoneLastModifiedDate;
+
+ private boolean recoveryPhoneModified;
+
+ public static RecoveryPhoneSaveResponse failure(String errorCode) {
+ RecoveryPhoneSaveResponse response = new RecoveryPhoneSaveResponse();
+ response.setSuccess(false);
+ response.setErrorCode(errorCode);
+ return response;
+ }
+
+ public boolean isSuccess() {
+ return success;
+ }
+
+ public void setSuccess(boolean success) {
+ this.success = success;
+ }
+
+ public String getErrorCode() {
+ return errorCode;
+ }
+
+ public void setErrorCode(String errorCode) {
+ this.errorCode = errorCode;
+ }
+
+ public String getMaskedRecoveryPhoneNumber() {
+ return maskedRecoveryPhoneNumber;
+ }
+
+ public void setMaskedRecoveryPhoneNumber(String maskedRecoveryPhoneNumber) {
+ this.maskedRecoveryPhoneNumber = maskedRecoveryPhoneNumber;
+ }
+
+ public Date getRecoveryPhoneCreationDate() {
+ return recoveryPhoneCreationDate;
+ }
+
+ public void setRecoveryPhoneCreationDate(Date recoveryPhoneCreationDate) {
+ this.recoveryPhoneCreationDate = recoveryPhoneCreationDate;
+ }
+
+ public Date getRecoveryPhoneLastModifiedDate() {
+ return recoveryPhoneLastModifiedDate;
+ }
+
+ public void setRecoveryPhoneLastModifiedDate(Date recoveryPhoneLastModifiedDate) {
+ this.recoveryPhoneLastModifiedDate = recoveryPhoneLastModifiedDate;
+ }
+
+ public boolean isRecoveryPhoneModified() {
+ return recoveryPhoneModified;
+ }
+
+ public void setRecoveryPhoneModified(boolean recoveryPhoneModified) {
+ this.recoveryPhoneModified = recoveryPhoneModified;
+ }
+
+}
diff --git a/orcid-web/src/main/java/org/orcid/frontend/recoveryphone/RecoveryPhoneSendCodeRequest.java b/orcid-web/src/main/java/org/orcid/frontend/recoveryphone/RecoveryPhoneSendCodeRequest.java
new file mode 100644
index 00000000000..1116255df44
--- /dev/null
+++ b/orcid-web/src/main/java/org/orcid/frontend/recoveryphone/RecoveryPhoneSendCodeRequest.java
@@ -0,0 +1,40 @@
+package org.orcid.frontend.recoveryphone;
+
+public class RecoveryPhoneSendCodeRequest {
+
+ private String phoneNumber;
+
+ private String locale;
+
+ /**
+ * Where the form is being shown: SETTINGS, ONBOARDING or INTERSTITIAL. It
+ * decides which proof of identity the request is accepted on, never what
+ * the request is allowed to do.
+ */
+ private String context;
+
+ public String getPhoneNumber() {
+ return phoneNumber;
+ }
+
+ public void setPhoneNumber(String phoneNumber) {
+ this.phoneNumber = phoneNumber;
+ }
+
+ public String getLocale() {
+ return locale;
+ }
+
+ public void setLocale(String locale) {
+ this.locale = locale;
+ }
+
+ public String getContext() {
+ return context;
+ }
+
+ public void setContext(String context) {
+ this.context = context;
+ }
+
+}
diff --git a/orcid-web/src/main/java/org/orcid/frontend/recoveryphone/RecoveryPhoneSendCodeResponse.java b/orcid-web/src/main/java/org/orcid/frontend/recoveryphone/RecoveryPhoneSendCodeResponse.java
new file mode 100644
index 00000000000..b234bdbc1fb
--- /dev/null
+++ b/orcid-web/src/main/java/org/orcid/frontend/recoveryphone/RecoveryPhoneSendCodeResponse.java
@@ -0,0 +1,63 @@
+package org.orcid.frontend.recoveryphone;
+
+/**
+ * The outcome of asking for a verification code. The code itself and the full
+ * phone number are never returned.
+ */
+public class RecoveryPhoneSendCodeResponse {
+
+ private boolean success;
+
+ private String errorCode;
+
+ /**
+ * Seconds the user has to wait before another code can be sent. Drives the
+ * countdown in the UI so the client and the throttle cannot disagree.
+ */
+ private int resendAfterSeconds;
+
+ public static RecoveryPhoneSendCodeResponse success(int resendAfterSeconds) {
+ RecoveryPhoneSendCodeResponse response = new RecoveryPhoneSendCodeResponse();
+ response.setSuccess(true);
+ response.setResendAfterSeconds(resendAfterSeconds);
+ return response;
+ }
+
+ public static RecoveryPhoneSendCodeResponse failure(String errorCode) {
+ RecoveryPhoneSendCodeResponse response = new RecoveryPhoneSendCodeResponse();
+ response.setSuccess(false);
+ response.setErrorCode(errorCode);
+ return response;
+ }
+
+ public static RecoveryPhoneSendCodeResponse failure(String errorCode, int resendAfterSeconds) {
+ RecoveryPhoneSendCodeResponse response = failure(errorCode);
+ response.setResendAfterSeconds(resendAfterSeconds);
+ return response;
+ }
+
+ public boolean isSuccess() {
+ return success;
+ }
+
+ public void setSuccess(boolean success) {
+ this.success = success;
+ }
+
+ public String getErrorCode() {
+ return errorCode;
+ }
+
+ public void setErrorCode(String errorCode) {
+ this.errorCode = errorCode;
+ }
+
+ public int getResendAfterSeconds() {
+ return resendAfterSeconds;
+ }
+
+ public void setResendAfterSeconds(int resendAfterSeconds) {
+ this.resendAfterSeconds = resendAfterSeconds;
+ }
+
+}
diff --git a/orcid-web/src/main/java/org/orcid/frontend/recoveryphone/RecoveryPhoneSigninSendCodeRequest.java b/orcid-web/src/main/java/org/orcid/frontend/recoveryphone/RecoveryPhoneSigninSendCodeRequest.java
new file mode 100644
index 00000000000..1512ddf9ae8
--- /dev/null
+++ b/orcid-web/src/main/java/org/orcid/frontend/recoveryphone/RecoveryPhoneSigninSendCodeRequest.java
@@ -0,0 +1,32 @@
+package org.orcid.frontend.recoveryphone;
+
+/**
+ * Asks for a code to be sent to the recovery number stored on the account the
+ * credentials belong to.
+ *
+ * The caller cannot choose the number: it is read from the record, so a user
+ * who has lost their authenticator cannot redirect the text somewhere else.
+ */
+public class RecoveryPhoneSigninSendCodeRequest {
+
+ private String username;
+
+ private String password;
+
+ public String getUsername() {
+ return username;
+ }
+
+ public void setUsername(String username) {
+ this.username = username;
+ }
+
+ public String getPassword() {
+ return password;
+ }
+
+ public void setPassword(String password) {
+ this.password = password;
+ }
+
+}
diff --git a/orcid-web/src/main/java/org/orcid/frontend/recoveryphone/RecoveryPhoneSigninSendCodeResponse.java b/orcid-web/src/main/java/org/orcid/frontend/recoveryphone/RecoveryPhoneSigninSendCodeResponse.java
new file mode 100644
index 00000000000..e54ae506af6
--- /dev/null
+++ b/orcid-web/src/main/java/org/orcid/frontend/recoveryphone/RecoveryPhoneSigninSendCodeResponse.java
@@ -0,0 +1,63 @@
+package org.orcid.frontend.recoveryphone;
+
+/**
+ * The outcome of asking, from the sign in screen, for a code to be sent to the
+ * recovery number on the account.
+ *
+ * The caller is anonymous, so this carries nothing it did not already supply
+ * beyond the masked number: never the code, never the number in full.
+ */
+public class RecoveryPhoneSigninSendCodeResponse {
+
+ private boolean success;
+
+ private String errorCode;
+
+ /**
+ * Seconds the user has to wait before another code can be sent. Drives the
+ * countdown in the UI so the client and the throttle cannot disagree.
+ */
+ private int resendAfterSeconds;
+
+ private String maskedRecoveryPhoneNumber;
+
+ public static RecoveryPhoneSigninSendCodeResponse failure(String errorCode) {
+ RecoveryPhoneSigninSendCodeResponse response = new RecoveryPhoneSigninSendCodeResponse();
+ response.setSuccess(false);
+ response.setErrorCode(errorCode);
+ return response;
+ }
+
+ public boolean isSuccess() {
+ return success;
+ }
+
+ public void setSuccess(boolean success) {
+ this.success = success;
+ }
+
+ public String getErrorCode() {
+ return errorCode;
+ }
+
+ public void setErrorCode(String errorCode) {
+ this.errorCode = errorCode;
+ }
+
+ public int getResendAfterSeconds() {
+ return resendAfterSeconds;
+ }
+
+ public void setResendAfterSeconds(int resendAfterSeconds) {
+ this.resendAfterSeconds = resendAfterSeconds;
+ }
+
+ public String getMaskedRecoveryPhoneNumber() {
+ return maskedRecoveryPhoneNumber;
+ }
+
+ public void setMaskedRecoveryPhoneNumber(String maskedRecoveryPhoneNumber) {
+ this.maskedRecoveryPhoneNumber = maskedRecoveryPhoneNumber;
+ }
+
+}
diff --git a/orcid-web/src/main/java/org/orcid/frontend/recoveryphone/RecoveryPhoneSigninVerifyRequest.java b/orcid-web/src/main/java/org/orcid/frontend/recoveryphone/RecoveryPhoneSigninVerifyRequest.java
new file mode 100644
index 00000000000..a91b803b715
--- /dev/null
+++ b/orcid-web/src/main/java/org/orcid/frontend/recoveryphone/RecoveryPhoneSigninVerifyRequest.java
@@ -0,0 +1,39 @@
+package org.orcid.frontend.recoveryphone;
+
+/**
+ * Confirms the code that was texted to the recovery number, which disables 2FA
+ * on the account so the ordinary sign in can be replayed without a code.
+ */
+public class RecoveryPhoneSigninVerifyRequest {
+
+ private String username;
+
+ private String password;
+
+ private String verificationCode;
+
+ public String getUsername() {
+ return username;
+ }
+
+ public void setUsername(String username) {
+ this.username = username;
+ }
+
+ public String getPassword() {
+ return password;
+ }
+
+ public void setPassword(String password) {
+ this.password = password;
+ }
+
+ public String getVerificationCode() {
+ return verificationCode;
+ }
+
+ public void setVerificationCode(String verificationCode) {
+ this.verificationCode = verificationCode;
+ }
+
+}
diff --git a/orcid-web/src/main/java/org/orcid/frontend/recoveryphone/RecoveryPhoneSigninVerifyResponse.java b/orcid-web/src/main/java/org/orcid/frontend/recoveryphone/RecoveryPhoneSigninVerifyResponse.java
new file mode 100644
index 00000000000..c69761b5b95
--- /dev/null
+++ b/orcid-web/src/main/java/org/orcid/frontend/recoveryphone/RecoveryPhoneSigninVerifyResponse.java
@@ -0,0 +1,55 @@
+package org.orcid.frontend.recoveryphone;
+
+/**
+ * The outcome of confirming a recovery number code from the sign in screen.
+ *
+ * On success 2FA is off, and the ORCID iD is returned so the client can replay
+ * the ordinary sign in against the record it has just recovered.
+ */
+public class RecoveryPhoneSigninVerifyResponse {
+
+ private boolean success;
+
+ private String errorCode;
+
+ private String orcid;
+
+ public static RecoveryPhoneSigninVerifyResponse failure(String errorCode) {
+ RecoveryPhoneSigninVerifyResponse response = new RecoveryPhoneSigninVerifyResponse();
+ response.setSuccess(false);
+ response.setErrorCode(errorCode);
+ return response;
+ }
+
+ public static RecoveryPhoneSigninVerifyResponse success(String orcid) {
+ RecoveryPhoneSigninVerifyResponse response = new RecoveryPhoneSigninVerifyResponse();
+ response.setSuccess(true);
+ response.setOrcid(orcid);
+ return response;
+ }
+
+ public boolean isSuccess() {
+ return success;
+ }
+
+ public void setSuccess(boolean success) {
+ this.success = success;
+ }
+
+ public String getErrorCode() {
+ return errorCode;
+ }
+
+ public void setErrorCode(String errorCode) {
+ this.errorCode = errorCode;
+ }
+
+ public String getOrcid() {
+ return orcid;
+ }
+
+ public void setOrcid(String orcid) {
+ this.orcid = orcid;
+ }
+
+}
diff --git a/orcid-web/src/main/java/org/orcid/frontend/recoveryphone/RecoveryPhoneVerificationService.java b/orcid-web/src/main/java/org/orcid/frontend/recoveryphone/RecoveryPhoneVerificationService.java
new file mode 100644
index 00000000000..29332118e7f
--- /dev/null
+++ b/orcid-web/src/main/java/org/orcid/frontend/recoveryphone/RecoveryPhoneVerificationService.java
@@ -0,0 +1,277 @@
+package org.orcid.frontend.recoveryphone;
+
+import java.security.SecureRandom;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.commons.lang3.StringUtils;
+import org.orcid.utils.phone.PhoneNumberValidationResult;
+import org.orcid.utils.phone.PhoneNumberValidator;
+import org.orcid.utils.sms.SmsSendResult;
+import org.orcid.utils.sms.VerificationCodeSender;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.beans.factory.annotation.Value;
+import org.springframework.stereotype.Component;
+
+/**
+ * Sends and confirms the one time codes used to prove a user controls the phone
+ * number they are setting as their 2FA recovery number.
+ *
+ * ORCID generates and checks the code; the provider only delivers it. Codes are
+ * held against the ORCID iD rather than the phone number, so a pending code
+ * belongs to one record and asking for a new one always retires the old one.
+ */
+@Component
+public class RecoveryPhoneVerificationService {
+
+ private static final Logger LOG = LoggerFactory.getLogger(RecoveryPhoneVerificationService.class);
+
+ private static final SecureRandom RANDOM = new SecureRandom();
+
+ public static final String INVALID_CODE = "INVALID_CODE";
+ public static final String CODE_EXPIRED = "CODE_EXPIRED";
+ public static final String TOO_MANY_ATTEMPTS = "TOO_MANY_ATTEMPTS";
+ public static final String PHONE_MISMATCH = "PHONE_MISMATCH";
+ public static final String RESEND_TOO_SOON = "RESEND_TOO_SOON";
+ public static final String SMS_SEND_FAILED = "SMS_SEND_FAILED";
+ public static final String SMS_RECIPIENT_NOT_ALLOWED = "SMS_RECIPIENT_NOT_ALLOWED";
+ public static final String SMS_PROVIDER_NOT_CONFIGURED = "SMS_PROVIDER_NOT_CONFIGURED";
+ public static final String CODE_STORAGE_UNAVAILABLE = "CODE_STORAGE_UNAVAILABLE";
+
+ @Autowired
+ private PhoneNumberValidator phoneNumberValidator;
+
+ @Autowired
+ private RecoveryPhoneCodeStore recoveryPhoneCodeStore;
+
+ private Map sendersByProvider = new HashMap();
+
+ @Value("${org.orcid.sms.provider:aws}")
+ private String provider;
+
+ @Value("${org.orcid.sms.defaultRegion:US}")
+ private String defaultRegion;
+
+ @Value("${org.orcid.sms.regexFilter:}")
+ private String regexFilter;
+
+ @Value("${org.orcid.sms.code.length:6}")
+ private int codeLength;
+
+ @Value("${org.orcid.sms.code.ttlSeconds:300}")
+ private int codeTtlSeconds;
+
+ @Value("${org.orcid.sms.code.maxAttempts:5}")
+ private int maxAttempts;
+
+ @Value("${org.orcid.sms.code.resendBufferSeconds:30}")
+ private int resendBufferSeconds;
+
+ @Autowired
+ public void setSenders(List senders) {
+ sendersByProvider.clear();
+ if (senders != null) {
+ for (VerificationCodeSender sender : senders) {
+ sendersByProvider.put(StringUtils.lowerCase(sender.getProvider()), sender);
+ }
+ }
+ }
+
+ /**
+ * Validates the number, sends a fresh code to it and stores that code
+ * against the record. Any previously issued code stops working.
+ */
+ public RecoveryPhoneSendCodeResponse sendCode(String orcid, RecoveryPhoneSendCodeRequest request) {
+ PhoneNumberValidationResult validationResult = phoneNumberValidator.validate(request == null ? null : request.getPhoneNumber(),
+ defaultRegion);
+ if (!validationResult.isValid()) {
+ return RecoveryPhoneSendCodeResponse.failure(validationResult.getErrorCode());
+ }
+
+ String phoneE164 = validationResult.getE164Number();
+ if (StringUtils.isNotBlank(regexFilter) && !phoneE164.matches(regexFilter)) {
+ return RecoveryPhoneSendCodeResponse.failure(SMS_RECIPIENT_NOT_ALLOWED);
+ }
+
+ // One pending code per record: refuse a resend until the buffer has passed
+ RecoveryPhoneCodeEntry pending = recoveryPhoneCodeStore.get(orcid);
+ if (pending != null) {
+ int remaining = remainingResendSeconds(pending);
+ if (remaining > 0) {
+ return RecoveryPhoneSendCodeResponse.failure(RESEND_TOO_SOON, remaining);
+ }
+ }
+
+ String selectedProvider = StringUtils.lowerCase(StringUtils.defaultIfBlank(provider, "aws"));
+ VerificationCodeSender sender = sendersByProvider.get(selectedProvider);
+ if (sender == null) {
+ return RecoveryPhoneSendCodeResponse.failure(SMS_PROVIDER_NOT_CONFIGURED);
+ }
+
+ String code = generateCode();
+ SmsSendResult result = sender.sendCode(phoneE164, code, sanitizeLocale(request.getLocale()));
+ if (!result.isSuccess()) {
+ LOG.warn("Unable to send a recovery phone verification code for {}: {}", orcid, result.getErrorCode());
+ return RecoveryPhoneSendCodeResponse.failure(SMS_SEND_FAILED);
+ }
+
+ RecoveryPhoneCodeEntry entry = new RecoveryPhoneCodeEntry(code, phoneE164, result.getProvider(), result.getProviderMessageId(), 0,
+ System.currentTimeMillis());
+ if (!recoveryPhoneCodeStore.save(orcid, entry, codeTtlSeconds)) {
+ // Better to fail loudly than to leave the user with a code we cannot confirm
+ return RecoveryPhoneSendCodeResponse.failure(CODE_STORAGE_UNAVAILABLE);
+ }
+ return RecoveryPhoneSendCodeResponse.success(resendBufferSeconds);
+ }
+
+ /**
+ * Confirms the code the user typed against the pending code for the record,
+ * and checks it was issued for the number they are trying to save.
+ *
+ * @return null when the code is good, otherwise the error code to report
+ */
+ public String verifyCode(String orcid, String rawPhoneNumber, String code) {
+ if (StringUtils.isBlank(code)) {
+ return INVALID_CODE;
+ }
+
+ PhoneNumberValidationResult validationResult = phoneNumberValidator.validate(rawPhoneNumber, defaultRegion);
+ if (!validationResult.isValid()) {
+ return validationResult.getErrorCode();
+ }
+
+ RecoveryPhoneCodeEntry entry = recoveryPhoneCodeStore.get(orcid);
+ if (entry == null) {
+ return CODE_EXPIRED;
+ }
+
+ // Redis drops the entry on its own, but never accept a stale code even if
+ // the store still has it
+ int remainingTtl = remainingTtlSeconds(entry);
+ if (remainingTtl <= 0) {
+ recoveryPhoneCodeStore.remove(orcid);
+ return CODE_EXPIRED;
+ }
+
+ // The code was sent to a different number, so it cannot authorise this one
+ if (!StringUtils.equals(entry.getPhoneE164(), validationResult.getE164Number())) {
+ return PHONE_MISMATCH;
+ }
+
+ if (entry.incrementAttempts() > maxAttempts) {
+ recoveryPhoneCodeStore.remove(orcid);
+ return TOO_MANY_ATTEMPTS;
+ }
+
+ if (!constantTimeEquals(entry.getCode(), code.trim())) {
+ // Persist the incremented attempt count, keeping the original expiry window
+ recoveryPhoneCodeStore.save(orcid, entry, remainingTtl);
+ return INVALID_CODE;
+ }
+
+ recoveryPhoneCodeStore.remove(orcid);
+ VerificationCodeSender sender = sendersByProvider.get(entry.getProvider());
+ if (sender != null) {
+ // Best effort provider feedback; a failure here does not undo a code ORCID already confirmed
+ sender.reportResult(entry.getPhoneE164(), entry.getCode(), entry.getProviderMessageId(), true);
+ }
+ return null;
+ }
+
+ /**
+ * The E.164 form of a number the caller has already validated, for storing
+ * against the record.
+ */
+ public String normalize(String rawPhoneNumber) {
+ PhoneNumberValidationResult validationResult = phoneNumberValidator.validate(rawPhoneNumber, defaultRegion);
+ return validationResult.isValid() ? validationResult.getE164Number() : null;
+ }
+
+ public void discardPendingCode(String orcid) {
+ recoveryPhoneCodeStore.remove(orcid);
+ }
+
+ private int remainingResendSeconds(RecoveryPhoneCodeEntry entry) {
+ long elapsed = (System.currentTimeMillis() - entry.getSentAt()) / 1000L;
+ long remaining = resendBufferSeconds - elapsed;
+ return remaining > 0 ? (int) remaining : 0;
+ }
+
+ private int remainingTtlSeconds(RecoveryPhoneCodeEntry entry) {
+ long elapsed = (System.currentTimeMillis() - entry.getSentAt()) / 1000L;
+ long remaining = codeTtlSeconds - elapsed;
+ return remaining > 0 ? (int) remaining : 0;
+ }
+
+ /**
+ * Accepts only a plausible BCP 47 tag from the caller; anything else is
+ * dropped so senders fall back to English.
+ */
+ private static String sanitizeLocale(String locale) {
+ if (StringUtils.isBlank(locale)) {
+ return null;
+ }
+ String trimmed = locale.trim();
+ return trimmed.matches("[A-Za-z]{2,8}([_-][A-Za-z0-9]{1,8}){0,3}") ? trimmed : null;
+ }
+
+ private String generateCode() {
+ int length = codeLength > 0 ? codeLength : 6;
+ StringBuilder builder = new StringBuilder(length);
+ for (int i = 0; i < length; i++) {
+ builder.append(RANDOM.nextInt(10));
+ }
+ return builder.toString();
+ }
+
+ private static boolean constantTimeEquals(String expected, String actual) {
+ if (expected == null || actual == null || expected.length() != actual.length()) {
+ return false;
+ }
+ int result = 0;
+ for (int i = 0; i < expected.length(); i++) {
+ result |= expected.charAt(i) ^ actual.charAt(i);
+ }
+ return result == 0;
+ }
+
+ void setPhoneNumberValidator(PhoneNumberValidator phoneNumberValidator) {
+ this.phoneNumberValidator = phoneNumberValidator;
+ }
+
+ void setRecoveryPhoneCodeStore(RecoveryPhoneCodeStore recoveryPhoneCodeStore) {
+ this.recoveryPhoneCodeStore = recoveryPhoneCodeStore;
+ }
+
+ void setProvider(String provider) {
+ this.provider = provider;
+ }
+
+ void setDefaultRegion(String defaultRegion) {
+ this.defaultRegion = defaultRegion;
+ }
+
+ void setRegexFilter(String regexFilter) {
+ this.regexFilter = regexFilter;
+ }
+
+ void setCodeLength(int codeLength) {
+ this.codeLength = codeLength;
+ }
+
+ void setCodeTtlSeconds(int codeTtlSeconds) {
+ this.codeTtlSeconds = codeTtlSeconds;
+ }
+
+ void setMaxAttempts(int maxAttempts) {
+ this.maxAttempts = maxAttempts;
+ }
+
+ void setResendBufferSeconds(int resendBufferSeconds) {
+ this.resendBufferSeconds = resendBufferSeconds;
+ }
+
+}
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
new file mode 100644
index 00000000000..0bdda042513
--- /dev/null
+++ b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/RecoveryPhoneSigninController.java
@@ -0,0 +1,276 @@
+package org.orcid.frontend.web.controllers;
+
+import jakarta.annotation.Resource;
+import jakarta.persistence.NoResultException;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpSession;
+
+import org.apache.commons.lang3.StringUtils;
+import org.orcid.authorization.authentication.MFAWebAuthenticationDetails;
+import org.orcid.core.manager.ProfileEntityCacheManager;
+import org.orcid.core.manager.RecoveryPhone;
+import org.orcid.core.manager.RecoveryPhoneManager;
+import org.orcid.core.manager.TwoFactorAuthenticationManager;
+import org.orcid.core.togglz.Features;
+import org.orcid.frontend.email.RecordEmailSender;
+import org.orcid.frontend.recoveryphone.RecoveryPhoneSendCodeRequest;
+import org.orcid.frontend.recoveryphone.RecoveryPhoneSendCodeResponse;
+import org.orcid.frontend.recoveryphone.RecoveryPhoneSigninSendCodeRequest;
+import org.orcid.frontend.recoveryphone.RecoveryPhoneSigninSendCodeResponse;
+import org.orcid.frontend.recoveryphone.RecoveryPhoneSigninVerifyRequest;
+import org.orcid.frontend.recoveryphone.RecoveryPhoneSigninVerifyResponse;
+import org.orcid.frontend.recoveryphone.RecoveryPhoneVerificationService;
+import org.orcid.frontend.web.exception.VerificationCodeFor2FARequiredException;
+import org.orcid.utils.OrcidStringUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.security.authentication.AuthenticationProvider;
+import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
+import org.springframework.security.core.Authentication;
+import org.springframework.security.core.AuthenticationException;
+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;
+
+import java.util.Locale;
+
+/**
+ * Lets a user who still knows their password, but has lost both their
+ * authentication app and their 2FA recovery codes, receive a text on the
+ * recovery number stored on their record and get back in.
+ *
+ * These are the only recovery phone endpoints reachable without a session, so
+ * everything they do is driven by the credentials in the body: they are used
+ * from the 2FA step of the ordinary sign in screen and, unchanged, from the
+ * OAuth sign in screen (R4.1). Confirming a code turns 2FA off, after which the
+ * browser replays the ordinary sign in with no code at all.
+ */
+@Controller
+@RequestMapping(value = { "/signin/recoveryPhone" })
+public class RecoveryPhoneSigninController extends BaseController {
+
+ private static final Logger LOG = LoggerFactory.getLogger(RecoveryPhoneSigninController.class);
+
+ /**
+ * Fixed length mask, so the mask does not leak how long the number is. The
+ * 2FA controller masks the same way; the constant is repeated rather than
+ * shared so neither controller has to widen its own internals.
+ */
+ private static final String RECOVERY_PHONE_MASK = "***********";
+
+ static final String FEATURE_DISABLED = "FEATURE_DISABLED";
+
+ static final String BAD_CREDENTIALS = "BAD_CREDENTIALS";
+
+ /** Wire value kept in step with the signed in recovery phone endpoints. */
+ static final String TWO_FACTOR_DISABLED = "2FA_DISABLED";
+
+ static final String NO_RECOVERY_PHONE = "NO_RECOVERY_PHONE";
+
+ /**
+ * The provider bean, deliberately not the "authenticationManager"
+ * ProviderManager: Spring Security wires that manager with an event
+ * publisher, so a return that is not an exception publishes an
+ * authentication success event. On an account whose 2FA is off that would
+ * put a sign in that never happened in the audit trail, and run every
+ * listener on that event, for a caller who only posted a password to a
+ * recovery endpoint. The provider does the identical lockout accounting
+ * R3.2 depends on and publishes nothing, so do not "simplify" this back to
+ * the manager.
+ */
+ @Resource(name = "authenticationProvider")
+ private AuthenticationProvider authenticationProvider;
+
+ @Resource
+ private RecoveryPhoneManager recoveryPhoneManager;
+
+ @Resource
+ private RecoveryPhoneVerificationService recoveryPhoneVerificationService;
+
+ @Resource
+ private TwoFactorAuthenticationManager twoFactorAuthenticationManager;
+
+ @Resource
+ private ProfileEntityCacheManager profileEntityCacheManager;
+
+ @Resource
+ private RecordEmailSender recordEmailSender;
+
+ /**
+ * Sends a fresh code to the recovery number stored on the account the given
+ * credentials belong to (R3.2).
+ */
+ @RequestMapping(value = "/sendCode.json", method = RequestMethod.POST)
+ public @ResponseBody RecoveryPhoneSigninSendCodeResponse sendCode(HttpServletRequest request,
+ @RequestBody RecoveryPhoneSigninSendCodeRequest form) {
+ // Checked before anything else so the endpoint is inert, rather than a
+ // credential check, while the feature is off
+ if (!Features.TWO_FACTOR_RECOVERY_PHONE.isActive()) {
+ return RecoveryPhoneSigninSendCodeResponse.failure(FEATURE_DISABLED);
+ }
+
+ String credentialsFailure = verifyCredentials(request, form.getUsername(), form.getPassword());
+ if (credentialsFailure != null) {
+ return RecoveryPhoneSigninSendCodeResponse.failure(credentialsFailure);
+ }
+
+ String orcid = resolveOrcid(form.getUsername());
+ if (orcid == null) {
+ return RecoveryPhoneSigninSendCodeResponse.failure(BAD_CREDENTIALS);
+ }
+
+ RecoveryPhone recoveryPhone = recoveryPhoneManager.getRecoveryPhone(orcid);
+ if (recoveryPhone == null) {
+ // Say so rather than leaving the user waiting for a text (R3.4)
+ return RecoveryPhoneSigninSendCodeResponse.failure(NO_RECOVERY_PHONE);
+ }
+
+ // The number comes from the record, never from the request, so nobody
+ // can point the text at a phone of their own, and this is the only
+ // point in the flow that needs it in the clear
+ String phoneNumber = recoveryPhoneManager.getDecryptedPhoneNumber(orcid);
+ if (phoneNumber == null) {
+ // A stored recovery phone always carries a number, so this is only
+ // reachable if the number went away between the two reads
+ return RecoveryPhoneSigninSendCodeResponse.failure(NO_RECOVERY_PHONE);
+ }
+
+ RecoveryPhoneSendCodeRequest sendCodeRequest = new RecoveryPhoneSendCodeRequest();
+ sendCodeRequest.setPhoneNumber(phoneNumber);
+ sendCodeRequest.setLocale(requestLocale());
+
+ RecoveryPhoneSendCodeResponse sendCodeResponse = recoveryPhoneVerificationService.sendCode(orcid, sendCodeRequest);
+
+ RecoveryPhoneSigninSendCodeResponse response = new RecoveryPhoneSigninSendCodeResponse();
+ response.setSuccess(sendCodeResponse.isSuccess());
+ response.setErrorCode(sendCodeResponse.getErrorCode());
+ response.setResendAfterSeconds(sendCodeResponse.getResendAfterSeconds());
+ // The mask goes back on a refused resend too, so the screen can keep
+ // naming the number the user is waiting on
+ response.setMaskedRecoveryPhoneNumber(RECOVERY_PHONE_MASK + recoveryPhone.getLastFour());
+ return response;
+ }
+
+ /**
+ * Confirms the code that was texted to the recovery number and, on success,
+ * turns 2FA off so the ordinary sign in can proceed on the password alone
+ * (R3.5).
+ */
+ @RequestMapping(value = "/verify.json", method = RequestMethod.POST)
+ public @ResponseBody RecoveryPhoneSigninVerifyResponse verify(HttpServletRequest request,
+ @RequestBody RecoveryPhoneSigninVerifyRequest form) {
+ if (!Features.TWO_FACTOR_RECOVERY_PHONE.isActive()) {
+ return RecoveryPhoneSigninVerifyResponse.failure(FEATURE_DISABLED);
+ }
+
+ String credentialsFailure = verifyCredentials(request, form.getUsername(), form.getPassword());
+ if (credentialsFailure != null) {
+ return RecoveryPhoneSigninVerifyResponse.failure(credentialsFailure);
+ }
+
+ String orcid = resolveOrcid(form.getUsername());
+ if (orcid == null) {
+ return RecoveryPhoneSigninVerifyResponse.failure(BAD_CREDENTIALS);
+ }
+
+ // Answered from the read that costs no decryption, as in sendCode
+ if (recoveryPhoneManager.getRecoveryPhone(orcid) == null) {
+ return RecoveryPhoneSigninVerifyResponse.failure(NO_RECOVERY_PHONE);
+ }
+
+ String phoneNumber = recoveryPhoneManager.getDecryptedPhoneNumber(orcid);
+ if (phoneNumber == null) {
+ return RecoveryPhoneSigninVerifyResponse.failure(NO_RECOVERY_PHONE);
+ }
+
+ String verificationFailure = recoveryPhoneVerificationService.verifyCode(orcid, phoneNumber, form.getVerificationCode());
+ if (verificationFailure != null) {
+ // Nothing about the account changes until the code is right
+ return RecoveryPhoneSigninVerifyResponse.failure(verificationFailure);
+ }
+
+ twoFactorAuthenticationManager.disable2FAByRecoveryPhone(orcid);
+ // Load bearing, and before anything that can throw: the browser
+ // re-submits the ordinary sign in straight away, and a cached profile
+ // still carrying using2FA=true would demand a 2FA code that no longer
+ // exists
+ profileEntityCacheManager.remove(orcid);
+ try {
+ recordEmailSender.send2FADisabledEmail(orcid);
+ } catch (RuntimeException e) {
+ // The notification must not be able to fail the operation it is
+ // reporting: 2FA is already off, the number and the backup codes are
+ // already gone, and telling the caller it failed would only send
+ // them back through a recovery they no longer need
+ LOG.error("Unable to send the 2FA disabled email for: " + orcid, e);
+ }
+ LOG.info("2FA disabled through the recovery phone number for: " + orcid);
+
+ return RecoveryPhoneSigninVerifyResponse.success(orcid);
+ }
+
+ /**
+ * Proves the password without establishing a session.
+ *
+ * The check deliberately goes through the real authentication provider
+ * rather than comparing the hash here: a wrong password on these endpoints
+ * then counts toward the same sign in lockout as a wrong password on the
+ * sign in form (R3.2), instead of handing out an unthrottled password
+ * oracle beside a throttled one. The security context is never touched, no
+ * success handler runs and no authentication event is published, so nothing
+ * here logs anyone in or says that anyone did.
+ *
+ * @return null when the password is right and the account is using 2FA,
+ * otherwise the error code to report
+ */
+ private String verifyCredentials(HttpServletRequest request, String username, String password) {
+ if (StringUtils.isBlank(username) || StringUtils.isBlank(password)) {
+ return BAD_CREDENTIALS;
+ }
+
+ UsernamePasswordAuthenticationToken token = new UsernamePasswordAuthenticationToken(username, password);
+ HttpSession session = request.getSession(false);
+ // No 2FA codes: the whole point is that the user has none to give
+ token.setDetails(new MFAWebAuthenticationDetails(request.getRemoteAddr(), session != null ? session.getId() : null, null, null));
+
+ try {
+ Authentication result = authenticationProvider.authenticate(token);
+ if (result != null && result.isAuthenticated()) {
+ // The password is right but there is no 2FA to recover from
+ return TWO_FACTOR_DISABLED;
+ }
+ return BAD_CREDENTIALS;
+ } catch (VerificationCodeFor2FARequiredException e) {
+ // The password was accepted and the account is using 2FA: the one
+ // state these endpoints exist to serve
+ return null;
+ } 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
+ return BAD_CREDENTIALS;
+ }
+ }
+
+ private String resolveOrcid(String username) {
+ if (OrcidStringUtils.isValidOrcid(username)) {
+ return username;
+ }
+ try {
+ return emailManagerReadOnly.findOrcidIdByEmail(username);
+ } catch (NoResultException e) {
+ // The password was just accepted for this username, so there should
+ // be a record behind it; report it as a credential failure rather
+ // than failing the request
+ LOG.warn("No record found for a username that just authenticated");
+ return null;
+ }
+ }
+
+ private String requestLocale() {
+ Locale locale = getLocale();
+ return locale == null ? null : locale.toLanguageTag();
+ }
+
+}
diff --git a/orcid-web/src/main/java/org/orcid/frontend/web/controllers/TwoFactorAuthenticationController.java b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/TwoFactorAuthenticationController.java
index 0ff5a544346..8108cfaff7a 100644
--- a/orcid-web/src/main/java/org/orcid/frontend/web/controllers/TwoFactorAuthenticationController.java
+++ b/orcid-web/src/main/java/org/orcid/frontend/web/controllers/TwoFactorAuthenticationController.java
@@ -4,11 +4,22 @@
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
+import org.apache.commons.lang3.StringUtils;
import org.orcid.core.manager.BackupCodeManager;
import org.orcid.core.manager.EncryptionManager;
import org.orcid.core.manager.ProfileEntityCacheManager;
+import org.orcid.core.manager.RecoveryPhone;
+import org.orcid.core.manager.RecoveryPhoneManager;
import org.orcid.core.manager.TwoFactorAuthenticationManager;
+import org.orcid.core.togglz.Features;
import org.orcid.frontend.email.RecordEmailSender;
+import org.orcid.frontend.recoveryphone.RecoveryPhoneChallengeSendCodeResponse;
+import org.orcid.frontend.recoveryphone.RecoveryPhoneChallengeVerifyRequest;
+import org.orcid.frontend.recoveryphone.RecoveryPhoneSaveRequest;
+import org.orcid.frontend.recoveryphone.RecoveryPhoneSaveResponse;
+import org.orcid.frontend.recoveryphone.RecoveryPhoneSendCodeRequest;
+import org.orcid.frontend.recoveryphone.RecoveryPhoneSendCodeResponse;
+import org.orcid.frontend.recoveryphone.RecoveryPhoneVerificationService;
import org.orcid.persistence.jpa.entities.ProfileEntity;
import org.orcid.pojo.*;
import org.slf4j.Logger;
@@ -25,6 +36,7 @@
import java.util.ArrayList;
import java.util.List;
+import java.util.Locale;
@Controller
@RequestMapping(value = { "/2FA" })
@@ -32,9 +44,54 @@ public class TwoFactorAuthenticationController extends BaseController {
private static final Logger LOG = LoggerFactory.getLogger(TwoFactorAuthenticationController.class);
+ private static final String RECOVERY_PHONE_ELEVATION_ATTRIBUTE = "RECOVERY_PHONE_ELEVATION_TS";
+
+ /** How long a passed authentication challenge lets the user keep working. */
+ private static final long RECOVERY_PHONE_ELEVATION_TTL_MILLIS = 15 * 60 * 1000L;
+
+ /** Fixed length mask, so the mask does not leak how long the number is. */
+ private static final String RECOVERY_PHONE_MASK = "***********";
+
+ /** Ignore the sub second gap between the insert and its last_modified. */
+ // A row's two timestamps are stamped by the same lifecycle callback when it is created, so they
+ // differ by at most a few milliseconds until the number is actually changed. A minute here would
+ // read a number corrected right after it was added as never modified, which the panel then dates
+ // from the wrong event.
+ private static final long RECOVERY_PHONE_MODIFIED_THRESHOLD_MILLIS = 1000L;
+
+ static final String FEATURE_DISABLED = "FEATURE_DISABLED";
+
+ static final String TWO_FACTOR_DISABLED = "2FA_DISABLED";
+
+ static final String CHALLENGE_REQUIRED = "CHALLENGE_REQUIRED";
+
+ /** The account has no recovery phone number to send a code to. */
+ static final String NO_RECOVERY_PHONE = "NO_RECOVERY_PHONE";
+
+ /** The password given with a challenge is not the account's password. */
+ static final String INVALID_PASSWORD = "INVALID_PASSWORD";
+
+ /**
+ * Where the recovery phone form is being shown. The context decides which
+ * proof of identity the request is accepted on; it never widens what the
+ * request may do, and every context is still ROLE_USER on the signed in
+ * record.
+ */
+ static final String CONTEXT_SETTINGS = "SETTINGS";
+
+ static final String CONTEXT_ONBOARDING = "ONBOARDING";
+
+ static final String CONTEXT_INTERSTITIAL = "INTERSTITIAL";
+
@Resource
private TwoFactorAuthenticationManager twoFactorAuthenticationManager;
+ @Resource
+ private RecoveryPhoneManager recoveryPhoneManager;
+
+ @Resource
+ private RecoveryPhoneVerificationService recoveryPhoneVerificationService;
+
@Resource
private ProfileEntityCacheManager profileEntityCacheManager;
@@ -58,10 +115,313 @@ public class TwoFactorAuthenticationController extends BaseController {
status.setTwoFactorCreationDate(org.orcid.pojo.ajaxForm.Date.valueOf(creationDate));
status.setRecoveryCodeCreationDate(org.orcid.pojo.ajaxForm.Date.valueOf(creationDate));
}
+ if (Features.TWO_FACTOR_RECOVERY_PHONE.isActive()) {
+ applyRecoveryPhoneState(orcid, status);
+ }
}
return status;
}
+ /**
+ * Verifies the user before they add or change their recovery phone number.
+ *
+ * The number has to be confirmed by text, which takes longer than a TOTP
+ * code stays valid, so a passed challenge elevates the session for a short
+ * window instead of being replayed on the final save.
+ */
+ @RequestMapping(value = "/recoveryPhone/verifyAuthChallenge.json", method = RequestMethod.POST)
+ public @ResponseBody AuthChallenge verifyRecoveryPhoneAuthChallenge(HttpServletRequest request, @RequestBody AuthChallenge form) {
+ String orcid = getCurrentUserOrcid();
+ if (!Features.TWO_FACTOR_RECOVERY_PHONE.isActive() || !twoFactorAuthenticationManager.userUsing2FA(orcid)) {
+ form.setSuccess(false);
+ return form;
+ }
+
+ ProfileEntity profile = profileEntityCacheManager.retrieve(orcid);
+ if (form.getPassword() == null || !encryptionManager.hashMatches(form.getPassword(), profile.getEncryptedPassword())) {
+ form.setInvalidPassword(true);
+ return form;
+ }
+ if (!twoFactorAuthenticationManager.validateTwoFactorAuthForm(orcid, form)) {
+ return form;
+ }
+
+ request.getSession().setAttribute(RECOVERY_PHONE_ELEVATION_ATTRIBUTE, System.currentTimeMillis());
+ form.setSuccess(true);
+ return form;
+ }
+
+ @RequestMapping(value = "/recoveryPhone/sendCode.json", method = RequestMethod.POST)
+ public @ResponseBody RecoveryPhoneSendCodeResponse sendRecoveryPhoneCode(HttpServletRequest request,
+ @RequestBody RecoveryPhoneSendCodeRequest form) {
+ String orcid = getCurrentUserOrcid();
+ String guardFailure = guardRecoveryPhoneRequest(request, orcid, form.getContext());
+ if (guardFailure != null) {
+ return RecoveryPhoneSendCodeResponse.failure(guardFailure);
+ }
+ return recoveryPhoneVerificationService.sendCode(orcid, form);
+ }
+
+ @RequestMapping(value = "/recoveryPhone/save.json", method = RequestMethod.POST)
+ public @ResponseBody RecoveryPhoneSaveResponse saveRecoveryPhone(HttpServletRequest request, @RequestBody RecoveryPhoneSaveRequest form) {
+ String orcid = getCurrentUserOrcid();
+ String guardFailure = guardRecoveryPhoneRequest(request, orcid, form.getContext());
+ if (guardFailure != null) {
+ return RecoveryPhoneSaveResponse.failure(guardFailure);
+ }
+
+ String verificationFailure = recoveryPhoneVerificationService.verifyCode(orcid, form.getPhoneNumber(), form.getVerificationCode());
+ if (verificationFailure != null) {
+ return RecoveryPhoneSaveResponse.failure(verificationFailure);
+ }
+
+ String phoneE164 = recoveryPhoneVerificationService.normalize(form.getPhoneNumber());
+ RecoveryPhone saved = recoveryPhoneManager.saveRecoveryPhone(orcid, phoneE164);
+ request.getSession().removeAttribute(RECOVERY_PHONE_ELEVATION_ATTRIBUTE);
+
+ // The answer is built from the row the save wrote, not read back: a read goes to
+ // the read-only pool, and on a deployed environment that is a replica which can
+ // still be answering with the previous number, or with none, for a moment after
+ // the primary has committed
+ RecoveryPhoneSaveResponse response = new RecoveryPhoneSaveResponse();
+ response.setSuccess(true);
+ TwoFactorAuthStatus status = new TwoFactorAuthStatus();
+ applyRecoveryPhoneState(saved, status);
+ response.setMaskedRecoveryPhoneNumber(status.getMaskedRecoveryPhoneNumber());
+ response.setRecoveryPhoneCreationDate(status.getRecoveryPhoneCreationDate());
+ response.setRecoveryPhoneLastModifiedDate(status.getRecoveryPhoneLastModifiedDate());
+ response.setRecoveryPhoneModified(status.isRecoveryPhoneModified());
+ return response;
+ }
+
+ /**
+ * Sends a code to the number already stored on the record, as the first
+ * half of passing an authentication challenge with the recovery phone.
+ *
+ * This endpoint deliberately does not ask for the session elevation the
+ * other recovery phone endpoints need: it is the challenge. Anyone who
+ * could pass the ordinary challenge would have no reason to be here, since
+ * they reach this route precisely because they have lost their
+ * authentication app and their recovery codes (R5.1). What it costs an
+ * attacker is a text sent to a number they do not hold.
+ */
+ @RequestMapping(value = "/recoveryPhone/challenge/sendCode.json", method = RequestMethod.POST)
+ public @ResponseBody RecoveryPhoneChallengeSendCodeResponse sendRecoveryPhoneChallengeCode() {
+ String orcid = getCurrentUserOrcid();
+ if (!Features.TWO_FACTOR_RECOVERY_PHONE.isActive()) {
+ return RecoveryPhoneChallengeSendCodeResponse.failure(FEATURE_DISABLED);
+ }
+ if (!twoFactorAuthenticationManager.userUsing2FA(orcid)) {
+ return RecoveryPhoneChallengeSendCodeResponse.failure(TWO_FACTOR_DISABLED);
+ }
+ RecoveryPhone recoveryPhone = recoveryPhoneManager.getRecoveryPhone(orcid);
+ if (recoveryPhone == null) {
+ // Tell the user there is no number rather than leave them waiting
+ // for a text that is never sent (R3.4)
+ return RecoveryPhoneChallengeSendCodeResponse.failure(NO_RECOVERY_PHONE);
+ }
+
+ // The user never types a number here: the code goes to the stored one,
+ // and only the mask comes back out. The number is asked for separately,
+ // since RecoveryPhone carries the mask and the dates only
+ String phoneNumber = recoveryPhoneManager.getDecryptedPhoneNumber(orcid);
+ if (phoneNumber == null) {
+ // A stored recovery phone always carries a number, so this is only
+ // reachable if the number went away between the two reads
+ return RecoveryPhoneChallengeSendCodeResponse.failure(NO_RECOVERY_PHONE);
+ }
+
+ RecoveryPhoneSendCodeRequest sendCodeRequest = new RecoveryPhoneSendCodeRequest();
+ sendCodeRequest.setPhoneNumber(phoneNumber);
+ Locale locale = getLocale();
+ if (locale != null) {
+ sendCodeRequest.setLocale(locale.toString());
+ }
+ RecoveryPhoneSendCodeResponse sendCodeResponse = recoveryPhoneVerificationService.sendCode(orcid, sendCodeRequest);
+ return RecoveryPhoneChallengeSendCodeResponse.from(sendCodeResponse, RECOVERY_PHONE_MASK + recoveryPhone.getLastFour());
+ }
+
+ /**
+ * Passes an authentication challenge with the recovery phone number, which
+ * costs the user their 2FA: the number is a one time way back in, and
+ * using it disables 2FA and resets every 2FA backup option (R5.3).
+ *
+ * It answers with {@link AuthChallenge}, as
+ * {@link #verifyRecoveryPhoneAuthChallenge} does, so the frontend's
+ * challenge component keeps one response shape across both ways of
+ * passing the same challenge.
+ */
+ @RequestMapping(value = "/recoveryPhone/challenge/verify.json", method = RequestMethod.POST)
+ public @ResponseBody AuthChallenge verifyRecoveryPhoneChallengeCode(HttpServletRequest request, @RequestBody RecoveryPhoneChallengeVerifyRequest form) {
+ String orcid = getCurrentUserOrcid();
+ AuthChallenge result = new AuthChallenge();
+ if (!Features.TWO_FACTOR_RECOVERY_PHONE.isActive()) {
+ result.setSuccess(false);
+ result.getErrors().add(FEATURE_DISABLED);
+ return result;
+ }
+ if (!twoFactorAuthenticationManager.userUsing2FA(orcid)) {
+ result.setSuccess(false);
+ result.getErrors().add(TWO_FACTOR_DISABLED);
+ return result;
+ }
+
+ // The password is checked before the code, so someone without the
+ // password can neither spend the code's attempts nor learn anything
+ // about it
+ ProfileEntity profile = profileEntityCacheManager.retrieve(orcid);
+ if (form.getPassword() == null || !encryptionManager.hashMatches(form.getPassword(), profile.getEncryptedPassword())) {
+ result.setInvalidPassword(true);
+ result.getErrors().add(INVALID_PASSWORD);
+ return result;
+ }
+
+ // Nothing here wants the mask, so the number comes from the one path
+ // that decrypts it; a record with no number stored answers null
+ String phoneE164 = recoveryPhoneManager.getDecryptedPhoneNumber(orcid);
+ if (phoneE164 == null) {
+ result.setSuccess(false);
+ result.getErrors().add(NO_RECOVERY_PHONE);
+ return result;
+ }
+
+ String verificationFailure = recoveryPhoneVerificationService.verifyCode(orcid, phoneE164, form.getVerificationCode());
+ if (verificationFailure != null) {
+ // Nothing is disabled unless the code was right
+ result.setSuccess(false);
+ result.getErrors().add(verificationFailure);
+ return result;
+ }
+
+ twoFactorAuthenticationManager.disable2FAByRecoveryPhone(orcid);
+ // The cache is evicted as soon as the record changes and before
+ // anything that can fail, so nothing downstream keeps answering that
+ // 2FA is on after it has been turned off
+ profileEntityCacheManager.remove(orcid);
+ try {
+ recordEmailSender.send2FADisabledEmail(orcid);
+ } catch (RuntimeException e) {
+ // The notification must not be able to fail the operation it is
+ // reporting: 2FA is already off, the number and the backup codes
+ // are already gone, and reporting a failed challenge would send the
+ // user back through a recovery they no longer need
+ LOG.error("Unable to send the 2FA disabled email for: " + orcid, e);
+ }
+ // 2FA is off, so the action this challenge was guarding proceeds on the
+ // password alone; an elevation granted earlier has nothing left to guard
+ request.getSession().removeAttribute(RECOVERY_PHONE_ELEVATION_ATTRIBUTE);
+ result.setSuccess(true);
+ return result;
+ }
+
+ /**
+ * @param context
+ * where the form is being shown, which decides what counts as
+ * recent proof of identity
+ * @return the error code to report, or null when the request may proceed
+ */
+ private String guardRecoveryPhoneRequest(HttpServletRequest request, String orcid, String context) {
+ if (!Features.TWO_FACTOR_RECOVERY_PHONE.isActive()) {
+ return FEATURE_DISABLED;
+ }
+ if (!twoFactorAuthenticationManager.userUsing2FA(orcid)) {
+ return TWO_FACTOR_DISABLED;
+ }
+ if (sessionIsElevated(request) || interstitialIsElevatedByRecentLogin(orcid, context)) {
+ return null;
+ }
+ return CHALLENGE_REQUIRED;
+ }
+
+ /** A challenge passed on this session within the elevation window. */
+ private boolean sessionIsElevated(HttpServletRequest request) {
+ Object elevatedAt = request.getSession().getAttribute(RECOVERY_PHONE_ELEVATION_ATTRIBUTE);
+ if (!(elevatedAt instanceof Long)) {
+ return false;
+ }
+ return System.currentTimeMillis() - (Long) elevatedAt <= RECOVERY_PHONE_ELEVATION_TTL_MILLIS;
+ }
+
+ /**
+ * The add-a-number interstitial is let in on a recent sign in rather than
+ * on a challenge of its own. The user completed 2FA seconds earlier to
+ * reach it, so a fresh login is the same proof a challenge would collect,
+ * and an interstitial has nowhere to put a password challenge: it is a
+ * dialog the user cannot dismiss, sitting between them and their record.
+ * The window is the same 15 minutes, so a session left open on the
+ * interstitial goes cold exactly as an elevated session does (R6.3).
+ *
+ * The context is posted in the request body, so it is a claim about where
+ * the form is being shown and never a mode the client may switch on: the
+ * conditions the interstitial is actually shown under are checked again
+ * here, server side. With all of them, the most a stolen session can do
+ * without the password is add a first recovery number, and even that needs
+ * the code texted to that number before anything is saved. What it does
+ * not close is a stolen session used inside the same fifteen minutes as
+ * the real user's sign in, which is the trade R6.3 makes deliberately.
+ */
+ private boolean interstitialIsElevatedByRecentLogin(String orcid, String context) {
+ if (!CONTEXT_INTERSTITIAL.equals(resolveContext(context))) {
+ return false;
+ }
+ if (!Features.LOGIN_RECOVERY_PHONE_INTERSTITIAL.isActive()) {
+ // The relaxed path dies with the interstitial that justifies it (R7.4)
+ return false;
+ }
+ if (recoveryPhoneManager.getRecoveryPhone(orcid) != null) {
+ // The interstitial is only ever offered when no number is stored,
+ // so it can add a first one and never replace one. A replacement
+ // without a challenge would let a hijacked session point the
+ // recovery number at a phone it holds, and that number then turns
+ // 2FA off at the next sign in (R6.1)
+ return false;
+ }
+ if (!orcid.equals(getRealUserOrcid())) {
+ // A delegate or an admin switched into the record is not the
+ // account owner, and their own sign in is no proof of this one (R6.1)
+ return false;
+ }
+ // last_login is read from the database, not from the profile cache: the
+ // cached entity is loaded while the user is being authenticated, before
+ // the success handler writes last_login, so it still carries the
+ // previous sign in. The database is also where the authorization server
+ // puts it when it is the one serving the sign in
+ java.util.Date lastLogin = profileEntityManager.getLastLogin(orcid);
+ if (lastLogin == null) {
+ // Nothing to date the sign in by, so this is not proof of anything
+ return false;
+ }
+ return System.currentTimeMillis() - lastLogin.getTime() <= RECOVERY_PHONE_ELEVATION_TTL_MILLIS;
+ }
+
+ /** SETTINGS is what an unstated context means: the strictest of the three. */
+ private static String resolveContext(String context) {
+ return StringUtils.isBlank(context) ? CONTEXT_SETTINGS : context.trim();
+ }
+
+ private void applyRecoveryPhoneState(String orcid, TwoFactorAuthStatus status) {
+ applyRecoveryPhoneState(recoveryPhoneManager.getRecoveryPhone(orcid), status);
+ }
+
+ private void applyRecoveryPhoneState(RecoveryPhone recoveryPhone, TwoFactorAuthStatus status) {
+ if (recoveryPhone == null) {
+ return;
+ }
+ status.setMaskedRecoveryPhoneNumber(RECOVERY_PHONE_MASK + recoveryPhone.getLastFour());
+ if (recoveryPhone.getDateCreated() != null) {
+ status.setRecoveryPhoneCreationDate(org.orcid.pojo.ajaxForm.Date.valueOf(recoveryPhone.getDateCreated()));
+ }
+ if (recoveryPhone.getLastModified() != null) {
+ status.setRecoveryPhoneLastModifiedDate(org.orcid.pojo.ajaxForm.Date.valueOf(recoveryPhone.getLastModified()));
+ }
+ // The dates we hand out only carry a day, so whether the number has ever
+ // been changed is decided here from the full timestamps
+ if (recoveryPhone.getDateCreated() != null && recoveryPhone.getLastModified() != null) {
+ long deltaMillis = recoveryPhone.getLastModified().getTime() - recoveryPhone.getDateCreated().getTime();
+ status.setRecoveryPhoneModified(deltaMillis > RECOVERY_PHONE_MODIFIED_THRESHOLD_MILLIS);
+ }
+ }
+
@RequestMapping("/setup")
public ModelAndView get2FASetupPage() {
TwoFactorAuthStatus status = get2FAStatus();
@@ -111,15 +471,27 @@ public byte[] generateQrCode(HttpServletResponse response) {
}
@RequestMapping(value = "/register.json", method = RequestMethod.POST)
- public @ResponseBody TwoFactorAuthRegistration validateVerificationCode(@RequestBody TwoFactorAuthRegistration registration) {
+ public @ResponseBody TwoFactorAuthRegistration validateVerificationCode(HttpServletRequest request, @RequestBody TwoFactorAuthRegistration registration) {
String orcid = getCurrentUserOrcid();
boolean valid = twoFactorAuthenticationManager.verificationCodeIsValid(registration.getVerificationCode(), orcid);
registration.setValid(valid);
if (valid) {
+ // Read before enable2FA, which is what makes this true for everyone
+ boolean wasAlreadyUsing2FA = twoFactorAuthenticationManager.userUsing2FA(orcid);
List backupCodes = twoFactorAuthenticationManager.enable2FA(orcid);
- registration.setBackupCodes(backupCodes);
+ registration.setBackupCodes(backupCodes);
//send email notification
recordEmailSender.send2FAEnabledEmail(orcid);
+ if (!wasAlreadyUsing2FA) {
+ // Step 1 of 2FA setup elevates the session for the recovery phone
+ // step that follows it: the user has just typed a live time based
+ // code, and asking for their password one screen later is the
+ // wrong trade (R2.6). Only setup gets that: on an account that
+ // already has 2FA on there is no step 2 to carry, and letting a
+ // time based code elevate there would put it in the place of the
+ // password challenge that guards changing a recovery number
+ request.getSession().setAttribute(RECOVERY_PHONE_ELEVATION_ATTRIBUTE, System.currentTimeMillis());
+ }
}
return registration;
}
diff --git a/orcid-web/src/main/resources/orcid-core-context-spam.xml b/orcid-web/src/main/resources/orcid-core-context-spam.xml
index 52d59220fb5..b984b495d99 100644
--- a/orcid-web/src/main/resources/orcid-core-context-spam.xml
+++ b/orcid-web/src/main/resources/orcid-core-context-spam.xml
@@ -700,6 +700,8 @@
+
+
diff --git a/orcid-web/src/main/resources/orcid-frontend-security.xml b/orcid-web/src/main/resources/orcid-frontend-security.xml
index b542821d62a..32c6766980a 100644
--- a/orcid-web/src/main/resources/orcid-frontend-security.xml
+++ b/orcid-web/src/main/resources/orcid-frontend-security.xml
@@ -253,6 +253,8 @@
access="IS_AUTHENTICATED_ANONYMOUSLY" />
+
+
+
diff --git a/orcid-web/src/test/java/org/orcid/frontend/recoveryphone/RecoveryPhoneVerificationServiceTest.java b/orcid-web/src/test/java/org/orcid/frontend/recoveryphone/RecoveryPhoneVerificationServiceTest.java
new file mode 100644
index 00000000000..fb64a0cc77c
--- /dev/null
+++ b/orcid-web/src/test/java/org/orcid/frontend/recoveryphone/RecoveryPhoneVerificationServiceTest.java
@@ -0,0 +1,273 @@
+package org.orcid.frontend.recoveryphone;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.orcid.utils.phone.PhoneNumberValidator;
+import org.orcid.utils.sms.SmsSendResult;
+import org.orcid.utils.sms.VerificationCodeSender;
+
+public class RecoveryPhoneVerificationServiceTest {
+
+ private static final String ORCID = "0000-0000-0000-0001";
+
+ private static final String OTHER_ORCID = "0000-0000-0000-0002";
+
+ private static final String PHONE = "+441234567890";
+
+ private RecoveryPhoneVerificationService service;
+
+ private CapturingSender awsSender;
+
+ private FakeStore store;
+
+ @Before
+ public void setUp() {
+ store = new FakeStore();
+ service = new RecoveryPhoneVerificationService();
+ service.setPhoneNumberValidator(new PhoneNumberValidator());
+ service.setRecoveryPhoneCodeStore(store);
+ service.setProvider("aws");
+ service.setDefaultRegion("GB");
+ service.setRegexFilter("");
+ service.setCodeLength(6);
+ service.setCodeTtlSeconds(300);
+ service.setMaxAttempts(3);
+ service.setResendBufferSeconds(30);
+ awsSender = new CapturingSender("aws");
+ service.setSenders(Arrays.asList(awsSender));
+ }
+
+ private static RecoveryPhoneSendCodeRequest sendRequest(String phone) {
+ RecoveryPhoneSendCodeRequest request = new RecoveryPhoneSendCodeRequest();
+ request.setPhoneNumber(phone);
+ return request;
+ }
+
+ private RecoveryPhoneSendCodeResponse send(String orcid, String phone) {
+ return service.sendCode(orcid, sendRequest(phone));
+ }
+
+ @Test
+ public void sendCodeDispatchesACodeAndReportsTheResendBuffer() {
+ RecoveryPhoneSendCodeResponse response = send(ORCID, PHONE);
+
+ assertTrue(response.isSuccess());
+ assertEquals(30, response.getResendAfterSeconds());
+ assertEquals(PHONE, awsSender.lastTo);
+ assertEquals(6, awsSender.lastCode.length());
+ }
+
+ @Test
+ public void sendCodeNeverReturnsTheCodeOrTheNumber() {
+ RecoveryPhoneSendCodeResponse response = send(ORCID, PHONE);
+
+ String serialized = response.getErrorCode() + String.valueOf(response.getResendAfterSeconds()) + response.isSuccess();
+ assertFalse(serialized.contains(awsSender.lastCode));
+ assertFalse(serialized.contains(PHONE));
+ }
+
+ @Test
+ public void tooShortAndTooLongNumbersAreReportedSeparately() {
+ assertEquals("PHONE_TOO_SHORT", send(ORCID, "+441234").getErrorCode());
+ assertEquals("PHONE_TOO_LONG", send(ORCID, "+4412345678901234").getErrorCode());
+ }
+
+ @Test
+ public void anUnparseableNumberIsReportedAsInvalid() {
+ assertEquals("INVALID_PHONE_NUMBER", send(ORCID, "?123456789").getErrorCode());
+ }
+
+ @Test
+ public void aNumberOutsideTheSafetyFilterIsRefused() {
+ service.setRegexFilter("\\+506.*");
+ assertEquals(RecoveryPhoneVerificationService.SMS_RECIPIENT_NOT_ALLOWED, send(ORCID, PHONE).getErrorCode());
+ }
+
+ @Test
+ public void resendIsRefusedUntilTheBufferHasPassed() {
+ send(ORCID, PHONE);
+
+ RecoveryPhoneSendCodeResponse response = send(ORCID, PHONE);
+
+ assertFalse(response.isSuccess());
+ assertEquals(RecoveryPhoneVerificationService.RESEND_TOO_SOON, response.getErrorCode());
+ assertTrue(response.getResendAfterSeconds() > 0);
+ }
+
+ @Test
+ public void resendIsAllowedOnceTheBufferHasPassed() {
+ send(ORCID, PHONE);
+ store.ageEntriesBySeconds(ORCID, 31);
+
+ assertTrue(send(ORCID, PHONE).isSuccess());
+ }
+
+ @Test
+ public void aNewCodeRetiresThePreviousOne() {
+ send(ORCID, PHONE);
+ String firstCode = awsSender.lastCode;
+ store.ageEntriesBySeconds(ORCID, 31);
+ send(ORCID, PHONE);
+
+ assertEquals(RecoveryPhoneVerificationService.INVALID_CODE, service.verifyCode(ORCID, PHONE, firstCode));
+ assertNull(service.verifyCode(ORCID, PHONE, awsSender.lastCode));
+ }
+
+ @Test
+ public void aCorrectCodeVerifiesAndIsThenConsumed() {
+ send(ORCID, PHONE);
+ String code = awsSender.lastCode;
+
+ assertNull(service.verifyCode(ORCID, PHONE, code));
+ assertEquals(RecoveryPhoneVerificationService.CODE_EXPIRED, service.verifyCode(ORCID, PHONE, code));
+ }
+
+ @Test
+ public void aCodeSentToOneNumberCannotAuthoriseAnother() {
+ send(ORCID, PHONE);
+
+ assertEquals(RecoveryPhoneVerificationService.PHONE_MISMATCH, service.verifyCode(ORCID, "+441234567891", awsSender.lastCode));
+ }
+
+ @Test
+ public void aCodeBelongsToTheRecordItWasSentFor() {
+ send(ORCID, PHONE);
+
+ assertEquals(RecoveryPhoneVerificationService.CODE_EXPIRED, service.verifyCode(OTHER_ORCID, PHONE, awsSender.lastCode));
+ }
+
+ @Test
+ public void wrongCodesAreRefusedAndEventuallyExhaustTheAttempts() {
+ send(ORCID, PHONE);
+
+ assertEquals(RecoveryPhoneVerificationService.INVALID_CODE, service.verifyCode(ORCID, PHONE, "000000"));
+ assertEquals(RecoveryPhoneVerificationService.INVALID_CODE, service.verifyCode(ORCID, PHONE, "000000"));
+ assertEquals(RecoveryPhoneVerificationService.INVALID_CODE, service.verifyCode(ORCID, PHONE, "000000"));
+ assertEquals(RecoveryPhoneVerificationService.TOO_MANY_ATTEMPTS, service.verifyCode(ORCID, PHONE, "000000"));
+ // the entry is gone, so even the right code no longer works
+ assertEquals(RecoveryPhoneVerificationService.CODE_EXPIRED, service.verifyCode(ORCID, PHONE, awsSender.lastCode));
+ }
+
+ @Test
+ public void anExpiredCodeIsRefused() {
+ send(ORCID, PHONE);
+ store.ageEntriesBySeconds(ORCID, 301);
+
+ assertEquals(RecoveryPhoneVerificationService.CODE_EXPIRED, service.verifyCode(ORCID, PHONE, awsSender.lastCode));
+ }
+
+ @Test
+ public void aBlankCodeIsRefusedWithoutTouchingTheStore() {
+ send(ORCID, PHONE);
+
+ assertEquals(RecoveryPhoneVerificationService.INVALID_CODE, service.verifyCode(ORCID, PHONE, " "));
+ assertNotNull(store.get(ORCID));
+ }
+
+ @Test
+ public void sendFailsWhenTheCodeCannotBeStored() {
+ store.failSaves = true;
+
+ RecoveryPhoneSendCodeResponse response = send(ORCID, PHONE);
+
+ assertFalse(response.isSuccess());
+ assertEquals(RecoveryPhoneVerificationService.CODE_STORAGE_UNAVAILABLE, response.getErrorCode());
+ }
+
+ @Test
+ public void sendFailsWhenTheProviderRejectsTheMessage() {
+ awsSender.fail = true;
+
+ RecoveryPhoneSendCodeResponse response = send(ORCID, PHONE);
+
+ assertFalse(response.isSuccess());
+ assertEquals(RecoveryPhoneVerificationService.SMS_SEND_FAILED, response.getErrorCode());
+ assertNull(store.get(ORCID));
+ }
+
+ @Test
+ public void normalizeReturnsTheE164FormOrNull() {
+ assertEquals(PHONE, service.normalize("01234 567890"));
+ assertNull(service.normalize("?12345"));
+ }
+
+ /**
+ * Stands in for the redis backed store, and lets a test pretend time has
+ * passed by rewriting the entry's sent-at stamp.
+ */
+ private static class FakeStore extends RecoveryPhoneCodeStore {
+
+ private final Map entries = new HashMap<>();
+
+ private boolean failSaves;
+
+ @Override
+ public boolean save(String orcid, RecoveryPhoneCodeEntry entry, int ttlSeconds) {
+ if (failSaves) {
+ return false;
+ }
+ // round trip through the serialized form, as the real store does
+ entries.put(orcid, RecoveryPhoneCodeEntry.parse(entry.serialize()));
+ return true;
+ }
+
+ @Override
+ public RecoveryPhoneCodeEntry get(String orcid) {
+ return entries.get(orcid);
+ }
+
+ @Override
+ public void remove(String orcid) {
+ entries.remove(orcid);
+ }
+
+ void ageEntriesBySeconds(String orcid, int seconds) {
+ RecoveryPhoneCodeEntry entry = entries.get(orcid);
+ if (entry != null) {
+ entries.put(orcid, new RecoveryPhoneCodeEntry(entry.getCode(), entry.getPhoneE164(), entry.getProvider(),
+ entry.getProviderMessageId(), entry.getAttempts(), entry.getSentAt() - (seconds * 1000L)));
+ }
+ }
+ }
+
+ private static class CapturingSender implements VerificationCodeSender {
+
+ private final String provider;
+
+ private String lastTo;
+
+ private String lastCode;
+
+ private boolean fail;
+
+ CapturingSender(String provider) {
+ this.provider = provider;
+ }
+
+ @Override
+ public String getProvider() {
+ return provider;
+ }
+
+ @Override
+ public SmsSendResult sendCode(String to, String code, String locale) {
+ this.lastTo = to;
+ this.lastCode = code;
+ if (fail) {
+ return SmsSendResult.failure(provider, "PROVIDER_ERROR", "boom");
+ }
+ return SmsSendResult.success(provider, "message-id", "PENDING");
+ }
+ }
+
+}
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
new file mode 100644
index 00000000000..a346223fe23
--- /dev/null
+++ b/orcid-web/src/test/java/org/orcid/frontend/web/controllers/RecoveryPhoneSigninControllerTest.java
@@ -0,0 +1,490 @@
+package org.orcid.frontend.web.controllers;
+
+import jakarta.annotation.Resource;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.mockito.ArgumentCaptor;
+import org.mockito.InOrder;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+import org.mockito.Spy;
+import org.orcid.authorization.authentication.MFAWebAuthenticationDetails;
+import org.orcid.core.manager.ProfileEntityCacheManager;
+import org.orcid.core.manager.RecoveryPhone;
+import org.orcid.core.manager.RecoveryPhoneManager;
+import org.orcid.core.manager.TwoFactorAuthenticationManager;
+import org.orcid.core.manager.v3.read_only.EmailManagerReadOnly;
+import org.orcid.core.togglz.Features;
+import org.orcid.frontend.email.RecordEmailSender;
+import org.orcid.frontend.recoveryphone.RecoveryPhoneSendCodeRequest;
+import org.orcid.frontend.recoveryphone.RecoveryPhoneSendCodeResponse;
+import org.orcid.frontend.recoveryphone.RecoveryPhoneSigninSendCodeRequest;
+import org.orcid.frontend.recoveryphone.RecoveryPhoneSigninSendCodeResponse;
+import org.orcid.frontend.recoveryphone.RecoveryPhoneSigninVerifyRequest;
+import org.orcid.frontend.recoveryphone.RecoveryPhoneSigninVerifyResponse;
+import org.orcid.frontend.recoveryphone.RecoveryPhoneVerificationService;
+import org.orcid.frontend.web.exception.VerificationCodeFor2FARequiredException;
+import org.springframework.mock.web.MockHttpServletRequest;
+import org.springframework.security.authentication.AuthenticationManager;
+import org.springframework.security.authentication.AuthenticationProvider;
+import org.springframework.security.authentication.BadCredentialsException;
+import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
+import org.springframework.security.core.Authentication;
+import org.togglz.junit.TogglzRule;
+
+import java.lang.reflect.Field;
+import java.util.Collections;
+import java.util.Date;
+import java.util.Locale;
+
+import static org.junit.Assert.*;
+import static org.mockito.ArgumentMatchers.*;
+import static org.mockito.Mockito.*;
+
+public class RecoveryPhoneSigninControllerTest {
+
+ private static final String ORCID = "0000-0000-0000-0001";
+
+ private static final String EMAIL = "user@orcid.org";
+
+ private static final String PASSWORD = "correct-horse";
+
+ private static final String STORED_NUMBER = "+441234567890";
+
+ private static final String REMOTE_ADDRESS = "198.51.100.7";
+
+ @Mock
+ private AuthenticationProvider authenticationProvider;
+
+ @Mock
+ private RecoveryPhoneManager recoveryPhoneManager;
+
+ @Mock
+ private RecoveryPhoneVerificationService recoveryPhoneVerificationService;
+
+ @Mock
+ private TwoFactorAuthenticationManager twoFactorAuthenticationManager;
+
+ @Mock
+ private ProfileEntityCacheManager profileEntityCacheManager;
+
+ @Mock
+ private RecordEmailSender recordEmailSender;
+
+ @Mock
+ private EmailManagerReadOnly emailManagerReadOnly;
+
+ @Spy
+ @InjectMocks
+ private RecoveryPhoneSigninController controller;
+
+ private final MockHttpServletRequest request = new MockHttpServletRequest();
+
+ @Rule
+ public TogglzRule togglzRule = TogglzRule.allDisabled(Features.class);
+
+ @Before
+ public void setUp() {
+ MockitoAnnotations.initMocks(this);
+ request.setRemoteAddr(REMOTE_ADDRESS);
+ doReturn(Locale.ENGLISH).when(controller).getLocale();
+ }
+
+ private static RecoveryPhoneSigninSendCodeRequest sendCodeRequest(String username) {
+ RecoveryPhoneSigninSendCodeRequest form = new RecoveryPhoneSigninSendCodeRequest();
+ form.setUsername(username);
+ form.setPassword(PASSWORD);
+ return form;
+ }
+
+ private static RecoveryPhoneSigninVerifyRequest verifyRequest(String username, String code) {
+ RecoveryPhoneSigninVerifyRequest form = new RecoveryPhoneSigninVerifyRequest();
+ form.setUsername(username);
+ form.setPassword(PASSWORD);
+ form.setVerificationCode(code);
+ return form;
+ }
+
+ private static RecoveryPhone storedRecoveryPhone() {
+ Date now = new Date();
+ return new RecoveryPhone("7890", now, now);
+ }
+
+ /** The password is right and the account is using 2FA: the recoverable state. */
+ private void passwordIsCorrectAnd2FAIsOn() {
+ when(authenticationProvider.authenticate(any(Authentication.class))).thenThrow(new VerificationCodeFor2FARequiredException());
+ }
+
+ /** The password is right but the account has nothing to recover from. */
+ private void passwordIsCorrectAnd2FAIsOff() {
+ when(authenticationProvider.authenticate(any(Authentication.class)))
+ .thenReturn(new UsernamePasswordAuthenticationToken(ORCID, PASSWORD, Collections.emptyList()));
+ }
+
+ private void passwordIsWrong() {
+ when(authenticationProvider.authenticate(any(Authentication.class))).thenThrow(new BadCredentialsException("Invalid username or password"));
+ }
+
+ /** The mask and the dates come from one read, the number from the other. */
+ private void aRecoveryPhoneIsStored() {
+ when(recoveryPhoneManager.getRecoveryPhone(ORCID)).thenReturn(storedRecoveryPhone());
+ when(recoveryPhoneManager.getDecryptedPhoneNumber(ORCID)).thenReturn(STORED_NUMBER);
+ }
+
+ @Test
+ public void testSendCodeIsInertWhenTheFeatureIsOff() {
+ RecoveryPhoneSigninSendCodeResponse response = controller.sendCode(request, sendCodeRequest(ORCID));
+
+ assertFalse(response.isSuccess());
+ assertEquals(RecoveryPhoneSigninController.FEATURE_DISABLED, response.getErrorCode());
+ verify(authenticationProvider, never()).authenticate(any(Authentication.class));
+ verify(recoveryPhoneManager, never()).getRecoveryPhone(anyString());
+ verify(recoveryPhoneVerificationService, never()).sendCode(anyString(), any(RecoveryPhoneSendCodeRequest.class));
+ }
+
+ @Test
+ public void testVerifyIsInertWhenTheFeatureIsOff() {
+ RecoveryPhoneSigninVerifyResponse response = controller.verify(request, verifyRequest(ORCID, "123456"));
+
+ assertFalse(response.isSuccess());
+ assertEquals(RecoveryPhoneSigninController.FEATURE_DISABLED, response.getErrorCode());
+ verify(authenticationProvider, never()).authenticate(any(Authentication.class));
+ verify(twoFactorAuthenticationManager, never()).disable2FAByRecoveryPhone(anyString());
+ }
+
+ /**
+ * A wrong password has to reach the authentication provider, because that is
+ * what counts it toward the sign in lockout (R3.2).
+ */
+ @Test
+ public void testAWrongPasswordIsCheckedThroughTheAuthenticationProviderAndSendsNoCode() {
+ togglzRule.enable(Features.TWO_FACTOR_RECOVERY_PHONE);
+ passwordIsWrong();
+
+ RecoveryPhoneSigninSendCodeResponse response = controller.sendCode(request, sendCodeRequest(ORCID));
+
+ assertFalse(response.isSuccess());
+ assertEquals(RecoveryPhoneSigninController.BAD_CREDENTIALS, response.getErrorCode());
+ assertNull(response.getMaskedRecoveryPhoneNumber());
+ verify(recoveryPhoneVerificationService, never()).sendCode(anyString(), any(RecoveryPhoneSendCodeRequest.class));
+ verify(recoveryPhoneManager, never()).getRecoveryPhone(anyString());
+ }
+
+ /**
+ * The token carries the credentials and the caller's address, and no 2FA
+ * codes at all, which is what makes the provider answer "2FA required"
+ * rather than trying to verify something.
+ */
+ @Test
+ public void testTheAuthenticationTokenCarriesNo2FACodes() {
+ togglzRule.enable(Features.TWO_FACTOR_RECOVERY_PHONE);
+ passwordIsWrong();
+
+ controller.sendCode(request, sendCodeRequest(ORCID));
+
+ ArgumentCaptor captor = ArgumentCaptor.forClass(Authentication.class);
+ verify(authenticationProvider).authenticate(captor.capture());
+ Authentication token = captor.getValue();
+ assertEquals(ORCID, token.getName());
+ assertEquals(PASSWORD, token.getCredentials());
+ MFAWebAuthenticationDetails details = (MFAWebAuthenticationDetails) token.getDetails();
+ assertNull(details.getVerificationCode());
+ assertNull(details.getRecoveryCode());
+ assertEquals(REMOTE_ADDRESS, details.getRemoteAddress());
+ }
+
+ @Test
+ public void testSendCodeRefusesAnAccountThatIsNotUsing2FA() {
+ togglzRule.enable(Features.TWO_FACTOR_RECOVERY_PHONE);
+ passwordIsCorrectAnd2FAIsOff();
+
+ 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 testSendCodeSaysSoWhenNoNumberIsStored() {
+ togglzRule.enable(Features.TWO_FACTOR_RECOVERY_PHONE);
+ passwordIsCorrectAnd2FAIsOn();
+ when(recoveryPhoneManager.getRecoveryPhone(ORCID)).thenReturn(null);
+
+ RecoveryPhoneSigninSendCodeResponse response = controller.sendCode(request, sendCodeRequest(ORCID));
+
+ assertFalse(response.isSuccess());
+ assertEquals(RecoveryPhoneSigninController.NO_RECOVERY_PHONE, response.getErrorCode());
+ verify(recoveryPhoneVerificationService, never()).sendCode(anyString(), any(RecoveryPhoneSendCodeRequest.class));
+ // Saying "no recovery phone" costs no decryption
+ verify(recoveryPhoneManager, never()).getDecryptedPhoneNumber(anyString());
+ }
+
+ /**
+ * The row says there is a number but the number cannot be read: the caller
+ * is told there is none rather than being texted at null.
+ */
+ @Test
+ public void testSendCodeSaysSoWhenTheStoredNumberCannotBeRead() {
+ togglzRule.enable(Features.TWO_FACTOR_RECOVERY_PHONE);
+ passwordIsCorrectAnd2FAIsOn();
+ when(recoveryPhoneManager.getRecoveryPhone(ORCID)).thenReturn(storedRecoveryPhone());
+ when(recoveryPhoneManager.getDecryptedPhoneNumber(ORCID)).thenReturn(null);
+
+ RecoveryPhoneSigninSendCodeResponse response = controller.sendCode(request, sendCodeRequest(ORCID));
+
+ assertFalse(response.isSuccess());
+ assertEquals(RecoveryPhoneSigninController.NO_RECOVERY_PHONE, response.getErrorCode());
+ verify(recoveryPhoneVerificationService, never()).sendCode(anyString(), any(RecoveryPhoneSendCodeRequest.class));
+ }
+
+ /**
+ * The number is read from the record, never taken from the request, and only
+ * its last four digits come back.
+ */
+ @Test
+ public void testSendCodeTextsTheStoredNumberAndReturnsTheMask() {
+ togglzRule.enable(Features.TWO_FACTOR_RECOVERY_PHONE);
+ passwordIsCorrectAnd2FAIsOn();
+ aRecoveryPhoneIsStored();
+ when(recoveryPhoneVerificationService.sendCode(eq(ORCID), any(RecoveryPhoneSendCodeRequest.class)))
+ .thenReturn(RecoveryPhoneSendCodeResponse.success(30));
+
+ RecoveryPhoneSigninSendCodeResponse response = controller.sendCode(request, sendCodeRequest(ORCID));
+
+ assertTrue(response.isSuccess());
+ assertNull(response.getErrorCode());
+ assertEquals(30, response.getResendAfterSeconds());
+ assertEquals("***********7890", response.getMaskedRecoveryPhoneNumber());
+
+ ArgumentCaptor captor = ArgumentCaptor.forClass(RecoveryPhoneSendCodeRequest.class);
+ verify(recoveryPhoneVerificationService).sendCode(eq(ORCID), captor.capture());
+ assertEquals(STORED_NUMBER, captor.getValue().getPhoneNumber());
+ assertEquals("en", captor.getValue().getLocale());
+ verify(recoveryPhoneManager).getDecryptedPhoneNumber(ORCID);
+ }
+
+ @Test
+ public void testSendCodePassesThroughAThrottledResend() {
+ togglzRule.enable(Features.TWO_FACTOR_RECOVERY_PHONE);
+ passwordIsCorrectAnd2FAIsOn();
+ aRecoveryPhoneIsStored();
+ when(recoveryPhoneVerificationService.sendCode(eq(ORCID), any(RecoveryPhoneSendCodeRequest.class)))
+ .thenReturn(RecoveryPhoneSendCodeResponse.failure(RecoveryPhoneVerificationService.RESEND_TOO_SOON, 12));
+
+ RecoveryPhoneSigninSendCodeResponse response = controller.sendCode(request, sendCodeRequest(ORCID));
+
+ assertFalse(response.isSuccess());
+ assertEquals(RecoveryPhoneVerificationService.RESEND_TOO_SOON, response.getErrorCode());
+ assertEquals(12, response.getResendAfterSeconds());
+ assertEquals("***********7890", response.getMaskedRecoveryPhoneNumber());
+ }
+
+ /** An email address is as good a username here as an iD is. */
+ @Test
+ public void testAnEmailAddressResolvesToTheRecord() {
+ togglzRule.enable(Features.TWO_FACTOR_RECOVERY_PHONE);
+ passwordIsCorrectAnd2FAIsOn();
+ when(emailManagerReadOnly.findOrcidIdByEmail(EMAIL)).thenReturn(ORCID);
+ aRecoveryPhoneIsStored();
+ when(recoveryPhoneVerificationService.sendCode(eq(ORCID), any(RecoveryPhoneSendCodeRequest.class)))
+ .thenReturn(RecoveryPhoneSendCodeResponse.success(30));
+
+ RecoveryPhoneSigninSendCodeResponse response = controller.sendCode(request, sendCodeRequest(EMAIL));
+
+ assertTrue(response.isSuccess());
+ verify(emailManagerReadOnly).findOrcidIdByEmail(EMAIL);
+ verify(recoveryPhoneManager).getRecoveryPhone(ORCID);
+ verify(recoveryPhoneVerificationService).sendCode(eq(ORCID), any(RecoveryPhoneSendCodeRequest.class));
+ }
+
+ @Test
+ public void testAnOrcidIdUsernameIsNotLookedUpAsAnEmail() {
+ togglzRule.enable(Features.TWO_FACTOR_RECOVERY_PHONE);
+ passwordIsCorrectAnd2FAIsOn();
+ aRecoveryPhoneIsStored();
+ when(recoveryPhoneVerificationService.sendCode(eq(ORCID), any(RecoveryPhoneSendCodeRequest.class)))
+ .thenReturn(RecoveryPhoneSendCodeResponse.success(30));
+
+ controller.sendCode(request, sendCodeRequest(ORCID));
+
+ verify(emailManagerReadOnly, never()).findOrcidIdByEmail(anyString());
+ }
+
+ @Test
+ public void testVerifyRejectsAWrongPasswordWithoutDisabling2FA() {
+ togglzRule.enable(Features.TWO_FACTOR_RECOVERY_PHONE);
+ passwordIsWrong();
+
+ RecoveryPhoneSigninVerifyResponse response = controller.verify(request, verifyRequest(ORCID, "123456"));
+
+ assertFalse(response.isSuccess());
+ assertEquals(RecoveryPhoneSigninController.BAD_CREDENTIALS, response.getErrorCode());
+ assertNull(response.getOrcid());
+ verify(recoveryPhoneVerificationService, never()).verifyCode(anyString(), anyString(), anyString());
+ verify(twoFactorAuthenticationManager, never()).disable2FAByRecoveryPhone(anyString());
+ }
+
+ @Test
+ public void testVerifySaysSoWhenNoNumberIsStored() {
+ togglzRule.enable(Features.TWO_FACTOR_RECOVERY_PHONE);
+ passwordIsCorrectAnd2FAIsOn();
+ when(recoveryPhoneManager.getRecoveryPhone(ORCID)).thenReturn(null);
+
+ RecoveryPhoneSigninVerifyResponse response = controller.verify(request, verifyRequest(ORCID, "123456"));
+
+ assertEquals(RecoveryPhoneSigninController.NO_RECOVERY_PHONE, response.getErrorCode());
+ verify(recoveryPhoneVerificationService, never()).verifyCode(anyString(), anyString(), anyString());
+ verify(recoveryPhoneManager, never()).getDecryptedPhoneNumber(anyString());
+ }
+
+ @Test
+ public void testVerifySaysSoWhenTheStoredNumberCannotBeRead() {
+ togglzRule.enable(Features.TWO_FACTOR_RECOVERY_PHONE);
+ passwordIsCorrectAnd2FAIsOn();
+ when(recoveryPhoneManager.getRecoveryPhone(ORCID)).thenReturn(storedRecoveryPhone());
+ when(recoveryPhoneManager.getDecryptedPhoneNumber(ORCID)).thenReturn(null);
+
+ RecoveryPhoneSigninVerifyResponse response = controller.verify(request, verifyRequest(ORCID, "123456"));
+
+ assertFalse(response.isSuccess());
+ assertEquals(RecoveryPhoneSigninController.NO_RECOVERY_PHONE, response.getErrorCode());
+ verify(recoveryPhoneVerificationService, never()).verifyCode(anyString(), anyString(), anyString());
+ verify(twoFactorAuthenticationManager, never()).disable2FAByRecoveryPhone(anyString());
+ }
+
+ /** A wrong code changes nothing at all about the account. */
+ @Test
+ public void testVerifyReportsAWrongCodeAndLeaves2FAEnabled() {
+ togglzRule.enable(Features.TWO_FACTOR_RECOVERY_PHONE);
+ passwordIsCorrectAnd2FAIsOn();
+ aRecoveryPhoneIsStored();
+ when(recoveryPhoneVerificationService.verifyCode(ORCID, STORED_NUMBER, "000000"))
+ .thenReturn(RecoveryPhoneVerificationService.INVALID_CODE);
+
+ RecoveryPhoneSigninVerifyResponse response = controller.verify(request, verifyRequest(ORCID, "000000"));
+
+ assertFalse(response.isSuccess());
+ assertEquals(RecoveryPhoneVerificationService.INVALID_CODE, response.getErrorCode());
+ assertNull(response.getOrcid());
+ verify(twoFactorAuthenticationManager, never()).disable2FAByRecoveryPhone(anyString());
+ verify(recordEmailSender, never()).send2FADisabledEmail(anyString());
+ verify(profileEntityCacheManager, never()).remove(anyString());
+ }
+
+ @Test
+ public void testVerifyPassesThroughAnExhaustedAttemptCount() {
+ togglzRule.enable(Features.TWO_FACTOR_RECOVERY_PHONE);
+ passwordIsCorrectAnd2FAIsOn();
+ aRecoveryPhoneIsStored();
+ when(recoveryPhoneVerificationService.verifyCode(ORCID, STORED_NUMBER, "000000"))
+ .thenReturn(RecoveryPhoneVerificationService.TOO_MANY_ATTEMPTS);
+
+ assertEquals(RecoveryPhoneVerificationService.TOO_MANY_ATTEMPTS,
+ controller.verify(request, verifyRequest(ORCID, "000000")).getErrorCode());
+ verify(twoFactorAuthenticationManager, never()).disable2FAByRecoveryPhone(anyString());
+ }
+
+ /**
+ * The good code disables 2FA, evicts the cached profile and only then tells
+ * the user by email: the sign in the browser replays next must not meet a
+ * cached using2FA=true, so the eviction cannot sit behind anything that can
+ * fail (R3.5).
+ */
+ @Test
+ public void testVerifyDisables2FAAndEvictsTheCachedProfileBeforeTheEmail() {
+ togglzRule.enable(Features.TWO_FACTOR_RECOVERY_PHONE);
+ passwordIsCorrectAnd2FAIsOn();
+ aRecoveryPhoneIsStored();
+ when(recoveryPhoneVerificationService.verifyCode(ORCID, STORED_NUMBER, "123456")).thenReturn(null);
+
+ RecoveryPhoneSigninVerifyResponse response = controller.verify(request, verifyRequest(ORCID, "123456"));
+
+ assertTrue(response.isSuccess());
+ assertNull(response.getErrorCode());
+ assertEquals(ORCID, response.getOrcid());
+
+ InOrder inOrder = inOrder(twoFactorAuthenticationManager, profileEntityCacheManager, recordEmailSender);
+ inOrder.verify(twoFactorAuthenticationManager).disable2FAByRecoveryPhone(ORCID);
+ inOrder.verify(profileEntityCacheManager).remove(ORCID);
+ inOrder.verify(recordEmailSender).send2FADisabledEmail(ORCID);
+ }
+
+ /**
+ * The disable is irreversible, so a failing notification must not report the
+ * operation as failed: 2FA is off, the number and the backup codes are gone,
+ * and the browser's next sign in has to find the cache already evicted.
+ */
+ @Test
+ public void testAFailing2FADisabledEmailStillLeavesTheRecoverySuccessful() {
+ togglzRule.enable(Features.TWO_FACTOR_RECOVERY_PHONE);
+ passwordIsCorrectAnd2FAIsOn();
+ aRecoveryPhoneIsStored();
+ when(recoveryPhoneVerificationService.verifyCode(ORCID, STORED_NUMBER, "123456")).thenReturn(null);
+ doThrow(new RuntimeException("mail server is down")).when(recordEmailSender).send2FADisabledEmail(ORCID);
+
+ RecoveryPhoneSigninVerifyResponse response = controller.verify(request, verifyRequest(ORCID, "123456"));
+
+ assertTrue(response.isSuccess());
+ assertNull(response.getErrorCode());
+ assertEquals(ORCID, response.getOrcid());
+ verify(twoFactorAuthenticationManager).disable2FAByRecoveryPhone(ORCID);
+ verify(profileEntityCacheManager).remove(ORCID);
+ }
+
+ /** The code is checked against the stored number, not one the caller sent. */
+ @Test
+ public void testVerifyChecksTheCodeAgainstTheStoredNumber() {
+ togglzRule.enable(Features.TWO_FACTOR_RECOVERY_PHONE);
+ passwordIsCorrectAnd2FAIsOn();
+ aRecoveryPhoneIsStored();
+ when(recoveryPhoneVerificationService.verifyCode(anyString(), anyString(), anyString())).thenReturn(null);
+
+ controller.verify(request, verifyRequest(ORCID, "123456"));
+
+ verify(recoveryPhoneVerificationService).verifyCode(ORCID, STORED_NUMBER, "123456");
+ }
+
+ @Test
+ public void testBlankCredentialsNeverReachTheAuthenticationProvider() {
+ togglzRule.enable(Features.TWO_FACTOR_RECOVERY_PHONE);
+ RecoveryPhoneSigninSendCodeRequest form = new RecoveryPhoneSigninSendCodeRequest();
+ form.setUsername(ORCID);
+ form.setPassword("");
+
+ assertEquals(RecoveryPhoneSigninController.BAD_CREDENTIALS, controller.sendCode(request, form).getErrorCode());
+ verify(authenticationProvider, never()).authenticate(any(Authentication.class));
+ }
+
+ /**
+ * The credential check is bound to the provider bean and not to the
+ * "authenticationManager" ProviderManager, which publishes an authentication
+ * success event: on an account with 2FA off that event would record a sign
+ * in that never happened, and run every listener on it, for a caller who
+ * only posted a password to a recovery endpoint (R3.2).
+ */
+ @Test
+ public void testTheCredentialCheckIsBoundToTheProviderAndNotToTheManager() throws Exception {
+ Field field = RecoveryPhoneSigninController.class.getDeclaredField("authenticationProvider");
+ assertEquals(AuthenticationProvider.class, field.getType());
+ assertEquals("authenticationProvider", field.getAnnotation(Resource.class).name());
+ for (Field declared : RecoveryPhoneSigninController.class.getDeclaredFields()) {
+ assertFalse("No authentication manager may be injected here", AuthenticationManager.class.isAssignableFrom(declared.getType()));
+ }
+ }
+
+ @Test
+ public void testAUsernameThatResolvesToNoRecordIsACredentialFailure() {
+ togglzRule.enable(Features.TWO_FACTOR_RECOVERY_PHONE);
+ passwordIsCorrectAnd2FAIsOn();
+ when(emailManagerReadOnly.findOrcidIdByEmail(EMAIL)).thenReturn(null);
+
+ RecoveryPhoneSigninSendCodeResponse response = controller.sendCode(request, sendCodeRequest(EMAIL));
+
+ assertEquals(RecoveryPhoneSigninController.BAD_CREDENTIALS, response.getErrorCode());
+ verify(recoveryPhoneManager, never()).getRecoveryPhone(anyString());
+ }
+}
diff --git a/orcid-web/src/test/java/org/orcid/frontend/web/controllers/TwoFactorAuthenticationControllerTest.java b/orcid-web/src/test/java/org/orcid/frontend/web/controllers/TwoFactorAuthenticationControllerTest.java
index 8419c86316a..9789097711a 100644
--- a/orcid-web/src/test/java/org/orcid/frontend/web/controllers/TwoFactorAuthenticationControllerTest.java
+++ b/orcid-web/src/test/java/org/orcid/frontend/web/controllers/TwoFactorAuthenticationControllerTest.java
@@ -1,7 +1,9 @@
package org.orcid.frontend.web.controllers;
import org.junit.Before;
+import org.junit.Rule;
import org.junit.Test;
+import org.mockito.ArgumentCaptor;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
@@ -9,16 +11,30 @@
import org.orcid.core.manager.BackupCodeManager;
import org.orcid.core.manager.EncryptionManager;
import org.orcid.core.manager.ProfileEntityCacheManager;
+import org.orcid.core.manager.RecoveryPhone;
+import org.orcid.core.manager.RecoveryPhoneManager;
import org.orcid.core.manager.TwoFactorAuthenticationManager;
+import org.orcid.core.manager.v3.ProfileEntityManager;
import org.orcid.frontend.email.RecordEmailSender;
+import org.orcid.frontend.recoveryphone.RecoveryPhoneChallengeSendCodeResponse;
+import org.orcid.frontend.recoveryphone.RecoveryPhoneChallengeVerifyRequest;
+import org.orcid.frontend.recoveryphone.RecoveryPhoneSaveRequest;
+import org.orcid.frontend.recoveryphone.RecoveryPhoneSaveResponse;
+import org.orcid.frontend.recoveryphone.RecoveryPhoneSendCodeRequest;
+import org.orcid.frontend.recoveryphone.RecoveryPhoneSendCodeResponse;
+import org.orcid.frontend.recoveryphone.RecoveryPhoneVerificationService;
import org.orcid.persistence.jpa.entities.ProfileEntity;
+import org.orcid.core.togglz.Features;
import org.orcid.pojo.*;
+import org.springframework.mock.web.MockHttpSession;
import org.springframework.web.servlet.ModelAndView;
+import org.togglz.junit.TogglzRule;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import java.util.Arrays;
import java.util.List;
+import java.util.Locale;
import static org.junit.Assert.*;
import static org.mockito.ArgumentMatchers.*;
@@ -34,9 +50,18 @@ public class TwoFactorAuthenticationControllerTest {
@Mock
private ProfileEntityCacheManager profileEntityCacheManager;
+ @Mock
+ private ProfileEntityManager profileEntityManager;
+
@Mock
private BackupCodeManager backupCodeManager;
+ @Mock
+ private RecoveryPhoneManager recoveryPhoneManager;
+
+ @Mock
+ private RecoveryPhoneVerificationService recoveryPhoneVerificationService;
+
@Mock
private RecordEmailSender recordEmailSender;
@@ -53,13 +78,22 @@ public class TwoFactorAuthenticationControllerTest {
@Mock
private HttpServletResponse response;
+ private final MockHttpSession session = new MockHttpSession();
+
+ @Rule
+ public TogglzRule togglzRule = TogglzRule.allDisabled(Features.class);
+
@Before
public void setUp() {
MockitoAnnotations.initMocks(this);
+ when(request.getSession()).thenReturn(session);
doReturn(ORCID).when(controller).getCurrentUserOrcid();
+ // The session is the account owner's own unless a test says otherwise
+ doReturn(ORCID).when(controller).getRealUserOrcid();
doReturn("redirectUrl").when(controller).calculateRedirectUrl(anyString());
doReturn("redirectUrl").when(controller).calculateRedirectUrl(any(HttpServletRequest.class), any(HttpServletResponse.class), anyBoolean());
doAnswer(invocation -> invocation.getArgument(0)).when(controller).getMessage(anyString(), any());
+ doReturn(Locale.ENGLISH).when(controller).getLocale();
}
@Test
@@ -169,7 +203,7 @@ public void testValidateVerificationCode_Valid() {
List backupCodes = Arrays.asList("code1", "code2");
when(twoFactorAuthenticationManager.enable2FA(ORCID)).thenReturn(backupCodes);
- TwoFactorAuthRegistration result = controller.validateVerificationCode(registration);
+ TwoFactorAuthRegistration result = controller.validateVerificationCode(request, registration);
assertTrue(result.isValid());
assertEquals(backupCodes, result.getBackupCodes());
}
@@ -180,7 +214,7 @@ public void testValidateVerificationCode_Invalid() {
registration.setVerificationCode("654321");
when(twoFactorAuthenticationManager.verificationCodeIsValid("654321", ORCID)).thenReturn(false);
- TwoFactorAuthRegistration result = controller.validateVerificationCode(registration);
+ TwoFactorAuthRegistration result = controller.validateVerificationCode(request, registration);
assertFalse(result.isValid());
assertNull(result.getBackupCodes());
}
@@ -234,4 +268,614 @@ public void testPost2FAVerificationCode_Invalid() {
assertEquals(1, result.getErrors().size());
assertEquals("2FA.verificationCode.invalid", result.getErrors().get(0));
}
+
+ // Recovery phone number
+
+ private void enableRecoveryPhoneFeature() {
+ togglzRule.enable(Features.TWO_FACTOR_RECOVERY_PHONE);
+ when(twoFactorAuthenticationManager.userUsing2FA(ORCID)).thenReturn(true);
+ }
+
+ /** Both flags: the interstitial carries a second one of its own (R7.4). */
+ private void enableInterstitialFeature() {
+ enableRecoveryPhoneFeature();
+ togglzRule.enable(Features.LOGIN_RECOVERY_PHONE_INTERSTITIAL);
+ }
+
+ private void elevateSession() {
+ session.setAttribute("RECOVERY_PHONE_ELEVATION_TS", System.currentTimeMillis());
+ }
+
+ private static RecoveryPhoneSendCodeRequest sendCodeRequest() {
+ RecoveryPhoneSendCodeRequest form = new RecoveryPhoneSendCodeRequest();
+ form.setPhoneNumber("+441234567890");
+ return form;
+ }
+
+ private static RecoveryPhoneSaveRequest saveRequest() {
+ RecoveryPhoneSaveRequest form = new RecoveryPhoneSaveRequest();
+ form.setPhoneNumber("+441234567890");
+ form.setVerificationCode("123456");
+ return form;
+ }
+
+ /**
+ * What the record carries for a stored number: the mask and the dates.
+ *
+ * Built with the real constructor rather than mocked. RecoveryPhone is an
+ * immutable value object, so there is nothing to fake; and a helper that
+ * stubs a mock cannot be called from inside another when(...), which is
+ * where every call below sits. Mockito rejects that with
+ * UnfinishedStubbingException, and its message points at the outer stub
+ * rather than at the nested one.
+ */
+ private static RecoveryPhone storedRecoveryPhone(String lastFour, java.util.Date dateCreated, java.util.Date lastModified) {
+ return new RecoveryPhone(lastFour, dateCreated, lastModified);
+ }
+
+ /**
+ * Puts a profile in the cache and records a last login of the given age in
+ * the database, or no last login at all when millisAgo is null.
+ *
+ * The cached profile deliberately carries a sign in from a day ago, out of
+ * every window: the cache is loaded while the user is being authenticated,
+ * before the success handler writes last_login, so what it holds is always
+ * the previous sign in. Only the database read may be believed.
+ */
+ private ProfileEntity profileWithLastLogin(Long millisAgo) {
+ ProfileEntity profile = new ProfileEntity();
+ profile.setEncryptedPassword("hashed");
+ profile.setLastLogin(new java.util.Date(System.currentTimeMillis() - (24 * 60 * 60 * 1000L)));
+ when(profileEntityCacheManager.retrieve(ORCID)).thenReturn(profile);
+ if (millisAgo != null) {
+ when(profileEntityManager.getLastLogin(ORCID)).thenReturn(new java.util.Date(System.currentTimeMillis() - millisAgo));
+ }
+ return profile;
+ }
+
+ private static RecoveryPhoneChallengeVerifyRequest challengeVerifyRequest(String password, String verificationCode) {
+ RecoveryPhoneChallengeVerifyRequest form = new RecoveryPhoneChallengeVerifyRequest();
+ form.setPassword(password);
+ form.setVerificationCode(verificationCode);
+ return form;
+ }
+
+ @Test
+ public void testStatusOmitsRecoveryPhoneWhenFeatureIsOff() {
+ when(twoFactorAuthenticationManager.userUsing2FA(ORCID)).thenReturn(true);
+
+ TwoFactorAuthStatus status = controller.get2FAStatus();
+
+ assertNull(status.getMaskedRecoveryPhoneNumber());
+ verify(recoveryPhoneManager, never()).getRecoveryPhone(anyString());
+ }
+
+ @Test
+ public void testStatusMasksTheRecoveryPhoneNumber() {
+ enableRecoveryPhoneFeature();
+ java.util.Date created = new java.util.Date();
+ when(recoveryPhoneManager.getRecoveryPhone(ORCID)).thenReturn(storedRecoveryPhone("7890", created, created));
+
+ TwoFactorAuthStatus status = controller.get2FAStatus();
+
+ assertEquals("***********7890", status.getMaskedRecoveryPhoneNumber());
+ assertNotNull(status.getRecoveryPhoneCreationDate());
+ assertFalse(status.isRecoveryPhoneModified());
+ }
+
+ @Test
+ public void testStatusReportsANumberChangedSecondsAfterItWasAddedAsModified() {
+ // A user who adds a number and corrects it straight away has changed it; the panel must date
+ // the row from the change, not from the first entry.
+ enableRecoveryPhoneFeature();
+ java.util.Date created = new java.util.Date(1_600_000_000_000L);
+ java.util.Date modified = new java.util.Date(1_600_000_000_000L + (5 * 1000L));
+ when(recoveryPhoneManager.getRecoveryPhone(ORCID)).thenReturn(storedRecoveryPhone("7890", created, modified));
+
+ assertTrue(controller.get2FAStatus().isRecoveryPhoneModified());
+ }
+
+ @Test
+ public void testStatusReportsANumberThatHasBeenChangedAsModified() {
+ enableRecoveryPhoneFeature();
+ java.util.Date created = new java.util.Date(1_600_000_000_000L);
+ java.util.Date modified = new java.util.Date(1_600_000_000_000L + (10 * 60 * 1000L));
+ when(recoveryPhoneManager.getRecoveryPhone(ORCID)).thenReturn(storedRecoveryPhone("7890", created, modified));
+
+ assertTrue(controller.get2FAStatus().isRecoveryPhoneModified());
+ }
+
+ @Test
+ public void testAuthChallengeRejectsAWrongPassword() {
+ enableRecoveryPhoneFeature();
+ ProfileEntity profile = new ProfileEntity();
+ profile.setEncryptedPassword("hashed");
+ when(profileEntityCacheManager.retrieve(ORCID)).thenReturn(profile);
+ when(encryptionManager.hashMatches("nope", "hashed")).thenReturn(false);
+
+ AuthChallenge form = new AuthChallenge();
+ form.setPassword("nope");
+ AuthChallenge result = controller.verifyRecoveryPhoneAuthChallenge(request, form);
+
+ assertTrue(result.isInvalidPassword());
+ assertNull(session.getAttribute("RECOVERY_PHONE_ELEVATION_TS"));
+ }
+
+ @Test
+ public void testAuthChallengeElevatesTheSessionOnSuccess() {
+ enableRecoveryPhoneFeature();
+ ProfileEntity profile = new ProfileEntity();
+ profile.setEncryptedPassword("hashed");
+ when(profileEntityCacheManager.retrieve(ORCID)).thenReturn(profile);
+ when(encryptionManager.hashMatches("correct", "hashed")).thenReturn(true);
+ when(twoFactorAuthenticationManager.validateTwoFactorAuthForm(eq(ORCID), any(AuthChallenge.class))).thenReturn(true);
+
+ AuthChallenge form = new AuthChallenge();
+ form.setPassword("correct");
+ AuthChallenge result = controller.verifyRecoveryPhoneAuthChallenge(request, form);
+
+ assertTrue(result.isSuccess());
+ assertNotNull(session.getAttribute("RECOVERY_PHONE_ELEVATION_TS"));
+ }
+
+ @Test
+ public void testSendCodeNeedsAPassedChallenge() {
+ enableRecoveryPhoneFeature();
+
+ RecoveryPhoneSendCodeResponse response = controller.sendRecoveryPhoneCode(request, sendCodeRequest());
+
+ assertEquals(TwoFactorAuthenticationController.CHALLENGE_REQUIRED, response.getErrorCode());
+ verify(recoveryPhoneVerificationService, never()).sendCode(anyString(), any(RecoveryPhoneSendCodeRequest.class));
+ }
+
+ @Test
+ public void testSendCodeRefusesAnExpiredChallenge() {
+ enableRecoveryPhoneFeature();
+ session.setAttribute("RECOVERY_PHONE_ELEVATION_TS", System.currentTimeMillis() - (16 * 60 * 1000L));
+
+ assertEquals(TwoFactorAuthenticationController.CHALLENGE_REQUIRED,
+ controller.sendRecoveryPhoneCode(request, sendCodeRequest()).getErrorCode());
+ }
+
+ @Test
+ public void testSendCodeIsRefusedWhenTheFeatureIsOff() {
+ when(twoFactorAuthenticationManager.userUsing2FA(ORCID)).thenReturn(true);
+ elevateSession();
+
+ assertEquals(TwoFactorAuthenticationController.FEATURE_DISABLED,
+ controller.sendRecoveryPhoneCode(request, sendCodeRequest()).getErrorCode());
+ }
+
+ @Test
+ public void testSendCodeIsRefusedWhen2FAIsOff() {
+ togglzRule.enable(Features.TWO_FACTOR_RECOVERY_PHONE);
+ when(twoFactorAuthenticationManager.userUsing2FA(ORCID)).thenReturn(false);
+ elevateSession();
+
+ assertEquals(TwoFactorAuthenticationController.TWO_FACTOR_DISABLED,
+ controller.sendRecoveryPhoneCode(request, sendCodeRequest()).getErrorCode());
+ }
+
+ @Test
+ public void testSendCodeDelegatesOnceElevated() {
+ enableRecoveryPhoneFeature();
+ elevateSession();
+ when(recoveryPhoneVerificationService.sendCode(eq(ORCID), any(RecoveryPhoneSendCodeRequest.class)))
+ .thenReturn(RecoveryPhoneSendCodeResponse.success(30));
+
+ RecoveryPhoneSendCodeResponse response = controller.sendRecoveryPhoneCode(request, sendCodeRequest());
+
+ assertTrue(response.isSuccess());
+ assertEquals(30, response.getResendAfterSeconds());
+ }
+
+ @Test
+ public void testSaveReportsAFailedCodeAndKeepsTheElevation() {
+ enableRecoveryPhoneFeature();
+ elevateSession();
+ when(recoveryPhoneVerificationService.verifyCode(eq(ORCID), anyString(), anyString())).thenReturn("INVALID_CODE");
+
+ RecoveryPhoneSaveResponse response = controller.saveRecoveryPhone(request, saveRequest());
+
+ assertFalse(response.isSuccess());
+ assertEquals("INVALID_CODE", response.getErrorCode());
+ verify(recoveryPhoneManager, never()).saveRecoveryPhone(anyString(), anyString());
+ assertNotNull(session.getAttribute("RECOVERY_PHONE_ELEVATION_TS"));
+ }
+
+ @Test
+ public void testSaveStoresTheNumberAndClearsTheElevation() {
+ enableRecoveryPhoneFeature();
+ elevateSession();
+ java.util.Date now = new java.util.Date();
+ when(recoveryPhoneVerificationService.verifyCode(eq(ORCID), anyString(), anyString())).thenReturn(null);
+ when(recoveryPhoneVerificationService.normalize("+441234567890")).thenReturn("+441234567890");
+ when(recoveryPhoneManager.saveRecoveryPhone(ORCID, "+441234567890")).thenReturn(storedRecoveryPhone("7890", now, now));
+
+ RecoveryPhoneSaveResponse response = controller.saveRecoveryPhone(request, saveRequest());
+
+ assertTrue(response.isSuccess());
+ assertEquals("***********7890", response.getMaskedRecoveryPhoneNumber());
+ assertFalse(response.isRecoveryPhoneModified());
+ verify(recoveryPhoneManager).saveRecoveryPhone(ORCID, "+441234567890");
+ // Answered from the row the save wrote. A read after the write would go to the
+ // read-only pool, a replica on a deployed environment, and could still carry the
+ // previous number or none at all
+ verify(recoveryPhoneManager, never()).getRecoveryPhone(anyString());
+ assertNull(session.getAttribute("RECOVERY_PHONE_ELEVATION_TS"));
+ }
+
+ // Onboarding: the live 2FA code posted to register.json stands in for a
+ // challenge on the recovery phone step that follows it (R2.6)
+
+ @Test
+ public void testOnboardingElevatesTheSessionForTheRecoveryPhoneStep() {
+ TwoFactorAuthRegistration registration = new TwoFactorAuthRegistration();
+ registration.setVerificationCode("123456");
+ when(twoFactorAuthenticationManager.verificationCodeIsValid("123456", ORCID)).thenReturn(true);
+ when(twoFactorAuthenticationManager.enable2FA(ORCID)).thenReturn(Arrays.asList("code1", "code2"));
+
+ assertTrue(controller.validateVerificationCode(request, registration).isValid());
+ assertNotNull(session.getAttribute("RECOVERY_PHONE_ELEVATION_TS"));
+
+ // and that elevation is what carries step 2 of setup, with no second
+ // challenge in between
+ enableRecoveryPhoneFeature();
+ when(recoveryPhoneVerificationService.sendCode(eq(ORCID), any(RecoveryPhoneSendCodeRequest.class)))
+ .thenReturn(RecoveryPhoneSendCodeResponse.success(30));
+ RecoveryPhoneSendCodeRequest form = sendCodeRequest();
+ form.setContext(TwoFactorAuthenticationController.CONTEXT_ONBOARDING);
+
+ assertTrue(controller.sendRecoveryPhoneCode(request, form).isSuccess());
+ }
+
+ @Test
+ public void testOnboardingDoesNotElevateOnAnInvalidCode() {
+ TwoFactorAuthRegistration registration = new TwoFactorAuthRegistration();
+ registration.setVerificationCode("654321");
+ when(twoFactorAuthenticationManager.verificationCodeIsValid("654321", ORCID)).thenReturn(false);
+
+ assertFalse(controller.validateVerificationCode(request, registration).isValid());
+
+ assertNull(session.getAttribute("RECOVERY_PHONE_ELEVATION_TS"));
+ verify(twoFactorAuthenticationManager, never()).enable2FA(anyString());
+ }
+
+ @Test
+ public void testRegisterDoesNotElevateWhen2FAWasAlreadyOn() {
+ // The elevation belongs to step 1 of setup. On an account that is
+ // already using 2FA there is no step 2 to carry, and a time based code
+ // there must not stand in for the password challenge that guards
+ // changing a recovery number (R2.6)
+ enableRecoveryPhoneFeature();
+ TwoFactorAuthRegistration registration = new TwoFactorAuthRegistration();
+ registration.setVerificationCode("123456");
+ when(twoFactorAuthenticationManager.verificationCodeIsValid("123456", ORCID)).thenReturn(true);
+ when(twoFactorAuthenticationManager.enable2FA(ORCID)).thenReturn(Arrays.asList("code1", "code2"));
+
+ assertTrue(controller.validateVerificationCode(request, registration).isValid());
+ assertNull(session.getAttribute("RECOVERY_PHONE_ELEVATION_TS"));
+
+ // so the recovery phone endpoints still ask for the challenge
+ RecoveryPhoneSendCodeRequest form = sendCodeRequest();
+ form.setContext(TwoFactorAuthenticationController.CONTEXT_ONBOARDING);
+ assertEquals(TwoFactorAuthenticationController.CHALLENGE_REQUIRED,
+ controller.sendRecoveryPhoneCode(request, form).getErrorCode());
+ }
+
+ // Interstitial: a recent sign in stands in for a challenge, since the user
+ // completed 2FA to reach it and it has nowhere to put one (R6.3). The
+ // context is a claim the client makes, so every condition the interstitial
+ // is shown under is checked again here (R6.1, R7.4)
+
+ @Test
+ public void testSendCodeAcceptsAFreshLastLoginFromTheInterstitial() {
+ enableInterstitialFeature();
+ profileWithLastLogin(60 * 1000L);
+ when(recoveryPhoneVerificationService.sendCode(eq(ORCID), any(RecoveryPhoneSendCodeRequest.class)))
+ .thenReturn(RecoveryPhoneSendCodeResponse.success(30));
+ RecoveryPhoneSendCodeRequest form = sendCodeRequest();
+ form.setContext(TwoFactorAuthenticationController.CONTEXT_INTERSTITIAL);
+
+ RecoveryPhoneSendCodeResponse response = controller.sendRecoveryPhoneCode(request, form);
+
+ assertTrue(response.isSuccess());
+ // The cached profile says a day ago and the database says a minute ago:
+ // only the database has the sign in the user has just completed (R6.3)
+ verify(profileEntityManager).getLastLogin(ORCID);
+ // No challenge was passed: the sign in is what let this request in
+ assertNull(session.getAttribute("RECOVERY_PHONE_ELEVATION_TS"));
+ }
+
+ @Test
+ public void testSendCodeRefusesAStaleLastLoginFromTheInterstitial() {
+ enableInterstitialFeature();
+ profileWithLastLogin(16 * 60 * 1000L);
+ RecoveryPhoneSendCodeRequest form = sendCodeRequest();
+ form.setContext(TwoFactorAuthenticationController.CONTEXT_INTERSTITIAL);
+
+ assertEquals(TwoFactorAuthenticationController.CHALLENGE_REQUIRED,
+ controller.sendRecoveryPhoneCode(request, form).getErrorCode());
+ verify(recoveryPhoneVerificationService, never()).sendCode(anyString(), any(RecoveryPhoneSendCodeRequest.class));
+ }
+
+ @Test
+ public void testSendCodeRefusesTheInterstitialWithNoRecordedLastLogin() {
+ enableInterstitialFeature();
+ profileWithLastLogin(null);
+ RecoveryPhoneSendCodeRequest form = sendCodeRequest();
+ form.setContext(TwoFactorAuthenticationController.CONTEXT_INTERSTITIAL);
+
+ assertEquals(TwoFactorAuthenticationController.CHALLENGE_REQUIRED,
+ controller.sendRecoveryPhoneCode(request, form).getErrorCode());
+ verify(recoveryPhoneVerificationService, never()).sendCode(anyString(), any(RecoveryPhoneSendCodeRequest.class));
+ }
+
+ @Test
+ public void testSendCodeRefusesTheInterstitialWhenItsOwnFeatureIsOff() {
+ // TWO_FACTOR_RECOVERY_PHONE on its own is not enough: the relaxed path
+ // belongs to the interstitial and goes away with its flag (R7.4)
+ enableRecoveryPhoneFeature();
+ profileWithLastLogin(60 * 1000L);
+ RecoveryPhoneSendCodeRequest form = sendCodeRequest();
+ form.setContext(TwoFactorAuthenticationController.CONTEXT_INTERSTITIAL);
+
+ assertEquals(TwoFactorAuthenticationController.CHALLENGE_REQUIRED,
+ controller.sendRecoveryPhoneCode(request, form).getErrorCode());
+ verify(recoveryPhoneVerificationService, never()).sendCode(anyString(), any(RecoveryPhoneSendCodeRequest.class));
+ }
+
+ @Test
+ public void testSendCodeRefusesTheInterstitialWhenANumberIsAlreadyStored() {
+ // The interstitial is only offered when there is no number, so it can
+ // add a first one and never replace one. Replacing without the password
+ // would let a stolen session point the number at a phone it holds, and
+ // that number turns 2FA off at the next sign in (R6.1)
+ enableInterstitialFeature();
+ profileWithLastLogin(60 * 1000L);
+ java.util.Date now = new java.util.Date();
+ when(recoveryPhoneManager.getRecoveryPhone(ORCID)).thenReturn(storedRecoveryPhone("7890", now, now));
+ RecoveryPhoneSendCodeRequest form = sendCodeRequest();
+ form.setContext(TwoFactorAuthenticationController.CONTEXT_INTERSTITIAL);
+
+ assertEquals(TwoFactorAuthenticationController.CHALLENGE_REQUIRED,
+ controller.sendRecoveryPhoneCode(request, form).getErrorCode());
+ verify(recoveryPhoneVerificationService, never()).sendCode(anyString(), any(RecoveryPhoneSendCodeRequest.class));
+ }
+
+ @Test
+ public void testSaveRefusesTheInterstitialWhenANumberIsAlreadyStored() {
+ enableInterstitialFeature();
+ profileWithLastLogin(60 * 1000L);
+ java.util.Date now = new java.util.Date();
+ when(recoveryPhoneManager.getRecoveryPhone(ORCID)).thenReturn(storedRecoveryPhone("7890", now, now));
+ RecoveryPhoneSaveRequest form = saveRequest();
+ form.setContext(TwoFactorAuthenticationController.CONTEXT_INTERSTITIAL);
+
+ assertEquals(TwoFactorAuthenticationController.CHALLENGE_REQUIRED, controller.saveRecoveryPhone(request, form).getErrorCode());
+ verify(recoveryPhoneManager, never()).saveRecoveryPhone(anyString(), anyString());
+ }
+
+ @Test
+ public void testSendCodeRefusesTheInterstitialForAnImpersonatedSession() {
+ // A delegate or an admin switched into the record: their sign in is
+ // not the account owner's, and the interstitial is never shown to
+ // them in the first place (R6.1)
+ enableInterstitialFeature();
+ profileWithLastLogin(60 * 1000L);
+ doReturn("0000-0000-0000-0002").when(controller).getRealUserOrcid();
+ RecoveryPhoneSendCodeRequest form = sendCodeRequest();
+ form.setContext(TwoFactorAuthenticationController.CONTEXT_INTERSTITIAL);
+
+ assertEquals(TwoFactorAuthenticationController.CHALLENGE_REQUIRED,
+ controller.sendRecoveryPhoneCode(request, form).getErrorCode());
+ verify(recoveryPhoneVerificationService, never()).sendCode(anyString(), any(RecoveryPhoneSendCodeRequest.class));
+ }
+
+ @Test
+ public void testSendCodeStillDemandsTheChallengeOutsideTheInterstitial() {
+ enableInterstitialFeature();
+ // A sign in this fresh would let the interstitial through
+ profileWithLastLogin(60 * 1000L);
+
+ RecoveryPhoneSendCodeRequest settings = sendCodeRequest();
+ settings.setContext(TwoFactorAuthenticationController.CONTEXT_SETTINGS);
+ assertEquals(TwoFactorAuthenticationController.CHALLENGE_REQUIRED,
+ controller.sendRecoveryPhoneCode(request, settings).getErrorCode());
+
+ // An unstated context is settings, the strictest of the three
+ assertEquals(TwoFactorAuthenticationController.CHALLENGE_REQUIRED,
+ controller.sendRecoveryPhoneCode(request, sendCodeRequest()).getErrorCode());
+
+ // Onboarding has its own elevation and does not ride on the sign in
+ RecoveryPhoneSendCodeRequest onboarding = sendCodeRequest();
+ onboarding.setContext(TwoFactorAuthenticationController.CONTEXT_ONBOARDING);
+ assertEquals(TwoFactorAuthenticationController.CHALLENGE_REQUIRED,
+ controller.sendRecoveryPhoneCode(request, onboarding).getErrorCode());
+
+ verify(recoveryPhoneVerificationService, never()).sendCode(anyString(), any(RecoveryPhoneSendCodeRequest.class));
+ }
+
+ @Test
+ public void testSaveAcceptsAFreshLastLoginFromTheInterstitial() {
+ enableInterstitialFeature();
+ profileWithLastLogin(60 * 1000L);
+ java.util.Date now = new java.util.Date();
+ when(recoveryPhoneVerificationService.verifyCode(eq(ORCID), anyString(), anyString())).thenReturn(null);
+ when(recoveryPhoneVerificationService.normalize("+441234567890")).thenReturn("+441234567890");
+ // Nothing stored while the guard looks; the save answers with what it wrote
+ when(recoveryPhoneManager.getRecoveryPhone(ORCID)).thenReturn(null);
+ when(recoveryPhoneManager.saveRecoveryPhone(ORCID, "+441234567890")).thenReturn(storedRecoveryPhone("7890", now, now));
+ RecoveryPhoneSaveRequest form = saveRequest();
+ form.setContext(TwoFactorAuthenticationController.CONTEXT_INTERSTITIAL);
+
+ assertTrue(controller.saveRecoveryPhone(request, form).isSuccess());
+ verify(recoveryPhoneManager).saveRecoveryPhone(ORCID, "+441234567890");
+ }
+
+ // The recovery phone as the authentication challenge itself (R5)
+
+ @Test
+ public void testChallengeSendCodeUsesTheStoredNumberAndReturnsTheMask() {
+ enableRecoveryPhoneFeature();
+ java.util.Date now = new java.util.Date();
+ when(recoveryPhoneManager.getRecoveryPhone(ORCID)).thenReturn(storedRecoveryPhone("7890", now, now));
+ when(recoveryPhoneManager.getDecryptedPhoneNumber(ORCID)).thenReturn("+441234567890");
+ when(recoveryPhoneVerificationService.sendCode(eq(ORCID), any(RecoveryPhoneSendCodeRequest.class)))
+ .thenReturn(RecoveryPhoneSendCodeResponse.success(30));
+
+ // No elevation on this session: this endpoint is the challenge (R5.1)
+ RecoveryPhoneChallengeSendCodeResponse response = controller.sendRecoveryPhoneChallengeCode();
+
+ assertTrue(response.isSuccess());
+ assertEquals(30, response.getResendAfterSeconds());
+ assertEquals("***********7890", response.getMaskedRecoveryPhoneNumber());
+
+ ArgumentCaptor sentRequest = ArgumentCaptor.forClass(RecoveryPhoneSendCodeRequest.class);
+ verify(recoveryPhoneVerificationService).sendCode(eq(ORCID), sentRequest.capture());
+ assertEquals("+441234567890", sentRequest.getValue().getPhoneNumber());
+ // The locale is the server's, not something the client can set here
+ assertEquals("en", sentRequest.getValue().getLocale());
+ }
+
+ @Test
+ public void testChallengeSendCodeIsRefusedWhenNoNumberIsStored() {
+ enableRecoveryPhoneFeature();
+ when(recoveryPhoneManager.getRecoveryPhone(ORCID)).thenReturn(null);
+
+ RecoveryPhoneChallengeSendCodeResponse response = controller.sendRecoveryPhoneChallengeCode();
+
+ assertFalse(response.isSuccess());
+ assertEquals(TwoFactorAuthenticationController.NO_RECOVERY_PHONE, response.getErrorCode());
+ assertNull(response.getMaskedRecoveryPhoneNumber());
+ verify(recoveryPhoneVerificationService, never()).sendCode(anyString(), any(RecoveryPhoneSendCodeRequest.class));
+ }
+
+ @Test
+ public void testChallengeSendCodeIsRefusedWhenTheFeatureIsOff() {
+ when(twoFactorAuthenticationManager.userUsing2FA(ORCID)).thenReturn(true);
+
+ RecoveryPhoneChallengeSendCodeResponse response = controller.sendRecoveryPhoneChallengeCode();
+
+ assertFalse(response.isSuccess());
+ assertEquals(TwoFactorAuthenticationController.FEATURE_DISABLED, response.getErrorCode());
+ verify(recoveryPhoneManager, never()).getRecoveryPhone(anyString());
+ verify(recoveryPhoneVerificationService, never()).sendCode(anyString(), any(RecoveryPhoneSendCodeRequest.class));
+ }
+
+ @Test
+ public void testChallengeSendCodeIsRefusedWhen2FAIsOff() {
+ togglzRule.enable(Features.TWO_FACTOR_RECOVERY_PHONE);
+ when(twoFactorAuthenticationManager.userUsing2FA(ORCID)).thenReturn(false);
+
+ assertEquals(TwoFactorAuthenticationController.TWO_FACTOR_DISABLED,
+ controller.sendRecoveryPhoneChallengeCode().getErrorCode());
+ verify(recoveryPhoneVerificationService, never()).sendCode(anyString(), any(RecoveryPhoneSendCodeRequest.class));
+ }
+
+ @Test
+ public void testChallengeVerifyRejectsAWrongPasswordAndDisablesNothing() {
+ enableRecoveryPhoneFeature();
+ profileWithLastLogin(60 * 1000L);
+ when(encryptionManager.hashMatches("nope", "hashed")).thenReturn(false);
+
+ AuthChallenge result = controller.verifyRecoveryPhoneChallengeCode(request, challengeVerifyRequest("nope", "123456"));
+
+ assertFalse(result.isSuccess());
+ assertTrue(result.isInvalidPassword());
+ assertTrue(result.getErrors().contains(TwoFactorAuthenticationController.INVALID_PASSWORD));
+ // The code is never looked at, so a wrong password cannot spend its attempts
+ verify(recoveryPhoneVerificationService, never()).verifyCode(anyString(), anyString(), anyString());
+ verify(twoFactorAuthenticationManager, never()).disable2FAByRecoveryPhone(anyString());
+ verify(recordEmailSender, never()).send2FADisabledEmail(anyString());
+ }
+
+ @Test
+ public void testChallengeVerifyRejectsAWrongCodeAndDisablesNothing() {
+ enableRecoveryPhoneFeature();
+ profileWithLastLogin(60 * 1000L);
+ when(encryptionManager.hashMatches("correct", "hashed")).thenReturn(true);
+ when(recoveryPhoneManager.getDecryptedPhoneNumber(ORCID)).thenReturn("+441234567890");
+ when(recoveryPhoneVerificationService.verifyCode(ORCID, "+441234567890", "000000"))
+ .thenReturn(RecoveryPhoneVerificationService.INVALID_CODE);
+
+ AuthChallenge result = controller.verifyRecoveryPhoneChallengeCode(request, challengeVerifyRequest("correct", "000000"));
+
+ assertFalse(result.isSuccess());
+ assertFalse(result.isInvalidPassword());
+ assertTrue(result.getErrors().contains(RecoveryPhoneVerificationService.INVALID_CODE));
+ verify(twoFactorAuthenticationManager, never()).disable2FAByRecoveryPhone(anyString());
+ verify(recordEmailSender, never()).send2FADisabledEmail(anyString());
+ verify(profileEntityCacheManager, never()).remove(anyString());
+ }
+
+ @Test
+ public void testChallengeVerifyIsRefusedWhenNoNumberIsStored() {
+ enableRecoveryPhoneFeature();
+ profileWithLastLogin(60 * 1000L);
+ when(encryptionManager.hashMatches("correct", "hashed")).thenReturn(true);
+ when(recoveryPhoneManager.getDecryptedPhoneNumber(ORCID)).thenReturn(null);
+
+ AuthChallenge result = controller.verifyRecoveryPhoneChallengeCode(request, challengeVerifyRequest("correct", "123456"));
+
+ assertFalse(result.isSuccess());
+ assertTrue(result.getErrors().contains(TwoFactorAuthenticationController.NO_RECOVERY_PHONE));
+ verify(recoveryPhoneVerificationService, never()).verifyCode(anyString(), anyString(), anyString());
+ verify(twoFactorAuthenticationManager, never()).disable2FAByRecoveryPhone(anyString());
+ }
+
+ @Test
+ public void testChallengeVerifyIsRefusedWhenTheFeatureIsOff() {
+ when(twoFactorAuthenticationManager.userUsing2FA(ORCID)).thenReturn(true);
+
+ AuthChallenge result = controller.verifyRecoveryPhoneChallengeCode(request, challengeVerifyRequest("correct", "123456"));
+
+ assertFalse(result.isSuccess());
+ assertTrue(result.getErrors().contains(TwoFactorAuthenticationController.FEATURE_DISABLED));
+ verify(encryptionManager, never()).hashMatches(anyString(), anyString());
+ verify(twoFactorAuthenticationManager, never()).disable2FAByRecoveryPhone(anyString());
+ }
+
+ @Test
+ public void testChallengeVerifyDisables2FAOnAGoodCode() {
+ enableRecoveryPhoneFeature();
+ // An elevation from an earlier challenge has nothing left to guard
+ elevateSession();
+ profileWithLastLogin(60 * 1000L);
+ when(encryptionManager.hashMatches("correct", "hashed")).thenReturn(true);
+ when(recoveryPhoneManager.getDecryptedPhoneNumber(ORCID)).thenReturn("+441234567890");
+ when(recoveryPhoneVerificationService.verifyCode(ORCID, "+441234567890", "123456")).thenReturn(null);
+
+ AuthChallenge result = controller.verifyRecoveryPhoneChallengeCode(request, challengeVerifyRequest("correct", "123456"));
+
+ assertTrue(result.isSuccess());
+ assertTrue(result.getErrors().isEmpty());
+ // The code is checked against the stored number, never one a client sent
+ verify(recoveryPhoneVerificationService).verifyCode(ORCID, "+441234567890", "123456");
+ verify(twoFactorAuthenticationManager).disable2FAByRecoveryPhone(ORCID);
+ verify(recordEmailSender).send2FADisabledEmail(ORCID);
+ verify(profileEntityCacheManager).remove(ORCID);
+ assertNull(session.getAttribute("RECOVERY_PHONE_ELEVATION_TS"));
+ }
+
+ @Test
+ public void testChallengeVerifySucceedsWhenTheEmailCannotBeSent() {
+ // The email only reports something that has already happened and
+ // cannot be undone. Telling the user the challenge failed while their
+ // 2FA is off, and leaving the cache saying it is still on, is worse
+ // than a missing notification (R5.3)
+ enableRecoveryPhoneFeature();
+ profileWithLastLogin(60 * 1000L);
+ when(encryptionManager.hashMatches("correct", "hashed")).thenReturn(true);
+ when(recoveryPhoneManager.getDecryptedPhoneNumber(ORCID)).thenReturn("+441234567890");
+ when(recoveryPhoneVerificationService.verifyCode(ORCID, "+441234567890", "123456")).thenReturn(null);
+ doThrow(new RuntimeException("the mail server is down")).when(recordEmailSender).send2FADisabledEmail(ORCID);
+
+ AuthChallenge result = controller.verifyRecoveryPhoneChallengeCode(request, challengeVerifyRequest("correct", "123456"));
+
+ assertTrue(result.isSuccess());
+ assertTrue(result.getErrors().isEmpty());
+ verify(twoFactorAuthenticationManager).disable2FAByRecoveryPhone(ORCID);
+ verify(profileEntityCacheManager).remove(ORCID);
+ }
}