diff --git a/oidc/class/oidc.class.php b/oidc/class/oidc.class.php
index 411a423..00572c3 100644
--- a/oidc/class/oidc.class.php
+++ b/oidc/class/oidc.class.php
@@ -284,13 +284,19 @@ public static function redirectUri()
/**
* 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).
+ * FOG's ordinary login page, which is the whole point of having ended
+ * the provider session: with forced redirect on (#17) that page sends
+ * the browser back to the provider, the provider now has no session,
+ * and it asks who you are. Signing out and then signing in as somebody
+ * else is one continuous journey rather than a dead end.
+ *
+ * There is no loop here BECAUSE single logout ran. That is the
+ * distinction against the two places that do point at login.php:
+ *
+ * OIDCFlow::_fail() -- the provider refused, so
+ * bouncing back to it is a loop
+ * OIDCLogout, single logout off -- the SSO session is still alive,
+ * so index.php signs you back in
*
* This URL has to be registered at the provider as a post-logout
* redirect URI, the same way the callback does. Providers that follow
@@ -302,6 +308,25 @@ public static function redirectUri()
* @return string
*/
public static function postLogoutUri()
+ {
+ return self::absoluteUrl('management/index.php');
+ }
+ /**
+ * The login page no provider setting can redirect away from.
+ *
+ * fogproject#1175. Core offers the LOGIN_PAGE_REDIRECT hook only when
+ * FOG_LOCAL_LOGIN is undefined, and management/login.php defines it --
+ * so on that URL this plugin is never asked where to send anybody, and
+ * a provider that is unreachable or misconfigured cannot take the page
+ * down with it.
+ *
+ * Used for the two landings that must NOT bounce back to a provider,
+ * and printed beside the forced-redirect setting so the admin turning
+ * it on is told the way back before they need it.
+ *
+ * @return string
+ */
+ public static function localLoginUrl()
{
return self::absoluteUrl('management/login.php');
}
diff --git a/oidc/hooks/oidclogout.hook.php b/oidc/hooks/oidclogout.hook.php
index 93336ce..a7359ab 100644
--- a/oidc/hooks/oidclogout.hook.php
+++ b/oidc/hooks/oidclogout.hook.php
@@ -97,8 +97,13 @@ public function providerLogout($arguments)
* It does not end the provider session -- only single logout
* does -- but it leaves somebody looking at a form instead of
* back where they started.
+ *
+ * Deliberately NOT postLogoutUri(), which is the ordinary login
+ * page: that is the right landing when single logout HAS run,
+ * because the provider will then ask who you are instead of
+ * waving you through. Here it has not.
*/
- $url = OIDC::postLogoutUri();
+ $url = OIDC::localLoginUrl();
}
if ('' === $url) {
return;
diff --git a/oidc/pages/oidcmanagement.page.php b/oidc/pages/oidcmanagement.page.php
index 33a7a95..c197836 100644
--- a/oidc/pages/oidcmanagement.page.php
+++ b/oidc/pages/oidcmanagement.page.php
@@ -511,7 +511,7 @@ public function oidcGeneral()
// locked out of their own server -- and the URL is not
// something they could guess at that point.
_('the local login form stays available at %s'),
- '' . Initiator::e(OIDC::postLogoutUri()) . ''
+ '' . Initiator::e(OIDC::localLoginUrl()) . ''
)
. ')'
) => self::makeInput(
diff --git a/tests/oidc-auto-redirect.test.php b/tests/oidc-auto-redirect.test.php
index 75625f9..b54c5d6 100644
--- a/tests/oidc-auto-redirect.test.php
+++ b/tests/oidc-auto-redirect.test.php
@@ -308,7 +308,7 @@ function methodBody($s, $needle)
bad('OIDCLogout::providerLogout() is missing');
} else {
if (false !== strpos($logoutListener, 'OIDCFlow::forcedProvider()')
- && false !== strpos($logoutListener, 'OIDC::postLogoutUri()')
+ && false !== strpos($logoutListener, 'OIDC::localLoginUrl()')
) {
ok('logging out of a forced-redirect install lands on the form');
} else {
@@ -355,7 +355,7 @@ function methodBody($s, $needle)
: substr($page, $labelAt, $inputAt - $labelAt);
if ('' === $label) {
bad('could not find the autoRedirect field on the management page');
-} elseif (false !== strpos($label, 'OIDC::postLogoutUri()')) {
+} elseif (false !== strpos($label, 'OIDC::localLoginUrl()')) {
ok('the management page prints the local login URL beside the setting');
} else {
bad('the autoRedirect label no longer names the local login URL; the'
diff --git a/tests/oidc-single-logout.test.php b/tests/oidc-single-logout.test.php
index be131ac..2b33c59 100644
--- a/tests/oidc-single-logout.test.php
+++ b/tests/oidc-single-logout.test.php
@@ -279,16 +279,20 @@ function methodBody($s, $needle)
. ' 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.
+ * The key as well as the value. Asserting only that postLogoutUri() is
+ * mentioned passes when it is bound to some other parameter name, and
+ * the provider then ends its session and leaves the person on its own
+ * page -- a logout that worked and looks broken.
*/
- if (false !== strpos($logoutUrl, 'OIDC::postLogoutUri()')) {
- ok('it returns the browser to the local login page');
+ if (false !== strpos(
+ $logoutUrl,
+ "'post_logout_redirect_uri'=>OIDC::postLogoutUri(),"
+ )) {
+ ok('it returns the browser to FOG\'s 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');
+ bad('logoutUrl() no longer sends a post_logout_redirect_uri built'
+ . ' by OIDC::postLogoutUri(); the person ends the journey on'
+ . ' the provider\'s own page with no way back to FOG');
}
/*
* Single use. Left in place, a second pass through logout would build
@@ -316,18 +320,46 @@ function methodBody($s, $needle)
}
}
-if (false !== strpos($model, 'publicstaticfunctionpostLogoutUri()')) {
- ok('OIDC::postLogoutUri() is defined');
-} else {
+/*
+ * The two landings are different pages and the difference is the whole
+ * point, so pin both.
+ *
+ * postLogoutUri() -> the ORDINARY login page. Single logout has just run,
+ * so a forced-redirect install sending the browser back to the provider is
+ * correct: the provider now has no session and asks who you are, which is
+ * how you sign out and back in as somebody else. Pointing this at
+ * login.php instead strands the person on the break-glass page after a
+ * perfectly successful logout.
+ *
+ * localLoginUrl() -> the page no provider setting can redirect away from,
+ * used by _fail() and by the single-logout-off fallback, where bouncing to
+ * the provider really would loop or silently sign the person back in.
+ */
+$postLogout = methodBody($model, 'publicstaticfunctionpostLogoutUri()');
+if ('' === $postLogout) {
bad('OIDC::postLogoutUri() is missing');
+} elseif (false !== strpos($postLogout, "'management/index.php'")) {
+ ok('postLogoutUri() names the ordinary login page');
+} else {
+ bad('postLogoutUri() no longer points at management/index.php; after a'
+ . ' successful single logout the person is stranded on whatever it'
+ . ' does name instead of being able to sign in again');
}
-$postLogout = methodBody($model, 'publicstaticfunctionpostLogoutUri()');
-if (false !== strpos($postLogout, "'management/login.php'")) {
- ok('postLogoutUri() names management/login.php');
+
+$localLogin = methodBody($model, 'publicstaticfunctionlocalLoginUrl()');
+if ('' === $localLogin) {
+ bad('OIDC::localLoginUrl() is missing');
+} elseif (false !== strpos($localLogin, "'management/login.php'")) {
+ ok('localLoginUrl() names management/login.php');
} else {
- bad('postLogoutUri() no longer points at management/login.php, the one'
+ bad('localLoginUrl() no longer points at management/login.php, the one'
. ' page a forced-redirect install cannot bounce to the provider');
}
+if ($postLogout === $localLogin && '' !== $postLogout) {
+ bad('postLogoutUri() and localLoginUrl() return the same thing; they'
+ . ' answer different questions and collapsing them re-creates'
+ . ' whichever bug the other one was avoiding');
+}
/*
* The callback URI must not move. It is registered at every provider by