diff --git a/backend/src/app/config.clj b/backend/src/app/config.clj index a4375c2184..dfc4bfd23e 100644 --- a/backend/src/app/config.clj +++ b/backend/src/app/config.clj @@ -81,6 +81,7 @@ :jobs-retention (ct/duration {:days 7}) :jobs-user-ttl (ct/duration {:days 7}) :jobs-request-timeout (ct/duration {:minutes 2}) + :job-session-ttl (ct/duration {:hours 1}) :media-max-file-size (* 1024 1024 30) ; 30MiB :font-max-file-size (* 1024 1024 30) ; 30MiB @@ -201,6 +202,7 @@ [:jobs-retention {:optional true} ::ct/duration] [:jobs-user-ttl {:optional true} ::ct/duration] [:jobs-request-timeout {:optional true} ::ct/duration] + [:job-session-ttl {:optional true} ::ct/duration] [:telemetry-enabled {:optional true} ::sm/boolean] [:default-blob-version {:optional true} ::sm/int] [:allow-demo-users {:optional true} ::sm/boolean] @@ -490,6 +492,15 @@ (or (c/get config :jobs-user-ttl) (ct/duration {:days 7}))) +(defn get-job-session-ttl + "How long the token minted by `create-job-session` verifies, beyond + the session row's own lifetime: an export never runs for hours, so a + short expiry is defense in depth for the worker that dies without + settling. The session row itself is swept by the sessions idle GC." + [] + (or (c/get config :job-session-ttl) + (ct/duration {:hours 1}))) + (defn get "A configuration getter. Helps code be more testable." ([key] diff --git a/backend/src/app/http/session.clj b/backend/src/app/http/session.clj index 4e9ce3bd77..4409bee8ea 100644 --- a/backend/src/app/http/session.clj +++ b/backend/src/app/http/session.clj @@ -157,22 +157,34 @@ (declare ^:private assign-session-cookie) (declare ^:private clear-session-cookie) +(defn generate-token + "The authentication token of a session, with exactly the claims the + authz resolver accepts: the session id (`:sid`) and the profile it + belongs to (`:uid`). The optional `:exp` bounds how long the token + verifies, independent of the session row: without it, the expiry of a + login session is its absolute max-age counted from `:created-at`." + [cfg session & {:keys [exp]}] + (let [absolute-max-age (cf/get :auth-token-cookie-max-age-absolute + default-cookie-max-age-absolute) + claims (cond-> {:iss "authentication" + :aud "penpot" + :sid (:id session) + :iat (:modified-at session) + :uid (:profile-id session) + :sso-provider-id (:sso-provider-id session) + :sso-session-id (:sso-session-id session)} + exp + (assoc :exp exp) + + (and (nil? exp) (ct/inst? (:created-at session))) + (assoc :exp (ct/plus (:created-at session) + absolute-max-age))) + header {:kid 1 :ver 1}] + (tokens/generate cfg claims header))) + (defn- assign-token [cfg session] - (let [absolute-max-age (cf/get :auth-token-cookie-max-age-absolute default-cookie-max-age-absolute) - claims {:iss "authentication" - :aud "penpot" - :sid (:id session) - :iat (:modified-at session) - :uid (:profile-id session) - :sso-provider-id (:sso-provider-id session) - :sso-session-id (:sso-session-id session)} - claims (if (:created-at session) - (assoc claims :exp (ct/plus (:created-at session) absolute-max-age)) - claims) - header {:kid 1 :ver 1} - token (tokens/generate cfg claims header)] - (assoc session :token token))) + (assoc session :token (generate-token cfg session))) (defn create-fn [{:keys [::manager] :as cfg} {profile-id :id :as profile} diff --git a/backend/src/app/rpc/management/jobs.clj b/backend/src/app/rpc/management/jobs.clj index 1e71acb101..6f069fcd55 100644 --- a/backend/src/app/rpc/management/jobs.clj +++ b/backend/src/app/rpc/management/jobs.clj @@ -19,9 +19,13 @@ [app.common.exceptions :as ex] [app.common.schema :as sm] [app.common.time :as ct] + [app.config :as cf] + [app.db :as db] + [app.http.session :as session] [app.jobs :as jobs] [app.rpc :as-alias rpc] [app.rpc.doc :as doc] + [app.setup :as-alias setup] [app.util.services :as sv])) ;; ---- RPC METHOD: CLAIM-JOB @@ -151,3 +155,87 @@ (if (pos? (jobs/fail cfg job-id error)) {:action :run} {:action :skip})) + +;; ---- RPC METHOD: CREATE-JOB-SESSION + +(def ^:private schema:create-job-session-params + [:map {:title "create-job-session-params"} + [:job-id ::sm/uuid]]) + +(def ^:private schema:create-job-session-result + [:map {:title "create-job-session-result"} + [:session-id ::sm/uuid] + [:session-token ::sm/text]]) + +(defn- validate-job! + "The job whose owner will render: it must exist, be `running` (the + claim already passed: a session for a waiting job would outlive a + job that may never run) and have an owner. A job of another tenant is + indistinguishable here on purpose: the management API is the ledger + of any row of the table, and the profile, not the tenant, is the + boundary of what the session can touch." + [cfg job-id] + (let [job (jobs/get-job cfg job-id)] + (when (nil? job) + (ex/raise :type :not-found + :code :job-not-found + :hint "the job does not exist" + :job-id job-id)) + (when-not (= "running" (:status job)) + (ex/raise :type :validation + :code :invalid-job-state + :hint "the session is minted for a running job" + :status (:status job) + :job-id job-id)) + (when-not (uuid? (:profile-id job)) + (ex/raise :type :validation + :code :job-without-profile + :hint "the job has no owner profile to mint the session for" + :job-id job-id)) + (:profile-id job))) + +(defn- validate-owner! + "The profile the session will render as: the same one the login + accepts (exists, active, not blocked, not deleted). A session is a + key to everything the owner can touch, so a worker that settles a job + of a dead profile never gets one." + [cfg profile-id] + (let [profile (db/get* cfg :profile {:id profile-id})] + (when (or (nil? profile) (some? (:deleted-at profile))) + (ex/raise :type :not-found + :code :profile-not-found + :hint "the owner profile of the job does not exist" + :profile-id profile-id)) + (when-not (:is-active profile) + (ex/raise :type :validation + :code :profile-not-active + :hint "the owner profile of the job is not active" + :profile-id profile-id)) + (when (:is-blocked profile) + (ex/raise :type :restriction + :code :profile-blocked + :hint "the owner profile of the job is marked as blocked" + :profile-id profile-id)))) + +(sv/defmethod ::create-job-session + "Mint an authentication session of the owner of a running job, for + the worker to render as the user: render.html and assets authenticate + by cookie and the RPC by Bearer, through the surfaces that already + exist. Every call mints a fresh session (worker retries never wait on + one a settle may have closed), and the token carries the short `:exp` + of `:job-session-ttl`: a worker that dies without settling leaves + nothing usable for long, and the sessions idle GC sweeps the row." + {::doc/added "2.20" + ::sm/params schema:create-job-session-params + ::sm/result schema:create-job-session-result + ::rpc/auth false} ;; shared-key enforced by route resolver + [cfg {:keys [job-id]}] + (let [profile-id (validate-job! cfg job-id)] + (validate-owner! cfg profile-id) + (let [session (session/create-session (::session/manager cfg) + {:profile-id profile-id}) + token (session/generate-token cfg session + :exp (ct/plus (ct/now) + (cf/get-job-session-ttl)))] + {:session-id (:id session) + :session-token token}))) diff --git a/backend/test/backend_tests/rpc_management_jobs_test.clj b/backend/test/backend_tests/rpc_management_jobs_test.clj index 9ff0882e68..40b91a5314 100644 --- a/backend/test/backend_tests/rpc_management_jobs_test.clj +++ b/backend/test/backend_tests/rpc_management_jobs_test.clj @@ -11,10 +11,13 @@ [app.common.uuid :as uuid] [app.config :as cf] [app.db :as db] + [app.http.session :as session] [app.jobs :as jobs] [app.metrics :as-alias mtx] + [app.setup :as-alias setup] [backend-tests.helpers :as th] - [clojure.test :as t])) + [clojure.test :as t] + [mockery.core :refer [with-mocks]])) (t/use-fixtures :once th/state-init) @@ -32,7 +35,7 @@ ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; (defn- mk-job - [{:keys [name status params scheduled-at] + [{:keys [name status params scheduled-at profile-id] :or {name "media-process" status "new" params {:x 1} @@ -47,9 +50,10 @@ :max-retries 3 :retry-num 0 :status status + :profile-id profile-id :scheduled-at scheduled-at :created-at (ct/now) - :modified-at (ct/now)}) + :modified-at (ct/now)}) id)) (defn- get-row @@ -80,6 +84,21 @@ [type params] (th/management-command! (assoc params ::th/type type))) +(defn- mk-profile + ([] (mk-profile 1)) + ([i] + ;; the created profile is inactive until the email is verified, and + ;; a session can only be minted for an owner the login would accept + (th/create-profile* i {:is-active true}))) + +(defn- get-session-row + [id] + (th/db-get :http-session-v2 {:id id})) + +(defn- decode-token + [token] + (session/decode-token {::setup/props (get th/*system* :app.setup/props)} token)) + ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; TESTS ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; @@ -421,3 +440,110 @@ :attempt 3})] (t/is (nil? (:error out))) (t/is (= 3 (:attempt (jobs/decode-job-error (:error (get-row job-id)))))))))) + +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +;; CREATE-JOB-SESSION +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; + +(t/deftest create-job-session-mints-a-session-of-the-owner + (let [profile (mk-profile) + job-id (mk-job {:status "running" + :profile-id (:id profile)}) + out (mgmt :create-job-session {:job-id job-id}) + result (:result out)] + + (t/is (nil? (:error out))) + + (t/testing "the answer is a real session of the job owner" + (t/is (uuid? (:session-id result))) + (t/is (string? (:session-token result))) + (let [row (get-session-row (:session-id result))] + (t/is (some? row)) + (t/is (= (:id profile) (:profile-id row))))) + + (t/testing "the token verifies and names the session and its owner" + (let [claims (decode-token (:session-token result))] + (t/is (some? claims)) + (t/is (= (:session-id result) (:sid claims))) + (t/is (= (:id profile) (:uid claims))) + ;; the short :exp of a job session: at most an hour by default, + ;; never the absolute max-age of a login session + (t/is (pos? (- (inst-ms (:exp claims)) (inst-ms (ct/now))))) + (t/is (pos? (compare (ct/plus (ct/now) (ct/duration {:hours 1})) + (:exp claims)))))))) + +(t/deftest create-job-session-honours-the-configured-ttl + (with-mocks [mock {:target 'app.config/get-job-session-ttl + :return (ct/duration {:minutes 5})}] + (let [profile (mk-profile) + job-id (mk-job {:status "running" + :profile-id (:id profile)}) + out (mgmt :create-job-session {:job-id job-id}) + claims (decode-token (get-in out [:result :session-token]))] + (t/is (nil? (:error out))) + (t/is (pos? (- (inst-ms (:exp claims)) (inst-ms (ct/now))))) + (t/is (pos? (compare (ct/plus (ct/now) (ct/duration {:minutes 5})) + (:exp claims))))))) + +(t/deftest create-job-session-mints-a-fresh-session-every-call + ;; every call is self-contained: a worker retry never waits on a + ;; session that a settle may already have closed + (let [profile (mk-profile) + job-id (mk-job {:status "running" + :profile-id (:id profile)}) + first (mgmt :create-job-session {:job-id job-id}) + second (mgmt :create-job-session {:job-id job-id})] + (t/is (nil? (:error first))) + (t/is (nil? (:error second))) + (t/is (not= (get-in first [:result :session-id]) + (get-in second [:result :session-id]))))) + +(t/deftest create-job-session-refuses-a-job-that-is-not-running + (let [profile (mk-profile)] + (doseq [status ["new" "scheduled" "completed" "failed" "cancelled" "aborted"]] + (let [job-id (mk-job {:status status :profile-id (:id profile)}) + out (mgmt :create-job-session {:job-id job-id})] + (t/is (some? (:error out))) + (t/is (= :validation (th/ex-type (:error out)))) + (t/is (= :invalid-job-state (th/ex-code (:error out)))) + (t/testing "and no session was minted" + (t/is (zero? (count (th/db-exec! ["SELECT id FROM http_session_v2"]))))))))) + +(t/deftest create-job-session-refuses-an-unknown-job + (let [out (mgmt :create-job-session {:job-id (uuid/next)})] + (t/is (some? (:error out))) + (t/is (= :not-found (th/ex-type (:error out)))) + (t/is (= :job-not-found (th/ex-code (:error out)))))) + +(t/deftest create-job-session-refuses-a-job-without-owner + (let [job-id (mk-job {:status "running"}) + out (mgmt :create-job-session {:job-id job-id})] + (t/is (some? (:error out))) + (t/is (= :validation (th/ex-type (:error out)))) + (t/is (= :job-without-profile (th/ex-code (:error out)))))) + +(t/deftest create-job-session-refuses-an-owner-the-login-would-refuse + (let [profile (mk-profile) + job-id (mk-job {:status "running" + :profile-id (:id profile)})] + + (t/testing "a blocked owner is a restriction, not a fault" + (th/db-update! :profile {:is-blocked true} {:id (:id profile)}) + (let [out (mgmt :create-job-session {:job-id job-id})] + (t/is (= :restriction (th/ex-type (:error out)))) + (t/is (= :profile-blocked (th/ex-code (:error out)))))) + + (t/testing "an inactive owner is refused like the login does" + (th/db-update! :profile {:is-blocked false :is-active false} {:id (:id profile)}) + (let [out (mgmt :create-job-session {:job-id job-id})] + (t/is (= :validation (th/ex-type (:error out)))) + (t/is (= :profile-not-active (th/ex-code (:error out))))))) + + (t/testing "a deleted owner does not exist" + (let [profile (mk-profile 2) + job-id (mk-job {:status "running" + :profile-id (:id profile)})] + (th/db-update! :profile {:deleted-at (ct/now)} {:id (:id profile)}) + (let [out (mgmt :create-job-session {:job-id job-id})] + (t/is (= :not-found (th/ex-type (:error out)))) + (t/is (= :profile-not-found (th/ex-code (:error out))))))))