Page MenuHomePhabricator
Differential D4352

[services][identity] PAKE login
ClosedPublic
Actions

Authored by varun on Jun 24 2022, 6:36 AM.
Tags
None
Referenced Files
F3115406: D4352.id13853.diff
Thu, Oct 31, 9:28 PM
Unknown Object (File)
Sat, Oct 26, 4:49 AM
Unknown Object (File)
Tue, Oct 22, 2:07 AM
Unknown Object (File)
Mon, Oct 21, 2:20 AM
Unknown Object (File)
Sun, Oct 20, 1:45 PM
Unknown Object (File)
Fri, Oct 18, 6:09 PM
Unknown Object (File)
Thu, Oct 10, 1:44 AM
Unknown Object (File)
Thu, Oct 10, 1:44 AM
View All Files
Subscribers
• abosh
• adrian
ashoat
atul

Details

Reviewers
tomek
• jim
• karol
Commits
rCOMMc38e924fcbe6: [services][identity] PAKE login
Summary

Implementation of the login_user RPC. Most of the logic is in the subroutines for starting and finishing login.

We track the number of messages received to make sure the client is not misusing the stream.

Depends on D4351

Test Plan

Called login_user RPC, tested all failure paths and the success case

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

varun created this revision.Jun 24 2022, 6:36 AM
Herald added subscribers: • abosh, atul, • adrian, ashoat. · View Herald TranscriptJun 24 2022, 6:36 AM
Harbormaster completed remote builds in B9948: Diff 13759.Jun 24 2022, 6:42 AM
varun requested review of this revision.Jun 24 2022, 6:42 AM
varun added 1 blocking reviewer(s): • jim.Jun 24 2022, 8:31 AM
varun added a child revision: D4354: [services][identity] helper function to start PAKE registration.Jun 24 2022, 8:56 AM
varun updated this revision to Diff 13818.Jun 27 2022, 1:02 AM

rebase

Harbormaster completed remote builds in B10001: Diff 13818.Jun 27 2022, 1:33 AM
tomek requested changes to this revision.Jun 27 2022, 4:15 AM

The indentation here is quite extreme - could you find a way to reduce it? Restructure the code, create separate functions, etc.

services/identity/src/service.rs
87–88 ↗(On Diff #13818)

For me String::new() better describes the intention here

This revision now requires changes to proceed.Jun 27 2022, 4:15 AM
varun updated this revision to Diff 13853.Jun 27 2022, 1:02 PM
varun marked an inline comment as done.

Decrease indentation and change "".to_string() to String::new()

Harbormaster completed remote builds in B10018: Diff 13853.Jun 27 2022, 1:08 PM
tomek accepted this revision.Jun 28 2022, 5:07 AM

Looks a lot more readable - thanks!

services/identity/src/service.rs
93–96 ↗(On Diff #13853)

Nice!

122–142 ↗(On Diff #13853)

What do you think about moving most of this logic outside of the if condition?

varun removed 1 blocking reviewer(s): • jim.Jun 28 2022, 6:44 AM
This revision is now accepted and ready to land.Jun 28 2022, 6:44 AM
varun added inline comments.Jun 28 2022, 6:49 AM
services/identity/src/service.rs
122–142 ↗(On Diff #13853)

I'm fine with this

varun added inline comments.Jun 28 2022, 7:10 AM
services/identity/src/service.rs
122–142 ↗(On Diff #13853)

I should do this in a bunch of places so will do in a separate diff actually

Closed by commit rCOMMc38e924fcbe6: [services][identity] PAKE login (authored by varun). · Explain WhyJun 28 2022, 7:12 AM
This revision was automatically updated to reflect the committed changes.
varun added a commit: rCOMMc38e924fcbe6: [services][identity] PAKE login.

Revision Contents
Changeset List

PathSize
services/
identity/
src/
120 lines

Diff 13897

View Options

services/identity/src/service.rs

Loading...