Skip to content

docs for disable/delete/anon, need to move to user admin section#7585

Closed
scolapasta wants to merge 9 commits intodevelopfrom
4475-delete-user-api
Closed

docs for disable/delete/anon, need to move to user admin section#7585
scolapasta wants to merge 9 commits intodevelopfrom
4475-delete-user-api

Conversation

@scolapasta
Copy link
Contributor

What this PR does / why we need it:
docs for delete
Which issue(s) this PR closes:

Closes #

Special notes for your reviewer:
work in progress.

Suggestions on how to test this:

Does this PR introduce a user interface change? If mockups are available, please link/include them here:

Is there a release notes update needed for this change?:

Additional documentation:

@scolapasta scolapasta marked this pull request as draft February 5, 2021 19:09
@djbrooke djbrooke self-assigned this Feb 8, 2021
@djbrooke
Copy link
Contributor

@pdurbin @scolapasta I've made the changes discussed last week and the docs reflect my current understanding of where we're planning to go with this.

Thoughts on what to do with this draft PR? @pdurbin I know you're into the code on another branch, so if you want to merge this into your branch, great, or we can do something else if y'all have better ideas.

Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a few comments.

With regard to getting this content into my branch, worst case we can just copy and paste it in at the end.

- Prevent a user from being assigned any roles
- Cancel any pending file access requests generated by the user
- Remove the user from all groups
- No longer have notifications generated or sent by the Dataverse installation
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to add something that prevents a user from recreating a disabled account with the same username and/or email.

Copy link
Contributor

@djbrooke djbrooke Feb 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mheppler Can you talk a bit more about why? Is this the case of a malicious user or something else?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first bullet here suggests we disable a user account in order to "disable the user's ability to log in to the Dataverse installation"... seems obvious that we don't leave the front door of the house open if we're trying to add a lock to the back door. I don't have a specific use case in mind. I don't see a list of use cases here in the guides, so I can't point to an existing one. Not trying to expand the scope or make more work for anyone. Just making sure we achieve what we set out to do.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mheppler thanks. As a concrete example, you're saying that if I have an account with danny@harvard.edu that gets disabled by an admin, I wouldn't be able to create a new account using danny@harvard.edu?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, email or username, both fields are validated on create to be unique.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just chiming in to make sure I understand. You both seem to be using "builtin" user examples (which is fine) but it applies equally to shib/oauth/oidc. For example, if my ORCID account is disabled on the Dataverse side, I am no longer able to create a Dataverse account using my ORCID account. Instead of a conflict of email addresses (like with builtin users) what's conflicting is the unique identifier within ORCID. Same with Shib (eppn is the unique identifier we use), etc.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed e9845c0 to reflect my understanding but am not super happy with the phrasing. Feedback welcome.

- Prevent a user from being assigned any roles
- Cancel any pending file access requests generated by the user
- Remove the user from all groups
- No longer have notifications generated or sent by the Dataverse installation
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just chiming in to make sure I understand. You both seem to be using "builtin" user examples (which is fine) but it applies equally to shib/oauth/oidc. For example, if my ORCID account is disabled on the Dataverse side, I am no longer able to create a Dataverse account using my ORCID account. Instead of a conflict of email addresses (like with builtin users) what's conflicting is the unique identifier within ORCID. Same with Shib (eppn is the unique identifier we use), etc.


- The user's contributions to datasets, including dataset creation, file uploads, and publishing.
- The user's access history to datafiles in the Dataverse installation, including guestbook records.
- The user's account information (specifically name, email, affiliation, and position)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just wanted to note that currently the authenticateduserlookup rows and builtinuser rows (in the case of Builtin users) are also being kept. There has been a suggestion of deleting these rows (which would surely close the door to a "undisable" option in the future) but I haven't explored it at all. I'm getting closer (I hope) to pushing some working code that we can talk about.

@pdurbin pdurbin mentioned this pull request Feb 19, 2021
@pdurbin
Copy link
Member

pdurbin commented Feb 19, 2021

Closing in favor of pull request #7629. The branch has been merged there.

@pdurbin pdurbin closed this Feb 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants