Repository navigation
Organization group: Error response when removing non-member from org group - #52588
Merged
Merged
Conversation
…group When attempting to remove a user who is not a member of an organization group, the endpoint should return 400 Bad Request, not silently succeed with 204 No Content. This ensures correct REST semantics and enables proper audit logging of failed removal operations. Fixes keycloak#52587 Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The failed removal still does not emit the audit event required by issue #52587.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Returns HTTP 400 when removing a non-member from an organization group.
Changes:
- Rejects invalid group-member removals.
- Adds integration coverage for the response status.
File summaries
| File | Description |
|---|---|
OrganizationGroupResource.java |
Adds the non-member validation response. |
OrganizationGroupMembershipTest.java |
Tests the HTTP 400 behavior. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
vramik
approved these changes
Sep 10, 2026
ahus1
approved these changes
Sep 10, 2026
Member
There was a problem hiding this comment.
Approving based on existingreview.
xata-bot
pushed a commit
to xataio/xata
that referenced
this pull request
Sep 15, 2026
* [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
xata-bot
pushed a commit
to xataio/xata
that referenced
this pull request
Oct 6, 2026
* [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 remove 8000 d 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
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.
When attempting to remove a user who is not a member of an organization group, the endpoint should return 400 Bad Request, not silently succeed with 204 No Content. This ensures correct REST semantics and enables proper audit logging of failed removal operations.
Fixes #52587