Skip to content

Fix UnknownError parsing Kerberos principal with escaped '@' - #51568

Open
ousamabenyounes wants to merge 1 commit into
keycloak:mainfrom
ousamabenyounes:fix/issue-29316
Open

Fix UnknownError parsing Kerberos principal with escaped '@'#51568
ousamabenyounes wants to merge 1 commit into
keycloak:mainfrom
ousamabenyounes:fix/issue-29316

Conversation

@ousamabenyounes

Copy link
Copy Markdown

Description

KerberosPrincipal(String) split the incoming principal on every @ and required exactly two parts:

String[] parts = kerberosPrincipal.split("@");
if (parts.length != 2) {
    throw new IllegalArgumentException("Kerberos principal '" + kerberosPrincipal + "' not valid");
}

A Kerberos principal may legally contain an escaped @ (written \@) inside its name component — see RFC 1964, section 2.1.1. For such a principal, e.g. ssiemel\@a.com@KEYCLOAK.ORG, split("@") yields three parts and the constructor throws IllegalArgumentException. On the LDAP user-sync path (LDAPStorageProvidernew KerberosPrincipal(...)) this is not handled gracefully and surfaces as Could not sync users: 'UnknownError' in the admin console (with nothing in the server log), exactly as reported in the issue.

Fix

Parse the realm as the part after the single unescaped @, treating an escaped \@ inside the name as a literal (escape detection via backslash parity). Behaviour is otherwise unchanged:

Input Result
john@KEYCLOAK.ORG prefix john, realm KEYCLOAK.ORG (unchanged)
ssiemel\@a.com@KEYCLOAK.ORG prefix ssiemel\@a.com, realm KEYCLOAK.ORG (fixed — previously threw)
a@b@KEYCLOAK.ORG (multiple unescaped @) still rejected as invalid
john / john@ (no realm / empty realm) still rejected as invalid

All four production callers (LDAPStorageProvider, KerberosFederationProvider, SPNEGOAuthenticator, KerberosUsernamePasswordAuthenticator) route through this one constructor, so fixing the shared parser repairs every path, including the reported LDAP-sync reproducer. The change is intentionally limited to KerberosPrincipal and preserves the original exception message.

Test verification (RED → GREEN)

New unit test KerberosPrincipalTest (plain JUnit, no running server required).

RED — on unmodified main, before the production change:

[ERROR] org.keycloak.federation.kerberos.KerberosPrincipalTest.testEscapedAtInPrincipalName -- Time elapsed: 0.005 s <<< ERROR!
java.lang.IllegalArgumentException: Kerberos principal 'ssiemel\@a.com@KEYCLOAK.ORG' not valid
	at org.keycloak.federation.kerberos.KerberosPrincipal.<init>(KerberosPrincipal.java:34)
[ERROR] Tests run: 6, Failures: 0, Errors: 1, Skipped: 0

GREEN — with the fix:

[INFO] Tests run: 7, Failures: 0, Errors: 0, Skipped: 0 -- in org.keycloak.federation.kerberos.KerberosPrincipalTest
[INFO] BUILD SUCCESS

./mvnw -pl federation/kerberos spotless:check and the module's full test suite (KerberosFederationProviderFactoryTest + KerberosPrincipalTest, 10 tests) also pass.

Note on AI usage

AI agents were used to assist with investigating and implementing this change. I understand every line of the change and can explain and revise it.

Closes #29316

KerberosPrincipal split the principal on every '@' and required exactly two
parts, so a principal whose name component contains an escaped '\@' (valid per
RFC 1964 section 2.1.1, e.g. ssiemel\@a.com@KEYCLOAK.ORG) was rejected and
surfaced as an "UnknownError" during LDAP user sync. Parse the realm as the
part after the single unescaped '@' instead, keeping the existing rejection of
ambiguous (multiple unescaped '@') and realm-less principals.

Closes keycloak#29316

Signed-off-by: Ben Younes <2910651+ousamabenyounes@users.noreply.github.com>

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.

Pull request overview

Updates Kerberos principal parsing to support escaped @ characters while rejecting ambiguous separators.

Changes:

  • Adds escape-aware realm separator detection.
  • Adds unit tests for valid, escaped, and invalid principals.

Reviewed changes

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

File Description
KerberosPrincipal.java Implements escape-aware principal parsing.
KerberosPrincipalTest.java Covers parsing behavior and edge cases.

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

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.

Getting "UnknownError" exception when KerberosPrincipal has more than one "@" character

2 participants