From 2370ccd7d0385c111df5324d93c55cabbf9a5000 Mon Sep 17 00:00:00 2001 From: Tom Elliott Date: Tue, 18 Aug 2026 06:55:18 -0500 Subject: [PATCH] OIDC: end the provider session when FOG's session ends 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 Claude-Session: https://claude.ai/code/session_017aBSWrDArXHTpKWkkN27LR --- oidc/class/oidc.class.php | 48 +++- oidc/class/oidcflow.class.php | 119 +++++++++ oidc/class/oidcmanager.class.php | 23 ++ oidc/hooks/oidclogout.hook.php | 92 +++++++ oidc/js/fog.oidc.export.js | 1 + oidc/pages/oidcmanagement.page.php | 56 ++++- tests/oidc-single-logout.test.php | 388 +++++++++++++++++++++++++++++ 7 files changed, 724 insertions(+), 3 deletions(-) create mode 100644 oidc/hooks/oidclogout.hook.php create mode 100644 tests/oidc-single-logout.test.php diff --git a/oidc/class/oidc.class.php b/oidc/class/oidc.class.php index ef34e4d..c5181cf 100644 --- a/oidc/class/oidc.class.php +++ b/oidc/class/oidc.class.php @@ -89,6 +89,11 @@ class OIDC extends FOGController // column in OIDCManager::createSql(). 'jitProvision' => 'opJITProvision', 'allowapi' => 'opAllowAPI', + // Signing out of FOG also ends the session at the provider (#15). + // Off by default: it is only the right answer where FOG is the only + // application behind that provider. See the note on the column in + // OIDCManager::createSql(). + 'singleLogout' => 'opSingleLogout', 'icon' => 'opIcon' ]; /** @@ -269,12 +274,53 @@ public static function normalizeScopes($scopes) * @return string */ public static function redirectUri() + { + return self::absoluteUrl(self::CALLBACK_PATH); + } + /** + * Where a provider sends the browser after ending its own session. + * + * management/login.php rather than management/index.php, and the + * difference is the whole point: index.php is the page an install with + * forced redirect on (#17) bounces straight back to the provider. A + * signed-out user landing there would be silently signed back in by the + * SSO session that was just ended -- or, if it really was ended, sent + * around the loop again. login.php always renders FOG's own form + * (fogproject#1175). + * + * This 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 next to the + * setting rather than leaving an admin to work out why logout ends + * somewhere unexpected. + * + * @return string + */ + public static function postLogoutUri() + { + return self::absoluteUrl('management/login.php'); + } + /** + * An absolute https URL for a path inside this FOG install. + * + * Built from FOG_WEB_HOST and FOG_WEB_ROOT rather than from the request + * for the reason spelled out on redirectUri(): these values are + * registered at a provider ahead of time and compared byte for byte, so + * they cannot be whatever the browser last claimed the server was + * called. + * + * @param string $path a path relative to the webroot + * + * @return string + */ + public static function absoluteUrl($path) { $host = trim((string)self::getSetting('FOG_WEB_HOST'), '/'); return sprintf( 'https://%s%s', $host, - rtrim(self::webrootBase(), '/') . self::CALLBACK_PATH + rtrim(self::webrootBase(), '/') . '/' . ltrim((string)$path, '/') ); } /** diff --git a/oidc/class/oidcflow.class.php b/oidc/class/oidcflow.class.php index 2091299..2ec2d3c 100644 --- a/oidc/class/oidcflow.class.php +++ b/oidc/class/oidcflow.class.php @@ -38,6 +38,17 @@ class OIDCFlow extends FOGBase * @var string */ const SESSION_KEY = 'FOG_OIDC_FLOW'; + /** + * Where the material for RP-initiated logout is kept (#15). + * + * Separate from SESSION_KEY because it has the opposite lifetime: the + * flow values are single use and deleted the moment the callback reads + * them, while this has to survive for as long as the session it belongs + * to -- it is read at logout, which may be days later. + * + * @var string + */ + const LOGOUT_KEY = 'FOG_OIDC_LOGOUT'; /** * How long a started flow stays usable, in seconds. * @@ -224,6 +235,15 @@ public static function callback() // this session from a password one, and the break-glass rules // count sessions by how they were made. $user->establishSession(OIDC::AUTH_SOURCE); + + /* + * After establishSession(), not before: the wipe above empties + * $_SESSION wholesale, so anything written earlier in this + * request would be thrown away with the identity it was + * guarding against. + */ + self::_rememberLogout($provider, $config, $token); + self::_redirect( OIDC::webrootBase() . 'management/index.php' ); @@ -231,6 +251,105 @@ public static function callback() self::_fail($e->getMessage()); } } + /** + * Stores what RP-initiated logout will need, if it is wanted. + * + * Recorded now rather than fetched at logout, and that is the whole + * design. Discovery is a network request; putting one on the sign-out + * path means a provider that has gone away turns "log out" into a page + * that hangs and then fails, at the exact moment somebody is trying to + * leave. Everything needed is already in hand here. + * + * The ID token is kept because id_token_hint is what tells the provider + * WHICH session to end, and it is the only thing that lets it skip the + * "are you sure you want to sign out?" interstitial. It is a token this + * session already holds the fruits of; the session file is not a weaker + * place to keep it than the authenticated session itself. + * + * @param OIDC $provider the provider signed in with + * @param array $config its discovery document + * @param array $token the token response + * + * @return void + */ + private static function _rememberLogout($provider, array $config, array $token) + { + if ('1' !== (string)$provider->get('singleLogout')) { + return; + } + $endpoint = (string)($config['end_session_endpoint'] ?? ''); + if (0 !== stripos($endpoint, 'https://')) { + /* + * An admin turned this on and it cannot work -- the provider + * publishes no end_session_endpoint, or publishes a plaintext + * one. Logging out will silently just be a FOG logout, which is + * indistinguishable from the setting being off, so say so + * somewhere an admin can find it. Not a thrown exception: the + * sign-in itself succeeded and refusing it here would turn a + * logout limitation into a login failure. + */ + error_log( + sprintf( + 'FOG OIDC: provider %d has single logout enabled but' + . ' published no https end_session_endpoint; signing out' + . ' of FOG will not end the provider session', + (int)$provider->get('id') + ) + ); + return; + } + $_SESSION[self::LOGOUT_KEY] = [ + 'provider' => (int)$provider->get('id'), + 'endpoint' => $endpoint, + 'idToken' => (string)$token['id_token'] + ]; + } + /** + * The provider logout URL for this session, or '' for none. + * + * Called from the USER_LOGGING_OUT listener, which core fires BEFORE it + * destroys the session -- so the values stored at callback time are + * still readable here, and this is the last moment they are. + * + * The provider row is re-read rather than trusted from the session. An + * admin who turns single logout off means it from that moment, not from + * the next time everybody happens to sign in; and a provider that has + * since been deleted or disabled must not have a URL built from a row + * that no longer says anything. + * + * @return string + */ + public static function logoutUrl() + { + if (session_status() !== PHP_SESSION_ACTIVE) { + return ''; + } + $stored = $_SESSION[self::LOGOUT_KEY] ?? null; + // Single use. A second call must not produce a second redirect, and + // the session is about to be destroyed anyway. + unset($_SESSION[self::LOGOUT_KEY]); + if (!is_array($stored) || empty($stored['endpoint'])) { + return ''; + } + $provider = self::getClass('OIDC', (int)($stored['provider'] ?? 0)); + if (!$provider->isValid() + || '1' !== (string)$provider->get('enabled') + || '1' !== (string)$provider->get('singleLogout') + ) { + return ''; + } + $query = [ + 'id_token_hint' => (string)$stored['idToken'], + 'post_logout_redirect_uri' => OIDC::postLogoutUri(), + // Sent alongside id_token_hint because some providers key the + // post-logout redirect allow-list on the client rather than on + // the token, and it is ignored by the ones that do not. + 'client_id' => (string)$provider->get('clientId') + ]; + return $stored['endpoint'] + . (false === strpos($stored['endpoint'], '?') ? '?' : '&') + . http_build_query($query); + } /** * Starts (or resumes) the session this flow needs. * diff --git a/oidc/class/oidcmanager.class.php b/oidc/class/oidcmanager.class.php index 78e20f4..62cf054 100644 --- a/oidc/class/oidcmanager.class.php +++ b/oidc/class/oidcmanager.class.php @@ -52,6 +52,7 @@ public function createSql() 'opEnabled', 'opJITProvision', 'opAllowAPI', + 'opSingleLogout', 'opIcon' ], [ @@ -69,6 +70,7 @@ public function createSql() "ENUM('0', '1')", "ENUM('0', '1')", "ENUM('0', '1')", + "ENUM('0', '1')", 'VARCHAR(255)' ], [ @@ -86,6 +88,7 @@ public function createSql() false, false, false, + false, false ], [ @@ -122,6 +125,14 @@ public function createSql() // being allowed into FOG. "'0'", "'0'", + // Signing out of FOG also signing the user out of the + // provider is off by default, and that is not timidity: it + // is only right when FOG is the only thing 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 is a surprise that + // reaches applications FOG has nothing to do with. + "'0'", "'fa fa-id-badge'" ], [ @@ -145,6 +156,7 @@ public function createSql() false, false, false, + false, false ], 'InnoDB', @@ -193,6 +205,17 @@ function () { function () { return self::getClass('OIDCUserGrantManager')->install(); }, + // 6 - RP-initiated logout, per provider (#15). Appended rather + // than folded into step 0, because installdb() SKIPS the first + // pSchema steps instead of replaying them: an install that has + // already passed step 0 would never see an edit to it, and would + // carry a providers table without this column forever. + // + // applyUpdates() tolerates 1060 (duplicate column), so an + // install created fresh from createSql() -- which already has + // the column -- runs this harmlessly too. + "ALTER TABLE `OIDCProviders` ADD COLUMN `opSingleLogout` " + . "ENUM('0', '1') NOT NULL DEFAULT '0'", ]; } /** diff --git a/oidc/hooks/oidclogout.hook.php b/oidc/hooks/oidclogout.hook.php new file mode 100644 index 0000000..f98be56 --- /dev/null +++ b/oidc/hooks/oidclogout.hook.php @@ -0,0 +1,92 @@ + + * @license http://opensource.org/licenses/gpl-3.0 GPLv3 + * @link https://fogproject.org + */ +/** + * Ends the identity provider's session when FOG's ends. + * + * @category OIDCLogout + * @package FOGProject + * @author Tom Elliott + * @license http://opensource.org/licenses/gpl-3.0 GPLv3 + * @link https://fogproject.org + */ +class OIDCLogout extends Hook +{ + /** + * The name of this hook. + * + * @var string + */ + public $name = 'OIDCLogout'; + /** + * The description. + * + * @var string + */ + public $description = 'End the provider session when FOG\'s session ends.'; + /** + * For posterity. + * + * @var bool + */ + public $active = true; + /** + * The node to work with. + * + * @var string + */ + public $node = 'oidc'; + /** + * Initialize object. + * + * @return void + */ + public function __construct() + { + parent::__construct(); + $this->registerInstalled([ + ['USER_LOGGING_OUT', 'providerLogout'] + ]); + } + /** + * Sends a signing-out user to the provider's end_session endpoint. + * + * Without this, clicking Log out destroys FOG's session and leaves the + * provider's SSO session untouched -- so clicking the provider button + * again re-authenticates silently and drops the same person straight + * back into the same account. There is then no way to sign in as + * somebody else short of clearing cookies, and on an account carrying + * uAuthSource='oidc' (which core refuses a local password) there is no + * way to sign in as anybody else at all. + * + * Only for a session that was actually made by this plugin: the values + * this reads are written by OIDCFlow::callback() and by nothing else, so + * a password session has nothing stored and this returns without + * touching $redirect. An install with no provider using the setting + * therefore behaves exactly as it did before. + * + * The hook fires before core destroys the session, which is what makes + * the stored ID token still readable -- see User::logout(). + * + * @param mixed $arguments where to send the browser instead + * + * @return void + */ + public function providerLogout($arguments) + { + $url = OIDCFlow::logoutUrl(); + if ('' === $url) { + return; + } + $arguments['redirect'] = $url; + } +} diff --git a/oidc/js/fog.oidc.export.js b/oidc/js/fog.oidc.export.js index 457e8a5..3cd1054 100644 --- a/oidc/js/fog.oidc.export.js +++ b/oidc/js/fog.oidc.export.js @@ -21,6 +21,7 @@ {data: 'enabled'}, {data: 'jitProvision', visible: false}, {data: 'allowapi', visible: false}, + {data: 'singleLogout', visible: false}, {data: 'icon', visible: false} ]); })(jQuery); diff --git a/oidc/pages/oidcmanagement.page.php b/oidc/pages/oidcmanagement.page.php index 7851a7a..59971e0 100644 --- a/oidc/pages/oidcmanagement.page.php +++ b/oidc/pages/oidcmanagement.page.php @@ -274,7 +274,8 @@ function (&$serverFault) { ->set('icon', trim((string)filter_input(INPUT_POST, 'icon'))) ->set('enabled', '0') ->set('jitProvision', '0') - ->set('allowapi', '0'); + ->set('allowapi', '0') + ->set('singleLogout', '0'); if (!$OIDC->save()) { $serverFault = true; throw new \Exception(_('Add provider failed!')); @@ -470,6 +471,53 @@ public function oidcGeneral() -1, -1, $checked('allowapi') + ), + self::makeLabel( + $this->_labelClass, + 'singleLogout', + _('Single Logout') + . '
(' + . _('signing out of FOG also signs out of this provider') + . ')' + ) => self::makeInput( + '', + 'singleLogout', + '', + 'checkbox', + 'singleLogout', + '', + false, + false, + -1, + -1, + $checked('singleLogout') + ), + // Read-only, and shown for the same reason the redirect URI is: + // a provider that follows the spec refuses a post-logout + // redirect it has not been told about, and then logout ends on + // the provider's error page instead of back at FOG. That looks + // like this plugin is broken, and the fix is a value an admin + // has to copy from somewhere. + self::makeLabel( + $this->_labelClass, + 'postLogoutUri', + _('Post-Logout Redirect URI') + . '
(' + . _('register this too, if you enable single logout') + . ')' + ) => self::makeInput( + 'form-control', + 'postLogoutUri', + '', + 'text', + 'postLogoutUri', + OIDC::postLogoutUri(), + false, + false, + -1, + -1, + '', + true ) ]; @@ -550,7 +598,11 @@ public function oidcGeneralPost() 'jitProvision', isset($_POST['jitProvision']) ? '1' : '0' ) - ->set('allowapi', isset($_POST['allowapi']) ? '1' : '0'); + ->set('allowapi', isset($_POST['allowapi']) ? '1' : '0') + ->set( + 'singleLogout', + isset($_POST['singleLogout']) ? '1' : '0' + ); // The secret is only written when the admin actually typed one. An // empty field and the placeholder both mean "unchanged"; without diff --git a/tests/oidc-single-logout.test.php b/tests/oidc-single-logout.test.php new file mode 100644 index 0000000..be131ac --- /dev/null +++ b/tests/oidc-single-logout.test.php @@ -0,0 +1,388 @@ +'opSingleLogout'")) { + ok('OIDC maps singleLogout to opSingleLogout'); +} else { + bad('OIDC no longer maps the singleLogout field'); +} + +/* + * BOTH halves. createSql() alone serves only servers installed after this + * commit; the ALTER alone serves only servers installed before it. Shipping + * one is the classic plugin-schema half-fix, and it looks complete in + * review because the install it was tested on happened to be the served + * kind. + */ +if (false !== strpos($mgr, "'opSingleLogout',")) { + ok('createSql() declares opSingleLogout (fresh installs)'); +} else { + bad('createSql() no longer declares opSingleLogout; a fresh install' + . ' would have no column for the setting'); +} +if (false !== strpos( + $mgr, + '"ALTERTABLE`OIDCProviders`ADDCOLUMN`opSingleLogout`"' +)) { + ok('a schema step adds opSingleLogout (existing installs)'); +} else { + bad('no ALTER TABLE step adds opSingleLogout; every install that' + . ' already passed step 0 keeps a table without the column'); +} + +/* + * Appended, not inserted. Plugin::installdb() SKIPS the first pSchema steps + * rather than replaying them, so putting a step anywhere but the end shifts + * every later step's index and silently skips one on installs that have + * already run. The LDAP plugin carries repair steps for exactly this. + */ +$alterAt = strpos($mgr, 'ALTERTABLE`OIDCProviders`ADDCOLUMN`opSingleLogout`'); +$grantAt = strpos($mgr, "getClass('OIDCUserGrantManager')->install()"); +if (false !== $alterAt && false !== $grantAt && $alterAt > $grantAt) { + ok('the step is appended after the existing ones'); +} else { + bad('the opSingleLogout step is not at the end of schema(); installdb()' + . ' skips by COUNT, so an inserted step silently skips a later one'); +} + +/* + * Default off, in both places a default can be stated. A column that + * defaults on turns an upgrade into a behaviour change for every install + * already using this plugin. + */ +if (false !== strpos($mgr, "ENUM('0','1')NOTNULLDEFAULT'0'\"")) { + ok("the ALTER defaults to '0'"); +} else { + bad('the ALTER does not default the column off; upgrading would enable' + . ' single logout for installs that never asked for it'); +} + +echo "\n2. the material is recorded at sign-in, not fetched at logout\n"; + +if (false !== strpos($flow, 'privatestaticfunction_rememberLogout(')) { + ok('_rememberLogout() is defined'); +} else { + bad('_rememberLogout() is gone'); +} + +$callbackBody = methodBody($flow, 'publicstaticfunctioncallback('); +if ('' === $callbackBody) { + bad('could not isolate callback()'); +} else { + if (false !== strpos($callbackBody, 'self::_rememberLogout(')) { + ok('callback() records the logout material'); + } else { + bad('callback() no longer records the logout material; single' + . ' logout is dead with nothing to show for it'); + } + /* + * Ordering, and it is the whole of property 2. callback() does + * `$_SESSION = []` immediately before establishSession(), to stop an + * identity already in the session deciding the new one. Anything stored + * before that point is wiped by it. + */ + $wipeAt = strpos($callbackBody, '$_SESSION=[];'); + $rememberAt = strpos($callbackBody, 'self::_rememberLogout('); + if (false !== $wipeAt && false !== $rememberAt && $rememberAt > $wipeAt) { + ok('it records AFTER the session wipe'); + } else { + bad('_rememberLogout() runs before callback() empties $_SESSION, so' + . ' everything it stores is thrown away'); + } +} + +$remember = methodBody($flow, 'privatestaticfunction_rememberLogout('); +if ('' === $remember) { + bad('could not isolate _rememberLogout()'); +} else { + /* + * Gated on the column, and gated with an early return rather than by + * wrapping the write -- either shape is fine, but the read has to be + * there. Without it every OIDC sign-in arms single logout. + */ + if (false !== strpos($remember, "get('singleLogout')")) { + ok('it is gated on the provider column'); + } else { + bad('_rememberLogout() no longer reads singleLogout; every OIDC' + . ' session would end its provider session, including on an' + . ' install sharing that provider with other applications'); + } + if (false !== strpos($remember, "stripos(\$endpoint,'https://')")) { + ok('the end_session_endpoint must be https'); + } else { + bad('_rememberLogout() no longer requires an https' + . ' end_session_endpoint; it comes from a fetched document and' + . ' ends up in a Location header carrying an ID token'); + } + if (false !== strpos($remember, "\$config['end_session_endpoint']")) { + ok('it reads the endpoint from the discovery document'); + } else { + bad('_rememberLogout() no longer reads end_session_endpoint'); + } + /* + * No network call in here. The reason the material is stored at all is + * that logout must not depend on the provider being reachable; a fetch + * that crept in here would be on the sign-in path instead, which is + * merely wrong rather than harmful -- but a fetch in logoutUrl() below + * is the actual failure, and both are worth refusing. + */ + foreach (['_getJson(', '_post(', '_http(', '_discover('] as $call) { + if (false !== strpos($remember, $call)) { + bad('_rememberLogout() calls ' . $call . '; everything it needs' + . ' is already in hand'); + } + } +} + +echo "\n3. the logout URL is built from what was stored, and nothing else\n"; + +$logoutUrl = methodBody($flow, 'publicstaticfunctionlogoutUrl('); +if ('' === $logoutUrl) { + bad('OIDCFlow::logoutUrl() is missing'); +} else { + foreach (['_getJson(', '_post(', '_http(', '_discover('] as $call) { + if (false !== strpos($logoutUrl, $call)) { + bad('logoutUrl() calls ' . $call . ' -- a network request on the' + . ' sign-out path means a provider that has gone away turns' + . ' Log out into a page that hangs and then fails'); + } + } + ok('logoutUrl() makes no network request'); + + if (false !== strpos($logoutUrl, 'id_token_hint')) { + ok('it sends id_token_hint'); + } else { + bad('logoutUrl() sends no id_token_hint; the provider cannot tell' + . ' which session to end and prompts instead'); + } + /* + * login.php, not index.php. This is property 5 and it is the one that + * only bites once #17 exists -- at which point index.php is exactly the + * page that redirects back to the provider. + */ + if (false !== strpos($logoutUrl, 'OIDC::postLogoutUri()')) { + ok('it returns the browser to the local login page'); + } else { + bad('logoutUrl() no longer uses OIDC::postLogoutUri(); returning to' + . ' index.php on a forced-redirect install signs the user' + . ' straight back in or loops'); + } + /* + * Single use. Left in place, a second pass through logout would build + * the redirect again from a session that is already gone. + */ + if (false !== strpos($logoutUrl, 'unset($_SESSION[self::LOGOUT_KEY]);')) { + ok('it clears what it read'); + } else { + bad('logoutUrl() leaves the stored material in the session'); + } + /* + * Re-read the provider. Turning the setting off has to mean from that + * moment, not from the next time everybody happens to sign in -- and a + * provider since deleted or disabled must not get a redirect built from + * a row that no longer says anything. + */ + if (false !== strpos($logoutUrl, "get('singleLogout')") + && false !== strpos($logoutUrl, "get('enabled')") + ) { + ok('it re-checks the provider row rather than trusting the session'); + } else { + bad('logoutUrl() trusts the session copy of the setting; turning' + . ' single logout off would not take effect until every user' + . ' had signed in again'); + } +} + +if (false !== strpos($model, 'publicstaticfunctionpostLogoutUri()')) { + ok('OIDC::postLogoutUri() is defined'); +} else { + bad('OIDC::postLogoutUri() is missing'); +} +$postLogout = methodBody($model, 'publicstaticfunctionpostLogoutUri()'); +if (false !== strpos($postLogout, "'management/login.php'")) { + ok('postLogoutUri() names management/login.php'); +} else { + bad('postLogoutUri() no longer points at management/login.php, the one' + . ' page a forced-redirect install cannot bounce to the provider'); +} + +/* + * The callback URI must not move. It is registered at every provider by + * hand and compared byte for byte, so a refactor that changes its output -- + * even by a slash -- breaks every existing install's sign-in with a + * provider-side error. + */ +$redirectUri = methodBody($model, 'publicstaticfunctionredirectUri()'); +if (false !== strpos($redirectUri, 'self::CALLBACK_PATH')) { + ok('redirectUri() still builds from CALLBACK_PATH'); +} else { + bad('redirectUri() no longer uses CALLBACK_PATH; this value is' + . ' registered at providers by hand and compared byte for byte'); +} + +echo "\n4. the hook wires it to core's seam and to nothing else\n"; + +if (false !== strpos($hook, "['USER_LOGGING_OUT','providerLogout']")) { + ok('OIDCLogout registers on USER_LOGGING_OUT'); +} else { + bad('OIDCLogout no longer registers on USER_LOGGING_OUT; nothing calls' + . ' logoutUrl() and Log out goes back to being FOG-only'); +} +if (false !== strpos($hook, 'registerInstalled(')) { + ok('it registers through registerInstalled()'); +} else { + bad('OIDCLogout registers unconditionally; a plugin that is present but' + . ' not installed must not touch logout'); +} +$listener = methodBody($hook, 'publicfunctionproviderLogout('); +if ('' === $listener) { + bad('OIDCLogout::providerLogout() is missing'); +} else { + /* + * The empty case must return without writing. Assigning '' into + * $arguments['redirect'] would be harmless today only because core + * checks for an empty string -- but it makes this listener able to + * overwrite a redirect another listener set, which is a rule about + * hooks and not about this plugin. + */ + $guardAt = strpos($listener, "if(''===\$url){return;}"); + $writeAt = strpos($listener, "\$arguments['redirect']="); + if (false !== $guardAt && false !== $writeAt && $guardAt < $writeAt) { + ok('it writes the redirect only when there is one'); + } else { + bad('OIDCLogout::providerLogout() writes to $arguments even with no' + . ' logout URL, so it can clobber another listener'); + } +} + +echo "\n"; +if ($fail > 0) { + echo "FAIL: $fail problem(s), $pass ok\n"; + exit(1); +} +echo "ok: $pass checks passed -- signing out of FOG can end the provider" + . " session, and only when asked\n"; +exit(0);