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

allow sysadmins to edit usernames, #4193 #8122

Merged
merged 3 commits into from Apr 19, 2024

Conversation

ThrawnCA
Copy link
Contributor

@ThrawnCA ThrawnCA commented Mar 18, 2024

Discussion on #4193 identified editing usernames as a potential security hole, allowing takeover of another account in specific situations. However, sysadmins already have more privileges than this security weakness could grant. Thus, there is no additional risk in permitting sysadmins to edit other accounts.

Proposed fixes:

Allow sysadmins to edit usernames.

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

- Username modification is a potential security risk, but sysadmins can access any account anyway
@amercader
Copy link
Member

This looks good @ThrawnCA , but the test failed. Can you check?

@ThrawnCA
Copy link
Contributor Author

This looks good @ThrawnCA , but the test failed. Can you check?

I'll look into it, but I'm not sure why the test is failing when I know this code works in production.

- Pass API token via 'headers' parameter instead of 'extra_environ'
@ThrawnCA ThrawnCA force-pushed the github-4193-username-editing branch from 655e224 to 4f04000 Compare April 19, 2024 11:44
@amercader amercader merged commit a638395 into ckan:master Apr 19, 2024
8 checks passed
@amercader
Copy link
Member

Thanks @ThrawnCA

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.

None yet

2 participants