diff --git a/CHANGELOG.md b/CHANGELOG.md index 46097985..e07b5cec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,36 @@ All notable changes to **Tiger Core** (`webtigers/tiger-core`). Format follows ## [Unreleased] +## [1.8.1] — 2026-09-15 + +Findings from an AI-driven install test of the web installer on a shared cPanel host (TIGER-138). + +### Security + +- **Comment moderation was open to anyone once comments were enabled.** `Comment_Service_Comment` + is granted to guests (anyone may post), and its admin-only methods gated on `_isAdmin()` — which + asks "is this role allowed on this service", true for a guest there. `moderate`, `datatable` and + deleting/editing someone else's comment now use the new `Tiger_Service_Service::_isAtLeastAdmin()` + (admin, or a role that inherits admin). The suite's own tests had been moderating as a plain + `user` and passing. Every other service that uses bare `_isAdmin()` is admin/superadmin-granted, + where it means what it says. +- **`/mcp` cookie sessions are honoured only from the site's own origin.** A cross-site POST riding + the admin's cookie (Sec-Fetch-Site not same-origin/none, or a foreign Origin) is 403; a + non-JSON Content-Type is 415 (closes the text/plain-form CSRF shape). Bearer clients are unaffected. + +### Fixed + +- **Bearer tokens never reached `/mcp` or `/api` on PHP-FPM hosts.** Apache drops the Authorization + header for CGI/FastCGI/FPM unless told otherwise, so a valid token degraded silently to the guest + surface (the headless-agent story was dead on every shared host). The skeleton's `public/.htaccess` + (1.0.21) now passes it through (`CGIPassAuth On`, with a rewrite-env fallback); Tiger reads the + header from `HTTP_AUTHORIZATION`, `REDIRECT_HTTP_AUTHORIZATION` and `apache_request_headers()` + (`Tiger_Ajax_ServiceFactory::authorizationHeader()`, one reader for both endpoints). +- **A Bearer that does not verify is 401** on `/mcp` (`WWW-Authenticate: Bearer`), never a silent + downgrade to guest — a client with a bad key learns it is bad. +- Comment admin methods answer "not allowed" BEFORE the feature flag, so an outsider is never told + the feature is off instead. + ## [1.8.0] — 2026-09-14 ### Added diff --git a/library/Tiger/Ajax/ServiceFactory.php b/library/Tiger/Ajax/ServiceFactory.php index bbe808f0..f81b21f0 100644 --- a/library/Tiger/Ajax/ServiceFactory.php +++ b/library/Tiger/Ajax/ServiceFactory.php @@ -290,10 +290,31 @@ protected function _identity() /** The Bearer token from the Authorization header, or null. */ protected function _bearerToken() { - $h = (string) $this->_request->getHeader('Authorization'); + $h = self::authorizationHeader($this->_request); return preg_match('/^\s*Bearer\s+(\S+)/i', $h, $m) ? $m[1] : null; } + /** + * The request's Authorization header, wherever the server put it. Apache drops it for CGI / + * FastCGI / PHP-FPM unless told otherwise (`CGIPassAuth On`, in public/.htaccess since the + * skeleton's 1.0.21); the rewrite-env fallback there surfaces it as REDIRECT_HTTP_AUTHORIZATION; + * some SAPIs only expose it through apache_request_headers(). One reader for /api and /mcp. + * + * @param Zend_Controller_Request_Abstract|null $request + * @return string '' when absent + */ + public static function authorizationHeader($request = null) + { + $h = ''; + if ($request !== null && method_exists($request, 'getHeader')) { $h = (string) $request->getHeader('Authorization'); } + if ($h === '') { $h = (string) ($_SERVER['HTTP_AUTHORIZATION'] ?? ''); } + if ($h === '') { $h = (string) ($_SERVER['REDIRECT_HTTP_AUTHORIZATION'] ?? ''); } + if ($h === '' && function_exists('apache_request_headers')) { + foreach ((array) apache_request_headers() as $k => $v) { if (strcasecmp((string) $k, 'Authorization') === 0) { $h = (string) $v; break; } } + } + return $h; + } + /** * Deny-by-default ACL pre-auth, shared by both modes. When no ACL is loaded * (only before the ACL engine exists) we fail-OPEN rather than hard-deny every diff --git a/library/Tiger/Service/Service.php b/library/Tiger/Service/Service.php index 3a99a6a4..4355242e 100644 --- a/library/Tiger/Service/Service.php +++ b/library/Tiger/Service/Service.php @@ -231,6 +231,29 @@ protected function _isAdmin($resource = null, $privilege = null) return $acl->isAllowed($role, $resource, $privilege); } + /** + * "Is the caller an admin (or a role that inherits admin)?" — the question _isAdmin() does NOT + * answer. _isAdmin() asks whether the role is ALLOWED ON THIS SERVICE; on a service granted to + * guests (comments, search, signup) every caller is. A method that is admin-only inside a + * public service must ask THIS. + * + * @return bool + */ + protected function _isAtLeastAdmin() + { + $identity = Zend_Auth::getInstance()->getIdentity(); + $role = (string) ($identity->role ?? ''); + if ($role === '') { return false; } + if ($role === 'admin') { return true; } + if (!Zend_Registry::isRegistered('Zend_Acl')) { return false; } + try { + $acl = Zend_Registry::get('Zend_Acl'); + return $acl->hasRole($role) && $acl->hasRole('admin') && $acl->inheritsRole($role, 'admin'); + } catch (Throwable $e) { + return false; + } + } + // ----- data / transactions ---------------------------------------------- /** The default DB adapter, or a clear failure if none is configured. */ diff --git a/library/Tiger/Version.php b/library/Tiger/Version.php index d5b24404..970d8cc4 100644 --- a/library/Tiger/Version.php +++ b/library/Tiger/Version.php @@ -9,5 +9,5 @@ class Tiger_Version { /** Current Tiger Core version. Keep in lockstep with the git tag cut for a release. */ - const VERSION = '1.8.0'; + const VERSION = '1.8.1'; } diff --git a/modules/comment/services/Comment.php b/modules/comment/services/Comment.php index 16d639da..d6249c2f 100644 --- a/modules/comment/services/Comment.php +++ b/modules/comment/services/Comment.php @@ -179,9 +179,9 @@ public function edit(array $params): void $userId = (string) ($this->_user_id ?? ''); $mine = $userId !== '' && (string) $row->user_id === $userId; - if (!$mine && !$this->_isAdmin()) { $this->_error('core.api.error.not_allowed'); return; } + if (!$mine && !$this->_isAtLeastAdmin()) { $this->_error('core.api.error.not_allowed'); return; } - if ($mine && !$this->_isAdmin() && (time() - strtotime((string) $row->created_at)) > Tiger_Comment::editWindow()) { + if ($mine && !$this->_isAtLeastAdmin() && (time() - strtotime((string) $row->created_at)) > Tiger_Comment::editWindow()) { $this->_error('comment.error.edit_window'); return; } @@ -219,8 +219,9 @@ public function edit(array $params): void */ public function moderate(array $params): void { + // Authorization BEFORE the feature flag: "not allowed" must never be masked by "not enabled". + if (!$this->_isAtLeastAdmin()) { $this->_error('core.api.error.not_allowed'); return; } if (!$this->_enabled()) { return; } - if (!$this->_isAdmin()) { $this->_error('core.api.error.not_allowed'); return; } $status = (string) ($params['status'] ?? ''); if (!in_array($status, Tiger_Model_Comment::STATUSES, true)) { $this->_error('comment.error.bad_status'); return; } @@ -256,7 +257,7 @@ public function delete(array $params): void $userId = (string) ($this->_user_id ?? ''); $mine = $userId !== '' && (string) $row->user_id === $userId; - if (!$mine && !$this->_isAdmin()) { $this->_error('core.api.error.not_allowed'); return; } + if (!$mine && !$this->_isAtLeastAdmin()) { $this->_error('core.api.error.not_allowed'); return; } try { $this->_transaction(function () use ($model, $row) { @@ -278,8 +279,8 @@ public function delete(array $params): void */ public function datatable(array $params): void { + if (!$this->_isAtLeastAdmin()) { $this->_error('core.api.error.not_allowed'); return; } if (!$this->_enabled()) { return; } - if (!$this->_isAdmin()) { $this->_error('core.api.error.not_allowed'); return; } $dt = $this->_dtParams(); $status = (string) ($params['status'] ?? Tiger_Model_Comment::STATUS_PENDING); diff --git a/modules/mcp/controllers/ServerController.php b/modules/mcp/controllers/ServerController.php index 0dfec03a..00cff5d4 100644 --- a/modules/mcp/controllers/ServerController.php +++ b/modules/mcp/controllers/ServerController.php @@ -53,6 +53,15 @@ public function indexAction() return; } + // JSON-RPC is JSON. Refusing any other Content-Type also closes the classic text/plain-form + // CSRF: a cross-site
cannot send application/json. + $ct = strtolower((string) $this->getRequest()->getHeader('Content-Type')); + if ($ct !== '' && strpos($ct, 'application/json') === false) { + $resp->setHttpResponseCode(415); + $this->_emit(['jsonrpc' => '2.0', 'id' => null, 'error' => ['code' => -32700, 'message' => 'Content-Type must be application/json']]); + return; + } + $msg = json_decode($this->_rawBody(), true); if (!is_array($msg)) { $resp->setHttpResponseCode(400); @@ -60,7 +69,24 @@ public function indexAction() return; } + // A Bearer that is presented but does not verify is 401 — never a silent downgrade to guest. + // A client with a bad key must learn it is bad, not receive the public tool list and wonder. + $bearer = $this->_bearer(); + if ($bearer !== null && (new Tiger_Service_Authentication())->identityFromToken($bearer) === null) { + $resp->setHttpResponseCode(401); + $resp->setHeader('WWW-Authenticate', 'Bearer realm="Tiger MCP"', true); + $this->_emit(['jsonrpc' => '2.0', 'id' => $msg['id'] ?? null, 'error' => ['code' => -32001, 'message' => 'Unauthorized: the Bearer token is not valid']]); + return; + } + $identity = $this->_identity(); + // A session (cookie) identity is honoured only for a same-origin request. A cross-site POST + // that rides the admin's cookie — the CSRF shape — is a guest here, whatever the cookie says. + if ($bearer === null && $identity !== null && !$this->_sameOrigin()) { + $resp->setHttpResponseCode(403); + $this->_emit(['jsonrpc' => '2.0', 'id' => $msg['id'] ?? null, 'error' => ['code' => -32002, 'message' => 'Forbidden: a session may only call /mcp from its own origin; use a Bearer token']]); + return; + } $role = ($identity && !empty($identity->role)) ? (string) $identity->role : 'guest'; // Resolve the token's MCP policy (scope + read-only + org-scoping + metering key). null for a @@ -90,7 +116,7 @@ public function indexAction() */ protected function _identity() { - $h = (string) $this->getRequest()->getHeader('Authorization'); + $h = Tiger_Ajax_ServiceFactory::authorizationHeader($this->getRequest()); if (preg_match('/^\s*Bearer\s+(\S+)/i', $h, $m)) { $id = (new Tiger_Service_Authentication())->identityFromToken($m[1]); if ($id !== null) { @@ -117,13 +143,37 @@ protected function _identity() protected function _tokenPolicy($identity) { if ($identity === null) { return [null, '']; } - $h = (string) $this->getRequest()->getHeader('Authorization'); + $h = Tiger_Ajax_ServiceFactory::authorizationHeader($this->getRequest()); if (!preg_match('/^\s*Bearer\s+tgr_([a-f0-9]{12})_/i', $h, $m)) { return [null, '']; } $prefix = $m[1]; $credId = (new Tiger_Model_UserCredential())->credentialIdByPrefix($prefix); return [Tiger_Mcp_Token::config((string) $credId), $prefix]; } + /** The presented Bearer token (verified or not), or null when the request carries none. */ + protected function _bearer() + { + $h = Tiger_Ajax_ServiceFactory::authorizationHeader($this->getRequest()); + return preg_match('/^\s*Bearer\s+(\S+)/i', $h, $m) ? $m[1] : null; + } + + /** + * Is this a same-origin request? Browsers say so themselves: Sec-Fetch-Site (same-origin / none) + * on every modern cross-site-capable request, Origin on every cross-origin POST. A request with + * neither header is not a browser (curl with a cookie jar, a test) and is taken at face value. + */ + protected function _sameOrigin() + { + $req = $this->getRequest(); + $site = strtolower((string) $req->getHeader('Sec-Fetch-Site')); + if ($site !== '') { return in_array($site, ['same-origin', 'none'], true); } + $origin = (string) $req->getHeader('Origin'); + if ($origin === '' || $origin === 'null') { return $origin === ''; } + $host = strtolower((string) ($req->getHttpHost() ?: ($_SERVER['HTTP_HOST'] ?? ''))); + return strtolower((string) parse_url($origin, PHP_URL_HOST)) === preg_replace('/:\d+$/', '', $host) + && (parse_url($origin, PHP_URL_PORT) === null || (string) parse_url($origin, PHP_URL_PORT) === (string) ($req->getServer('SERVER_PORT') ?? '')); + } + /** * Run one tool: enforce the token's scope + read-only, meter it, dispatch it (as the org for an * org-scoped token, else the token/session identity), and audit the outcome. Returns the /api envelope diff --git a/tests/Integration/Comment/CommentServiceTest.php b/tests/Integration/Comment/CommentServiceTest.php index b9b12a72..42a24a81 100644 --- a/tests/Integration/Comment/CommentServiceTest.php +++ b/tests/Integration/Comment/CommentServiceTest.php @@ -179,6 +179,7 @@ public function approving_a_rating_updates_the_rollup(): void $row = (new Tiger_Model_Comment())->byStatus(Tiger_Model_Comment::STATUS_PENDING, 5)[0]; + $this->loginAs('admin'); // moderation is an admin's act $svc = new Comment_Service_Comment(); $svc->moderate(['comment_id' => $row['comment_id'], 'status' => Tiger_Model_Comment::STATUS_APPROVED]); @@ -226,6 +227,47 @@ public function a_comment_without_a_rating_is_fine_from_the_owner(): void $this->assertSame(1, (int) $svc->getResponse()->result, 'a vendor may reply, just not rate'); } + /** + * The service is granted to GUESTS (anyone may post), so "allowed on this service" is not "is an + * admin". Moderation, the moderation grid and deleting someone else's comment are admin acts — + * refused to a signed-in user and to a guest — and refused BEFORE the feature flag speaks, so + * "not allowed" is never disguised as "not enabled" (TIGER-138). + */ + #[Test] + public function moderation_is_refused_to_a_user_and_a_guest_and_before_the_feature_flag(): void + { + $this->loginAs('user'); + $this->post(['rating' => 2]); + $row = (new Tiger_Model_Comment())->byStatus(Tiger_Model_Comment::STATUS_PENDING, 5)[0]; + + $svc = new Comment_Service_Comment(); + $svc->moderate(['comment_id' => $row['comment_id'], 'status' => Tiger_Model_Comment::STATUS_APPROVED]); + $this->assertSame(0, (int) $svc->getResponse()->result, 'a plain user cannot moderate'); + $this->assertSame(Tiger_Model_Comment::STATUS_PENDING, (new Tiger_Model_Comment())->findById($row['comment_id'])->status, 'and nothing changed'); + + $svc = new Comment_Service_Comment(); + $svc->datatable([]); + $this->assertSame(0, (int) $svc->getResponse()->result, 'nor read the moderation grid'); + + $this->logout(); + $svc = new Comment_Service_Comment(); + $svc->delete(['comment_id' => $row['comment_id']]); + $this->assertSame(0, (int) $svc->getResponse()->result, 'a guest cannot delete another\'s comment'); + $this->assertNotNull((new Tiger_Model_Comment())->findById($row['comment_id'])); + + // Feature flag OFF: an outsider is still told "not allowed", not "not enabled". + $this->enable(false); + $this->loginAs('admin'); + $svc = new Comment_Service_Comment(); + $svc->moderate(['comment_id' => $row['comment_id'], 'status' => Tiger_Model_Comment::STATUS_APPROVED]); + $notEnabled = json_encode($svc->getResponse()->messages); // what an ADMIN hears when the feature is off + $this->logout(); + $svc = new Comment_Service_Comment(); + $svc->moderate(['comment_id' => $row['comment_id'], 'status' => Tiger_Model_Comment::STATUS_APPROVED]); + $this->assertSame(0, (int) $svc->getResponse()->result); + $this->assertNotSame($notEnabled, json_encode($svc->getResponse()->messages), 'authorization answers before the feature flag: a guest is not told the feature is off'); + } + #[Test] public function a_guest_is_refused_unless_guests_are_allowed(): void { @@ -255,9 +297,11 @@ public function a_public_projection_never_leaks_an_email_or_ip(): void $this->post([]); $row = (new Tiger_Model_Comment())->byStatus(Tiger_Model_Comment::STATUS_PENDING, 5)[0]; + $this->loginAs('admin'); $svc = new Comment_Service_Comment(); $svc->moderate(['comment_id' => $row['comment_id'], 'status' => Tiger_Model_Comment::STATUS_APPROVED]); + $this->logout(); $svc = new Comment_Service_Comment(); $svc->list(['subject' => 'test.thing:t1']); $comment = ((array) $svc->getResponse()->data)['comments'][0]; @@ -274,6 +318,7 @@ public function deleting_a_comment_updates_the_rollup(): void $this->post(['rating' => 5]); $row = (new Tiger_Model_Comment())->byStatus(Tiger_Model_Comment::STATUS_PENDING, 5)[0]; + $this->loginAs('admin'); $svc = new Comment_Service_Comment(); $svc->moderate(['comment_id' => $row['comment_id'], 'status' => Tiger_Model_Comment::STATUS_APPROVED]); $this->assertSame(1, (new Tiger_Model_CommentAggregate())->forSubject('test.thing', 't1')['rating_count']); diff --git a/tests/Integration/Mcp/McpControllerTest.php b/tests/Integration/Mcp/McpControllerTest.php index 3fa03f39..8d344c08 100644 --- a/tests/Integration/Mcp/McpControllerTest.php +++ b/tests/Integration/Mcp/McpControllerTest.php @@ -33,6 +33,7 @@ protected function setUp(): void protected function tearDown(): void { if ($this->origConfig !== null) { Zend_Registry::set('Zend_Config', $this->origConfig); } + foreach (['HTTP_AUTHORIZATION', 'REDIRECT_HTTP_AUTHORIZATION', 'HTTP_SEC_FETCH_SITE', 'HTTP_ORIGIN', 'CONTENT_TYPE', 'HTTP_CONTENT_TYPE'] as $k) { unset($_SERVER[$k]); } parent::tearDown(); } @@ -98,6 +99,56 @@ public function tools_list_reflects_the_role_catalog_when_enabled(): void $this->assertArrayHasKey('title', $schema['properties'], 'the Cms_Form_Page fields are typed into the schema'); $this->assertArrayHasKey('slug', $schema['properties']); } + + /** A presented Bearer that does not verify is 401 — never a silent downgrade to the guest surface (TIGER-138). */ + #[Test] + public function an_invalid_bearer_is_401_not_guest(): void + { + $this->enableMcp(); + $_SERVER['HTTP_AUTHORIZATION'] = 'Bearer tgr_deadbeefdead_' . str_repeat('0', 48); + [$code, $out] = $this->post(['jsonrpc' => '2.0', 'id' => 7, 'method' => 'tools/list']); + $this->assertSame(401, $code); + $this->assertSame(7, $out['id']); + $this->assertStringContainsString('not valid', $out['error']['message']); + $this->assertArrayNotHasKey('result', $out, 'no tool list for a bad key'); + } + + /** Apache/FPM may surface the header only as REDIRECT_HTTP_AUTHORIZATION (the .htaccess rewrite-env fallback). */ + #[Test] + public function the_authorization_header_is_read_from_the_redirect_env_too(): void + { + $this->enableMcp(); + $_SERVER['REDIRECT_HTTP_AUTHORIZATION'] = 'Bearer tgr_deadbeefdead_' . str_repeat('0', 48); + [$code] = $this->post(['jsonrpc' => '2.0', 'id' => 8, 'method' => 'tools/list']); + $this->assertSame(401, $code, 'the token was SEEN (and refused) — it did not vanish into the guest path'); + } + + /** A session (cookie) identity is honoured only from its own origin: the CSRF shape is refused (F4). */ + #[Test] + public function a_cross_site_request_on_a_session_is_403(): void + { + $this->enableMcp(); + $this->loginAs('admin'); + $_SERVER['HTTP_SEC_FETCH_SITE'] = 'cross-site'; + [$code, $out] = $this->post(['jsonrpc' => '2.0', 'id' => 9, 'method' => 'tools/list']); + $this->assertSame(403, $code); + $this->assertStringContainsString('own origin', $out['error']['message']); + + $_SERVER['HTTP_SEC_FETCH_SITE'] = 'same-origin'; + [$code, $out] = $this->post(['jsonrpc' => '2.0', 'id' => 10, 'method' => 'tools/list']); + $this->assertSame(200, $code); + $this->assertNotEmpty($out['result']['tools'], 'the same admin, same-origin, is served'); + } + + /** JSON-RPC is JSON: a text/plain body (the enctype a cross-site form can send) is 415. */ + #[Test] + public function a_non_json_content_type_is_415(): void + { + $this->enableMcp(); + $_SERVER['CONTENT_TYPE'] = 'text/plain'; + [$code] = $this->post(['jsonrpc' => '2.0', 'id' => 11, 'method' => 'tools/list']); + $this->assertSame(415, $code); + } } /** Test double: inject the JSON-RPC body without php://input. */