Cannot delete a user who owns releases.

[Cryptophobia]

From @mattk42 on February 14, 2017 22:17

I have a user who was an admin on our cluster and had pushed to multiple apps that he was not the owner for.

After transferring ownership of the apps that he owns to another user, I get the following when trying to delete him.

$ deis auth:cancel --username=someuser cancel account someuser at https://deis.mycluster.com? (y/N): y Error: Unknown Error (409): {"detail":"(\"Cannot delete some instances of model 'User' because they are referenced through a protected foreign key: 'Release.owner'\", <QuerySet [<Release: some-app-v7>, <Release: another-app-v10>]

Copied from original issue: deis/controller#1244

[Cryptophobia]

From @bacongobbler on February 14, 2017 22:24

Which version was this on? I believe this should be fixed in v2.11.0 with deis apps:transfer but I could be wrong.

[Cryptophobia]

From @mattk42 on February 14, 2017 22:48

Oops, sorry for not including that. I am on v2.11.0. I did deis apps:transfer for the apps that he owned but the releases he is attached to otherwise are apps he had deployed to but did not actually own.

[Cryptophobia]

From @mattk42 on February 14, 2017 22:54

As a workaround, I was able to to transfer those specific apps over to the user I want to delete and then back. This updated the release objects to now be owned by the correct user and I was able to delete the user in question.

I am not sure how better to handle the case where a user owns releases for an application but not the application itself, but it does seem like it may need some thought.

[undeadops]

So, I've had this issue also, I'll write some notes here about what I had to do. My main workaround has been to just remove all access from the user accounts and change their password to some random long string. Starting to get quite a few users that are no longer with the company so I started in to get them removed.

Changing user password with: deis auth:passwd --username=<username>

What I want to do is completely remove the user:

deis auth:cancel --username=<username>

This fails because the user is the owner of a few apps still. I found the apps they're the owner of with the following quick shell for loop:

for a in `deis apps:list | egrep -v '^==='`; do echo $a; deis apps:info -a $a | egrep '^owner: '; done;

After transferring ownership on those apps, I get try to remove the account again.
This results in an Error

Error: Unknown Error (409): {"detail":"("Cannot delete some instances of model 'User' because they are referenced through a protected foreign key: 'Release.owner'", <QuerySet [<Release: stockimporter-prod-v12>, <Release: stockimporter-prod-v11>, <Release: stockimporter-prod-v10>, <Release: stockimporter-prod-v9>, <Release: stockimporter-prod-v8>, <Release: stockimporter-prod-v7>, <Release: stockimporter-prod-v6>, <Release: stockimporter-prod-v5>, <Release: stockimporter-stage-v9>, <Release: stockimporter-stage-v8>, <Release: stockimporter-stage-v7>, <Release: stockimporter-stage-v6>, <Release: stockimporter-stage-v5>, <Release: watchman-stage-v25>, <Release: watchman-stage-v23>, <Release: watchman-stage-v22>, <Release: watchman-stage-v21>, <Release: watchman-stage-v19>, <Release: watchman-stage-v17>, <Release: watchman-stage-v14>, '...(remaining elements truncated)...']>)"}

To get past this, I run a SQL query against the Database of:

UPDATE public.api_release SET owner_id = 6 WHERE owner_id = 11;

Where user_id 6 is a CI user account that won't be going anywhere, and user_id 11 was the user i'm trying to remove.

Running the cancel user again from above, and I get a new error:

Error: Unknown Error (409): {"detail":"("Cannot delete some instances of model 'User' because they are referenced through a protected foreign key: 'Config.owner'", <QuerySet [<Config: stockimporter-prod-da1a28d>, <Config: stockimporter-prod-17ae864>, <Config: stockimporter-prod-9cefce1>, <Config: stockimporter-prod-72fc1ea>, <Config: stockimporter-prod-9bca503>, <Config: stockimporter-prod-d387ff3>, <Config: stockimporter-prod-b045be1>, <Config: stockimporter-prod-a212c7a>, <Config: stockimporter-stage-e08f4fb>, <Config: stockimporter-stage-fafe033>, <Config: stockimporter-stage-bfe789f>, <Config: stockimporter-stage-13a636a>, <Config: stockimporter-stage-76d00eb>, <Config: watchman-stage-f6978cd>, <Config: watchman-stage-18552e0>, <Config: watchman-stage-13c29d0>, <Config: watchman-stage-0d42794>, <Config: watchman-stage-c58a2d3>, <Config: watchman-stage-7bbd97d>, <Config: watchman-stage-87447db>, '...(remaining elements truncated)...']>)"}

Then Fixing that table with the following SQL:

UPDATE public.api_config SET owner_id = 6 WHERE owner_id = 11;

New Error:

Error: Unknown Error (409): {"detail":"("Cannot delete some instances of model 'User' because they are referenced through a protected foreign key: 'AppSettings.owner'", <QuerySet [<AppSettings: watchman-stage-75c3256>, <AppSettings: watchman-stage-07a7ce4>]>)"}

