Read query parameters through Route::queryParam()

oidc/class/oidcflow.class.php

@@ -81,14 +81,28 @@ class OIDCFlow extends FOGBase
     /**
      * Send the browser to the provider.
      *
+     * Every query parameter here and in callback() is read through
+     * Route::queryParam(), never filter_input(INPUT_GET). A route under
+     * /ext/ is reached by an internal rewrite to api/index.php, and on
+     * nginx that rewrite used to hand the router an EMPTY query string --
+     * so ?provider=3 arrived as nothing and a configured, enabled provider
+     * was refused as "Unknown identity provider". fogproject#1163 gives the
+     * installer's vhost $is_args$args, but a server that has not re-run the
+     * installer keeps the old one, and queryParam() is what recovers the
+     * value from REQUEST_URI on those. Apache carries QSA and never had the
+     * problem; this code cannot tell which it is running under.
+     *
      * @return void
      */
     public static function start()
     {
         self::_session();
         try {
             $provider = self::_enabledProvider(
-                (int)filter_input(INPUT_GET, 'provider', FILTER_VALIDATE_INT)
+                (int)filter_var(
+                    (string)Route::queryParam('provider'),
+                    FILTER_VALIDATE_INT
+                )
             );
             $config = self::_discover($provider);
 
@@ -160,7 +174,7 @@ public static function callback()
                     _('The sign-in took too long; please start again')
                 );
             }
-            $error = trim((string)filter_input(INPUT_GET, 'error'));
+            $error = trim((string)Route::queryParam('error'));
             if ('' !== $error) {
                 // The provider's own words, which are the useful ones --
                 // 'access_denied' means somebody pressed cancel.
@@ -171,13 +185,13 @@ public static function callback()
                     )
                 );
             }
-            $state = (string)filter_input(INPUT_GET, 'state');
+            $state = (string)Route::queryParam('state');
             if (!hash_equals((string)$flow['state'], $state)) {
                 // Constant time, and the message says nothing about which
                 // half was wrong.
                 throw new \Exception(_('The sign-in could not be verified'));
             }
-            $code = (string)filter_input(INPUT_GET, 'code');
+            $code = (string)Route::queryParam('code');
             if ('' === $code) {
                 throw new \Exception(_('The identity provider sent no code'));
             }

tests/oidc-flow-safety.test.php

@@ -644,6 +644,49 @@ function methodBody($src, $method)
     }
 }
 
+/*
+ * N. Query parameters come from Route::queryParam(), never filter_input().
+ *
+ * Both entry points are reached by an internal rewrite to api/index.php.
+ * On nginx that rewrite handed the router an EMPTY query string, so
+ * filter_input(INPUT_GET, 'provider') returned null and start() refused a
+ * configured, enabled provider as "Unknown identity provider"; callback()
+ * would have lost state, code and error the same way. fogproject#1163
+ * fixes the vhost, but only for a server that re-runs the installer, and
+ * Route::queryParam() is what recovers the value from REQUEST_URI on every
+ * server that does not.
+ */
+$flowCode = '';
+foreach (token_get_all($flowSrc) as $tok) {
+    // Comments stripped first: this file's own docblock names the wrong
+    // call so a reader knows what not to write, and a gate that reads its
+    // own documentation as a violation is a gate nobody can document.
+    if (is_array($tok)
+        && ($tok[0] === T_COMMENT || $tok[0] === T_DOC_COMMENT)
+    ) {
+        continue;
+    }
+    $flowCode .= is_array($tok) ? $tok[1] : $tok;
+}
+if (false !== strpos($flowCode, 'filter_input(INPUT_GET')) {
+    fail(
+        'OIDCFlow reads a query parameter with filter_input(INPUT_GET, ...),'
+        . ' which is empty on a routed request behind an nginx vhost that'
+        . ' predates fogproject#1163 -- use Route::queryParam()'
+    );
+}
+foreach (['provider', 'error', 'state', 'code'] as $param) {
+    if (false === strpos($flowCode, "Route::queryParam('" . $param . "')")) {
+        fail(
+            sprintf(
+                'OIDCFlow no longer reads the %s query parameter through'
+                . ' Route::queryParam()',
+                $param
+            )
+        );
+    }
+}
+
 if (count($fails) > 0) {
     fwrite(STDERR, 'FAIL: ' . count($fails) . " problem(s):\n");
     foreach ($fails as $f) {