Page MenuHomePhabricator

[IGNORE] Client hacks for testing multiple keyservers
AbandonedPublic

Authored by inka on Feb 22 2024, 4:06 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Nov 25, 3:41 PM
Unknown Object (File)
Mon, Nov 25, 2:55 PM
Unknown Object (File)
Mon, Nov 25, 2:53 PM
Unknown Object (File)
Mon, Nov 25, 2:33 PM
Unknown Object (File)
Mon, Nov 25, 2:25 PM
Unknown Object (File)
Mon, Nov 25, 2:23 PM
Unknown Object (File)
Wed, Nov 20, 10:20 AM
Unknown Object (File)
Sat, Nov 9, 6:23 PM
Subscribers

Details

Reviewers
None
Summary

issue: ENG-6576
This diff contains hacks that should allow adding a second keyserver form a client

Test Plan

This is hard to test because I'm having trouble loging in using the identity flow, but I did thest this in the following way:

  1. I was logged in with the old flow
  2. I checked that in the keyservers screen, the authoritative keyserver does show up
  3. I tried adding a second keyserver (which I had running in Docker) - checked that it showed up on the list of keyservers as well. I didn't get logged in to, because I didn't have the CSAT

For the identity changes:

  1. I was logged out
  2. I typed in crenetials of a user that was registered with identity
  3. I checked that CSAT is being saved

Diff Detail

Repository
rCOMM Comm
Branch
inka/testing_multikeyservers
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

inka requested review of this revision.Feb 22 2024, 4:21 AM

Work around keys being in different shapes on ks and clients

lib/selectors/keyserver-selectors.js
129–130

We might not have the keyservers admin in userStore

137

Admin id is the same as keyserver id

lib/utils/services-utils.js
7

This flag lets us actually use the identity code

native/account/log-in-panel.react.js
335–340

We need to save the access token to commCoreModule, so that comm services auth metadata emitter can emit it back to js code that listens for it

Handling this correctly is tracked in ENG-6606

web/account/account-hooks.js
337–340

Clients and keyservers use a different format of the prey key. This is a workaround for now. Fixing this is for now tracked in ENG-6902

web/redux/action-types.js
48–59

This is not needed if we are actually connecting to the keyservers we add to our store

Use the real user id on native as well

web/account/account-hooks.js
9 ↗(On Diff #38093)

Now that D11319 has landed, I don't think the changes to this file are necessary anymore

Love to see more hacks getting removed here!

lib/reducers/services-access-token-reducer.js
7 ↗(On Diff #38981)

Nit: this change can be removed

native/account/log-in-panel.react.js
329–334 ↗(On Diff #38981)

Same as below; should be covered by IdentityServiceContextProvider.logInPasswordUser

native/account/registration/registration-server-call.js
101–106 ↗(On Diff #38981)

Wondering – why is this necessary? It seems like there is a setCommServicesAuthMetadata call inside IdentityServiceContextProvider.registerPasswordUser, which was introduced in D11021

It looks like that change was landed shortly after this diff was first introduced, so I suspect you had it here initially because it was necessary, but it likely is not necessary anymore

I tested this and I was indeed able to login and register without these lines!

Remove unnecessary lines

Exciting that there's only one hack left here!

web/redux/action-types.js
48–59 ↗(On Diff #39214)

Now that you've removed this last hack in D11701, should this diff be abandoned, and the Notion doc updated to just direct devs to set usingCommServicesAccessToken to true?

web/redux/action-types.js
48–59 ↗(On Diff #39214)

Ping on this!

inka added inline comments.
web/redux/action-types.js
48–59 ↗(On Diff #39214)

Yes, I think we can do that!