fix: auto-detect secureCookie to match Auth.js cookie prefix #21

Merged

forgejo_admin merged 1 commit from 20-bug-event-locals-accesstoken-always-null into main

2026-03-15 02:03:08 +00:00

forgejo_admin commented

2026-03-15 01:59:05 +00:00

Owner

Summary

Fixes event.locals.accessToken always being null by auto-detecting the secureCookie parameter from the request URL protocol, matching Auth.js internal cookie-prefix logic
Root cause: site runs on HTTPS via Tailscale funnel, so Auth.js sets __Secure-authjs.session-token cookies, but getToken() was looking for authjs.session-token (no prefix)

Changes

src/hooks.server.js: Changed secureCookie: false to secureCookie: isSecure where isSecure is derived from event.request.url.startsWith('https'), matching Auth.js internal auto-detection (@auth/core/lib/init.js line 69)
src/auth.js: Documentation-only -- added comments explaining token persistence across requests and the Keycloak access token expiration limitation (default 5 min, no auto-refresh, token rotation deferred)

Test Plan

Sign in via Keycloak on the HTTPS site
Navigate to /admin/teams -- team data loads (no more "Basketball API not reachable")
Navigate to /teams -- team data loads for authenticated users
Verify browser cookies show __Secure-authjs.session-token (confirming HTTPS prefix)
Verify no access tokens appear in browser-visible session data (tokens stay server-side)
No regressions in existing auth flows (role detection, admin/coach guards, signout)

Review Checklist

Passed automated review-fix loop
No secrets committed
No unnecessary file changes
Commit messages are descriptive

Closes #20
plan-2026-03-08-tryout-prep -- Phase 10b (Frontend admin draft board + coach filtered roster)

## Summary - Fixes `event.locals.accessToken` always being null by auto-detecting the `secureCookie` parameter from the request URL protocol, matching Auth.js internal cookie-prefix logic - Root cause: site runs on HTTPS via Tailscale funnel, so Auth.js sets `__Secure-authjs.session-token` cookies, but `getToken()` was looking for `authjs.session-token` (no prefix) ## Changes - `src/hooks.server.js`: Changed `secureCookie: false` to `secureCookie: isSecure` where `isSecure` is derived from `event.request.url.startsWith('https')`, matching Auth.js internal auto-detection (`@auth/core/lib/init.js` line 69) - `src/auth.js`: Documentation-only -- added comments explaining token persistence across requests and the Keycloak access token expiration limitation (default 5 min, no auto-refresh, token rotation deferred) ## Test Plan - [ ] Sign in via Keycloak on the HTTPS site - [ ] Navigate to `/admin/teams` -- team data loads (no more "Basketball API not reachable") - [ ] Navigate to `/teams` -- team data loads for authenticated users - [ ] Verify browser cookies show `__Secure-authjs.session-token` (confirming HTTPS prefix) - [ ] Verify no access tokens appear in browser-visible session data (tokens stay server-side) - [ ] No regressions in existing auth flows (role detection, admin/coach guards, signout) ## Review Checklist - [x] Passed automated review-fix loop - [x] No secrets committed - [x] No unnecessary file changes - [x] Commit messages are descriptive ## Related - Closes #20 - `plan-2026-03-08-tryout-prep` -- Phase 10b (Frontend admin draft board + coach filtered roster)

forgejo_admin added 1 commit

2026-03-15 01:59:05 +00:00

fix: auto-detect secureCookie from request URL to match Auth.js cookie prefix 2f8e9b6781

getToken() was called with secureCookie: false, which looks for the
cookie named "authjs.session-token". But on HTTPS (Tailscale funnel),
Auth.js sets cookies with the __Secure- prefix, naming it
"__Secure-authjs.session-token". The mismatch meant the cookie was
never found and event.locals.accessToken was always null.

Now secureCookie is derived from the request URL protocol, matching
Auth.js internal auto-detection logic (init.js line 69).

