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

[Payment card / Subscription] make backend handle 3ds intent verification in new dot #44795

Closed
blimpich opened this issue Jul 4, 2024 · 17 comments
Assignees
Labels
Daily KSv2 Engineering Internal Requires API changes or must be handled by Expensify staff

Comments

@blimpich
Copy link
Contributor

blimpich commented Jul 4, 2024

Problem:

#44321 is implementing the frontend portion of our add payment flow for 3DS secure payment cards. However, the PR is blocked because upon completing the stripe authentication flow the backend doesn't properly update the private_stripeCustomerID nvp.

A lot of slack discussions were had over why this is happening and how to fix it (1, 2, 3).

Basically, the way we did it in Old Dot was very round about and required tunneling into Web-Secure via an iframe in order to fire off a User_VerifyPaymentIntent command. This doesn't work for new dot because (1) it is actually quite tricky to tunnel into old dot with valid auth token passed to the script we were using to fire off the necessary command, and (2) we shouldn't be relying on web-secure to begin with since we intend to get rid of it entirely, so building out new dot functionality to rely on a strange and confusing web-secure redirect would be carrying forward a lot of technical debt.

Solution:

Lets use stripe webhooks to fix this. We already have a stripe webhook endpoint. Hopefully we just need to add these actions to the ProccessStripeWebhooks command in Auth and have them queue a CreateStripeSetupIntent command with parameters accountID and isVerifying.

Stripe webhook events that (I think) need to be added

setup_intent.succeeded
setup_intent.setup_failed
setup_intent.canceled

We also need to make sure that this part of the design doc works / was completed:

We will also update the CreateStripeSetupIntent Auth command to push an Onyx update for the PRIVATE_STRIPE_CUSTOMER_ID NVP with the new status, which will be used by the frontend to complete the auth flow.

Otherwise the frontend won't respond to our new backend data.

Turns out doing this via stripe webhooks is harder than expected. Lets figure out a simpler way to do this using what we already have in place.

