Page MenuHomePhabricator
Differential D11687

[identity] Allow UpdateDeviceList RPC to remove devices
ClosedPublic
Actions

Authored by bartek on Thu, Apr 18, 6:16 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Apr 30, 2:50 PM
Unknown Object (File)
Tue, Apr 30, 2:39 PM
Unknown Object (File)
Sat, Apr 27, 11:27 PM
Unknown Object (File)
Sat, Apr 27, 6:44 PM
Unknown Object (File)
Thu, Apr 25, 12:02 PM
Unknown Object (File)
Wed, Apr 24, 1:42 PM
Unknown Object (File)
Wed, Apr 24, 10:34 AM
Unknown Object (File)
Tue, Apr 23, 12:28 PM
View All 11 Files
Subscribers
ashoat
tomek

Details

Reviewers
varun
Commits
rCOMMe69f415a5bd8: [identity] Allow UpdateDeviceList RPC to remove devices
Summary

Updated device list validation logic. Added possibility to remove a device in UpdateDeviceList RPC.

Depends on D11686

Test Plan

Called the RPC with a few scenarios:

  • Add one device - pass
  • Add two devices - fails
  • Remove one device - pass
  • Remove two devices - fail
  • Remove primary device - fail
  • Both remove and add device - fail
  • Keep list unchanged - fail

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

bartek created this revision.Thu, Apr 18, 6:16 AM
bartek held this revision as a draft.
Herald added subscribers: tomek, ashoat. · View Herald TranscriptThu, Apr 18, 6:16 AM
Harbormaster failed remote builds in B28293: Diff 39217!Thu, Apr 18, 6:38 AM
bartek updated this revision to Diff 39270.Fri, Apr 19, 2:33 AM

Rebase

Harbormaster completed remote builds in B28320: Diff 39270.Fri, Apr 19, 2:54 AM
bartek published this revision for review.Fri, Apr 19, 3:41 AM
bartek added inline comments.
services/identity/src/device_list.rs
89–91 ↗(On Diff #39270)

Currently, the UpdateDeviceList RPC is used only when adding or [soon] logging out a secondary device.
In the future diffs, I'll add the possibility of e.g. replacing a secondary device with another one (replacing a keyserver when we already have one).

We don't do primary device switch with this RPC so it shouldn't be able to do this operation. There's a reason - we're switching primary device only with Identity-managed device list updates because we need to do it in sync with updating social proof for ETH users. Otherwise, we could end up with having social proof for device that is no longer the primary one.

bartek edited the summary of this revision. (Show Details)Fri, Apr 19, 3:43 AM
bartek edited the test plan for this revision. (Show Details)
bartek added a child revision: D11711: [identity] Add verification for replacing a device.Mon, Apr 22, 6:46 AM
varun accepted this revision.Tue, Apr 23, 5:49 PM
varun added inline comments.
services/identity/src/device_list.rs
67 ↗(On Diff #39270)
102 ↗(On Diff #39270)
This revision is now accepted and ready to land.Tue, Apr 23, 5:49 PM
bartek updated this revision to Diff 39478.Thu, Apr 25, 12:08 AM

Address feedback

Harbormaster completed remote builds in B28461: Diff 39478.Thu, Apr 25, 12:24 AM
Closed by commit rCOMMe69f415a5bd8: [identity] Allow UpdateDeviceList RPC to remove devices (authored by bartek). · Explain WhyThu, Apr 25, 12:37 AM
This revision was automatically updated to reflect the committed changes.
bartek added a commit: rCOMMe69f415a5bd8: [identity] Allow UpdateDeviceList RPC to remove devices.

Revision Contents
Changeset List

PathSize
services/
identity/
src/
104 lines
grpc_services/
10 lines

Diff 39481

View Options

services/identity/src/device_list.rs

Loading...
View Options

services/identity/src/grpc_services/authenticated.rs

Loading...