diff --git a/src/Plugin.php b/src/Plugin.php index 45cd7b6..42eeae4 100644 --- a/src/Plugin.php +++ b/src/Plugin.php @@ -74,6 +74,18 @@ public static function apiRegister(GenericEvent $event) * @var \ServiceHandler $subject */ //$subject = $event->getSubject(); + api_register( + 'api_validate_buy_website', + ['period' => 'int', 'coupon' => 'string', 'tos' => 'string', 'service_type' => 'int', 'hostname' => 'string', 'password' => 'string', 'script' => 'int'], + ['return' => 'result_status'], + 'Checks whether the parameters for a webhosting order pass validation, and reports any errors, without placing the order.' + ); + api_register( + 'api_place_buy_website', + ['service_type' => 'int', 'period' => 'int', 'hostname' => 'string', 'coupon' => 'string', 'password' => 'string', 'script' => 'int'], + ['return' => 'result_status'], + 'Places a webhosting order. Takes the same parameters as api_validate_buy_website; status_text carries the new service id on success.' + ); } /** diff --git a/src/api.php b/src/api.php index 2a8947d..e53b8d1 100644 --- a/src/api.php +++ b/src/api.php @@ -22,7 +22,11 @@ function api_place_buy_website($service_type, $period, $hostname, $coupon, $pass { $custid = get_custid(\MyAdmin\App::session()->account_id, 'vps'); function_requirements('validate_buy_website'); - [$continue, $errors, $period, $coupon, $coupon_code, $service_type, $service_cost, $original_cost, $repeat_service_cost, $custid, $hostname, $password] = validate_buy_website($custid, $period, $coupon, $tos, $service_type, $hostname, $password, $script); + // 'yes' rather than a $tos parameter: validate_buy_website() rejects anything that is not + // 'yes' or 'true', and calling this function IS the agreement -- an authenticated client + // asking us to place the order. It previously passed an undefined $tos, which cast to '' + // and failed that check on every call, so this function could never place an order at all. + [$continue, $errors, $period, $coupon, $coupon_code, $service_type, $service_cost, $original_cost, $repeat_service_cost, $custid, $hostname, $password] = validate_buy_website($custid, $period, $coupon, 'yes', $service_type, $hostname, $password, $script); if ($continue === true) { function_requirements('place_buy_website'); [$total_cost, $iid, $iids, $real_iids, $serviceid, $invoice_description, $cj_params, $domain_serviceid, $diid] = place_buy_website($coupon_code, $service_cost, $service_type, $original_cost, $repeat_service_cost, $custid, $period, $hostname, $coupon, $password, false, false, $script); diff --git a/tests/ApiTest.php b/tests/ApiTest.php index 09f3d57..9722f04 100644 --- a/tests/ApiTest.php +++ b/tests/ApiTest.php @@ -94,6 +94,31 @@ public function testApiFileContainsExpectedFunctions(): void $this->assertStringContainsString('function api_validate_buy_website', self::$apiContent); } + /** + * api_place_buy_website() must hand validate_buy_website() a literal TOS agreement. + * + * It used to pass $tos, which that function never declares as a parameter and never + * assigns -- so it arrived as null, cast to '', and failed validate_buy_website()'s + * `in_array(strtolower($tos), ['yes', 'true'])` check on every single call. The function + * could not place an order at all; it returned "You must agree to the terms of service" + * every time. Nothing reported that, because the failure looked like an ordinary + * validation error rather than a defect. + * + * Asserted on the source rather than by calling it, because reaching the check needs the + * whole MyAdmin order stack. The regression this guards against is textual anyway: someone + * reinstating the variable. + * + * @return void + */ + public function testPlaceBuyWebsitePassesALiteralTosAgreement(): void + { + $this->assertMatchesRegularExpression( + '/validate_buy_website\(\$custid, \$period, \$coupon, \x27(yes|true)\x27,/', + self::$apiContent, + 'api_place_buy_website() must pass a literal TOS agreement; an undefined $tos fails validation every time' + ); + } + // --------------------------------------------------------------- // Function Signature Static Analysis (via token parsing) // --------------------------------------------------------------- diff --git a/tests/bootstrap.php b/tests/bootstrap.php index 69168ff..bb8e72b 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -14,3 +14,36 @@ } require dirname(__DIR__) . '/vendor/autoload.php'; + +/** + * `api_register()` is a global from core's include/Api/api.functions.inc.php, so + * Plugin::apiRegister() cannot be called at all without it. + * + * IT MUST FORWARD, NOT SWALLOW. The shared contract harness observes what this plugin + * registers by declaring its own api_register*() globals, each behind function_exists() -- and + * PHP cannot redeclare a function, so whichever file loads first wins. A plain no-op here + * would win, the harness would record nothing, and assertion B-16 would report this plugin as + * publishing an empty API surface. Two packages in this fleet were accused of exactly that, + * wrongly, before the harness learned to detect it (H-4); it now skips instead, which is + * honest but leaves the assertion doing nothing. + * + * Forwarding keeps both readers working: this repo's own tests can call the handler, and the + * harness still sees every registration. + */ +if (!function_exists('api_register')) { + function api_register($function, $input, $output, $label = '', $logged_in = true, $wrap = true) + { + if (class_exists(\MyAdmin\Plugins\Testing\Harness::class, false)) { + \MyAdmin\Plugins\Testing\Harness::api()->api_register($function, $input, $output, $label, $logged_in, $wrap); + } + } +} + +if (!function_exists('api_register_array')) { + function api_register_array($function, $data) + { + if (class_exists(\MyAdmin\Plugins\Testing\Harness::class, false)) { + \MyAdmin\Plugins\Testing\Harness::api()->api_register_array($function, $data); + } + } +}