🐛 Re-key session when X-Auth-Request identity differs

[awais786]

Replaces #21 with a minimal scope. Closes the QA-reported session-sharing repro and nothing else.

130 insertions / 53 deletions across 3 files (vs 540/70 on #21).

The bug

wrap-authz skipped when wrap-session had already set ::session/profile-id from the auth-token cookie. Once Penpot issued its own cookie, the upstream X-Auth-Request-Email was no longer consulted — so after a portal "Log out of all apps" + login as a different user, refreshing the Penpot tab kept serving the previous user. The shared _oauth2_proxy cookie + Cognito session were cleared, but Penpot's own auth-token on paint.<domain> survived.

The fix

Detect mismatch and re-key.

• Match (session-pid == header profile.id) → pass through unchanged.
• Mismatch (or no existing session) → assoc the new profile-id into the request and issue a fresh auth-token cookie via session/create-fn.
• Plus a one-condition session.clj guard: don't renew if the response already has the auth-token cookie. Without this, the renewal path (which runs after wrap-authz on the way out) would clobber the freshly-issued cookie with a renewed copy of the previous user's.

Blocked / inactive / nil-profile / API-key (::actoken/profile-id) branches are unchanged.

Repro

1. Log in to the FOSS portal as user A.
2. Open Penpot (paint.<domain>) in a tab.
3. On the portal, click Log out of all apps.
4. Log in as user B via password.
5. Refresh the Penpot tab → still shows user A (without this fix).

What's NOT in this PR (dropped from #21 — can land separately)

These tighten edge cases but are not needed to fix the QA repro:

• Openspec Rule 2 — flush on unresolvable email via session/delete-fn
• Openspec Rule 2 — flush on blocked/inactive mismatch
• Transient-exception classification for DB outages
• ::http/auth-data cleanup channel
• Don't-renew-on-4xx clause in session.clj
• with-session-id + clear-stale-session + response-has-auth-cookie? helpers

If/when any of those become load-bearing, open as follow-ups against the same capability.

Test plan

• clj -X:test :nses '[backend-tests.http-middleware-test]' passes — three new/renamed cases:
  • x-auth-request-rekeys-when-session-identity-differs — alice session-pid + bob email → captured request has bob's pid, response has fresh auth-token cookie
  • x-auth-request-no-rekey-when-session-matches-header — steady-state guard (no new cookie when match)
  • x-auth-request-preserves-session-when-header-email-unresolvable — renamed from the prior skips-when-session-present; invariant is now "preserve existing session when there's no real alternative identity to switch to"
• Existing tests still pass — x-auth-request-no-email-header, -skips-when-access-token-present, -authenticates-existing-active-profile, -blocked-profile-returns-403, -inactive-profile-returns-403, -unknown-email-no-autoregister, -auto-register-creates-active-profile.
• Manual: rebuild Penpot backend, run the QA repro — refreshing Penpot tab as user B now serves user B.

Action requested

Either:

1. Merge this and close Fix/proxy auth stale session on user switch #21, or
2. Force-push this branch over Fix/proxy auth stale session on user switch #21's (git push origin +shrink/proxy-auth-stale-session-minimal:fix/proxy-auth-stale-session-on-user-switch) and close this PR.

Same outcome either way — pick whichever preserves the review history you'd rather keep.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR updates backend X-Auth-Request session handling so proxy-authenticated identity can replace a stale local session when the resolved profile differs.

Changes:

• Reworks wrap-authz to resolve proxy identity even when a session exists, re-keying on mismatch.
• Adds a session renewal guard to avoid overwriting an auth cookie already set by downstream middleware.
• Adds middleware tests for re-keying, steady-state behavior, stale auth-data cleanup, and renewal interaction.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
backend/src/app/http/auth_request.clj	Adds mismatch detection and re-key behavior for X-Auth-Request sessions.
backend/src/app/http/session.clj	Skips session renewal when the response already includes the auth-token cookie.
backend/test/backend_tests/http_middleware_test.clj	Adds and updates tests covering X-Auth-Request re-key scenarios.

Comments suppressed due to low confidence (5)

backend/src/app/http/auth_request.clj:219

• This leaves ::session/session pointing at the previously decoded session while the request's ::session/profile-id has been changed to the proxy user. That stale session is observable: session/get-session is used by update-profile-password to call invalidate-others, so a password-change RPC on the re-keying request would invalidate sessions for the old user instead of the newly authenticated one.

                                        ;; ::session/session is intentionally
                                        ;; NOT dissoc'd here — the surrounding
                                        ;; session.clj wrap-authz closes over
                                        ;; the originally-decoded session for
                                        ;; renewal, and no external code reads
                                        ;; ::session/session post-handler.
                                        (dissoc ::http/auth-data)

backend/src/app/http/auth_request.clj:223

• This unconditionally creates a new auth-token after the downstream handler has already built its response. If the re-keying request is an RPC that intentionally clears or replaces the auth-token (for example logout or delete-profile via their response transforms), this outer create-session call overwrites that cookie with a fresh proxy session, making the logout/delete response leave the browser authenticated again.

                ;; Fresh auth-token cookie; replaces the stale one the
                ;; browser still has (if any).
                (create-session! request response)))))))))

