feat(auth): enable otp by clientid and service

StaberindeZA · StaberindeZA · commit a7bb469f03f5 · 2026-03-18T13:45:09.000-04:00
Because: - Allow Passwordless OTP to be enabled for a clientId and then only specific services of that clientId or all services. This commit: - Adds a new env var, PASSWORDLESS_ALLOWED_CLIENT_SERVICES, to configure which client and service are allowed to use the passwordless OTP feature. - Removes env var, PASSWORDLESS_ALLOWED_CLIENTIDS env var. - Updates metricsContext to include service Closes #FXA-13178
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -177,6 +177,7 @@ executors:
       # passwordless otp feature
       PASSWORDLESS_ENABLED: true
       PASSWORDLESS_ALLOWED_SERVICES: '98e6508e88680e1a,5882386c6d801776,dcdb5ae7add825d2'
+      PASSWORDLESS_ALLOWED_CLIENT_SERVICES: '{"98e6508e88680e1a":{"allowedServices":["*"]},"5882386c6d801776":{"allowedServices":["sync"]},"dcdb5ae7add825d2":{"allowedServices":["*"]}}'
       # Seeing if clear customs approach works! RATE_LIMIT__RULES: ""
       # RATE_LIMIT__IGNORE_EMAILS: .*@restmail.net$
 
