Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

task(recovery-phone): Support twilio api key #18500

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -776,6 +776,9 @@ jobs:
command: ./packages/functional-tests/scripts/start-services.sh
environment:
NODE_ENV: test
# Functional tests use 'real' numbers.
# Switch out of the default 'test' credential mode.
RECOVERY_PHONE__TWILIO__CREDENTIAL_MODE: 'apiKeys'
- run-playwright-tests:
project: local
- store-artifacts
Expand Down
20 changes: 20 additions & 0 deletions libs/accounts/recovery-phone/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,23 @@ Run `nx test-unit accounts-recovery-phone` to execute the unit tests via [Jest](
## Running integration tests

Run `nx test-integration accounts-recovery-phone` to execute the integration tests via [Jest](https://jestjs.io).

## Testing webhook callbacks for message status updates

Important Tip! To manually test webhook callbacks isn't super straight forward. Per Twilio's docs the best way to manually test is as follows.

1. Use ngrok to reverse proxy: ngrok http 9000
2. Whatever url ngrok spits out, update the auth server configuration to use this value plus `/v1/recovery_phone/status` as the twilio webHook url value. eg.
`RECOVERY_PHONE__TWILIO__WEBHOOK_URL=https://YOUR_NGROK_SUBDOMAIN.ngrok-free.app/v1/recovery_phone/status`
3. Start up the server again `dotenv -- yarn restart auth --update-env`. Or the whole stack if you are starting from scratch `dotenv -- yarn start`.
4. Create an account
5. Enable 2FA
6. Register a recovery phone
7. Watch the logs, you'll see twilio making the call back to the `/v1/recovery_phone/status` end point.

(Based on, https://www.twilio.com/en-us/blog/test-your-webhooks-locally-with-ngrok-html)

Also it's good to note that there are some caveats about configuration for webhooks. If you have an twilio authToken in used, the webhook will validate
the `X-Twilio-Signature` header. If you are using twilio API keys, then we need the fxaPublicKey/fxaPrivateKey pair set in the config. Reach
out to a teammate for this value, or generate one yourself. There's a method that will do this in the `util.ts`. And make sure this key pair
is in the config!
177 changes: 143 additions & 34 deletions libs/accounts/recovery-phone/src/lib/recovery-phone.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ import { MessageStatus } from 'twilio/lib/rest/api/v2010/account/message';
import { TwilioConfig } from './twilio.config';
import { getExpectedTwilioSignature } from 'twilio/lib/webhooks/webhooks';
import { SegmentedMessage } from 'sms-segments-calculator';
import {
createNewFxaKeyPair,
createRandomFxaMessage,
signFxaMessage,
} from './util';

describe('RecoveryPhoneService', () => {
const phoneNumber = '+15005551234';
Expand Down Expand Up @@ -70,11 +75,22 @@ describe('RecoveryPhoneService', () => {
},
} satisfies RecoveryPhoneConfig;

const mockAccountSid = 'AC00000000000000000000000000000000';
const mockAuthToken = '00000000000000000000000000000000';
const mockValidateWebhookCalls = true;
const mockWebhookUrl =
'https://accounts.firefox.com/v1/recovery_phone/message_status';

const mockKeys = createNewFxaKeyPair();

const mockTwilioConfig: TwilioConfig = {
accountSid: 'AC00000000000000000000000000000000',
authToken: '00000000000000000000000000000000',
webhookUrl: 'http://accounts.firefox.com/recovery-phone/message-status',
validateWebhookCalls: true,
credentialMode: 'default',
accountSid: mockAccountSid,
authToken: undefined,
webhookUrl: mockWebhookUrl,
validateWebhookCalls: mockValidateWebhookCalls,
fxaPublicKey: mockKeys.publicKey,
fxaPrivateKey: mockKeys.privateKey,
} satisfies TwilioConfig;

const mockGetFormattedMessage = async (code: string) => {
Expand Down Expand Up @@ -151,6 +167,7 @@ describe('RecoveryPhoneService', () => {
expect(mockSmsManager.sendSMS).toBeCalledWith({
to: phoneNumber,
body: (await mockGetFormattedMessage(code)).msg,
statusCallback: expect.stringContaining(mockWebhookUrl),
});
expect(mockRecoveryPhoneManager.storeUnconfirmed).toBeCalledWith(
uid,
Expand Down Expand Up @@ -182,6 +199,7 @@ describe('RecoveryPhoneService', () => {
expect(mockSmsManager.sendSMS).toBeCalledWith({
to: phoneNumber,
body: (await mockGetFormattedMessage(code)).msg,
statusCallback: expect.stringContaining(mockWebhookUrl),
});
expect(mockRecoveryPhoneManager.storeUnconfirmed).toBeCalledWith(
uid,
Expand All @@ -206,6 +224,7 @@ describe('RecoveryPhoneService', () => {
expect(mockSmsManager.sendSMS).toBeCalledWith({
to: phoneNumber,
body: (await mockGetFormattedMessage(code)).msg,
statusCallback: expect.stringContaining(mockWebhookUrl),
});
expect(mockRecoveryPhoneManager.storeUnconfirmed).toBeCalledWith(
uid,
Expand Down Expand Up @@ -500,6 +519,7 @@ describe('RecoveryPhoneService', () => {
expect(mockSmsManager.sendSMS).toBeCalledWith({
to: phoneNumber,
body: (await mockGetFormattedMessage(code)).msg,
statusCallback: expect.stringContaining(mockWebhookUrl),
});
});

Expand All @@ -520,6 +540,7 @@ describe('RecoveryPhoneService', () => {
expect(mockSmsManager.sendSMS).toBeCalledWith({
to: phoneNumber,
body: `Your Mozilla Account code is ${code}`,
statusCallback: expect.stringContaining(mockWebhookUrl),
});
});

Expand All @@ -540,6 +561,7 @@ describe('RecoveryPhoneService', () => {
expect(mockSmsManager.sendSMS).toBeCalledWith({
to: phoneNumber,
body: `Failsafe: ${code}`,
statusCallback: expect.stringContaining(mockWebhookUrl),
});
});

Expand Down Expand Up @@ -592,6 +614,7 @@ describe('RecoveryPhoneService', () => {
expect(mockSmsManager.sendSMS).toBeCalledWith({
to: phoneNumber,
body: (await mockGetFormattedMessage(code)).msg,
statusCallback: expect.stringContaining(mockWebhookUrl),
});

expect(mockRecoveryPhoneManager.removeCode).toBeCalledWith(
Expand Down Expand Up @@ -716,49 +739,135 @@ describe('RecoveryPhoneService', () => {
});
});

describe('verify twilio signature', () => {
// This is how Twilio generates the signature, see following doc for more info:
// https://www.twilio.com/docs/usage/security#test-the-validity-of-your-webhook-signature
const signature = getExpectedTwilioSignature(
mockTwilioConfig.authToken,
mockTwilioConfig.webhookUrl,
{
foo: 'bar',
}
);
describe('verify twilio webhook signature', () => {
const AccountSid = 'AC123';
const From = '+123456789';

afterEach(() => {
mockTwilioConfig.authToken = undefined;
mockTwilioConfig.validateWebhookCalls = true;
});

it('can validate twilio signature', () => {
const valid = service.validateTwilioSignature(signature, {
foo: 'bar',
it('will always validate if validateWebhookCalls is false', () => {
mockTwilioConfig.validateWebhookCalls = false;
expect(service.validateTwilioWebhookCallback({})).toBeTruthy();
});

describe('twilio signature', () => {
/**
* This is how Twilio generates the signature, see following doc for more info:
* https://www.twilio.com/docs/usage/security#test-the-validity-of-your-webhook-signature
*
* Note that when using twilio API keys instead of the main twilio auth token, this approach won't work!
* Unfortunately twilio doesn't offer any way to validate the signature with api keys.
*
*/
const twilioSignature = getExpectedTwilioSignature(
mockAuthToken,
mockWebhookUrl,
{
AccountSid,
From,
}
);

it('can validate twilio signature', () => {
mockTwilioConfig.authToken = mockAuthToken;
expect(
service.validateTwilioWebhookCallback({
twilio: {
signature: twilioSignature,
params: {
AccountSid,
From,
},
},
})
).toBeTruthy();
});
expect(valid).toBeTruthy();
});

it('can invalidate twilio signature due to bad payload', () => {
const valid = service.validateTwilioSignature(signature, {
foo: 'bar',
bar: 'baz',
it('can invalidate twilio signature due to bad payload', () => {
mockTwilioConfig.authToken = mockAuthToken;
const valid = service.validateTwilioWebhookCallback({
twilio: {
signature: twilioSignature,
params: {
AccountSid: AccountSid + '0',
From: From + '0',
},
},
});
expect(valid).toBeFalsy();
});
expect(valid).toBeFalsy();
});

it('can invalidate twilio signature due to bad signature', () => {
const valid = service.validateTwilioSignature(signature + '0', {
foo: 'bar',
it('can invalidate twilio signature due to bad signature', () => {
mockTwilioConfig.authToken = mockAuthToken;
const valid = service.validateTwilioWebhookCallback({
twilio: {
signature: twilioSignature + '0',
params: {
AccountSid,
From,
},
},
});
expect(valid).toBeFalsy();
});

it('can create twilio webhook callback url', () => {
mockTwilioConfig.authToken = mockAuthToken;
mockTwilioConfig.webhookUrl = mockWebhookUrl;

expect(service.createMessageStatusCallback()).toEqual(mockWebhookUrl);
});
expect(valid).toBeFalsy();
});

it('will always validate if validateWebhookCalls is false', () => {
mockTwilioConfig.validateWebhookCalls = false;
const valid = service.validateTwilioSignature(signature + '0', {
foo: 'bar',
describe('fxa signature', () => {
const { privateKey, publicKey } = createNewFxaKeyPair();
const fxaMessage = createRandomFxaMessage();
const fxaSignature = signFxaMessage(privateKey, fxaMessage);

it('can validate fxa signature', () => {
const valid = service.validateTwilioWebhookCallback({
fxa: {
signature: fxaSignature,
message: fxaMessage,
},
});
expect(valid).toBeFalsy();
});

it('can invalidate fxa-signature due to bad payload', () => {
const valid = service.validateTwilioWebhookCallback({
fxa: {
signature: fxaSignature,
message: '00',
},
});
expect(valid).toBeFalsy();
});

it('can validate fxa-signature due to bad signature', () => {
const valid = service.validateTwilioWebhookCallback({
fxa: {
signature: '00',
message: fxaMessage,
},
});
expect(valid).toBeFalsy();
});

it('can create fxa webhook callback url', () => {
mockTwilioConfig.authToken = undefined;
mockTwilioConfig.webhookUrl = mockWebhookUrl;
mockTwilioConfig.fxaPrivateKey = privateKey;
mockTwilioConfig.fxaPublicKey = publicKey;

const url = service.createMessageStatusCallback();
expect(url).toContain(mockWebhookUrl);
expect(url).toContain('?fxaSignature=');
expect(url).toContain('&fxaMessage=');
});
expect(valid).toBeTruthy();
});
});
});
Loading