mirror of
https://github.com/penpot/penpot.git
synced 2026-09-24 21:06:14 +00:00
* ✨ Add storage operation metrics for S3 and buckets Expose Prometheus metrics for the object storage subsystem. The S3 backend now attaches an AWS SDK MetricPublisher that counts API calls, retries and latency per operation and target. The storage layer counts logical operations and deduplication outcomes per Penpot bucket, and the assets handlers count served requests per route. Closes #11676 AI-assisted-by: muse-spark-1.3-contributor * ✨ Fix storage metrics labels, errors and test gaps Address the review findings on the storage metrics commit. Label reads with the object's own backend, count failed asset serving as errors without swallowing them, and cover the failed S3 call, S3 asset path and permission-denied branches with tests. Also share the label helper and reuse the metrics test helper. Closes #11676 AI-assisted-by: muse-spark-1.3-contributor * ✨ Harden storage metrics and fill test gaps Address the second-round review findings on storage metrics. Unknown backends now fail explicitly and count as errors, exists stays paired with its dedup outcome, and the thumbnail, missing storage, expired reads, unknown buckets and write failure paths are covered by tests. Label coercion goes through the shared metrics helper. Closes #11676 AI-assisted-by: muse-spark-1.3-contributor * ✨ Harden storage metrics accuracy and coverage Address the full-branch review findings on storage metrics. Touch and delete emit only on changed rows, reads emit after the backend fetch, unknown backends fail explicitly, and tempfile mismatches count as unauthorized. Publisher nil policy, pairing rules and attempt semantics are documented and covered by tests. Closes #11676 AI-assisted-by: muse-spark-1.3-contributor * ✨ Address full-branch review findings on storage metrics Touch and delete resolve labels from the row, reads stay paired, failures are covered by tests, and logging, ranges and docs are tightened. Includes the label helper unit tests and the retries wording clarification. Closes #11676 AI-assisted-by: muse-spark-1.3-contributor * ⚡ Label touch and del metrics from UPDATE RETURNING The storage metrics change resolved metric labels for touch-object! and del-object! with an extra SELECT per id-based call. Since app.main instruments storage unconditionally, every GC collector and binfile import paid that extra round trip: deleting a team with 10k media objects doubled the storage_object statements exactly on the paths that already process the most rows. touch-object! and del-object! now take only the object id (UUID) and read the labels from the updated row itself via RETURNING id, backend, metadata: one statement, no pre-read, and labels that always match the row actually mutated. del-object! additionally guards on deleted_at IS NULL, so a repeated delete returns false and emits no metric. Also from the review of the full branch: extract the duplicated serve/emit/rethrow block in app.http.assets into one helper; give penpot_storage_s3_timing explicit histogram buckets up to 60s (the default cap at 7.5s hid the slow S3 calls the metric exists for); drop the unused ::target-id config key from the S3 backend and hardcode the :default target label until per-bucket routing lands. AI-assisted-by: glm-5.3-flash * ✨ Harden storage metric recording and definitions The metric definition schema is now closed and declares every key the collectors read: buckets, quantiles, max-age and reg. A typo such as a misspelled ::mdef/buckets used to compile and silently fall back to the default histogram buckets; it now fails at startup. The asset result-label fallback coerced an absent status to 500, so a future serve path without a status would have counted successes as errors. The mapping is now explicit and documented: served below 400, unauthorized for 401/403, not-found for 404, and error for everything else, including an absent status. The never-fail try/catch around metric recording existed four times with drift. One app.metrics/run-safe! helper replaces them: it no-ops on a nil metrics instance and logs the first failure per hint at warn level, then at debug, so a broken setup surfaces once without flooding the log. The S3 publisher keeps its outer try/catch: it is the SDK MetricPublisher contract boundary. AI-assisted-by: glm-5.3-flash * ✨ Make metrics mandatory and run! safe by default Recording a metric must never change the behavior of the operation being measured, so `run!` now catches recording failures itself: the first failure per metric id logs at warn, later ones at debug. This replaces the `run-safe!` helper, whose four copies had drifted, and applies the guarantee to every emit site instead of only storage. The metrics instance precondition is a plain assert, and the collector lookup stays outside the recording guard, so a missing instance fails hard even when asserts are disabled. Metrics is therefore no longer optional: the storage, s3-backend and db-pool schemas require `::mtx/metrics`, and the assets handler cfg always carries it. `wrap-publisher` no longer returns nil for a nil instance, and the db pool wires the prometheus tracker unconditionally. AI-assisted-by: deepseek-v4.1-flash
1195 lines
59 KiB
Clojure
1195 lines
59 KiB
Clojure
;; This Source Code Form is subject to the terms of the Mozilla Public
|
|
;; License, v. 2.0. If a copy of the MPL was not distributed with this
|
|
;; file, You can obtain one at http://mozilla.org/MPL/2.0/.
|
|
;;
|
|
;; Copyright (c) KALEIDOS SUBSIDIARY SL
|
|
|
|
(ns backend-tests.http-assets-test
|
|
(:require
|
|
[app.common.time :as ct]
|
|
[app.common.uri :as u]
|
|
[app.common.uuid :as uuid]
|
|
[app.config :as cf]
|
|
[app.db :as db]
|
|
[app.http :as-alias http]
|
|
[app.http.access-token :as actoken]
|
|
[app.http.assets :as assets]
|
|
[app.http.session :as session]
|
|
[app.main :as main]
|
|
[app.metrics :as mtx]
|
|
[app.metrics.definition :as-alias mdef]
|
|
[app.rpc :as-alias rpc]
|
|
[app.rpc.commands.access-token :as access-token]
|
|
[app.storage :as sto]
|
|
[backend-tests.helpers :as th]
|
|
[clojure.test :as t]
|
|
[datoteka.fs :as fs]
|
|
[integrant.core :as ig]
|
|
[mockery.core :refer [with-mocks]]
|
|
[yetti.response :as-alias yres])
|
|
(:import
|
|
io.prometheus.client.Counter
|
|
io.prometheus.client.Counter$Child))
|
|
|
|
(t/use-fixtures :once th/state-init)
|
|
(t/use-fixtures :each (th/serial
|
|
th/database-reset
|
|
th/clean-storage))
|
|
|
|
;; ----------------------------------------------------------------
|
|
;; Helpers
|
|
;; ----------------------------------------------------------------
|
|
|
|
(defn- configure-storage-backend
|
|
"Given storage map, returns a storage configured with the
|
|
appropriate backend for assets."
|
|
[storage]
|
|
(assoc storage ::sto/backend :fs))
|
|
|
|
(defn- create-storage-object!
|
|
"Create a storage object with the given bucket and content.
|
|
Optional opts map can include :profile-id to set the owner."
|
|
([storage bucket content]
|
|
(create-storage-object! storage bucket content {}))
|
|
([storage bucket content {:keys [profile-id]}]
|
|
(sto/put-object! storage (cond-> {::sto/content (sto/content content)
|
|
:bucket bucket
|
|
:content-type "text/plain"}
|
|
(some? profile-id)
|
|
(assoc :profile-id profile-id)))))
|
|
|
|
(defn- make-metrics
|
|
[]
|
|
(ig/init-key :app.metrics/metrics
|
|
{:default (select-keys main/default-metrics
|
|
[:storage-asset-requests])}))
|
|
|
|
(defn- make-handler-cfg
|
|
"Build a minimal cfg map for the assets handlers. It carries a metrics
|
|
instance because the handlers require one."
|
|
[storage]
|
|
{::sto/storage storage
|
|
::mtx/metrics (make-metrics)
|
|
::assets/path "/assets"})
|
|
|
|
(defn- make-metrics-cfg
|
|
"Build a handler cfg map with an isolated metrics instance."
|
|
[storage metrics]
|
|
(assoc (make-handler-cfg storage) ::mtx/metrics metrics))
|
|
|
|
(defn- counter-value
|
|
[metrics labels]
|
|
(let [collector (mtx/get-collector metrics :storage-asset-requests)
|
|
instance (::mdef/instance collector)
|
|
child (.labels ^Counter instance (into-array String labels))]
|
|
(.get ^Counter$Child child)))
|
|
|
|
;; ----------------------------------------------------------------
|
|
;; Tests: get-id
|
|
;; ----------------------------------------------------------------
|
|
|
|
(t/deftest get-id-with-valid-uuid
|
|
(let [id (uuid/next)
|
|
request {:path-params {:id (str id)}}
|
|
result (assets/get-id request)]
|
|
(t/is (= id result))))
|
|
|
|
(t/deftest get-id-with-invalid-uuid
|
|
(let [request {:path-params {:id "not-a-uuid"}}]
|
|
(try
|
|
(assets/get-id request)
|
|
(t/is false "should have thrown")
|
|
(catch Exception e
|
|
(t/is (= :not-found (:type (ex-data e))))))))
|
|
|
|
(t/deftest get-id-with-missing-id
|
|
(let [request {:path-params {}}]
|
|
(try
|
|
(assets/get-id request)
|
|
(t/is false "should have thrown")
|
|
(catch Exception e
|
|
(t/is (= :not-found (:type (ex-data e))))))))
|
|
|
|
;; ----------------------------------------------------------------
|
|
;; Tests: objects-handler — non-existent objects
|
|
;; ----------------------------------------------------------------
|
|
|
|
(t/deftest objects-handler-non-existent-object
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
cfg (make-handler-cfg storage)
|
|
request {:path-params {:id (str (uuid/next))}}
|
|
response (assets/objects-handler cfg request)]
|
|
(t/is (= 404 (::yres/status response)))))
|
|
|
|
(t/deftest objects-handler-invalid-uuid
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
cfg (make-handler-cfg storage)
|
|
request {:path-params {:id "not-a-uuid"}}]
|
|
(try
|
|
(assets/objects-handler cfg request)
|
|
(t/is false "should have thrown")
|
|
(catch Exception e
|
|
(t/is (= :not-found (:type (ex-data e))))))))
|
|
|
|
;; ----------------------------------------------------------------
|
|
;; Tests: objects-handler — public buckets (no auth required)
|
|
;; ----------------------------------------------------------------
|
|
|
|
(t/deftest objects-handler-public-bucket-no-auth
|
|
;; Objects in public buckets should be accessible without authentication.
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
cfg (make-handler-cfg storage)]
|
|
|
|
(doseq [bucket ["file-media-object"
|
|
"file-object-thumbnail"
|
|
"team-font-variant"
|
|
"file-data-fragment"
|
|
"organization"]]
|
|
(t/testing (str "bucket: " bucket)
|
|
(let [object (create-storage-object! storage bucket "public data")
|
|
request {:path-params {:id (str (:id object))}}
|
|
response (assets/objects-handler cfg request)]
|
|
(t/is (not= 401 (::yres/status response))
|
|
(str "bucket " bucket " should not require auth"))
|
|
(t/is (not= 404 (::yres/status response))
|
|
(str "bucket " bucket " object should exist")))))))
|
|
|
|
(t/deftest objects-handler-organization-logo-no-auth
|
|
;; Organization logos are embedded in unauthenticated contexts, such as
|
|
;; the invitation email image shown to a not-yet-registered invitee, so
|
|
;; they must be servable without a session or access token.
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
cfg (make-handler-cfg storage)
|
|
object (create-storage-object! storage "organization" "logo data")
|
|
request {:path-params {:id (str (:id object))}}
|
|
response (assets/objects-handler cfg request)]
|
|
(t/is (not= 401 (::yres/status response)))
|
|
(t/is (not= 404 (::yres/status response)))))
|
|
|
|
(t/deftest objects-handler-public-bucket-with-auth
|
|
;; Objects in public buckets should also be accessible WITH authentication.
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
cfg (make-handler-cfg storage)
|
|
profile (th/create-profile* 1)
|
|
object (create-storage-object! storage "file-media-object" "public data")
|
|
request {:path-params {:id (str (:id object))}
|
|
::session/profile-id (:id profile)}
|
|
response (assets/objects-handler cfg request)]
|
|
(t/is (not= 401 (::yres/status response)))
|
|
(t/is (not= 404 (::yres/status response)))))
|
|
|
|
;; ----------------------------------------------------------------
|
|
;; Tests: objects-handler — non-public buckets (auth required)
|
|
;; ----------------------------------------------------------------
|
|
|
|
(t/deftest objects-handler-file-thumbnail-bucket-link-preview-flag
|
|
;; Objects in the file-thumbnail bucket are public only when the
|
|
;; link-preview flag is enabled.
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
cfg (make-handler-cfg storage)
|
|
object (create-storage-object! storage "file-thumbnail" "thumbnail data")
|
|
request {:path-params {:id (str (:id object))}}]
|
|
|
|
(t/testing "flag enabled"
|
|
(with-redefs [cf/flags (conj cf/flags :link-preview)]
|
|
(let [response (assets/objects-handler cfg request)]
|
|
(t/is (not= 401 (::yres/status response)))
|
|
(t/is (not= 404 (::yres/status response))))))
|
|
|
|
(t/testing "flag disabled"
|
|
(with-redefs [cf/flags (disj cf/flags :link-preview)]
|
|
(let [response (assets/objects-handler cfg request)]
|
|
(t/is (= 401 (::yres/status response))))))))
|
|
|
|
(t/deftest objects-handler-non-public-bucket-no-auth
|
|
;; Objects in non-public buckets should return 401 without authentication.
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
cfg (make-handler-cfg storage)
|
|
object (create-storage-object! storage "profile" "profile photo")
|
|
request {:path-params {:id (str (:id object))}}
|
|
response (assets/objects-handler cfg request)]
|
|
(t/is (= 401 (::yres/status response)))))
|
|
|
|
(t/deftest objects-handler-non-public-bucket-with-session-auth
|
|
;; Objects in non-public buckets should be accessible with session auth.
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
cfg (make-handler-cfg storage)
|
|
profile (th/create-profile* 1)
|
|
object (create-storage-object! storage "profile" "profile photo")
|
|
request {:path-params {:id (str (:id object))}
|
|
::session/profile-id (:id profile)}
|
|
response (assets/objects-handler cfg request)]
|
|
(t/is (not= 401 (::yres/status response)))
|
|
(t/is (not= 404 (::yres/status response)))))
|
|
|
|
(t/deftest objects-handler-non-public-bucket-with-access-token-auth
|
|
;; Objects in non-public buckets should be accessible with access token auth.
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
cfg (make-handler-cfg storage)
|
|
profile (th/create-profile* 1)
|
|
object (create-storage-object! storage "profile" "profile photo")
|
|
request {:path-params {:id (str (:id object))}
|
|
::actoken/profile-id (:id profile)}
|
|
response (assets/objects-handler cfg request)]
|
|
(t/is (not= 401 (::yres/status response)))
|
|
(t/is (not= 404 (::yres/status response)))))
|
|
|
|
(t/deftest objects-handler-non-public-bucket-with-both-auth
|
|
;; Objects should be accessible when both session and access token auth are present.
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
cfg (make-handler-cfg storage)
|
|
profile (th/create-profile* 1)
|
|
object (create-storage-object! storage "profile" "profile photo")
|
|
request {:path-params {:id (str (:id object))}
|
|
::session/profile-id (:id profile)
|
|
::actoken/profile-id (:id profile)}
|
|
response (assets/objects-handler cfg request)]
|
|
(t/is (not= 401 (::yres/status response)))
|
|
(t/is (not= 404 (::yres/status response)))))
|
|
|
|
;; ----------------------------------------------------------------
|
|
;; Tests: objects-handler — all non-public buckets
|
|
;; ----------------------------------------------------------------
|
|
|
|
(t/deftest objects-handler-all-non-public-buckets-require-auth
|
|
;; Verify that all buckets NOT in the public set require authentication.
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
cfg (make-handler-cfg storage)
|
|
profile (th/create-profile* 1)]
|
|
|
|
;; NOTE: file-thumbnail is not included here because it is public
|
|
;; when the link-preview flag is enabled; see
|
|
;; objects-handler-file-thumbnail-bucket-link-preview-flag.
|
|
(doseq [bucket ["profile"
|
|
"tempfile"
|
|
"file-data"
|
|
"file-change"]]
|
|
(t/testing (str "bucket: " bucket)
|
|
(let [object (create-storage-object! storage bucket "some data")
|
|
request {:path-params {:id (str (:id object))}}]
|
|
|
|
;; Without auth → 401
|
|
(let [response (assets/objects-handler cfg request)]
|
|
(t/is (= 401 (::yres/status response))
|
|
(str "bucket " bucket " should require auth")))
|
|
|
|
;; With session auth → not 401
|
|
(let [response (assets/objects-handler cfg (assoc request ::session/profile-id (:id profile)))]
|
|
(t/is (not= 401 (::yres/status response))
|
|
(str "bucket " bucket " should be accessible with session auth")))
|
|
|
|
;; With access token auth → not 401
|
|
(let [response (assets/objects-handler cfg (assoc request ::actoken/profile-id (:id profile)))]
|
|
(t/is (not= 401 (::yres/status response))
|
|
(str "bucket " bucket " should be accessible with access token auth"))))))))
|
|
|
|
;; ----------------------------------------------------------------
|
|
;; Tests: objects-handler — serve-object response (FS backend)
|
|
;; ----------------------------------------------------------------
|
|
|
|
(t/deftest objects-handler-fs-backend-serves-object
|
|
;; Verify that the FS backend returns the correct response structure.
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
cfg (make-handler-cfg storage)
|
|
profile (th/create-profile* 1)
|
|
object (create-storage-object! storage "file-media-object" "file content")
|
|
request {:path-params {:id (str (:id object))}
|
|
::session/profile-id (:id profile)}
|
|
response (assets/objects-handler cfg request)]
|
|
;; FS backend returns 204 with x-accel-redirect header
|
|
(t/is (= 204 (::yres/status response)))
|
|
(t/is (some? (get (::yres/headers response) "x-accel-redirect")))
|
|
(t/is (= "text/plain" (get (::yres/headers response) "content-type")))
|
|
(t/is (some? (get (::yres/headers response) "cache-control")))))
|
|
|
|
(t/deftest objects-handler-fs-backend-accel-redirect-path
|
|
;; Verify that x-accel-redirect contains the object's relative path.
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
cfg (make-handler-cfg storage)
|
|
object (create-storage-object! storage "file-media-object" "file content")
|
|
request {:path-params {:id (str (:id object))}}
|
|
response (assets/objects-handler cfg request)
|
|
redirect (get (::yres/headers response) "x-accel-redirect")]
|
|
;; The redirect path should contain the object's relative path
|
|
(t/is (string? redirect))
|
|
(t/is (clojure.string/includes? redirect (sto/object->relative-path object)))))
|
|
|
|
;; ----------------------------------------------------------------
|
|
;; Tests: objects-handler — content disposition
|
|
;; ----------------------------------------------------------------
|
|
|
|
(t/deftest objects-handler-non-public-bucket-served-as-attachment
|
|
;; A non-public bucket holds bytes the user uploaded and is reachable by
|
|
;; direct navigation, so the response marks it as an attachment.
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
cfg (make-handler-cfg storage)
|
|
profile (th/create-profile* 1)]
|
|
|
|
(doseq [bucket ["profile"
|
|
"tempfile"
|
|
"file-data"
|
|
"file-thumbnail"
|
|
"file-change"]]
|
|
(t/testing (str "bucket: " bucket)
|
|
(let [object (create-storage-object! storage bucket "some data")
|
|
request {:path-params {:id (str (:id object))}
|
|
::session/profile-id (:id profile)}
|
|
response (assets/objects-handler cfg request)]
|
|
(t/is (= "attachment" (get (::yres/headers response) "content-disposition"))
|
|
(str "bucket " bucket " should be served as an attachment")))))))
|
|
|
|
(t/deftest objects-handler-public-bucket-served-inline
|
|
;; Public buckets are embedded by the viewer and by outgoing mail, so they
|
|
;; keep being served without a disposition.
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
cfg (make-handler-cfg storage)]
|
|
|
|
(doseq [bucket ["file-media-object"
|
|
"file-object-thumbnail"
|
|
"team-font-variant"
|
|
"file-data-fragment"
|
|
"organization"]]
|
|
(t/testing (str "bucket: " bucket)
|
|
(let [object (create-storage-object! storage bucket "some data")
|
|
request {:path-params {:id (str (:id object))}}
|
|
response (assets/objects-handler cfg request)]
|
|
(t/is (nil? (get (::yres/headers response) "content-disposition"))
|
|
(str "bucket " bucket " should stay inline")))))))
|
|
|
|
;; ----------------------------------------------------------------
|
|
;; Tests: objects-handler — cache headers
|
|
;; ----------------------------------------------------------------
|
|
|
|
(t/deftest objects-handler-cache-control-header
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
cfg (make-handler-cfg storage)
|
|
object (create-storage-object! storage "file-media-object" "content")
|
|
request {:path-params {:id (str (:id object))}}
|
|
response (assets/objects-handler cfg request)
|
|
cc (get (::yres/headers response) "cache-control")]
|
|
(t/is (string? cc))
|
|
(t/is (clojure.string/starts-with? cc "max-age="))))
|
|
|
|
;; ----------------------------------------------------------------
|
|
;; Tests: middleware integration — session auth end-to-end
|
|
;; ----------------------------------------------------------------
|
|
|
|
(t/deftest session-auth-integration
|
|
;; Test the full session auth flow: create session → assign token →
|
|
;; authenticate request → access protected asset.
|
|
(let [cfg th/*system*
|
|
manager (session/inmemory-manager)
|
|
profile (th/create-profile* 1)
|
|
|
|
;; Create a session and generate a token
|
|
session (->> (session/create-session manager {:profile-id (:id profile)
|
|
:user-agent "test-agent"})
|
|
(#'session/assign-token cfg))
|
|
|
|
;; Create a storage object in a non-public bucket
|
|
storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
object (create-storage-object! storage "profile" "profile data")
|
|
|
|
;; Simulate what the middleware chain does:
|
|
;; 1. mw/auth extracts token from cookie and sets ::http/auth-data
|
|
;; 2. session/authz reads ::http/auth-data and sets ::session/profile-id
|
|
request {::http/auth-data {:type :cookie
|
|
:token (:token session)
|
|
:claims {:sid (:id session)
|
|
:uid (:id profile)}
|
|
:metadata {:ver 1}}
|
|
:path-params {:id (str (:id object))}}
|
|
|
|
;; Apply session/authz middleware
|
|
handler (#'session/wrap-authz
|
|
(fn [req]
|
|
;; This is where the actual handler would be called
|
|
;; We verify that ::session/profile-id is set
|
|
req)
|
|
{::session/manager manager})
|
|
result (handler request)]
|
|
|
|
;; Verify the session auth set the profile-id
|
|
(t/is (= (:id profile) (::session/profile-id result)))
|
|
(t/is (some? (::session/session result)))))
|
|
|
|
;; ----------------------------------------------------------------
|
|
;; Tests: middleware integration — access token auth end-to-end
|
|
;; ----------------------------------------------------------------
|
|
|
|
(t/deftest access-token-auth-integration
|
|
;; Test the full access token flow: create token → authenticate
|
|
;; request → access protected asset.
|
|
(let [profile (th/create-profile* 1)
|
|
|
|
;; Create an access token in the database
|
|
atoken (db/tx-run! th/*system*
|
|
access-token/create-access-token
|
|
(:id profile) "test-token" nil nil)
|
|
|
|
;; Create a storage object in a non-public bucket
|
|
storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
object (create-storage-object! storage "profile" "profile data")
|
|
|
|
;; Simulate what the middleware chain does:
|
|
;; 1. mw/auth extracts token from Authorization header and sets ::http/auth-data
|
|
;; 2. actoken/authz reads ::http/auth-data and sets ::actoken/profile-id
|
|
request {::http/auth-data {:type :token
|
|
:token (:token atoken)
|
|
:claims {:tid (:id atoken)}}
|
|
:path-params {:id (str (:id object))}}
|
|
|
|
;; Apply actoken/authz middleware
|
|
handler (#'actoken/wrap-authz
|
|
(fn [req]
|
|
;; Verify that ::actoken/profile-id is set
|
|
req)
|
|
th/*system*)
|
|
result (handler request)]
|
|
|
|
;; Verify the access token auth set the profile-id
|
|
(t/is (= (:id profile) (::actoken/profile-id result)))))
|
|
|
|
;; ----------------------------------------------------------------
|
|
;; Tests: middleware chain — combined session + access token
|
|
;; ----------------------------------------------------------------
|
|
|
|
(t/deftest combined-middleware-chain
|
|
;; Test that both session/authz and actoken/authz work together
|
|
;; in the middleware chain, matching the assets route configuration.
|
|
(let [cfg th/*system*
|
|
manager (session/inmemory-manager)
|
|
profile (th/create-profile* 1)
|
|
|
|
;; Create a session
|
|
session (->> (session/create-session manager {:profile-id (:id profile)
|
|
:user-agent "test-agent"})
|
|
(#'session/assign-token cfg))
|
|
|
|
;; Create an access token
|
|
atoken (db/tx-run! th/*system*
|
|
access-token/create-access-token
|
|
(:id profile) "test-token" nil nil)
|
|
|
|
;; Build the middleware chain like assets routes do:
|
|
;; session/authz → actoken/authz → handler
|
|
inner-handler (fn [request] request)
|
|
with-actoken (#'actoken/wrap-authz inner-handler th/*system*)
|
|
with-session (#'session/wrap-authz with-actoken {::session/manager manager})]
|
|
|
|
(t/testing "session cookie auth sets ::session/profile-id"
|
|
(let [request {::http/auth-data {:type :cookie
|
|
:token (:token session)
|
|
:claims {:sid (:id session)
|
|
:uid (:id profile)}
|
|
:metadata {:ver 1}}}
|
|
result (with-session request)]
|
|
(t/is (= (:id profile) (::session/profile-id result)))))
|
|
|
|
(t/testing "access token auth sets ::actoken/profile-id"
|
|
(let [request {::http/auth-data {:type :token
|
|
:token (:token atoken)
|
|
:claims {:tid (:id atoken)}}}
|
|
result (with-session request)]
|
|
(t/is (= (:id profile) (::actoken/profile-id result)))))
|
|
|
|
(t/testing "no auth sets neither profile-id"
|
|
(let [request {}
|
|
result (with-session request)]
|
|
(t/is (nil? (::session/profile-id result)))
|
|
(t/is (nil? (::actoken/profile-id result)))))))
|
|
|
|
;; ----------------------------------------------------------------
|
|
;; Tests: objects-handler — edge cases
|
|
;; ----------------------------------------------------------------
|
|
|
|
(t/deftest objects-handler-nil-profile-id-in-session
|
|
;; When session auth is present but profile-id is nil (e.g. invalid session),
|
|
;; non-public objects should still be denied.
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
cfg (make-handler-cfg storage)
|
|
object (create-storage-object! storage "profile" "data")
|
|
request {:path-params {:id (str (:id object))}
|
|
::session/profile-id nil}
|
|
response (assets/objects-handler cfg request)]
|
|
(t/is (= 401 (::yres/status response)))))
|
|
|
|
(t/deftest objects-handler-nil-profile-id-in-access-token
|
|
;; When access token auth is present but profile-id is nil,
|
|
;; non-public objects should still be denied.
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
cfg (make-handler-cfg storage)
|
|
object (create-storage-object! storage "profile" "data")
|
|
request {:path-params {:id (str (:id object))}
|
|
::actoken/profile-id nil}
|
|
response (assets/objects-handler cfg request)]
|
|
(t/is (= 401 (::yres/status response)))))
|
|
|
|
(t/deftest objects-handler-empty-request
|
|
;; A request with no path-params should raise a not-found error.
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
cfg (make-handler-cfg storage)
|
|
request {}]
|
|
(try
|
|
(assets/objects-handler cfg request)
|
|
(t/is false "should have thrown")
|
|
(catch Exception e
|
|
(t/is (= :not-found (:type (ex-data e))))))))
|
|
|
|
;; ----------------------------------------------------------------
|
|
;; Tests: objects-handler — expired objects
|
|
;; ----------------------------------------------------------------
|
|
|
|
;; ----------------------------------------------------------------
|
|
;; Tests: file-objects-handler — authz required (T2-N1-01)
|
|
;; ----------------------------------------------------------------
|
|
|
|
(t/deftest file-objects-handler-unauthenticated-returns-404
|
|
;; Unauthenticated requests to file-media assets must return 404
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
cfg (make-handler-cfg storage)
|
|
profile (th/create-profile* 1)
|
|
team (th/create-team* 1 {:profile-id (:id profile)})
|
|
project (th/create-project* 1 {:profile-id (:id profile)
|
|
:team-id (:id team)})
|
|
file (th/create-file* 1 {:profile-id (:id profile)
|
|
:project-id (:id project)})
|
|
media-storage (create-storage-object! storage "file-media-object" "image data")
|
|
media-obj (th/create-file-media-object* {:file-id (:id file)
|
|
:media-id (:id media-storage)})
|
|
request {:path-params {:id (str (:id media-obj))}}
|
|
response (assets/file-objects-handler cfg request)]
|
|
(t/is (= 404 (::yres/status response)))))
|
|
|
|
(t/deftest file-objects-handler-no-file-perms-returns-404
|
|
;; Authenticated user without file read permissions must get 404
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
cfg (make-handler-cfg storage)
|
|
owner (th/create-profile* 1)
|
|
team (th/create-team* 1 {:profile-id (:id owner)})
|
|
project (th/create-project* 1 {:profile-id (:id owner)
|
|
:team-id (:id team)})
|
|
file (th/create-file* 1 {:profile-id (:id owner)
|
|
:project-id (:id project)})
|
|
media-storage (create-storage-object! storage "file-media-object" "image data")
|
|
media-obj (th/create-file-media-object* {:file-id (:id file)
|
|
:media-id (:id media-storage)})
|
|
stranger (th/create-profile* 2)
|
|
request {:path-params {:id (str (:id media-obj))}
|
|
::session/profile-id (:id stranger)}
|
|
response (assets/file-objects-handler cfg request)]
|
|
(t/is (= 404 (::yres/status response)))))
|
|
|
|
(t/deftest file-objects-handler-with-file-perms-succeeds
|
|
;; Authenticated user with file read permissions must get the object
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
cfg (make-handler-cfg storage)
|
|
owner (th/create-profile* 1)
|
|
team (th/create-team* 1 {:profile-id (:id owner)})
|
|
project (th/create-project* 1 {:profile-id (:id owner)
|
|
:team-id (:id team)})
|
|
file (th/create-file* 1 {:profile-id (:id owner)
|
|
:project-id (:id project)})
|
|
media-storage (create-storage-object! storage "file-media-object" "image data")
|
|
media-obj (th/create-file-media-object* {:file-id (:id file)
|
|
:media-id (:id media-storage)})
|
|
request {:path-params {:id (str (:id media-obj))}
|
|
::session/profile-id (:id owner)}
|
|
response (assets/file-objects-handler cfg request)]
|
|
(t/is (= 204 (::yres/status response)))))
|
|
|
|
(t/deftest file-thumbnails-handler-unauthenticated-returns-404
|
|
;; Unauthenticated requests to file-thumbnail assets must return 404
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
cfg (make-handler-cfg storage)
|
|
profile (th/create-profile* 1)
|
|
team (th/create-team* 1 {:profile-id (:id profile)})
|
|
project (th/create-project* 1 {:profile-id (:id profile)
|
|
:team-id (:id team)})
|
|
file (th/create-file* 1 {:profile-id (:id profile)
|
|
:project-id (:id project)})
|
|
media-storage (create-storage-object! storage "file-media-object" "image data")
|
|
media-obj (th/create-file-media-object* {:file-id (:id file)
|
|
:media-id (:id media-storage)})
|
|
request {:path-params {:id (str (:id media-obj))}}
|
|
response (assets/file-thumbnails-handler cfg request)]
|
|
(t/is (= 404 (::yres/status response)))))
|
|
|
|
(t/deftest file-thumbnails-handler-with-file-perms-succeeds
|
|
;; Authenticated user with file read permissions must get the thumbnail
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
cfg (make-handler-cfg storage)
|
|
owner (th/create-profile* 1)
|
|
team (th/create-team* 1 {:profile-id (:id owner)})
|
|
project (th/create-project* 1 {:profile-id (:id owner)
|
|
:team-id (:id team)})
|
|
file (th/create-file* 1 {:profile-id (:id owner)
|
|
:project-id (:id project)})
|
|
thumb-storage (create-storage-object! storage "file-object-thumbnail" "thumb data")
|
|
media-obj (th/create-file-media-object* {:file-id (:id file)
|
|
:media-id (:id thumb-storage)})
|
|
request {:path-params {:id (str (:id media-obj))}
|
|
::session/profile-id (:id owner)}
|
|
response (assets/file-thumbnails-handler cfg request)]
|
|
;; Falls back to media-id since no thumbnail-id, but still serves
|
|
(t/is (= 204 (::yres/status response)))))
|
|
|
|
(t/deftest file-objects-handler-non-existent-media-returns-404
|
|
;; Request for non-existent file-media-object returns 404
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
cfg (make-handler-cfg storage)
|
|
profile (th/create-profile* 1)
|
|
request {:path-params {:id (str (uuid/next))}
|
|
::session/profile-id (:id profile)}
|
|
response (assets/file-objects-handler cfg request)]
|
|
(t/is (= 404 (::yres/status response)))))
|
|
|
|
(t/deftest file-objects-handler-nil-profile-id-returns-404
|
|
;; When profile-id is nil (invalid session), must return 404
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
cfg (make-handler-cfg storage)
|
|
profile (th/create-profile* 1)
|
|
team (th/create-team* 1 {:profile-id (:id profile)})
|
|
project (th/create-project* 1 {:profile-id (:id profile)
|
|
:team-id (:id team)})
|
|
file (th/create-file* 1 {:profile-id (:id profile)
|
|
:project-id (:id project)})
|
|
media-storage (create-storage-object! storage "file-media-object" "image data")
|
|
media-obj (th/create-file-media-object* {:file-id (:id file)
|
|
:media-id (:id media-storage)})
|
|
request {:path-params {:id (str (:id media-obj))}
|
|
::session/profile-id nil}
|
|
response (assets/file-objects-handler cfg request)]
|
|
(t/is (= 404 (::yres/status response)))))
|
|
|
|
;; ----------------------------------------------------------------
|
|
;; Tests: file-objects-handler — share-link authz (issue #11338)
|
|
;; ----------------------------------------------------------------
|
|
|
|
(t/deftest file-objects-handler-anonymous-with-valid-share-id-succeeds
|
|
;; Anonymous request with a valid share-id matching the file must
|
|
;; succeed (share-link viewers are unauthenticated by definition).
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
cfg (make-handler-cfg storage)
|
|
owner (th/create-profile* 1)
|
|
team (th/create-team* 1 {:profile-id (:id owner)})
|
|
project (th/create-project* 1 {:profile-id (:id owner)
|
|
:team-id (:id team)})
|
|
file (th/create-file* 1 {:profile-id (:id owner)
|
|
:project-id (:id project)})
|
|
media-storage (create-storage-object! storage "file-media-object" "image data")
|
|
media-obj (th/create-file-media-object* {:file-id (:id file)
|
|
:media-id (:id media-storage)})
|
|
slink (:result (th/command! {::th/type :create-share-link
|
|
::rpc/profile-id (:id owner)
|
|
:file-id (:id file)
|
|
:pages #{}
|
|
:who-comment "team"
|
|
:who-inspect "all"}))
|
|
request {:path-params {:id (str (:id media-obj))}
|
|
:query-params {:share-id (str (:id slink))}}
|
|
response (assets/file-objects-handler cfg request)]
|
|
(t/is (= 204 (::yres/status response)))))
|
|
|
|
(t/deftest file-objects-handler-anonymous-with-share-id-for-other-file-returns-404
|
|
;; A share-id from file A must not grant access to assets of file B.
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
cfg (make-handler-cfg storage)
|
|
owner (th/create-profile* 1)
|
|
team (th/create-team* 1 {:profile-id (:id owner)})
|
|
project (th/create-project* 1 {:profile-id (:id owner)
|
|
:team-id (:id team)})
|
|
file-a (th/create-file* 1 {:profile-id (:id owner)
|
|
:project-id (:id project)})
|
|
file-b (th/create-file* 2 {:profile-id (:id owner)
|
|
:project-id (:id project)})
|
|
media-a (create-storage-object! storage "file-media-object" "image A")
|
|
media-obj-a (th/create-file-media-object* {:file-id (:id file-a)
|
|
:media-id (:id media-a)})
|
|
media-b (create-storage-object! storage "file-media-object" "image B")
|
|
media-obj-b (th/create-file-media-object* {:file-id (:id file-b)
|
|
:media-id (:id media-b)})
|
|
slink (:result (th/command! {::th/type :create-share-link
|
|
::rpc/profile-id (:id owner)
|
|
:file-id (:id file-a)
|
|
:pages #{}
|
|
:who-comment "team"
|
|
:who-inspect "all"}))
|
|
request {:path-params {:id (str (:id media-obj-b))}
|
|
:query-params {:share-id (str (:id slink))}}
|
|
response (assets/file-objects-handler cfg request)]
|
|
(t/is (= 404 (::yres/status response)))))
|
|
|
|
(t/deftest file-objects-handler-anonymous-with-malformed-share-id-returns-404
|
|
;; Malformed share-id must not raise; it must short-circuit to 404.
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
cfg (make-handler-cfg storage)
|
|
profile (th/create-profile* 1)
|
|
team (th/create-team* 1 {:profile-id (:id profile)})
|
|
project (th/create-project* 1 {:profile-id (:id profile)
|
|
:team-id (:id team)})
|
|
file (th/create-file* 1 {:profile-id (:id profile)
|
|
:project-id (:id project)})
|
|
media-storage (create-storage-object! storage "file-media-object" "image data")
|
|
media-obj (th/create-file-media-object* {:file-id (:id file)
|
|
:media-id (:id media-storage)})
|
|
request {:path-params {:id (str (:id media-obj))}
|
|
:query-params {:share-id "not-a-uuid"}}
|
|
response (assets/file-objects-handler cfg request)]
|
|
(t/is (= 404 (::yres/status response)))))
|
|
|
|
(t/deftest file-thumbnails-handler-anonymous-with-valid-share-id-succeeds
|
|
;; Thumbnail endpoint must also honor the share-id query param.
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
cfg (make-handler-cfg storage)
|
|
owner (th/create-profile* 1)
|
|
team (th/create-team* 1 {:profile-id (:id owner)})
|
|
project (th/create-project* 1 {:profile-id (:id owner)
|
|
:team-id (:id team)})
|
|
file (th/create-file* 1 {:profile-id (:id owner)
|
|
:project-id (:id project)})
|
|
thumb-storage (create-storage-object! storage "file-object-thumbnail" "thumb data")
|
|
media-obj (th/create-file-media-object* {:file-id (:id file)
|
|
:media-id (:id thumb-storage)})
|
|
slink (:result (th/command! {::th/type :create-share-link
|
|
::rpc/profile-id (:id owner)
|
|
:file-id (:id file)
|
|
:pages #{}
|
|
:who-comment "team"
|
|
:who-inspect "all"}))
|
|
request {:path-params {:id (str (:id media-obj))}
|
|
:query-params {:share-id (str (:id slink))}}
|
|
response (assets/file-thumbnails-handler cfg request)]
|
|
;; Falls back to media-id since no thumbnail-id, but still serves
|
|
(t/is (= 204 (::yres/status response)))))
|
|
|
|
(t/deftest objects-handler-expired-object
|
|
;; Expired objects should return 404 (get-object filters them out).
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
cfg (make-handler-cfg storage)
|
|
profile (th/create-profile* 1)
|
|
object (sto/put-object! storage {::sto/content (sto/content "expired")
|
|
::sto/expired-at (ct/now)
|
|
:bucket "profile"
|
|
:content-type "text/plain"})
|
|
request {:path-params {:id (str (:id object))}
|
|
::session/profile-id (:id profile)}
|
|
response (assets/objects-handler cfg request)]
|
|
(t/is (= 404 (::yres/status response)))))
|
|
|
|
;; ----------------------------------------------------------------
|
|
;; Tests: objects-handler — tempfile bucket ownership (T9-F-10)
|
|
;; ----------------------------------------------------------------
|
|
|
|
(t/deftest objects-handler-tempfile-owner-can-access
|
|
;; Owner of a tempfile should be able to access it via session auth.
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
cfg (make-handler-cfg storage)
|
|
owner (th/create-profile* 1)
|
|
object (create-storage-object! storage "tempfile" "temp data" {:profile-id (:id owner)})
|
|
request {:path-params {:id (str (:id object))}
|
|
::session/profile-id (:id owner)}
|
|
response (assets/objects-handler cfg request)]
|
|
(t/is (= 204 (::yres/status response)))))
|
|
|
|
(t/deftest objects-handler-tempfile-non-owner-gets-404
|
|
;; Non-owner accessing a tempfile should get 404 (not 403, to avoid leaking existence).
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
cfg (make-handler-cfg storage)
|
|
owner (th/create-profile* 1)
|
|
stranger (th/create-profile* 2)
|
|
object (create-storage-object! storage "tempfile" "temp data" {:profile-id (:id owner)})
|
|
request {:path-params {:id (str (:id object))}
|
|
::session/profile-id (:id stranger)}
|
|
response (assets/objects-handler cfg request)]
|
|
(t/is (= 404 (::yres/status response)))))
|
|
|
|
(t/deftest objects-handler-tempfile-access-token-owner-can-access
|
|
;; Owner of a tempfile should be able to access it via access token auth.
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
cfg (make-handler-cfg storage)
|
|
owner (th/create-profile* 1)
|
|
object (create-storage-object! storage "tempfile" "temp data" {:profile-id (:id owner)})
|
|
request {:path-params {:id (str (:id object))}
|
|
::actoken/profile-id (:id owner)}
|
|
response (assets/objects-handler cfg request)]
|
|
(t/is (= 204 (::yres/status response)))))
|
|
|
|
(t/deftest objects-handler-tempfile-access-token-non-owner-gets-404
|
|
;; Non-owner accessing a tempfile via access token should get 404.
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
cfg (make-handler-cfg storage)
|
|
owner (th/create-profile* 1)
|
|
stranger (th/create-profile* 2)
|
|
object (create-storage-object! storage "tempfile" "temp data" {:profile-id (:id owner)})
|
|
request {:path-params {:id (str (:id object))}
|
|
::actoken/profile-id (:id stranger)}
|
|
response (assets/objects-handler cfg request)]
|
|
(t/is (= 404 (::yres/status response)))))
|
|
|
|
(t/deftest objects-handler-tempfile-no-stored-profile-id-serves
|
|
;; Legacy tempfile objects without stored profile-id should be accessible
|
|
;; to any authenticated user (backward compatibility).
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
cfg (make-handler-cfg storage)
|
|
stranger (th/create-profile* 1)
|
|
object (create-storage-object! storage "tempfile" "legacy temp data")
|
|
request {:path-params {:id (str (:id object))}
|
|
::session/profile-id (:id stranger)}
|
|
response (assets/objects-handler cfg request)]
|
|
(t/is (= 204 (::yres/status response)))))
|
|
|
|
;; ----------------------------------------------------------------
|
|
;; Tests: asset request metrics
|
|
;; ----------------------------------------------------------------
|
|
|
|
(t/deftest objects-handler-emits-served-metric
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
metrics (make-metrics)
|
|
cfg (make-metrics-cfg storage metrics)
|
|
object (create-storage-object! storage "file-media-object" "file content")
|
|
request {:path-params {:id (str (:id object))}}
|
|
response (assets/objects-handler cfg request)]
|
|
(t/is (= 204 (::yres/status response)))
|
|
(t/is (= 1.0 (counter-value metrics ["by-id" "fs" "file-media-object" "served"])))))
|
|
|
|
(t/deftest objects-handler-emits-not-found-metric
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
metrics (make-metrics)
|
|
cfg (make-metrics-cfg storage metrics)
|
|
request {:path-params {:id (str (uuid/next))}}
|
|
response (assets/objects-handler cfg request)]
|
|
(t/is (= 404 (::yres/status response)))
|
|
(t/is (= 1.0 (counter-value metrics ["by-id" "unknown" "unknown" "not-found"])))))
|
|
|
|
(t/deftest objects-handler-emits-unauthorized-metric
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
metrics (make-metrics)
|
|
cfg (make-metrics-cfg storage metrics)
|
|
object (create-storage-object! storage "profile" "profile photo")
|
|
request {:path-params {:id (str (:id object))}}
|
|
response (assets/objects-handler cfg request)]
|
|
(t/is (= 401 (::yres/status response)))
|
|
(t/is (= 1.0 (counter-value metrics ["by-id" "fs" "profile" "unauthorized"])))))
|
|
|
|
(t/deftest file-objects-handler-emits-route-metric
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
metrics (make-metrics)
|
|
cfg (make-metrics-cfg storage metrics)
|
|
owner (th/create-profile* 1)
|
|
team (th/create-team* 1 {:profile-id (:id owner)})
|
|
project (th/create-project* 1 {:profile-id (:id owner)
|
|
:team-id (:id team)})
|
|
file (th/create-file* 1 {:profile-id (:id owner)
|
|
:project-id (:id project)})
|
|
media-storage (create-storage-object! storage "file-media-object" "image data")
|
|
media-obj (th/create-file-media-object* {:file-id (:id file)
|
|
:media-id (:id media-storage)})
|
|
request {:path-params {:id (str (:id media-obj))}
|
|
::session/profile-id (:id owner)}
|
|
response (assets/file-objects-handler cfg request)]
|
|
(t/is (= 204 (::yres/status response)))
|
|
(t/is (= 1.0 (counter-value metrics ["by-file-media-id" "fs" "file-media-object" "served"])))))
|
|
|
|
(t/deftest file-objects-handler-no-perms-emits-unauthorized-metric
|
|
;; Permission-denied file-media requests answer 404 but are counted as
|
|
;; unauthorized (no existence is leaked over HTTP).
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
metrics (make-metrics)
|
|
cfg (make-metrics-cfg storage metrics)
|
|
owner (th/create-profile* 1)
|
|
team (th/create-team* 1 {:profile-id (:id owner)})
|
|
project (th/create-project* 1 {:profile-id (:id owner)
|
|
:team-id (:id team)})
|
|
file (th/create-file* 1 {:profile-id (:id owner)
|
|
:project-id (:id project)})
|
|
media-storage (create-storage-object! storage "file-media-object" "image data")
|
|
media-obj (th/create-file-media-object* {:file-id (:id file)
|
|
:media-id (:id media-storage)})
|
|
stranger (th/create-profile* 2)
|
|
request {:path-params {:id (str (:id media-obj))}
|
|
::session/profile-id (:id stranger)}
|
|
response (assets/file-objects-handler cfg request)]
|
|
(t/is (= 404 (::yres/status response)))
|
|
(t/is (= 1.0 (counter-value metrics ["by-file-media-id" "unknown" "unknown" "unauthorized"])))
|
|
(t/is (= 0.0 (counter-value metrics ["by-file-media-id" "unknown" "unknown" "not-found"])))))
|
|
|
|
(t/deftest asset-requests-default-metrics-definition
|
|
(let [defs main/default-metrics]
|
|
(t/is (= "penpot_storage_asset_requests_total" (::mdef/name (:storage-asset-requests defs))))
|
|
(t/is (= ["route" "backend" "bucket" "result"] (::mdef/labels (:storage-asset-requests defs))))))
|
|
|
|
(t/deftest objects-handler-serve-failure-emits-error-and-rethrows
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
metrics (make-metrics)
|
|
cfg (make-metrics-cfg storage metrics)
|
|
object (create-storage-object! storage "file-media-object" "file content")
|
|
request {:path-params {:id (str (:id object))}}]
|
|
(with-mocks [_mock {:target 'app.storage/object->relative-path
|
|
:throw (ex-info "boom" {})}]
|
|
(t/is (thrown? clojure.lang.ExceptionInfo
|
|
(assets/objects-handler cfg request))))
|
|
(t/is (= 1.0 (counter-value metrics ["by-id" "fs" "file-media-object" "error"])))
|
|
(t/is (= 0.0 (counter-value metrics ["by-id" "fs" "file-media-object" "served"])))))
|
|
|
|
(t/deftest objects-handler-s3-backend-emits-served-metric
|
|
;; The S3 path is exercised without a real object store: the row is
|
|
;; inserted directly and the presigned URL is mocked.
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
metrics (make-metrics)
|
|
cfg (make-metrics-cfg storage metrics)
|
|
id (uuid/next)]
|
|
(db/insert! th/*pool* :storage-object
|
|
{:id id
|
|
:size 9
|
|
:backend "s3"
|
|
:metadata (db/tjson {:bucket "file-media-object"
|
|
:content-type "text/plain"})
|
|
:status "valid"})
|
|
(with-mocks [_mock {:target 'app.storage/get-object-url
|
|
:return (fn [_ _ & _] (u/uri "https://example.invalid/object"))}]
|
|
(let [response (assets/objects-handler cfg {:path-params {:id (str id)}})]
|
|
(t/is (= 307 (::yres/status response)))))
|
|
(t/is (= 1.0 (counter-value metrics ["by-id" "s3" "file-media-object" "served"])))))
|
|
|
|
(t/deftest objects-handler-unknown-backend-raises-and-emits-error
|
|
;; A row with an unexpected backend fails explicitly instead of
|
|
;; returning nil to the router; the failure is counted and rethrown.
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
metrics (make-metrics)
|
|
cfg (make-metrics-cfg storage metrics)
|
|
id (uuid/next)]
|
|
(db/insert! th/*pool* :storage-object
|
|
{:id id
|
|
:size 9
|
|
:backend "bogus"
|
|
:metadata (db/tjson {:bucket "file-media-object"
|
|
:content-type "text/plain"})
|
|
:status "valid"})
|
|
(t/is (thrown? clojure.lang.ExceptionInfo
|
|
(assets/objects-handler cfg {:path-params {:id (str id)}})))
|
|
(t/is (= 1.0 (counter-value metrics ["by-id" "bogus" "file-media-object" "error"])))))
|
|
|
|
(t/deftest file-thumbnails-handler-emits-thumbnail-route-metric
|
|
;; Served through the real thumbnail-id path (not the media-id fallback).
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
metrics (make-metrics)
|
|
cfg (make-metrics-cfg storage metrics)
|
|
owner (th/create-profile* 1)
|
|
team (th/create-team* 1 {:profile-id (:id owner)})
|
|
project (th/create-project* 1 {:profile-id (:id owner)
|
|
:team-id (:id team)})
|
|
file (th/create-file* 1 {:profile-id (:id owner)
|
|
:project-id (:id project)})
|
|
media-storage (create-storage-object! storage "file-media-object" "image data")
|
|
thumb-storage (create-storage-object! storage "file-object-thumbnail" "thumb data")
|
|
media-obj (th/create-file-media-object* {:file-id (:id file)
|
|
:media-id (:id media-storage)})]
|
|
(th/db-update! :file-media-object
|
|
{:thumbnail-id (:id thumb-storage)}
|
|
{:id (:id media-obj)})
|
|
(let [request {:path-params {:id (str (:id media-obj))}
|
|
::session/profile-id (:id owner)}
|
|
response (assets/file-thumbnails-handler cfg request)]
|
|
(t/is (= 204 (::yres/status response)))
|
|
(t/is (= 1.0 (counter-value metrics ["thumbnail" "fs" "file-object-thumbnail" "served"]))))))
|
|
|
|
(t/deftest file-thumbnails-handler-fallback-emits-thumbnail-route-metric
|
|
;; Served through the media-id fallback (no thumbnail-id set).
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
metrics (make-metrics)
|
|
cfg (make-metrics-cfg storage metrics)
|
|
owner (th/create-profile* 1)
|
|
team (th/create-team* 1 {:profile-id (:id owner)})
|
|
project (th/create-project* 1 {:profile-id (:id owner)
|
|
:team-id (:id team)})
|
|
file (th/create-file* 1 {:profile-id (:id owner)
|
|
:project-id (:id project)})
|
|
media-storage (create-storage-object! storage "file-media-object" "image data")
|
|
media-obj (th/create-file-media-object* {:file-id (:id file)
|
|
:media-id (:id media-storage)})
|
|
request {:path-params {:id (str (:id media-obj))}
|
|
::session/profile-id (:id owner)}
|
|
response (assets/file-thumbnails-handler cfg request)]
|
|
(t/is (= 204 (::yres/status response)))
|
|
(t/is (= 1.0 (counter-value metrics ["thumbnail" "fs" "file-media-object" "served"])))))
|
|
|
|
(t/deftest file-thumbnails-handler-missing-storage-emits-not-found-metric
|
|
;; Media row exists but the storage object is gone: 404 with unknown labels.
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
metrics (make-metrics)
|
|
cfg (make-metrics-cfg storage metrics)
|
|
owner (th/create-profile* 1)
|
|
team (th/create-team* 1 {:profile-id (:id owner)})
|
|
project (th/create-project* 1 {:profile-id (:id owner)
|
|
:team-id (:id team)})
|
|
file (th/create-file* 1 {:profile-id (:id owner)
|
|
:project-id (:id project)})
|
|
media-storage (create-storage-object! storage "file-media-object" "image data")
|
|
media-obj (th/create-file-media-object* {:file-id (:id file)
|
|
:media-id (:id media-storage)})
|
|
request {:path-params {:id (str (:id media-obj))}
|
|
::session/profile-id (:id owner)}]
|
|
(with-mocks [_mock {:target 'app.storage/get-object
|
|
:return (fn [_ _] nil)}]
|
|
(let [response (assets/file-thumbnails-handler cfg request)]
|
|
(t/is (= 404 (::yres/status response)))))
|
|
(t/is (= 1.0 (counter-value metrics ["thumbnail" "unknown" "unknown" "not-found"])))))
|
|
|
|
(t/deftest objects-handler-tempfile-mismatch-emits-unauthorized-metric
|
|
;; The response stays 404 to avoid leaking existence, but the metric
|
|
;; records the internal auth outcome.
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
metrics (make-metrics)
|
|
cfg (make-metrics-cfg storage metrics)
|
|
owner (th/create-profile* 1)
|
|
stranger (th/create-profile* 2)
|
|
object (create-storage-object! storage "tempfile" "temp data" {:profile-id (:id owner)})
|
|
request {:path-params {:id (str (:id object))}
|
|
::session/profile-id (:id stranger)}
|
|
response (assets/objects-handler cfg request)]
|
|
(t/is (= 404 (::yres/status response)))
|
|
(t/is (= 1.0 (counter-value metrics ["by-id" "fs" "tempfile" "unauthorized"])))
|
|
(t/is (= 0.0 (counter-value metrics ["by-id" "fs" "tempfile" "not-found"])))))
|
|
|
|
(t/deftest handlers-require-metrics
|
|
;; Metrics is no longer optional: a handler wired without it must fail
|
|
;; loudly instead of silently dropping the measurement.
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
cfg (dissoc (make-handler-cfg storage) ::mtx/metrics)
|
|
owner (th/create-profile* 1)
|
|
team (th/create-team* 1 {:profile-id (:id owner)})
|
|
project (th/create-project* 1 {:profile-id (:id owner)
|
|
:team-id (:id team)})
|
|
file (th/create-file* 1 {:profile-id (:id owner)
|
|
:project-id (:id project)})
|
|
object (create-storage-object! storage "file-media-object" "file content")
|
|
media-obj (th/create-file-media-object* {:file-id (:id file)
|
|
:media-id (:id object)})]
|
|
(t/is (thrown? Throwable
|
|
(assets/objects-handler
|
|
cfg {:path-params {:id (str (:id object))}})))
|
|
(t/is (thrown? Throwable
|
|
(assets/file-objects-handler
|
|
cfg {:path-params {:id (str (:id media-obj))}
|
|
::session/profile-id (:id owner)})))
|
|
(t/is (thrown? Throwable
|
|
(assets/file-thumbnails-handler
|
|
cfg {:path-params {:id (str (:id media-obj))}
|
|
::session/profile-id (:id owner)})))
|
|
(t/is (thrown? Throwable
|
|
(assets/objects-handler
|
|
cfg {:path-params {:id (str (uuid/next))}})))))
|
|
|
|
(t/deftest malformed-uuid-emits-nothing
|
|
;; get-id raises before any metric emission point is reached.
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
metrics (make-metrics)
|
|
cfg (make-metrics-cfg storage metrics)
|
|
request {:path-params {:id "not-a-uuid"}}]
|
|
(t/is (thrown? clojure.lang.ExceptionInfo
|
|
(assets/objects-handler cfg request)))
|
|
(t/is (= 0.0 (counter-value metrics ["by-id" "fs" "file-media-object" "served"])))
|
|
(t/is (= 0.0 (counter-value metrics ["by-id" "unknown" "unknown" "not-found"])))
|
|
(t/is (= 0.0 (counter-value metrics ["by-id" "unknown" "unknown" "error"])))))
|
|
|
|
(t/deftest assets-handler-survives-metrics-failure
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
metrics (make-metrics)
|
|
cfg (make-metrics-cfg storage metrics)
|
|
object (create-storage-object! storage "file-media-object" "file content")
|
|
request {:path-params {:id (str (:id object))}}]
|
|
(with-mocks [_mock {:target 'app.metrics/run-collector!
|
|
:throw (ex-info "boom" {})}]
|
|
(let [response (assets/objects-handler cfg request)]
|
|
(t/is (= 204 (::yres/status response)))))))
|
|
|
|
(t/deftest file-objects-handler-serve-failure-emits-error-and-rethrows
|
|
(let [storage (-> (:app.storage/storage th/*system*)
|
|
(configure-storage-backend))
|
|
metrics (make-metrics)
|
|
cfg (make-metrics-cfg storage metrics)
|
|
owner (th/create-profile* 1)
|
|
team (th/create-team* 1 {:profile-id (:id owner)})
|
|
project (th/create-project* 1 {:profile-id (:id owner)
|
|
:team-id (:id team)})
|
|
file (th/create-file* 1 {:profile-id (:id owner)
|
|
:project-id (:id project)})
|
|
media-storage (create-storage-object! storage "file-media-object" "image data")
|
|
media-obj (th/create-file-media-object* {:file-id (:id file)
|
|
:media-id (:id media-storage)})
|
|
request {:path-params {:id (str (:id media-obj))}
|
|
::session/profile-id (:id owner)}]
|
|
(with-mocks [_mock {:target 'app.storage/object->relative-path
|
|
:throw (ex-info "boom" {})}]
|
|
(t/is (thrown? clojure.lang.ExceptionInfo
|
|
(assets/file-objects-handler cfg request))))
|
|
(t/is (= 1.0 (counter-value metrics ["by-file-media-id" "fs" "file-media-object" "error"])))
|
|
(t/is (= 0.0 (counter-value metrics ["by-file-media-id" "fs" "file-media-object" "served"])))))
|
|
|
|
(t/deftest result-label-mapping
|
|
(t/are [status expected]
|
|
(= expected (#'app.http.assets/result-label status))
|
|
nil "error"
|
|
200 "served"
|
|
204 "served"
|
|
307 "served"
|
|
401 "unauthorized"
|
|
403 "unauthorized"
|
|
404 "not-found"
|
|
429 "error"
|
|
500 "error"))
|