backend/src/app/http/auth_request.clj:219

• The PR description lists ::http/auth-data cleanup as intentionally dropped from this minimal change, but this branch now adds that cleanup and a dedicated test for it. Please either update the description/test plan or remove the cleanup so reviewers are evaluating the actual scope.

                                        ;; Drop ::http/auth-data so downstream
                                        ;; readers don't see the previous user's
                                        ;; claims after we re-key. Confirmed
                                        ;; consumers: errors.clj logs
                                        ;; auth-data.claims.uid as
                                        ;; :request/profile-id; rpc/helpers
                                        ;; exposes the map to RPC handlers via
                                        ;; get-auth-data.
                                        ;;
                                        ;; ::session/session is intentionally
                                        ;; NOT dissoc'd here — the surrounding
                                        ;; session.clj wrap-authz closes over
                                        ;; the originally-decoded session for
                                        ;; renewal, and no external code reads
                                        ;; ::session/session post-handler.
                                        (dissoc ::http/auth-data)

backend/src/app/http/auth_request.clj:161

• This log message is no longer accurate when session-pid is present: the branch preserves the existing session and calls the handler authenticated as that user, not unauthenticated. This will make mismatch/unresolvable-header incidents harder to diagnose from logs.

              (l/wrn :hint "x-auth-request: no profile found for email, passing through unauthenticated"
                     :email email)
              (handler request))

backend/src/app/http/auth_request.clj:161

• When a proxy-authenticated email cannot be resolved, this preserves any existing local session and continues the request as that previous user. In the same stale-cookie flow, logging in upstream as an unprovisioned user (or during an auto-register/config issue) would still serve the old Penpot account instead of clearing/denying the stale session.

            (nil? profile)
            ;; Header email doesn't resolve to a profile (and auto-register
            ;; is off). No identity to switch *to* — pass through with
            ;; whatever wrap-session set.
            (do
              (l/wrn :hint "x-auth-request: no profile found for email, passing through unauthenticated"
                     :email email)
              (handler request))

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[awais786]

Thanks @copilot — three valid findings. Addressed in 016c30b00:

1. ::session/session stale → wrong-user invalidation in update-profile-password 🔴

Real bug; my earlier "no external readers" claim was wrong — I grepped for the literal keyword but missed the indirect read through session/get-session. Fixed by dissoc'ing ::session/session in the rekey branch. Comment updated to call out the invalidate-others consumer specifically. New test x-auth-request-rekey-clears-stale-session-map asserts the captured request has no ::session/session after rekey.

2. Renewal extends alice's cookie on a 403 mismatch denial 🔴

Real bug. When proxy-asserted profile is blocked/inactive, wrap-authz returns {::yres/status 403} with no cookie on the response; the existing renewal guard (no-cookie-on-response → renew) then refreshes alice's session cookie, extending her session lifetime on the very denial that signaled her identity is no longer valid. Fixed by also gating renewal on (or (nil? status) (< status 400)). New test session-authz-does-not-renew-on-error-response exercises the 403 path with a renewal-due session.

3. Performance regression — every steady-state SSO request hits the DB 🟡

Real cost; intentionally documented rather than fixed. Removing the "skip if session already resolved" fast path is what enables mismatch detection — without resolving the header email's profile, we can't know if the existing session still matches. Trading throughput for correctness on the steady-state path. Inline ;; Perf note: comment added describing two follow-up optimizations (pre-load session profile's email; or a short-lived email→pid cache) if this shows up hot in profiling. Out of scope for this PR per the user's request to keep the implementation minimal.

PR head: 016c30b00. CI commit-message check is green; other jobs running.