Page MenuHomePhabricator

[lib][native] Automatically retry keyserverAuth during login/registration
ClosedPublic

Authored by ashoat on Apr 30 2024, 12:28 PM.
Tags
None
Referenced Files
F3538074: D11848.id39747.diff
Wed, Dec 25, 10:21 PM
F3533033: D11848.id39730.diff
Wed, Dec 25, 11:08 AM
F3532740: D11848.id39713.diff
Wed, Dec 25, 9:47 AM
F3529948: D11848.diff
Tue, Dec 24, 10:26 PM
Unknown Object (File)
Sat, Dec 21, 2:56 PM
Unknown Object (File)
Sat, Dec 21, 2:56 PM
Unknown Object (File)
Sat, Dec 21, 2:56 PM
Unknown Object (File)
Sat, Dec 21, 2:55 PM

Details

Summary

This addresses ENG-7661.

When keyserverAuth is called during login or registration, if it fails we want to immediately try again. This is to guard against a situation where we are vended an OTK that is invalid. While we think we've addressed all scenarios where OTKs can be invalid, it's possible that we will introduce some new scenarios in the future, and we've had sporadic reports of this (eg. ENG-7927).

On the other hand, when keyserverAuth is called by KeyserverConnectionHandler for auth with secondary keyservers, or for keyserver session recovery, we'll continue only trying once. For secondary keyserver auth we'll end up retrying after AUTH_RETRY_DELAY_MS anyways. And for keyserver session recovery, the recovery attempt is arguably already the second try.

Note – would normally put @kamil on this review since we came up with the idea together in a 1:1, but since he's out for the rest of this week, I'm asking @varun to review.

Depends on D11847

Test Plan
  1. Force keyserver auth to fail with this patch
  2. Add some logs that get printed inside keyserver auth
  3. Test new registration flow and confirm that the logs get printed twice, but otherwise that the behavior is the same

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

varun requested changes to this revision.May 1 2024, 10:51 AM

shouldn't we check that the ServerError is olm_session_creation_failure when determining whether to retry? We'll be spending one of the keyserver's OTKs unnecessarily if the error is not OLM-related

This revision now requires changes to proceed.May 1 2024, 10:51 AM

Only retry if we get olm_session_creation_failure. Tested with keyserver patch that returns that error (saw two attempts in logs), and keyserver patch that returns different error (saw only one attempt in logs)

This revision is now accepted and ready to land.May 2 2024, 5:13 AM