Skip to content

fix: require current password for user management#738

Closed
ShiroKSH wants to merge 1 commit into
NeptuneHub:mainfrom
ShiroKSH:fix/auth-current-password-confirmation
Closed

fix: require current password for user management#738
ShiroKSH wants to merge 1 commit into
NeptuneHub:mainfrom
ShiroKSH:fix/auth-current-password-confirmation

Conversation

@ShiroKSH

@ShiroKSH ShiroKSH commented Jul 9, 2026

Copy link
Copy Markdown

AS-IS

Admins could create additional users without confirming their own password. Admin resets for admin accounts also skipped current-password confirmation, while only non-admin self-service password changes required it.

TO-BE

Creating a user requires the current authenticated admin password. Password changes require current-password confirmation for self-service changes and when the target account is an admin, while admin resets for regular user accounts keep the existing flow.

Test

  • uv run --python 3.12 --with pytest --with flask --with PyJWT --with argon2-cffi --with psycopg2-binary --with redis --with rq --with pyyaml --with requests --with numpy --with rapidfuzz pytest test/unit/test_app_auth.py
  • uv run --with ruff ruff check app_auth.py test/unit/test_app_auth.py
  • node --check static/users.js
  • git diff --check

Other useful information

Split from closed #735 and limited to user-management password confirmation.

Checklist

Type of change:

  • Bug fix
  • New feature
  • Documentation
  • Refactor
  • Breaking change

Tested on architecture:

  • Intel
  • ARM
  • -noavx2
  • NVIDIA image

Tested on media server:

  • Navidrome
  • Jellyfin
  • Emby
  • Lyrion

Updated:

  • Documentation
  • Unit test
  • Integration test

Other:

  • CONTRIBUTING.md read and accepted
  • Checked performance on a big library (> 150k songs) works without issues

Related ISSUE: N/A

@sonarqubecloud

sonarqubecloud Bot commented Jul 9, 2026

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a security enhancement requiring the current password when creating a new user or updating an admin's password. It adds backend validation in app_auth.py via a new helper function _current_password_error, updates the API schemas, and integrates the corresponding input fields and logic into the frontend (users.js and users.html). Comprehensive unit tests have also been added to verify these validation flows. There are no review comments, and I have no additional feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@NeptuneHub NeptuneHub left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Hi and thanks for the PR. Please the current password field, when is requested for activity on OTHER users (so not yourself) need to be clear that is your password to confirm the activity so:

  • Instead of current password label put something like "Insert your admin password to confirm the operation"
  • put it in the bottom, separated maybe with some space from the information of the user that you are creating/modifing

@NeptuneHub

Copy link
Copy Markdown
Owner

Thanks for taking the time to put this together and for the suggestion. I think the underlying idea is worthwhile, but after reviewing the changes, I don't think this implementation is the right fit.

At this point, I think it will be more efficient for me to implement the change directly rather than iterate further on this PR, so I'm going to close it.

Thanks again for the contribution and the idea.

@NeptuneHub NeptuneHub closed this Jul 10, 2026
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.

2 participants