Lmendoa/pd 14333 pd 14423 pd 14449 pd 14450 recovery phone

orcid-core/src/main/java/org/orcid/core/manager/RecoveryPhoneManager.java

@@ -12,11 +12,14 @@ public interface RecoveryPhoneManager {
     /**
      * 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.
+     * This is the only path that decrypts the stored number. It exists for the
+     * flows that have to send a text to it without the user re-typing it, for
+     * the ones that check a code against it, and for the one response allowed
+     * to carry it: the manage page's prefilled field, owner only, behind a
+     * passed authentication challenge (F4.2).
      * 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.
+     * or value, or returned over HTTP anywhere else (R1.2): everything else
+     * that reaches the user carries the last four digits only.
      */
     String getDecryptedPhoneNumber(String orcid);
 

orcid-core/src/main/java/org/orcid/core/manager/impl/RecoveryPhoneManagerImpl.java

@@ -45,8 +45,9 @@ public String getDecryptedPhoneNumber(String orcid) {
             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.
+        // that one for the mask and the dates alone; only the flows that have to text the number, and
+        // the manage page's owner-only read behind a passed challenge (F4.2), pay for the crypto. The
+        // result must not reach a log line, a RUM attribute or any other HTTP response.
         return encryptionManager.decryptForInternalUse(entity.getEncryptedPhoneNumber());
     }
 

orcid-core/src/main/java/org/orcid/core/togglz/Features.java

@@ -121,6 +121,9 @@ public enum Features implements Feature {
     @Label("Login - recovery phone interstitial")
     LOGIN_RECOVERY_PHONE_INTERSTITIAL,
 
+    @Label("OAUTH - recovery phone interstitial")
+    OAUTH_RECOVERY_PHONE_INTERSTITIAL,
+
     @Label("Mandatory password reset: block password sign in and show the reset notice for records flagged with force_password_reset")
     FORCE_PASSWORD_RESET;
     public boolean isActive() {

orcid-web/src/main/java/org/orcid/frontend/recoveryphone/RecoveryPhoneNumberResponse.java

@@ -0,0 +1,55 @@
+package org.orcid.frontend.recoveryphone;
+
+/**
+ * The stored recovery number in full, in E.164 form, for the one field that
+ * shows it: the manage page's phone field, which starts from the number on
+ * file (F4.2). This is the only response that carries the number in the
+ * clear; every other one carries the mask `***********NNNN` (R1.2).
+ */
+public class RecoveryPhoneNumberResponse {
+
+    private boolean success;
+
+    private String errorCode;
+
+    private String phoneNumber;
+
+    public static RecoveryPhoneNumberResponse failure(String errorCode) {
+        RecoveryPhoneNumberResponse response = new RecoveryPhoneNumberResponse();
+        response.setSuccess(false);
+        response.setErrorCode(errorCode);
+        return response;
+    }
+
+    public static RecoveryPhoneNumberResponse success(String phoneNumber) {
+        RecoveryPhoneNumberResponse response = new RecoveryPhoneNumberResponse();
+        response.setSuccess(true);
+        response.setPhoneNumber(phoneNumber);
+        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 getPhoneNumber() {
+        return phoneNumber;
+    }
+
+    public void setPhoneNumber(String phoneNumber) {
+        this.phoneNumber = phoneNumber;
+    }
+
+}

orcid-web/src/main/java/org/orcid/frontend/recoveryphone/RecoveryPhoneSigninStatusRequest.java

@@ -0,0 +1,29 @@
+package org.orcid.frontend.recoveryphone;
+
+/**
+ * Asks whether the account the credentials belong to has a recovery number,
+ * so the 2FA step of sign in knows which way out to offer (F1.2).
+ */
+public class RecoveryPhoneSigninStatusRequest {
+
+    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;
+    }
+
+}

orcid-web/src/main/java/org/orcid/frontend/recoveryphone/RecoveryPhoneSigninStatusResponse.java

@@ -0,0 +1,55 @@
+package org.orcid.frontend.recoveryphone;
+
+/**
+ * Whether the account behind the credentials has a recovery number stored
+ * (F1.2). It carries that one boolean and nothing else - not the mask, not the
+ * dates - because the screen asking only has to choose between offering a
+ * text and offering the help centre.
+ */
+public class RecoveryPhoneSigninStatusResponse {
+
+    private boolean success;
+
+    private String errorCode;
+
+    private boolean hasRecoveryPhone;
+
+    public static RecoveryPhoneSigninStatusResponse failure(String errorCode) {
+        RecoveryPhoneSigninStatusResponse response = new RecoveryPhoneSigninStatusResponse();
+        response.setSuccess(false);
+        response.setErrorCode(errorCode);
+        return response;
+    }
+
+    public static RecoveryPhoneSigninStatusResponse success(boolean hasRecoveryPhone) {
+        RecoveryPhoneSigninStatusResponse response = new RecoveryPhoneSigninStatusResponse();
+        response.setSuccess(true);
+        response.setHasRecoveryPhone(hasRecoveryPhone);
+        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 boolean isHasRecoveryPhone() {
+        return hasRecoveryPhone;
+    }
+
+    public void setHasRecoveryPhone(boolean hasRecoveryPhone) {
+        this.hasRecoveryPhone = hasRecoveryPhone;
+    }
+
+}

orcid-web/src/main/java/org/orcid/frontend/web/controllers/RecoveryPhoneSigninController.java

@@ -17,6 +17,8 @@
 import org.orcid.frontend.recoveryphone.RecoveryPhoneSendCodeResponse;
 import org.orcid.frontend.recoveryphone.RecoveryPhoneSigninSendCodeRequest;
 import org.orcid.frontend.recoveryphone.RecoveryPhoneSigninSendCodeResponse;
+import org.orcid.frontend.recoveryphone.RecoveryPhoneSigninStatusRequest;
+import org.orcid.frontend.recoveryphone.RecoveryPhoneSigninStatusResponse;
 import org.orcid.frontend.recoveryphone.RecoveryPhoneSigninVerifyRequest;
 import org.orcid.frontend.recoveryphone.RecoveryPhoneSigninVerifyResponse;
 import org.orcid.frontend.recoveryphone.RecoveryPhoneVerificationService;
@@ -99,6 +101,39 @@ public class RecoveryPhoneSigninController extends BaseController {
     @Resource
     private RecordEmailSender recordEmailSender;
 
+    /**
+     * Tells the 2FA step of sign in whether the account behind the credentials
+     * has a recovery number, so the step offers to text one only where there is
+     * a number to text, and the help centre everywhere else (F1.2).
+     *
+     * The answer goes only to a caller holding the password: the check is the
+     * one sendCode makes, through the same provider, so a wrong password here
+     * counts toward the sign in lockout like any other. The screen asks right
+     * after the sign in has accepted that password and said 2FA is on, so all
+     * this adds for the password holder is whether a number is stored.
+     */
+    @RequestMapping(value = "/status.json", method = RequestMethod.POST)
+    public @ResponseBody RecoveryPhoneSigninStatusResponse status(HttpServletRequest request,
+            @RequestBody RecoveryPhoneSigninStatusRequest form) {
+        if (!Features.TWO_FACTOR_RECOVERY_PHONE.isActive()) {
+            return RecoveryPhoneSigninStatusResponse.failure(FEATURE_DISABLED);
+        }
+
+        String credentialsFailure = verifyCredentials(request, form.getUsername(), form.getPassword());
+        if (credentialsFailure != null) {
+            return RecoveryPhoneSigninStatusResponse.failure(credentialsFailure);
+        }
+
+        String orcid = resolveOrcid(form.getUsername());
+        if (orcid == null) {
+            return RecoveryPhoneSigninStatusResponse.failure(BAD_CREDENTIALS);
+        }
+
+        // The read that costs no decryption: whether there is a number is all
+        // this answers
+        return RecoveryPhoneSigninStatusResponse.success(recoveryPhoneManager.getRecoveryPhone(orcid) != null);
+    }
+
     /**
      * Sends a fresh code to the recovery number stored on the account the given
      * credentials belong to (R3.2).

orcid-web/src/main/java/org/orcid/frontend/web/controllers/TwoFactorAuthenticationController.java

@@ -15,6 +15,7 @@
 import org.orcid.frontend.email.RecordEmailSender;
 import org.orcid.frontend.recoveryphone.RecoveryPhoneChallengeSendCodeResponse;
 import org.orcid.frontend.recoveryphone.RecoveryPhoneChallengeVerifyRequest;
+import org.orcid.frontend.recoveryphone.RecoveryPhoneNumberResponse;
 import org.orcid.frontend.recoveryphone.RecoveryPhoneSaveRequest;
 import org.orcid.frontend.recoveryphone.RecoveryPhoneSaveResponse;
 import org.orcid.frontend.recoveryphone.RecoveryPhoneSendCodeRequest;
@@ -46,6 +47,13 @@ public class TwoFactorAuthenticationController extends BaseController {
 
     private static final String RECOVERY_PHONE_ELEVATION_ATTRIBUTE = "RECOVERY_PHONE_ELEVATION_TS";
 
+    /**
+     * When the account verification challenge was last passed on this session.
+     * Kept apart from the elevation, which 2FA setup also grants, because only a
+     * passed challenge lets the number on file be read back (F4.2).
+     */
+    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.
@@ -78,6 +86,12 @@ public class TwoFactorAuthenticationController extends BaseController {
     /** The password given with a challenge is not the account's password. */
     static final String INVALID_PASSWORD = "INVALID_PASSWORD";
 
+    /**
+     * The session's real user is not the record's owner: a delegate, or an
+     * admin switched into the record.
+     */
+    static final String NOT_ACCOUNT_OWNER = "NOT_ACCOUNT_OWNER";
+
     /**
      * 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
@@ -153,11 +167,47 @@ public class TwoFactorAuthenticationController extends BaseController {
             return form;
         }
 
-        request.getSession().setAttribute(RECOVERY_PHONE_ELEVATION_ATTRIBUTE, System.currentTimeMillis());
+        long passedAt = System.currentTimeMillis();
+        request.getSession().setAttribute(RECOVERY_PHONE_ELEVATION_ATTRIBUTE, passedAt);
+        request.getSession().setAttribute(RECOVERY_PHONE_CHALLENGE_ATTRIBUTE, passedAt);
         form.setSuccess(true);
         return form;
     }
 
+    /**
+     * Hands the stored recovery number back in full for the manage page, whose
+     * phone field starts from the number on file rather than empty (F4.2).
+     *
+     * Who may see it is decided here and nowhere else. Only the account owner -
+     * never a delegate, never an admin switched into the record - and only on
+     * a session that passed the authentication challenge inside the elevation
+     * window. The interstitial's recent sign in does not count: the
+     * interstitial only ever adds a first number, so it has none to show. A
+     * POST, answered no-store, so no cache along the way keeps a copy.
+     */
+    @RequestMapping(value = "/recoveryPhone/number.json", method = RequestMethod.POST)
+    public @ResponseBody RecoveryPhoneNumberResponse getRecoveryPhoneNumber(HttpServletRequest request, HttpServletResponse response) {
+        response.setHeader("Cache-Control", "no-cache, no-store, must-revalidate");
+        String orcid = getCurrentUserOrcid();
+        if (!Features.TWO_FACTOR_RECOVERY_PHONE.isActive()) {
+            return RecoveryPhoneNumberResponse.failure(FEATURE_DISABLED);
+        }
+        if (!orcid.equals(getRealUserOrcid())) {
+            return RecoveryPhoneNumberResponse.failure(NOT_ACCOUNT_OWNER);
+        }
+        if (!twoFactorAuthenticationManager.userUsing2FA(orcid)) {
+            return RecoveryPhoneNumberResponse.failure(TWO_FACTOR_DISABLED);
+        }
+        if (!challengePassedRecently(request)) {
+            return RecoveryPhoneNumberResponse.failure(CHALLENGE_REQUIRED);
+        }
+        String phoneNumber = recoveryPhoneManager.getDecryptedPhoneNumber(orcid);
+        if (phoneNumber == null) {
+            return RecoveryPhoneNumberResponse.failure(NO_RECOVERY_PHONE);
+        }
+        return RecoveryPhoneNumberResponse.success(phoneNumber);
+    }
+
     @RequestMapping(value = "/recoveryPhone/sendCode.json", method = RequestMethod.POST)
     public @ResponseBody RecoveryPhoneSendCodeResponse sendRecoveryPhoneCode(HttpServletRequest request,
             @RequestBody RecoveryPhoneSendCodeRequest form) {
@@ -185,6 +235,7 @@ public class TwoFactorAuthenticationController extends BaseController {
         String phoneE164 = recoveryPhoneVerificationService.normalize(form.getPhoneNumber());
         RecoveryPhone saved = recoveryPhoneManager.saveRecoveryPhone(orcid, phoneE164);
         request.getSession().removeAttribute(RECOVERY_PHONE_ELEVATION_ATTRIBUTE);
+        request.getSession().removeAttribute(RECOVERY_PHONE_CHALLENGE_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
@@ -317,6 +368,7 @@ public class TwoFactorAuthenticationController extends BaseController {
         // 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);
+        request.getSession().removeAttribute(RECOVERY_PHONE_CHALLENGE_ATTRIBUTE);
         result.setSuccess(true);
         return result;
     }
@@ -340,6 +392,19 @@ private String guardRecoveryPhoneRequest(HttpServletRequest request, String orci
         return CHALLENGE_REQUIRED;
     }
 
+    /**
+     * The account verification challenge itself passed on this session within
+     * the window. Narrower than {@link #sessionIsElevated}: 2FA setup elevates
+     * the session too, and that grant must not read the number back (F4.2).
+     */
+    private boolean challengePassedRecently(HttpServletRequest request) {
+        Object passedAt = request.getSession().getAttribute(RECOVERY_PHONE_CHALLENGE_ATTRIBUTE);
+        if (!(passedAt instanceof Long)) {
+            return false;
+        }
+        return System.currentTimeMillis() - (Long) passedAt <= RECOVERY_PHONE_ELEVATION_TTL_MILLIS;
+    }
+
     /** A challenge passed on this session within the elevation window. */
     private boolean sessionIsElevated(HttpServletRequest request) {
         Object elevatedAt = request.getSession().getAttribute(RECOVERY_PHONE_ELEVATION_ATTRIBUTE);
@@ -371,8 +436,12 @@ 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)
+        if (!Features.LOGIN_RECOVERY_PHONE_INTERSTITIAL.isActive() && !Features.OAUTH_RECOVERY_PHONE_INTERSTITIAL.isActive()) {
+            // The relaxed path dies with the interstitial that justifies it
+            // (R7.4). Either flow's flag keeps it open: which flow the form is
+            // in is as much a claim of the client's as the context is, so the
+            // server cannot tell the two apart, and both rest on the same
+            // proof - a sign in completed inside the window (F2.2)
             return false;
         }
         if (recoveryPhoneManager.getRecoveryPhone(orcid) != null) {