[Mobile Payments] Fix Tap to Pay on iPhone setup failures on second account

[joshheald]

Part of: #8290

Description

When logging out of an account after using IPP, then logging in to a second account, The Tap to Pay on iPhone connection flow was always failing, and entering a loop.

This PR prevents the flow from always failing, but does not yet fix the loop, which can still be seen if you cancel the connection flow.

Cause of the failures: not updating the tokenProvider

The cause of this bug is that in the attempts on the second account, we're using an old tokenProvider in the Terminal, so the locationIDs we get are being used against the wrong Stripe account (unconfirmed, but at least with the wrong token.)

We correctly call Terminal.clearCachedCredentials when the store is switched and when the user logs out. That means that the Terminal calls the original tokenProvider (which it still has a reference to) to get a new token when we next connect to a reader.

We can't set a new tokenProvider, because the app crashes with this exception from Stripe:

[StripeTerminal] ⚠️ An integration error was detected in your app. You can only call setTokenProvider before requesting the shared Terminal instance for the first time. We recommend calling setTokenProvider in your AppDelegate's application:didFinishLaunchingWithOptions: method.

Fix

To fix it, I've put the token provider on the ServiceLocator, so that we keep hold of it for the lifetime of the app, and then the existing code will update the token that it vends, and the provider that it uses.

Concerns

This is likely just one presentation of this bug, and the fix will affect the existing bluetooth reader flow. We need to test these flows carefully.

Testing instructions

For the original bug

1. Launch the app fresh, and log in to an account with a US WCPay store, selecting that store
2. Navigate to Menu > Settings > Experimental features
3. Turn on Tap to Pay on iPhone
4. Navigate to Menu > Payments > Collect payment
5. Go through the payment flow, and select Card on the payment method screen
6. When asked for a reader type, tap Tap to Pay on iPhone and go through the Terms of Service Apple ID linking (if you've not done so before)
7. Take the payment, then log out
8. Log in to a different account, which has another store that has not had the ToS linking process done
9. Repeat the above steps to take a payment
10. When you get to the ToS Apple ID linking step, observe that you can complete it and then take a payment.

Connecting a Bluetooth reader

1. Launch the app fresh, and log in to an account with a US WCPay store, selecting that store
2. Navigate to Menu > Settings > Experimental features
3. Turn off Tap to Pay on iPhone
4. Navigate to Menu > Payments > Collect payment
5. Go through the payment flow, and select Card on the payment method screen
6. Connect to a Bluetooth reader: Chipper, M2, or WisePad3
7. Take the payment, then log out
8. Log in to a different account, which has another store
9. Repeat the above steps to take a payment
10. Observe that you can connect to a reader and then take a payment. (On trunk, you can't connect.)

Screenshots

ttpoi-account-linking-fix.mp4

---

• I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

[joshheald]

@lmischner Here's a tentative fix for part of the ToS acceptance bug. It doesn't address what happens if you cancel the ToS flow, I'll do that next week, but it does fix the happy path.

It's quite a high impact fix though, so it would be great if you have some time to do exploratory testing around it, particularly covering the bluetooth reader flows, and flows where the ToS acceptance is already done. I will also do some more of this next week.

Thanks!

[wpmobilebot]

You can test the changes from this Pull Request by:

• Clicking here or scanning the QR code below to access App Center
• Then installing the build number pr8633-009a740 on your iPhone

If you need access to App Center, please ask a maintainer to add you.

[joshheald]

@lmischner I've found the Bluetooth flavour of this issue in the production app, and added testing instructions for that.

On trunk, when you try to connect to the Bluetooth reader in the second account you'll get this error:

bluetooth-connection-bug-switching-accounts.mp4

N.B. These repro steps don't seem to quite be 100%, and I'm not sure why it doesn't always fail, but I've tested the fix enough to have confidence in it.

Based on the fix, the same can probably be done switching between reader types too, but all are fixed by this PR.

[joshheald]

@toupper Just to note that this fix is in the production code, not just the feature flagged TTPoI bit. It also fixes the bug in production as you can see from comments above.

I've moved the CommonReaderConfigProvider to the ServiceLocator, so it's explicitly handled as a singleton, just like the CardReaderService is.

Up to now it's treated as a singleton by StripeTerminal, which gets and keeps a reference to it when first used. However, we didn't keep a reference to it, so we couldn't effectively update it for a new user login.

I considered just keeping it on the CardReaderService, as a let property, but it's Yosemite which needs to update it, so left it in there as its own Singleton. Let me know if you have any thoughts on the way it's done.

[toupper]

@joshheald The code looks good to me ✅ I didn't test it as the setup would take a while, so I will rely on @lmischner's and your testing. Great job!

[lmischner]

If I connect to the bluetooth reader in one account and then log out/login to another account, should the connection be maintained?

Otherwise, no questions/issues found!

[joshheald]

If I connect to the bluetooth reader in one account and then log out/login to another account, should the connection be maintained?

Thanks for the review @lmischner – no, the connection should not be maintained in this case. When we connect to a reader we specify the (Stripe) account we're connecting for, which is unique to the store. So whenever we change account or store, we'll need to reconnect to the reader. In my testing, that's maintained, but sorry that I didn't call it out in the testing notes.