diff --git a/README.md b/README.md index 0177c49..286ba3b 100644 --- a/README.md +++ b/README.md @@ -97,12 +97,15 @@ The following features are intentionally **not planned** for this system: - **Admin UI**: Generate and manage API keys at `/admin/api-keys` (paginated) - **Bearer Token Auth**: Use API keys with `Authorization: Bearer YOUR_KEY` header - **Key Scopes**: `read` (GET only) or `read_write` (create/comment/close). A `read` key cannot mutate anything, including creating tickets. Existing keys default to `read_write`. +- **Visibility Scope**: keys default to **public-visibility tickets only** — Confidential and Internal tickets are excluded from `/api/tickets_api.php`, same as they'd be for a regular user with no special access. Check **See all visibility** when generating a key only if that specific integration genuinely needs the full queue; this is a deliberate opt-in, not something a key gets by having `read_write` scope or any other setting. - **Expiration**: Optional expiration dates for keys - **Revocation**: Revoke compromised keys instantly ### Bearer API (automation / triage) All Bearer-authenticated, rate-limited, and (like `create_ticket_api.php`) exempt from Authelia at the reverse proxy — the API key is the only credential. Comments/closes made via the API are attributed to the **key's name** (linked to the key's owner). +`/api/tickets_api.php` filters by ticket visibility according to the key's **Visibility Scope** setting above (public-only by default); every other Bearer endpoint below operates on a single ticket_id supplied by the caller and does not filter a list, so this setting doesn't apply to them. + | Endpoint | Method | Scope | Purpose | |----------|--------|-------|---------| | `/create_ticket_api.php` | POST | read_write | Create a ticket (hwmonDaemon, external tools) | diff --git a/api/generate_api_key.php b/api/generate_api_key.php index 394c203..936d6a2 100644 --- a/api/generate_api_key.php +++ b/api/generate_api_key.php @@ -67,6 +67,7 @@ try { $keyName = trim($input['key_name'] ?? ''); $expiresInDays = $input['expires_in_days'] ?? null; $scope = $input['scope'] ?? 'read_write'; + $seeAllVisibility = !empty($input['see_all_visibility']); if (empty($keyName)) { http_response_code(400); @@ -100,7 +101,7 @@ try { // Generate API key $apiKeyModel = new ApiKeyModel($conn); - $result = $apiKeyModel->createKey($keyName, $_SESSION['user']['user_id'], $expiresInDays, $scope); + $result = $apiKeyModel->createKey($keyName, $_SESSION['user']['user_id'], $expiresInDays, $scope, $seeAllVisibility); if (!$result['success']) { throw new Exception($result['error'] ?? "Failed to generate API key"); @@ -113,7 +114,7 @@ try { 'create', 'api_key', $result['key_id'], - ['key_name' => $keyName, 'expires_in_days' => $expiresInDays, 'scope' => $scope] + ['key_name' => $keyName, 'expires_in_days' => $expiresInDays, 'scope' => $scope, 'see_all_visibility' => $seeAllVisibility] ); // Clear output buffer @@ -127,6 +128,7 @@ try { 'key_prefix' => $result['key_prefix'], 'key_id' => $result['key_id'], 'scope' => $result['scope'], + 'see_all_visibility' => $result['see_all_visibility'], 'expires_at' => $result['expires_at'] ]); } catch (Exception $e) { diff --git a/api/tickets_api.php b/api/tickets_api.php index 95e220c..7bb85e0 100644 --- a/api/tickets_api.php +++ b/api/tickets_api.php @@ -4,8 +4,10 @@ * tickets_api.php — Bearer-key read endpoint (list/triage + read-one). * * GET only. Requires 'read' scope (a 'read_write' key also satisfies it). - * Acts as a trusted automation/server credential: reads return the full queue - * (no per-user visibility filtering). + * By default, a key only sees public-visibility tickets — Confidential and + * Internal tickets are excluded, the same as they would be for a regular user + * with no special access. An admin can mark a specific key see_all_visibility + * (API Key Management) when it genuinely needs the full queue. * * GET ?ticket_id=NNN -> {success, ticket, comments} * GET ?status=&priority=&host= -> {success, tickets, page, total, pages} @@ -48,6 +50,20 @@ try { // Reads only need the 'read' scope. $apiKeyAuth->requireScope('read'); +// Keys are public-ticket-only by default (#70) — a key must be explicitly +// marked see_all_visibility to bypass Confidential/Internal restrictions. +// Reuse TicketModel's existing per-user visibility plumbing with a synthetic +// "no special access" user rather than a separate SQL path, so this stays in +// lockstep with however visibility rules evolve for real users. user_id is +// -1, not 0: canUserAccessTicket() does a PHP-level (int) cast for the +// confidential-ticket check, and (int)null === 0, so an unassigned +// confidential ticket's assigned_to would otherwise false-positive-match a +// synthetic user_id of 0. No real user_id is ever <= 0, so -1 can't collide. +$keyContext = $apiKeyAuth->getKeyContext(); +$visibilityUser = !empty($keyContext['see_all_visibility']) + ? null + : ['user_id' => -1, 'is_admin' => false, 'groups' => '']; + if ($_SERVER['REQUEST_METHOD'] !== 'GET') { http_response_code(405); echo json_encode(['success' => false, 'error' => 'Method not allowed. Use GET.']); @@ -67,6 +83,15 @@ if (isset($_GET['ticket_id']) && trim((string)$_GET['ticket_id']) !== '') { exit; } + // A public-only key gets a plain 404 for a non-public ticket — same as a + // regular user hitting a ticket they can't see — rather than a 403 that + // would confirm the ticket exists. + if ($visibilityUser !== null && !$ticketModel->canUserAccessTicket($ticket, $visibilityUser)) { + http_response_code(404); + echo json_encode(['success' => false, 'error' => 'Ticket not found']); + exit; + } + // Flat list of comments (newest first) — same fetch the ticket view uses. $commentModel = new CommentModel($conn); $comments = $commentModel->getCommentsByTicketId($ticketId, false); @@ -114,8 +139,8 @@ if (isset($_GET['host']) && trim((string)$_GET['host']) !== '') { $search = trim((string)$_GET['host']); } -// user = null => getAllTickets skips visibility filtering and returns the full -// queue (this is a trusted server credential, not an end user). +// $visibilityUser is null (skip filtering, full queue) only for a key marked +// see_all_visibility; otherwise it restricts to public tickets (see above). $result = $ticketModel->getAllTickets( $page, $limit, @@ -126,7 +151,7 @@ $result = $ticketModel->getAllTickets( null, $search, $filters, - null + $visibilityUser ); echo json_encode([ diff --git a/middleware/ApiKeyAuth.php b/middleware/ApiKeyAuth.php index cf37670..cbf900e 100644 --- a/middleware/ApiKeyAuth.php +++ b/middleware/ApiKeyAuth.php @@ -37,6 +37,7 @@ class ApiKeyAuth { $this->keyContext = [ 'scope' => $keyData['scope'] ?? 'read_write', + 'see_all_visibility' => !empty($keyData['see_all_visibility']), 'key_name' => $keyData['key_name'] ?? null, 'created_by' => $keyData['created_by'] ?? null, 'api_key_id' => $keyData['api_key_id'] ?? null, @@ -46,7 +47,7 @@ class ApiKeyAuth /** * Get the context of the authenticated API key. * - * @return array|null ['scope', 'key_name', 'created_by', 'api_key_id'] or null + * @return array|null ['scope', 'see_all_visibility', 'key_name', 'created_by', 'api_key_id'] or null */ public function getKeyContext(): ?array { diff --git a/migrations/006_api_key_visibility_scope.sql b/migrations/006_api_key_visibility_scope.sql new file mode 100644 index 0000000..feb1797 --- /dev/null +++ b/migrations/006_api_key_visibility_scope.sql @@ -0,0 +1,13 @@ +-- Restrict Bearer API keys to public-visibility tickets by default (#70). +-- +-- Previously any 'read'-scope key bypassed ticket visibility entirely — +-- Confidential and Internal tickets were readable by any key, regardless +-- of who it was issued to. see_all_visibility is an explicit opt-in an +-- admin sets per-key when a key genuinely needs to see non-public tickets; +-- it defaults to 0 (public-only) for both new and existing keys, since the +-- prior blanket-access behavior is the thing being restricted. +-- +-- Safe to re-run. + +ALTER TABLE `api_keys` + ADD COLUMN IF NOT EXISTS `see_all_visibility` tinyint(1) NOT NULL DEFAULT 0 AFTER `scope`; diff --git a/models/ApiKeyModel.php b/models/ApiKeyModel.php index 9213030..1af924a 100644 --- a/models/ApiKeyModel.php +++ b/models/ApiKeyModel.php @@ -19,9 +19,11 @@ class ApiKeyModel * @param int $createdBy User ID who created the key * @param int|null $expiresInDays Number of days until expiration (null for no expiration) * @param string $scope Access scope: 'read' or 'read_write' (default 'read_write') + * @param bool $seeAllVisibility If true, the key bypasses ticket visibility (Confidential/ + * Internal included); defaults to false (public tickets only) * @return array Array with 'success', 'api_key' (plaintext), 'key_prefix', 'scope', 'error' */ - public function createKey($keyName, $createdBy, $expiresInDays = null, $scope = 'read_write') + public function createKey($keyName, $createdBy, $expiresInDays = null, $scope = 'read_write', $seeAllVisibility = false) { // Validate the requested scope — only the two known values are allowed if (!in_array($scope, ['read', 'read_write'], true)) { @@ -47,11 +49,12 @@ class ApiKeyModel } // Insert API key into database + $seeAllVisibilityInt = $seeAllVisibility ? 1 : 0; $stmt = $this->conn->prepare( - "INSERT INTO api_keys (key_name, key_hash, key_prefix, scope, created_by, expires_at) " - . "VALUES (?, ?, ?, ?, ?, ?)" + "INSERT INTO api_keys (key_name, key_hash, key_prefix, scope, see_all_visibility, created_by, expires_at) " + . "VALUES (?, ?, ?, ?, ?, ?, ?)" ); - $stmt->bind_param("ssssis", $keyName, $keyHash, $keyPrefix, $scope, $createdBy, $expiresAt); + $stmt->bind_param("ssssiis", $keyName, $keyHash, $keyPrefix, $scope, $seeAllVisibilityInt, $createdBy, $expiresAt); if ($stmt->execute()) { $keyId = $this->conn->insert_id; @@ -63,6 +66,7 @@ class ApiKeyModel 'key_prefix' => $keyPrefix, 'key_id' => $keyId, 'scope' => $scope, + 'see_all_visibility' => $seeAllVisibility, 'expires_at' => $expiresAt ]; } else { @@ -114,6 +118,13 @@ class ApiKeyModel $keyData['scope'] = 'read_write'; } + // Unlike scope's backward-compatible fallback above, an un-migrated or + // null see_all_visibility defaults to the RESTRICTIVE value (public + // tickets only) — this column exists specifically to lock down a + // previously-unrestricted default, so a missing value must not fall + // back to the permissive behavior it's replacing. + $keyData['see_all_visibility'] = !empty($keyData['see_all_visibility']); + // Check expiration if ($keyData['expires_at'] !== null) { $expiresAt = strtotime($keyData['expires_at']); diff --git a/views/admin/ApiKeysView.php b/views/admin/ApiKeysView.php index 24b8e6e..9502d90 100644 --- a/views/admin/ApiKeysView.php +++ b/views/admin/ApiKeysView.php @@ -45,10 +45,18 @@ include __DIR__ . '/../../views/layout_header.php'; +
Scope: read = GET only; read_write = create/comment/close. + By default a key only sees public-visibility tickets — check + See all visibility only if this key genuinely needs Confidential/Internal tickets too.
@@ -74,6 +82,7 @@ include __DIR__ . '/../../views/layout_header.php';