Page MenuHomePhabricator

[identity] GetSessionInitializationInfo RPC
ClosedPublic

Authored by varun on Feb 27 2023, 2:36 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Nov 27, 9:54 PM
Unknown Object (File)
Wed, Nov 27, 8:50 PM
Unknown Object (File)
Wed, Nov 27, 6:32 PM
Unknown Object (File)
Fri, Nov 8, 3:50 PM
Unknown Object (File)
Fri, Nov 8, 2:40 PM
Unknown Object (File)
Fri, Nov 8, 2:37 PM
Unknown Object (File)
Fri, Nov 8, 2:29 PM
Unknown Object (File)
Fri, Nov 8, 2:15 PM
Subscribers

Details

Summary

clients will call this RPC with a user ID and get in response a map of session initialization info (public keys, social proof, etc.) it needs, keyed by the device's ed25119 signing public key.

adding @ashoat to OK the API design

Test Plan

called this RPC from the keyserver and got back the test data i created in DDB. also tested the failure case with an invalid user ID and got back an exception in JS.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

services/identity/src/service.rs
286–289 ↗(On Diff #23182)

the DB client gives us back a HashMap<String, HashMap<String, String>>. We need to wrap the inner HashMap in a SessionInitializationInfo struct to satisfy the protobuf definition

shared/protos/identity.proto
88–90 ↗(On Diff #23182)

protobuf doesn't allow defining nested maps inline, but it's happy if we wrap the inner map in a message

varun requested review of this revision.Feb 27 2023, 2:56 PM
ashoat requested changes to this revision.Feb 27 2023, 4:16 PM
ashoat added a subscriber: marcin.
ashoat added inline comments.
shared/protos/identity.proto
89 ↗(On Diff #23182)

Can you add an inline comment explaining what the keys and values are?

I think for the four keys we're starting with, @atul and @marcin have settled on a nested JSON format (see D6822).

Perhaps we could have three keys here:

  1. Nested JSON of all four keys as string
  2. Signature for that string
  3. Social proof, if it exists
196 ↗(On Diff #23182)

Can you add an inline comment explaining what it's keyed on?

This revision now requires changes to proceed.Feb 27 2023, 4:16 PM
This revision is now accepted and ready to land.Feb 27 2023, 7:10 PM
shared/protos/identity.proto
90 ↗(On Diff #23208)

Might be worth explaining that this is where the signing ed25519 key is to be found, and that the signature part refers to primary (not notification)

shared/protos/identity.proto
89 ↗(On Diff #23182)

Really not a fan of having a having a heterogenous collection of values in a hashmap. Would much rather see:

message SessionInitializationInfo {
  string payloadJSON = 1;
  string payloadEd25519Signature = 2;
  optional string socialProof = 3;
}

It helps with compile time checking, bindings, and reasoning about the shape of your data.

Yeah but it makes it much harder whenever we want to change it. Protobufs / gRPC don't handle versioning of schemas very well. @jon, do you have a good story on how we'll handle it if we need to add a new entry?

In general we need to land this ASAP, so would prefer to prioritize finishing up the monthly goal over further discussion here. Maybe we can create a stub task and discuss there once everything is landed?

This revision was automatically updated to reflect the committed changes.