Paths

Table of Contentst

Differential D4285

[services][identity] wallet login
ClosedPublic
Actions

Authored by varun on Jun 16 2022, 12:46 PM.

Tags

None

Referenced Files

	Unknown Object (File)
	Tue, May 14, 4:09 PM

	Unknown Object (File)
	Tue, May 14, 4:08 PM

	Unknown Object (File)
	Thu, Apr 18, 6:25 PM

	Unknown Object (File)
	Thu, Apr 18, 6:25 PM

	Unknown Object (File)
	Thu, Apr 18, 6:25 PM

	Unknown Object (File)
	Thu, Apr 18, 6:25 PM

	Unknown Object (File)
	Thu, Apr 18, 6:25 PM

	Unknown Object (File)
	Thu, Apr 18, 6:25 PM

Subscribers

Details

Reviewers

• karol
tomek

Commits

rCOMMe7885e5a97fa: [services][identity] wallet login

Summary

Full implementation of wallet login. Steps:

Parse SIWE message and signature from request
Verify message using siwe-rs crate
If verification is successful, generate a token, store it in the token table in DynamoDB, and then return it to the caller

Note that PAKE login is not covered here. It will be in a separate diff.

Test Plan

Successfully signed in with a metamask wallet

Diff Detail

Repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

varun created this revision.Jun 16 2022, 12:46 PM

Herald added subscribers: • abosh, atul, • adrian, ashoat. · View Herald TranscriptJun 16 2022, 12:46 PM

Harbormaster returned this revision to the author for changes because remote builds failed.Jun 16 2022, 12:49 PM

Harbormaster failed remote builds in B9784: Diff 13543!

rebase

Harbormaster completed remote builds in B9786: Diff 13545.Jun 16 2022, 1:15 PM

varun requested review of this revision.Jun 16 2022, 1:15 PM

changed the type of token in the .proto file to string, so this is the corresponding code change

Harbormaster completed remote builds in B9790: Diff 13549.Jun 17 2022, 8:31 AM

New external dependencies look good

• jim requested changes to this revision.Jun 17 2022, 11:53 AM

• jim added inline comments.

services/identity/src/service.rs
71 ↗	(On Diff #13549)	Error case isn't handled
72 ↗	(On Diff #13549)	When would this be None?
73 ↗	(On Diff #13549)	This is really hard to read, please refactor. The error handling in the match arms is heavily duplicated. For starters, make a subroutine that returns a Result, and in here do the `if let Err(err) = result { log and break }` thing once.

This revision now requires changes to proceed.Jun 17 2022, 11:53 AM

create a subroutine to improve error handling

varun planned changes to this revision.Jun 23 2022, 12:52 PM

Harbormaster completed remote builds in B9930: Diff 13730.Jun 23 2022, 12:57 PM

handle unhandled error case and unhandled option case

services/identity/src/service.rs
72 ↗	(On Diff #13549)	`oneof` fields are completely optional, which is why this can be `None`. we are reconsidering the use of `oneof` across all our .proto files, but i've handled the `None` case in the latest revision.

Harbormaster completed remote builds in B9932: Diff 13732.Jun 23 2022, 1:32 PM

varun added inline comments.Jun 23 2022, 1:34 PM

services/identity/src/service.rs
87–105 ↗	(On Diff #13732)	Don't love how I'm repeating myself here but wasn't sure how to avoid it

tomek accepted this revision.Jun 24 2022, 4:37 AM

tomek added inline comments.

services/identity/src/service.rs
62 ↗	(On Diff #13732)	What is the purpose of it?
87–105 ↗	(On Diff #13732)	I think you can create a function that contains the repeated logic

varun added inline comments.Jun 24 2022, 5:28 AM

services/identity/src/service.rs
62 ↗	(On Diff #13732)	`MyIdentityService` doesn't implement `Debug`, so we have to skip `self` when we do tracing

varun added a child revision: D4350: [services][identity] helper function to start PAKE login.Jun 24 2022, 6:32 AM

rebase

Harbormaster completed remote builds in B9994: Diff 13811.Jun 27 2022, 12:39 AM

varun added a parent revision: D4360: [services][identity] PAKE workflow enum.Jun 27 2022, 12:55 AM

rebase

varun edited parent revisions, added: D4361: [services][identity] update put_token_helper to return a String; removed: D4360: [services][identity] PAKE workflow enum.Jun 27 2022, 12:58 AM

varun added a child revision: D4362: [services][identity] update Config struct to hold keypair instead of just private key.Jun 27 2022, 1:00 AM

varun removed a child revision: D4350: [services][identity] helper function to start PAKE login.

Harbormaster completed remote builds in B9997: Diff 13814.Jun 27 2022, 1:11 AM

varun removed a reviewer: • jim.Jun 27 2022, 2:45 PM

This revision is now accepted and ready to land.Jun 27 2022, 2:45 PM

Closed by commit rCOMMe7885e5a97fa: [services][identity] wallet login (authored by varun). · Explain WhyJun 27 2022, 3:04 PM

This revision was automatically updated to reflect the committed changes.

varun added a commit: rCOMMe7885e5a97fa: [services][identity] wallet login.

ashoat added inline comments.Jun 28 2022, 1:10 PM

services/identity/src/service.rs
75–123	Readability is not great here imo....

varun added inline comments.Jun 28 2022, 1:19 PM

services/identity/src/service.rs
75–123	Yeah, agreed. Made it a little better in a subsequent diff that handled PAKE login, and have a diff out to improve it further

Revision Contents
Changeset List

Path

Size

services/

identity/

2 lines

src/

1 line

140 lines

Diff 13860

services/identity/Cargo.lock

Loading...

services/identity/Cargo.toml

Loading...

services/identity/src/database.rs

Loading...

services/identity/src/service.rs

Loading...