IQSS / dataverse

Open source research data repository software
http://dataverse.org
Other
857 stars 482 forks source link

Delete user via API #4475

Closed laertecchla closed 3 years ago

laertecchla commented 6 years ago

Hi, I would like to have a delete button on the Manage Users page in order to exclude an account. Is it possible?

pdurbin commented 6 years ago

@laertecchla thanks for your feedback.

For more context, @laertecchla and I were discussing the lack of a delete button on the "Manage Users" table in the dashboard I explained how to use curl as a workaround to delete the user: http://irclog.iq.harvard.edu/dataverse/2018-02-26#i_63734

ajs6f commented 6 years ago

As of now, as far as I can tell, the "Manage Users" page is actually just a list of users, and the only options available are the "Superuser" check box or "Remove All Roles". Do I understand correctly that any other management task is to be performed using curl or other HTTP client?

pdurbin commented 6 years ago

@ajs6f good point. What other users managements tasks would you like to perform?

ajs6f commented 6 years ago

I don't think they are unusual: Add/remove users, edit user metadata, issue password resets, add/remove roles, etc.

ajs6f commented 6 years ago

As a side note, this is not just a convenience for me: I will be filing a bug in a few minutes that in our new evaluation installlation, new users (who sign themselves) up are created with no roles at all (I checked the db to be sure) and the UI is failing for them. I would like to quickly correct that situation for at least some users so that we can go on with our evaluation of the product, but it now seems that I will have to learn something of the Dataverse API to do it. I.e. this issue is preventing us from even evaluating Dataverse as a product.

pdurbin commented 6 years ago

@ajs6f I'm sorry to hear about your trouble. Please do open that issue because I'm confused about why the UI is failing for new users (not having any roles is normal).

2029 was closed a while ago for security reasons. It has a title of "SuperUser: grant ability to see and edit all user info and reset passwords (like we did in 3.6 as network admins)" and it referred to the previous generation of Dataverse, which was DVN 3. I'm not sure what the current thinking is.

@laertecchla do you and @ajs6f want the same things? Originally, you only wanted a delete button. I'm asking because we try to work in small chunks. The smaller the change, the faster it moves across our kanban board at https://waffle.io/IQSS/dataverse

ajs6f commented 6 years ago

I didn't mean to hijack this ticket. If you want to split up my larger request for different user management functions and leave this one as delete only, that's fine with me.

pdurbin commented 6 years ago

@ajs6f the conversation is way more valuable to me than our process, but yes, we might end up breaking the work into smaller issues, as needed. First I want to understand what the needs are. I do agree that the things you're asking for are very common in the type of software Dataverse is. I tend to think of it as "enterprise" software but maybe there's a better term for it.

laertecchla commented 6 years ago

@pdurbin I haven't understood what @ajs6f wants at all. Yes, I only wanted a delete button in order to exclude a user...

pdurbin commented 6 years ago

Heh. Ok, thanks, @laertecchla

@ajs6f if you would create a new issue about the user management functions you're interested in, it would be most appreciated!

ajs6f commented 6 years ago

I'd rather take the conversation over to https://github.com/IQSS/dataverse/issues/4515 for now, for reasons that will become apparent in my next comment to that ticket.

pdurbin commented 5 years ago

@laertecchla are you still interested in a delete button for users? You might be interested in the " User accounts - should users be able to request deletion of their accounts?" thread at https://groups.google.com/d/msg/dataverse-community/nGVCS5L3vPk/GSjkx1-xCAAJ

laertecchla commented 5 years ago

Hi Philip. Thank you. Laerte.

