API: allow reads to filter by client_id on top of RBAC client scoping

The RBAC rework dropped the client_id read filter along with per-key client scope, so an all-clients key could no longer narrow to one client. apiClientScopeSql() now appends the filter after the scope fragment, so it can only narrow, never widen. Reads only - writes take client_id as their target client.
This commit is contained in:
johnnyq
2026-08-31 18:30:09 -04:00
parent b0c12ac89d
commit 36ea4a9a80
4 changed files with 43 additions and 13 deletions

View File

@@ -12,6 +12,10 @@
* the user. Reads are scoped with apiClientScopeSql(); writes act on a single
* target client supplied in the request and validated against the user's access.
*
* Reads may also pass client_id as an optional filter (e.g. every asset belonging to
* client 5). It is applied inside apiClientScopeSql() on top of the user's scope, so it
* can only narrow the result set, never widen it.
*
* Reuses the existing RBAC machinery so there is no parallel permission model.
* Included by validate_api_key.php - runs in global scope with $mysqli,
* $api_key_user_id and $return_arr available.
@@ -44,11 +48,32 @@ function apiUserCanAccessClient($client_id) {
return empty($client_access_array) || in_array($client_id, $client_access_array, true);
}
// Client-scope SQL fragment for a read query, from the user's allow / deny lists.
// Thin wrapper over clientScopeSql() in functions/auth.php so the API and the UI share one
// implementation. Kept under the api* name because every endpoint already calls it.
// Client-scope SQL fragment for a read query: the user's allow / deny lists, plus the
// optional client_id filter the caller asked for.
//
// The scope half is a thin wrapper over clientScopeSql() in functions/auth.php so the API
// and the UI share one implementation. Kept under the api* name because every endpoint
// already calls it.
//
// The filter half is what lets a key that can see every client ask for just one of them -
// GET assets/read.php?api_key=...&client_id=5. It is appended AFTER the scope fragment, so
// it can only narrow: a user who cannot see client 5 still gets nothing back.
//
// Reads only. Writes take client_id as the target client they act on and validate it
// against the user's access further down, so filtering a write query by it would be wrong.
function apiClientScopeSql($column) {
return clientScopeSql($column);
global $client_id, $client_id_supplied, $is_write;
$sql = clientScopeSql($column);
// Tested on $client_id_supplied, not on the value: client_id = 0 is a real request
// (records with no client), and require_get_method.php turns an absent client_id
// into "%", which would silently compare as 0 against an integer column.
if (!empty($client_id_supplied) && empty($is_write)) {
$sql .= " AND $column = " . intval($client_id);
}
return $sql;
}
// --- Every key must be tied to a user (legacy keys were removed in the 2.4.7 migration) ---
@@ -145,11 +170,13 @@ if (lookupUserPermission($resource_module[$resource]) < $required_level) {
}
// --- 3) Target client for writes: taken from the request, validated against the user ---
// Reads ignore this and use apiClientScopeSql(). Create/update/delete act on a single
// client the caller names (client_id in the body/query); it must be within the user's
// access. Callers that omit it get $client_id = 0 (creates that require a client fail
// their own !empty($client_id) guard, which is the intended "must name a client").
// Create/update/delete act on a single client the caller names (client_id in the
// body/query); it must be within the user's access. Callers that omit it get
// $client_id = 0 (creates that require a client fail their own !empty($client_id) guard,
// which is the intended "must name a client"). On a read the same parameter is not a
// permission at all, just an optional filter applied inside apiClientScopeSql().
$client_id = intval($_POST['client_id'] ?? $_GET['client_id'] ?? 0);
$client_id_supplied = isset($_POST['client_id']) || isset($_GET['client_id']);
$is_write = in_array($operation_file, ['create.php', 'update.php', 'delete.php'], true);
if ($is_write && !apiUserCanAccessClient($client_id)) {
// Writes act on a single client the caller names (client_id 0 = a global record).
@@ -157,4 +184,5 @@ if ($is_write && !apiUserCanAccessClient($client_id)) {
// writing global (client_id 0) records, which their access does not include.
apiDeny("The user linked to this API key does not have access to the target client for this write.");
}
// Reads ignore $client_id entirely and are scoped by apiClientScopeSql().
// Reads never treat $client_id as a permission - apiClientScopeSql() enforces the user's
// scope first and only then narrows to $client_id if the caller supplied one.

View File

@@ -10,17 +10,17 @@
* invoice_id required* - Return items for a single invoice
* item_id required* - Return a single line item by its own ID
* * One of invoice_id or item_id must be provided
* client_id optional - Only return items on invoices for this client
* limit optional - Max rows to return (default 50)
* offset optional - Offset for pagination (default 0)
*
* Security:
* - invoice_items are always joined to invoices so that invoice_client_id
* is checked against the API key's client scope. A scoped key can never
* is checked against the key user's client scope. A restricted key can never
* read items belonging to another client, even when item_id is supplied
* directly.
* - $client_id is set to "%" by validate_api_key.php for All-Clients keys,
* which causes the LIKE to match every client — consistent with other
* endpoints in this API.
* - An optional client_id narrows the result further, within that scope. It is
* a filter, not a grant - it can never widen what the key can see.
*/
require_once '../validate_api_key.php';
require_once '../require_get_method.php';

View File

@@ -9,6 +9,7 @@
* year (optional) - Filter by year (default: current year)
* month (optional) - Filter by month 1-12 (default: current month)
* technician_id (optional) - Filter by specific technician user ID
* client_id (optional) - Only count time on tickets for this client
* limit (optional) - Number of results to return (default: 50)
* offset (optional) - Offset for pagination (default: 0)
*/

View File

@@ -10,6 +10,7 @@
* ticket_reply_id optional - Return a single reply by its own ID
* ticket_id optional - Return all replies on a single ticket
* type optional - Filter by reply type: Internal, Public or Client
* client_id optional - Only return replies on tickets for this client
* include_archived optional - Set to 1 to include archived replies (default: excluded)
* limit optional - Max rows to return (default 50)
* offset optional - Offset for pagination (default 0)