Skip to content

Fix NPE in convertDeprecatedCredentialsFormat when credential fields are null#49316

Open
nouhouari wants to merge 1 commit into
keycloak:mainfrom
nouhouari:fix/41640-npe-deprecated-credentials
Open

Fix NPE in convertDeprecatedCredentialsFormat when credential fields are null#49316
nouhouari wants to merge 1 commit into
keycloak:mainfrom
nouhouari:fix/41640-npe-deprecated-credentials

Conversation

@nouhouari

Copy link
Copy Markdown

Summary

  • Validate required fields before converting deprecated credential format in RepresentationToModel.convertDeprecatedCredentialsFormat()
  • Throws ModelException (returned as 400 Bad Request) instead of crashing with NPE (500 Internal Server Error)
  • Covers both password credentials (hashIterations, algorithm, hashedSaltedValue, salt) and OTP credentials (digits, counter, period, algorithm, hashedSaltedValue)

Why

When creating a user via the Admin REST API with an incomplete deprecated credential payload (e.g. {"type": "password"} with null value), the Integer unboxing of getHashIterations() causes an uncaught NullPointerException.

Test plan

  • Added integration test UsersTest#createUserWithIncompleteDeprecatedPasswordCredentialReturnsBadRequest
  • Verified test fails without fix (500) and passes with fix (400)
  • ./mvnw spotless:check passes

AI Disclosure

This fix was developed with assistance from Claude Code (AI). The author has reviewed, understands, and can explain all changes.

Closes #41640

@nouhouari
nouhouari requested a review from a team as a code owner May 26, 2026 01:10
@nouhouari
nouhouari force-pushed the fix/41640-npe-deprecated-credentials branch from c554865 to 4f8d8cd Compare May 26, 2026 02:16
…are null

Validate required fields before converting deprecated credential format
to prevent NullPointerException from Integer unboxing. Returns 400 Bad
Request with a descriptive message instead of a 500 server error.

Closes keycloak#41640

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: nouhouari <houarinourredine@gmail.com>
@nouhouari
nouhouari force-pushed the fix/41640-npe-deprecated-credentials branch from a4a46c8 to 8678b63 Compare June 3, 2026 05:52
@sguilhen

Copy link
Copy Markdown
Contributor

Hi @nouhouari, thank you for this fix. The approach is correct - this is a deprecated credential import path where the hash, salt, algorithm, iteration count (for passwords) and digits, counter, period, algorithm (for OTP) all come from an external source. Defaulting missing fields would silently create credentials that import successfully but fail at verification time (wrong hashIterations breaks PBEKeySpec or mismatches the actual hash; wrong OTP parameters produce codes that don't match the user's authenticator). Returning a 400 Bad Request with a clear error message is the right thing to do.

I'd like to note that @vivekksk submitted essentially the same fix back in August 2025 (PR #41922), so please consider adding a Co-authored-by credit if you're open to it.

Could you expand the test coverage? The current test covers the case where all credential fields are empty, but it would be good to also cover:

  • A password credential with hashedSaltedValue, salt, and algorithm present but hashIterations missing (this is the exact scenario from issue NullPointerException after after 26.3.2 migration #41640)
  • An OTP credential with hashedSaltedValue present but digits/counter/period missing
  • Optionally, a happy path test that a complete deprecated credential still imports successfully

We'll proceed with merging this one once the tests are updated. Thanks!

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.

NullPointerException after after 26.3.2 migration

4 participants