Skip to content

Commit

Permalink
task(recovery-phone): Support twilio api key
Browse files Browse the repository at this point in the history
Because:
- We want to use twilio API keys
- We don't want to use the twilio Auth Token

This Commit:
- Switches over to using api keys
- Adds method for signing webhook urls, so we can validate webhook calls originated from us.
- Adds ability to configure webhook url, keys for signing, and api keys.
- Still supports authToken so it's backwards compatible
- Creates custom mechanism for validating webhook calls, since the authToken is used by twilio to validate the signed payload.
- Removes the twilio-signature auth strategy and moves validation inline due to hapi limitations accessing the payload in the authorization.
- Adds lots of comments and some doc about how to manually test this with ngrok.
- Updates functional tests so they work with api keys
- Updates functional tests so they can work for smoke tests as well as ci pipeline tests.
- Updates functional tests so that twilio client can be configured independently of auth servers twilio client.
  • Loading branch information
dschom committed Mar 6, 2025
1 parent e962279 commit 37ea146
Show file tree
Hide file tree
Showing 21 changed files with 805 additions and 243 deletions.
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!
176 changes: 142 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,21 @@ 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,
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 +166,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 +198,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 +223,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 +518,7 @@ describe('RecoveryPhoneService', () => {
expect(mockSmsManager.sendSMS).toBeCalledWith({
to: phoneNumber,
body: (await mockGetFormattedMessage(code)).msg,
statusCallback: expect.stringContaining(mockWebhookUrl),
});
});

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

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

Expand Down Expand Up @@ -592,6 +613,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 +738,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

0 comments on commit 37ea146

Please sign in to comment.