diff --git a/libs/shared/metrics/glean/src/lib/metrics.context.ts b/libs/shared/metrics/glean/src/lib/metrics.context.ts
@@ -29,6 +29,7 @@ export class MetricsContext {
   utmSource?: string;
   utmTerm?: string;
   clientId?: string;
+  service?: string;
 
   constructor(queryParams?: Record<string, string | undefined>) {
     queryParams = queryParams || {};
@@ -40,6 +41,7 @@ export class MetricsContext {
         ? Number(queryParams['flowBeginTime'] || queryParams['flow_begin_time'])
         : undefined;
     this.clientId = queryParams['clientId'] || queryParams['client_id'];
+    this.service = queryParams['service'];
     this.utmCampaign =
       queryParams['utmCampaign'] || queryParams['utm_campaign'];
     this.utmContent = queryParams['utmContent'] || queryParams['utm_content'];
diff --git a/packages/fxa-auth-server/config/dev.json b/packages/fxa-auth-server/config/dev.json
@@ -472,10 +472,16 @@
   },
   "passwordlessOtp": {
     "enabled": true,
-    "allowedClientIds": [
-      "98e6508e88680e1a",
-      "5882386c6d801776",
-      "dcdb5ae7add825d2"
-    ]
+    "allowedClientServices": {
+      "98e6508e88680e1a": {
+        "allowedServices": ["*"]
+      },
+      "5882386c6d801776": {
+        "allowedServices": ["sync"]
+      },
+      "dcdb5ae7add825d2": {
+        "allowedServices": ["*"]
+      }
+    }
   }
-}
+}
diff --git a/packages/fxa-auth-server/config/index.ts b/packages/fxa-auth-server/config/index.ts
@@ -2173,11 +2173,11 @@ const convictConf = convict({
       format: Boolean,
       env: 'PASSWORDLESS_ENABLED',
     },
-    allowedClientIds: {
-      doc: 'Array of clients ids allowed to use passwordless authentication. Empty array means no service is allowed.',
-      format: Array,
-      default: [],
-      env: 'PASSWORDLESS_ALLOWED_SERVICES',
+    allowedClientServices: {
+      doc: 'Map of client IDs to their allowed services for passwordless authentication. Format: {"clientId": {"allowedServices": ["service1", "service2"]}}. Use "*" in allowedServices for all services. Empty array denies all services.',
+      format: Object,
+      default: {},
+      env: 'PASSWORDLESS_ALLOWED_CLIENT_SERVICES',
     },
     digits: {
       doc: 'Number of digits in passwordless OTP code',
@@ -2962,3 +2962,4 @@ export type ConfigType = ReturnType<conf['getProperties']>;
 
 export { convictConf as config };
 export default convictConf;
+
diff --git a/packages/fxa-auth-server/lib/metrics/context.js b/packages/fxa-auth-server/lib/metrics/context.js
@@ -46,6 +46,7 @@ const SCHEMA = isA
     productId: isA.string().max(128).optional(),
     planId: isA.string().max(128).optional(),
     clientId: isA.string().length(16).regex(HEX_STRING).optional(),
+    service: isA.string().max(128).optional(),
   })
   .unknown(false)
   .and('flowId', 'flowBeginTime');
diff --git a/packages/fxa-auth-server/lib/routes/account.ts b/packages/fxa-auth-server/lib/routes/account.ts
@@ -1639,15 +1639,21 @@ export class AccountHandler {
         // should use their linked provider (Google/Apple), not passwordless OTP.
         const hasLinkedAccount = (account.linkedAccounts?.length || 0) > 0;
         const isExistingPasswordless = account.verifierSetAt === 0 && !hasLinkedAccount;
+        
+        // Get service from metricsContext
+        const metricsContext = await request.app.metricsContext;
+        const service = metricsContext?.service;
+        
         result.passwordlessSupported = isExistingPasswordless ||
           (isPasswordlessEligible(
             account,
             email,
             this.config.passwordlessOtp.enabled
           ) &&
           isClientAllowedForPasswordless(
-            this.config.passwordlessOtp.allowedClientIds as string[],
-            clientId
+            this.config.passwordlessOtp.allowedClientServices,
+            clientId,
+            service
           ));
       } else {
         const exist = await this.db.accountExists(email);
@@ -1672,17 +1678,23 @@ export class AccountHandler {
         if (thirdPartyAuthStatus) {
           // Passwordless is supported if:
           // 1. Account is eligible (doesn't exist OR enabled globally)
-          // 2. AND clientId is allowed
+          // 2. AND clientId is allowed for the service
           const isEligible = isPasswordlessEligible(
             null, // null = account doesn't exist
             email,
             this.config.passwordlessOtp.enabled
           );
+          
+          // Get service from metricsContext
+          const metricsContext = await request.app.metricsContext;
+          const service = metricsContext?.service;
+          
           result.passwordlessSupported =
             isEligible &&
             isClientAllowedForPasswordless(
-              this.config.passwordlessOtp.allowedClientIds as string[],
-              clientId
+              this.config.passwordlessOtp.allowedClientServices,
+              clientId,
+              service
             );
         }
         if (this.customs.v2Enabled()) {
@@ -3214,4 +3226,4 @@ export const accountRoutes = (
   }
 
   return routes;
-};
+};
diff --git a/packages/fxa-auth-server/lib/routes/passwordless.ts b/packages/fxa-auth-server/lib/routes/passwordless.ts
@@ -111,23 +111,31 @@ class PasswordlessHandler {
    * Existing passwordless accounts bypass the client allowlist and feature flag.
    * Third-party auth accounts (with linked accounts) are not passwordless.
    */
-  private validatePasswordlessAccess(
+  private async validatePasswordlessAccess(
     account: any,
     email: string,
     clientId: string | undefined,
+    request: AuthRequest,
     logTag: string
   ) {
     const hasLinkedAccount = account && (account.linkedAccounts?.length || 0) > 0;
     const isExistingPasswordless = account && account.verifierSetAt === 0 && !hasLinkedAccount;
-    if (
-      !isExistingPasswordless &&
-      !isClientAllowedForPasswordless(
-        this.config.passwordlessOtp.allowedClientIds as string[],
-        clientId
-      )
-    ) {
-      this.log.error(`passwordless.${logTag}.clientIdNotAllowed`, { clientId });
-      throw error.featureNotEnabled();
+    if (!isExistingPasswordless) {
+      // Get service from metricsContext
+      const metricsContext = await request.app.metricsContext;
+      const service = metricsContext?.service;
+      
+      if (!isClientAllowedForPasswordless(
+        this.config.passwordlessOtp.allowedClientServices,
+        clientId,
+        service
+      )) {
+        this.log.error(`passwordless.${logTag}.clientIdOrServiceNotAllowed`, { 
+          clientId,
+          service 
+        });
+        throw error.featureNotEnabled();
+      }
     }
     if (!isPasswordlessEligible(account, email, this.config.passwordlessOtp.enabled)) {
       throw error.cannotCreatePassword();
@@ -145,7 +153,7 @@ class PasswordlessHandler {
     await this.customs.check(request, email, 'passwordlessSendOtp');
 
     const { account, isNewAccount } = await this.lookupAccount(email);
-    this.validatePasswordlessAccess(account, email, clientId, 'sendCode');
+    await this.validatePasswordlessAccess(account, email, clientId, request, 'sendCode');
 
     return this.generateAndSendOtp(request, email, account, isNewAccount);
   }
@@ -167,7 +175,7 @@ class PasswordlessHandler {
     const lookupResult = await this.lookupAccount(email);
     let account = lookupResult.account;
     const { isNewAccount } = lookupResult;
-    this.validatePasswordlessAccess(account, email, clientId, 'confirmCode');
+    await this.validatePasswordlessAccess(account, email, clientId, request, 'confirmCode');
 
     // Verify OTP
     const otpKey = account ? account.uid : email;
@@ -268,7 +276,7 @@ class PasswordlessHandler {
     await this.customs.check(request, email, 'passwordlessSendOtp');
 
     const { account, isNewAccount } = await this.lookupAccount(email);
-    this.validatePasswordlessAccess(account, email, clientId, 'resendCode');
+    await this.validatePasswordlessAccess(account, email, clientId, request, 'resendCode');
 
     // Delete existing code before sending a new one
     const otpKey = account ? account.uid : email;
@@ -545,4 +553,4 @@ export function passwordlessRoutes(
       handler: (request: AuthRequest) => handler.resendCode(request),
     },
   ];
-}
+}
diff --git a/packages/fxa-auth-server/lib/routes/utils/passwordless.spec.ts b/packages/fxa-auth-server/lib/routes/utils/passwordless.spec.ts
@@ -5,6 +5,7 @@
 import {
   isPasswordlessEligible,
   isClientAllowedForPasswordless,
+  AllowedClientServices,
 } from './passwordless';
 
 describe('isPasswordlessEligible', () => {
@@ -67,19 +68,105 @@ describe('isPasswordlessEligible', () => {
 });
 
 describe('isClientAllowedForPasswordless', () => {
-  it('returns true when clientId is in the allowed list', () => {
-    expect(isClientAllowedForPasswordless(['abc123'], 'abc123')).toBe(true);
+  const allowedClientServices: AllowedClientServices = {
+    abc123: { allowedServices: ['sync', 'profile'] },
+    xyz789: { allowedServices: ['*'] },
+    empty123: { allowedServices: [] },
+  };
+
+  describe('with valid clientId and service combinations', () => {
+    it('returns true when clientId and service are both allowed', () => {
+      expect(
+        isClientAllowedForPasswordless(allowedClientServices, 'abc123', 'sync')
+      ).toBe(true);
+      expect(
+        isClientAllowedForPasswordless(
+          allowedClientServices,
+          'abc123',
+          'profile'
+        )
+      ).toBe(true);
+    });
+
+    it('returns false when clientId is allowed but service is not', () => {
+      expect(
+        isClientAllowedForPasswordless(
+          allowedClientServices,
+          'abc123',
+          'monitor'
+        )
+      ).toBe(false);
+    });
+
+    it('returns true when clientId is in config and no service specified', () => {
+      expect(isClientAllowedForPasswordless(allowedClientServices, 'abc123')).toBe(
+        true
+      );
+    });
   });
 
-  it('returns false when clientId is not in the allowed list', () => {
-    expect(isClientAllowedForPasswordless(['abc123'], 'xyz789')).toBe(false);
+  describe('with wildcard support', () => {
+    it('returns true when allowedServices includes wildcard', () => {
+      expect(
+        isClientAllowedForPasswordless(
+          allowedClientServices,
+          'xyz789',
+          'any-service'
+        )
+      ).toBe(true);
+      expect(
+        isClientAllowedForPasswordless(
+          allowedClientServices,
+          'xyz789',
+          'another-service'
+        )
+      ).toBe(true);
+    });
   });
 
-  it('returns false when no clientId is provided', () => {
-    expect(isClientAllowedForPasswordless(['abc123'])).toBe(false);
+  describe('with empty allowedServices', () => {
+    it('returns false when allowedServices is empty array', () => {
+      expect(
+        isClientAllowedForPasswordless(
+          allowedClientServices,
+          'empty123',
+          'sync'
+        )
+      ).toBe(false);
+    });
+
+    it('returns true when allowedServices is empty but no service specified', () => {
+      expect(
+        isClientAllowedForPasswordless(allowedClientServices, 'empty123')
+      ).toBe(true);
+    });
   });
 
-  it('returns false when allowedClientIds is empty', () => {
-    expect(isClientAllowedForPasswordless([], 'abc123')).toBe(false);
+  describe('with invalid inputs', () => {
+    it('returns false when clientId is not in the config', () => {
+      expect(
+        isClientAllowedForPasswordless(
+          allowedClientServices,
+          'unknown',
+          'sync'
+        )
+      ).toBe(false);
+    });
+
+    it('returns false when no clientId is provided', () => {
+      expect(
+        isClientAllowedForPasswordless(allowedClientServices, undefined, 'sync')
+      ).toBe(false);
+    });
+
+    it('returns false when allowedClientServices is empty', () => {
+      expect(isClientAllowedForPasswordless({}, 'abc123', 'sync')).toBe(false);
+    });
+
+    it('returns false when allowedClientServices is undefined', () => {
+      expect(
+        isClientAllowedForPasswordless(undefined as any, 'abc123', 'sync')
+      ).toBe(false);
+    });
   });
 });
diff --git a/packages/fxa-auth-server/lib/routes/utils/passwordless.ts b/packages/fxa-auth-server/lib/routes/utils/passwordless.ts
@@ -2,11 +2,61 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+export interface PasswordlessClientConfig {
+  allowedServices: string[];
+}
+
+export type AllowedClientServices = Record<string, PasswordlessClientConfig>;
+
+/**
+ * Checks if a client is allowed to use passwordless authentication for a specific service.
+ * 
+ * @param allowedClientServices - Map of clientId to their allowed services configuration
+ * @param clientId - The OAuth client ID
+ * @param service - The service being requested (from metricsContext)
+ * @returns true if the client is allowed for the service, false otherwise
+ * 
+ * Behavior:
+ * - If clientId is not in config: deny
+ * - If clientId is in config but no service specified: allow (client is authorized)
+ * - If allowedServices is empty array: deny all services
+ * - If allowedServices includes "*": allow all services
+ * - Otherwise: check if service is in allowedServices array
+ */
 export function isClientAllowedForPasswordless(
-  allowedClientIds: string[],
-  clientId?: string
+  allowedClientServices: AllowedClientServices,
+  clientId?: string,
+  service?: string
 ): boolean {
-  return !!clientId && allowedClientIds?.includes(clientId);
+  if (!clientId) {
+    return false;
+  }
+
+  const clientConfig = allowedClientServices?.[clientId];
+  
+  if (!clientConfig) {
+    return false;  // Client not in allowlist
+  }
+
+  // If no service specified in request, allow (client is authorized)
+  if (!service) {
+    return true;
+  }
+
+  const allowedServices = clientConfig.allowedServices;
+  
+  // Empty array denies all services
+  if (!allowedServices || allowedServices.length === 0) {
+    return false;
+  }
+
+  // Support wildcard for "all services"
+  if (allowedServices.includes('*')) {
+    return true;
+  }
+
+  // Check if service is in the allowed list
+  return allowedServices.includes(service);
 }
 
 /**
diff --git a/packages/fxa-auth-server/test/local/routes/account.js b/packages/fxa-auth-server/test/local/routes/account.js
diff --git a/packages/fxa-auth-server/test/local/routes/passwordless.js b/packages/fxa-auth-server/test/local/routes/passwordless.js
