Repository navigation
Multi-IdP for orgs: Data Model - #52401
Conversation
13c38be to
a1687c2
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Migration edge cases and Admin UI round-tripping can fail upgrades or erase domain routing configuration.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Moves organization identity-provider routing from IdP configuration into normalized organization-domain data and introduces an organization–IdP join table.
Changes:
- Adds per-domain IdP routing and automatic redirect settings.
- Migrates existing JPA and imported realm data to the new model.
- Updates authentication, validation, caching, UI, tests, and migration CI.
File summaries
| File | Description |
|---|---|
testsuite/.../OrganizationPostBrokerLoginTest.java |
Uses domain-level routing. |
testsuite/.../OrganizationIdentityProviderTest.java |
Updates IdP/domain association tests. |
testsuite/.../AbstractBrokerSelfRegistrationTest.java |
Migrates broker tests to domain routing. |
testsuite/.../OrganizationThemeTest.java |
Disables redirects through domains. |
testsuite/.../admin/AbstractOrganizationTest.java |
Configures test domain routing. |
testsuite/.../migration/MigrationTest.java |
Adds 26.x migration coverage. |
testsuite/.../migration/AbstractMigrationTest.java |
Verifies domain-routing migration. |
tests/.../OrganizationMemberTest.java |
Updates member routing setup. |
tests/.../OrganizationMemberIdpLinkTest.java |
Configures linked domain routing. |
tests/.../OrganizationCacheTest.java |
Adapts cache tests. |
tests/.../OrganizationAdminRolesPermissionsTest.java |
Removes legacy domain configuration. |
tests/.../admin/OrganizationTest.java |
Uses domain redirect settings. |
tests/.../admin/AbstractOrganizationTest.java |
Updates organization test fixtures. |
services/.../OrganizationMemberValidator.java |
Validates against routed domains. |
services/.../OrganizationAuthenticator.java |
Redirects using domain metadata. |
server-spi/.../OrganizationModel.java |
Removes legacy routing constants. |
server-spi/.../OrganizationDomainModel.java |
Adds routing state to domains. |
server-spi-private/.../RepresentationToModel.java |
Maps and cleans routing fields. |
server-spi-private/.../ModelToRepresentation.java |
Exposes domain routing fields. |
server-spi-private/.../MigrationUtils.java |
Retains legacy migration keys. |
model/storage-private/.../DefaultExportImportManager.java |
Links IdPs before importing domains. |
model/storage-private/.../MigrateTo26_8_0.java |
Migrates imported routing data. |
model/jpa/.../jpa-changelog-26.8.0.xml |
Adds normalized schema and migration. |
model/jpa/.../default-persistence.xml |
Registers the join entity. |
model/jpa/.../OrganizationAdapter.java |
Persists domain routing relationships. |
model/jpa/.../JpaOrganizationProvider.java |
Manages organization–IdP links. |
model/jpa/.../JpaRealmProvider.java |
Deletes join rows with realms. |
model/jpa/.../JpaIdentityProviderStorageProvider.java |
Resolves links through the join table. |
model/jpa/.../OrganizationIdentityProviderEntity.java |
Defines the new join entity. |
model/jpa/.../OrganizationEntity.java |
Adds IdP-link relationships. |
model/jpa/.../OrganizationDomainEntity.java |
Adds realm and routing columns. |
model/jpa/.../IdentityProviderEntity.java |
Removes direct organization ownership. |
model/jpa/.../JpaUpdate26_8_0_PopulateOrgDomainRealmId.java |
Backfills domain realm IDs. |
model/jpa/.../JpaUpdate26_8_0_DomainIdpRoutingMigration.java |
Migrates legacy routing data. |
model/infinispan/.../InfinispanOrganizationProvider.java |
Invalidates affected IdP caches. |
model/infinispan/.../InfinispanIdentityProviderStorageProvider.java |
Exposes cache-key helper. |
js/apps/admin-ui/test/organization/idp.ts |
Removes legacy domain input. |
js/apps/admin-ui/test/organization/idp.spec.ts |
Updates IdP-link UI test. |
js/apps/admin-ui/src/organizations/LinkIdentityProviderModal.tsx |
Removes legacy routing controls. |
js/apps/admin-ui/src/organizations/IdentityProviders.tsx |
Removes legacy domain column. |
core/.../OrganizationDomainRepresentation.java |
Adds domain routing API fields. |
.github/workflows/ci.yml |
Tests migration from 26.6.0. |
Review details
Suppressed comments (1)
model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/custom/JpaUpdate26_8_0_DomainIdpRoutingMigration.java:169
- The duplicate guard is scoped to one IdP invocation and
existingDomainNamesonly queries the current organization. BecauseUK_ORG_DOMAIN_REALM_NAMEis installed before this task and Liquibase defers these inserts, two legacy IdPs sharing an excluded pattern—or an exclusion already owned by another organization in the realm—queue duplicate(REALM_ID, NAME)rows and abort the upgrade; deduplicate against both persisted and already-planned realm-wide domain names.
Set<String> existingDomainNames = new HashSet<>();
Set<String> insertedDomains = new HashSet<>();
- Files reviewed: 42/43 changed files
- Comments generated: 5
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| private String identityProviderAlias; | ||
| private boolean autoRedirect; |
| type LinkRepresentation = { | ||
| alias: string[] | string; | ||
| hideOnLogin: boolean; | ||
| config: { | ||
| "kc.org.domain": string; | ||
| "kc.org.excluded.domains": string; | ||
| }; | ||
| config: Record<string, string>; |
Unreported flaky test detectedIf the flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR. org.keycloak.testsuite.adapter.servlet.SAMLServletAdapterTest#testUserAttributeStatementMapperGroupsAggregateKeycloak CI - Adapter IT Strict Cookies org.keycloak.testsuite.adapter.servlet.SAMLServletAdapterTest#testUserAttributeStatementMapperUserGroupsNoAggregateKeycloak CI - Adapter IT Strict Cookies org.keycloak.testsuite.adapter.servlet.SAMLServletAdapterTest#idpInitiatedLoginTestKeycloak CI - Adapter IT Strict Cookies |
Closes keycloak#48907 Signed-off-by: vramik <vramik@redhat.com>
a1687c2 to
f9b167b
Compare
Unreported flaky test detectedIf the flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR. org.keycloak.testsuite.federation.ldap.LDAPUserLoginTest#loginLDAPUserAuthenticationSimpleEncryptionStartTLSKeycloak CI - Java Distribution IT (windows-latest - temurin - 21) |
There was a problem hiding this comment.
🟡 Changes recommended
Migration conflicts, an N+1 query regression, and incomplete public API compatibility can break upgrades and consumers.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (3)
model/storage-private/src/main/java/org/keycloak/migration/migrators/MigrateTo26_8_0.java:105
- An old exclusion can already be owned as a domain by another organization in the realm. Materializing it on the current organization then fails
validateDomain, so importing an otherwise valid pre-26.8 realm fails; realm-wide existing exclusions need to be skipped or translated without creating a conflicting domain.
for (String excluded : excludedDomains) {
if (!existingNames.contains(excluded)) {
result.add(new OrganizationDomainModel(excluded, false, null, false));
model/jpa/src/main/java/org/keycloak/models/jpa/JpaIdentityProviderStorageProvider.java:607
toModelis applied per row bygetAllStream, so this call now executes an additional organization-link query for every identity provider. That introduces an N+1 query on login/admin list paths; load the link through the entity query/association or batch organization IDs for streamed results.
identityProviderModel.setOrganizationId(getOrganizationIdForIdp(entity.getInternalId()));
core/src/main/java/org/keycloak/representations/idm/OrganizationDomainRepresentation.java:36
- The corresponding public TypeScript admin-client type (
js/libs/keycloak-admin-client/src/defs/organizationDomainRepresentation.ts) still exposes onlynameandverified. TypeScript consumers therefore cannot use these new REST fields without bypassing type checking; update the mirrored client representation as part of this API change.
private String identityProviderAlias;
private boolean autoRedirect;
- Files reviewed: 39/39 changed files
- Comments generated: 3
- Review effort level: Balanced
| statements.add(new InsertStatement(null, null, orgDomainTable) | ||
| .addColumnValue("ID", domainId) | ||
| .addColumnValue("NAME", excludedPattern) | ||
| .addColumnValue("VERIFIED", false) | ||
| .addColumnValue("REALM_ID", realmId) | ||
| .addColumnValue("ORG_ID", orgId) |
| String ORGANIZATION_NAME_ATTRIBUTE = "kc.org.name"; | ||
| String ORGANIZATION_DOMAIN_ATTRIBUTE = "kc.org.domain"; | ||
| String ORGANIZATION_EXCLUDED_DOMAIN_ATTRIBUTE = "kc.org.excluded.domains"; | ||
| String ALIAS = "alias"; |
| if (excludedDomainsValue != null) { | ||
| for (String d : excludedDomainsValue.split(",")) { | ||
| excludedDomains.add(d.trim()); | ||
| } | ||
| } |
* [Roles] Treat removing a user from a group they are not in as done Keycloak 26.8 answers a DELETE on an organization group member who is not in the group with 400 "User not a member" (keycloak/keycloak#52588), where 26.7 answered 204. Changing a role clears every other role group, and removing a member clears all of them, so almost every call lands on a group the user was never in. On 26.8 each of those would fail the role change or member removal. Match the message as well as the status, since 400 also covers malformed requests. Verified against a Keycloak built from main: 26.7.3 returns 204 and the 26.8 build returns 400 {"errorMessage":"User not a member"}. Claude-Session: https://claude.ai/code/session_01RvN9oeQKKP4e5H5TvnNa6h * [Auth] Keep Keycloak 26.8 domain routing through organization updates Keycloak 26.8 moves an organization's SSO routing off the identity provider config and onto each domain, as identityProviderAlias and autoRedirect (keycloak/keycloak#52401). Its migration copies the existing kc.org.domain and redirect-on-email-match config across and deletes it. From then on an organization PUT replaces every domain with what it carries, so a domain sent without those fields loses its provider and redirect. Domain carried only name and verified, so every organization update maki makes (billing, pending SSO domains, SetOrganizationDomains) would silently turn SSO enforcement off on 26.8. Round-trip both fields, omitted when unset. 26.7.3 answers 400 "Unrecognized field identityProviderAlias" for a domain carrying either, and never returns them, so this changes nothing before the upgrade and must land before it. Claude-Session: https://claude.ai/code/session_01RvN9oeQKKP4e5H5TvnNa6h * Trim comments Claude-Session: https://claude.ai/code/session_01RvN9oeQKKP4e5H5TvnNa6h * Apply suggestion from @SferaDev
* [Roles] Treat removing a user from a group they are not in as done Keycloak 26.8 answers a DELETE on an organization group member who is not in the group with 400 "User not a member" (keycloak/keycloak#52588), where 26.7 answered 204. Changing a role clears every other role group, and removing a member clears all of them, so almost every call lands on a group the user was never in. On 26.8 each of those would fail the role change or member removal. Match the message as well as the status, since 400 also covers malformed requests. Verified against a Keycloak built from main: 26.7.3 returns 204 and the 26.8 build returns 400 {"errorMessage":"User not a member"}. Claude-Session: https://claude.ai/code/session_01RvN9oeQKKP4e5H5TvnNa6h * [Auth] Keep Keycloak 26.8 domain routing through organization updates Keycloak 26.8 moves an organization's SSO routing off the identity provider config and onto each domain, as identityProviderAlias and autoRedirect (keycloak/keycloak#52401). Its migration copies the existing kc.org.domain and redirect-on-email-match config across and deletes it. From then on an organization PUT replaces every domain with what it carries, so a domain sent without those fields loses its provider and redirect. Domain carried only name and verified, so every organization update maki makes (billing, pending SSO domains, SetOrganizationDomains) would silently turn SSO enforcement off on 26.8. Round-trip both fields, omitted when unset. 26.7.3 answers 400 "Unrecognized field identityProviderAlias" for a domain carrying either, and never returns them, so this changes nothing before the upgrade and must land before it. Claude-Session: https://claude.ai/code/session_01RvN9oeQKKP4e5H5TvnNa6h * Trim comments Claude-Session: https://claude.ai/code/session_01RvN9oeQKKP4e5H5TvnNa6h * [Keycloak] Upgrade to 26.8.0 Keycloak 26.8 moves organization domain routing off identity provider config and onto each organization domain (keycloak/keycloak#52401). It deletes OrganizationModel.ORGANIZATION_DOMAIN_ATTRIBUTE, ORGANIZATION_EXCLUDED_DOMAIN_ATTRIBUTE and IdentityProviderRedirectMode, so the extensions no longer compile, and its migration removes the config keys DomainSso read, so the old logic would have found nothing to enforce. DomainSso now reads the routing from the most specific matching organization domain, as OrganizationAuthenticator does. A subdomain held without a provider opts out of a wildcard that has one, which replaces the excluded-domains list. assertsOwnDomain binds a provider to the domains routed to it across the organizations it is linked to. Claude-Session: https://claude.ai/code/session_01RvN9oeQKKP4e5H5TvnNa6h * [Keycloak] Accept invitations through brokers with Keycloak's own step Keycloak 26.8 keeps an organization invitation across the redirect to an identity provider and has idp-add-organization-member accept it, whichever provider the invitee used (keycloak/keycloak#52183). The xata capture and accept authenticators existed only to do that, so the flows stop calling them. The providers stay in the image for now. Keycloak cannot list the executions of a flow that names a provider it does not have, and keycloak-config-cli reads them before it rewrites the flow, so an image without the providers fails the realm import that would have removed them. They go in a follow-up. Claude-Session: https://claude.ai/code/session_01RvN9oeQKKP4e5H5TvnNa6h * [SSO] Route providers through Keycloak 26.8 organization domains Keycloak 26.8 keeps an organization's SSO routing on each domain, as identityProviderAlias and autoRedirect, instead of in identity provider config (keycloak/keycloak#52401). Its migration moves kc.org.domain and the redirect-on-email-match flag across and deletes them, and every later write of an organization-linked provider strips them again. The SSO service still read and wrote those keys, so after the upgrade every provider would show no domain and no enforcement, and turning enforcement on would silently do nothing. Read a provider's domain and enforcement from the domain routed to it, and write routing through SetOrganizationDomains: CreateProvider routes the domain once the provider is linked (Keycloak refuses a provider the organization is not linked to), SetProviderEnforcement flips autoRedirect, and credential rotation no longer carries routing because Keycloak leaves it in place. Revoking a lapsed domain now unverifies and unroutes it in one write, which removes the ordering the separate provider update needed. Claude-Session: https://claude.ai/code/session_01RvN9oeQKKP4e5H5TvnNa6h * [Keycloak] Trust SSO emails on verified domains and refuse unrouted providers Keycloak 26.8 takes a brokered email's verified state from the provider's email_verified claim and reads a missing one as false (keycloak/keycloak#50699), even with trustEmail. Entra's userinfo never sends the claim, so its users would be asked to verify an address their organization already controls. xata-trust-domain-email, in post broker login, marks the email verified when an organization's own provider asserted it on a verified domain routed to that provider. The organization proved it controls the domain's DNS, and with it the mail, so a verification mail would prove nothing more. It leaves the email alone when it was changed on the review page, when it is not what this login asserted, and for shared providers. xata-require-domain-sso also stops treating an organization's provider that no domain routes to as shared. That is what the domain recheck leaves once a domain lapses, and it could otherwise assert any address. Claude-Session: https://claude.ai/code/session_01RvN9oeQKKP4e5H5TvnNa6h * Trim comments Claude-Session: https://claude.ai/code/session_01RvN9oeQKKP4e5H5TvnNa6h * [SSO] Rely on Keycloak 26.8 to keep a domain to one organization Keycloak 26.8 adds a unique constraint on an organization domain's realm and name. Two organizations verifying the same domain at once no longer both succeed: the loser's update is rejected with 409, which a local run confirmed across repeated concurrent claims. Before, uniqueness was checked in application code only, so verification wrote the domain, listed every holder, and backed itself out when it found another. Map the 409 to ErrDomainAlreadyClaimed and let VerifyDomain return ErrDomainClaimedElsewhere from it directly. That drops backOutIfHeldElsewhere, the playbook section for recovering a domain held by two organizations, and OrganizationsForDomain's list of holders, which is now OrganizationForDomain. Also: - CreateProvider and DeleteDomain read the provider off the domain's routing instead of listing every identity provider. - SetOrganizationDomains matches Keycloak's one wording for each rejection. - autoRedirect is always sent; identityProviderAlias stays omitted when unset, since 26.8 rejects an empty alias. Claude-Session: https://claude.ai/code/session_01RvN9oeQKKP4e5H5TvnNa6h * Disable SSO enforcement through domain routing in the admin gate test Main added the test against redirect-on-email-match provider config, which Keycloak 26.8 replaces with org domain autoRedirect. * Keep SSO-brokered organization members managed on Keycloak 26.8 Keycloak 26.8 moves membership config onto each organization and identity provider link. The link POST takes only the provider alias and creates the link with autoMembership=true and membershipType=UNMANAGED, while links that exist at upgrade time are migrated to MANAGED. Organizations that set up SSO after the upgrade would therefore get unmanaged members, whose accounts survive removal from the organization, unlike every SSO member today. Set the link to autoMembership=true and MANAGED right after linking, including when the link already exists, so every organization keeps the behaviour it has on 26.7. * Make every organization member unmanaged Keycloak deletes a managed member's account when they are removed from the organization or the organization is deleted. An account can belong to other organizations and outlive any one of them, so no member should be managed. Drop the PUT that set SSO links to MANAGED after linking. The link POST already creates them with autoMembership=true and UNMANAGED, so the PUT has nothing left to do for new links, and depending only on the POST keeps SSO setup working against a Keycloak that predates the per-link endpoint. Links that the 26.8 migration turns MANAGED are fixed once by a separate tool. Keycloak adds an invitee who registers through the invitation link as a managed member. Have the invitation role listener make them unmanaged after the join commits, in its own transaction so a failure there is logged and cannot stop the role grant. * [SSO] Fix the merged SSO tests for domain routing * [SSO] Describe enforcement on 26.8 domain routing in the lockout playbook * [SSO] Leave a provider being created alone in the domain recheck The recheck disabled any enabled provider no domain routed to, but CreateProvider links a provider before routing its domain to it, so a recheck landing between the two switched off a provider the create then reported as working. Disable only providers whose domain is no longer verified. Re-verify now routes before enabling, so a failure between the two leaves the provider off. * [SSO] Name the provider when enforcing one no domain routes to Enforcing a provider whose domain the recheck revoked answered "domain is not verified" with no domain, since none routes to it. Answer with ErrProviderUnbound as UpdateProvider does, and say in the broker log that the provider is outside every domain routed to it. * [SSO] Expect the unbound error when enforcing an unrouted provider * [Keycloak] Vendor the 26.8.0 operator manifest and CRDs Regenerated from keycloak-k8s-resources 26.8.0 rather than editing the version strings: the operator moves to Quarkus 3.40.1 and sets a 10s termination grace period, and the realm import CRD gains organization links and per-domain routing. Our namespace templating, kube-linter annotations, enableServiceLinks and tolerations are reapplied. * [Keycloak] Turn off SCIM and block cluster node registration Keycloak 26.8 turns the SCIM API on by default, and we don't use it, so disable it at build time and in the Keycloak CR together. 26.8 also adds an executor for the legacy register-node endpoint, which otherwise lets a confidential client point management callbacks at any host; add it to the SSRF profile with no allowed hostnames, since nothing registers nodes. * [Keycloak] Hold the realm import until Keycloak runs the current spec Flux applies the Keycloak CR and the import-keycloak-realm Job together when the image changes. The operator takes a while to replace the pods (update compatibility Job, then a recreate or rolling update), and in that window the old Keycloak still answers, so keycloak-config-cli's availability check passes and it imports the new realm.json into the old version. On the 26.8 upgrade that fails on authenticators the old image lacks, or applies part of the realm before the pod goes away. Add a wait-for-keycloak init container that polls until the Keycloak CR's Ready condition is True at its current generation and the StatefulSet has rolled out that spec: observed its own generation, current revision equals update revision, every instance ready, and the keycloak container image equals the CR image. Ready and observedGeneration alone are not enough: the operator computes status from the StatefulSet it read before reconciling, so after deciding an update it reports Ready=True at the new generation while the StatefulSet is only now being scaled down or rolled. The image and rollout checks close that window. When only realm.json changes the CR and StatefulSet are already settled and the wait passes on its first poll. The init container reuses alpine/k8s, pinned by digest, under a dedicated ServiceAccount allowed to get only the auth-keycloak Keycloak and StatefulSet. realmImport.waitForKeycloak.timeoutSeconds bounds the wait. * [Keycloak] Hold the realm import until Keycloak runs the current spec Flux applies the Keycloak CR and the import-keycloak-realm Job together when the image changes. The operator takes a while to replace the pods (update compatibility Job, then a recreate or rolling update), and in that window the old Keycloak still answers, so keycloak-config-cli's availability check passes and it imports the new realm.json into the old version. On the 26.8 upgrade that fails on authenticators the old image lacks, or applies part of the realm before the pod goes away. Add a wait-for-keycloak init container that polls until the Keycloak CR's Ready condition is True at its current generation and the StatefulSet has rolled out that spec: observed its own generation, current revision equals update revision, every instance ready, and the keycloak container image equals the CR image. Ready and observedGeneration alone are not enough: the operator computes status from the StatefulSet it read before reconciling, so after deciding an update it reports Ready=True at the new generation while the StatefulSet is only now being scaled down or rolled. The image and rollout checks close that window. When only realm.json changes the CR and StatefulSet are already settled and the wait passes on its first poll. The init container reuses alpine/k8s, pinned by digest, under a dedicated ServiceAccount allowed to get only the auth-keycloak Keycloak and StatefulSet. realmImport.waitForKeycloak.timeoutSeconds bounds the wait. * [Keycloak] Bind the realm import to the intended chart revision * [Keycloak] Move invitation membership protection to the migration PR * [SSO] Defer domain uniqueness cleanup until after the upgrade * [Keycloak] Test realm import deployment ordering * Test realm import readiness in CI and bound API requests * Refresh domain ownership before disabling SSO providers * Use existing chart tooling for realm import regression tests * Replace the import polling guard with an explicit two-stage upgrade * Keep upstream SCIM feature defaults without enabling it for the realm * Separate brokered email trust from the Keycloak compatibility upgrade * Defer the built-in invitation flow switch to the invitation rollout * Document validation gates for the narrower upgrade boundary * Remove the Keycloak upgrade playbook
Closes #48907