fix: prevent NPE in convertDeprecatedCredentialsFormat when credential fields are null by SaadAhmed7 · Pull Request #51557 · keycloak/keycloak

SaadAhmed7 · 2026-08-08T13:22:07Z

Summary

Fixed a NullPointerException in RepresentationToModel.convertDeprecatedCredentialsFormat() that occurs when creating a user via the Admin REST API with credentials that have null hashIterations.

Root Cause

CredentialRepresentation declares hashIterations, digits, counter, and period as boxed Integer (nullable), but PasswordCredentialData and OTPCredentialData constructors expect primitive int. When any of these fields are null, Java's auto-unboxing calls Integer.intValue() on null, producing a NullPointerException.

Fix

Added null-safe defaults (0) for all nullable Integer fields before passing them to the credential data constructors:

cred.getHashIterations() → cred.getHashIterations() != null ? cred.getHashIterations() : 0
cred.getDigits() → cred.getDigits() != null ? cred.getDigits() : 0
cred.getCounter() → cred.getCounter() != null ? cred.getCounter() : 0
cred.getPeriod() → cred.getPeriod() != null ? cred.getPeriod() : 0

Changes

server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java

Test plan

Verified the NPE stacktrace from the issue matches the fix location
Also fixed the same auto-unboxing risk for OTP credential fields (digits, counter, period)
Default of 0 is safe — credential data constructors accept 0 as a valid value

CredentialRepresentation uses boxed Integer for hashIterations, digits, counter, and period, but PasswordCredentialData and OTPCredentialData constructors take primitive int. When these fields are null during user creation via the Admin REST API, auto-unboxing throws a NullPointerException in convertDeprecatedCredentialsFormat. Added null-safe defaults (0) for all nullable Integer fields before passing them to the credential data constructors. Closes keycloak#41640 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Prevents null auto-unboxing failures when converting deprecated credential representations.

Changes:

Defaults nullable password and OTP integer fields.
Reformats OTP credential construction.

Suppressed comments (1)

server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java:292

