Skip to content

Commit de6b0bf

Browse files
authored
Merge pull request #18276 from mozilla/FXA-10370
feat(sms): Set up flow to add recovery phone from Settings
2 parents f270419 + 6dd1055 commit de6b0bf

31 files changed

Lines changed: 737 additions & 310 deletions

File tree

libs/accounts/recovery-phone/src/lib/recovery-phone.service.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,6 @@ export class RecoveryPhoneService {
8080
if (!this.isSuccessfulSmsSend(msg)) {
8181
return false;
8282
}
83-
8483
await this.recoveryPhoneManager.storeUnconfirmed(
8584
uid,
8685
code,
@@ -168,7 +167,6 @@ export class RecoveryPhoneService {
168167
* @returns True if successful
169168
*/
170169
public async removePhoneNumber(uid: string) {
171-
172170
if (!this.config.enabled) {
173171
throw new RecoveryPhoneNotEnabled();
174172
}

packages/fxa-auth-server/config/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2182,7 +2182,7 @@ const convictConf = convict({
21822182
},
21832183
maxMessageLength: {
21842184
default: 60,
2185-
doc: 'Max allows sms message lenght',
2185+
doc: 'Max allows sms message length',
21862186
env: 'RECOVERY_PHONE__SMS__MAX_MESSAGE_LENGTH',
21872187
format: Number,
21882188
},

packages/fxa-auth-server/lib/routes/recovery-phone.ts

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,6 @@ class RecoveryPhoneHandler {
212212
try {
213213
success = await this.recoveryPhoneService.removePhoneNumber(uid);
214214
} catch (error) {
215-
216215
if (error instanceof RecoveryPhoneNotEnabled) {
217216
throw AppError.featureNotEnabled();
218217
}
@@ -229,7 +228,7 @@ class RecoveryPhoneHandler {
229228
'RecoveryPhoneService',
230229
'destroy',
231230
{ uid },
232-
error,
231+
error
233232
);
234233
}
235234

@@ -274,16 +273,6 @@ class RecoveryPhoneHandler {
274273
available,
275274
};
276275
} catch (error) {
277-
if (error instanceof RecoveryPhoneNotEnabled) {
278-
// In this case we won't throw an AppError. Unlike other endpoints,
279-
// this drives whether or not the feature shows up in the UI, so
280-
// if the recovery phone services isn't enabled, we can simply
281-
// return available false.
282-
return {
283-
available: false,
284-
};
285-
}
286-
287276
throw AppError.backendServiceFailure(
288277
'RecoveryPhoneService',
289278
'destroy',

packages/fxa-graphql-api/src/gql/account.resolver.spec.ts

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { ProfileClientService } from '../backend/profile-client.service';
2020
import { AccountResolver } from './account.resolver';
2121
import { NotifierService, NotifierSnsService } from '@fxa/shared/notifier';
2222
import { RecoveryPhoneService } from '@fxa/accounts/recovery-phone';
23+
import { GraphQLResolveInfo } from 'graphql';
2324

2425
let USER_1: any;
2526
let SESSION_1: any;
@@ -202,18 +203,44 @@ describe('#integration - AccountResolver', () => {
202203
const linkedAccounts = resolver.linkedAccounts(user!);
203204
expect(linkedAccounts).toEqual([]);
204205
});
205-
206206
it('resolves recovery phone number', async () => {
207+
authClient.recoveryPhoneAvailable = jest
208+
.fn()
209+
.mockResolvedValue({ available: true });
207210
recoveryPhoneService.hasConfirmed = jest
208211
.fn()
209-
.mockResolvedValue({ exists: false });
212+
.mockResolvedValue({ exists: true, phoneNumber: '+11234567890' });
213+
210214
const user = await Account.findByUid(USER_1.uid);
211-
const result = await resolver.recoveryPhone(user!);
215+
// Make the private method public for testing in favor of mocking 'info',
216+
// which is a very large object that's challenging to mock
217+
Object.defineProperty(
218+
resolver,
219+
'shouldIncludeRecoveryPhoneAvailability',
220+
{
221+
value: jest.fn().mockReturnValue(true),
222+
}
223+
);
224+
225+
const result = await resolver.recoveryPhone(
226+
'token',
227+
headers,
228+
user!,
229+
{} as unknown as GraphQLResolveInfo
230+
);
212231

213-
// Data shouldn't exist because no number has been registered yet.
214-
expect(result).toEqual({ exists: false });
215-
expect(recoveryPhoneService.hasConfirmed).toBeCalledTimes(1);
216-
expect(recoveryPhoneService.hasConfirmed).toBeCalledWith(USER_1.uid);
232+
expect(recoveryPhoneService.hasConfirmed).toHaveBeenCalledWith(
233+
user!.uid
234+
);
235+
expect(authClient.recoveryPhoneAvailable).toHaveBeenCalledWith(
236+
'token',
237+
headers
238+
);
239+
expect(result).toStrictEqual({
240+
phoneNumber: '+11234567890',
241+
exists: true,
242+
available: true,
243+
});
217244
});
218245

219246
it('resolves linked accounts when loaded', async () => {

packages/fxa-graphql-api/src/gql/account.resolver.ts

Lines changed: 52 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ import { EmailBounceStatusPayload } from './dto/payload/email-bounce';
9191
import { NotifierService } from '@fxa/shared/notifier';
9292
import { MozLoggerService } from '@fxa/shared/mozlog';
9393
import { RecoveryPhoneService } from '@fxa/accounts/recovery-phone';
94+
import { RecoveryPhone } from './model/recoveryPhone';
9495

9596
function snakeToCamel(str: string) {
9697
return str.replace(/(_\w)/g, (m: string) => m[1].toUpperCase());
@@ -148,6 +149,22 @@ export class AccountResolver {
148149
return simplified.fields.hasOwnProperty('securityEvents');
149150
}
150151

152+
private shouldIncludeRecoveryPhoneAvailability(
153+
info: GraphQLResolveInfo
154+
): boolean {
155+
// Introspect the query to determine if we should check recovery phone availability
156+
const parsed: ResolveTree = parseResolveInfo(info) as ResolveTree;
157+
const simplified = simplifyParsedResolveInfoFragmentWithType(
158+
parsed,
159+
info.returnType
160+
) as {
161+
fields: {
162+
available?: boolean;
163+
};
164+
};
165+
return !!simplified.fields.hasOwnProperty('available');
166+
}
167+
151168
@Mutation((returns) => CreateTotpPayload, {
152169
description:
153170
'Create a new randomly generated TOTP token for a user if they do not currently have one.',
@@ -849,9 +866,41 @@ export class AccountResolver {
849866
return [];
850867
}
851868

852-
@ResolveField()
853-
public async recoveryPhone(@Parent() account: Account) {
854-
return this.recoveryPhoneService.hasConfirmed(account.uid);
869+
@ResolveField(() => RecoveryPhone)
870+
public async recoveryPhone(
871+
@GqlSessionToken() sessionToken: string,
872+
@GqlXHeaders() headers: Headers,
873+
@Parent() account: Account,
874+
@Info() info: GraphQLResolveInfo
875+
) {
876+
const includeAvailability =
877+
this.shouldIncludeRecoveryPhoneAvailability(info);
878+
879+
try {
880+
const recoveryPhone = await this.recoveryPhoneService.hasConfirmed(
881+
account.uid
882+
);
883+
884+
if (includeAvailability) {
885+
// This queries the auth-server endpoint instead of directly due to
886+
// this endpoint needing maxmind
887+
const { available } = await this.authAPI.recoveryPhoneAvailable(
888+
sessionToken,
889+
headers
890+
);
891+
return { ...recoveryPhone, available };
892+
}
893+
894+
return recoveryPhone;
895+
} catch (_) {
896+
// The service either failed, or we have the backend config for this off.
897+
// Return some reasonable defaults.
898+
return {
899+
exists: false,
900+
...(includeAvailability ? { available: false } : {}),
901+
phoneNumber: null,
902+
};
903+
}
855904
}
856905

857906
@ResolveField()

packages/fxa-graphql-api/src/gql/model/recoveryPhone.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,15 @@ export class RecoveryPhone {
1313

1414
@Field({
1515
nullable: true,
16-
description: 'The registered recovery phone number',
16+
description:
17+
'The registered recovery phone number. If the user does not have a verified session, this field will return the last 4 digits of the phone number with a mask on the rest.',
1718
})
1819
public phoneNumber!: string;
20+
21+
@Field({
22+
nullable: true,
23+
description:
24+
'Returns true if the user is eligible to set up a recovery phone.',
25+
})
26+
public available!: boolean;
1927
}

packages/fxa-settings/src/components/App/gql.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,3 +108,15 @@ export const GET_BACKUP_CODES_STATUS = gql`
108108
}
109109
}
110110
`;
111+
112+
export const GET_RECOVERY_PHONE = gql`
113+
query GetRecoveryPhone {
114+
account {
115+
recoveryPhone {
116+
available
117+
exists
118+
phoneNumber
119+
}
120+
}
121+
}
122+
`;

packages/fxa-settings/src/components/Settings/FlowSetupRecoveryPhoneConfirmCode/en.ftl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,5 @@ flow-setup-phone-confirm-code-button = Confirm
1313
# followed by a button to resend a code
1414
flow-setup-phone-confirm-code-expired = Code expired?
1515
flow-setup-phone-confirm-code-resend-code-button = Resend code
16+
flow-setup-phone-confirm-code-resend-code-success = Code sent
1617
flow-setup-phone-confirm-code-success-message-v2 = Recovery phone added

packages/fxa-settings/src/components/Settings/FlowSetupRecoveryPhoneConfirmCode/index.stories.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import SettingsLayout from '../SettingsLayout';
99
import { action } from '@storybook/addon-actions';
1010
import { AuthUiErrors } from '../../../lib/auth-errors/auth-errors';
1111
import FlowSetupRecoveryPhoneConfirmCode from '.';
12+
import { MOCK_FULL_PHONE_NUMBER } from '../../../pages/mocks';
1213

1314
export default {
1415
title: 'Components/Settings/FlowSetupRecoveryPhoneConfirmCode',
@@ -43,7 +44,7 @@ const verifyRecoveryCodeFailure = async (code: string) => {
4344
return Promise.reject(AuthUiErrors.UNEXPECTED_ERROR);
4445
};
4546

46-
const formattedPhoneNumber = '+1 123-456-3019';
47+
const formattedPhoneNumber = MOCK_FULL_PHONE_NUMBER;
4748

4849
export const Success = () => (
4950
<SettingsLayout>

packages/fxa-settings/src/components/Settings/FlowSetupRecoveryPhoneConfirmCode/index.test.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ describe('FlowSetupRecoveryPhoneConfirmCode', () => {
8787

8888
await waitFor(() => {
8989
expect(mockSendCode).toHaveBeenCalledTimes(1);
90-
expect(screen.getByText(/Resend code/i)).toBeInTheDocument();
90+
expect(screen.getByText(/Code sent/i)).toBeInTheDocument();
9191
});
9292
});
9393

0 commit comments

Comments
 (0)