[keyserver] processOlmAccountCreation
ClosedPublic
Actions

Authored by varun on Dec 29 2023, 8:43 AM.

Details

Reviewers

ashoat
atul

Commits

rCOMM26a1b6eddb80: [keyserver] processOlmAccountCreation

Summary

similar to SIWE account creation logic. differences:

signedIdentityKeysBlob must be present
we use the UserIdentifier type to get the username (and eth address if present)
we don't send a reserved username request since this user has already registered with identity

not deduping because we eventually want to deprecate the siwe account creation function anyway

Depends on D10491

Test Plan

tested in next diff by calling the olm auth responder and confirming that the above listed changes all work as expected

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

varun created this revision.Dec 29 2023, 8:43 AM

Herald added a subscriber: tomek. · View Herald TranscriptDec 29 2023, 8:43 AM

varun added a child revision: D10493: [keyserver] olm auth responder.Dec 29 2023, 8:48 AM

varun published this revision for review.Dec 29 2023, 8:51 AM

Harbormaster completed remote builds in B25403: Diff 35101.Dec 29 2023, 9:01 AM

ashoat added inline comments.Dec 29 2023, 10:59 AM

keyserver/src/creators/account-creator.js
323 ↗	(On Diff #35101)	We seem to be allowing the user to pick any `username` here. Instead, we should ask the identity service for the `username`, to prevent the user from picking a `username` that doesn't match the identity service's records for the given `userID`

not deduping because we eventually want to deprecate the siwe account creation function anyway

I don't love this reasoning. This code will be around for a while with duplication. It seems like at least some of the shared code can be factored out here, especially the stuff at the end. Can you try to factor it out?

This revision now requires changes to proceed.Dec 29 2023, 11:07 AM

checking delta between two account creation functions

Harbormaster completed remote builds in B25561: Diff 35327.Jan 5 2024, 10:24 PM

varun added inline comments.Jan 5 2024, 10:34 PM

keyserver/src/creators/account-creator.js
323 ↗	(On Diff #35101)	the caller of this function will pass the username retrieved from the identity service

dedup

Harbormaster completed remote builds in B25564: Diff 35330.Jan 5 2024, 10:59 PM

varun added inline comments.Jan 5 2024, 11:16 PM

keyserver/src/creators/account-creator.js
15 ↗	(On Diff #35330)	not sure what happened here but i'll make sure to add the .js suffix back
300–302 ↗	(On Diff #35330)	not sure if we need to block on `setNewSession` but that's what we were doing in the existing code so i haven't changed it

ashoat accepted this revision.Jan 6 2024, 1:29 PM

ashoat added inline comments.

keyserver/src/creators/account-creator.js
300–302 ↗	(On Diff #35330)	`setNewSession` mutates `viewer`, so yeah probably best to `await` on it
305 ↗	(On Diff #35330)	You could just pass the `viewer` in only, and use `viewer.userID`
lib/types/account-types.js
175–178 ↗	(On Diff #35330)	To be honest, I don't think there's much of a point to this new type. It could be collapsed into `ProcessOLMAccountCreationRequest` to remove a layer of indirection