Remove allow_request_if in favor of allow_if.

And document allowance conditions.

devel/manual/components.md

@@ -105,6 +105,8 @@ returned. The merge process works as follows.
     still be used to check selected conference properties, as in
     `"allow_if":"conf.external_login"`.
 
+    `allow_if` syntax is described below.
+
 4. The highest-priority remaining component, if any, is the result of the
    merge process.
 
@@ -137,3 +139,29 @@ the **group** defined by everything up to the last slash; for instance,
 components `a/b`, `a/c`, and `a/b/c` are members of groups `a`, `a`, and
 `a/b`, respectively. If present, the component’s `group` property overrides
 the group value derived from the component name.
+
+## Allowance conditions
+
+A component `allow_if` property should be `true`, `false`, or a string
+describing a condition. Examples include:
+
+* `admin`: The viewer is a system administrator
+* `manager`: The viewer is a manager (system administrator, track
+  administrator, or paper administrator)
+* `pc`: The viewer is on the PC or a system administrator
+* `author`: The viewer is an author
+* `reviewer`: The viewer is on the PC, a system administrator, or an assigned
+  reviewer
+* `view_review`: The viewer may be allowed to see a review
+* `empty`: The viewer is empty (has not signed in and has not provided any
+  review tokens or other capabilities)
+* `email`: The viewer has signed in
+* `disabled`: The viewer is disabled
+* `opt.XXX`: Conference option `$Opt[XXX]` is truthy
+* `setting.XXX`: Conference setting `XXX` is truthy
+
+Some component types define additional conditions.
+
+It is also possible to combine multiple conditions using parentheses, `!`
+(negation), `&&`, and `||`. For instance, the condition `author || reviewer`
+is true for both authors and reviewers.

devel/manual/pages.md

@@ -49,10 +49,9 @@ form, HotCRP calls the named function statically. In the second form (with
 `Contact, Qrequest, ComponentSet`) and then calls the relevant `FUNCTIONNAME`
 on that object. All calls with the same `*CLASSNAME` will use the same object.
 
-A component’s `request_function` may be blocked by the `allow_request_if` as
-well as the `allow_if` property. For instance, `"allow_request_if":
-"req.clearbug"` allows the `request_function` only if the request has a
-`clearbug` parameter.
+A component’s `request_function` may be blocked by the `allow_if` property.
+For instance, `"allow_if": "req.clearbug"` allows the `request_function` only
+if the request has a `clearbug` parameter.
 
 If a `request_function` explicitly returns `false` (as opposed to `null`, or
 by simply falling off the end), then HotCRP quits calling `request_function`s
