HomePhabricator
Diffusion Comm dfce845aa066

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

Description

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

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

Reviewers: inka, tomek

Reviewed By: tomek

Differential Revision: https://phab.comm.dev/D11222

Details

Provenance
ashoatAuthored on Feb 29 2024, 3:42 PM
Reviewer
tomek
Differential Revision
D11222: [lib] Don't cancel an auth that the keyserver completes
Parents
rCOMMf5323f9d084b: [lib] Factor out CANCELLED_ERROR into keyserver-conn-types.js
Branches
Unknown
Tags
Unknown