diff --git a/backend/src/app/config.clj b/backend/src/app/config.clj index 6f69047382..d338bb4813 100644 --- a/backend/src/app/config.clj +++ b/backend/src/app/config.clj @@ -58,6 +58,7 @@ :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 " @@ -206,6 +207,7 @@ [:auth-token-cookie-name {:optional true} :string] [:auth-token-cookie-max-age {:optional true} ::ct/duration] + [:auth-token-cookie-max-age-absolute {:optional true} ::ct/duration] [:registration-domain-whitelist {:optional true} [::sm/set :string]] [:email-verify-threshold {:optional true} ::ct/duration] diff --git a/backend/src/app/http/session.clj b/backend/src/app/http/session.clj index 914dfc169c..288fdc4396 100644 --- a/backend/src/app/http/session.clj +++ b/backend/src/app/http/session.clj @@ -36,6 +36,9 @@ ;; Default age for automatic session renewal (def default-renewal-max-age (ct/duration {:hours 6})) +;; Default absolute maximum session duration +(def default-cookie-max-age-absolute (ct/duration {:days 30})) + ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; PROTOCOLS ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; @@ -169,15 +172,19 @@ (defn- assign-token [cfg session] - (let [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)} - header {:kid 1 :ver 1} - token (tokens/generate cfg claims header)] + (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))) (defn create-fn @@ -353,15 +360,23 @@ or (updated_at is null and created_at < ?::timestamptz)") +(def ^:private + sql:delete-expired-v2 + "DELETE FROM http_session_v2 + WHERE created_at < ?::timestamptz") + (defn- collect-expired-tasks [{:keys [::db/conn ::tasks/max-age]}] (let [threshold (ct/minus (ct/now) max-age) - result (-> (db/exec-one! conn [sql:delete-expired threshold threshold]) - (db/get-update-count))] + 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))] (l/dbg :task "gc" :hint "clean http sessions" - :deleted result) - result)) + :deleted-legacy result-legacy + :deleted-v2 result-v2) + (+ result-legacy result-v2))) (defmethod ig/init-key ::tasks/gc [_ {:keys [::tasks/max-age] :as cfg}] diff --git a/backend/test/backend_tests/http_middleware_test.clj b/backend/test/backend_tests/http_middleware_test.clj index 6ec83924be..e5f3f48863 100644 --- a/backend/test/backend_tests/http_middleware_test.clj +++ b/backend/test/backend_tests/http_middleware_test.clj @@ -277,6 +277,70 @@ (t/is (= (:id session) (:sid claims))) (t/is (= (:id profile) (:uid claims))))) +(t/deftest session-token-contains-exp-claim + (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)) + exp (:exp claims)] + (t/is (some? exp) "session token should contain :exp claim") + (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}))] + (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 + 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)] + (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) + handler (-> (fn [req] req) + (#'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 (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"))) + (t/deftest parse-request-illegal-argument-exception ;; clojure.data.json raises IllegalArgumentException (case ;; fall-through) on several kinds of malformed input. The