From 5e5465a0fe8a18b4425247255f7899daa2aacd4e Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 30 Jul 2026 07:32:02 +0200 Subject: [PATCH] :bug: Fix audit event validation for error reports with string profile-id (#10898) The audit event validation was failing when processing error reports that contain string profile-id values. The error report storage converts profile-id to string format, but the audit schema expects a UUID. Changes: - Modified prepare-rpc-event to convert string profile-id to UUID using uuid/parse* (exception-safe parsing) - Updated access token middleware to set ::id and ::type on request so audit context includes token identification - Added tests for profile-id conversion and token context population Closes #10897 AI-assisted-by: qwen3.7-plus --- backend/src/app/http/access_token.clj | 11 +++-- backend/src/app/loggers/audit.clj | 4 +- .../backend_tests/http_middleware_test.clj | 15 +++++++ backend/test/backend_tests/rpc_audit_test.clj | 45 +++++++++++++++++++ .../rpc_commands_error_reports_test.clj | 39 +++++++++++++++- 5 files changed, 109 insertions(+), 5 deletions(-) diff --git a/backend/src/app/http/access_token.clj b/backend/src/app/http/access_token.clj index 7c8f2665e4..02d877b1df 100644 --- a/backend/src/app/http/access_token.clj +++ b/backend/src/app/http/access_token.clj @@ -24,7 +24,7 @@ :cause cause)))) (def sql:get-token-data - "SELECT perms, profile_id, expires_at + "SELECT perms, profile_id, expires_at, type FROM access_token WHERE id = ? AND (expires_at IS NULL @@ -42,14 +42,19 @@ (fn [request] (let [{:keys [type claims]} (get request ::http/auth-data)] (if (= :token type) - (let [{:keys [perms profile-id expires-at]} (some->> claims (get-token-data pool))] + (let [{:keys [perms profile-id expires-at type]} (some->> claims (get-token-data pool)) + token-id (get claims :tid)] (handler (cond-> request (some? perms) (assoc ::perms perms) (some? profile-id) (assoc ::profile-id profile-id) (some? expires-at) - (assoc ::expires-at expires-at)))) + (assoc ::expires-at expires-at) + (some? token-id) + (assoc ::id token-id) + (some? type) + (assoc ::type type)))) (handler request))))) diff --git a/backend/src/app/loggers/audit.clj b/backend/src/app/loggers/audit.clj index 9de442a48a..1141ade205 100644 --- a/backend/src/app/loggers/audit.clj +++ b/backend/src/app/loggers/audit.clj @@ -336,7 +336,9 @@ (let [resultm (meta result) request (-> params meta ::http/request) profile-id (or (::profile-id resultm) - (:profile-id result) + (some-> (:profile-id result) + (cond-> (string? (:profile-id result)) + uuid/parse*)) (::rpc/profile-id params) uuid/zero) diff --git a/backend/test/backend_tests/http_middleware_test.clj b/backend/test/backend_tests/http_middleware_test.clj index 357ddbad91..f751d8aca6 100644 --- a/backend/test/backend_tests/http_middleware_test.clj +++ b/backend/test/backend_tests/http_middleware_test.clj @@ -113,6 +113,21 @@ (t/is (= #{} (:app.http.access-token/perms response))) (t/is (= (:id profile) (:app.http.access-token/profile-id response)))))) +(t/deftest access-token-authz-sets-token-id-and-type + (let [profile (th/create-profile* 1) + token (db/tx-run! th/*system* app.rpc.commands.access-token/create-access-token + (:id profile) "test" nil "mcp") + handler (#'app.http.access-token/wrap-authz identity th/*system*) + request {::http/auth-data {:type :token :token "foobar" :claims {:tid (:id token)}}} + response (handler request)] + ;; Must set ::actoken/id from claims :tid + (t/is (= (:id token) (:app.http.access-token/id response))) + ;; Must set ::actoken/type from database + (t/is (= "mcp" (:app.http.access-token/type response))) + ;; Existing assertions still pass + (t/is (= #{} (:app.http.access-token/perms response))) + (t/is (= (:id profile) (:app.http.access-token/profile-id response))))) + (defrecord MethodAwareDummyRequest [req-method headers] yreq/IRequest (method [_] req-method) diff --git a/backend/test/backend_tests/rpc_audit_test.clj b/backend/test/backend_tests/rpc_audit_test.clj index 9b6c0181a4..f4cb76f2a3 100644 --- a/backend/test/backend_tests/rpc_audit_test.clj +++ b/backend/test/backend_tests/rpc_audit_test.clj @@ -13,6 +13,7 @@ [app.db :as db] [app.loggers.audit :as audit] [app.rpc :as-alias rpc] + [app.util.services :as sv] [backend-tests.helpers :as th] [clojure.test :as t] [yetti.request])) @@ -498,3 +499,47 @@ (t/is (some? (:tracked-at row))) (t/is (= {} (:props row))) (t/is (= {} (:context row)))))) + +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +;; PREPARE-RPC-EVENT PROFILE-ID CONVERSION +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; + +(t/deftest prepare-rpc-event-converts-string-profile-id-to-uuid + ;; When result contains a string :profile-id (e.g. from error reports), + ;; prepare-rpc-event must convert it to a UUID for audit schema compliance. + (let [prof (th/create-profile* 1 {:is-active true}) + string-pid "33601240-a00b-11ea-ba1b-c554cc60e361" + expected #uuid "33601240-a00b-11ea-ba1b-c554cc60e361" + mdata {::sv/name "test-cmd"} + params {::rpc/profile-id (:id prof) + ::rpc/request-id (uuid/next) + ::rpc/request-at (ct/now)} + mock-req (reify + yetti.request/IRequest + (get-header [_ _] nil) + (remote-addr [_] "127.0.0.1")) + params (with-meta params {:app.http/request mock-req}) + result {:profile-id string-pid :some-data "value"} + event (audit/prepare-rpc-event th/*system* mdata params result)] + ;; profile-id must be a UUID, not a string + (t/is (uuid? (:profile-id event))) + (t/is (= expected (:profile-id event))))) + +(t/deftest prepare-rpc-event-handles-invalid-string-profile-id + ;; When result contains an invalid string :profile-id, it should fall back + ;; to the RPC params profile-id (which is always a valid UUID). + (let [prof (th/create-profile* 1 {:is-active true}) + mdata {::sv/name "test-cmd"} + params {::rpc/profile-id (:id prof) + ::rpc/request-id (uuid/next) + ::rpc/request-at (ct/now)} + mock-req (reify + yetti.request/IRequest + (get-header [_ _] nil) + (remote-addr [_] "127.0.0.1")) + params (with-meta params {:app.http/request mock-req}) + result {:profile-id "not-a-valid-uuid"} + event (audit/prepare-rpc-event th/*system* mdata params result)] + ;; profile-id must fall back to the RPC params profile-id + (t/is (uuid? (:profile-id event))) + (t/is (= (:id prof) (:profile-id event))))) diff --git a/backend/test/backend_tests/rpc_commands_error_reports_test.clj b/backend/test/backend_tests/rpc_commands_error_reports_test.clj index 7e22c05691..c317fe0cbd 100644 --- a/backend/test/backend_tests/rpc_commands_error_reports_test.clj +++ b/backend/test/backend_tests/rpc_commands_error_reports_test.clj @@ -8,11 +8,15 @@ (:require [app.common.time :as ct] [app.common.uuid :as uuid] + [app.config :as cf] [app.db :as db] + [app.loggers.audit :as audit] [app.rpc :as-alias rpc] + [app.util.services :as sv] [backend-tests.helpers :as th] [clojure.test :as t] - [cuerdas.core :as str])) + [cuerdas.core :as str] + [yetti.request])) (t/use-fixtures :once th/state-init) (t/use-fixtures :each th/database-reset) @@ -249,3 +253,36 @@ (t/is (not (th/success? out))) (t/is (= :not-found (th/ex-type (:error out)))) (t/is (= :report-not-found (th/ex-code (:error out)))))) + +;; --- Audit event tests + +(t/deftest get-error-report-audit-event-has-uuid-profile-id + ;; When get-error-report returns a report with string profile-id in content, + ;; the audit event must have a proper UUID profile-id (not a string). + ;; This tests the prepare-rpc-event function directly since the test RPC + ;; flow doesn't include the audit middleware wrapper. + (let [profile (th/create-profile* 1 {:is-active true}) + id (uuid/next) + orig-pid "33601240-a00b-11ea-ba1b-c554cc60e361" + ;; Simulate the result from get-error-report with string profile-id + result {:id id + :source "logging" + :hint "test error" + :profile-id orig-pid} + mdata {::sv/name "get-error-report"} + params {::rpc/profile-id (:id profile) + ::rpc/request-id (uuid/next) + ::rpc/request-at (ct/now)} + mock-req (reify yetti.request/IRequest + (get-header [_ _] nil) + (remote-addr [_] "127.0.0.1")) + params (with-meta params {:app.http/request mock-req}) + event (audit/prepare-rpc-event th/*system* mdata params result)] + ;; profile-id must be a UUID, not a string + (t/is (uuid? (:profile-id event))) + (t/is (= #uuid "33601240-a00b-11ea-ba1b-c554cc60e361" (:profile-id event))))) + +;; Note: The integration of access token middleware with audit context is tested +;; via unit tests in rpc_audit_test.clj and http_middleware_test.clj. +;; The middleware sets ::id and ::type on the request, and prepare-context-from-request +;; reads these values to populate :access-token-id and :access-token-type in the context.