Benchmark PR 38446

[celmis-codereviewer]

Benchmark reproduction of keycloak#38446

[celmis-codereviewer]

❌ CHANGES REQUESTED — blocking findings

Full findings and scope are in the review summary comment on this pull request — one persistent comment, updated in place on every run.

[celmis-codereviewer]

Why: myUser returned by getMyUser(user) on line 232 can be null when the user is not found in storage; dereferenced without a check on line 234, throwing a NullPointerException.

🟠 NullPointerException when user is not present in storage

In getCredentials, getMyUser(user) on line 232 will return null if the user is not managed by this storage provider. Line 234 then accesses myUser.recoveryCodes directly without checking if myUser is null, which will result in a NullPointerException. Other methods in this class (such as isConfiguredFor and isValid) explicitly check if (myUser == null) before accessing user fields.

Suggested change

	List<CredentialModel> credentialModels = new ArrayList<>();
	var myUser = getMyUser(user);
	if (myUser == null) {
	return Stream.empty();
	}
	RecoveryAuthnCodesCredentialModel model;

_{agent: defect · rule: defect.null-dereference · confidence: 0.95}

[celmis-codereviewer]

Why: myUser.recoveryCodes.getCredentialData() holds a JSON array of strings, so JsonSerialization.readValue on line 237 returns a List of String objects, causing a ClassCastException when RecoveryAuthnCodesCredentialModel methods expect elements of type RecoveryAuthnCodeRepresentation.

🟠 Incompatible list element type deserialized for RecoveryAuthnCodesCredentialModel

In BackwardsCompatibilityUserStorage.updateCredential, recovery codes credentials are stored by serializing generatedCodes (List<String>) into JSON format. When getCredentials reads this credential data on line 237 using JsonSerialization.readValue(..., List.class), Jackson produces a List<String>. Passing this raw list to RecoveryAuthnCodesCredentialModel.createFromValues(...) succeeds at compile time due to raw type unchecked conversion, but causes a ClassCastException at runtime whenever the model attempts to access elements expecting RecoveryAuthnCodeRepresentation objects.

Suggested change

	model = RecoveryAuthnCodesCredentialModel.createFromValues(
	List<String> rawCodes = JsonSerialization.readValue(myUser.recoveryCodes.getCredentialData(), List.class);
	model = RecoveryAuthnCodesCredentialModel.createFromRawValues(
	rawCodes,
	myUser.recoveryCodes.getCreatedDate(),
	myUser.recoveryCodes.getUserLabel()
	);

_{agent: defect · rule: defect.class-cast · confidence: 0.95}

[celmis-codereviewer]

Why: When testRecoveryKeysSetupAndLogin runs on line 259 with expectedCode 0, getRecoveryAuthnCodeToEnterNumber on line 479 returns 1 for the 1-indexed recovery code, causing the assertion on line 480 to fail.

🟠 Incorrect expected code number passed to enterRecoveryCodes

In Keycloak, recovery code numbers presented on the login screen are 1-indexed (e.g. code #1). Therefore, enterRecoveryAuthnCodePage.getRecoveryAuthnCodeToEnterNumber() on line 479 returns 1 for the first code. Passing 0 as expectedCode on line 259 causes assertEquals("Incorrect code presented to login", 0, 1) on line 480 to fail with an AssertionError during test execution.

Suggested change

	enterRecoveryCodes(enterRecoveryAuthnCodePage, driver, 0, recoveryKeys);
	enterRecoveryCodes(enterRecoveryAuthnCodePage, driver, 1, recoveryKeys);

_{agent: defect · rule: defect.assertion-failure · confidence: 0.95}

[celmis-codereviewer]

Why: requestedCode on line 479 holds 1 for the first recovery code; used directly as a 0-based index into generatedRecoveryAuthnCodes on line 481, fetching index 1 (the second code) instead of index 0.

🟠 Off-by-one indexing into generatedRecoveryAuthnCodes list

Recovery code numbers returned by getRecoveryAuthnCodeToEnterNumber() are 1-based (1, 2, ...), while generatedRecoveryAuthnCodes is a zero-indexed List. Line 481 uses requestedCode directly as the index, so when code #1 is requested (requestedCode == 1), generatedRecoveryAuthnCodes.get(1) fetches the second code in the list instead of the first code at index 0.

Suggested change

	org.junit.Assert.assertEquals("Incorrect code presented to login", expectedCode, requestedCode);
	enterRecoveryAuthnCodePage.enterRecoveryAuthnCode(generatedRecoveryAuthnCodes.get(requestedCode - 1));

_{agent: defect · rule: defect.off-by-one · confidence: 0.95}

[celmis-codereviewer]

🤖 Code Review for PR #17

❌ CHANGES REQUESTED — blocking findings

Findings

• 🟠 Error: 4

Scope

• Files changed: 8
• Lines: +256 / -31

Performance

• Analysis time: 113.9s · agents: cve, structural, security, contract, defect · tokens: 49,996/30,300

---

_{Powered by Code Analyzer · context: tree-sitter graph + cve, structural, security, contract, defect}