From f5996a72359273686d04c7ab79a59944d19421be Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Mon, 26 Jan 2026 14:41:03 +0100 Subject: [PATCH] :recycle: Make several improvements to management API authentication --- backend/scripts/_env | 7 ++- backend/src/app/config.clj | 2 + backend/src/app/http/management.clj | 50 ++++++++++++------- backend/src/app/http/middleware.clj | 25 ++++++---- backend/src/app/loggers/audit.clj | 6 ++- backend/src/app/main.clj | 12 +++-- backend/src/app/rpc.clj | 17 +++---- backend/src/app/setup.clj | 34 ++++++++++++- .../backend_tests/http_middleware_test.clj | 5 +- exporter/src/app/config.cljs | 17 ++++--- exporter/src/app/handlers/resources.cljs | 2 +- 11 files changed, 122 insertions(+), 55 deletions(-) diff --git a/backend/scripts/_env b/backend/scripts/_env index 3155974924..0026d9f901 100644 --- a/backend/scripts/_env +++ b/backend/scripts/_env @@ -1,7 +1,12 @@ #!/usr/bin/env bash -export PENPOT_MANAGEMENT_API_KEY=super-secret-management-api-key +export PENPOT_NITRATE_SHARED_KEY=super-secret-nitrate-api-key +export PENPOT_EXPORTER_SHARED_KEY=super-secret-exporter-api-key export PENPOT_SECRET_KEY=super-secret-devenv-key + +# DEPRECATED: only used for subscriptions +export PENPOT_MANAGEMENT_API_KEY=super-secret-management-api-key + export PENPOT_HOST=devenv export PENPOT_PUBLIC_URI=https://localhost:3449 diff --git a/backend/src/app/config.clj b/backend/src/app/config.clj index 7ba69d7710..6b3f784170 100644 --- a/backend/src/app/config.clj +++ b/backend/src/app/config.clj @@ -102,6 +102,8 @@ [:http-server-io-threads {:optional true} ::sm/int] [:http-server-max-worker-threads {:optional true} ::sm/int] + [:exporter-shared-key {:optional true} :string] + [:nitrate-shared-key {:optional true} :string] [:management-api-key {:optional true} :string] [:telemetry-uri {:optional true} :string] diff --git a/backend/src/app/http/management.clj b/backend/src/app/http/management.clj index b32b7f4077..6f461dbc1e 100644 --- a/backend/src/app/http/management.clj +++ b/backend/src/app/http/management.clj @@ -13,13 +13,13 @@ [app.common.time :as ct] [app.config :as cf] [app.db :as db] - [app.http.middleware :as mw] [app.main :as-alias main] [app.rpc.commands.profile :as cmd.profile] [app.setup :as-alias setup] [app.tokens :as tokens] [app.worker :as-alias wrk] [integrant.core :as ig] + [yetti.request :as yreq] [yetti.response :as-alias yres])) ;; ---- ROUTES @@ -49,28 +49,40 @@ (fn [cfg request] (db/tx-run! cfg handler request)))))}) +(def ^:private shared-key-auth + {:name ::shared-key-auth + :compile + (fn [_ _] + (fn [handler key] + (if key + (fn [request] + (if-let [key' (yreq/get-header request "x-shared-key")] + (if (= key key') + (handler request) + {::yres/status 403}) + {::yres/status 403})) + (fn [_ _] + {::yres/status 403}))))}) + (defmethod ig/init-key ::routes - [_ {:keys [::setup/props] :as cfg}] + [_ cfg] - (let [management-key (or (cf/get :management-api-key) - (get props :management-key))] + ["" {:middleware [[shared-key-auth (cf/get :management-api-key)] + [default-system cfg] + [transaction]]} + ["/authenticate" + {:handler authenticate + :allowed-methods #{:post}}] - ["" {:middleware [[mw/shared-key-auth management-key] - [default-system cfg] - [transaction]]} - ["/authenticate" - {:handler authenticate - :allowed-methods #{:post}}] + ["/get-customer" + {:handler get-customer + :transaction true + :allowed-methods #{:post}}] - ["/get-customer" - {:handler get-customer - :transaction true - :allowed-methods #{:post}}] - - ["/update-customer" - {:handler update-customer - :allowed-methods #{:post} - :transaction true}]])) + ["/update-customer" + {:handler update-customer + :allowed-methods #{:post} + :transaction true}]]) ;; ---- HELPERS diff --git a/backend/src/app/http/middleware.clj b/backend/src/app/http/middleware.clj index 7d4e29b721..48f7c36149 100644 --- a/backend/src/app/http/middleware.clj +++ b/backend/src/app/http/middleware.clj @@ -16,7 +16,6 @@ [app.http.errors :as errors] [app.tokens :as tokens] [app.util.pointer-map :as pmap] - [buddy.core.codecs :as bc] [cuerdas.core :as str] [yetti.adapter :as yt] [yetti.middleware :as ymw] @@ -301,16 +300,20 @@ :compile (constantly wrap-auth)}) (defn- wrap-shared-key-auth - [handler shared-key] - (if shared-key - (let [shared-key (if (string? shared-key) - shared-key - (bc/bytes->b64-str shared-key true))] - (fn [request] - (let [key (yreq/get-header request "x-shared-key")] - (if (= key shared-key) - (handler (assoc request ::http/auth-with-shared-key true)) - {::yres/status 403})))) + [handler keys] + (if (seq keys) + (fn [request] + (if-let [[key-id key] (some-> (yreq/get-header request "x-shared-key") + (str/split #"\s+" 2))] + (let [key-id (str/lower key-id)] + (if (and (string? key) + (contains? keys key-id) + (= key (get keys key-id))) + (-> request + (assoc ::http/auth-key-id key-id) + (handler)) + {::yres/status 403})) + {::yres/status 403})) (fn [_ _] {::yres/status 403}))) diff --git a/backend/src/app/loggers/audit.clj b/backend/src/app/loggers/audit.clj index b620cdd6e6..859b84de7d 100644 --- a/backend/src/app/loggers/audit.clj +++ b/backend/src/app/loggers/audit.clj @@ -140,10 +140,14 @@ client-version (get-client-version request) client-user-agent (get-client-user-agent request) session-id (get-external-session-id request) - token-id (::actoken/id request)] + key-id (::http/auth-key-id request) + token-id (::actoken/id request) + token-type (::actoken/type request)] (d/without-nils {:external-session-id session-id + :initiator (or key-id "app") :access-token-id (some-> token-id str) + :access-token-type (some-> token-type str) :client-event-origin client-event-origin :client-user-agent client-user-agent :client-version client-version diff --git a/backend/src/app/main.clj b/backend/src/app/main.clj index 1923b5f054..f28d14db68 100644 --- a/backend/src/app/main.clj +++ b/backend/src/app/main.clj @@ -275,8 +275,7 @@ ::email/whitelist (ig/ref ::email/whitelist)} ::mgmt/routes - {::db/pool (ig/ref ::db/pool) - ::setup/props (ig/ref ::setup/props)} + {::db/pool (ig/ref ::db/pool)} :app.http/router {::session/manager (ig/ref ::session/manager) @@ -357,13 +356,13 @@ ::setup/props (ig/ref ::setup/props)} ::rpc/routes - {::rpc/methods (ig/ref :app.rpc/methods) + {::rpc/methods (ig/ref :app.rpc/methods) ::rpc/management-methods (ig/ref :app.rpc/management-methods) ;; FIXME: revisit if db/pool is necessary here ::db/pool (ig/ref ::db/pool) ::session/manager (ig/ref ::session/manager) - ::setup/props (ig/ref ::setup/props)} + ::setup/shared-keys (ig/ref ::setup/shared-keys)} ::wrk/registry {::mtx/metrics (ig/ref ::mtx/metrics) @@ -451,6 +450,11 @@ ;; module requires the migrations to run before initialize. ::migrations (ig/ref :app.migrations/migrations)} + ::setup/shared-keys + {::setup/props (ig/ref ::setup/props) + :nitrate (cf/get :nitrate-shared-key) + :exporter (cf/get :exporter-shared-key)} + ::setup/clock {} diff --git a/backend/src/app/rpc.clj b/backend/src/app/rpc.clj index 7e286360ad..947b78f7d1 100644 --- a/backend/src/app/rpc.clj +++ b/backend/src/app/rpc.clj @@ -92,11 +92,11 @@ (fn [{:keys [params path-params method] :as request}] (let [handler-name (:type path-params) etag (yreq/get-header request "if-none-match") + + key-id (get request ::http/auth-key-id) profile-id (or (::session/profile-id request) (::actoken/profile-id request) - (if (::http/auth-with-shared-key request) - uuid/zero - nil)) + (if key-id uuid/zero nil)) ip-addr (inet/parse-request request) @@ -346,23 +346,20 @@ (defmethod ig/assert-key ::routes [_ params] + (assert (map? (::setup/shared-keys params))) (assert (db/pool? (::db/pool params)) "expect valid database pool") - (assert (some? (::setup/props params))) (assert (session/manager? (::session/manager params)) "expect valid session manager") (assert (valid-methods? (::methods params)) "expect valid methods map") (assert (valid-methods? (::management-methods params)) "expect valid methods map")) (defmethod ig/init-key ::routes - [_ {:keys [::methods ::management-methods ::setup/props] :as cfg}] - - (let [public-uri (cf/get :public-uri) - management-key (or (cf/get :management-api-key) - (get props :management-key))] + [_ {:keys [::methods ::management-methods ::setup/shared-keys] :as cfg}] + (let [public-uri (cf/get :public-uri)] ["/api" ["/management" ["/methods/:type" - {:middleware [[mw/shared-key-auth management-key] + {:middleware [[mw/shared-key-auth shared-keys] [session/authz cfg]] :handler (make-rpc-handler management-methods)}] diff --git a/backend/src/app/setup.clj b/backend/src/app/setup.clj index 03bed90182..2ef0c49b66 100644 --- a/backend/src/app/setup.clj +++ b/backend/src/app/setup.clj @@ -17,6 +17,7 @@ [app.setup.templates] [buddy.core.codecs :as bc] [buddy.core.nonce :as bn] + [cuerdas.core :as str] [integrant.core :as ig])) (defn- generate-random-key @@ -88,7 +89,38 @@ (-> (get-all-props conn) (assoc :secret-key secret) (assoc :tokens-key (keys/derive secret :salt "tokens")) - (assoc :management-key (keys/derive secret :salt "management")) (update :instance-id handle-instance-id conn (db/read-only? pool))))))) (sm/register! ::props [:map-of :keyword ::sm/any]) + + +(defmethod ig/init-key ::shared-keys + [_ {:keys [::props] :as cfg}] + (let [secret (get props :secret-key)] + (d/without-nils + {"exporter" + (let [key (or (get cfg :exporter) + (-> (keys/derive secret :salt "exporter") + (bc/bytes->b64-str true)))] + (if (or (str/empty? key) + (str/blank? key)) + (do + (l/wrn :hint "exporter key is disabled because empty string found") + nil) + (do + (l/inf :hint "exporter key initialized" :key (d/obfuscate-string key)) + key))) + + "nitrate" + (let [key (or (get cfg :nitrate) + (-> (keys/derive secret :salt "nitrate") + (bc/bytes->b64-str true)))] + (if (or (str/empty? key) + (str/blank? key)) + (do + (l/wrn :hint "nitrate key is disabled because empty string found") + nil) + (do + (l/inf :hint "nitrate key initialized" :key (d/obfuscate-string key)) + key)))}))) + diff --git a/backend/test/backend_tests/http_middleware_test.clj b/backend/test/backend_tests/http_middleware_test.clj index 2d4b8ac029..2c313fef07 100644 --- a/backend/test/backend_tests/http_middleware_test.clj +++ b/backend/test/backend_tests/http_middleware_test.clj @@ -86,7 +86,7 @@ (t/deftest shared-key-auth (let [handler (#'app.http.middleware/wrap-shared-key-auth (fn [req] {::yres/status 200}) - "secret-key")] + {"test1" "secret-key"})] (let [response (handler (->DummyRequest {} {}))] (t/is (= 403 (::yres/status response)))) @@ -95,6 +95,9 @@ (t/is (= 403 (::yres/status response)))) (let [response (handler (->DummyRequest {"x-shared-key" "secret-key"} {}))] + (t/is (= 403 (::yres/status response)))) + + (let [response (handler (->DummyRequest {"x-shared-key" "test1 secret-key"} {}))] (t/is (= 200 (::yres/status response)))))) (t/deftest access-token-authz diff --git a/exporter/src/app/config.cljs b/exporter/src/app/config.cljs index 03f2b56b8a..2c421689c2 100644 --- a/exporter/src/app/config.cljs +++ b/exporter/src/app/config.cljs @@ -12,11 +12,14 @@ ["node:process" :as process] [app.common.data :as d] [app.common.flags :as flags] + [app.common.logging :as l] [app.common.schema :as sm] [app.common.version :as v] [cljs.core :as c] [cuerdas.core :as str])) +(l/set-level! :info) + (def ^:private defaults {:public-uri "http://localhost:3449" :tenant "default" @@ -30,7 +33,7 @@ [:map {:title "config"} [:secret-key :string] [:public-uri {:optional true} ::sm/uri] - [:management-api-key {:optional true} :string] + [:exporter-shared-key {:optional true} :string] [:host {:optional true} :string] [:tenant {:optional true} :string] [:flags {:optional true} [::sm/set :keyword]] @@ -98,8 +101,10 @@ (c/get config key default))) (def management-key - (or (c/get config :management-api-key) - (let [secret-key (c/get config :secret-key) - derived-key (crypto/hkdfSync "blake2b512" secret-key, "management" "" 32)] - (-> (.from buffer/Buffer derived-key) - (.toString "base64url"))))) + (let [key (or (c/get config :exporter-shared-key) + (let [secret-key (c/get config :secret-key) + derived-key (crypto/hkdfSync "blake2b512" secret-key, "exporter" "" 32)] + (-> (.from buffer/Buffer derived-key) + (.toString "base64url"))))] + (l/inf :hint "exporter key initialized" :key (d/obfuscate-string key)) + key)) diff --git a/exporter/src/app/handlers/resources.cljs b/exporter/src/app/handlers/resources.cljs index 5394e673a8..f0f655c498 100644 --- a/exporter/src/app/handlers/resources.cljs +++ b/exporter/src/app/handlers/resources.cljs @@ -73,7 +73,7 @@ (p/mcat (fn [blob] (let [fdata (new http/FormData) agent (new http/Agent #js {:connect #js {:rejectUnauthorized false}}) - headers #js {"X-Shared-Key" cf/management-key + headers #js {"X-Shared-Key" (str "exporter " cf/management-key) "Authorization" (str "Bearer " auth-token)} request #js {:headers headers