Require current password for sensitive user changes - #512
Open
tmfrnz wants to merge 10 commits into
Open
Conversation
Base automatically changed from
sg-dev-restrict-user-update-attributes
to
sg-dev
July 21, 2026 07:51
parndt
approved these changes
Jul 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds re-authentication (current password) to sensitive account actions, so a session cannot make them without the acting user proving their identity.
Changes:
Password change: sets config.check_current_password_before_update = :password. Previously current_password was validated only if supplied; omitting it bypassed the check. Now it is required whenever the password changes. The token-based reset flow stays exempt (allow_password_change bypasses the check in DTA).
Role changes: require_current_password! gates all role assignment and removal on UserRolesController (not just promotions, so it stays correct if non-hierarchical roles are added later). Re-authentication runs after authorization, so a user who isn't permitted to change roles gets 403 (forbidden) rather than being prompted for a password.
Email change: same gate on UsersController#update, but only when the email is actually changing. Dormant for SG (update_email is disabled, so email is stripped from permitted attributes); active on installs that enable email editing.
The gate (require_current_password! in ApplicationController) is defence-in-depth: the client sends current_password on these requests, so it primarily blocks direct API calls (e.g. a hijacked session).
Existing UserRoles controller and API specs updated to send current_password on the success cases and to reflect the authorization-before-re-authentication ordering.
Verified on UAT: password change and role assign/remove reject without current_password (401/422) and succeed with it. Email gate covered by spec (stubbed on), since it is disabled on this build.
Depends on #508. Based on that branch; the email gate builds on its permitted-attributes change.
Client side implementation still TODO