Page MenuHomePhabricator
Differential D14492

[protos][identity] Remove primary device info from GetKeyserverKeys
ClosedPublic
Actions

Authored by bartek on Mon, Mar 24, 5:54 AM.
Tags
None
Referenced Files
F5081185: D14492.id47569.diff
Sat, Mar 29, 5:09 AM
Unknown Object (File)
Fri, Mar 28, 12:16 PM
Unknown Object (File)
Fri, Mar 28, 5:26 AM
Unknown Object (File)
Thu, Mar 27, 2:32 PM
Unknown Object (File)
Thu, Mar 27, 1:47 PM
Unknown Object (File)
Thu, Mar 27, 9:34 AM
Unknown Object (File)
Thu, Mar 27, 1:50 AM
Unknown Object (File)
Wed, Mar 26, 8:16 PM
View All 9 Files
Subscribers
tomek

Details

Reviewers
ashoat
kamil
Commits
rCOMMd62e2ca88a20: [protos][identity] Remove primary device info from GetKeyserverKeys
Summary

Address ENG-10247.
We can remove this field.
Although this is backwards compatible since this field wasn't used anywhere, I marked the field as 'reserved' to avoid reusing its number. Doing so would cause parsing errors on old clients

Test Plan

flow, staging identity

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

bartek created this revision.Mon, Mar 24, 5:54 AM
bartek held this revision as a draft.
Herald added a subscriber: tomek. · View Herald TranscriptMon, Mar 24, 5:54 AM
Harbormaster completed remote builds in B33815: Diff 47526.Mon, Mar 24, 6:13 AM
bartek published this revision for review.Mon, Mar 24, 6:32 AM
bartek added inline comments.
shared/protos/identity_auth.proto
153–157 ↗(On Diff #47526)

Protobuf docs: https://protobuf.dev/programming-guides/proto3/#deleting

When you no longer need a field and all references have been deleted from client code, you may delete the field definition from the message. However, you must reserve the deleted field number. If you do not reserve the field number, it is possible for a developer to reuse that number in the future.

You should also reserve the field name to allow JSON and TextFormat encodings of your message to continue to parse.

kamil accepted this revision.Tue, Mar 25, 2:44 AM
ashoat accepted this revision.Wed, Mar 26, 7:55 AM
ashoat added inline comments.
shared/protos/identity_auth.proto
154 ↗(On Diff #47526)

Nit: I'm worried that we'll forget to adjust this to NEXT_CODE_VERSION. Is there a task where you're tracking this?

This revision is now accepted and ready to land.Wed, Mar 26, 7:55 AM
bartek added inline comments.Wed, Mar 26, 8:00 AM
shared/protos/identity_auth.proto
154 ↗(On Diff #47526)

Good call, I made sure I won't forget about it

Closed by commit rCOMMd62e2ca88a20: [protos][identity] Remove primary device info from GetKeyserverKeys (authored by bartek). · Explain WhyWed, Mar 26, 8:01 AM
This revision was automatically updated to reflect the committed changes.
bartek added a commit: rCOMMd62e2ca88a20: [protos][identity] Remove primary device info from GetKeyserverKeys.

Revision Contents
Changeset List

PathSize
services/
identity/
src/
grpc_services/
7 lines
shared/
protos/
6 lines
web/
protobufs/
6 lines

Diff 47569

View Options

services/identity/src/grpc_services/authenticated.rs

Show First 20 Lines • Show All 292 Lines • ▼ Show 20 Lines let Some(keyserver_info) = self
.get_keyserver_keys_for_user(&message.user_id) .get_keyserver_keys_for_user(&message.user_id)
.await? .await?
else { else {
return Err(Status::not_found( return Err(Status::not_found(
tonic_status_messages::KEYSERVER_NOT_FOUND, tonic_status_messages::KEYSERVER_NOT_FOUND,
)); ));
}; };
let primary_device_data = self
.db_client
.get_primary_device_data(&message.user_id)
.await?;
let primary_device_keys = primary_device_data.device_key_info;
let response = Response::new(KeyserverKeysResponse { let response = Response::new(KeyserverKeysResponse {
keyserver_info: Some(keyserver_info.into()), keyserver_info: Some(keyserver_info.into()),
identity: Some(identifier.into()), identity: Some(identifier.into()),
primary_device_identity_info: Some(primary_device_keys.into()),
}); });
return Ok(response); return Ok(response);
} }
#[tracing::instrument(skip_all)] #[tracing::instrument(skip_all)]
async fn upload_one_time_keys( async fn upload_one_time_keys(
&self, &self,
▲ Show 20 Lines • Show All 1,063 Lines • Show Last 20 Lines
View Options

shared/protos/identity_auth.proto

Show First 20 Lines • Show All 144 Lines • ▼ Show 20 Linesmessage OutboundKeyInfo {
identity.unauth.IdentityKeyInfo identity_info = 1; identity.unauth.IdentityKeyInfo identity_info = 1;
identity.unauth.Prekey content_prekey = 2; identity.unauth.Prekey content_prekey = 2;
identity.unauth.Prekey notif_prekey = 3; identity.unauth.Prekey notif_prekey = 3;
optional string one_time_content_prekey = 4; optional string one_time_content_prekey = 4;
optional string one_time_notif_prekey = 5; optional string one_time_notif_prekey = 5;
} }
message KeyserverKeysResponse { message KeyserverKeysResponse {
// this was never used, we can reuse this field again
// when min supported version is FUTURE_CODE_VERSION
reserved 3;
reserved "primary_device_identity_info";
OutboundKeyInfo keyserver_info = 1; OutboundKeyInfo keyserver_info = 1;
Identity identity = 2; Identity identity = 2;
identity.unauth.IdentityKeyInfo primary_device_identity_info = 3;
} }
// GetOutboundKeysForUser // GetOutboundKeysForUser
message OutboundKeysForUserResponse { message OutboundKeysForUserResponse {
// Map is keyed on devices' public ed25519 key used for signing // Map is keyed on devices' public ed25519 key used for signing
map<string, OutboundKeyInfo> devices = 1; map<string, OutboundKeyInfo> devices = 1;
} }
▲ Show 20 Lines • Show All 204 Lines • Show Last 20 Lines
View Options

web/protobufs/identity-auth-structs.cjs

Loading...
View Options

web/protobufs/identity-auth-structs.cjs.flow

Loading...