Page MenuHomePhabricator
Differential D7356

[keyserver] update registerUser function
ClosedPublic
Actions

Authored by varun on Apr 7 2023, 10:47 AM.
Tags
None
Referenced Files
F4093331: D7356.id24832.diff
Sun, Feb 16, 6:46 PM
Unknown Object (File)
Sun, Feb 9, 4:25 PM
Unknown Object (File)
Wed, Jan 29, 6:49 PM
Unknown Object (File)
Sat, Jan 25, 8:52 AM
Unknown Object (File)
Sat, Jan 25, 8:52 AM
Unknown Object (File)
Sat, Jan 25, 8:52 AM
Unknown Object (File)
Sat, Jan 25, 8:52 AM
Unknown Object (File)
Sat, Jan 25, 8:52 AM
View All Files
Subscribers
atul
tomek

Details

Reviewers
ashoat
• jon
Commits
rCOMMecfcdf3660fb: [keyserver] update registerUser function
Summary

this function now uses comm-opaque2 to handle OPAQUE registration and follows the new API spec defined in identity_client.proto

this function can be deprecated once we've successfully double-written all users from ashoat's keyserver to the identity service

Depends on D7355

Test Plan

TBD, will rebase on jon's stack to test this later

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

varun created this revision.Apr 7 2023, 10:47 AM
Herald added subscribers: atul, tomek. · View Herald TranscriptApr 7 2023, 10:47 AM
Harbormaster returned this revision to the author for changes because remote builds failed.Apr 7 2023, 10:58 AM
Harbormaster failed remote builds in B18120: Diff 24822!
varun updated this revision to Diff 24825.Apr 7 2023, 11:00 AM

fix docker build

Harbormaster returned this revision to the author for changes because remote builds failed.Apr 7 2023, 11:16 AM
Harbormaster failed remote builds in B18123: Diff 24825!
varun updated this revision to Diff 24829.Apr 7 2023, 11:20 AM

some small fixes

Harbormaster completed remote builds in B18127: Diff 24829.Apr 7 2023, 11:37 AM
varun requested review of this revision.Apr 7 2023, 11:37 AM
varun added inline comments.Apr 7 2023, 11:39 AM
keyserver/src/responders/user-responders.js
452–457 ↗(On Diff #24829)

I think it's fine to supply empty strings/arrays for these values for now since this is just for double-writing existing users. Later, when these users are on devices that talk directly to the identity service, they can provide the missing info

@ashoat let me know if i've missed something here

varun updated this revision to Diff 24832.Apr 7 2023, 12:47 PM

refactor registerUser to use existing type

varun updated this revision to Diff 24834.Apr 7 2023, 12:49 PM

rename arg

varun added a child revision: D7361: [keyserver] update login_user_wallet function.Apr 7 2023, 12:59 PM
Harbormaster completed remote builds in B18130: Diff 24832.Apr 7 2023, 1:04 PM
Harbormaster completed remote builds in B18132: Diff 24834.
• jon accepted this revision.Apr 7 2023, 1:30 PM
• jon added inline comments.
keyserver/addons/rust-node-addon/src/identity_client/register_user.rs
27–28 ↗(On Diff #24834)

there's also https://github.com/CommE2E/comm/blob/0072734eded47b495218284713f029a94bd117e3/shared/comm-opaque2/src/grpc.rs#L10

which can give slightly more meaningful errors

41 ↗(On Diff #24834)

We will need some way for the user to reconcile these later, not entirely sure how to handle the transition of missing X3DH items. But I don't think there's a current linear task for this.

This revision is now accepted and ready to land.Apr 7 2023, 1:30 PM
• jon added 1 blocking reviewer(s): ashoat.Apr 7 2023, 1:30 PM
This revision now requires review to proceed.Apr 7 2023, 1:30 PM
ashoat accepted this revision.Apr 7 2023, 6:13 PM

JS seems to follow from Rust changes. Docker changes make sense

This revision is now accepted and ready to land.Apr 7 2023, 6:13 PM
varun added inline comments.Apr 10 2023, 11:50 AM
keyserver/addons/rust-node-addon/src/identity_client/register_user.rs
27–28 ↗(On Diff #24834)

this is an NAPI Status type, not Tonic. but error handling could definitely be improved. i'll tackle this after implementing the remaining APIs

41 ↗(On Diff #24834)

created one

Closed by commit rCOMMecfcdf3660fb: [keyserver] update registerUser function (authored by varun). · Explain WhyApr 10 2023, 12:28 PM
This revision was automatically updated to reflect the committed changes.
varun added a commit: rCOMMecfcdf3660fb: [keyserver] update registerUser function.

Revision Contents
Changeset List

PathSize
keyserver/
1 line
addons/
rust-node-addon/
1 line
10 lines
6 lines
src/
identity_client/
32 lines
249 lines
src/
creators/
11 lines
responders/
2 lines

Diff 24923

View Options

keyserver/Dockerfile

Loading...
View Options

keyserver/addons/rust-node-addon/Cargo.lock

Loading...
View Options

keyserver/addons/rust-node-addon/Cargo.toml

Loading...
View Options

keyserver/addons/rust-node-addon/build.rs

Loading...
View Options

keyserver/addons/rust-node-addon/rust-binding-types.js

Loading...
View Options

keyserver/addons/rust-node-addon/src/identity_client/mod.rs

Loading...
View Options

keyserver/addons/rust-node-addon/src/identity_client/register_user.rs

Loading...
View Options

keyserver/src/creators/account-creator.js

Loading...
View Options

keyserver/src/responders/user-responders.js

Loading...