Restrict API keys to public-visibility tickets by default (#70)
api/tickets_api.php (both the single-ticket read and the list/triage path) bypassed ticket visibility entirely for any 'read'-scope key, regardless of who it was issued to or what it was for — any key got blanket read access to Confidential and Internal ticket titles, descriptions, and comments, with no way to scope a key more narrowly. Added see_all_visibility to api_keys (migration 006), defaulting to false for both new and existing keys — the prior blanket-access behavior is what's being restricted here, so unlike scope's own un-migrated-database fallback (which defaults toward preserving old behavior), a missing/null value here defaults to the new, restrictive one. An admin can opt a specific key in via a new checkbox in the API Key Management UI when it genuinely needs the full queue. tickets_api.php now builds a synthetic "no special access" user and runs it through TicketModel's existing per-user visibility plumbing (getVisibilityFilter/canUserAccessTicket) instead of a separate SQL path, so this stays in lockstep with however visibility rules evolve for real users. That synthetic user_id is -1, not 0: testing surfaced that canUserAccessTicket()'s confidential-ticket check does a PHP-level (int) cast, and (int)null === 0, so an unassigned confidential ticket's NULL assigned_to would otherwise false-positive-match a user_id of 0. Verified against real MariaDB with public/confidential/internal test tickets: a public-only-scoped key's list only returns the public ticket, and canUserAccessTicket() correctly returns false for both the confidential ticket (unassigned, then reassigned to a real user — both cases) and the internal one; a see_all_visibility key sees all three, unchanged from the prior behavior. Also verified createKey()/ validateKey()'s default-false and explicit-true paths round-trip correctly through the real DB. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0117oBw2jN4kALYeS8HPq4zV
This commit is contained in:
+15
-4
@@ -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']);
|
||||
|
||||
Reference in New Issue
Block a user