De: "Philip Durbin" notifications@github.com Para: "IQSS/dataverse" dataverse@noreply.github.com Cc: "laertecchla" laerte@cchla.ufpb.br, "Mention" mention@noreply.github.com Enviadas: Sexta-feira, 13 de julho de 2018 0:14:03 Assunto: Re: [IQSS/dataverse] Admin Manage Users (#4475)

[ https://github.com/laertecchla | @laertecchla ] are you still interested in a delete button for users? You might be interested in the " User accounts - should users be able to request deletion of their accounts?" thread at [ https://groups.google.com/d/msg/dataverse-community/nGVCS5L3vPk/GSjkx1-xCAAJ | https://groups.google.com/d/msg/dataverse-community/nGVCS5L3vPk/GSjkx1-xCAAJ ]

— You are receiving this because you were mentioned. Reply to this email directly, [ https://github.com/IQSS/dataverse/issues/4475#issuecomment-404713174 | view it on GitHub ] , or [ https://github.com/notifications/unsubscribe-auth/AjJ--2E2C-GDt-wlcX26oxhbFoLCCgMcks5uGBB7gaJpZM4ST440 | mute the thread ] .

pdurbin commented 5 years ago

@laertecchla you're welcome. Are you still interested in this issue? If so, can you please give it a more specific title?

laertecchla commented 5 years ago

Philip, I am still interested in this issue for sure. But, at the moment, only on technical aspect once I have been created some users for test in dataverse.ufpb.br and I would like to exclude some of them. I've been reading Janet McDougall post as well as your post about delete a user and I've been concluded that we have got an unresolved question... So, a more specific title would be: how can I delete a user created for tests? Thanks. Laerte.

De: "Philip Durbin" notifications@github.com Para: "IQSS/dataverse" dataverse@noreply.github.com Cc: "laertecchla" laerte@cchla.ufpb.br, "Mention" mention@noreply.github.com Enviadas: Quarta-feira, 18 de julho de 2018 12:50:23 Assunto: Re: [IQSS/dataverse] Admin Manage Users (#4475)

[ https://github.com/laertecchla | @laertecchla ] you're welcome. Are you still interested in this issue? If so, can you please give it a more specific title?

— You are receiving this because you were mentioned. Reply to this email directly, [ https://github.com/IQSS/dataverse/issues/4475#issuecomment-405980205 | view it on GitHub ] , or [ https://github.com/notifications/unsubscribe-auth/AjJ--32rn0pqW0d968sjnlgX0GDYFWKwks5uH1k_gaJpZM4ST440 | mute the thread ] .

pdurbin commented 5 years ago

@laertecchla the new title sounds fine to me so I changed it. It reminds me of #1929 and you might find some useful information in there. If not, please let us know how we can help!

laertecchla commented 5 years ago

Peter, I've read #1929 and your answer to @amberleahey drawn my attention. In that case, how is it possible to remove the user "sparrow", for instance? Laerte.

De: "Philip Durbin" notifications@github.com Para: "IQSS/dataverse" dataverse@noreply.github.com Cc: "laertecchla" laerte@cchla.ufpb.br, "Mention" mention@noreply.github.com Enviadas: Quarta-feira, 18 de julho de 2018 14:09:22 Assunto: Re: [IQSS/dataverse] how can I delete a user created for tests? (#4475)

[ https://github.com/laertecchla | @laertecchla ] the new title sounds fine to me so I changed it. It reminds me of [ https://github.com/IQSS/dataverse/issues/1929 | #1929 ] and you might find some useful information in there. If not, please let us know how we can help!

— You are receiving this because you were mentioned. Reply to this email directly, [ https://github.com/IQSS/dataverse/issues/4475#issuecomment-406005776 | view it on GitHub ] , or [ https://github.com/notifications/unsubscribe-auth/AjJ--yIqLHrlxGb_XsIxnAS2MxFrzPxqks5uH2vCgaJpZM4ST440 | mute the thread ] .

pdurbin commented 5 years ago

@laertecchla it's Phil not Peter but that's ok. 😄

Like this:

curl -X DELETE http://localhost:8080/api/admin/authenticatedUsers/sparrow
laertecchla commented 5 years ago

Sorry, Phil! I have two more doubts:

  1. Why dataverse hasn't got a button to remove a user like "sparrow"?
  2. If sparrow had got datasets, the comand [curl -X DELETE http...] could remove the datasets as well? Thanks. Laerte.

De: "Philip Durbin" notifications@github.com Para: "IQSS/dataverse" dataverse@noreply.github.com Cc: "laertecchla" laerte@cchla.ufpb.br, "Mention" mention@noreply.github.com Enviadas: Sexta-feira, 20 de julho de 2018 11:03:22 Assunto: Re: [IQSS/dataverse] how can I delete a user created for tests? (#4475)

[ https://github.com/laertecchla | @laertecchla ] it's Phil not Peter but that's ok. \uD83D\uDE04

Like this: curl -X DELETE http://localhost:8080/api/admin/authenticatedUsers/sparrow

— You are receiving this because you were mentioned. Reply to this email directly, [ https://github.com/IQSS/dataverse/issues/4475#issuecomment-406610160 | view it on GitHub ] , or [ https://github.com/notifications/unsubscribe-auth/AjJ--5Hvd2i0X0HYmoedd-QY6XOakfoDks5uIeMqgaJpZM4ST440 | mute the thread ] .

pdurbin commented 5 years ago

@laertecchla as #2419 says, "Removing the account altogether would create a hole in existing datasets." It sounds a little scary to me to give superusers a button to click that deletes a bunch of datasets. What if the datasets have been published and have a real DOI and that DOI has been cited? We use a special word for deleting published datasets: destroy. And we don't offer a button for datasets to be destroyed. You have to use the API (but it's not currently documented, which is what #2593 is about).

Is this helping? Please keep the questions coming? We should be figuring out what the "definition of done" is for this issue. 😄

adam3smith commented 5 years ago

We'd like such a button (for superusers), too. Not urgently, but it'd be more convenient than to use the API. The issue with owned content is very real, though. Here's what Drupal offers when deleting user accounts. Maybe DV could consider something similar, potentially without the delete user and content option? image

pdurbin commented 5 years ago

@adam3smith that "anonymous user" concept is interesting. I will say that @sekmiller @matthew-a-dunlap @scolapasta and I have been thinking about users a lot lately because there's been a lot of discussion around pull request #5626 which has to do with merging accounts (#5514). A lot of the same problems apply.

I'm curious what your experience with deleting users via API has been. Does it usually work? Do you do some checking of related tables before you attempt it? Maybe the next "small chunk" would be to improve the delete user API to do more pre-flight checking of related tables. You can see a list of related tables in the pull request above.

adam3smith commented 5 years ago

We don't do it a lot and we (i.e. @qqmyers ) do check related tables. Adding preflight checks to the API would definitely help (and be a useful step/precondition towards an eventual UI solution) and could then likely be expanded to include methods for deletion and eventually a button if that's not considered too risky. Thanks!

pdurbin commented 5 years ago

@adam3smith thanks. I think of #1929 as the issue for adding pre-flight checks since this issue (#4475) feel like it's more about adding a GUI.

@laertecchla is that right? You'd like a button in the GUI?

@ajs6f are you still with us? 😄 I hope you're well!

adam3smith commented 5 years ago

Just to be clear, I also want a button in the UI, I'll just also take related improvements ;)

pdurbin commented 5 years ago

@adam3smith sure. Mostly I'm just wondering if we should change the title of this issue to say something about a GUI. You are, of course, welcome to create new issues. It's easier to discuss and estimate issues where the scope is clear. Often we'll try to build the API first in one issue. Then we'll try to add a GUI later in a later issue. Of course, when a pull request from the community comes along, we don't have to spend as much time discussing scope. The scope in that case is whatever is implemented in the pull request. All we have to think about is, "Do we want this or not?" More often than not, we want it. Free code! 😄

scolapasta commented 5 years ago

@adam3smith @pdurbin Yes, the reason we have been wary to add the current API to the UI (for deletable users) or add to documentation as a fully supported API is the fact that it does not do this pre flight stuff, nor does it handle deleting roleassignments.

So, I think the steps could be:

  1. Clean up current API to make sure it will not orphan any roleassignments when/if the user is deleted. (additionally we'll want it to use a command and log something in the actionlogrecord, i.e. override the describe method, to make it clear the user was deleted for auditing purposes.
  2. Add an method to determine if a user is deletable.
  3. Use (2) to add a button the deletable users on the dashboard (plus confirmation popup, validation msg's, et al).
  4. Consider how to handle non deletable users*.

(*) we have considered doing something similar to Drupal to anonymize users downloads, but didn't want to do that with content that was created as it. For downloads it can also be tricky - for example, with green data tags, we require a user is logged in, so that they can be contacted in the event that the data needs to be deleted. By anonymizing the download, we (obviously) can't contact them anymore.

laertecchla commented 5 years ago

@pdurbin, I am a little bit lost here, but I've captured a print from users/groups section and attached it. So, do you mean something about the "removed assigned role" button? Captura_Remove_Button

pdurbin commented 5 years ago

@laertecchla hi! On that screenshot, do you want a "Delete user" button?

laertecchla commented 5 years ago

Hi, @pdurbin . Yes, I would like to have this button. I would like to exclude a user as I can do it in DSpace... Thanks.

pdurbin commented 5 years ago

@laertecchla ok, I just updated the title of this issue. If you think a different title would be better, please go ahead and change it. Thanks!

laertecchla commented 5 years ago

ok, @pdurbin. I follow you. Thanks!

scolapasta commented 4 years ago

Discussed with @djbrooke and we'll implement the first two steps above (possibly the 3rd, depending on UX team bandwidth). Step 4 is more about considering how to handle deleting users that have actual stuff (whether datasets or downloads) and will need discussion with others on what we want to allow..

mheppler commented 4 years ago

Re: UI/UX, should be low impact. The Manage Users pg is simply a table, so the impact is merely adding another column with a button in it, linked to an "Are you sure..." confirmation popup, as we use on most delete actions.

djbrooke commented 4 years ago

@mheppler @scolapasta Yeah, it'll be simple to handle in the UI when we can delete all users, but in the meantime we'd also need to build in messaging for why some (most?) deletions will fail. I don't feel strongly about this, but that was my thinking. We can discuss for sure, and I'm excited about this moving forward.

scolapasta commented 4 years ago

To be clear, my suggestion was to add the delete for users you could delete (so delete should never fail). But we would need messaging explaining why only those get the button.

scolapasta commented 4 years ago

Going forward for other users, the things will need to consider is when a user has done "something". Basically either created something (a dataverse or dataset), made changes (as we track them as a contributor) or downloaded something. In these cases we've discussed potentially anonymizing the info*, but we definitely need to consider that more, especially in the cases of downloaded Green files (Green files require having contact info from the downloaders).

File downloads should already be nullable (as we do allow anonymous users to download public files). Other affected tables would be things like dvObject (for created by user) and DatasetContributors. In those cases, the columns are currently not null.

djbrooke commented 3 years ago

Retitling to reflect that we'll implement this API-based first. We have an endpoint (https://guides.dataverse.org/en/latest/api/native-api.html#delete-a-user) but it can't delete users who have taken action in the system (as @scolapasta mentions above).

djbrooke commented 3 years ago
TaniaSchlatter commented 3 years ago

This was the topic of discussion in the design meeting today.

pdurbin commented 3 years ago

I just created pull request #7629 to add a "disable user account" feature and right now it's marked to close this issue. I recognize that this issue's title has changed from "As a superuser, I would like a button to delete a user" to "Delete user via API" and that it's gotten so long that some comments are hidden. I'd suggest a fresh issue for any unfinished business.