OIDC: end the provider session when FOG's session ends - #18
Merged
Conversation
Closes #15. Clicking Log out destroyed FOG's session and left the identity provider's SSO session completely untouched. Clicking the provider button again then re-authenticated silently and dropped the same person straight back into the same account, so there was no way to sign in as somebody else short of clearing cookies or opening a private window. On an account carrying uAuthSource='oidc' -- which core refuses a local password by design -- there was no way to sign in as anybody else at all. The plugin implemented no logout of any kind. The ID token was decoded during verification and then discarded, so even the standard id_token_hint for RP-initiated logout was not available to send. ## The setting `opSingleLogout`, per provider, **off by default**, added by an appended schema step so an existing install picks it up and by createSql() so a fresh one does. Off is not timidity: ending the provider session is only the right answer where FOG is the only application behind that provider. Where an install shares an identity provider with a mail client and a ticket system, ending the SSO session because somebody left FOG reaches applications FOG has nothing to do with. ## Recorded at sign-in, never fetched at logout `_rememberLogout()` stores the end_session_endpoint and the ID token in the session at callback time, and `logoutUrl()` builds the redirect from that and nothing else. The alternative -- discovering at logout -- puts a network request on the sign-out path, which turns a provider that has gone away into a Log out button that hangs and then fails, at the exact moment somebody is trying to leave. Two orderings in there are load-bearing and both are pinned by the gate: - It records **after** establishSession(). callback() empties $_SESSION wholesale immediately before that call, to stop an identity already in the session deciding the new one, so anything stored earlier is thrown away and the feature is simply dead with nothing to show for it. - The hook fires **before** core destroys the session (fogproject#1174), which is what leaves the stored token readable. The provider row is re-read at logout rather than trusted from the session, so turning the setting off means from that moment rather than from the next time everybody happens to sign in, and a provider since deleted or disabled gets no redirect built from a row that no longer says anything. ## post_logout_redirect_uri points at management/login.php Not index.php, and this is the part that matters once #17 lands: index.php is precisely the page a forced-redirect install bounces back to the provider. A signed-out user landing there is either signed straight back in by an SSO session that was not really ended, or sent around the loop. management/login.php (fogproject#1175) always renders FOG's own form. That URL has to be registered at the provider as a post-logout redirect URI, the same way the callback does -- providers that follow the spec refuse an unregistered one and show their own error page instead of coming back. So the management page prints it read-only next to the checkbox, exactly as it already prints the redirect URI, rather than leaving an admin to work out why logout ends somewhere unexpected. `OIDC::redirectUri()` now goes through a shared `absoluteUrl()` helper. Verified byte-identical output for webroots `/fog/`, `/` and `/deep/path/` -- that value is registered at providers by hand and compared byte for byte, so a changed slash would break every existing install's sign-in. ## Verification `tests/oidc-single-logout.test.php`, 22 checks, **20/20 mutations caught**, including both schema halves separately (dropping createSql's column serves only new installs, dropping the ALTER serves only old ones -- and shipping one of the two looks complete in review because whichever install it was tested on happened to be the served kind), the two orderings above, a network call sneaking into the logout path, and the step being inserted rather than appended. `sh tests/run-all.sh` -> 5 passed, 0 failed. Not exercised end to end against the lab Keycloak yet; that needs a live sign-in and sign-out and is the next thing to do. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017aBSWrDArXHTpKWkkN27LR
This was referenced Aug 18, 2026
mastacontrola
added a commit
that referenced
this pull request
Aug 18, 2026
…edirects (#20) Reported from the lab: with single logout and forced redirect both on, signing out left you on management/login.php -- the break-glass page -- rather than on FOG's ordinary login page. That was wrong, and the reasoning behind it was wrong in a specific way. #18 pointed post_logout_redirect_uri at login.php to avoid a loop: index.php on a forced-redirect install sends the browser back to the provider, and if the provider session were still alive that is a silent re-login. But single logout is exactly the case where the provider session is NOT still alive. It was just ended, by the request that is carrying this parameter. So the browser goes back to index.php, index.php sends it to the provider, the provider has no session, and it asks who you are. That is signing out and signing in as somebody else in one continuous journey, which is the thing single logout exists to make possible -- and it was being cut short one page early. So the two landings are now separate, because they answer different questions: OIDC::postLogoutUri() -> management/index.php. The provider session HAS been ended, so bouncing back to it is correct. OIDC::localLoginUrl() -> management/login.php. Used where bouncing back really would loop or silently sign the person in again: - OIDCFlow::_fail(), where the provider just refused, so returning to it is a loop - the USER_LOGGING_OUT fallback when single logout is OFF and forced redirect is on, where the SSO session is untouched and for the URL printed beside the forced-redirect setting, which is the escape hatch and must never be the redirecting page. ADMINS MUST RE-REGISTER: the post-logout redirect URI to register at the provider changes from .../management/login.php to .../management/index.php. The management page prints the current value; providers that follow the spec refuse an unregistered one and show their own error page instead of returning, so an install upgrading from v1.6.9 has to update it there. Both gates updated. The single-logout gate now pins the two URLs separately, pins that they differ (collapsing them re-creates whichever bug the other was avoiding), and pins post_logout_redirect_uri as a KEY and value pair rather than merely mentioning the method -- a mutation that rebound the value to another parameter name survived the first draft. 7/7 mutations caught; sh tests/run-all.sh -> 6 passed, 0 failed. Claude-Session: https://claude.ai/code/session_017aBSWrDArXHTpKWkkN27LR Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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 #15. Consumes the
USER_LOGGING_OUTredirect seam fromFOGProject/fogproject#1174.
The problem
Clicking Log out destroyed FOG's session and left the provider's SSO
session completely untouched, so clicking the provider button again
re-authenticated silently and dropped the same person straight back into the
same account. No way to sign in as somebody else short of clearing cookies. On
an account carrying
uAuthSource='oidc'— which core refuses a local passwordby design — no way to sign in as anybody else at all.
The plugin implemented no logout of any kind. The ID token was decoded during
verification and then discarded, so even the standard
id_token_hintwas notavailable to send.
The setting
opSingleLogout, per provider, off by default — added by an appendedschema step (existing installs) and by
createSql()(fresh ones).Off is not timidity. Ending the provider session is only right where FOG is the
only application behind that provider; where an install shares an IdP with a
mail client and a ticket system, ending the SSO session because somebody left
FOG reaches applications FOG has nothing to do with.
Recorded at sign-in, never fetched at logout
_rememberLogout()stores theend_session_endpointand the ID token in thesession at callback time;
logoutUrl()builds the redirect from that andnothing else. Discovering at logout would put a network request on the sign-out
path — turning a provider that has gone away into a Log out button that hangs
and then fails, at the exact moment somebody is trying to leave.
Two orderings are load-bearing, both pinned:
establishSession().callback()empties$_SESSIONwholesale immediately before that call (to stop an identity already in the
session deciding the new one), so anything stored earlier is thrown away and
the feature is dead with nothing to show for it.
stored token readable.
The provider row is re-read at logout rather than trusted from the session, so
turning the setting off means from that moment — not from the next time
everybody happens to sign in.
post_logout_redirect_uri→management/login.phpNot
index.php, and this is what matters once #17 lands:index.phpisprecisely the page a forced-redirect install bounces back to the provider. A
signed-out user landing there is either signed straight back in by an SSO
session that was not really ended, or sent around the loop.
management/login.php(FOGProject/fogproject#1175) always renders FOG's ownform.
That URL must be registered at the provider as a post-logout redirect URI, the
same way the callback is — providers that follow the spec refuse an
unregistered one and show their own error page instead of coming back. So the
management page prints it read-only next to the checkbox, exactly as it already
prints the redirect URI.
OIDC::redirectUri()now goes through a sharedabsoluteUrl()helper.Verified byte-identical for webroots
/fog/,/and/deep/path/— thatvalue is registered at providers by hand and compared byte for byte.
Verification
tests/oidc-single-logout.test.php— 22 checks, 20/20 mutations caught.Comments are stripped first; the prose above each method names every symbol the
test searches for.
Worth calling out among the mutations: the two schema halves are pinned
separately. Dropping
createSql()'s column serves only installs made afterthis commit; dropping the ALTER serves only installs made before it. Shipping
one of the two looks complete in review, because whichever install it was tested
on happened to be the served kind.
sh tests/run-all.sh→ 5 passed, 0 failed.Not yet exercised end to end against the lab Keycloak — that needs a live
sign-in and sign-out, and is the next thing to do.
Follow-ups
redirect without single logout means the first user to sign in owns the
browser.
FOGProject/fog-docs, and are better written alongsideOIDC: option to send the login page straight to the provider #17 when there are two settings to document together.
🤖 Generated with Claude Code
https://claude.ai/code/session_017aBSWrDArXHTpKWkkN27LR