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.

Sign up for GitHub

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

Jump to bottom

Improvements to User Management #2838

Closed
mbellotti opened this issue Jan 17, 2016 · 18 comments
Closed

Improvements to User Management #2838

mbellotti opened this issue Jan 17, 2016 · 18 comments
Assignees
@rossjones

Comments

@mbellotti
Copy link
Contributor

There are a couple things that bother me about the way CKAN handles Users. Emails are not validated and multiple accounts can be registered under one email. This encourages spam attacks. It also makes logging in by email impossible.

On HDX we rewrote a lot of the user backend to fix this problem and also to add the ability to extend users with extras same way one might with groups or datasets. Would definitely be interested in taking that work and rolling it into CKAN core.

Thoughts?
@rossjones
Copy link
Contributor

I'd be keen to see this and user-extras in CKAN core, although might involve some care that we don't lock out users in existing installations.

@rossjones rossjones self-assigned this Jan 19, 2016
@rossjones
Copy link
Contributor

Sorry for the delay getting back, entirely my fault.

This was discussed at the meeting and all thought it may be best to split these into two, solely to avoid potential problems with existing instances that don't have the single email restriction.

We thought it would be great to get email validation and single-email restriction into CKAN as a core-extension, which is a normal plugin that is distributed with CKAN at https://github.com/ckan/ckan/tree/master/ckanext - that way it is simple to activate and won't cause problems for those that have existing duplicate emails, they can migrate and activate when ready. Particularly if it were possible to activate them independently.

The user-extras, all believed would be easier to implement, and much more appropriate as a core part of CKAN. Presumably it'd also need an IUserForm interface in the same way that they exist for datasets.

Does that seem like a good approach?
If you're happy with that, is there anything you might need to help you get started with it?

I'd also suggest it would make sense to start the PR early, so that it's easier for others to spot potential issues early and save you some time, hopefully, in the long run. Marking it as WIP will mean that it doesn't get merged prematurely.

@mbellotti
Copy link
Contributor Author

How far a long is the IAuthenticator Interface now? It is currently still listed as "experimental" in the docs and this made it pretty difficult to design these changes as an extension. It ended up being easier to add a little code in the core to allow login via email: https://github.com/OCHA-DAP/hdx-ckan/blob/65fb4f74bda4eed21fc8070b1e30493902cdcf07/ckan/lib/authenticator.py#L22

@rossjones
Copy link
Contributor

I doubt there would be objections to login-by-email, as long as the validation of re-used email addresses and validating emails was in the extension (presumably in the logic layer and a controller for the validation url). So putting login-by-email into core with user-extras should be fine.

@mbellotti
Copy link
Contributor Author

Okay. I got I good idea of how I want to start this. One housekeeping question though, should the branch be based off master of some specific release branch?

@rossjones
Copy link
Contributor

Master would be best.

This was referenced Feb 10, 2016
Closed
Closed
@tsufz
Copy link

tsufz commented Mar 1, 2016

Is there a good reason why a user is not deleted from an organisation if the user is deleted from the system? In my opinion, the users should be deleted recursively in order to avoid zombie user entries. I use ckan 2.4.

@amercader
Copy link
Member

@tsufz If that's the case that sounds like a bug. Could you create a new issue for this?

@tsufz
Copy link

tsufz commented Mar 1, 2016

Well, I try to repeat the case in fresh installations of 2.4 and 2.5 and
open the issue if reproducible.

@tsufz https://github.com/tsufz If that's the case that sounds like a
bug. Could you create a new issue for this?

—
Reply to this email directly or view it on GitHub
#2838 (comment).

@mevey
Copy link

mevey commented Sep 30, 2016

Hey @rossjones. Was this user email verification issue addressed?

@rossjones
Copy link
Contributor

@mevey not as far as I'm aware - at least I didn't see a PR.

@tsufz
Copy link

tsufz commented Oct 4, 2016

sorry, I neither create an issue nor checked the versions as promised in order of other duties...

@tsufz
Copy link

tsufz commented Oct 4, 2016

Hi, I created issue #3265.

@kmbn
Copy link
Contributor

kmbn commented Jan 22, 2019

We decided to close old issues that are not actively worked on so that we can focus our effort and attention on issues affecting the current versions of CKAN.

If this issue is still affecting the version of CKAN you're working with now, please feel free to comment or reopen the issue.

If you do reopen this issue, please update it with new details. One reason it might not have been resolved in the past is that it wasn't clear how a contributor could address the issue.

@kmbn kmbn closed this as completed Jan 22, 2019
@MandanaMoshref
Copy link

Is there any update for this task/idea?

@mutantsan
Copy link
Contributor

If validate an email for unique is a good idea, i would like to work on it.

@MandanaMoshref
Copy link

If validate an email for unique is a good idea, i would like to work on it.

For me personally there are couple of things which are important:

  1. For new registration:
    a. confirmation email
    b. unique email validation

