Page MenuHomePhabricator
Differential D10049

[grpc clients] Update Rust lib to use authenticated RPCs
ClosedPublic
Actions

Authored by bartek on Nov 27 2023, 4:22 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, May 23, 4:36 AM
Unknown Object (File)
Thu, May 23, 4:36 AM
Unknown Object (File)
Thu, May 23, 4:36 AM
Unknown Object (File)
Thu, May 23, 4:36 AM
Unknown Object (File)
Thu, May 23, 4:36 AM
Unknown Object (File)
Thu, May 23, 4:36 AM
Unknown Object (File)
Thu, May 23, 4:36 AM
Unknown Object (File)
Mon, May 20, 6:58 AM
View All 30 Files
Subscribers
ashoat
tomek

Details

Reviewers
varun
Commits
rCOMMac34eefc8e8b: [grpc clients] Update Rust lib to use authenticated RPCs
Summary

Updated the client method to use the auth proto RPCs for GetOutboundKeys and GetInboundKeys.
Needed to add more params, cuz we need two distinct userID/deviceID pairs: for current user (auth) and for the user we want the keys for.

Depends on D10048.

Test Plan

Native and keyserver compiles. These functions aren't called anywhere at the moment in the JS code

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

bartek created this revision.Nov 27 2023, 4:22 AM
bartek held this revision as a draft.
Herald added subscribers: tomek, ashoat. · View Herald TranscriptNov 27 2023, 4:22 AM
bartek added a child revision: D10050: [identity] Delete unauthenticated X3DH RPCs.Nov 27 2023, 4:23 AM
Harbormaster failed remote builds in B24494: Diff 33699!Nov 27 2023, 4:55 AM
bartek updated this revision to Diff 33705.Nov 27 2023, 5:11 AM

Fix broken rebase

bartek retitled this revision from [native] Update Rust lib to use authenticated GetOutboundKeys to [grpc clients] Update Rust lib to use authenticated RPCs.Nov 27 2023, 5:12 AM
bartek edited the summary of this revision. (Show Details)
bartek edited the test plan for this revision. (Show Details)
Harbormaster failed remote builds in B24500: Diff 33705!Nov 27 2023, 5:20 AM
bartek updated this revision to Diff 33707.Nov 27 2023, 5:31 AM

One more fix

Harbormaster failed remote builds in B24502: Diff 33707!Nov 27 2023, 5:41 AM
bartek updated this revision to Diff 33718.Nov 27 2023, 5:55 AM

Rebase

Harbormaster completed remote builds in B24512: Diff 33718.Nov 27 2023, 7:57 AM
bartek published this revision for review.Nov 28 2023, 8:04 AM
varun accepted this revision.Nov 29 2023, 6:16 AM
This revision is now accepted and ready to land.Nov 29 2023, 6:16 AM
varun added a comment.Nov 29 2023, 6:21 AM

can we improve the test plan here? just want to make sure these calls work as expected

bartek added a comment.Dec 6 2023, 12:40 AM
In D10049#293825, @varun wrote:

can we improve the test plan here? just want to make sure these calls work as expected

Sure, tested this once again:

  • Started Identity Service
  • Registered a signle-device user manually (user A).

Keyserver part:

  • Started Keyserver, (filed the credentials file in secrets), made sure it registered in Identity (user B)
  • Called getInboundKeys of userA while given auth credentialsfor user B (the keyserver user) - console.log'ged the result (did this at the end of registerOrLogin() for simplicity)

Native part:

  • Did kind of opposite - used loginPasswordUser to log in as user A
  • Called getOutboundKeys for user B, given userA's auth, then printed the results (stringified JSON with prekeys appeared)
Closed by commit rCOMMac34eefc8e8b: [grpc clients] Update Rust lib to use authenticated RPCs (authored by bartek). · Explain WhyDec 6 2023, 3:17 AM
This revision was automatically updated to reflect the committed changes.
bartek added a commit: rCOMMac34eefc8e8b: [grpc clients] Update Rust lib to use authenticated RPCs.
bartek added a comment.Dec 6 2023, 3:19 AM

Forgot to attach screenshots:

Screenshot 2023-12-06 at 12.11.21.png (359×897 px, 132 KB)
Screenshot 2023-12-06 at 12.11.39.png (403×583 px, 86 KB)

Some values deliberately hardcoded to make sure they come from the place I'm expecting. Got it using

awslocal dynamodb scan --table-name identity-users
ashoat added a comment.Dec 6 2023, 7:36 AM

Those screenshots need to be "attached" on Phabricator still (see here)

bartek attached a referenced file: F914015: Screenshot 2023-12-06 at 12.11.39.png. (Show Details)Dec 6 2023, 7:37 AM
bartek attached a referenced file: F914016: Screenshot 2023-12-06 at 12.11.21.png. (Show Details)

Revision Contents
Changeset List

PathSize
keyserver/
addons/
rust-node-addon/
6 lines
src/
identity_client/
25 lines
native/
cpp/
CommonCpp/
NativeModules/
6 lines
16 lines
_generated/
native_rust_library/
src/
66 lines
schema/
6 lines

Diff 34329

View Options

keyserver/addons/rust-node-addon/rust-binding-types.js

Loading...
View Options

keyserver/addons/rust-node-addon/src/identity_client/get_inbound_keys_for_user.rs

Loading...
View Options

native/cpp/CommonCpp/NativeModules/CommRustModule.h

Loading...
View Options

native/cpp/CommonCpp/NativeModules/CommRustModule.cpp

Loading...
View Options

native/cpp/CommonCpp/_generated/rustJSI-generated.cpp

Loading...
View Options

native/cpp/CommonCpp/_generated/rustJSI.h

Loading...
View Options

native/native_rust_library/src/lib.rs

Loading...
View Options

native/schema/CommRustModuleSchema.js

Loading...