Skip to content

Fix NPE when deprecated credential fields are null during conversion#51009

Closed
satyamkumarmishra2005 wants to merge 1 commit into
keycloak:mainfrom
satyamkumarmishra2005:fix/41640-npe
Closed

Fix NPE when deprecated credential fields are null during conversion#51009
satyamkumarmishra2005 wants to merge 1 commit into
keycloak:mainfrom
satyamkumarmishra2005:fix/41640-npe

Conversation

@satyamkumarmishra2005

@satyamkumarmishra2005 satyamkumarmishra2005 commented Jul 18, 2026

Copy link
Copy Markdown

Summary

This PR prevents a NullPointerException in
RepresentationToModel.convertDeprecatedCredentialsFormat() when deprecated
password or OTP credentials contain nullable Integer fields that are
auto-unboxed into primitive int constructor parameters.

Changes

  • Use a default value of -1 when hashIterations is absent.
  • Use default OTP values (digits=6, counter=0, period=30) when the
    corresponding fields are absent.
  • Add regression tests covering:
    • Missing password hashIterations
    • Missing OTP digits, counter, and period
    • Existing values are preserved
    • Modern credentials remain unchanged

Related to #41640

Copilot AI review requested due to automatic review settings July 18, 2026 14:28
@satyamkumarmishra2005
satyamkumarmishra2005 requested a review from a team as a code owner July 18, 2026 14:28
Copilot AI reviewed Jul 18, 2026

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@satyamkumarmishra2005
Fix NPE when converting deprecated credentials
e55bed5 
Signed-off-by: Satyam Mishra <satyamkumarmishra2005@gmail.com>
@sguilhen

sguilhen commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Hi @satyamkumarmishra2005, thank you for the contribution and the thorough unit tests - the test coverage in your PR is excellent.

We're going to go with the validation approach from PR #49316 instead. The -1 default for hashIterations has a subtle problem: while -1 is a recognized sentinel in encodedCredential() and encode(), the verify() method in Pbkdf2PasswordHashProvider passes the stored value directly to PBEKeySpec, which rejects negative iteration counts. This means a credential stored with hashIterations=-1 would be created successfully but could never be verified.

Similarly, defaulting OTP parameters (digits/counter/period) risks silently creating credentials that don't match the user's authenticator configuration - they'd import fine but verification would fail with no clear indication why. Rejecting with a clear error is the safer path.

Closing this as a duplicate - thank you for the contribution, and we hope you'll pick up another issue!

@sguilhen sguilhen closed this Jul 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@satyamkumarmishra2005 @sguilhen