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

do not allow to modify exsiting username #3531

Merged
merged 5 commits into from
Jun 13, 2017
Merged

Conversation

fanjinfei
Copy link
Contributor

@fanjinfei fanjinfei commented Apr 5, 2017

Fixes #

Proposed fixes:

Features:

  • includes tests covering changes
  • includes updated documentation
  • includes user-visible changes
  • includes API changes
  • includes bugfix for possible backport

Please [X] all the boxes above that apply

@fanjinfei fanjinfei force-pushed the ckanmaster branch 4 times, most recently from 686af57 to f42789d Compare April 6, 2017 13:04
@fanjinfei
Copy link
Contributor Author

ready for review

@amercader amercader self-assigned this Apr 6, 2017
@amercader amercader added the WIP label Apr 6, 2017
@@ -349,6 +349,14 @@ def _save_edit(self, id, context):

email_changed = data_dict['email'] != c.userobj.email

if id != data_dict['name']:
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this check be done at the validation level? Otherwise the name will be able to be changed via the API

@amercader
Copy link
Member

On second thoughts I'm not too sure about this. For instance when you get an invite to an organization (via email) you get a user name assigned based on your email plus some random characters, eg adria-mercader-1559. Even the invite email says "You can change it later."

Another use case is if someone leaves the organization but you still want to keep using the CKAN user changing its name.

I think it's a useful feature and if there are auth implications somewhere related to changing the name we should fix those.

@wardi
Copy link
Contributor

wardi commented Apr 6, 2017

invites shouldn't be creating users, they should just send a URL that can be used to create a user already added to an org

@wardi
Copy link
Contributor

wardi commented Apr 6, 2017

If changing usernames is genuinely useful we could live with just being able to override the schema so we can disable it on our site cleanly. That's not something we can do right now.

@wardi wardi removed the WIP label Apr 25, 2017
@amercader amercader merged commit 42c0c7f into ckan:master Jun 13, 2017
fanjinfei pushed a commit to fanjinfei/ckan that referenced this pull request Jun 20, 2017
do not allow to modify exsiting username
@camfindlay
Copy link
Contributor

Happy to provide the related reasoning behind this security enhancement - is there a CKAN core team email I can interact with?

klikstermkd pushed a commit to klikstermkd/ckan that referenced this pull request Jul 23, 2017
@latchmananr
Copy link

latchmananr commented Oct 14, 2019

Hello Adrià and crew,

Could you please advise the security reason why the username editing is now been disabled?

In our organisation (gov), we use a distributed publishing model where each dept (~40) manages its own publishers (~400). To keep the publisher names anonymous from the public, we use "agency-acronym_publisher_123" username pattern (e.g. transport_publisher_123). We do maintain an internal registry with the publisher's details etc. However, like every other gov, we do have dept name changes after elections/new gov, where we need to update the usernames to reflect the name changes.

With this feature now been disabled, it is a massive headache for us as we have no other option than creating new user account and delete old ones. This is very time consuming and also publishers will lose history, follows etc.

I do appreciate you may have valid reasons behind this change, however, is it possible for the sysadmin or on the production.ini to set the flag if the username is editable or not instead of bake it to the code? Happy to hear your suggestions too.

Thank you and appreciate the team's contribution to CKAN.

Kind regards,
Ray

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

Successfully merging this pull request may close these issues.

None yet

5 participants