[$250] [Payment card / Subscription] Add SCA support in Transfer Owner flow

[amyevans]

Per this discussion in the Payment card and subscription management in NewDot design doc, we need to support adding a GBP payment card in the "Transfer Owner" flow.

To test

1. As the owner of a Collect policy, add a new admin
2. Log into NewDot as that admin
3. Navigate to Account Settings > Workspaces > [workspace] > Members > click on the row of the current owner > Transfer Owner

Currently, when transferring ownership of a policy in NewDot, the AddBillingCardAndRequestPolicyOwnerChange API command is called. That command is calling UserAPI::addPaymentCard, which doesn't account for “Strong Customer Authentication” aka SCA requirements (which are relevant for GBP-currency payment cards).

Once SCA support is built out for NewDot in the Payment card project, we should use it as a base to extend to the Transfer Owner flow.

Update September 5th 2024

SCA support is now in new dot but making SCA possible for the transfer owner flow has required doing a lot of backend work to make it follow our 1:1:1 rule where 1 action equals with call to the web layer which equals one call to the Auth layer.

Once both of these PRs are merge though (1, 2) then this issue should be unblocked. From there we should follow the plan outlined in this comment to use the new VerifySetupIntentAndRequestPolicyOwnerChange command to complete the SCA transfer owner flow.

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021832164283072592668
• Upwork Job ID: 1832164283072592668
• Last Price Increase: 2024-09-06

Issue Owner

Current Issue Owner: @DylanDylann

[blimpich]

Still waiting for SCA to work in the subscription page, making progress.

[blimpich]

Picking this up.

[blimpich]

Hmm okay, looked at this this afternoon. Notes for myself:

• this is where we decide whether to go through with the transfer or to ask for the user to add a card in App. Depends on whether RequestWorkspaceOwnerChange returns an error or not.
• It seems like we have a couple options here, but I think the best is gonna be a purely frontend change. I think we can get rid of AddBillingCardAndRequestPolicyOwnerChange entirely, which is nice since that command is not 1:1:1

Current thinking: lets just do the same thing we did with the add payment card form here. We can use addSubscriptionPaymentCard() in place of addBillingCardAndRequestPolicyOwnerChange() right here. If its a non-GBP currency it'll go through as normal, and if it is GBP it'll start the setupIntent flow and will give us an authenticationLink that we can use to render a CardAuthenticationModal component. Then, in WorkspaceOwnerPaymentCardForm.tsx we can add two useEffect hooks, one to wait for the onyx key fundList to update, and one to waits for a 3DS-authentication-complete postMessage. The hook that is waiting on the 3DS post message will do basically the exact same thing that we do in add Payment card (see here), and the hook that waits for the fundList to update will check if the fundList has updated, if it has it'll check to make sure the billing card doesn't have any errors associated with it, and if it doesn't then it'll call the existing RequestWorkspaceOwnerChange command to change ownership.

I'm not actually even sure that we need to do that last part where we call RequestWorkspaceOwnerChange since it looks like we might currently be making duplicate calls. Before assigning this out to anyone I'm going to look into it more. But yeah, on initial inspection I think this can be all frontend and we can maybe even get rid of a whole command on the backend.

Follow up items for @blimpich to look into:

• does the transfer ownership flow break if we delete this line in web? Seems like it shouldn't, seems like we might be calling the web api PolicyAPI::transferOwnership twice right now in this flow, which would make it redundant

[blimpich]

Yeah that line seems to be redundant, I deleted it and the transfer owner flow still works fine. The one thing is that the onyx data doesn't seem to be getting updated correctly. Looking into that. Well maybe I spoke too soon. Seems to be some weirdness after deleting that line.

[blimpich]

Okay I made a proof of concept branch just to see if this idea would even work, and it does! I can transfer the owner by just waiting to see if the fund list updates and then triggering a RequestOwner. However after thinking about this more I think this might be violating our 1:1:1 philosophy.

[blimpich]

Ran out of time but I will ask in engineering chat tomorrow if my plan violates 1:1:1. If that is the case then we will need to probably do a good amount of work to make a new endpoint and make it 1:1:1.

[melvin-bot]

Triggered auto assignment to @RachCHopkins (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details. Please add this Feature request to a GH project, as outlined in the SO.

[melvin-bot]

@JmillsExpensify, @ntdiary, @blimpich, @waterim Still overdue 6 days?! Let's take care of this!

[JmillsExpensify]

Looks like we're still working through the PR

[melvin-bot]

@JmillsExpensify, @ntdiary, @blimpich, @waterim 8 days overdue is a lot. Should this be a Weekly issue? If so, feel free to change it!

[waterim]

Not overdue, under development
Im in contact with @blimpich

[melvin-bot]

@JmillsExpensify, @ntdiary, @blimpich, @waterim 10 days overdue. I'm getting more depressed than Marvin.

[blimpich]

PR is in review, @ntdiary you will need my ngrok credentials in order to test this PR. Please see this comment for the credentials and some common questions about using ngrok. Please put those credentials into your .env file in App. Let me or @waterim know if you need any help or encounter any issues while trying to test this, this is a bit of an odd PR to test due to the nature of how the backend handles all the data.

[ntdiary]

@blimpich, sorry, we have a time zone difference, and it seems that connecting to the local backend is required here? I'll request other available C+ members to take over. :)

[melvin-bot]

📣 @DylanDylann 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[blimpich]

Reassigning C+ as per this discussion.

@DylanDylann please see this comment for my ngrok credentials and details on the flows that we are testing: #42490 (comment)

[blimpich]

@DylanDylann I'm signing off for the day but are you going to be able to review this tomorrow during my work hours (9am-5pm PST)? I'd like to keep the ball rolling on this PR. Thank you!

[DylanDylann]

@JmillsExpensify The PR is deployed to production on 2/10. The payment should be ready on 9/10

[JmillsExpensify]

Ah sorry about that.

Payment summary: $250 for @DylanDylann for PR review and testing. Payment via Upwork.

[JmillsExpensify]

All paid out.