Skip to content

task(recovery-phone): Add support for message status updates#18417

Merged
dschom merged 1 commit intomainfrom
FXA-11133
Feb 22, 2025
Merged

task(recovery-phone): Add support for message status updates#18417
dschom merged 1 commit intomainfrom
FXA-11133

Conversation

@dschom
Copy link
Copy Markdown
Contributor

@dschom dschom commented Feb 20, 2025

Because

  • We want to monitor sms delivery status

This pull request

  • Adds endpoint for twilio webhook
  • Logs info about delivery status
  • Records metrics about delivery status

Issue that this pull request solves

Closes: FXA-11133

Checklist

Put an x in the boxes that apply

  • My commit is GPG signed.
  • If applicable, I have modified or added tests which pass locally.
  • I have added necessary documentation (if appropriate).
  • I have verified that my changes render correctly in RTL (if appropriate).

Screenshots (Optional)

Please attach the screenshots of the changes made in case of change in user interface.

Other information (Optional)

We can fully test this once we get it on stage.

@dschom dschom requested a review from a team as a code owner February 20, 2025 18:24
@dschom dschom requested a review from vbudhram February 20, 2025 19:13
}

// Only write log entries for certain statuses.
switch (messageStatus.MessageStatus) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏽

},
{
method: 'POST',
path: '/message-status',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like this route should be authenticated?

Copy link
Copy Markdown
Contributor Author

@dschom dschom Feb 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's protected by the twilio account sid. It needs to be a public endpoint though that twilio can call it. If the proper account sid is not provided in the payload, the request is just terminated and is a no op.

Do you think we should create a custom auth handler for the account sid? Or is it okay to let the end point do the work?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I meant based on their doc, they recommended verifying the X-Twilio-Signature header using their library. You could make a new auth-scheme or do it route handler.

Because:
- We want to monitor sms delivery status

This Commit:
- Adds endpoint for twilio webhook
- Logs info about delivery status
- Records metrics about delivery status
- Adds auth scheme for validating X-Twilio-Signature header
@dschom dschom merged commit b027920 into main Feb 22, 2025
@dschom dschom deleted the FXA-11133 branch February 22, 2025 02:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants