Skip to content

Commit 9172ae7

Browse files
authored
Merge pull request #20052 from mozilla/fix-recovery-phone-available
Use RecoveryPhoneService.available in /account endpoint
2 parents 5e4eb9b + d9b7482 commit 9172ae7

2 files changed

Lines changed: 86 additions & 2 deletions

File tree

packages/fxa-auth-server/lib/routes/account.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2335,6 +2335,7 @@ export class AccountHandler {
23352335
backupCodesResult,
23362336
recoveryKeyResult,
23372337
recoveryPhoneResult,
2338+
recoveryPhoneAvailableResult,
23382339
securityEventsResult,
23392340
devicesResult,
23402341
authorizedClientsResult,
@@ -2346,13 +2347,17 @@ export class AccountHandler {
23462347
Container.get(BackupCodeManager).getCountForUserId(uid),
23472348
this.db.getRecoveryKeyRecordWithHint(uid),
23482349
Container.get(RecoveryPhoneService).hasConfirmed(uid),
2350+
request.app.geo?.location?.countryCode
2351+
? Container.get(RecoveryPhoneService).available(uid, request.app.geo.location.countryCode)
2352+
: Promise.resolve(false),
23492353
this.db.securityEventsByUid({ uid }),
23502354
this.db.devices(uid),
23512355
listAuthorizedClients(uid),
23522356
]);
23532357

2354-
// Check if recovery phone feature is enabled globally (region check requires geo context)
2355-
const recoveryPhoneAvailable = this.config.recoveryPhone?.enabled ?? false;
2358+
const recoveryPhoneAvailable = recoveryPhoneAvailableResult.status === 'fulfilled'
2359+
? recoveryPhoneAvailableResult.value
2360+
: false;
23562361

23572362
// Account record is required
23582363
if (accountRecord.status === 'rejected') {

packages/fxa-auth-server/test/local/routes/account.js

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ const {
4444
} = require('../../../lib/senders/oauth_client_info');
4545

4646
const { FxaMailer } = require('../../../lib/senders/fxa-mailer');
47+
const { RecoveryPhoneService } = require('@fxa/accounts/recovery-phone');
4748

4849
const glean = mocks.mockGlean();
4950
const profile = mocks.mockProfile();
@@ -5159,6 +5160,84 @@ describe('/account', () => {
51595160
});
51605161
});
51615162
});
5163+
5164+
describe('recoveryPhone.available', () => {
5165+
function buildRouteWithRecoveryPhone(recoveryPhoneConfig) {
5166+
const accountRoutes = makeRoutes({
5167+
config: {
5168+
subscriptions: { enabled: false },
5169+
recoveryPhone: recoveryPhoneConfig,
5170+
},
5171+
log,
5172+
db: mocks.mockDB({ email, uid }),
5173+
stripeHelper: mockStripeHelper,
5174+
});
5175+
return getRoute(accountRoutes, '/account');
5176+
}
5177+
5178+
it('should pass geo countryCode to the service', () => {
5179+
const mockService = {
5180+
hasConfirmed: sinon.fake.resolves({ exists: false, phoneNumber: null }),
5181+
available: sinon.fake.resolves(true),
5182+
};
5183+
Container.set(RecoveryPhoneService, mockService);
5184+
const route = buildRouteWithRecoveryPhone({
5185+
enabled: true,
5186+
allowedRegions: ['US'],
5187+
});
5188+
return runTest(route, request, (result) => {
5189+
assert.equal(mockService.available.firstCall.args[1], 'US');
5190+
assert.equal(result.recoveryPhone.available, true);
5191+
});
5192+
});
5193+
5194+
it('should return available false when service returns false', () => {
5195+
Container.set(RecoveryPhoneService, {
5196+
hasConfirmed: sinon.fake.resolves({ exists: false, phoneNumber: null }),
5197+
available: sinon.fake.resolves(false),
5198+
});
5199+
const route = buildRouteWithRecoveryPhone({
5200+
enabled: true,
5201+
allowedRegions: ['US'],
5202+
});
5203+
return runTest(route, request, (result) => {
5204+
assert.equal(result.recoveryPhone.available, false);
5205+
});
5206+
});
5207+
5208+
it('should return available false when service call fails', () => {
5209+
Container.set(RecoveryPhoneService, {
5210+
hasConfirmed: sinon.fake.resolves({ exists: false, phoneNumber: null }),
5211+
available: sinon.fake.rejects(new Error('service error')),
5212+
});
5213+
const route = buildRouteWithRecoveryPhone({
5214+
enabled: true,
5215+
allowedRegions: ['US'],
5216+
});
5217+
return runTest(route, request, (result) => {
5218+
assert.equal(result.recoveryPhone.available, false);
5219+
});
5220+
});
5221+
5222+
it('should return available false when geo location cannot be parsed', () => {
5223+
Container.set(RecoveryPhoneService, {
5224+
hasConfirmed: sinon.fake.resolves({ exists: false, phoneNumber: null }),
5225+
available: sinon.fake.resolves(false),
5226+
});
5227+
const noGeoRequest = mocks.mockRequest({
5228+
credentials: { uid, email },
5229+
log,
5230+
geo: {},
5231+
});
5232+
const route = buildRouteWithRecoveryPhone({
5233+
enabled: true,
5234+
allowedRegions: ['US'],
5235+
});
5236+
return runTest(route, noGeoRequest, (result) => {
5237+
assert.equal(result.recoveryPhone.available, false);
5238+
});
5239+
});
5240+
});
51625241
});
51635242

51645243
describe('/account/email_bounce_status', () => {

0 commit comments

Comments
 (0)