Cleanup Pusher subscriptions when signing out in another tab

[marcaaron]

Details

When signing out in another tab we are not always unsubscribing from Pusher. This is because the authToken gets removed, but none of the logic that runs in redirectToSignIn() runs on additional tabs. Additionally, the Onyx cache does not get cleared in other tabs although storage does (because it is shared). This PR solves these two issues by:

• moving unsubscribe logic into the lifecycle of AuthScreens component which will unmount whenever the authToken is unset
• call Onyx.clear() on all tabs (for the session) and not just the one where we sign out

Fixed Issues

$ #5034
$ https://github.com/Expensify/Expensify/issues/177689

Tests / QA Steps

1. Sign in as User A in one tab
2. Open an additional tab
3. Send a message from User B to User A from an entirely different browser session (e.g. incognito tab)
4. Log out of User A in the second tab opened
5. Log in as User C in that same tab
6. Send a message as User B to User A
7. Verify that User C is not able to access the chat between User B and User A and that no chats that are not supposed to exists show up.

Tested On

• Web
• Mobile Web
• Desktop
• iOS
• Android

Screenshots

Web

Mobile Web

Desktop

iOS

Android

[aldo-expensify]

I'll review this after lunch!

[aldo-expensify]

NAB: I was wondering why do we do this here instead of in clearStorageAndRedirect. I feels like the clean up logic is spread and it could be more centralized.

[marcaaron]

Ah, mainly because we are setting up the subscriptions in AuthScreens componentDidMount() here:

App/src/libs/Navigation/AppNavigator/AuthScreens.js

Lines 157 to 159 in ccef3d9

	componentDidMount() {
	NetworkConnection.listenForReconnect();
	PusherConnectionManager.init();

so I thought it made more sense to tear them down in componentWillUnmount().

We definitely could do it in clearStorageAndRedirect() but I think making the "cleanup" stuff a side effect of losing the authToken is OK because we set up a lot of logic in componentDidMount() that could also be triggered by the presence of the authToken.

Basically the assumption is that AuthScreens mounted = we have authToken and AuthScreen unmount = we lost the authToken.

But yeah I wouldn't mind cleaning this up eventually or doing something differently if it becomes complicated enough to where people are thoroughly confused about how it works.

[aldo-expensify]

Ah, mainly because we are setting up the subscriptions in AuthScreens componentDidMount() here:

Ohh that makes sense, I only looked at the code in componentWillUnmount

But yeah I wouldn't mind cleaning this up eventually or doing something differently if it becomes complicated enough to where people are thoroughly confused about how it works.

Agreed, I think it is good for now like it is, there is a good enough reason to keep it there.

[aldo-expensify]

Tested it in web, it works 🎉

Left a NAB.

[OSBotify]

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

[OSBotify]

🚀 Deployed to staging by @aldo-expensify in version: 1.1.3-2 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

[OSBotify]

🚀 Deployed to production by @chiragsalian in version: 1.1.4-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