[HOLD for payment 2021-09-22] After sign out, user should be signed out of all tabs

[isagoico]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

1. Sign in to New Expensify in one tab (A), open a second tab (B) with New Expensify (you should be signed in automatically)
2. In tab A, sign out then sign in with a new account
3. In tab B, navigate through several conversations.

Expected Result:

User should be signed out from all tabs.

Actual Result:

User is able to navigate through conversations after logging out from Tab A. An error can be seen in JS console.

Workaround:

User has to refresh the page to see correct conversations.

Platform:

Where is this issue occurring?

• Web
• Mobile Web

Version Number: 1.0.90-0

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

Grabando.211.mp4

Expensify/Expensify Issue URL:

View all open jobs on GitHub

---

From @Beamanator https://expensify.slack.com/archives/C01GTK53T8Q/p1630410901192800

BUG: Seeing combined chats from previously signed-in user in tab that stayed open through signout/signin. Flow:
Sign in to New Expensify in one tab (A), open a second tab (B) with New Expensify (you should be signed in automatically)
In tab A, sign out then sign in with a new account
In tab B, I’m seeing existing chats from the new account (good) & the previously signed in account (bad)

[MelvinBot]

Triggered auto assignment to @cead22 (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[MelvinBot]

Triggered auto assignment to @Gonals (Demolition), see https://stackoverflow.com/c/expensify/questions/8099 for more details.

[MelvinBot]

Huh... This is 4 days overdue. Who can take care of this?

[Hanaffi]

Proposal

It would be great to pass signals between tabs. There are a couple of methods, I want to tell you about using localStorage.
You can attach a listener to a storage event (fired when storage item is changed) and send a logout-event signal.

localStorage.setItem('logout-event', 'logout' + Math.random());

Every other tab will get it with a listener.

window.addEventListener('storage', function(event){
    if (event.key == 'logout-event') { 
        // ..
    }
});

We could also make an ajax request to your server every 3 seconds is useful just in case some of the users use very old browsers.

[Beamanator]

Hello @Hanaffi , thanks for your proposal! Have you looked into our code base much? Specifically how react-native-onyx works? I ask because this library ends up storing & retrieving data from localStorage :)

[parasharrajat]

This issue is caused but calling Onyx.clear instead of resetting the key.
Recently a change was made 72bf6c0 to fix another issue. But calling Onyx.clear ==> localstorage.clear does not pass any key to the storage event which is used to sync the tabs.

App/src/libs/listenToStorageEvents/index.js

Line 8 in 77dea94

window.addEventListener('storage', (e) => {

So change is never triggered across tabs for Onyx.clear. For this reason we used to reset the session key before. Now no update is pass to other tabs for the session key thus logout is not synced across tabs.

But this change is necessary as explained in the commit, I think the fix would be rather simple. we just need to reset the session explicitly with Onyx clear.

@Beamanator First, no one is assigned to this issue. Second, to fix this issue

App/src/libs/actions/SignInRedirect.js

Line 49 in 72bf6c0

Onyx.clear()

 Onyx.clear()
        .then(() => {
            if (preferredLocale) {
                Onyx.set(ONYXKEYS.NVP_PREFERRED_LOCALE, preferredLocale);
            }
            if (errorMessage) {
                Onyx.set(ONYXKEYS.SESSION, {error: errorMessage});
            }
            if (activeClients && activeClients.length > 0) {
                Onyx.set(ONYXKEYS.ACTIVE_CLIENTS, activeClients);
            }
      // Explicitly reset the session so that logout could be synced across tabs.
            Onyx.set(ONYXKEYS.SESSION, {authToken: null});
        });

we can't revert the above-mentioned commit as it fixes another issue and Kidroca has explained why it is needed.

[Beamanator]

@parasharrajat Thanks very much for your investigation! I noted this in the issue @kidroca 's PR fixed, let's collaborate and make sure this is the best solution 👍

[parasharrajat]

Sure, I think the solution I proposed is easiest. We use to do previously but in reverse order. I mean first session used to reset and then we clear it. But that had many side-effects. So We just need to tell other tabs that session key is changed and I set authToken to null as It is implicitly null or undefined after logout.

Hmm, happy to help.

[kidroca]

Confirming the discussion and the proposed solution.

No storage event is raised and so other tabs are unaware of the change

Thanks for the investigation and for pointing that out in the new code comment

Onyx.clear notifies only the current tab subscribers through keysChanged

[MelvinBot]

Triggered auto assignment to @puneetlath (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[Beamanator]

Thanks for confirming, @kidroca 👍

@parasharrajat thanks for bearing with me, I'm assigning this job to you - please submit a PR when you have a chance 💯

[puneetlath]

Invited @parasharrajat to the Upwork job.

[botify]

🚀 Deployed to staging by @marcaaron in version: 1.0.97-3 🚀

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

[botify]

🚀 Deployed to production by @francoisl in version: 1.0.98-1 🚀

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

[botify]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.0.98-1 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• https://github.com/Expensify/App/pulls/5190\n

If no regressions arise, payment will be issued on 2021-09-27. 🎊

[mountiny]

This is a bug, caused by today's deploy and some weird GH API behaviour. Updated the title back to what it was 🙇

[puneetlath]

Paid!