Repository navigation
Reject a WebAuthn user handle that cannot be a stored user id - #55039
Draft
amalsaidov wants to merge 1 commit into
Draft
amalsaidov wants to merge 1 commit into
amalsaidov wants to merge 1 commit into
Conversation
Closes keycloak#53897 A discoverable WebAuthn credential carries a user handle that WebAuthnAuthenticator.action() decodes as UTF-8 and uses as the user id for session.users().getUserById(). A handle whose decoded value contains a NUL character cannot match any stored user, but it was passed straight to the lookup. On stores that reject a NUL byte, such as PostgreSQL, that turned an unknown credential into a server error and a generic error page instead of the normal unknown-user response, and the login event was recorded as invalid_user_credentials rather than user_not_found. Guard the decoded user id before the lookup and return the existing user-not-found response when it cannot represent a stored user id. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: amalsaidov <amalsaidov@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #53897
Decision: the decoded user id is validated at the point it is produced from the user handle, before any user lookup, so the guard covers every path that consumes it rather than only the database call. The rejection is kept narrow to a NUL character, which is the value the report shows reaching the store, rather than a broader "valid id" check that would risk rejecting legitimate external ids coming from user storage providers.
The rejected handle reuses the existing user-not-found response, so the event is recorded as user_not_found and the user sees the normal unknown-user message. The response does not add the offending value as an event detail, which keeps the same NUL out of the store that persists events.
Testing: a unit test was added for the new check and run with
./mvnw -pl services -am test -Dtest=WebAuthnAuthenticatorTest(3 passing). A full end-to-end reproduction needs a virtual authenticator and a database that rejects NUL bytes, which is beyond a unit test.AI usage: this change was prepared with the assistance of an AI agent (Claude). I have reviewed it, understand it, and will maintain it through review.
🤖 Generated with Claude Code