Skip to content

Mark the session cookie Secure behind a TLS-terminating proxy - #64

Closed
rangoDJ wants to merge 1 commit into
fix/client-ip-behind-proxyfrom
fix/cookie-secure-behind-proxy
Closed

rangoDJ wants to merge 1 commit into
fix/client-ip-behind-proxyfrom
fix/cookie-secure-behind-proxy

Conversation

@rangoDJ

@rangoDJ rangoDJ commented Oct 10, 2026

Copy link
Copy Markdown
Owner

Fixes #56

Depends on #63 (TRUSTED_PROXIES) and is based on its branch: merge #63 first, then this PR's base switches to main.

What changed

  • default.conf: the /api/ and /auth/ locations (where cookies are set) also pass X-Proxy-Forwarded-Proto: $http_x_forwarded_proto, i.e. what the proxy in front received. X-Forwarded-Proto stays nginx's own $scheme.
  • _cookie_secure() (COOKIE_SECURE=auto) uses X-Proxy-Forwarded-Proto only when the peer is in TRUSTED_PROXIES; otherwise unchanged.
  • README: the proxy section mentions it.

Testing

  • pytest: 261 passed, 8 skipped. New tests cover: direct HTTPS; a trusted tunnel terminating TLS → Secure; the same without TRUSTED_PROXIES → not believed; a direct visitor claiming https → not believed; a trusted proxy that received http; a trusted proxy sending no scheme. The tunnel case fails without the fix.
  • Not tested in a running container; to confirm after deploy, the connecthub_session cookie should show Secure in the browser's dev tools when signed in through the tunnel.

🤖 Generated with Claude Code

COOKIE_SECURE=auto read X-Forwarded-Proto, which nginx overwrites with its
own scheme: http when a proxy or tunnel terminates TLS and talks plain HTTP
to port 3000. The cookie then lacked Secure on an HTTPS site. nginx now
passes on what the proxy received as X-Proxy-Forwarded-Proto, believed only
from TRUSTED_PROXIES.

Fixes #56

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@rangoDJ
rangoDJ deleted the branch fix/client-ip-behind-proxy October 10, 2026 22:21
@rangoDJ rangoDJ closed this Oct 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant