diff --git a/backend/src/app/http/auth_request.clj b/backend/src/app/http/auth_request.clj index b0057519077..a77be7d84f8 100644 --- a/backend/src/app/http/auth_request.clj +++ b/backend/src/app/http/auth_request.clj @@ -20,6 +20,7 @@ [app.common.logging :as l] [app.config :as cf] [app.db :as db] + [app.http :as-alias http] [app.http.access-token :as-alias actoken] [app.http.session :as session] [app.rpc.commands.auth :as auth] @@ -76,62 +77,213 @@ ;; MIDDLEWARE ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +;; The profile-lookup try/catch (below) narrows what counts as +;; "transient" to known DB/IO failure classes — Postgres exceptions, +;; JDBC SQL exceptions, JDBC connection exceptions, java.io.IOException. +;; A broader `catch Throwable` would also swallow validation errors, +;; bad-input parses, and programming bugs, masking them as transient +;; failures that silently extend a stale session. On transient +;; failure we preserve whatever wrap-session decided and pass through +;; unchanged — graceful degradation, but only for the failure modes +;; that genuinely warrant it. Any other exception escapes the catch +;; and surfaces through the standard error handler as a 500. + +(def ^:private transient-exception-classes + #{java.sql.SQLException + java.sql.SQLTransientException + java.sql.SQLNonTransientConnectionException + java.io.IOException + org.postgresql.util.PSQLException}) + +(defn- transient-exception? + "True iff cause (or any cause-chain ancestor) is an instance of a + class we treat as transient. Walks the cause chain so a wrapped + ExecutionException with a SQLException root still classifies." + [cause] + (loop [ex cause] + (cond + (nil? ex) false + (some #(instance? % ex) transient-exception-classes) true + :else (recur (.getCause ^Throwable ex))))) + +(defn- clear-stale-session + "Returns a request with the local-session keys AND ::http/auth-data + removed. Both must be cleared together because downstream handlers + read identity from BOTH `::session/profile-id` (the post-wrap-authz + shorthand) and `::http/auth-data.claims.uid` (used by errors.clj for + log enrichment on cookie-auth requests). Leaving auth-data in place + after re-keying or after a mismatch flush leaks the stale identity + to that reader. + + (access_token.clj reads ::http/auth-data too, but only branches on + `:type :token` — the API-key path is short-circuited before we get + here, so it's not relevant to the cookie-auth flow this middleware + reconciles.)" + [request] + (dissoc request + ::session/profile-id + ::session/session + ::http/auth-data)) + +(defn- with-session-id + "Attach ::session/id from the decoded local session map when present so + session/delete-fn can remove the backing server-side row." + [request] + (if-let [sid (some-> request ::session/session :id)] + (assoc request ::session/id sid) + request)) + +(defn- response-has-auth-cookie? + "True when the handler already wrote the auth-token cookie on the + response. Used to avoid clobbering an explicit logout / re-issue + written by a downstream handler with a fresh re-key cookie. Same + guard shape as session.clj's renewal path." + [response] + (contains? (::yres/cookies response) + (cf/get :auth-token-cookie-name))) + (defn- wrap-authz [handler cfg] (fn [request] - ;; Skip when a prior middleware (session or access-token) already - ;; resolved a profile — we only act as a fallback. - (if (or (some? (::session/profile-id request)) - (some? (::actoken/profile-id request))) - (handler request) - (let [email-claim (yreq/get-header request "x-auth-request-email")] - (if (str/blank? email-claim) - (handler request) - (let [local-part (first (str/split email-claim #"@")) - email (resolve-email email-claim) - fullname (or (not-empty (yreq/get-header request "x-auth-request-user")) - local-part) - profile (try - (get-or-register-profile cfg email fullname) - (catch Throwable cause - (l/err :hint "x-auth-request: error resolving profile" - :email email - :cause cause) - nil))] - (cond - (nil? profile) - (do - (l/wrn :hint "x-auth-request: no profile found for email, passing through unauthenticated" - :email email) - (handler request)) - - (:is-blocked profile) - (do - (l/wrn :hint "x-auth-request: profile is blocked, denying access" - :email email - :profile-id (str (:id profile))) - {::yres/status 403}) - - (not (:is-active profile)) - (do - (l/wrn :hint "x-auth-request: profile is not active, denying access" - :email email - :profile-id (str (:id profile))) - {::yres/status 403}) - - :else - (do - (l/dbg :hint "x-auth-request: authenticating via forwarded header" - :email email - :profile-id (str (:id profile))) - (let [create-session! (session/create-fn cfg profile) - ;; Inject profile-id into the request so this very - ;; request is also treated as authenticated downstream. - response (-> request - (assoc ::session/profile-id (:id profile)) - handler)] - ;; Attach a session cookie so the browser is authenticated - ;; for all subsequent requests without needing to log in. + (let [atoken-pid (::actoken/profile-id request) + session-pid (::session/profile-id request) + email-claim (yreq/get-header request "x-auth-request-email")] + (cond + ;; Access-token (API key) — programmatic identity issued out-of-band + ;; by the user. Not a browser SSO session, so the header is not + ;; meaningful here. Pass through unconditionally. + (some? atoken-pid) + (handler request) + + ;; No proxy header — trust whatever wrap-session decided (session + ;; cookie, or anonymous). Without a header we have no upstream + ;; identity to compare against. + (str/blank? email-claim) + (handler request) + + :else + (let [local-part (first (str/split email-claim #"@")) + email (resolve-email email-claim) + fullname (or (not-empty (yreq/get-header request "x-auth-request-user")) + local-part) + profile-state (try + {:status :ok + :profile (get-or-register-profile cfg email fullname)} + (catch Throwable cause + (if (transient-exception? cause) + (do + (l/err :hint "x-auth-request: transient error resolving profile, preserving session" + :email email + :cause cause) + {:status :error + :cause cause}) + ;; Non-transient — programming bug, validation + ;; failure, bad input. Rethrow so the standard + ;; error pipeline returns 500 instead of + ;; silently masking it as a "preserve session" + ;; pass-through. + (throw cause)))) + profile (:profile profile-state)] + (cond + ;; Transient failure resolving the proxy identity. We have NO + ;; basis to flush the existing session (a Postgres restart or + ;; network blip would otherwise log everyone out), so preserve + ;; whatever wrap-session decided. The next successful request + ;; reconciles normally. + (= :error (:status profile-state)) + (do + (l/wrn :hint "x-auth-request: preserving local auth state because profile resolution failed" + :email email + :session-profile-id (some-> session-pid str)) + (handler request)) + + (nil? profile) + ;; Header email doesn't resolve to a profile (and auto-register + ;; is off). The upstream identity is something the local DB + ;; doesn't know — we cannot safely keep serving whatever session + ;; cookie alice happens to have in this browser, because the + ;; upstream says alice is no longer the active identity. Clear + ;; the in-flight local session markers AND auth-data, then + ;; expire the browser's auth-token cookie so subsequent + ;; requests cannot resurrect the stale local session. The + ;; request then continues unauthenticated (downstream handlers + ;; will respond with 401/redirect-to-login per their own rules). + (do + (l/wrn :hint "x-auth-request: no profile found for email, clearing local session" + :email email + :session-profile-id (some-> session-pid str)) + (let [delete-session! (session/delete-fn cfg) + cleared (clear-stale-session request) + response (handler cleared)] + ;; delete-fn reads ::session/id from the request. Preserve + ;; the original request shape for deletion and project id + ;; from the decoded ::session/session map when available. + (delete-session! (with-session-id request) response))) + + ;; Blocked / inactive incoming identity. Return 403 — and if + ;; there was a session for a DIFFERENT user, flush it too. The + ;; openspec Rule 2 contract (identity mismatch SHALL flush) + ;; applies regardless of whether the new identity is usable; + ;; otherwise alice's session survives an attempted switch to + ;; the blocked user bob and the next request still serves alice. + (or (:is-blocked profile) + (not (:is-active profile))) + (do + (l/wrn :hint (if (:is-blocked profile) + "x-auth-request: profile is blocked, denying access" + "x-auth-request: profile is not active, denying access") + :email email + :profile-id (str (:id profile)) + :session-profile-id (some-> session-pid str)) + (let [response {::yres/status 403}] + (if (and session-pid (not= session-pid (:id profile))) + ;; Identity mismatch with a refused incoming user — also + ;; clear the existing local session cookie so alice + ;; doesn't keep her session via the still-valid + ;; auth-token cookie on her browser. + (let [delete-session! (session/delete-fn cfg)] + (delete-session! (with-session-id request) response)) + response))) + + ;; Existing browser session matches the proxy-asserted identity. + ;; Steady-state case — no work to do. + (and session-pid (= session-pid (:id profile))) + (handler request) + + ;; Either no existing session, or the session points at a + ;; *different* profile than oauth2-proxy is asserting. Re-key. + ;; + ;; Re-keying is what fixes the stale-session bug after the + ;; portal "log out of all apps" + new-user login pattern: + ;; oauth2-proxy + Cognito are cleared, but Penpot's own + ;; auth-token cookie on its subdomain survives. Without this + ;; branch, wrap-session would resolve the old session-pid and + ;; this middleware (under the previous always-skip-when-session + ;; rule) would never override it. + :else + (do + (when session-pid + (l/inf :hint "x-auth-request: proxy identity differs from existing session — re-keying" + :session-profile-id (str session-pid) + :header-profile-id (str (:id profile)))) + (l/dbg :hint "x-auth-request: authenticating via forwarded header" + :email email + :profile-id (str (:id profile))) + (let [create-session! (session/create-fn cfg profile) + response (-> request + clear-stale-session + (assoc ::session/profile-id (:id profile)) + handler)] + ;; Issue a fresh auth-token cookie unless the downstream + ;; handler already wrote the auth-token cookie on the + ;; response (e.g. an explicit logout that called + ;; clear-session-cookie, or an auth endpoint that re-issued + ;; the cookie itself). Otherwise the re-key would clobber + ;; the handler's intent — most visibly, it would prevent a + ;; user from ever completing a logout while the SSO + ;; cookie is still present. + (if (response-has-auth-cookie? response) + response (create-session! request response)))))))))) (def authz diff --git a/backend/src/app/http/session.clj b/backend/src/app/http/session.clj index bcbb2517a02..1fc1614f780 100644 --- a/backend/src/app/http/session.clj +++ b/backend/src/app/http/session.clj @@ -248,12 +248,17 @@ (binding [ct/*clock* (clock/get-clock (:profile-id session))] (handler request))] - (if (and session (renew-session? session)) - (let [session (->> session - (update-session manager) - (assign-token cfg))] - (assign-session-cookie response session)) - response)) + (let [status (::yres/status response)] + (if (and session + (renew-session? session) + (or (nil? status) (< status 400)) + (not (contains? (::yres/cookies response) + (cf/get :auth-token-cookie-name)))) + (let [session (->> session + (update-session manager) + (assign-token cfg))] + (assign-session-cookie response session)) + response))) (= type :bearer) (let [session (case (:ver metadata) diff --git a/backend/test/backend_tests/http_middleware_test.clj b/backend/test/backend_tests/http_middleware_test.clj index 78174841de3..0aa40bc692c 100644 --- a/backend/test/backend_tests/http_middleware_test.clj +++ b/backend/test/backend_tests/http_middleware_test.clj @@ -140,6 +140,27 @@ (t/is (= (:id session) (:sid claims))) (t/is (= (:id profile) (:uid claims))))) +(t/deftest session-authz-does-not-renew-on-error-response + (let [cfg th/*system* + manager (session/inmemory-manager) + profile (th/create-profile* 91) + t0 (ct/inst "2025-01-01T00:00:00Z") + t1 (ct/plus t0 (ct/duration {:seconds 2})) + threshold (ct/duration {:seconds 1}) + handler (-> (fn [_req] {::yres/status 403}) + (#'session/wrap-authz {::session/manager manager}) + (#'mw/wrap-auth {:bearer (partial session/decode-token cfg) + :cookie (partial session/decode-token cfg)})) + token (binding [ct/*clock* (ct/fixed-clock t0)] + (->> (session/create-session manager {:profile-id (:id profile) + :user-agent "user agent"}) + (#'session/assign-token cfg))) + response (binding [cf/config (assoc cf/config :auth-token-cookie-renewal-max-age threshold) + ct/*clock* (ct/fixed-clock t1)] + (handler (->DummyRequest {} {"auth-token" (:token token)})))] + (t/is (= 403 (::yres/status response))) + (t/is (not (contains? (::yres/cookies response) "auth-token"))))) + ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; X-Auth-Request middleware tests ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; @@ -156,17 +177,134 @@ (handler (->DummyRequest {} {})) (t/is (nil? (::session/profile-id @captured))))) -(t/deftest x-auth-request-skips-when-session-present - (let [profile-id (random-uuid) - called? (volatile! false) - handler (#'app.http.auth-request/wrap-authz - (fn [req] (vreset! called? true) req) - (make-xauth-cfg)) - request (-> (->DummyRequest {"x-auth-request-email" "user@example.com"} {}) - (assoc ::session/profile-id profile-id)) - result (handler request)] - ;; profile-id must pass through unchanged — middleware must not overwrite it - (t/is (= profile-id (::session/profile-id result))))) +(t/deftest x-auth-request-drops-local-session-when-header-email-unresolvable + ;; When wrap-session has resolved alice's profile-id from the local + ;; auth-token cookie, and the proxy header asserts an email that does + ;; NOT resolve to a Penpot profile (unknown upstream user + auto-register + ;; off), the middleware MUST drop alice's session-pid before calling + ;; the downstream handler. The upstream identity has changed; continuing + ;; to serve alice would leak her data to whoever is now upstream. + ;; + ;; Per openspec proxy-auth-middleware Rule 2: "Identity mismatch SHALL + ;; flush the existing session immediately", even when we cannot re-key + ;; to a known new identity. + (let [profile-id (random-uuid) + stale-session {:id (random-uuid) :profile-id profile-id} + captured (volatile! nil) + handler (#'app.http.auth-request/wrap-authz + (fn [req] (vreset! captured req) req) + (make-xauth-cfg)) + request (-> (->DummyRequest {"x-auth-request-email" "user@example.com"} {}) + (assoc ::session/profile-id profile-id) + (assoc ::session/session stale-session)) + response (handler request)] + ;; Downstream handler must NOT see alice's profile-id. + (t/is (nil? (::session/profile-id @captured))) + ;; Downstream handler must NOT see alice's stale local session. + (t/is (nil? (::session/session @captured))) + ;; Browser cookie is explicitly expired. + (t/is (= 0 (get-in response [::yres/cookies "auth-token" :max-age]))))) + +(t/deftest x-auth-request-preserves-local-session-when-profile-lookup-errors + (let [profile-id (random-uuid) + stale-session {:id (random-uuid) :profile-id profile-id} + captured (volatile! nil) + handler (#'app.http.auth-request/wrap-authz + (fn [req] (vreset! captured req) req) + (make-xauth-cfg)) + request (-> (->DummyRequest {"x-auth-request-email" "user@example.com"} {}) + (assoc ::session/profile-id profile-id) + (assoc ::session/session stale-session)) + response (with-redefs [app.http.auth-request/get-or-register-profile + (fn [& _] + (throw (java.sql.SQLException. "db down")))] + (handler request))] + ;; Operational errors should not be treated as "unknown user" and + ;; destructively clear local session state. + (t/is (= profile-id (::session/profile-id @captured))) + (t/is (= stale-session (::session/session @captured))) + (t/is (not (contains? (::yres/cookies response) "auth-token"))))) + +(t/deftest x-auth-request-rekeys-when-session-identity-differs + ;; Repro of the QA-reported bug: alice's auth-token cookie persists on + ;; Penpot's subdomain after the portal "log out of all apps"; bob then + ;; logs in upstream. wrap-session resolves alice's profile-id from the + ;; old cookie, but oauth2-proxy is forwarding bob's email. The middleware + ;; must re-key to bob. + (let [alice (th/create-profile* 1 {:is-active true}) + bob (th/create-profile* 2 {:is-active true}) + captured (volatile! nil) + cfg (make-xauth-cfg) + handler (#'app.http.auth-request/wrap-authz + (fn [req] (vreset! captured req) {::yres/status 200}) + cfg) + request (-> (->DummyRequest {"x-auth-request-email" (:email bob)} {}) + (assoc ::session/profile-id (:id alice)) + (assoc ::session/session {:id (random-uuid) + :profile-id (:id alice)})) + response (handler request)] + ;; Downstream handler sees bob's profile-id, not alice's. + (t/is (= (:id bob) (::session/profile-id @captured))) + ;; Stale local session object is removed before downstream handling. + (t/is (nil? (::session/session @captured))) + ;; A fresh auth-token cookie is issued for bob's session. + (t/is (contains? (::yres/cookies response) "auth-token")))) + +(t/deftest x-auth-request-no-rekey-when-session-matches-header + ;; Steady-state: the browser session matches the proxy identity. No + ;; re-key, no new cookie — the session passes through cleanly. This + ;; guards against issuing a fresh cookie on every authenticated request. + (let [profile (th/create-profile* 1 {:is-active true}) + captured (volatile! nil) + cfg (make-xauth-cfg) + handler (#'app.http.auth-request/wrap-authz + (fn [req] (vreset! captured req) {::yres/status 200}) + cfg) + request (-> (->DummyRequest {"x-auth-request-email" (:email profile)} {}) + (assoc ::session/profile-id (:id profile))) + response (handler request)] + (t/is (= (:id profile) (::session/profile-id @captured))) + ;; No new auth-token cookie when the session already matches. + (t/is (not (contains? (::yres/cookies response) "auth-token"))))) + +(t/deftest x-auth-request-rekey-not-overwritten-by-session-renewal + ;; Integration guard: session/authz wraps x-auth-request and can renew the + ;; incoming cookie after inner middleware returns. Ensure bob's re-keyed + ;; cookie wins even when renewal is forced. + (let [alice (th/create-profile* 1 {:is-active true}) + bob (th/create-profile* 2 {:is-active true}) + cfg (make-xauth-cfg) + t0 (ct/inst "2025-01-01T00:00:00Z") + t1 (ct/plus t0 (ct/duration {:seconds 2})) + ;; Keep this lower than (t1 - t0) so the stale incoming session is + ;; always considered due for renewal in this test. + renewal-threshold (ct/duration {:seconds 1}) + middleware (-> (fn [req] {::yres/status 200 + :seen-profile-id (::session/profile-id req)}) + (#'app.http.auth-request/wrap-authz cfg) + (#'session/wrap-authz cfg) + (#'mw/wrap-auth {:bearer (partial session/decode-token cfg) + :cookie (partial session/decode-token cfg)})) + seeded-token (binding [ct/*clock* (ct/fixed-clock t0)] + (get-in ((session/create-fn cfg alice) + (->DummyRequest {} {}) + {::yres/status 200}) + [::yres/cookies "auth-token" :value])) + response (binding [cf/config (assoc cf/config + ;; Renewal is triggered once elapsed age exceeds + ;; this threshold. + :auth-token-cookie-renewal-max-age + renewal-threshold) + ct/*clock* (ct/fixed-clock t1)] + (middleware (->DummyRequest {"x-auth-request-email" (:email bob)} + {"auth-token" seeded-token}))) + rekeyed-token (get-in response [::yres/cookies "auth-token" :value]) + followup (middleware (->DummyRequest {} {"auth-token" rekeyed-token}))] + (t/is (neg? (compare renewal-threshold (ct/diff t0 t1)))) + (t/is (some? rekeyed-token)) + (t/is (not= seeded-token rekeyed-token)) + (t/is (= (:id bob) (:seen-profile-id response))) + (t/is (= (:id bob) (:seen-profile-id followup))))) (t/deftest x-auth-request-skips-when-access-token-present (let [profile-id (random-uuid) @@ -216,6 +354,181 @@ (handler (->DummyRequest {"x-auth-request-email" "nobody@example.com"} {})) (t/is (nil? (::session/profile-id @captured))))) +(t/deftest x-auth-request-preserves-session-on-transient-lookup-error + ;; A Postgres blip / network timeout while resolving the header email's + ;; profile MUST NOT be conflated with "email doesn't resolve". The + ;; former is transient and the local session should survive; the + ;; latter is an identity mismatch and the session should flush. + ;; Without this guard, a single DB error would silently log every + ;; SSO user out. + ;; + ;; Graceful degradation: on transient failure pass through with + ;; whatever wrap-session decided. Downstream gets alice's existing + ;; session-pid (or anonymous if none), and the next successful + ;; request reconciles normally. + (with-mocks [_ {:target 'app.http.auth-request/get-or-register-profile + :return (fn [& _] (throw (java.sql.SQLException. "db boom")))}] + (let [captured (volatile! nil) + profile-id (random-uuid) + cfg (make-xauth-cfg) + handler (#'app.http.auth-request/wrap-authz + (fn [req] (vreset! captured req) {::yres/status 200}) + cfg) + request (-> (->DummyRequest {"x-auth-request-email" "user@example.com"} {}) + (assoc ::session/profile-id profile-id)) + response (handler request)] + ;; Downstream handler ran with alice's session intact. + (t/is (= profile-id (::session/profile-id @captured))) + (t/is (= 200 (::yres/status response))) + ;; No cookie clear / no fresh cookie issued — session untouched. + (t/is (not (contains? (::yres/cookies response) "auth-token")))))) + +(t/deftest x-auth-request-rethrows-non-transient-lookup-error + ;; A genuine bug (validation failure, NPE, ExceptionInfo from input + ;; validation, etc.) MUST NOT be silently absorbed as "transient". A + ;; broad catch Throwable would mask programming errors as a quiet + ;; "preserve session" pass-through and the bug would never surface. + ;; Narrow the transient-exception classification to DB/IO failures + ;; only; rethrow everything else so the standard 500 path runs. + (with-mocks [_ {:target 'app.http.auth-request/get-or-register-profile + :return (fn [& _] (throw (ex-info "bad input" {:type :validation})))}] + (let [cfg (make-xauth-cfg) + handler (#'app.http.auth-request/wrap-authz + (fn [_] {::yres/status 200}) + cfg) + request (-> (->DummyRequest {"x-auth-request-email" "user@example.com"} {}) + (assoc ::session/profile-id (random-uuid)))] + (t/is (thrown? clojure.lang.ExceptionInfo (handler request)))))) + +(t/deftest x-auth-request-rekey-clears-stale-auth-data + ;; The re-keyed request must have ::http/auth-data removed alongside + ;; the session keys. Without this, downstream code that reads + ;; ::http/auth-data.claims.uid (errors.clj, access_token.clj) would + ;; still see alice's profile-id after we re-keyed to bob, leaking the + ;; stale identity across the boundary the rest of the middleware + ;; thinks it has crossed. + (let [alice (th/create-profile* 1 {:is-active true}) + bob (th/create-profile* 2 {:is-active true}) + captured (volatile! nil) + cfg (make-xauth-cfg) + handler (#'app.http.auth-request/wrap-authz + (fn [req] (vreset! captured req) {::yres/status 200}) + cfg) + request (-> (->DummyRequest {"x-auth-request-email" (:email bob)} {}) + (assoc ::session/profile-id (:id alice)) + (assoc ::http/auth-data {:type :cookie + :claims {:uid (:id alice) + :sid (random-uuid)}}))] + (handler request) + (t/is (= (:id bob) (::session/profile-id @captured))) + (t/is (nil? (::http/auth-data @captured))))) + +(t/deftest x-auth-request-unresolvable-email-clears-stale-auth-data + ;; Same invariant on the unresolvable-email branch: ::http/auth-data + ;; must be removed so downstream readers don't see alice's stale + ;; claims when we've explicitly decided to flush her session. + (let [profile-id (random-uuid) + captured (volatile! nil) + handler (#'app.http.auth-request/wrap-authz + (fn [req] (vreset! captured req) {::yres/status 200}) + (make-xauth-cfg)) + request (-> (->DummyRequest {"x-auth-request-email" "ghost@example.com"} {}) + (assoc ::session/profile-id profile-id) + (assoc ::http/auth-data {:type :cookie + :claims {:uid profile-id}}))] + (handler request) + (t/is (nil? (::http/auth-data @captured))) + (t/is (nil? (::session/profile-id @captured))))) + +(t/deftest x-auth-request-unresolvable-email-passes-session-id-to-delete-fn + (let [profile-id (random-uuid) + session-id (random-uuid) + deleted-req (volatile! nil) + handler (#'app.http.auth-request/wrap-authz + (fn [_req] {::yres/status 200}) + (make-xauth-cfg)) + request (-> (->DummyRequest {"x-auth-request-email" "ghost@example.com"} {}) + (assoc ::session/profile-id profile-id) + (assoc ::session/session {:id session-id + :profile-id profile-id})) + response (with-mocks [_ {:target 'app.http.session/delete-fn + :return (fn [_cfg] + (fn [req resp] + (vreset! deleted-req req) + (session/clear-session-cookie resp)))}] + (handler request))] + (t/is (= session-id (::session/id @deleted-req))) + (t/is (= 0 (get-in response [::yres/cookies "auth-token" :max-age]))))) + +(t/deftest x-auth-request-blocked-incoming-clears-existing-mismatched-session + ;; openspec proxy-auth-middleware Rule 2: "Identity mismatch SHALL + ;; flush". When alice is logged in locally and oauth2-proxy forwards + ;; bob's email but bob's profile is blocked, we still 403 — and we + ;; still flush alice's session, because the upstream identity has + ;; changed regardless of whether the new identity is usable. + ;; Otherwise alice's auth-token cookie stays valid and her next + ;; request still serves her, defeating the flush guarantee. + (let [alice (th/create-profile* 1 {:is-active true}) + bob (th/create-profile* 2 {:is-active true}) + _ (th/db-update! :profile {:is-blocked true} {:id (:id bob)}) + handler (#'app.http.auth-request/wrap-authz + (fn [_] {::yres/status 200}) + (make-xauth-cfg)) + request (-> (->DummyRequest {"x-auth-request-email" (:email bob)} {}) + (assoc ::session/profile-id (:id alice)) + (assoc ::session/session {:id (random-uuid) + :profile-id (:id alice)})) + response (handler request)] + (t/is (= 403 (::yres/status response))) + (t/is (= 0 (get-in response [::yres/cookies "auth-token" :max-age]))))) + +(t/deftest x-auth-request-blocked-incoming-passes-session-id-to-delete-fn + (let [alice (th/create-profile* 1 {:is-active true}) + bob (th/create-profile* 2 {:is-active true}) + session-id (random-uuid) + deleted-req (volatile! nil) + _ (th/db-update! :profile {:is-blocked true} {:id (:id bob)}) + handler (#'app.http.auth-request/wrap-authz + (fn [_] {::yres/status 200}) + (make-xauth-cfg)) + request (-> (->DummyRequest {"x-auth-request-email" (:email bob)} {}) + (assoc ::session/profile-id (:id alice)) + (assoc ::session/session {:id session-id + :profile-id (:id alice)})) + response (with-mocks [_ {:target 'app.http.session/delete-fn + :return (fn [_cfg] + (fn [req resp] + (vreset! deleted-req req) + (session/clear-session-cookie resp)))}] + (handler request))] + (t/is (= 403 (::yres/status response))) + (t/is (= session-id (::session/id @deleted-req))) + (t/is (= 0 (get-in response [::yres/cookies "auth-token" :max-age]))))) + +(t/deftest x-auth-request-rekey-does-not-overwrite-handler-cookie + ;; If the downstream handler already wrote the auth-token cookie on + ;; the response (e.g. an explicit logout that called + ;; session/clear-session-cookie), the re-key path MUST NOT clobber + ;; that cookie with a fresh one. Otherwise a logout from a + ;; re-key-eligible session would be silently undone. + (let [alice (th/create-profile* 1 {:is-active true}) + bob (th/create-profile* 2 {:is-active true}) + cookie-name (cf/get :auth-token-cookie-name) + logout-cookie {:path "/" :value "" :max-age 0} + handler-resp {::yres/status 200 + ::yres/cookies {cookie-name logout-cookie}} + cfg (make-xauth-cfg) + handler (#'app.http.auth-request/wrap-authz + (fn [_] handler-resp) + cfg) + request (-> (->DummyRequest {"x-auth-request-email" (:email bob)} {}) + (assoc ::session/profile-id (:id alice))) + response (handler request)] + ;; The handler's logout cookie is preserved — the middleware + ;; recognised the response already carried an auth-token cookie and + ;; stepped aside. + (t/is (= logout-cookie (get-in response [::yres/cookies cookie-name]))))) + (t/deftest x-auth-request-auto-register-creates-active-profile (binding [cf/flags (conj cf/flags :x-auth-request-auto-register)] (let [email "newuser@example.com"