Also documents the Keycloak token expiration limitation in the jwt
callback -- the access token is persisted correctly but will expire
after Keycloak's configured lifetime (default 5 min).

Closes #20

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

forgejo_admin commented

2026-03-15 01:59:32 +00:00

Author

Owner

Self-Review: PASS

Correctness: The core fix (secureCookie: isSecure derived from event.request.url.startsWith('https')) matches Auth.js internal auto-detection logic (url.protocol === "https:" in @auth/core/lib/init.js line 69). Cookie name and HKDF salt derivation are consistent between encode (Auth.js) and decode (getToken()).

Verified:

@auth/core@0.41.1 source confirms getToken() uses defaultCookies(secureCookie ?? false).sessionToken.name for both cookie lookup and salt
Auth.js defaultCookies(true) produces __Secure-authjs.session-token, matching what the browser receives on HTTPS
JWT callback correctly persists accessToken and roles across requests (Auth.js re-encodes the full token object returned by the callback)
No access tokens exposed to browser (session callback omits accessToken by design)
Backward compatible: HTTP (local dev) gets secureCookie: false, HTTPS (prod) gets secureCookie: true

No issues found. Minimal, targeted fix.

## Self-Review: PASS **Correctness**: The core fix (`secureCookie: isSecure` derived from `event.request.url.startsWith('https')`) matches Auth.js internal auto-detection logic (`url.protocol === "https:"` in `@auth/core/lib/init.js` line 69). Cookie name and HKDF salt derivation are consistent between encode (Auth.js) and decode (`getToken()`). **Verified**: - `@auth/core@0.41.1` source confirms `getToken()` uses `defaultCookies(secureCookie ?? false).sessionToken.name` for both cookie lookup and salt - Auth.js `defaultCookies(true)` produces `__Secure-authjs.session-token`, matching what the browser receives on HTTPS - JWT callback correctly persists `accessToken` and `roles` across requests (Auth.js re-encodes the full token object returned by the callback) - No access tokens exposed to browser (session callback omits `accessToken` by design) - Backward compatible: HTTP (local dev) gets `secureCookie: false`, HTTPS (prod) gets `secureCookie: true` **No issues found**. Minimal, targeted fix.

forgejo_admin commented

2026-03-15 02:00:55 +00:00

Author

Owner

PR #21 Frontend Review

BLOCKERS

None.

NITS

@ts-ignore comments (lines 25, 28 in hooks.server.js) -- These suppress TypeScript errors for custom event.locals properties. Consider defining a proper App.Locals interface in src/app.d.ts to get type safety on accessToken. Non-blocking but would eliminate the need for suppression comments entirely.
Token expiration note is documentation-only -- The comment in auth.js correctly flags that Keycloak access tokens expire after 5 minutes and Auth.js does not auto-refresh. This is a known limitation, not a PR concern, but it should be tracked as a follow-up issue if not already.

DOMAIN REVIEW

Accessibility: No UI changes in this PR. N/A.
Performance: Negligible. event.request.url.startsWith('https') is a trivial string check -- no performance concern.
Responsive: No UI changes. N/A.
Design quality: N/A -- this is a server-side auth plumbing fix.
Security posture: Positive. The fix correctly aligns cookie prefix detection with Auth.js internals. The __Secure- prefix is a browser security feature that prevents cookie access over non-HTTPS connections. The previous hardcoded secureCookie: false was silently breaking authentication on HTTPS deployments. The access token correctly remains server-side only (never attached to the session object sent to the browser). .gitignore properly excludes .env files.

SOP COMPLIANCE

