[identity] use siwe message and signature from primary device as social proof
ClosedPublic
Actions

Authored by varun on Jan 31 2024, 2:10 PM.

Details

Reviewers

bartek
will
ashoat

Commits

rCOMM61b880c7f0e1: [identity] use siwe message and signature from primary device as social proof

Summary

we don't need the social_proof field in IdentityKeyInfo on wallet login. when we are registering a new wallet user, the SIWE message and signature are the social proof for the user. we can serialize this struct for now to avoid any DDB changes. in the future, we may want to store the signature and message separately, but @will can handle this as part of ENG-6372.

Test Plan

haven't tested yet, but two diffs after this one will test by calling the login_wallet_user RPC from native and confirming that the serialized social proof is present in DDB

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

varun created this revision.Jan 31 2024, 2:10 PM

Herald added a subscriber: tomek. · View Herald TranscriptJan 31 2024, 2:10 PM

Harbormaster completed remote builds in B26383: Diff 36478.Jan 31 2024, 2:29 PM

varun requested review of this revision.Jan 31 2024, 2:29 PM

varun added a child revision: D10904: [native] don't pass social proof string to identity.Jan 31 2024, 2:34 PM

varun edited the test plan for this revision. (Show Details)Jan 31 2024, 2:41 PM

@varun asked me to confirm that for the LogInWalletUser RPC, we should be looking at siwe_message / siwe_signature in WalletLoginRequest instead of social_proof in IdentityKeyInfo.

Confirming that here, but resigning since I don't have context on the Rust code.

Sidenote – It looks like GetInboundKeysForUser and GetOutboundKeysForUser use IdentityKeyInfo in their response types. If we remove social_proof from IdentityKeyInfo, we'll need to find another place to put it for those RPCs.

Might make sense to just fork IdentityKeyInfo. cc @will

ashoat added a subscriber: ashoat.Jan 31 2024, 3:16 PM

Looks good to me. How about removing the social_proof from DeviceKeyUpload?

In D10902#314497, @ashoat wrote:

Sidenote – It looks like GetInboundKeysForUser and GetOutboundKeysForUser use IdentityKeyInfo in their response types. If we remove social_proof from IdentityKeyInfo, we'll need to find another place to put it for those RPCs.

Might make sense to just fork IdentityKeyInfo. cc @will

This is what ENG-6481 is about. We agreed that social proof can be removed from devices' IdentityKeyInfo and moved to the identity response field.

This revision is now accepted and ready to land.Jan 31 2024, 10:53 PM

Looks good to me. How about removing the social_proof from DeviceKeyUpload?

I assume you meant the DeviceKeyUploadActions trait. I agree we can remove social_proof() from that trait and the implementation. Will make this change before landing

rebase before landing

Harbormaster completed remote builds in B26426: Diff 36537.Feb 1 2024, 10:04 AM

reordering diffs

This revision was landed with ongoing or failed builds.Feb 1 2024, 10:20 AM

Closed by commit rCOMM61b880c7f0e1: [identity] use siwe message and signature from primary device as social proof (authored by varun). · Explain Why

This revision was automatically updated to reflect the committed changes.

varun added a commit: rCOMM61b880c7f0e1: [identity] use siwe message and signature from primary device as social proof.

Harbormaster completed remote builds in B26428: Diff 36539.Feb 1 2024, 10:27 AM

In D10902#314783, @varun wrote:

Looks good to me. How about removing the social_proof from DeviceKeyUpload?

I assume you meant the DeviceKeyUploadActions trait. I agree we can remove social_proof() from that trait and the implementation. Will make this change before landing

Yes, exactly 👍