Repository navigation
Resolve the organization when an invitation is confirmed - #52397
Conversation
88ae257 to
aa62b16
Compare
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Fixes missing organization resolution for existing users confirming an organization invitation via action token, ensuring FreeMarker templates receive the expected org attributes.
Changes:
- Set the organization on the request context during
handleToken(not onlypreHandleToken). - Ensure the confirm-membership (and related) pages can resolve
orgfor already-registered invitees.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
aa62b16 to
dd0091f
Compare
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
* [Keycloak] Accept organization invitations through an identity provider An invitation sent to somebody with no account produces a registration link, and only the local registration form can consume it. An invitee who signs in with Google or GitHub instead is left with an account and no membership, which is why AUTH_SEED_INVITED_USERS exists: seeding an account first keeps every invite on the action-token link. Seeding is a poor trade. It leaves an orphan account behind whenever an invitation is cancelled or the address was a typo, it turns password signup into a password reset, and the nameless account it creates makes Keycloak ask every invitee to fill in a profile form. This does what the invitation actually needs instead. Two authenticators: one pins the invitation token to the authentication session while it is still on the request, the other adds the member once the invitee comes back from the provider. The token has to be carried by hand because the broker login URL does not include it and resetFlow clears every auth note before first broker login runs, though not client notes. Additive on its own: while seeding is enabled every invite is still action-token shaped, so neither authenticator does anything. Turning AUTH_SEED_INVITED_USERS off is what switches the behaviour over. Claude-Session: https://claude.ai/code/session_01JFGXVUDeerTBCXWDed8k7d * Simplify the invitation authenticators Return the organization alongside the token so the accept side does not look it up twice, drop the overrides AbstractIdpAuthenticator already provides, and take the request rather than the flow context in the helper. configuredFor stays true and now says why: a REQUIRED execution whose authenticator is not configured for the user aborts with "credential setup required", which is what returning false produced. Claude-Session: https://claude.ai/code/session_01JFGXVUDeerTBCXWDed8k7d * [Auth] Remove the invite seeding workaround Seeding pre-created a Keycloak account for every invitee so that Keycloak would mint an action-token invite link rather than a registration one. The authenticators added in the previous commit accept the registration link directly, so nothing needs the account to exist up front. Removing it takes three problems with it: the enabled, pre-verified, credential-less account left behind whenever an invitation was cancelled or the address was a typo; password signup turning into a password reset; and the profile form every invitee saw, because a seeded account has no name for VERIFY_PROFILE to find. Claude-Session: https://claude.ai/code/session_01JFGXVUDeerTBCXWDed8k7d * [Keycloak] Rename the turnstile plugin to keycloak-extensions The module started as a Turnstile plugin and now also carries the organization invitation authenticators, with more likely to follow. The name no longer describes it. Renames the directory, the artifact, the jar, the make targets and the Maven cache volume. Turnstile itself is untouched, and the README now describes the module rather than one of its providers. Claude-Session: https://claude.ai/code/session_01JFGXVUDeerTBCXWDed8k7d * [Keycloak] Show organization display names on the invite page Organization invitations addressed to someone who already has an account render Keycloak's confirm-membership page, and that page names the organization by its id rather than its display name. Keycloak resolves the organization onto the request context only in InviteOrgActionTokenHandler.preHandleToken, which LoginActionsService calls from the registration entry points alone; executeActionToken dispatches straight to handleToken. Organizations.resolveOrganization then falls through to a membership check the invitee cannot pass yet, so FreeMarkerLoginFormsProvider leaves the organization out of the template model and the page is left with the bare kc.org.name attribute. Subclass the handler to set the organization in handleToken and register it under the same ORGIVT id: ProviderManager prefers a factory with a higher order(), and prefers ours over an org.keycloak one regardless. This is what auth-theme 0.4.8 needs to replace the id with the displayName attribute. Without it that theme change only takes effect on the registration page, where the resolution happens to succeed because there is no user yet to fail the membership check. Claude-Session: https://claude.ai/code/session_012jDEZeEHna3t2uAcrDvVGW * [Keycloak] Guard the invite handler and link the upstream fix Only resolve the organization when organizations are enabled and the organization itself is enabled. Organizations.isEnabled null-checks the provider, so with the feature off the lookup would have thrown before the stock handler could return its disabled-organization page, and the enabled check mirrors where the upstream patch sits. Filed as keycloak/keycloak#52396 with the fix in keycloak/keycloak#52397, so this can go once a release carries it. Claude-Session: https://claude.ai/code/session_012jDEZeEHna3t2uAcrDvVGW * [Keycloak] Accept organization invitations through an identity provider An invitation sent to somebody with no account produces a registration link, and only the local registration form can consume it. An invitee who signs in with Google or GitHub instead is left with an account and no membership, which is why AUTH_SEED_INVITED_USERS exists: seeding an account first keeps every invite on the action-token link. Seeding is a poor trade. It leaves an orphan account behind whenever an invitation is cancelled or the address was a typo, it turns password signup into a password reset, and the nameless account it creates makes Keycloak ask every invitee to fill in a profile form. This does what the invitation actually needs instead. Two authenticators: one pins the invitation token to the authentication session while it is still on the request, the other adds the member once the invitee comes back from the provider. The token has to be carried by hand because the broker login URL does not include it and resetFlow clears every auth note before first broker login runs, though not client notes. Additive on its own: while seeding is enabled every invite is still action-token shaped, so neither authenticator does anything. Turning AUTH_SEED_INVITED_USERS off is what switches the behaviour over. Claude-Session: https://claude.ai/code/session_01JFGXVUDeerTBCXWDed8k7d * Simplify the invitation authenticators Return the organization alongside the token so the accept side does not look it up twice, drop the overrides AbstractIdpAuthenticator already provides, and take the request rather than the flow context in the helper. configuredFor stays true and now says why: a REQUIRED execution whose authenticator is not configured for the user aborts with "credential setup required", which is what returning false produced. Claude-Session: https://claude.ai/code/session_01JFGXVUDeerTBCXWDed8k7d * [Auth] Remove the invite seeding workaround Seeding pre-created a Keycloak account for every invitee so that Keycloak would mint an action-token invite link rather than a registration one. The authenticators added in the previous commit accept the registration link directly, so nothing needs the account to exist up front. Removing it takes three problems with it: the enabled, pre-verified, credential-less account left behind whenever an invitation was cancelled or the address was a typo; password signup turning into a password reset; and the profile form every invitee saw, because a seeded account has no name for VERIFY_PROFILE to find. Claude-Session: https://claude.ai/code/session_01JFGXVUDeerTBCXWDed8k7d * [Keycloak] Rename the turnstile plugin to keycloak-extensions The module started as a Turnstile plugin and now also carries the organization invitation authenticators, with more likely to follow. The name no longer describes it. Renames the directory, the artifact, the jar, the make targets and the Maven cache volume. Turnstile itself is untouched, and the README now describes the module rather than one of its providers. Claude-Session: https://claude.ai/code/session_01JFGXVUDeerTBCXWDed8k7d
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
05c62e5 to
6dc6aa4
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Organization attributes can be exposed before expired or revoked invitations are validated.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Balanced
martin-kanis
left a comment
There was a problem hiding this comment.
Thanks for the issue and PR.
6dc6aa4 to
5ad64d7
Compare
martin-kanis
left a comment
There was a problem hiding this comment.
Thanks for the update. Looks good.
I noticed one more thing, please see below.
InviteOrgActionTokenHandler sets the organization on the request context in preHandleToken, which is only reached through LoginActionsService.preHandleActionToken and so only runs on the registration entry points. An invitee who already has an account opens the action token URL, which executeActionToken dispatches straight to handleToken, leaving the context organization unset. Organizations.resolveOrganization then has neither a context organization nor an ORGANIZATION_ATTRIBUTE auth note, falls through to the membership check an invitee cannot pass yet, and returns null, so FreeMarkerLoginFormsProvider leaves org out of the login form model. An organization aware theme therefore cannot brand the confirm membership page and can only name the organization by its name. Set it in handleToken as well, under the same rule as preHandleToken: for an existing member, or once the invitation is known to be valid, so a revoked or expired link does not expose the organization's attributes. Cover it from the new test framework, with an organization login theme in the custom providers that prints what the org bean exposes, and give RealmBuilder a loginTheme option so the test can select that theme. Closes keycloak#52396 Signed-off-by: Alexis Rico <sferadev@gmail.com>
5ad64d7 to
18fdd85
Compare
|
Thanks again, @SferaDev. |
* [Keycloak] Remove the organization invitation authenticators No flow calls xata-capture-org-invitation or xata-accept-org-invitation any more: Keycloak 26.8 accepts an invitation through a broker itself (keycloak/keycloak#52183). Deploy only after the flows change is live, since a realm import against an image without these providers fails while the flows still name them. OrgInvitationTokenHandler stays until keycloak/keycloak#52397 is released. * [Keycloak] Drop the invitation token handler now that 26.8 sets the organization Keycloak 26.8 includes keycloak/keycloak#52397, which puts the inviting organization on the request context in handleToken as well, so the confirm-membership page gets org without our override.
Closes #52396
InviteOrgActionTokenHandlersets the organization on the request context inpreHandleToken:preHandleTokenis only reached throughLoginActionsService.preHandleActionToken, which has two callers, both on the registration path:AuthorizationEndpoint.registerandLoginActionsService.registerRequest. An invitee who already has an account opens the action token URL instead, andexecuteActionTokencallshandleActionToken(key, execution, clientId, tabId, clientData, null), so the pre-handler is skipped andhandleTokenruns on its own.With no context organization and no
OrganizationModel.ORGANIZATION_ATTRIBUTEauth note,Organizations.resolveOrganizationreacheswhich an invitee cannot satisfy, since not yet being a member is what the invitation is for. It returns null, so
FreeMarkerLoginFormsProvider.createCommonAttributesnever runsand the confirm-membership page is left with only the
kc.org.nameattribute thatconfirmMembershipResponsesets.register.ftlis unaffected because a brand new invitee has no user yet andresolveOrganizationreturns the organization early whenuser == null.This sets the organization in
handleTokentoo, after theisEnabledcheck soalreadyMemberResponseandinvalidTokenResponseresolve it as well. It reuses theOrganizationModelthe method already holds and theKeycloakSessionalready in scope, so there are no new lookups or imports.I also considered having
executeActionTokenrunpreHandleTokenbeforehandleToken. That is a wider change for no extra coverage:InviteOrgActionTokenHandleris the only handler that overridespreHandleTokenand every other one inherits thereturn nulldefault, and it would have to run ahead of the verifiers that check the invited email against the authenticated user. Happy to switch if you would rather have it there.Verified on a 26.7.3 build with an organization named
k9d2m7carryingdisplayName = Acme Corp Ltd:orgis now present on the confirm-membership page with its attributes, accepting the invitation still adds the member and redirects to the organization, and the registration, already-member and stale-link pages are unchanged.AI usage disclosure: an AI agent assisted with the investigation and drafting. I have reviewed and understand the change, and will handle review feedback myself.