@blimpich blimpich added Engineering Daily KSv2 Internal Requires API changes or must be handled by Expensify staff labels Jul 4, 2024
@blimpich blimpich self-assigned this Jul 4, 2024
@blimpich blimpich moved this to Release 2: Summer 2024 (Aug) in [#whatsnext] #wave-collect Jul 4, 2024
@blimpich
Copy link
Contributor Author

blimpich commented Jul 4, 2024

Update: requested developer access to stripe so that we can update our webhooks here. Followed this SO.

@blimpich
Copy link
Contributor Author

blimpich commented Jul 8, 2024

Not overdue, waiting on deployment of the first Auth PR before updating stripe webhook endpoint with new events.

@melvin-bot melvin-bot bot removed the Overdue label Jul 8, 2024
@blimpich
Copy link
Contributor Author

blimpich commented Jul 9, 2024

Update: new webhook events were added, but apparently I still need more dev access in order to test this end to end locally. I'm trying to follow this SO which makes it seem like I should be able to request dev access to Stripe even though I'm not ring1. I asked again about getting dev access here. Waiting on response to that.

@blimpich
Copy link
Contributor Author

blimpich commented Jul 9, 2024

Draft PR up for adding new webhook functionality, waiting to see if I can get access change to test the webhooks end-to-end before moving forward.

@blimpich
Copy link
Contributor Author

blimpich commented Jul 10, 2024

Access change still not allowed. Its still possible for me to complete this issue but its going to be difficult since I'm going to have to ask a ring1 engineer to basically test everything I do.

I'm going to try and see if a ring1 dev can setup a testing endpoint for me on stripe's dashboard next. If that works then I can test this locally and also help verify the PRs that rely on this. Without it I either need to get a ring1 person to take up this issue or play the risky game of guessing what will work using mocked data rather than real Stripe webhook requests.

@blimpich
Copy link
Contributor Author

Banged my head against this today.

Should probably be using https requests instead of command forwarding in the main auth PR.

Got a web-PR up that I didn't realize I needed.

The sad thing though is that I got a ring1/ring0 engineer to add my ngrok to the stripe dashboard but I learned that I still need to have a stripe developer access to use those test endpoints, which is really frustrating. So now I'm just going to test as best I can using mocked data and then ask a ring0/ring1 engineer to test it end to end locally for me, which will slow down this whole process.

Working on getting it working with mock data, which is proving difficult because there is a lot of data to mock to get it to work.

Soldiering on.

@blimpich
Copy link
Contributor Author

blimpich commented Jul 11, 2024

Was too busy catching up on chores to work on this today. Server bugs, urgent customer issues. Will work on it tomorrow.

@blimpich
Copy link
Contributor Author

Good progress! I got a proof-of-concept working with mocked data. I got it working in old dot and new dot. I need to figure out how to make it work without needing to refresh the page to see the changes (asked about that here), but I'm 99% sure that is solvable and can be figured out soon.

Next steps:

  • make sure new dot updates dynamically, doesn't need refresh
  • figure out how to make this work in old dot without having to do a hard refresh, maybe by preserving old dot flow but passing a parameter (could be passed in the metaData in the commands since we have separate commands for oldDot vs newDot) to make it do one thing in old dot and another in new dot
  • make the code nice
  • have a ring1/ring0 test this locally using actual stripe webhooks and confirm that this works both for old dot and new dot

@melvin-bot melvin-bot bot added the Overdue label Jul 15, 2024
@blimpich
Copy link
Contributor Author

blimpich commented Jul 15, 2024

Not overdue, trying to prioritize this as my number 1 issue after finishing chores.

@melvin-bot melvin-bot bot removed the Overdue label Jul 15, 2024
@blimpich
Copy link
Contributor Author

Seems like in order to get it updating nvp_private_stripeCustomerID in real time, and therefore get new dot to update without doing a page refresh, I need to utilize the pusher api in web. Which means I need to get the right data to web too. Working on that now.

@blimpich
Copy link
Contributor Author

blimpich commented Jul 15, 2024

Okay I think the reason the onyx updates aren't working is because the request is being initiated through Stripe and not New Dot. We disabled onyx updates during the 4/29 fire for all requests except new dot requests. So now I need to figure out how to enable onyx updates for stripe requests I think. I asked about how to do that here.

@blimpich
Copy link
Contributor Author

Pretty stumped on this, can't seem to get the onyx updates to happen despite my best efforts.

I did think of an alternative way of doing this that I am going to test out that might be a lot easier.

@blimpich blimpich changed the title [Payment card / Subscription] make backend handle 3ds intent verification via Stripe webhooks rather than through frontend redirects [Payment card / Subscription] make backend handle 3ds intent verification in new dot Jul 16, 2024
@blimpich
Copy link
Contributor Author

The alternate way works, it doesn't require refreshing the page, and works for both old dot and new dot. Going to get PRs out for that now. It can also be tested without needing to be ring1, so much better for QA purposes.

@blimpich
Copy link
Contributor Author

blimpich commented Jul 16, 2024

Okay, so I figured out how to solve this after more carefully reading through the stripe documentation on 3ds verification. The basic idea is that callback.php will post a message to the parent of the iframe so that we know when the user is done verifying, and new dot will be listening for that message. When new dot sees that message it triggers a backend request User_VerifySetupIntent, which goes and confirms the user's response by pinging stripe, updating our private_stripeCustomerID nvp in the database, and then updates the necessary onyx data we need too. This is basically the exact same way that old dot does it, just instead of calling VerifySetupIntent from Web-Secure we instead call it from new dot directly.

This is a lot easier than making stripe webhooks work, both in terms of implementation and testing.

Next steps:

@melvin-bot melvin-bot bot added the Overdue label Jul 19, 2024
@blimpich
Copy link
Contributor Author

Not overdue, making progress, refer to last update's checklist

@melvin-bot melvin-bot bot removed the Overdue label Jul 19, 2024
@melvin-bot melvin-bot bot added the Overdue label Jul 22, 2024
@blimpich
Copy link
Contributor Author

Not overdue, I think this might be done actually but I'm going to wait until the frontened can confirm everything works for them.

@melvin-bot melvin-bot bot removed the Overdue label Jul 22, 2024
@blimpich
Copy link
Contributor Author

All the steps outlined here are either completed or close to being completed in other PRs. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Daily KSv2 Engineering Internal Requires API changes or must be handled by Expensify staff
Projects
No open projects
Archived in project
Development

No branches or pull requests

1 participant