I have already sent an email to ckan-dev group and stated exactly what I am looking for:
https://lists.okfn.org/pipermail/ckan-dev/2019-November/023331.html

I have seen couple of closed issues but now solutions....

mutantsan pushed a commit to mutantsan/ckan that referenced this issue Nov 26, 2019
[ckan#2838] Provide email validation for unique
97641a8 
Currently there is no email validation for unique, so it's possible
to create multiple accounts with the same email. Because of this,
the is no way to login via email and it's also spam vulnerability.

This PR provides the is_email_unique validator so there is no way
to create two or more users with the same email. Also, you can't
change your email on already existed one.
mutantsan pushed a commit to mutantsan/ckan that referenced this issue Nov 26, 2019
[ckan#2838] Email unique validator
fc14a22 
At the moment, there is no validation for the uniqueness of the email,
which does not allow us to use login via email.
And it's also can be a vulnerability to spam attacks.

This changes implements the is_email_unique validator, which prevents
from creation two or more users with the same email.
@mutantsan
Copy link
Contributor

Hey, I've created a PR #5100 with implementation of email uniqueness validation.

mutantsan pushed a commit to mutantsan/ckan that referenced this issue Dec 3, 2019
[ckan#2838] move duplicate_emails to db subcommand
d9f809d
mutantsan pushed a commit to mutantsan/ckan that referenced this issue Dec 3, 2019
[ckan#2838] Email unique validator
3ffe476 
At the moment, there is no validation for the uniqueness of the email,
which does not allow us to use login via email.
And it's also can be a vulnerability to spam attacks.

This changes implements the is_email_unique validator, which prevents
from creation two or more users with the same email.
mutantsan pushed a commit to mutantsan/ckan that referenced this issue Dec 3, 2019
[ckan#2838] fix pep8 test issue
07ea100
mutantsan pushed a commit to mutantsan/ckan that referenced this issue Dec 24, 2019
[ckan#2838] fix conflicts in tests
cec6c83
mutantsan pushed a commit to mutantsan/ckan that referenced this issue Dec 24, 2019
[ckan#2838] fix test_user test
202cc23
mutantsan pushed a commit to mutantsan/ckan that referenced this issue Dec 24, 2019
[ckan#2838] fix test: using class-level fixtures
c44a8ed
mutantsan pushed a commit to mutantsan/ckan that referenced this issue Jan 10, 2020
[ckan#2838] rewrite duplicate_emails cmd
089efcb
mutantsan pushed a commit to mutantsan/ckan that referenced this issue Jan 10, 2020
[ckan#2838] add email_is_unique validator to schema
076d8b5
mutantsan pushed a commit to mutantsan/ckan that referenced this issue Jan 10, 2020
[ckan#2838] rewrite validator wrong logic
f4bf1d6
mutantsan pushed a commit to mutantsan/ckan that referenced this issue Jan 10, 2020
[ckan#2838] delete irrelevant test which uses duplicate emails
29852d4
mutantsan pushed a commit to mutantsan/ckan that referenced this issue Jan 10, 2020
[ckan#2838] make sysadmin_user_dict email key unique to pass validation
7692630
mutantsan pushed a commit to mutantsan/ckan that referenced this issue Jan 10, 2020
[ckan#2838] change logic in create_user>create func to create user wi…
caa19bb 
…th unique email
mutantsan pushed a commit to mutantsan/ckan that referenced this issue Jan 10, 2020
[ckan#2838] rewrite email_is_unique validator tests
b460df1
mutantsan pushed a commit to mutantsan/ckan that referenced this issue Jan 10, 2020
[ckan#2838] test_works_even_if_username_already_exists use unique email
8b3d894
mutantsan pushed a commit to mutantsan/ckan that referenced this issue Jan 10, 2020
[ckan#2838] fix multiple tests to pass email_is_unique validator
3aa3ee0
mutantsan pushed a commit to mutantsan/ckan that referenced this issue Jan 10, 2020
[ckan#2838] trying to fix tests
d5da609
mutantsan pushed a commit to mutantsan/ckan that referenced this issue Jan 27, 2020
[ckan#2838] fix tests
aef247b
mutantsan pushed a commit to mutantsan/ckan that referenced this issue Jan 27, 2020
[ckan#2838] fix python3 tests
41d8ea0
mutantsan pushed a commit to mutantsan/ckan that referenced this issue Jan 27, 2020
[ckan#2838] fix pep8 and app_context in py3 tests
7286bc3
amercader added a commit that referenced this issue Jan 27, 2020
@amercader
Merge pull request #5100 from mutantsan/2838-validate-email-for-unique
bb36a33 
[#2838] Email unique validator
@paulmueller paulmueller mentioned this issue Sep 4, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rossjones rossjones

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
8 participants
@rossjones @amercader @mbellotti @tsufz @mevey @kmbn @MandanaMoshref @mutantsan