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

[Security] Disable updating username #4193

Closed
anotheredward opened this issue Apr 18, 2018 · 5 comments
Closed

[Security] Disable updating username #4193

anotheredward opened this issue Apr 18, 2018 · 5 comments
Assignees

Comments

@anotheredward
Copy link
Contributor

In the ckanext-security project we don't allow users to update their usernames. This was highlighted as an issue in our security review and we've implemented a workaround.

Would ckan-core be willing to accept a PR that removes this functionality?

CKAN Version if known (or site URL)

2.6/2.7

Please describe the expected behaviour

  • User/Sysadmin is not allowed to update usernames

Please describe the actual behaviour

  • User/Sysadmin is allowed to update usernames

What steps can be taken to reproduce the issue?

  • Update a username for an existing user
  • See that the username has been updated
@amercader
Copy link
Member

@anotheredward This was implemented in #3531, which was part of 2.7. You can just change the user name the first time on user invites because you get a non-friendly user name by default.

Feel free to reopen if I missed something.

@latchmananr
Copy link

latchmananr commented Oct 16, 2019

Hello @camfindlay, @wardi, @anotheredward & @amercader,

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 accounts 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

@camfindlay
Copy link
Contributor

I'll talk nicely to the new Product Owner of data.govt.nz and see if I can get the logic behind this one (it was legit and in a pen test report which I no longer have the access to as no longer on the product that this change came about in!).

@latchmananr
Copy link

Thanks @camfindlay. Appreciate it.

@ThrawnCA
Copy link
Contributor

@camfindlay It looks like the security concerns were centered around login cookie forgery, since usernames are embedded in the cookies (eg a user could sign up with a username that someone else was likely to use, generate a valid cookie, then change their own username and wait for the target to register, at which point they could use the cookie to hijack the target's account).

Would it be reasonable to permit only sysadmins to update usernames? We have an implementation in our fork, if you'd like to copy it.

ThrawnCA added a commit to qld-gov-au/ckan that referenced this issue Mar 18, 2024
ThrawnCA added a commit to qld-gov-au/ckan that referenced this issue Mar 18, 2024
ThrawnCA added a commit to qld-gov-au/ckan that referenced this issue Apr 18, 2024
- Pass API token via 'headers' parameter instead of 'extra_environ'
ThrawnCA added a commit to qld-gov-au/ckan that referenced this issue Apr 19, 2024
- Pass API token via 'headers' parameter instead of 'extra_environ'
amercader added a commit that referenced this issue Apr 19, 2024
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

No branches or pull requests

5 participants