@@ -93,5 +92,19 @@ same as an with the corresponding properties:
 }
 ```
 
+## Allowance conditions
+
+In addition to the global `allow_if` conditions, such as `"admin"`, page
+components support conditions relating to the current request.
+
+* `post`: The current request is a POST with a valid CSRF token
+* `anypost`: The current request is a POST, whether or not it has a valid CSRF
+  token
+* `getpost`: The current request is a GET, POST, or HEAD with a valid CSRF
+  token
+* `get`, `head`: The current request has the specified method
+* `req.XXX`: The current request defines the `XXX` parameter to some value,
+  possibly the empty string
+
 
 [components]: ./components.md

etc/listactions.json

@@ -98,15 +98,15 @@
         "name": "get/revau", "get": true,
         "title": "Reviews/Reviews (text, author view)",
         "order": 30200,
-        "allow_if": ["view_review", "admin"],
+        "allow_if": "view_review && admin",
         "function": "+GetReviews_ListAction",
         "zip": false, "abstract": false, "author_view": true
     },
     {
         "name": "get/revauz", "get": true,
         "title": "Reviews/Reviews (zip, author view)",
         "order": 30205,
-        "allow_if": ["view_review", "admin"],
+        "allow_if": "view_review && admin",
         "function": "+GetReviews_ListAction",
         "zip": true, "abstract": false, "author_view": true
     },
@@ -121,7 +121,7 @@
         "name": "get/revaucsv", "get": true,
         "title": "Reviews/Reviews (CSV, author view)",
         "order": 30210,
-        "allow_if": ["view_review", "admin"],
+        "allow_if": "view_review && admin",
         "function": "+GetReviewCSV_ListAction",
         "author_view": true
     },
@@ -134,20 +134,20 @@
     },
     {
         "name": "get/rank", "get": true,
-        "allow_if": ["setting.tag_rank", "reviewer"], "display_if": false,
+        "allow_if": "setting.tag_rank && reviewer", "display_if": false,
         "function": "+GetRank_ListAction"
     },
     {
         "name": "get/lead", "get": true,
         "title": "Reviews/Discussion leads (CSV)", "order": 30600,
-        "allow_if": "pc", "display_if": "lead",
+        "allow_if": "pc", "display_if": "conf.has_any_lead_or_shepherd",
         "function": "+GetLead_ListAction",
         "type": "lead"
     },
     {
         "name": "get/shepherd", "get": true,
         "title": "Reviews/Shepherds (CSV)", "order": 30650,
-        "allow_if": "pc", "display_if": "shepherd",
+        "allow_if": "pc", "display_if": "conf.has_any_lead_or_shepherd",
         "function": "+GetLead_ListAction",
         "type": "shepherd"
     },

etc/pages.json

@@ -30,14 +30,16 @@
         "request_function": "Home_Page::profile_redirect_request"
     },
     {
-        "name": "home/admin", "order": 900, "allow_if": "chair",
-        "allow_request_if": ["getpost", "req.clearbug || req.clearnewpcrev"],
-        "request_function": "AdminHome_Page::check_admin",
+        "name": "home/admin_request", "order": 900,
+        "allow_if": "chair && getpost && (req.clearbug || req.clearnewpcrev)",
+        "request_function": "AdminHome_Page::check_admin"
+    },
+    {
+        "name": "home/admin", "order": 901, "allow_if": "chair",
         "print_function": "AdminHome_Page::print"
     },
     {
-        "name": "home/reviewtokenreport",
-        "allow_request_if": "req.reviewtokenreport",
+        "name": "home/reviewtokenreport", "allow_if": "req.reviewtokenreport",
         "request_function": "Home_Page::reviewtokenreport_request"
     },
 
@@ -118,8 +120,7 @@
 
     {   "name": "newaccount", "allow_disabled": true   },
     {
-        "name": "newaccount/request", "order": 100,
-        "allow_request_if": "anypost",
+        "name": "newaccount/request", "order": 100, "allow_if": "anypost",
         "request_function": "*Signin_Page::create_request"
     },
     [   "newaccount/head", 1000, "Signin_Page::print_newaccount_head"   ],
@@ -132,8 +133,7 @@
 
     {   "name": "signin", "allow_disabled": true   },
     {
-        "name": "signin/request", "order": 100,
-        "allow_request_if": "anypost",
+        "name": "signin/request", "order": 100, "allow_if": "anypost",
         "request_function": "*Signin_Page::signin_request"
     },
     {
@@ -157,17 +157,15 @@
 
     {   "name": "signout", "allow_disabled": true   },
     {
-        "name": "signout/request", "order": 100,
-        "allow_request_if": "anypost",
+        "name": "signout/request", "order": 100, "allow_if": "anypost",
         "request_function": "Signin_Page::signout_request"
     },
     [   "signout/content", 1000, "Signin_Page::print_signout"   ],
 
 
     {   "name": "forgotpassword", "allow_disabled": true   },
     {
-        "name": "forgotpassword/request", "order": 100,
-        "allow_request_if": "anypost",
+        "name": "forgotpassword/request", "order": 100, "allow_if": "anypost",
         "request_function": "*Signin_Page::forgot_request"
     },
     [   "forgotpassword/head", 1000, "Signin_Page::print_forgot_head"   ],

index.php

@@ -9,29 +9,16 @@
  * @param object $pagej
  * @param ComponentSet $pc */
 function handle_request_components($user, $qreq, $pagej, $pc) {
-    $pc->add_xt_checker([$qreq, "xt_allow"]);
-    $reqgj = [];
-    $nfound = 0;
-    $not_allowed = false;
-    if (isset($pagej->request_function)) {
-        ++$nfound;
-        if ($pc->allowed($gj->allow_request_if ?? null, $gj)) {
-            $reqgj[] = $gj;
-        }
+    if (isset($pagej->request_function)
+        && $pc->call_function($pagej, $pagej->request_function, $pagej) === false) {
+        return;
     }
     foreach ($pc->members($pagej->group, "request_function") as $gj) {
-        ++$nfound;
-        if ($pc->allowed($gj->allow_request_if ?? null, $gj)) {
-            $reqgj[] = $gj;
+        if (isset($gj->allow_request_if)) { /* XXX backward compat */
+            error_log("Warning: allow_request_if is deprecated");
+            if (!$pc->allowed($gj->allow_request_if, $gj))
+                continue;
         }
-    }
-    if ($nfound > 0
-        && $nfound < count($reqgj)
-        && $qreq->is_post()
-        && !$qreq->valid_token()) {
-        $user->conf->error_msg($user->conf->_i("badpost"));
-    }
-    foreach ($reqgj as $gj) {
         if ($pc->call_function($gj, $gj->request_function, $gj) === false) {
             break;
         }

lib/qrequest.php

@@ -508,19 +508,17 @@ function post_empty() {
      * @return ?bool */
     function xt_allow($e) {
         if ($e === "post") {
-            return $this->_method === "POST" && $this->_post_ok;
+            return $this->_post_ok && $this->_method === "POST";
         } else if ($e === "anypost") {
             return $this->_method === "POST";
         } else if ($e === "getpost") {
             return in_array($this->_method, ["POST", "GET", "HEAD"]) && $this->_post_ok;
+        } else if ($e === "get") {
+            return $this->_method === "GET";
+        } else if ($e === "head") {
+            return $this->_method === "HEAD";
         } else if (str_starts_with($e, "req.")) {
-            foreach (explode(" ", $e) as $w) {
-                if (str_starts_with($w, "req.")
-                    && $this->has(substr($w, 4))) {
-                    return true;
-                }
-            }
-            return false;
+            return $this->has(substr($w, 4));
         } else {
             return null;
         }

src/conference.php

@@ -5600,6 +5600,7 @@ function page_components(Contact $viewer, Qrequest $qreq) {
             || $pc->arg(1) !== $qreq) {
             $pc = new ComponentSet($viewer, ["etc/pages.json"], $this->opt("pages"));
             $pc->set_context_args($viewer, $qreq, $pc);
+            $pc->add_xt_checker([$qreq, "xt_allow"]);
             $this->_page_components = $pc;
         }
         return $pc;

src/userstatus.php

@@ -1761,8 +1761,9 @@ function print_start_section($title, $hashid = null) {
     function request_group($name) {
         $cs = $this->cs();
         foreach ($cs->members($name, "request_function") as $gj) {
-            if ($cs->allowed($gj->allow_request_if ?? null, $gj)) {
-                $cs->call_function($gj, $gj->request_function, $gj);
+            assert(!isset($gj->allow_request_if));
+            if ($cs->call_function($gj, $gj->request_function, $gj) === false) {
+                break;
             }
         }
     }

src/xtparams.php

@@ -63,8 +63,6 @@ function check_string($s, $xt) {
             return !$user || $user->is_reviewer();
         } else if ($s === "view_review") {
             return !$user || $user->can_view_some_review();
-        } else if ($s === "lead" || $s === "shepherd") {
-            return $this->conf->has_any_lead_or_shepherd();
         } else if ($s === "!empty") {
             return !$user || !$user->is_empty();
         } else if ($s === "empty") {