Page MenuHomePhabricator

replace single deleteUser RPC with deleteWalletUser, deletePasswordUserStart, and deletePasswordUserFinish
ClosedPublic

Authored by varun on Apr 8 2024, 7:33 AM.
Tags
None
Referenced Files
F3385379: D11587.diff
Fri, Nov 29, 12:10 AM
Unknown Object (File)
Mon, Nov 25, 12:52 PM
Unknown Object (File)
Sun, Nov 24, 9:46 AM
Unknown Object (File)
Sat, Nov 16, 5:21 PM
Unknown Object (File)
Sat, Nov 16, 5:17 PM
Unknown Object (File)
Thu, Nov 14, 12:40 PM
Unknown Object (File)
Thu, Nov 14, 3:45 AM
Unknown Object (File)
Thu, Nov 14, 12:51 AM
Subscribers

Details

Summary

we decided that we want password users to confirm their password before we delete their account on the identity service. since OPAQUE is a 2-part process, this means we need two new RPCs for password users and the existing RPC can be repurposed for wallet users.

Test Plan

tested on web and native later in the stack by calling the client methods

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

services/identity/src/client_service.rs
306 ↗(On Diff #38914)

threw this change in, but it's not really related to the diff. just noticed that we were using the wrong status here

services/identity/src/database.rs
958 ↗(On Diff #38914)

this didn't need to be pub

shared/protos/identity_auth.proto
54–57 ↗(On Diff #38914)

technically these could be unauth protos since they require a password.... but it seemed weird to split up the delete RPCs and i couldn't imagine a scenario where we'd want a user without an access token to be able to delete their account

varun requested review of this revision.Apr 8 2024, 8:04 AM

Proto changes make sense to me

web/grpc/identity-service-client-wrapper.js
133 ↗(On Diff #38914)

Hmmm... do we actually want to allow account deletion on a secondary device? I think in the whitepaper so far we are requiring the primary device to initiate. Since web can't be a primary device (at this time, anyways) this might save us some work

This revision is now accepted and ready to land.Apr 8 2024, 11:16 PM
web/grpc/identity-service-client-wrapper.js
133 ↗(On Diff #38914)

removing this

services/identity/src/grpc_services/authenticated.rs
335

This change made all requests to delete_wallet_user for password users fail. But the diff also updates the clients to call delete_wallet_user for password users. This should have been caught in review