weaveworks / service

☁️ Images for Weave Cloud (R) (TM) (C) ☁️
https://cloud.weave.works
2 stars 2 forks source link

Admin user deletion may leave instances memberless #2396

Closed rndstr closed 2 years ago

rndstr commented 5 years ago

If a user is deleted in the admin, all instances where they were the only member are now memberless.

A better way would be to cascade-delete all the instances but make the admin confirm that this is going to happen.

rndstr commented 5 years ago

deleted instances without any members:

  id  |    external_id                                                                                                                                                               
------+--------------------                                                                                                                                                          
 1064 | young-glade-43                                                                                                                                                               
 592  | crimson-sound-00                                                                                                                                                             
 1024 | empty-meteor-86                                                                                                                                                              
 591  | new-sea-45                                                                                                                                                                   
 924  | shy-dew-18                                                                                                                                                                   
 925  | new-snow-84                                                                                                                                                                  
 784  | merry-sea-78                                                                                                                                                                 
 1016 | joyful-flower-60                                                                                                                                                             
 1020 | chilly-rock-91                                                                                                                                                               
 577  | crimson-shape-38                                                                                                                                                             
 923  | misty-rain-51                                                                                                                                                                
 1056 | wispy-resonance-85                                                                                                                                                           
 867  | cool-river-97                                                                                                                                                                
(13 rows) 

with query

select o.id, o.external_id
from organizations o
where o.deleted_at is null
group by o.id, o.external_id
having (
        (
        select count(*)
        from organizations om
        join memberships m on om.id=m.organization_id and m.deleted_at is null
        where om.id=o.id
       )
        +
        (
        select count(*)
        from organizations otm
        join team_memberships tm on otm.team_id=tm.team_id and tm.deleted_at is null
        where otm.id=o.id
       )
       ) = 0
;
ngehani commented 5 years ago

When you say admin, do you mean cloud.weave.works/admin interface? Does this impact customers?

rade commented 5 years ago

What problems do memberless instances cause?

rndstr commented 5 years ago

When you say admin, do you mean cloud.weave.works/admin interface?

yes

Does this impact customers?

yes, if we delete an account that was their last member then they can't access that instance anymore. they might want to do that for disabling things such as notifications or marking their data as removable from our system.

What problems do memberless instances cause?

ngehani commented 5 years ago

Does this just happen accidentally or a weaveworks person intentionally wants to do this? If intentional, why would someone internally want to delete the user via the admin interface for a customer's instance? I must be missing something.

rndstr commented 5 years ago

if someone requests to be deleted, we delete their account through the admin interface.

rade commented 5 years ago

Deleting a user through the admin interface is already supposed to delete instances that become memberless. The code says so and we have a test to check that. HOWEVER, that very test fails when the user is a team member, as happens when applying the following patch to the test code:

diff --git a/users/api/helpers_test.go b/users/api/helpers_test.go
index 672ed9f6..f4112620 100644
--- a/users/api/helpers_test.go
+++ b/users/api/helpers_test.go
@@ -144,7 +144,8 @@ func createOrgForUser(t *testing.T, u *users.User) *users.Organization {
 }

 func getOrg(t *testing.T) (*users.User, *users.Organization) {
-       return dbtest.GetOrg(t, database)
+       user, org, _ := dbtest.GetOrgAndTeam(t, database)
+       return user, org
 }

 type jsonBody map[string]interface{}

I suspect that introduction of teams has somehow broken cascading deletes of instances on user deletion.

rade commented 5 years ago

AFAIK, in the new world of teams, instances do not have any members; instead they belong to a team, which in turn has members. So in this new world, the deletion of a user should delete all teams that become memberless, and that in turn should delete all instances which now belong to these teams.