OIDC: after a successful single logout, land on the login page that redirects

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');
     }

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;

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'),
-                    '<code>' . Initiator::e(OIDC::postLogoutUri()) . '</code>'
+                    '<code>' . Initiator::e(OIDC::localLoginUrl()) . '</code>'
                 )
                 . ')'
             ) => self::makeInput(

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'

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