Skip to content

Throw exception and rollback when token verification fails in reset credentials#50859

Merged
ahus1 merged 1 commit into
keycloak:mainfrom
rmartinc:issue-50850
Jul 15, 2026
Merged

Throw exception and rollback when token verification fails in reset credentials#50859
ahus1 merged 1 commit into
keycloak:mainfrom
rmartinc:issue-50850

Conversation

@rmartinc

Copy link
Copy Markdown
Contributor

Closes #50850

Throw an exception and rollback the transaction when ErrorPageException is thrown. I have changed to do the rollback when this exception is thrown. I think that it's OK as it is used just a few times and it makes sense (we are doing the same in other exceptions). If you think that it is better just do the rollback for the the reset credentials case, just let me know.

…redentials

Closes keycloak#50850

Signed-off-by: rmartinc <rmartinc@redhat.com>
Copilot AI review requested due to automatic review settings July 14, 2026 09:46
@rmartinc
rmartinc requested review from a team as code owners July 14, 2026 09:46

Copilot AI 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.

Pull request overview

Ensures failed action-token verification rolls back credential updates, preventing reuse of password-reset links.

Changes:

  • Throws ErrorPageException when action-token verification fails.
  • Marks transactions rollback-only when rendering this exception.
  • Verifies the valid password remains unchanged after duplicate reset attempts.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
AuthenticationManager.java Throws on invalid or reused action tokens.
ErrorPageException.java Marks the current transaction for rollback.
ResetPasswordTest.java Tests password preservation after duplicate reset.

@mabartos mabartos left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@rmartinc As mentioned in the comment above, I'm ok to proceed with this approach, but for the future, we should have a better general rollback handling.

I'll create a follow-up issue. LGTM

@ahus1
ahus1 merged commit 0f8222e into keycloak:main Jul 15, 2026
91 checks passed
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.

New Password is commited when multiple Password Reset is detected

4 participants