[lib][web][native] Pass preRequestUserState into invalidSessionRecovery
ClosedPublic
Actions

Authored by inka on Apr 11 2024, 6:05 AM.

Details

Reviewers

tomek
ginsu

Commits

rCOMM7b3fd475e97a: [lib][web][native] Pass preRequestUserState into invalidSessionRecovery

Summary

issue: ENG-6126
We want to cross reference the user info from before the action was called with the current user info, to check if the same user is still logged in. Otherwise we could end up overriding a new users session with a recovery that was startd for the previous user.

Test Plan

Tested on both platforms that normal logins and loguts work as expected
Tested on native that if a COOKIE_INVALIDATION_RESOLUTION_ATTEMPT login with a different user id is dispatched, then the action is disregarded

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

inka created this revision.Apr 11 2024, 6:05 AM

Herald added a subscriber: ashoat. · View Herald TranscriptApr 11 2024, 6:06 AM

inka added a parent revision: D11631: [lib] Stop attempting recovery when client version unsupported.Apr 11 2024, 6:06 AM

Harbormaster completed remote builds in B28169: Diff 39056.Apr 11 2024, 6:25 AM

inka requested review of this revision.Apr 11 2024, 6:25 AM

inka added a child revision: D11643: [web] Show a popup when client_version_unsupported.Apr 12 2024, 1:40 AM

tomek accepted this revision.Apr 15 2024, 1:21 AM

tomek added inline comments.

lib/actions/siwe-actions.js
83 ↗	(On Diff #39056)	Why do we set it to `null`?
native/redux/redux-setup.js
203–210 ↗	(On Diff #39056)	What's the reason behind having `preRequestUserState` in one and `preRequestUserInfo` in another? Can we make this consistent?
203–210 ↗	(On Diff #39056)	Are the old values the same as the new ones, or are we introducing a change in the logic here?

This revision is now accepted and ready to land.Apr 15 2024, 1:21 AM

inka added inline comments.Apr 15 2024, 2:07 AM

lib/actions/siwe-actions.js
83 ↗	(On Diff #39056)	Siwe auth is never a recovery, so we know the user was logged out
native/redux/redux-setup.js
203–210 ↗	(On Diff #39056)	This changes the logic, because this fixes it: `invalidSessionRecovery` is supposed to check if the user that is logged in is the same user as the one for whom the action was dispatched. It's not a valid session recovery, if it's a different user. For login this change shouldn't change anything, since the user info returned by login should be the same as the user info before the login (unless the user was logged out before the login, but that is handled as well). For session change the new user info can be a logged out user info though, so we need to check the `preRequestUserState`
203–210 ↗	(On Diff #39056)	`PreRequestUserState` includes +cookiesAndSessions: { +[keyserverID: string]: PreRequestUserKeyserverSessionInfo, }, which are needed for `setNewSessionActionType`. They are not needed for `logInActionTypes`, so we don't include them there. So I would rather leave this as is, and not make the login action unnecessarily large.