Page MenuHomePhabricator

[keyserver] Split up processSuccessfulLogin
ClosedPublic

Authored by ashoat on May 14 2024, 4:17 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Jun 24, 4:06 AM
Unknown Object (File)
Mon, Jun 24, 4:06 AM
Unknown Object (File)
Thu, Jun 20, 5:04 PM
Unknown Object (File)
Thu, Jun 20, 2:40 PM
Unknown Object (File)
Thu, Jun 20, 3:33 AM
Unknown Object (File)
Wed, Jun 19, 7:39 PM
Unknown Object (File)
Wed, Jun 19, 9:48 AM
Unknown Object (File)
Wed, Jun 19, 8:20 AM
Subscribers

Details

Summary

This diff splits up processSuccessfulLogin into two in order to resolve ENG-8101:

  1. processSuccessfulLogin (same name as before), which does cookie/session stuff, Olm initialization, policy stuff, and other general "session setup" work
  2. fetchRegistrationResult, which fetches the data that we need to send to the user in the response.

There's also a new utility handleSuccessfulLoginResult, which looks at the response from processSuccessfulLogin and decides whether to call fetchRegistrationResult.

The reason for splitting it into two is that in the registration case for both siweAuthResponder and keyserverAuthResponder, we want to do some additional steps between these two (sendMessagesOnAccountCreation, formerly known as processAccountCreationCommon).

Depends on D12038

Test Plan

I tested the following scenarios and made sure that ENG-8101 was resolved (no "anonymous" username):

  1. I tested account creation via keyserver auth in a multi-keyserver environment
  2. I tested login via keyserver auth in a multi-keyserver environment
  3. I tested legacy account creation in a legacy environment
  4. I tested legacy SIWE account creation in a legacy environment
  5. I tested legacy SIWE login in a legacy environment

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

ashoat edited the test plan for this revision. (Show Details)

Thank you for catching and fixing this!

keyserver/src/responders/user-responders.js
482 ↗(On Diff #40186)

This is called registration, but don't we do this after login as well?

This revision is now accepted and ready to land.May 15 2024, 2:07 AM
keyserver/src/responders/user-responders.js
482 ↗(On Diff #40186)

Yes, you're totally right – will fix this