Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API: Deleting a user does not clean up roles, can introduce undesirable behavior. #1929

Closed
kcondon opened this issue Apr 8, 2015 · 14 comments
Labels

Comments

@kcondon
Copy link
Contributor

kcondon commented Apr 8, 2015

Phil indicated that the delete user api only deletes the user and does not clean up any associated roles, nor does it check whether it will orphan any object: dv, ds. This could lead to undesirable behavior, perhaps security issues.

@pdurbin
Copy link
Member

pdurbin commented Apr 9, 2015

Related is the "Stop using @pete and @uma in role assignments" issue: #1151

@scolapasta
Copy link
Contributor

What do you mean by orphan any objects?

@kcondon
Copy link
Contributor Author

kcondon commented Apr 9, 2015

Well, I assume there needs to be an admin of a dataverse, no? Anything else that assumes a specific user?


From: Gustavo Durand [notifications@github.com]
Sent: Wednesday, April 08, 2015 11:58 PM
To: IQSS/dataverse
Cc: Condon, Kevin
Subject: Re: [dataverse] API: Deleting a user does not clean up roles, can introduce undesirable behavior. (#1929)

What do you mean by orphan any objects?


Reply to this email directly or view it on GitHubhttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_IQSS_dataverse_issues_1929-23issuecomment-2D91109636&d=BQMCaQ&c=WO-RGvefibhHBZq3fL85hQ&r=TUpjWt9sVfaAC8ETCY_cDPtqJKl7s242PLg6-Wx6UpM&m=40b6BP8U7lZeCgQyA6CYL2kakz9bjwNmRU-Anb-PcJg&s=T3WPqqnFWFsEqxEfECp-T7uIVUnkHTZHyz20pWFL85w&e=.

@scolapasta
Copy link
Contributor

This API is in the admin block and superuser only, so we will defer to 4.0.1 to review how we want to handle.

@scolapasta scolapasta modified the milestones: 4.0.1, In Review - 4.0 Apr 9, 2015
@scolapasta scolapasta self-assigned this Apr 18, 2015
@mercecrosas mercecrosas modified the milestones: Candidates for 4.0.2, Candidates for 4.0.1 May 8, 2015
@scolapasta scolapasta modified the milestones: Candidates for 4.0.2, In Review Jul 2, 2015
@scolapasta scolapasta removed their assignment Jan 27, 2016
@scolapasta scolapasta removed this from the Not Assigned to a Release milestone Jan 28, 2016
@kcondon kcondon added Type: Feature a feature request and removed Type: Bug a defect labels Apr 1, 2016
@kcondon
Copy link
Contributor Author

kcondon commented Apr 1, 2016

Making a feature because it is currently internal, unsupported.

@pdurbin
Copy link
Member

pdurbin commented Apr 1, 2016

#2825 may be a duplicate of this issue. Also, #2419 about how user accounts can't be disabled (which was possible in DVN 3.x) is highly related.

@kcondon
Copy link
Contributor Author

kcondon commented Apr 14, 2016

Combining ticket #2825 which is to improve messaging when deleting user that has history, causing delete to fail.

@pdurbin
Copy link
Member

pdurbin commented Jun 6, 2017

Since this came up in the community call today, I'll mention that while deleting users is possible, it's not especially recommended until this issue is resolved. It's documented at http://guides.dataverse.org/en/4.6.2/installation/shibboleth.html#exchange-metadata-with-your-identity-provider and looks like this:

curl -X DELETE http://localhost:8080/api/admin/authenticatedUsers/myself

@amberleahey
Copy link

Hi @pdurbin , is this the latest issue related to deleting users? We would like to be able to delete a user who's e-mail is no longer valid and has moved on, using the Dashboard UI as superadmin. They've since been removed from most known datasets but there is a chance we missed something so we don't want to introduce issues by deleting them using the DELETE call. Thanks for any updates!

@pdurbin
Copy link
Member

pdurbin commented Nov 30, 2017

@amberleahey it sounds like you're saying you already clicked the "Remove All" button in the new-ish "Manage Users" dashboard:

screen shot 2017-11-30 at 10 51 56 am

That new button should go a long way in mitigating this only issue (#1929). Before you delete the user, you should probably check if the user was the creator of any dataverses, datasets, or files. I don't think there's an API for this, so can you please suggest an addition to the Google doc linked from #4169 ask for help defining this query?

What's wrong with leaving the user in the database anyway? 😄 What problem does it solve to remove the user from the database? Should we add a "delete user" button?

@scolapasta
Copy link
Contributor

Some more background:

Part of the reason we don't allow deleting if users is that if they have done something, like create a dataverse or dataset, or even download, that is data that we want to keep. So we need to keep the row in the authenticated user table.

We used to have the concept of deactivate, but decided in favor of the above "remove all roles".

@amberleahey
Copy link

Thanks @pdurbin I don't see the remove all button, this must be new! We are running 4.7.1
We think we've removed all roles but could use a query to help with confirming this, will add to doc. Nothing wrong per se, just piece of mind our database is clean :)

@pdurbin
Copy link
Member

pdurbin commented Nov 30, 2017

@amberleahey yep, that button is new as of 4.8, implemented in #4055.

@pdurbin
Copy link
Member

pdurbin commented Jan 16, 2018

Related: #4356

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants