Page MenuHomePhabricator

[services][identity] helper function to start PAKE registration
ClosedPublic

Authored by varun on Jun 24 2022, 8:56 AM.
Tags
None
Referenced Files
F3388585: D4354.id13819.diff
Fri, Nov 29, 3:02 PM
Unknown Object (File)
Wed, Nov 27, 3:45 PM
Unknown Object (File)
Wed, Nov 27, 3:42 PM
Unknown Object (File)
Wed, Nov 27, 1:47 PM
Unknown Object (File)
Thu, Nov 21, 6:05 PM
Unknown Object (File)
Wed, Nov 13, 2:08 AM
Unknown Object (File)
Wed, Nov 13, 2:08 AM
Unknown Object (File)
Wed, Nov 13, 2:08 AM

Details

Summary

checks the state of the stream (that the number of messages received matches expectation), starts PAKE registration, updates state, and returns a RegistrationResponse

Depends on D4352

Test Plan

Tested along with the following two diffs by calling the register_user RPC

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Jun 24 2022, 9:05 AM
Harbormaster failed remote builds in B9953: Diff 13766!
varun requested review of this revision.Jun 24 2022, 9:11 AM

Change Option<String> variables to String vars

tomek requested changes to this revision.Jun 27 2022, 4:26 AM
tomek added inline comments.
services/identity/src/service.rs
82–83 ↗(On Diff #13819)

Maybe String::new()?

90 ↗(On Diff #13819)

What if this expression is false? Is it an error or something we should safely ignore?

95–107 ↗(On Diff #13819)

There are a lot of options to reduce the indentation, e.g. by creating more variables

112–113 ↗(On Diff #13819)

Is it possible for a single expression to do this assignment?

574 ↗(On Diff #13819)

We should avoid modifying the parameters and instead return result value

This revision now requires changes to proceed.Jun 27 2022, 4:26 AM
varun marked 2 inline comments as done.

address feedback

services/identity/src/service.rs
90 ↗(On Diff #13819)

this was handled in a subsequent diff, but now that i'm refactoring this code to make it flatter, i'm combining this case with the "message is Error" case since we handle them the same

95–107 ↗(On Diff #13819)

i've flattened by changing the patterns in the match statement. i can do this, too, if you think the code is still too nested

112–113 ↗(On Diff #13819)

no i think that only works if registration_response_and_server_registration is a tuple

This revision is now accepted and ready to land.Jun 28 2022, 6:44 AM