Page MenuHomePhabricator

[Protos] Add UpdateUser definitions
ClosedPublic

Authored by jon on Feb 28 2023, 3:51 AM.
Tags
None
Referenced Files
F1439383: D6912.diff
Thu, Mar 28, 7:25 PM
F1433294: D6912.diff
Thu, Mar 28, 7:00 AM
Unknown Object (File)
Feb 27 2024, 11:49 PM
Unknown Object (File)
Feb 27 2024, 11:49 PM
Unknown Object (File)
Feb 27 2024, 11:49 PM
Unknown Object (File)
Feb 27 2024, 11:49 PM
Unknown Object (File)
Feb 27 2024, 11:49 PM
Unknown Object (File)
Feb 27 2024, 11:49 PM
Subscribers

Details

Summary

gRPC definition of updateUser protocol

This is primarily meant to garner feedback, while
I still work implmenting the vertical slice with
keyserver.

Notable differences from RegisterUser is that the
final PAKE Register message and first PAKE Login messages are kept
separate. This is done to separate concerns around
the two PAKE actions for https://linear.app/comm/issue/ENG-3149

https://linear.app/comm/issue/ENG-2763

Test Plan

This is mostly intended to be protocol oriented

cd services/identity
cargo build

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.Feb 28 2023, 3:58 AM
Harbormaster failed remote builds in B16985: Diff 23225!

could we just do exactly what we do for RegisterUser today with an additional check that the accessToken is valid? i understand wanting to make the code DRY and decoupling the parts a bit, but I'd rather we backlog that work and just get this out and working

varun requested changes to this revision.Feb 28 2023, 11:47 AM
This revision now requires changes to proceed.Feb 28 2023, 11:47 AM

Match registration shape exactly

Use user_id instead of username for identitifying users

API looks good as far as I can tell, but I don't really understand the OPAQUE protocol definition so mostly deferring to you

Reuse registerUser request message

varun requested changes to this revision.Mar 2 2023, 7:30 PM

We need PakeRegistrationRequestAndUserID plus the existing access token for the user in the first UpdateUserRequest message, right?

This revision now requires changes to proceed.Mar 2 2023, 7:30 PM

We talked about the usage of access tokens, and decided that we can defer doing access_token validation when the client (not keyserver) is able to provide them.

Defer to you guys, I don't know enough about PAKE to comment on the .proto, and I don't know Rust so can't comment on the rest

Let's capitalize Identity Service everywhere in the .proto file for consistency. otherwise looks good

This revision is now accepted and ready to land.Mar 3 2023, 1:46 PM

Apply feedback, rebase on master

This revision was automatically updated to reflect the committed changes.