diff --git a/.serena/memories/backend/auth-permissions-product-domains.md b/.serena/memories/backend/auth-permissions-product-domains.md index 0423a838a6..02e2e83462 100644 --- a/.serena/memories/backend/auth-permissions-product-domains.md +++ b/.serena/memories/backend/auth-permissions-product-domains.md @@ -13,7 +13,7 @@ - Logout may return an OIDC provider redirect URI when the session claims include provider/session data and the provider has a logout URI. - Invitation tokens are verified through token issuers and only accepted when the token member id/email matches the authenticated profile; otherwise login proceeds without consuming the invitation. - When public registration is disabled, invitation-based password registration also requires a matching, non-expired `team_invitation` row; registration revalidates it under a row lock before creating the profile. -- HTTP/session parsing details such as cookie/header precedence, JWT session token versions, and SameSite behavior are in `mem:backend/subtleties`. +- HTTP/session parsing details such as cookie/header precedence and SameSite behavior are in `mem:backend/subtleties`. ## Permission model diff --git a/.serena/memories/backend/core.md b/.serena/memories/backend/core.md index 8f67530e71..71714d12c3 100644 --- a/.serena/memories/backend/core.md +++ b/.serena/memories/backend/core.md @@ -7,6 +7,7 @@ Backend: JVM Clojure; Integrant; PostgreSQL; Redis/Valkey; RPC; HTTP; storage; m - Cross-cutting backend subtleties (RPC, DB, workers, cron, HTTP/sessions, storage, file data): `mem:backend/subtleties` - Storage abstraction, logical buckets, object lifecycle, deduplication, access, and garbage collection: `mem:backend/storage`. - Embedded Ladybug graph experiment, projection, incremental sync, console, and risks: `mem:backend/graph-experiment` +- Session lifetime config, token `:exp`, and idle/absolute GC: `mem:backend/session-expiration`. - Auth flows, permission model, teams, projects, invitations, comments, webhooks, audit: `mem:backend/auth-permissions-product-domains` - Audit-log event collection (RPC wrapper, frontend ingestion), telemetry duality, webhook fan-out, error reporters, Nexus archival and retention: `mem:backend/audit-log` - Services, task-queue/Pub-Sub topology constraints -> `mem:prod-infra/core`. diff --git a/.serena/memories/backend/session-expiration.md b/.serena/memories/backend/session-expiration.md new file mode 100644 index 0000000000..99c0d6acac --- /dev/null +++ b/.serena/memories/backend/session-expiration.md @@ -0,0 +1,35 @@ +# Backend Session Expiration + +## Config + +- `:auth-token-cookie-name` / `PENPOT_AUTH_TOKEN_COOKIE_NAME` (default `auth-token`): cookie name. +- `:auth-token-cookie-max-age` / `PENPOT_AUTH_TOKEN_COOKIE_MAX_AGE` (default 7d): idle window; drives the sliding cookie `Expires` and the GC idle threshold. +- `:auth-token-cookie-max-age-absolute` / `PENPOT_AUTH_TOKEN_COOKIE_MAX_AGE_ABSOLUTE` (default 30d): hard cap from `created-at`; drives the token `:exp` and the GC absolute threshold. +- Durations decode as `` with hour/minute/second units (`168h`, `30m`); day units like `7d` are rejected by the `Duration/parse` string path, use hours (`168h`, `720h`). +- Session lifetime defaults live only as code constants in `session.clj` (`default-cookie-max-age`, `default-cookie-max-age-absolute`); do not duplicate them in the `config.clj` default map. Call sites always pass the constant as the `cf/get` fallback. + +## Token and session model + +- Sessions live only in `http_session_v2`. Legacy v1 support (`http_session`, string ids, `:ver 0` tokens) was removed and the table dropped (`0155-drop-http-session-table`). +- `assign-token` emits header `{:kid 1 :ver 1}` with claims `:sid`, `:iat` (= `modified-at`) and `:exp` = `created-at + absolute-max-age` (omitted only when `created-at` is nil, which neither manager produces today; the branch is defensive). +- `:exp` is anchored to `created-at`, never `modified-at`, so renewal cannot extend the absolute maximum. +- Tokens issued before `:exp` existed carry no `:exp`; they are still bounded by the GC's `created_at` condition and acquire `:exp` on their next renewal (self-healing, no operator action). +- `wrap-authz` resolves the session by `(:sid claims)` only; there is no fallback to reading a session by the raw token string. +- `middleware/wrap-auth` attaches `::http/auth-data` only for `kid=1`/`ver=1` tokens with a configured decoder; anything else stays unauthenticated. +- Renewal fires when `modified-at` is older than 6h (`default-renewal-max-age`, not configurable). It `UPDATE`s the same row (`modified-at` only) and issues a new token that keeps the original `:exp`. +- `read-session` does not check age. Idle expiration is enforced on every request by `wrap-authz` (sessions past the idle window authenticate like missing ones and are never renewed); the GC only deletes the rows afterwards. + +## GC (`::tasks/gc`, cron `session-gc`, daily) + +- Deletes from `http_session_v2` where `modified_at < now - :auth-token-cookie-max-age` **or** `created_at < now - :auth-token-cookie-max-age-absolute`. +- The two thresholds are separate task params (`::tasks/max-age`, `::tasks/max-age-absolute`) built in `ig/expand-key`; both must be durations. +- Do not collapse the two conditions: `modified_at` alone never collects active sessions; `created_at` alone logs out active users at the idle window. + +## Related lifetimes + +- Organization SSO entries in session `:props` (`:sso {org-id exp}`) last 4h, hardcoded in `app.auth.oidc`. +- Access tokens (`app.rpc.commands.access-token`) have a user-chosen `:expires-at` (Never/30/60/90/180d), independent from HTTP sessions. +- Cookie `SameSite`/`secure` depend on `:cors`, `:strict-session-cookies`, `:secure-session-cookies`; `parse-flags` auto-adds `:disable-secure-session-cookies` for non-localhost HTTP `public-uri`. +- Without `PENPOT_SECRET_KEY`, derived subsystem keys change on restart and all sessions/invitations are invalidated. +- Read-only DB pool: sessions fall back to `inmemory-manager`, lost on restart, no GC. +- HTTP middleware and cookie/header precedence details: `mem:backend/subtleties`. diff --git a/.serena/memories/backend/subtleties.md b/.serena/memories/backend/subtleties.md index 354b9291a9..ad6c4321ae 100644 --- a/.serena/memories/backend/subtleties.md +++ b/.serena/memories/backend/subtleties.md @@ -33,10 +33,10 @@ - The backend sets Clojure `*assert*` globally from the `:backend-asserts` feature flag. Assertion-dependent checks can therefore differ by runtime flags. - Request body parsing is mostly POST-oriented and supports Transit JSON plus plain JSON. Plain JSON request keys are kebab-decoded before being merged into `:params`. - Response formatting negotiates with `Accept` or `_fmt=json`. Transit is the default for collection/boolean bodies; JSON encoding has special pointer-map handling. -- Auth prefers the session cookie token before the `Authorization` header. Headers may be `Token` or `Bearer`; JWTs with `kid=1` and `ver=1` are decoded as v1 session tokens, otherwise they are treated as legacy tokens. +- Auth prefers the session cookie token before the `Authorization` header. Headers may be `Token` or `Bearer`. Only `kid=1`/`ver=1` tokens are decoded as session tokens; anything else is left unauthenticated (legacy v1 tokens were removed). - Shared-key auth requires `x-shared-key` as ` ` and stores the lowercased key id on the request. If no shared keys are configured it always rejects. -- Session management uses DB storage unless the DB pool is read-only, then falls back to the in-memory manager. DB sessions support both legacy string ids and v2 UUID session ids. -- Session cookies are renewed when using a legacy string id or when `modified-at` is older than the renewal interval. SameSite is `none` for CORS, otherwise strict/lax based on config. +- Session management uses DB storage unless the DB pool is read-only, then falls back to the in-memory manager. Sessions use only the v2 UUID model (`http_session_v2`); legacy string ids were removed. +- Session cookies are renewed when `modified-at` is older than the 6h renewal interval. SameSite is `none` for CORS, otherwise strict/lax based on config. Session lifetime config and GC: `mem:backend/session-expiration`. ## HTTP server self-metrics diff --git a/backend/src/app/config.clj b/backend/src/app/config.clj index 08c48476b5..a9bc88e7eb 100644 --- a/backend/src/app/config.clj +++ b/backend/src/app/config.clj @@ -61,7 +61,6 @@ :objects-storage-fs-directory "assets" :auth-token-cookie-name "auth-token" - :auth-token-cookie-max-age-absolute (ct/duration {:days 30}) :assets-path "/internal/assets/" :smtp-default-reply-to "Penpot " diff --git a/backend/src/app/http/debug.clj b/backend/src/app/http/debug.clj index 0a4929b2ec..5884e8e4bd 100644 --- a/backend/src/app/http/debug.clj +++ b/backend/src/app/http/debug.clj @@ -675,7 +675,7 @@ (contains? params :block) (do (db/update! conn :profile {:is-blocked true} {:id (:id profile)}) - (db/delete! conn :http-session {:profile-id (:id profile)}) + (session/invalidate-all cfg (:id profile)) {::yres/status 200 ::yres/headers {"content-type" "text/plain"} diff --git a/backend/src/app/http/middleware.clj b/backend/src/app/http/middleware.clj index 6aa8e9d3a2..45f8abecca 100644 --- a/backend/src/app/http/middleware.clj +++ b/backend/src/app/http/middleware.clj @@ -306,16 +306,15 @@ (let [decode-fn (get decoders type)] (if (or (= type :cookie) (= type :bearer)) (let [metadata (tokens/decode-header token)] - ;; NOTE: we only proceed to decode claims on new - ;; cookie tokens. The old cookies dont need to be - ;; decoded because they use the token string as ID + ;; NOTE: only current (kid=1/ver=1) cookie tokens carry + ;; decodable claims. Anything else is left unauthenticated. (if (and (= (:kid metadata) 1) (= (:ver metadata) 1) (some? decode-fn)) (assoc request ::http/auth-data (assoc auth :claims (decode-fn token) :metadata metadata)) - (assoc request ::http/auth-data (assoc auth :metadata {:ver 0})))) + request)) (if decode-fn (assoc request ::http/auth-data (assoc auth :claims (decode-fn token))) diff --git a/backend/src/app/http/session.clj b/backend/src/app/http/session.clj index 288fdc4396..c5909ca432 100644 --- a/backend/src/app/http/session.clj +++ b/backend/src/app/http/session.clj @@ -81,14 +81,8 @@ [pool] (reify ISessionManager (read-session [_ id] - (if (string? id) - ;; Backward compatibility: http_session (v1) has no props column - (let [session (db/exec-one! pool (sql/select :http-session {:id id}))] - (-> session - (assoc :modified-at (:updated-at session)) - (dissoc :updated-at))) - (some-> (db/exec-one! pool (sql/select :http-session-v2 {:id id})) - (decode-session)))) + (some-> (db/exec-one! pool (sql/select :http-session-v2 {:id id})) + (decode-session))) (create-session [_ params] (assert (valid-params? params) "expect valid session params") @@ -103,23 +97,15 @@ (update-session [_ session] (let [modified-at (ct/now)] - (if (string? (:id session)) - (db/insert! pool :http-session-v2 - (-> session - (assoc :id (uuid/next)) - (assoc :created-at modified-at) - (assoc :modified-at modified-at))) - (db/update! pool :http-session-v2 - (cond-> {:modified-at modified-at} - (some? (:props session)) - (assoc :props (db/tjson (:props session)))) - {:id (:id session)} - {::db/return-keys true})))) + (db/update! pool :http-session-v2 + (cond-> {:modified-at modified-at} + (some? (:props session)) + (assoc :props (db/tjson (:props session)))) + {:id (:id session)} + {::db/return-keys true}))) (delete-session [_ id] - (if (string? id) - (db/delete! pool :http-session {:id id} {::db/return-keys false}) - (db/delete! pool :http-session-v2 {:id id} {::db/return-keys false})) + (db/delete! pool :http-session-v2 {:id id} {::db/return-keys false}) nil))) (defn inmemory-manager @@ -256,25 +242,28 @@ (db/exec! pool [sql:clear-organization-sso-sessions organization-key organization-key]))) (defn- renew-session? - [{:keys [id modified-at] :as session}] - (or (string? id) - (and (ct/inst? modified-at) - (let [elapsed (ct/diff modified-at (ct/now))] - (neg? (compare default-renewal-max-age elapsed)))))) + [{:keys [modified-at]}] + (and (ct/inst? modified-at) + (let [elapsed (ct/diff modified-at (ct/now))] + (neg? (compare default-renewal-max-age elapsed))))) + +(defn- session-expired? + [{:keys [modified-at]}] + (and (ct/inst? modified-at) + (let [max-age (cf/get :auth-token-cookie-max-age default-cookie-max-age)] + (pos? (compare (ct/now) (ct/plus modified-at max-age)))))) (defn- wrap-authz [handler {:keys [::manager] :as cfg}] (assert (manager? manager) "expected valid session manager") (fn [request] - (let [{:keys [type token claims metadata]} (get request ::http/auth-data)] + (let [{:keys [type claims]} (get request ::http/auth-data)] (cond (= type :cookie) - (let [session - (case (:ver metadata) - ;; BACKWARD COMPATIBILITY WITH OLD TOKENS - 0 (read-session manager token) - 1 (some->> (:sid claims) (read-session manager)) - nil) + (let [session (some->> (:sid claims) (read-session manager)) + ;; Idle-expired sessions authenticate like missing ones: + ;; no profile, no renewal. The GC deletes the row later. + session (when-not (session-expired? session) session) request (cond-> request @@ -294,11 +283,8 @@ response)) (= type :bearer) - (let [session (case (:ver metadata) - ;; BACKWARD COMPATIBILITY WITH OLD TOKENS - 0 (read-session manager token) - 1 (some->> (:sid claims) (read-session manager)) - nil) + (let [session (some->> (:sid claims) (read-session manager)) + session (when-not (session-expired? session) session) request (cond-> request (some? session) (-> (assoc ::profile-id (:profile-id session)) @@ -317,9 +303,9 @@ (defn- assign-session-cookie [response {token :token modified-at :modified-at}] (let [max-age (cf/get :auth-token-cookie-max-age default-cookie-max-age) - created-at modified-at - renewal (ct/plus created-at default-renewal-max-age) - expires (ct/plus created-at max-age) + renewal-at modified-at + renewal (ct/plus renewal-at default-renewal-max-age) + expires (ct/plus renewal-at max-age) secure? (contains? cf/flags :secure-session-cookies) strict? (contains? cf/flags :strict-session-cookies) cors? (contains? cf/flags :cors) @@ -346,40 +332,46 @@ (defmethod ig/assert-key ::tasks/gc [_ params] (assert (db/pool? (::db/pool params)) "expected valid database pool") - (assert (ct/duration? (::tasks/max-age params)))) + (assert (ct/duration? (::tasks/max-age params))) + (assert (ct/duration? (::tasks/max-age-absolute params))) + ;; NOTE: explicit throw (not `assert`) so the guard fires even when + ;; the `:backend-asserts` flag leaves `*assert*` false. + (when (pos? (compare (::tasks/max-age params) + (::tasks/max-age-absolute params))) + (throw (IllegalArgumentException. + "absolute session max-age must be greater than or equal to idle max-age")))) (defmethod ig/expand-key ::tasks/gc [k v] - (let [max-age (cf/get :auth-token-cookie-max-age default-cookie-max-age)] - {k (merge {::tasks/max-age max-age} (d/without-nils v))})) - -(def ^:private - sql:delete-expired - "DELETE FROM http_session - WHERE updated_at < ?::timestamptz - or (updated_at is null and - created_at < ?::timestamptz)") + (let [max-age (cf/get :auth-token-cookie-max-age default-cookie-max-age) + max-age-absolute (cf/get :auth-token-cookie-max-age-absolute + default-cookie-max-age-absolute)] + {k (merge {::tasks/max-age max-age + ::tasks/max-age-absolute max-age-absolute} + (d/without-nils v))})) (def ^:private sql:delete-expired-v2 "DELETE FROM http_session_v2 - WHERE created_at < ?::timestamptz") + WHERE modified_at < ?::timestamptz + OR created_at < ?::timestamptz") (defn- collect-expired-tasks - [{:keys [::db/conn ::tasks/max-age]}] - (let [threshold (ct/minus (ct/now) max-age) - result-legacy (-> (db/exec-one! conn [sql:delete-expired threshold threshold]) - (db/get-update-count)) - result-v2 (-> (db/exec-one! conn [sql:delete-expired-v2 threshold]) - (db/get-update-count))] + [{:keys [::db/conn ::tasks/max-age ::tasks/max-age-absolute]}] + (let [idle-threshold (ct/minus (ct/now) max-age) + abs-threshold (ct/minus (ct/now) max-age-absolute) + result (-> (db/exec-one! conn [sql:delete-expired-v2 + idle-threshold abs-threshold]) + (db/get-update-count))] (l/dbg :task "gc" :hint "clean http sessions" - :deleted-legacy result-legacy - :deleted-v2 result-v2) - (+ result-legacy result-v2))) + :deleted result) + result)) (defmethod ig/init-key ::tasks/gc - [_ {:keys [::tasks/max-age] :as cfg}] - (l/dbg :hint "initializing session gc task" :max-age max-age) + [_ {:keys [::tasks/max-age ::tasks/max-age-absolute] :as cfg}] + (l/dbg :hint "initializing session gc task" + :max-age max-age + :max-age-absolute max-age-absolute) (fn [_] (db/tx-run! cfg collect-expired-tasks))) diff --git a/backend/src/app/migrations.clj b/backend/src/app/migrations.clj index c458f45150..ff9b9a4c6f 100644 --- a/backend/src/app/migrations.clj +++ b/backend/src/app/migrations.clj @@ -510,6 +510,9 @@ {:name "0155-normalize-storage-object-metadata" :fn (mg/resource "app/migrations/sql/0155-normalize-storage-object-metadata.sql")} + {:name "0155-drop-http-session-table" + :fn (mg/resource "app/migrations/sql/0155-drop-http-session-table.sql")} + {:name "0156-add-storage-object-json-dedup-index" :fn (mg/resource "app/migrations/sql/0156-add-storage-object-json-dedup-index.sql")}]) diff --git a/backend/src/app/migrations/sql/0155-drop-http-session-table.sql b/backend/src/app/migrations/sql/0155-drop-http-session-table.sql new file mode 100644 index 0000000000..09db78c632 --- /dev/null +++ b/backend/src/app/migrations/sql/0155-drop-http-session-table.sql @@ -0,0 +1,4 @@ +-- Legacy v1 HTTP sessions have been removed from the backend; the +-- http_session table is no longer read or written by any code path. + +DROP TABLE http_session; diff --git a/backend/src/app/srepl/main.clj b/backend/src/app/srepl/main.clj index b8d0a353af..7123917900 100644 --- a/backend/src/app/srepl/main.clj +++ b/backend/src/app/srepl/main.clj @@ -125,7 +125,7 @@ {:columns [:id :email]})] (when-not (:is-blocked profile) (db/update! conn :profile {:is-blocked true} {:id (:id profile)}) - (db/delete! conn :http-session {:profile-id (:id profile)}) + (session/invalidate-all system (:id profile)) :blocked)))))) (defn reset-password! diff --git a/backend/test/backend_tests/http_middleware_test.clj b/backend/test/backend_tests/http_middleware_test.clj index e058e2fb53..1651d92e0a 100644 --- a/backend/test/backend_tests/http_middleware_test.clj +++ b/backend/test/backend_tests/http_middleware_test.clj @@ -8,6 +8,8 @@ (:require [app.common.exceptions :as ex] [app.common.time :as ct] + [app.common.uuid :as uuid] + [app.config :as cf] [app.db :as db] [app.http :as-alias http] [app.http.access-token] @@ -21,6 +23,7 @@ [backend-tests.helpers :as th] [clojure.string :as str] [clojure.test :as t] + [integrant.core :as ig] [mockery.core :refer [with-mocks]] [yetti.request :as yreq] [yetti.response :as yres]) @@ -56,12 +59,11 @@ (handler (th/make-dummy-request {})) (t/is (nil? (::http/auth-data @request))) + ;; A bearer token is only attached when it is a current session + ;; token (kid=1/ver=1) and a decoder is configured. Otherwise the + ;; request stays unauthenticated. (handler (th/make-dummy-request {:headers {"authorization" "Bearer aaaa"}})) - - (let [{:keys [token claims] token-type :type} (get @request ::http/auth-data)] - (t/is (= :bearer token-type)) - (t/is (= "aaaa" token)) - (t/is (nil? claims))))) + (t/is (nil? (::http/auth-data @request))))) (t/deftest auth-middleware-3 (let [request (volatile! nil) @@ -73,11 +75,7 @@ (t/is (nil? (::http/auth-data @request))) (handler (th/make-dummy-request {:cookies {"auth-token" "foobar"}})) - - (let [{:keys [token claims] token-type :type} (get @request ::http/auth-data)] - (t/is (= :cookie token-type)) - (t/is (= "foobar" token)) - (t/is (nil? claims))))) + (t/is (nil? (::http/auth-data @request))))) (t/deftest shared-key-auth (let [handler (#'app.http.middleware/wrap-shared-key-auth @@ -230,56 +228,282 @@ (t/is (ct/inst? exp) "exp should be an instant"))) (t/deftest session-token-exp-based-on-created-at - (let [cfg th/*system* - manager (session/inmemory-manager) - profile (th/create-profile* 1) - session (->> (session/create-session manager {:profile-id (:id profile) - :user-agent "user agent"}) - (#'session/assign-token cfg)) - claims (tokens/decode cfg (:token session)) - expected-exp (ct/plus (:created-at session) (ct/duration {:days 30}))] + (let [cfg th/*system* + manager (session/inmemory-manager) + profile (th/create-profile* 1) + session (->> (session/create-session manager {:profile-id (:id profile) + :user-agent "user agent"}) + (#'session/assign-token cfg)) + claims (tokens/decode cfg (:token session)) + expected-exp (ct/plus (:created-at session) (ct/duration {:days 30}))] (t/is (some? (:exp claims)) "session token should contain :exp claim") (t/is (= (inst-ms (:exp claims)) (inst-ms expected-exp)) "exp should equal created-at + 30 days"))) (t/deftest session-token-past-exp-is-rejected - (let [cfg th/*system* - manager (session/inmemory-manager) - profile (th/create-profile* 1) - session (->> (session/create-session manager {:profile-id (:id profile) - :user-agent "user agent"}) - (#'session/assign-token cfg)) - claims (tokens/decode cfg (:token session)) - ;; Manually create a token with exp in the past + (let [cfg th/*system* + manager (session/inmemory-manager) + profile (th/create-profile* 1) + session (->> (session/create-session manager {:profile-id (:id profile) + :user-agent "user agent"}) + (#'session/assign-token cfg)) + claims (tokens/decode cfg (:token session)) past-claims (assoc claims :exp (ct/minus (ct/now) (ct/duration {:days 1}))) - header {:kid 1 :ver 1} - past-token (tokens/generate cfg past-claims header)] + past-token (tokens/generate cfg past-claims {:kid 1 :ver 1})] (t/is (nil? (session/decode-token cfg past-token)) "token with exp in the past should be rejected"))) (t/deftest session-renewal-preserves-original-exp (let [cfg th/*system* - manager (session/inmemory-manager) profile (th/create-profile* 1) + created (ct/minus (ct/now) (ct/duration {:days 1})) + session {:id (uuid/random) + :profile-id (:id profile) + :user-agent "user agent" + :created-at created + :modified-at (ct/minus (ct/now) (ct/duration {:hours 7}))} + manager (reify session/ISessionManager + (read-session [_ _] session) + (create-session [_ _] session) + (update-session [_ s] (assoc s :modified-at (ct/now))) + (delete-session [_ _] nil)) + + old-token (:token (#'session/assign-token cfg session)) + original-exp (:exp (tokens/decode cfg old-token)) + handler (-> (fn [req] req) + (#'session/wrap-authz (assoc th/*system* ::session/manager manager)) + (#'mw/wrap-auth {:bearer (partial session/decode-token cfg) + :cookie (partial session/decode-token cfg)})) + response (handler (th/make-dummy-request {:cookies {"auth-token" old-token}})) + renewed-token (get-in response [::yres/cookies "auth-token" :value]) + renewed-claims (tokens/decode cfg renewed-token)] + (t/is (some? original-exp) "original token should have :exp") + (t/is (not= old-token renewed-token) "renewal should issue a new token string") + (t/is (= (inst-ms original-exp) (inst-ms (:exp renewed-claims))) + "renewed token should preserve the original :exp, not extend it"))) + +(t/deftest session-renewal-preserves-exp-with-db-manager + (let [cfg th/*system* + manager (::session/manager th/*system*) + profile (th/create-profile* 1) + created (session/create-session manager {:profile-id (:id profile) + :user-agent "user agent"}) + _ (th/db-exec-one! ["UPDATE http_session_v2 + SET modified_at = now() - interval '7 hours' + WHERE id = ?" (:id created)]) + stale (session/read-session manager (:id created)) + old-token (:token (#'session/assign-token cfg stale)) + original-exp (:exp (tokens/decode cfg old-token)) + handler (-> (fn [req] req) + (#'session/wrap-authz cfg) + (#'mw/wrap-auth {:bearer (partial session/decode-token cfg) + :cookie (partial session/decode-token cfg)})) + response (handler (th/make-dummy-request {:cookies {"auth-token" old-token}})) + renewed (get-in response [::yres/cookies "auth-token" :value]) + renewed-exp (:exp (tokens/decode cfg renewed)) + expected-exp (ct/plus (:created-at created) (ct/duration {:days 30})) + current (session/read-session manager (:id created))] + (t/is (some? original-exp) "original token should have :exp") + (t/is (some? renewed) "renewal should issue a new cookie token") + (t/is (not= old-token renewed) "renewal should issue a new token string") + (t/is (= (inst-ms original-exp) (inst-ms renewed-exp)) + "renewed token should preserve the original :exp, not extend it") + (t/is (= (inst-ms expected-exp) (inst-ms renewed-exp)) + "renewed :exp should equal created-at + 30 days") + (t/is (some? current) "session row must still exist after renewal") + (t/is (pos? (compare (:modified-at current) (:modified-at stale))) + "persisted modified_at must move forward on renewal"))) + +(t/deftest session-renewal-cookie-expires-diverges-from-token-exp + (let [cfg th/*system* + manager (::session/manager th/*system*) + profile (th/create-profile* 1) + created (session/create-session manager {:profile-id (:id profile) + :user-agent "user agent"}) + _ (th/db-exec-one! ["UPDATE http_session_v2 + SET created_at = now() - interval '29 days', + modified_at = now() - interval '7 hours' + WHERE id = ?" (:id created)]) + stale (session/read-session manager (:id created)) + old-token (:token (#'session/assign-token cfg stale)) + handler (-> (fn [req] req) + (#'session/wrap-authz cfg) + (#'mw/wrap-auth {:bearer (partial session/decode-token cfg) + :cookie (partial session/decode-token cfg)})) + response (handler (th/make-dummy-request {:cookies {"auth-token" old-token}})) + cookie (get-in response [::yres/cookies "auth-token"]) + renewed (:value cookie) + renewed-exp (:exp (tokens/decode cfg renewed)) + expected-exp (ct/plus (:created-at stale) (ct/duration {:days 30})) + close-to? (fn [a b tolerance-ms] + (<= (Math/abs (- (inst-ms a) (inst-ms b))) tolerance-ms))] + (t/is (some? renewed) "renewal should issue a new cookie token") + (t/is (not= old-token renewed) "renewal should issue a new token string") + (t/is (= (inst-ms expected-exp) (inst-ms renewed-exp)) + "renewed :exp should equal created-at + 30 days") + (t/is (close-to? renewed-exp (ct/plus (ct/now) (ct/duration {:days 1})) + (* 10 60 1000)) + "renewed :exp should be ~1 day out (absolute cap is near)") + (t/is (close-to? (:expires cookie) (ct/plus (ct/now) (ct/duration {:days 7})) + (* 10 60 1000)) + "cookie Expires should slide ~7 days out from now") + (t/is (pos? (compare (:expires cookie) renewed-exp)) + "cookie Expires should stay ahead of the token :exp"))) + +(t/deftest legacy-session-token-is-rejected + (let [cfg th/*system* + manager (session/inmemory-manager) handler (-> (fn [req] req) - (#'session/wrap-authz {::session/manager manager}) + (#'session/wrap-authz {::session/manager manager}) (#'mw/wrap-auth {:bearer (partial session/decode-token cfg) :cookie (partial session/decode-token cfg)})) - session (->> (session/create-session manager {:profile-id (:id profile) - :user-agent "user agent"}) - (#'session/assign-token cfg)) - original-exp (:exp (tokens/decode cfg (:token session))) - ;; Force renewal by setting modified-at to 7 hours ago - old-session (assoc session :modified-at (ct/minus (ct/now) (ct/duration {:hours 7}))) - response (handler (th/make-dummy-request {:cookies {"auth-token" (:token old-session)}})) - {:keys [token claims]} (get response ::http/auth-data) - new-exp (:exp claims)] - (t/is (some? original-exp) "original token should have :exp") - (t/is (some? new-exp) "renewed token should have :exp") - (t/is (= (inst-ms original-exp) - (inst-ms new-exp)) - "renewed token should preserve original :exp, not extend it"))) + token (tokens/generate cfg {:sid "legacy-session-id"} {:kid 0 :ver 0}) + response (handler (th/make-dummy-request {:cookies {"auth-token" token}}))] + (t/is (nil? (get response ::http/auth-data)) + "legacy tokens must not be attached as auth data") + (t/is (nil? (::session/profile-id response)) + "legacy tokens must not authenticate"))) + +(t/deftest session-gc-deletes-idle-and-absolute-expired-rows + (let [profile (th/create-profile* 1) + fresh (uuid/random) + idle (uuid/random) + absolute (uuid/random) + valid (uuid/random)] + + (th/db-exec-one! ["INSERT INTO http_session_v2 (id, profile_id, created_at, modified_at) + VALUES (?, ?, now(), now())" + fresh (:id profile)]) + (th/db-exec-one! ["INSERT INTO http_session_v2 (id, profile_id, created_at, modified_at) + VALUES (?, ?, now() - interval '1 day', now() - interval '8 days')" + idle (:id profile)]) + (th/db-exec-one! ["INSERT INTO http_session_v2 (id, profile_id, created_at, modified_at) + VALUES (?, ?, now() - interval '31 days', now())" + absolute (:id profile)]) + (th/db-exec-one! ["INSERT INTO http_session_v2 (id, profile_id, created_at, modified_at) + VALUES (?, ?, now() - interval '1 day', now() - interval '6 days')" + valid (:id profile)]) + + (db/tx-run! th/*system* + (fn [cfg] + (#'session/collect-expired-tasks + (assoc cfg + :app.http.session.tasks/max-age (ct/duration {:days 7}) + :app.http.session.tasks/max-age-absolute (ct/duration {:days 30}))))) + + (let [ids (->> (th/db-exec! ["SELECT id FROM http_session_v2 WHERE profile_id = ?" (:id profile)]) + (map :id) + (set))] + (t/is (contains? ids fresh) "fresh session must be kept") + (t/is (contains? ids valid) "session within both windows must be kept") + (t/is (not (contains? ids idle)) "idle session must be deleted") + (t/is (not (contains? ids absolute)) "session past the absolute cap must be deleted")))) + +(t/deftest session-gc-config-wiring + (let [idle (ct/duration {:days 3}) + absolute (ct/duration {:days 10})] + (with-redefs [cf/get (fn + ([k] (case k + :auth-token-cookie-max-age idle + :auth-token-cookie-max-age-absolute absolute + nil)) + ([k default] (case k + :auth-token-cookie-max-age idle + :auth-token-cookie-max-age-absolute absolute + default)))] + (let [expanded (ig/expand-key :app.http.session.tasks/gc {})] + (t/is (= idle + (get-in expanded [:app.http.session.tasks/gc + :app.http.session.tasks/max-age])) + "task max-age should carry the configured idle window") + (t/is (= absolute + (get-in expanded [:app.http.session.tasks/gc + :app.http.session.tasks/max-age-absolute])) + "task max-age-absolute should carry the configured absolute cap"))) + (with-redefs [cf/get (fn + ([_k] nil) + ([_k default] default))] + (let [expanded (ig/expand-key :app.http.session.tasks/gc {})] + (t/is (= session/default-cookie-max-age + (get-in expanded [:app.http.session.tasks/gc + :app.http.session.tasks/max-age])) + "task max-age should fall back to the default idle window") + (t/is (= session/default-cookie-max-age-absolute + (get-in expanded [:app.http.session.tasks/gc + :app.http.session.tasks/max-age-absolute])) + "task max-age-absolute should fall back to the default absolute cap"))))) + +(t/deftest session-gc-rejects-absolute-below-idle + (let [pool (:app.db/pool th/*system*) + params {:app.db/pool pool + :app.http.session.tasks/max-age (ct/duration {:days 7}) + :app.http.session.tasks/max-age-absolute (ct/duration {:days 30})}] + (t/is (nil? (ig/assert-key :app.http.session.tasks/gc params)) + "absolute cap above the idle window should pass") + (t/is (nil? (ig/assert-key :app.http.session.tasks/gc + (assoc params + :app.http.session.tasks/max-age-absolute + (ct/duration {:days 7})))) + "absolute cap equal to the idle window should pass") + (t/is (thrown? IllegalArgumentException + (ig/assert-key :app.http.session.tasks/gc + (assoc params + :app.http.session.tasks/max-age-absolute + (ct/duration {:days 3})))) + "absolute cap below the idle window should fail fast"))) + +(t/deftest idle-expired-session-is-rejected + (let [cfg th/*system* + profile (th/create-profile* 1) + updated? (atom false) + session {:id (uuid/random) + :profile-id (:id profile) + :user-agent "user agent" + :created-at (ct/now) + :modified-at (ct/minus (ct/now) (ct/duration {:days 8}))} + manager (reify session/ISessionManager + (read-session [_ _] session) + (create-session [_ _] session) + (update-session [_ s] (reset! updated? true) s) + (delete-session [_ _] nil)) + token (:token (#'session/assign-token cfg session)) + handler (-> (fn [req] req) + (#'session/wrap-authz (assoc cfg ::session/manager manager)) + (#'mw/wrap-auth {:bearer (partial session/decode-token cfg) + :cookie (partial session/decode-token cfg)})) + cookie-r (handler (th/make-dummy-request {:cookies {"auth-token" token}})) + bearer-r (handler (th/make-dummy-request {:headers {"authorization" (str "Bearer " token)}}))] + (t/is (nil? (::session/profile-id cookie-r)) + "idle-expired session must not authenticate via cookie") + (t/is (nil? (::session/session cookie-r)) + "idle-expired session must not be attached via cookie") + (t/is (nil? (::session/profile-id bearer-r)) + "idle-expired session must not authenticate via bearer") + (t/is (false? @updated?) + "idle-expired session must not be renewed"))) + +(t/deftest idle-session-within-window-is-accepted + (let [cfg th/*system* + profile (th/create-profile* 1) + session {:id (uuid/random) + :profile-id (:id profile) + :user-agent "user agent" + :created-at (ct/now) + :modified-at (ct/minus (ct/now) (ct/duration {:days 6}))} + manager (reify session/ISessionManager + (read-session [_ _] session) + (create-session [_ _] session) + (update-session [_ s] (assoc s :modified-at (ct/now))) + (delete-session [_ _] nil)) + token (:token (#'session/assign-token cfg session)) + handler (-> (fn [req] req) + (#'session/wrap-authz (assoc cfg ::session/manager manager)) + (#'mw/wrap-auth {:bearer (partial session/decode-token cfg) + :cookie (partial session/decode-token cfg)})) + response (handler (th/make-dummy-request {:cookies {"auth-token" token}}))] + (t/is (= (:id profile) (::session/profile-id response)) + "session within the idle window must authenticate"))) (t/deftest parse-request-illegal-argument-exception ;; clojure.data.json raises IllegalArgumentException (case diff --git a/docs/technical-guide/configuration.md b/docs/technical-guide/configuration.md index 9fd0282761..d730abd8fc 100644 --- a/docs/technical-guide/configuration.md +++ b/docs/technical-guide/configuration.md @@ -643,6 +643,44 @@ And configure it: PENPOT_SECRET_KEY: my-super-secure-key ``` +### Session expiration + +__Since version 2.19.0__ + +User sessions are stored server-side and expire on two independent conditions: an +**idle timeout** and an **absolute maximum lifetime**. Both are backend only. + +```bash +# Backend +# Idle timeout: the session stops working after this much inactivity. +# Default: 168h +PENPOT_AUTH_TOKEN_COOKIE_MAX_AGE: 168h + +# Absolute maximum lifetime from the moment the session was created, +# regardless of activity. Default: 720h +PENPOT_AUTH_TOKEN_COOKIE_MAX_AGE_ABSOLUTE: 720h +``` + +Durations use the `` form with hour, minute or second units, +for example `168h`, `30m` or `90s` (day units like `7d` are not accepted). + +While a user is active the session is automatically renewed every 6 hours (not +configurable). Renewal extends the cookie, but never the absolute maximum. The +backend checks the idle timeout on every request, measured from the last +renewal: once it passes, the session stops working at once, even for a copied +token. A daily task (`session-gc`) then deletes the sessions that have exceeded +either window. Legacy v1 sessions and the old `http_session` table are no +longer used. + +Sessions created before 2.19.0 carry no `:exp` in their token; they are still +removed by the 30-day `created_at` cleanup and acquire `:exp` on their next +renewal. + +The `secure` and `same-site` attributes of the session cookie are controlled by +the `disable-secure-session-cookies`, `strict-session-cookies` and `enable-cors` +flags, and by whether `PENPOT_PUBLIC_URI` is served over HTTPS. See +[Penpot URI](#penpot-uri). + ### Database Penpot only supports PostgreSQL and we highly recommend >=13 version. If you are using official diff --git a/docs/technical-guide/developer/subsystems/authentication.md b/docs/technical-guide/developer/subsystems/authentication.md index da437194ee..b5946696ac 100644 --- a/docs/technical-guide/developer/subsystems/authentication.md +++ b/docs/technical-guide/developer/subsystems/authentication.md @@ -94,14 +94,45 @@ Similarly as the OIDC backend, it checks if the profile exists, and calls ## Sessions -User sessions are created when a user logs in via any one of the backends. A -session token is generated (a JWT token that does not currently contain any data) -and returned to frontend as a cookie. +User sessions are created when a user logs in via any one of the backends. The +backend generates a signed JWT token and returns it to the frontend as an +auth-token cookie. A matching row is stored in +the http_session_v2 table with the profile id +and the session timestamps. -Normally the session is stored in a DB table with the information of the user -profile and the session expiration. But if a frontend connects to the backend in -"read only" mode (for example, to debug something in production with the local -devenv), sessions are stored in memory (may be lost if the backend restarts). +A request is authenticated only when the token verifies, its session row still +exists and the session is within the idle timeout. The token claims carry the session row id (:sid), the last activity instant (:iat) and an absolute expiration (:exp). The server enforces two independent +limits: + +* **Idle timeout:** a session that is not renewed within + PENPOT_AUTH_TOKEN_COOKIE_MAX_AGE (default 7 + days) stops working at once. The backend checks the session's last renewal + time on every request, so the row does not need to be deleted first. +* **Absolute maximum:** a session cannot live longer than + PENPOT_AUTH_TOKEN_COOKIE_MAX_AGE_ABSOLUTE + (default 30 days) from its creation, no matter how much it is renewed. The + :exp claim enforces this even when the + cookie is still present. + +Sessions are automatically renewed every 6 hours of use (not configurable). +Renewal issues a new token but keeps the same session row, so the absolute +maximum is not extended. A daily garbage collector +(session-gc) deletes rows that exceed either +the idle window or the absolute maximum. + +Sessions created before 2.19.0 carry no :exp in their token; they are still removed by +the 30-day created_at cleanup and acquire :exp on their next renewal. + +The normal storage is the database. When the backend uses a read-only database +pool (for example, to debug something in production with the local devenv), +sessions are kept in memory and are lost when the backend restarts. The +organization SSO gate keeps an additional 4-hour entry inside the same session +row, separate from the session token lifetime. ## Team invitations