Page MenuHomePhabricator

[identity] fail account deletion if we can't delete tunnelbroker or backup data
ClosedPublic

Authored by varun on Sep 26 2024, 9:27 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 8, 2:54 PM
Unknown Object (File)
Fri, Nov 8, 2:51 PM
Unknown Object (File)
Fri, Nov 8, 1:39 AM
Unknown Object (File)
Fri, Nov 8, 1:35 AM
Unknown Object (File)
Fri, Nov 8, 1:21 AM
Unknown Object (File)
Sat, Nov 2, 1:50 AM
Unknown Object (File)
Fri, Nov 1, 5:06 PM
Unknown Object (File)
Fri, Nov 1, 5:06 PM
Subscribers

Details

Summary

account deletion should fail if we can't delete data from other services. we should also attempt to delete this data first so that if we're unable to, the user is still able to call the delete RPCs on identity.

Test Plan

temporarily disabled tunnelbroker and called delete_wallet_user. it failed, as expected. enabled tunnelbroker again and delete_wallet_user succeeded

Diff Detail

Repository
rCOMM Comm
Branch
delete
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Sep 26 2024, 9:31 PM
Harbormaster failed remote builds in B31891: Diff 44615!
varun requested review of this revision.Sep 26 2024, 9:48 PM
bartek added inline comments.
services/identity/src/grpc_services/authenticated.rs
626

You can also remove the let device_ids = [...[].get_current_device_list(&user_id) call inside delete_user() and make it return Result<()>.
It unnecessarily fetches device list now

This revision is now accepted and ready to land.Sep 26 2024, 11:01 PM
services/identity/src/grpc_services/authenticated.rs
626

Never mind, didn't notice the next diff

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

I kept this function because I think it's fine if we spawn a new task for tunnelbroker data deletion during the logout workflows