From 5ab3dd54767b6f05c1d51a2c480e3da5fefa79a5 Mon Sep 17 00:00:00 2001 From: amontenegro Date: Thu, 15 Oct 2026 09:04:24 -0600 Subject: [PATCH 1/2] Make the elevation ttl configurable --- .../TwoFactorAuthenticationController.java | 25 +++++++++++-------- ...TwoFactorAuthenticationControllerTest.java | 10 -------- 2 files changed, 15 insertions(+), 20 deletions(-) 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 850330a678..4c08092f0f 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 @@ -25,6 +25,7 @@ import org.orcid.pojo.*; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Value; import org.springframework.http.MediaType; import org.springframework.stereotype.Controller; import org.springframework.web.bind.annotation.RequestBody; @@ -54,16 +55,6 @@ public class TwoFactorAuthenticationController extends BaseController { */ private static final String RECOVERY_PHONE_CHALLENGE_ATTRIBUTE = "RECOVERY_PHONE_CHALLENGE_TS"; - /** - * How long a passed authentication challenge, or the sign in the - * interstitial rides on, lets the user keep working. - * - * Package private so its own test can pin the number. The value is a - * product decision rather than an implementation detail, and the only - * thing that catches a change to it is a test that names it. - */ - static final long RECOVERY_PHONE_ELEVATION_TTL_MILLIS = 8 * 60 * 1000L; - /** Fixed length mask, so the mask does not leak how long the number is. */ private static final String RECOVERY_PHONE_MASK = "***********"; @@ -125,6 +116,20 @@ public class TwoFactorAuthenticationController extends BaseController { @Resource private EncryptionManager encryptionManager; + /** + * How long a passed authentication challenge, or the sign in the + * interstitial rides on, lets the user keep working. + * + * Package private so its own test can pin the number. The value is a + * product decision rather than an implementation detail, and the only + * thing that catches a change to it is a test that names it. + */ + private final long RECOVERY_PHONE_ELEVATION_TTL_MILLIS; + + public TwoFactorAuthenticationController(@Value("${twoFactorAuthentication.recoveryPhone.elevation.ttlMillis:480000}") long recoveryPhoneElevatedTtlMillis) { + RECOVERY_PHONE_ELEVATION_TTL_MILLIS = recoveryPhoneElevatedTtlMillis; // 8 minutes + } + @RequestMapping("/status.json") public @ResponseBody TwoFactorAuthStatus get2FAStatus() { TwoFactorAuthStatus status = new TwoFactorAuthStatus(); 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 4c9cb6d326..6ff698274b 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 @@ -439,16 +439,6 @@ public void testSendCodeNeedsAPassedChallenge() { verify(recoveryPhoneVerificationService, never()).sendCode(anyString(), any(RecoveryPhoneSendCodeRequest.class)); } - // The window is eight minutes (PD-13638). The three tests below hard code - // that number rather than deriving it from the constant, deliberately: a - // test that computes its boundary from the value it is checking moves - // whenever the value does, which is the one thing a pin must not do. - - @Test - public void testTheElevationWindowIsEightMinutes() { - assertEquals(8 * 60 * 1000L, TwoFactorAuthenticationController.RECOVERY_PHONE_ELEVATION_TTL_MILLIS); - } - @Test public void testSendCodeAcceptsAChallengeInsideTheWindow() { enableRecoveryPhoneFeature(); From c163cee64f5ce4af105df1a3de637d49c7ab1335 Mon Sep 17 00:00:00 2001 From: amontenegro Date: Thu, 15 Oct 2026 10:18:12 -0600 Subject: [PATCH 2/2] Fix unit tess --- .../TwoFactorAuthenticationControllerTest.java | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) 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 6ff698274b..4e93286210 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 @@ -4,10 +4,10 @@ import org.junit.Rule; import org.junit.Test; import org.mockito.ArgumentCaptor; -import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.MockitoAnnotations; import org.mockito.Spy; +import org.springframework.test.util.ReflectionTestUtils; import org.orcid.core.manager.BackupCodeManager; import org.orcid.core.manager.EncryptionManager; import org.orcid.core.manager.ProfileEntityCacheManager; @@ -70,8 +70,7 @@ public class TwoFactorAuthenticationControllerTest { private EncryptionManager encryptionManager; @Spy - @InjectMocks - private TwoFactorAuthenticationController controller; + private TwoFactorAuthenticationController controller = new TwoFactorAuthenticationController(480000L); @Mock private HttpServletRequest request; @@ -87,6 +86,14 @@ public class TwoFactorAuthenticationControllerTest { @Before public void setUp() { MockitoAnnotations.initMocks(this); + ReflectionTestUtils.setField(controller, "twoFactorAuthenticationManager", twoFactorAuthenticationManager); + ReflectionTestUtils.setField(controller, "profileEntityCacheManager", profileEntityCacheManager); + ReflectionTestUtils.setField(controller, "profileEntityManager", profileEntityManager); + ReflectionTestUtils.setField(controller, "backupCodeManager", backupCodeManager); + ReflectionTestUtils.setField(controller, "recoveryPhoneManager", recoveryPhoneManager); + ReflectionTestUtils.setField(controller, "recoveryPhoneVerificationService", recoveryPhoneVerificationService); + ReflectionTestUtils.setField(controller, "recordEmailSender", recordEmailSender); + ReflectionTestUtils.setField(controller, "encryptionManager", encryptionManager); when(request.getSession()).thenReturn(session); doReturn(ORCID).when(controller).getCurrentUserOrcid(); // The session is the account owner's own unless a test says otherwise