Fixing that one up with SQL:

UPDATE public.api_appsettings SET owner_id = 6 WHERE owner_id = 11;

Rerunning the cancel user again, and it was successful this time.

[undeadops]

I'm willing to take a look into fixing this, just would need some direction on what to do with the foreign keys, or if adding an additional option to transfer ownership to a specific user essentially like I did above is the correct approach

[undeadops]

Looks like there is another table that will be affected... when I'm removing another user:

Error: Unknown Error (409): {"detail":"("Cannot delete some instances of model 'User' because they are referenced through a protected foreign key: 'Build.owner'", <QuerySet [<Build: bender-13e1208>, <Build: bender-1577cce>, <Build: bender-fb91b2d>, <Build: bender-1e2023a>, <Build: bender-0535ce2>, <Build: bender-0e06ceb>, <Build: bender-ebd068f>, <Build: bender-3762810>]>)"}

[kingdonb]

Thank you very much @undeadops for posting the follow-up, this is definitely something we will want to address as part of User/App lifecycle. When an app is transferred through the "transfer" API verb, we should probably not be leaving builds, app settings, configs, or releases associated with the old owner. The database is doing the right thing by refusing to orphan them.

... actually I'm reading the history on this bug, and it sounds like the problem is not the apps that user owned, which can be transferred, but the other apps they had access to and collaborated on! The builds/releases/other artifacts are apparently transferred properly when that user has ownership of the app, but for example any config that I pushed to an app which someone else has configured to allow my access on, is labeled as belonging to me (not as belonging to the owner of the app.)

So, the solution which was used back in 2017 when this issue was last discussed, was to transfer the apps in question to the user-to-delete, and then transfer them away, which cleans up all those foreign key references (since any references associated with the app to transfer are transferred with, apparently.) That's obviously not an optimal solution, transferring to the user to delete. At least the current situation guarantees that we will not leave any orphan references. We should maintain that guarantee.

What you did with the database tables is maybe a little less wrong, but obviously still much less than optimal from UX perspective.

When I worked with users provisioning in a previous role, we came up with a solution to prevent orphaned artifacts (talking about Google Docs specifically) where a Docs Czar was appointed, and any time someone's Gapps account was going to be deleted, we would transfer all of their owned docs to the Docs Czar through a portal that Google provided. This is a regular part of the account lifecycle... we should support a --transfer-to option on the auth:cancel verb, in similar fashion.

Transferring to the CI user seems like the right thing to do, if you have one, since any other change (reassigning to another real user) could be misinterpreted as attribution. If you don't want to transfer your stuff anywhere, I don't think auth:cancel is going to be prepared to handle that because of the possibility of orphaned references. I don't think we want to add a "just nuke everything" option, although that could be up for debate.

Other alternative possible ways of handling this I'm open to suggestions?

[dmcnaught]

I usually just do a deis auth:regenerate -u username so that the account is locked

[undeadops]

I would vote for a --transfer-to option And yes, all my SQL updates were pointed at that CI users ID.

[Cryptophobia]

I would vote for a --transfer-to option And yes, all my SQL updates were pointed at that CI users ID.

I agree that --transfer-to option is probably the way to go and overwriting the foreign keys with the user_id of the admin who performs the command.

I would not discount that --force-delete option may also be useful which should nuke all the apps, configs, releases, and delete that user.

@undeadops would you like to take a look at an initial PR for this? It would require changes in https://github.com/teamhephy/controller and in https://github.com/teamhephy/workflow-cli. Thank you for taking a look into this.

[kingdonb]

So, the solution which was used back in 2017 when this issue was last discussed, was to transfer the apps in question to the user-to-delete, and then transfer them away, which cleans up all those foreign key references (since any references associated with the app to transfer are transferred with, apparently.) That's obviously not an optimal solution, transferring to the user to delete. At least the current situation guarantees that we will not leave any orphan references. We should maintain that guarantee.

If we are going to support --force-delete it needs to balk by default, unless you provide also --confirm=username in the spirit of --confirm=<app> on deis apps:destroy command that exists. Since it could transitively destroy a bunch of apps, in fact, I'd argue it needs to be more strongly protected than even just that.

(Like maybe this is strong enough confirmation:

/bin/yes | deis auth:cancel --force-delete --confirm=<username> --username=<username>

Well, there is already --yes in there too maybe, but this option is serious business...

[Cryptophobia]

Yeah, I agree. It should be behind several flags and confirmations if possible.

[kingdonb]

Probably both --confirm=<username> and --username=<username> with the --force-delete flag, for the admin who deletes another user's account. Only --confirm=<username> when it's a user cancelling their own account with --force-delete.

(With the other confirmations already mentioned. --yes as an alternative to pipe from /bin/yes or instead typing 'y' for yourself.)

That's probably less important than the --transfer-to behavior though.