Branch named after issue (20-bug-event-locals-accesstoken-always-null references issue #20)
PR body has Summary, Changes, Test Plan, Related sections
Closes #20 present in PR body
Related section references plan-2026-03-08-tryout-prep
No secrets or .env files committed (confirmed: all secret references use runtime $env/dynamic/private)
No unnecessary file changes (2 files, both directly related to the bug)
Commit messages are descriptive
Design Decisions section -- not present, but acceptable for a bug fix of this scope. The Summary adequately explains the root cause and the rationale for the approach.
Tests -- no automated tests exist for auth hooks in this repo. Manual test plan is provided and appropriate for this change.

PROCESS OBSERVATIONS

Test gap: Auth hook behavior (cookie prefix matching, token extraction) has no automated test coverage. An integration test that validates getToken() returns a token on HTTPS requests would prevent regression. This is a repo-wide gap, not specific to this PR.
Convention candidate: The pattern of deriving secureCookie from the request URL should be documented as a convention for any Auth.js + Tailscale funnel deployment, since all such sites will be HTTPS and encounter this same mismatch if secureCookie is hardcoded.

VERDICT: APPROVED

Clean, minimal, correct bug fix. The root cause analysis is well-documented in both the PR body and inline comments. The change mirrors Auth.js's own internal detection logic, which is the right approach. No blockers.

## PR #21 Frontend Review ### BLOCKERS None. ### NITS 1. **`@ts-ignore` comments (lines 25, 28 in `hooks.server.js`)** -- These suppress TypeScript errors for custom `event.locals` properties. Consider defining a proper `App.Locals` interface in `src/app.d.ts` to get type safety on `accessToken`. Non-blocking but would eliminate the need for suppression comments entirely. 2. **Token expiration note is documentation-only** -- The comment in `auth.js` correctly flags that Keycloak access tokens expire after 5 minutes and Auth.js does not auto-refresh. This is a known limitation, not a PR concern, but it should be tracked as a follow-up issue if not already. ### DOMAIN REVIEW - **Accessibility:** No UI changes in this PR. N/A. - **Performance:** Negligible. `event.request.url.startsWith('https')` is a trivial string check -- no performance concern. - **Responsive:** No UI changes. N/A. - **Design quality:** N/A -- this is a server-side auth plumbing fix. - **Security posture:** Positive. The fix correctly aligns cookie prefix detection with Auth.js internals. The `__Secure-` prefix is a browser security feature that prevents cookie access over non-HTTPS connections. The previous hardcoded `secureCookie: false` was silently breaking authentication on HTTPS deployments. The access token correctly remains server-side only (never attached to the session object sent to the browser). `.gitignore` properly excludes `.env` files. ### SOP COMPLIANCE - [x] Branch named after issue (`20-bug-event-locals-accesstoken-always-null` references issue #20) - [x] PR body has Summary, Changes, Test Plan, Related sections - [x] `Closes #20` present in PR body - [x] Related section references `plan-2026-03-08-tryout-prep` - [x] No secrets or `.env` files committed (confirmed: all secret references use runtime `$env/dynamic/private`) - [x] No unnecessary file changes (2 files, both directly related to the bug) - [x] Commit messages are descriptive - [ ] Design Decisions section -- not present, but acceptable for a bug fix of this scope. The Summary adequately explains the root cause and the rationale for the approach. - [ ] Tests -- no automated tests exist for auth hooks in this repo. Manual test plan is provided and appropriate for this change. ### PROCESS OBSERVATIONS - **Test gap:** Auth hook behavior (cookie prefix matching, token extraction) has no automated test coverage. An integration test that validates `getToken()` returns a token on HTTPS requests would prevent regression. This is a repo-wide gap, not specific to this PR. - **Convention candidate:** The pattern of deriving `secureCookie` from the request URL should be documented as a convention for any Auth.js + Tailscale funnel deployment, since all such sites will be HTTPS and encounter this same mismatch if `secureCookie` is hardcoded. ### VERDICT: APPROVED Clean, minimal, correct bug fix. The root cause analysis is well-documented in both the PR body and inline comments. The change mirrors Auth.js's own internal detection logic, which is the right approach. No blockers.