Page MenuHomePhabricator
Differential D11547

[protos] Refactor user Identity message type
ClosedPublic
Actions

Authored by bartek on Thu, Apr 4, 4:22 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Apr 26, 4:03 PM
Unknown Object (File)
Fri, Apr 26, 1:53 PM
Unknown Object (File)
Tue, Apr 16, 8:40 PM
Unknown Object (File)
Fri, Apr 12, 11:55 AM
Unknown Object (File)
Thu, Apr 11, 10:31 PM
Unknown Object (File)
Wed, Apr 10, 4:01 PM
Unknown Object (File)
Wed, Apr 10, 3:45 PM
Unknown Object (File)
Wed, Apr 10, 10:18 AM
View All 13 Files
Subscribers
tomek

Details

Reviewers
ashoat
varun
will
Commits
rCOMM883fffc6adc1: [protos] Refactor user Identity message type
Summary

Addresses ENG-7532.

  • Replaced oneof with username and optional EthIdentity
  • Replaced social_proof with separate siwe message and signature fields

Depends on D11546

Test Plan

Registered password and wallet user. Called FindUserIdentity for both and verified the respponses.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

bartek created this revision.Thu, Apr 4, 4:22 AM
bartek held this revision as a draft.
Herald added a subscriber: tomek. · View Herald TranscriptThu, Apr 4, 4:22 AM
bartek added a child revision: D11548: [identity] Store social proof as a Map in DDB.Thu, Apr 4, 4:24 AM
bartek added a reviewer: will.Thu, Apr 4, 4:27 AM
Harbormaster failed remote builds in B27966: Diff 38761!Thu, Apr 4, 4:45 AM
bartek updated this revision to Diff 38764.Thu, Apr 4, 4:49 AM

Fix commtest

Harbormaster completed remote builds in B27968: Diff 38764.Thu, Apr 4, 5:05 AM
bartek published this revision for review.Thu, Apr 4, 5:06 AM
bartek added inline comments.
services/identity/src/siwe.rs
49–58 ↗(On Diff #38764)

This is temporary. I get rid of this serialize-then-deserialize pattern in the next diff (D11548)

ashoat accepted this revision.Fri, Apr 5, 6:46 AM
varun requested changes to this revision.Fri, Apr 5, 7:21 AM
varun added inline comments.
keyserver/addons/rust-node-addon/src/identity_client/get_inbound_keys_for_user.rs
39–51 ↗(On Diff #38764)

how about something like this?

we eventually expect users to have usernames and wallet addresses, so i'm wondering if there's any need to have them be mutually exclusive here. we could then simplify this js code in user-responders.js:

const username = inboundKeysForUser.username
  ? inboundKeysForUser.username
  : inboundKeysForUser.walletAddress;

becomes

const username = inboundKeysForUser.username;
services/identity/src/grpc_utils.rs
257 ↗(On Diff #38764)

did you mean just "failed to construct wallet identity" ?

This revision now requires changes to proceed.Fri, Apr 5, 7:21 AM
bartek added inline comments.Sun, Apr 7, 11:53 PM
keyserver/addons/rust-node-addon/src/identity_client/get_inbound_keys_for_user.rs
39–51 ↗(On Diff #38764)

I'm not opposed to that.
I simply kept the previous logic here but can update it if you think that fits better.

services/identity/src/grpc_utils.rs
257 ↗(On Diff #38764)

yep, thanks

bartek updated this revision to Diff 38883.Mon, Apr 8, 12:24 AM

Address feedback

Harbormaster completed remote builds in B28037: Diff 38883.Mon, Apr 8, 12:40 AM
varun accepted this revision.Wed, Apr 10, 10:17 AM
This revision is now accepted and ready to land.Wed, Apr 10, 10:17 AM
Closed by commit rCOMM883fffc6adc1: [protos] Refactor user Identity message type (authored by bartek). · Explain WhyMon, Apr 15, 1:44 AM
This revision was automatically updated to reflect the committed changes.
bartek added a commit: rCOMM883fffc6adc1: [protos] Refactor user Identity message type.

Revision Contents
Changeset List

PathSize
keyserver/
addons/
rust-node-addon/
src/
identity_client/
22 lines
services/
commtest/
tests/
4 lines
identity/
src/
grpc_services/
35 lines
35 lines
19 lines
shared/
protos/
10 lines
web/
protobufs/
14 lines

Diff 39103

View Options

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

Loading...
View Options

services/commtest/tests/identity_integration_tests.rs

Loading...
View Options

services/identity/src/grpc_services/authenticated.rs

Loading...
View Options

services/identity/src/grpc_utils.rs

Loading...
View Options

services/identity/src/siwe.rs

Loading...
View Options

shared/protos/identity_auth.proto

Loading...
View Options

web/protobufs/identity-auth-structs.cjs

Loading...
View Options

web/protobufs/identity-auth-structs.cjs.flow

Loading...