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

User account cannot be disabled/deactivated #2419

Closed
bencomp opened this issue Jul 28, 2015 · 14 comments · Fixed by #7629
Closed

User account cannot be disabled/deactivated #2419

bencomp opened this issue Jul 28, 2015 · 14 comments · Fixed by #7629

Comments

@bencomp
Copy link
Contributor

bencomp commented Jul 28, 2015

In DVN 3.6, a Network administrator could deactivate a user account from the network options. This prevented the user from logging in and has been useful in cases that a dataverse creator left an organisation and should be prevented admin access to the dataverse. Unless this user had a Shibboleth account, the organisation has little control over access to the user's account used for Dataverse. Removing the account altogether would create a hole in existing datasets.

A work-around could be to change the account's password and email address to values that the original user has no access to, but this could conflict with #2170 and does not make clear that an account is disabled.

We have deactivated accounts in our DVN 3.6 installation and plan to clean them up, but we may need to migrate some of them and preferably keep them disabled.

(Discussed on IRC: http://irclog.iq.harvard.edu/dataverse/2015-07-28#i_22050)

@bencomp
Copy link
Contributor Author

bencomp commented May 11, 2016

Looking into this issue again, I don't think it's really possible to "clean up" inactive user accounts as we don't want to change history if they have a link with any dataverse, study or other item.

Any ideas?

@pdurbin
Copy link
Member

pdurbin commented May 11, 2016

@bencomp "clean up"? Can you please provide a "user story" from the point of view of someone who is a superuser (and/or has access to the database) of a Dataverse installation?

@bencomp
Copy link
Contributor Author

bencomp commented May 11, 2016

I thought to remove deactivated accounts that have no history, i.e. that did not create or contribute to dataverses or studies. The only way to do this is via direct database access.

But removing accounts does not seem to be an option when they have history. I'm about to send an email to the migration list about this.

@scolapasta
Copy link
Contributor

@bencomp one current way to effectively deactivate users is to remove all role assignments. In that way, while they could still login, they can't do anything special. This is a change from 3.6 where creators had inherit permission; in 4.0 that is decoupled. (when you create something, you are given a role at create time, but that role can be revoked).

@pdurbin
Copy link
Member

pdurbin commented Jun 1, 2016

In talking with @mcrosas and @kcondon I believe we'll want to provide the ability to disable/deactivate accounts as part of the Sensitive Data project so I'm assigning this issue to myself for now.

@pdurbin pdurbin self-assigned this Jun 1, 2016
@pdurbin
Copy link
Member

pdurbin commented Jun 3, 2016

Specifically, http://policy.security.harvard.edu/sa6-appropriate-user-acccess says, "SA6: Users must only be permitted to access a server or application after their current business need for access has been established." Under "How to Comply" it says, "Disable account access if user leaves University or changes jobs such that they no longer have a business need to access the information; the best way to do this is by using the central authentication service." For Harvard users who log in to Dataverse through HarvardKey (Harvard's authentication system), when they lose their access to HarvardKey, they will lose access to Dataverse unless the account is migrated to a local account (#2915).

@bencomp I just saw your comment about https://groups.google.com/d/msg/dataverse-migration-wg/NJFS0_-C7nA/ZfKpzRoEFQAJ and I don't have an authoritative answer for you but I'm in the process of gathering security-related issues under my name to help figure out the scope of work for the Support for Sensitive Data project. Please sit tight while we figure this out. 😄

@jonc1438
Copy link

Odum will be needing this too. Our case would be a situation where an Admin wants to remove a users that has left an organization. Especially for case of sensitive data.
I think step one would be a deactivation of login that would occur either manually by sysadmin or by repeated login failures. Step two would be a final cleanup of the records and movement of related objects to another user maybe default to sysadmin.

Not sure how Shib complicated the issues

@pdurbin
Copy link
Member

pdurbin commented Jul 25, 2016

@jonc1438 hi! There's a lot to unpack in your comment but I wanted to clarify that for #3153 we are working on having repeated login failures lock an account temporarily. What you're calling deactivation of login I'm calling "locking an account indefinitely." It's just an implementation detail, I guess.

I already pinged @donsizemore at http://irclog.iq.harvard.edu/dataverse/2016-07-21#i_38624 to ask if Odum would like to play around with the code I'm working on. It's not a pull request but I just created a new branch in the main IQSS repo. Details of how to run the code (there's a SQL update) are at #3153 (comment)

I'm hoping this approach of locking accounts will be suitable. I'm eager for feedback on the code I've been writing, the bulk of which is at becc694.

@pdurbin
Copy link
Member

pdurbin commented Aug 4, 2016

Today @mheppler and I looked at the latest code in the 3153-2419-lockuser branch (v. 4.4 build 3153-2419-lockuser-4f05d67 deployed to https://dev1.dataverse.org ).

When a user is disabled/deactivated and attempts to log in it currently says the user is locked out until the year 9999 (this is an allowed date in the SQL spec from what I understand) but we'd like it to say "indefinitely" instead.

Mike will weigh in on other possible changes as well (and may go ahead and make the changes in the branch, if he's ready) so I'll co-assign this issue to him.

pdurbin added a commit that referenced this issue Aug 19, 2016
Conflicts:
src/main/java/edu/harvard/iq/dataverse/api/Admin.java
src/main/java/edu/harvard/iq/dataverse/authorization/AuthenticationServiceBean.java
@pdurbin
Copy link
Member

pdurbin commented Sep 2, 2016

Per @djbrooke the Sensitive Data project is being deferred until Dataverse 5.0 so I'm unassigning myself since I like having https://github.com/IQSS/dataverse/issues/assigned/pdurbin reflect what I'm actually working on. I'll also add the 5.0 milestone.

@pdurbin pdurbin removed their assignment Sep 2, 2016
pdurbin added a commit that referenced this issue Feb 26, 2021
That means it's safe not to have this check.
pdurbin added a commit that referenced this issue Mar 4, 2021
Conflicts:
src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java
(just moved a comment back into place)
pdurbin added a commit that referenced this issue Mar 4, 2021
pdurbin added a commit that referenced this issue Mar 16, 2021
pdurbin added a commit that referenced this issue Mar 16, 2021
This should have been included in 4f81fbd
pdurbin added a commit that referenced this issue Mar 17, 2021
Restore the query to what it was in previous releases.
Put the logic in the method.
pdurbin added a commit that referenced this issue Mar 18, 2021
Previously, in 3e59e70 we added logic to check if the user in the
session has been deleted or deactivated. The problem is that this code
is called often so now we only do this check if the user tries to go to
the account page or tries to execute a command.
pdurbin added a commit that referenced this issue Mar 22, 2021
pdurbin added a commit that referenced this issue Mar 23, 2021
pdurbin added a commit that referenced this issue Mar 25, 2021
The createAuthenticatedUserForView method can't handle a null
for the deactivated boolean so we'll set the boolean to false
for existing users. For new users, this is set to false already.
pdurbin added a commit that referenced this issue Mar 25, 2021
Conflicts (just imports):
src/main/java/edu/harvard/iq/dataverse/api/Admin.java
sekmiller added a commit that referenced this issue Mar 26, 2021
sekmiller added a commit that referenced this issue Mar 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants