mirror of
https://github.com/penpot/penpot.git
synced 2026-07-30 09:56:22 +00:00
🐛 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
This commit is contained in:
parent
c8d1f5b397
commit
5e5465a0fe
@ -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)))))
|
||||
|
||||
|
||||
@ -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)
|
||||
|
||||
|
||||
@ -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)
|
||||
|
||||
@ -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)))))
|
||||
|
||||
@ -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.
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user