Page MenuHomePhabricator

[lib] Don't cancel an auth that the keyserver completes
ClosedPublic

Authored by ashoat on Mar 3 2024, 8:57 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Nov 28, 1:43 PM
Unknown Object (File)
Thu, Nov 28, 11:41 AM
Unknown Object (File)
Fri, Nov 22, 3:56 PM
Unknown Object (File)
Oct 29 2024, 9:48 AM
Unknown Object (File)
Oct 4 2024, 3:27 AM
Unknown Object (File)
Oct 4 2024, 3:27 AM
Unknown Object (File)
Oct 4 2024, 3:26 AM
Unknown Object (File)
Oct 4 2024, 3:26 AM
Subscribers
None

Details

Summary

While testing my stack, I was investigating an issue where a SET_NEW_SESSION from a successful auth would "cancel" the log-in recovery, leading to a LOG_IN_FAILED action getting dispatched.

It actually wasn't visible to the user, since the SET_NEW_SESSION resulted in a successful session recovery from the perspective of KeyserverConnectionHandler. Nevertheless, we should prevent the recovery from cancelling itself. The LOG_IN_FAILED here is causing the client to ignore most of the keyserver's response, that would normally be included in a LOG_IN_SUCCESS.

At first, I thought it would be best to implement a mechanism that would prevent an auth from cancelling itself, but preserve the ability of a different auth (perhaps triggered in the UI) from cancelling it.

But after thinking about it, I think we should instead prevent cancellation after getting the keyserver response. Here's why:

  1. It would be surprising to see a "different auth" like this... in order to get to the login screen, the user would need to log out first, which should cancel any pending recoveries anyways. (Even if it doesn't cancel it in time, invalidSessionRecovery should prevent the pending recovery from succeeding.)
  2. Currently, a different auth is able to cancel the log in action, but not the SET_NEW_SESSION. Making it possible to cancel the SET_NEW_SESSION would require drilling something into callSingleKeyserverEndpoint (where the SET_NEW_SESSION is dispatched), which would be annoying.
  3. When a keyserver auth succeeds, it moves the user's device token to the newest entry in the cookies table. If we ignore the successful keyserver auth and keep an older cookie, the device token may eventually be deleted when that newer cookie is pruned due to inactivity.

Depends on D11221

Test Plan

I used this test plan for the whole stack: https://gist.github.com/Ashoat/75ab690d5c53cdd68a51b02e03e27c58

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage