Page MenuHomePhabricator

[lib] Let keyserver session recovery code handle calling setNewSession
ClosedPublic

Authored by ashoat on Feb 5 2024, 12:28 PM.
Tags
None
Referenced Files
F1920419: D10952.diff
Fri, May 31, 8:44 PM
Unknown Object (File)
Thu, May 23, 1:55 AM
Unknown Object (File)
Mon, May 20, 10:55 AM
Unknown Object (File)
Mon, May 20, 10:55 AM
Unknown Object (File)
Mon, May 20, 10:55 AM
Unknown Object (File)
Mon, May 20, 10:55 AM
Unknown Object (File)
Mon, May 20, 10:55 AM
Unknown Object (File)
Mon, May 20, 10:55 AM
Subscribers
None

Details

Summary

Currently, if two keyserver calls are dispatched simultaneously from a client using an invalid session, both of them will end up calling setNewSession.

Instead, we should let the keyserver session recovery code handle calling setNewSession.

Exceptions are:

  1. Where the setNewSession is NOT the result of an invalid session (eg. logIn). In these cases, we don't expect multiple simultaneous calls, and won't be triggering the keyserver session recovery code anyways.
  2. Where session recovery is not possible. Right now, this is web and cases where we specify paramOverride. When usingCommServicesAccessToken is true, this will only be cases where we specify paramOverride. At that point, we can consider removing the boundSetNewSession call introduced in this diff, as arguably the paramOverride cases should not be messing with Redux anyways.

Depends on D10951

Test Plan
  1. Test successful session invalidation in single keyserver world
    1. I prevented the Socket from rendering by adding a return null line before the other returns in KeyserverConnectionHandler. This avoided the Socket triggering session recovery.
    2. I started the iOS simulator and logged in using a test user.
    3. I opened the Redux Dev Tools
    4. I deleted the test user's cookie from the MariaDB database: DELETE FROM cookies WHERE user = 6390578 AND platform = 'ios'
    5. I sent a message as the test user
    6. I confirmed that session recovery was triggered in the Redux dev tools (and through some console logs)
    7. I repeated the process above several times to make sure it consistently worked multiple times in a single run
    8. I confirmed that the message was delivered "transparently" (without any visible issues, or evidence of session invalidation)
  2. Test failed session invalidation in single keyserver world
    1. I ran through the above test, but I hacked legacy-recover-keyserver-session.js to use the wrong password so the session recovery would fail
    2. I confirmed that I was logged out, and that an alert appeared explaining that my session was invalidated
  3. Test logging out during session recovery
    1. I triggered an infinite loop of session recoveries by running through the above test, but swallowing the SET_NEW_SESSION
    2. I logged out of the app
    3. I confirmed that the session recovery loop stopped, and that I was logged out successfully

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

ashoat requested review of this revision.Feb 5 2024, 1:11 PM
This revision is now accepted and ready to land.Feb 6 2024, 4:47 AM

Add detail to code comment about removing boundSetNewSession call once usingCommServicesAccessToken is true