Allow OAuth scope characters in organization alias - #53092
Open
martin-kanis wants to merge 9 commits into
Open
martin-kanis wants to merge 9 commits into
martin-kanis wants to merge 9 commits into
Conversation
Organization alias validation reused the generic RFC 3986 reserved-character blocklist shared by realms, clients, and IdP aliases, which is stricter than OAuth 2.0 scope-token syntax (RFC 6749, Section 3.3). Validate the alias against the RFC 6749 scope-token character range instead, so characters such as '/' and ':' are permitted. Closes keycloak#50703 Signed-off-by: TamaraMurazyan <murazyan.tamara5566@gmail.com>
The broadened OAuth scope-token character set for aliases newly allowed '*' and ':', which conflicted with how OrganizationScope already uses those characters: - '*' is reserved by OrganizationScope.ALL to mean "every organization the user belongs to", so an alias equal to '*' could never be selected specifically via organization:*. Reject that exact alias value while still allowing '*' elsewhere in an alias. - SCOPE_PATTERN used a greedy regex that split organization:<alias> on the last ':' instead of the first, so alias values containing ':' were parsed incorrectly. Made the first group non-greedy so it splits on the first separator instead. Signed-off-by: TamaraMurazyan <murazyan.tamara5566@gmail.com>
…e names by longest match - Move alias validation into JpaOrganizationProvider.create() so it applies unconditionally to the effective alias (after the blank name-fallback default), instead of only validating the fallback name. The previous placement let realm import (DefaultExportImportManager#importOrganizations) persist an alias with disallowed characters, since it calls the provider directly and never went through OrganizationsResource. The now-redundant pre-check in OrganizationsResource is removed since the provider is the single enforcement point all creation paths funnel through. - Replace OrganizationScope's regex-based "name:value" scope splitting with a longest-prefix match against the client's actual registered scopes. Client scope names are already allowed to contain ':' (see ClientScopeResource#scopeNamePattern), so neither a greedy nor a non-greedy regex split can correctly separate the scope name from the value in every case once alias values may also contain ':'. Trying progressively shorter prefixes against real client scopes resolves both a colon-containing scope name and a colon-containing alias value correctly. Signed-off-by: TamaraMurazyan <murazyan.tamara5566@gmail.com>
The session-cached lookup in resolveClientScope returned the first matching entry from an unordered Set, not necessarily the longest. Once a shorter scope name (e.g. organization) was cached from an earlier request, a later request for a more specific overlapping scope (e.g. organization:team, itself organization-enabled) could incorrectly resolve to the shorter cached one, corrupting the parsed value. Track and return the longest matching cached name instead, consistent with resolveParameterizedClientScope. Signed-off-by: TamaraMurazyan <murazyan.tamara5566@gmail.com>
…them StringUtil.isBlank() treats whitespace-only strings as blank, so a caller-supplied alias like " " or "\n" was being silently replaced with the org name (the "no alias provided" fallback) instead of being validated and rejected. Use isNullOrEmpty() for the fallback decision instead, so only a truly absent alias (null or "") defaults to the name; any non-empty value, including whitespace-only, is validated as-is and rejected if invalid. Signed-off-by: TamaraMurazyan <murazyan.tamara5566@gmail.com>
…meterized scope values with separators - Rename validateAliasAcceptsBlankOrNull to validateAliasAcceptsEmptyOrNull to accurately describe the null/empty inputs it covers. - Add an OIDC integration test verifying that an organization scope value containing the ':' separator (e.g. organization:ABC:Google) still resolves to the correct organization. Signed-off-by: TamaraMurazyan <murazyan.tamara5566@gmail.com>
…auth-scope-chars Signed-off-by: TamaraMurazyan <murazyan.tamara5566@gmail.com>
…pfront Organization aliases are immutable once set (OrganizationAdapter#setAlias rejects changing an existing alias), so updating the alias after creation silently returned 400 and left the persisted alias as "orga", meaning the test never actually exercised a colon-containing alias. Create the representation with the alias set before the create call instead. Signed-off-by: TamaraMurazyan <murazyan.tamara5566@gmail.com>
…it tests Closes keycloak#50703 Signed-off-by: Martin Kanis <mkanis@ibm.com>
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The implementation, regression coverage, and documentation consistently address expanded alias support and scope ambiguity.
Review effort: Balanced
Findings: None
What changed in this PR
Broadens organization aliases to support OAuth scope characters while preserving safe scope parsing and validation.
Changes:
- Centralizes alias validation in the organization provider.
- Resolves colon-containing aliases using longest matching client-scope prefixes.
- Adds regression tests and updates administrator/upgrade documentation.
| File | Description |
|---|---|
OrganizationOIDCProtocolMapperTest.java |
Tests special-character scopes and prefix ambiguity. |
OrganizationTest.java |
Tests alias validation and legacy imports. |
OrganizationScope.java |
Supports separators within scope values. |
OrganizationsResource.java |
Delegates alias validation to the provider. |
OrganizationsValidation.java |
Defines allowed and reserved alias characters. |
JpaOrganizationProvider.java |
Applies validation across creation paths. |
changes-26_8_0.adoc |
Documents compatibility implications. |
managing-organization.adoc |
Documents the revised alias rules. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
Closes #50703