A 0 period is not usable for TOTP: TimeBasedOTP.Clock#getCurrentInterval() divides by the configured interval, so the first validation attempt throws ArithmeticException. Use the established 30-second default or reject an incomplete TOTP credential.

                                    cred.getPeriod() != null ? cred.getPeriod() : 0,

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

                        } else if (OTPCredentialModel.TOTP.equals(cred.getType()) || OTPCredentialModel.HOTP.equals(cred.getType())) {
-                            OTPCredentialData credentialData = new OTPCredentialData(cred.getType(), cred.getDigits(), cred.getCounter(), cred.getPeriod(), cred.getAlgorithm(), null);
+                            OTPCredentialData credentialData = new OTPCredentialData(cred.getType(),
+                                    cred.getDigits() != null ? cred.getDigits() : 0,



                        if (PasswordCredentialModel.TYPE.equals(cred.getType()) || PasswordCredentialModel.PASSWORD_HISTORY.equals(cred.getType())) {
-                            PasswordCredentialData credentialData = new PasswordCredentialData(cred.getHashIterations(), cred.getAlgorithm());
+                            PasswordCredentialData credentialData = new PasswordCredentialData(cred.getHashIterations() != null ? cred.getHashIterations() : 0, cred.getAlgorithm());


Defaulting digits to 0 would produce a trivially guessable OTP ("0") since HmacOTP computes modulo DIGITS_POWER[0] (= 1). Defaulting period to 0 would cause ArithmeticException in TimeBasedOTP.Clock due to division by zero. Use OTPPolicy.DEFAULT_POLICY values (6 digits, 30-second period) instead, matching Keycloak's standard OTP defaults. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

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

Suppressed comments (3)

server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java:284

The existing UserCreateTest.createUserWithDeprecatedCredentialsFormat covers this Admin REST path only when hashIterations is present. Add a regression case with the field omitted so the reported null-input behavior and resulting response are verified end to end.

                            PasswordCredentialData credentialData = new PasswordCredentialData(cred.getHashIterations() != null ? cred.getHashIterations() : 0, cred.getAlgorithm());

server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java:293

These fallbacks ignore the target realm's OTP policy, so a realm configured for (for example) 8 digits or a 60-second period will store incompatible 6-digit/30-second credential metadata. Pass the realm into this conversion and use its OTP policy, including initialCounter, as OTPCredentialModel.createFromPolicy does.

                                    cred.getDigits() != null ? cred.getDigits() : OTPPolicy.DEFAULT_POLICY.getDigits(),
                                    cred.getCounter() != null ? cred.getCounter() : 0,
                                    cred.getPeriod() != null ? cred.getPeriod() : OTPPolicy.DEFAULT_POLICY.getPeriod(),

server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java:284

Defaulting a missing iteration count to 0 stores a password credential that PBKDF2 cannot verify: Pbkdf2PasswordHashProvider passes this value to PBEKeySpec, which rejects non-positive iteration counts. Since the original hash parameters cannot be inferred, reject the malformed deprecated credential with a client validation error rather than persisting it.

This issue also appears on line 291 of the same file.

                            PasswordCredentialData credentialData = new PasswordCredentialData(cred.getHashIterations() != null ? cred.getHashIterations() : 0, cred.getAlgorithm());

Address review feedback on the null-handling fallbacks: Password: defaulting a missing hashIterations to 0 does not fix the problem, it relocates it. Both Pbkdf2PasswordHashProvider and Argon2PasswordHashProvider feed the stored iteration count back into the KDF at verify time, and a non-positive value is rejected there. Since the original hash parameters cannot be inferred, the credential could never be verified, so reject it with a ModelValidationException (HTTP 400) rather than persisting an unusable password. OTP: fall back to the target realm's OTP policy instead of the global default, so a realm configured for e.g. 8 digits or a 60-second period does not silently store incompatible 6-digit/30-second metadata. The counter now also uses the policy's initial counter, matching OTPCredentialModel.createFromPolicy. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

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

Suppressed comments (3)

server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java:289

The existing deprecated-credential test always supplies hashIterations, so the reported null case and its new validation path have no regression coverage. Add an Admin REST test omitting this field that asserts the intended status/logging behavior; otherwise this fix can regress back to a 500 unnoticed.

                            if (cred.getHashIterations() == null) {
                                // The stored hash cannot be reproduced without the iteration count it was
                                // generated with, so such a credential could never be verified. Reject it
                                // instead of persisting an unusable password.
                                throw new ModelValidationException("Credential of type '" + cred.getType()
                                        + "' for user '" + user.getUsername() + "' is missing 'hashIterations'");

server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java:289

This rejects the request with a 400 (and UsersResource logs the ModelValidationException) instead of applying the documented null-safe 0 fallback, so it does not implement the Fix/Test Plan or the issue's “no exception in logs” outcome. Either use the documented fallback here or update the PR's intended behavior and acceptance criteria to explicitly require rejection.

                            if (cred.getHashIterations() == null) {
                                // The stored hash cannot be reproduced without the iteration count it was
                                // generated with, so such a credential could never be verified. Reject it
                                // instead of persisting an unusable password.
                                throw new ModelValidationException("Credential of type '" + cred.getType()
                                        + "' for user '" + user.getUsername() + "' is missing 'hashIterations'");

server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java:302

These three new fallback branches are untested, although deprecated credential conversion already has integration coverage. Add TOTP/HOTP import cases with omitted digits, counter, and period and verify the persisted credential data uses the realm policy values.

                            OTPPolicy otpPolicy = realm.getOTPPolicy();
                            OTPCredentialData credentialData = new OTPCredentialData(cred.getType(),
                                    cred.getDigits() != null ? cred.getDigits() : otpPolicy.getDigits(),
                                    cred.getCounter() != null ? cred.getCounter() : otpPolicy.getInitialCounter(),
                                    cred.getPeriod() != null ? cred.getPeriod() : otpPolicy.getPeriod(),
                                    cred.getAlgorithm(), null);

SaadAhmed7 requested a review from a team as a code owner August 8, 2026 13:22

Copilot AI balanced review requested due to automatic review settings August 8, 2026 13:22

keycloak-github-bot Bot added team/core-iam team/core-shared labels Aug 8, 2026

Copilot started reviewing on behalf of SaadAhmed7 August 8, 2026 13:22 View session

Copilot AI reviewed Aug 8, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings August 8, 2026 13:35

Copilot started reviewing on behalf of SaadAhmed7 August 8, 2026 13:35 View session

Copilot AI reviewed Aug 8, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings August 9, 2026 10:04

Copilot started reviewing on behalf of SaadAhmed7 August 9, 2026 10:05 View session

Copilot AI reviewed Aug 9, 2026

View reviewed changes

Navigation Menu

Uh oh!

fix: prevent NPE in convertDeprecatedCredentialsFormat when credential fields are null - #51557

fix: prevent NPE in convertDeprecatedCredentialsFormat when credential fields are null#51557
SaadAhmed7 wants to merge 3 commits into
keycloak:mainfrom
SaadAhmed7:fix/npe-deprecated-credentials-null-hash-iterations

SaadAhmed7 commented Aug 8, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

SaadAhmed7 commented Aug 8, 2026

Summary

Root Cause

Fix

Changes

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants