fix(otp): sign in by phone only with a verified phone

.changeset/verified-phone-for-sign-in.md

@@ -0,0 +1,10 @@
+---
+'seamless-auth-api': patch
+---
+
+Only a verified phone number signs in by phone OTP.
+
+- `POST /login` finds an account by phone only when that phone is verified, and offers `phone_otp` only for a verified phone.
+- `/otp/generate-login-phone-otp` answers an unverified phone as it answers an account with no phone (`400`), and `/otp/verify-login-phone-otp` refuses it (`401`).
+- The registration phone OTP routes (`/otp/generate-phone-otp`, `/otp/verify-phone-otp`) no longer verify a phone on an account that is already verified. Sending answers `200` and sends nothing; verifying answers `401`. Add or verify a phone on a verified account with an access session through `/registration/phone`.
+- The policy dry run counts an unverified phone as no phone.

src/controllers/authentication.ts

@@ -270,7 +270,7 @@ export const login = async (req: Request, res: Response) => {
   } else if (isValidPhoneNumber(identifier) && normalizedIdentifier) {
     try {
       user = await User.findOne({
-        where: { phone: normalizedIdentifier },
+        where: { phone: normalizedIdentifier, phoneVerified: true },
       });
       identifierType = 'phone';
       decoyIdentifier = normalizedIdentifier;

src/controllers/otp.ts

@@ -226,6 +226,43 @@ export const sendLoginPhoneOTP = async (req: Request, res: Response) => {
     return;
   }
 
+  const user = (req as AuthenticatedRequest).user;
+
+  // Answered as for an account with no phone, which is what `/login` offered it.
+  if (user.phone && !user.phoneVerified) {
+    await AuthEventService.log({
+      userId: user.id,
+      type: 'otp_suspicious',
+      req,
+      metadata: { reason: 'Login phone OTP requested for an unverified phone.' },
+    });
+    return res.status(400).json({ error: 'Invalid data' });
+  }
+
+  return sendPhoneOTP(req, res);
+};
+
+/**
+ * Proving a phone on an account that already signs in changes what it can sign in with,
+ * which takes an access session (`/registration/phone`). The pre-auth token here can come
+ * from an email address alone, so for a verified account nothing is sent and the answer
+ * is the one a decoy gives.
+ */
+export const sendRegistrationPhoneOTP = async (req: Request, res: Response) => {
+  const authReq = req as AuthenticatedRequest;
+  const user = authReq.user;
+
+  if (user.verified && user.phone) {
+    await AuthEventService.log({
+      userId: user.id,
+      type: 'otp_suspicious',
+      req,
+      metadata: { reason: 'Registration phone OTP requested for a verified account.' },
+    });
+    const token = await signEphemeralToken(user.id, authReq.attemptId);
+    return res.status(200).json({ message: 'success', token });
+  }
+
   return sendPhoneOTP(req, res);
 };
 
@@ -256,6 +293,18 @@ export const verifyPhoneNumber = async (req: Request, res: Response) => {
 
   logger.info('Verifying phone number');
 
+  // See sendRegistrationPhoneOTP. A code sent before the account was claimed must not
+  // verify a phone on it afterwards.
+  if (user.verified) {
+    await AuthEventService.log({
+      userId: user.id,
+      type: 'verify_otp_suspicious',
+      req,
+      metadata: { reason: 'Registration phone verification on a verified account.' },
+    });
+    return res.status(401).json({ error: 'Not allowed' });
+  }
+
   if (!user || !user.phoneVerificationTokenExpiry || !user.phoneVerificationToken) {
     logger.warn('Failed to find a user for this phone verification token');
     await AuthEventService.log({
@@ -463,6 +512,18 @@ export const verifyLoginPhoneNumber = async (req: Request, res: Response) => {
 
   logger.info('Verifying login phone number');
 
+  // A code sent to an unverified phone, through registration before the account was
+  // claimed, is not a sign-in.
+  if (!user.phoneVerified) {
+    await AuthEventService.log({
+      userId: user.id,
+      type: 'verify_otp_suspicious',
+      req,
+      metadata: { reason: 'Login phone verification for an unverified phone.' },
+    });
+    return res.status(401).json({ error: 'Not allowed' });
+  }
+
   if (!user || !user.phoneVerificationTokenExpiry || !user.phoneVerificationToken) {
     logger.warn('Failed to find a user for this phone verification token');
     await AuthEventService.log({

src/routes/otp.routes.ts

@@ -17,8 +17,8 @@ import {
 import {
   sendLoginEmailOTP,
   sendLoginPhoneOTP,
-  sendPhoneOTP,
   sendRegistrationEmailOTP,
+  sendRegistrationPhoneOTP,
   verifyEmail,
   verifyLoginEmail,
   verifyLoginPhoneNumber,
@@ -97,7 +97,7 @@ otpRouter.post(
       },
     },
   },
-  sendPhoneOTP,
+  sendRegistrationPhoneOTP,
 );
 
 otpRouter.get(
@@ -121,7 +121,7 @@ otpRouter.get(
       },
     },
   },
-  sendPhoneOTP,
+  sendRegistrationPhoneOTP,
 );
 
 otpRouter.post(

src/services/loginPolicyService.ts

@@ -20,6 +20,7 @@ export interface LoginPolicy {
 type LoginMethodUser = {
   email?: string | null;
   phone?: string | null;
+  phoneVerified?: boolean | null;
 };
 
 const LOGIN_METHOD_ORDER: LoginMethod[] = [
@@ -126,7 +127,9 @@ export function resolveAvailableLoginMethods({
       return false;
     }
 
-    return hasValue(user.phone);
+    // An unverified phone may have been planted by whoever registered the address
+    // first, so it is never a way in.
+    return hasValue(user.phone) && user.phoneVerified === true;
   });
 }
 

src/services/policyDryRun.ts

@@ -109,7 +109,7 @@ export function hasUsableFactor(
 ) {
   const methods = resolveAvailableLoginMethods({
     policy,
-    user: { email: account.email, phone: account.phone },
+    user: { email: account.email, phone: account.phone, phoneVerified: account.phone !== null },
     hasPasskeyCredential: account.hasPasskey,
   });
 
@@ -158,7 +158,7 @@ async function signInsInWindow(from: Date, to: Date, cap: number) {
         )
     ),
     scoped AS (
-      SELECT u.id, u.email, u.phone
+      SELECT u.id, u.email, CASE WHEN u.phone_verified THEN u.phone END AS phone
       FROM users u
       WHERE u.revoked = false
         AND u.id IN (SELECT user_id FROM sign_ins)

tests/integration/authentication/authentication.spec.ts

@@ -216,7 +216,9 @@ describe('POST /login', () => {
 
     expect(res.status).toBe(200);
     expect(res.body.identifierType).toBe('phone');
-    expect(User.findOne).toHaveBeenCalledWith({ where: { phone: expect.any(String) } });
+    expect(User.findOne).toHaveBeenCalledWith({
+      where: { phone: expect.any(String), phoneVerified: true },
+    });
   });
 
   it('reports a server error when the email lookup throws', async () => {

tests/integration/otp/otp.spec.ts

@@ -49,11 +49,11 @@ beforeEach(() => {
 });
 
 describe('OTP - Generate', () => {
-  it('generates phone OTP', async () => {
+  it('sends no registration phone OTP to a verified account', async () => {
     const res = await request(app).get('/otp/generate-phone-otp');
 
     expect(res.status).toBe(200);
-    expect(generatePhoneOTP).toHaveBeenCalled();
+    expect(generatePhoneOTP).not.toHaveBeenCalled();
     expect(res.body.message).toBe('success');
   });
 
@@ -126,23 +126,13 @@ describe('OTP - Verify Phone', () => {
     expect(res.status).toBe(401);
   });
 
-  it('succeeds when OTP valid', async () => {
-    (verifyPhoneOTP as any).mockResolvedValue({
-      user: {
-        id: 'user-1',
-        emailVerified: true,
-        phoneVerified: true,
-        verified: true,
-        roles: ['user'],
-      },
-      verified: true,
-    });
-
+  it('refuses a registration phone code on a verified account', async () => {
     const res = await request(app)
       .post('/otp/verify-phone-otp')
       .send({ verificationToken: '123456' });
 
-    expect(res.status).toBe(200);
+    expect(res.status).toBe(401);
+    expect(verifyPhoneOTP).not.toHaveBeenCalled();
   });
 });