feat(auth): expose refresh_expires_in via KeycloakTokens so hook cookie Max-Age tracks refresh window #22

New issue

Open

opened 2026-05-03 16:24:06 +00:00 by forgejo_admin · 0 comments

forgejo_admin commented 2026-05-03 16:24:06 +00:00

Owner

Copy link

Type

Feature

Lineage

Discovered scope from QA review of PR #20 (closes #15). The hook in PR #20 had to ship with Max-Age = access_exp (~5 min) because the frozen KeycloakTokens interface from forgejo_admin/westside-admin#14 (PR #18) doesn't carry refresh_expires_in. Result: browser discards the session cookie at every access-token expiry — even though the refresh token is still valid for ~30 min — so users hit the login form every ~5 minutes instead of every ~30 minutes.

Repo

forgejo_admin/westside-admin

User Story

story-westside-admin-admin-row-crud. Marcus shouldn't be re-logging-in every 5 minutes while he's in the middle of an admin task. The cookie's lifetime should track the refresh window so requests reach the server-side refresh path before the browser drops the session.

Context

The Keycloak /token response includes refresh_expires_in alongside expires_in. The lib's RefreshResponseBody interface (lines 463-475 of src/lib/server/keycloak.ts in main, post-#18) consumes the field but maps to a KeycloakTokens interface (lines 476-481) that drops it. The hook in PR #20 reads tokens.exp (access exp, derived from expires_in) and writes it to Max-Age. The fix:

1. Extend KeycloakTokens to carry both exp (access) and refresh_exp (refresh) — derived from Date.now() + refresh_expires_in*1000 at the same moment exp is derived.
2. Update refreshTokensIfNeeded to populate the new field on every refresh.
3. Update hooks.server.ts to use refresh_exp for Max-Age (clamped to a sane upper bound — 24h say — to avoid pathological tokens).

File Targets

Update:

• src/lib/server/keycloak.ts — extend KeycloakTokens interface; update refreshTokensIfNeeded to populate refresh_exp; update the initial-grant code path (in PR #21's /auth/callback if not yet merged, or in main if it is) to populate it too.
• src/lib/server/keycloak.test.ts — add test cases that assert refresh_exp is populated on initial token grant + on refresh.
• src/hooks.server.ts — replace Max-Age = exp - now with Max-Age = min(refresh_exp - now, 86400) (24h cap).

Do NOT touch:

• src/routes/auth/* (already merged — they use the lib's surface, will get the new field for free).
• src/routes/(unauthorized)/* — unrelated.

Acceptance Criteria

• KeycloakTokens carries refresh_exp: number (epoch seconds).
• refreshTokensIfNeeded populates refresh_exp on every refresh response.
• The initial-grant code path (callback) populates refresh_exp on the first cookie write.
• hooks.server.ts reads tokens.refresh_exp and writes Max-Age = max(0, min(refresh_exp - now, 86400)) to the cookie.
• After a successful login + 6 minutes idle, the cookie is still on the browser (not expired); next request triggers server-side refresh path.
• After 25 hours idle (exceeds 24h cap), cookie is gone; user re-authenticates.
• npm test exits 0 (existing 25 cases still pass + new cases for refresh_exp population).
• No regressions in PR #20's hook ACs.

Test Expectations

• Unit (npm test):
  • 2+ new vitest cases: initial grant populates refresh_exp; refresh response populates refresh_exp.
  • Existing 25 cases unchanged.
• Manual: log in to staging, idle 6 min, click around — expect no re-login form; idle 25 hr, expect re-login form.

Constraints

• Keep KeycloakTokens backward-compatible if possible (additive field), but if not, update all consumers in the same PR.
• Do NOT widen the 24h cap without surfacing — pathological refresh tokens (e.g., 30-day) shouldn't bind a cookie that long if the user closes the tab.
• Do NOT log refresh_expires_in or any token material.

Checklist

• PR opened
• Tests pass
• No regressions
• No tokens in logs (grep verified)
• PR body includes brief funnel-auth review per feedback_funnel_requires_auth (cookie lifetime is part of the auth surface)

Related

• project-westside-admin
• forgejo_admin/westside-admin#14 — original lib (the interface this issue extends)
• forgejo_admin/westside-admin#15 — hook that consumes the cookie Max-Age (PR #20)
• review-1135-2026-05-03-iter-qa — QA review that surfaced this gap (verdict APPROVED-with-followup)
• feedback_funnel_requires_auth

### Type Feature ### Lineage Discovered scope from QA review of PR #20 (closes #15). The hook in PR #20 had to ship with `Max-Age = access_exp` (~5 min) because the frozen `KeycloakTokens` interface from `forgejo_admin/westside-admin#14` (PR #18) doesn't carry `refresh_expires_in`. Result: browser discards the session cookie at every access-token expiry — even though the refresh token is still valid for ~30 min — so users hit the login form every ~5 minutes instead of every ~30 minutes. ### Repo `forgejo_admin/westside-admin` ### User Story `story-westside-admin-admin-row-crud`. Marcus shouldn't be re-logging-in every 5 minutes while he's in the middle of an admin task. The cookie's lifetime should track the refresh window so requests reach the server-side refresh path before the browser drops the session. ### Context The Keycloak `/token` response includes `refresh_expires_in` alongside `expires_in`. The lib's `RefreshResponseBody` interface (lines 463-475 of `src/lib/server/keycloak.ts` in main, post-#18) consumes the field but maps to a `KeycloakTokens` interface (lines 476-481) that drops it. The hook in PR #20 reads `tokens.exp` (access exp, derived from `expires_in`) and writes it to `Max-Age`. The fix: 1. Extend `KeycloakTokens` to carry both `exp` (access) and `refresh_exp` (refresh) — derived from `Date.now() + refresh_expires_in*1000` at the same moment `exp` is derived. 2. Update `refreshTokensIfNeeded` to populate the new field on every refresh. 3. Update `hooks.server.ts` to use `refresh_exp` for `Max-Age` (clamped to a sane upper bound — 24h say — to avoid pathological tokens). ### File Targets Update: - `src/lib/server/keycloak.ts` — extend `KeycloakTokens` interface; update `refreshTokensIfNeeded` to populate `refresh_exp`; update the initial-grant code path (in PR #21's `/auth/callback` if not yet merged, or in main if it is) to populate it too. - `src/lib/server/keycloak.test.ts` — add test cases that assert `refresh_exp` is populated on initial token grant + on refresh. - `src/hooks.server.ts` — replace `Max-Age = exp - now` with `Max-Age = min(refresh_exp - now, 86400)` (24h cap). Do NOT touch: - `src/routes/auth/*` (already merged — they use the lib's surface, will get the new field for free). - `src/routes/(unauthorized)/*` — unrelated. ### Acceptance Criteria - [ ] `KeycloakTokens` carries `refresh_exp: number` (epoch seconds). - [ ] `refreshTokensIfNeeded` populates `refresh_exp` on every refresh response. - [ ] The initial-grant code path (callback) populates `refresh_exp` on the first cookie write. - [ ] `hooks.server.ts` reads `tokens.refresh_exp` and writes `Max-Age = max(0, min(refresh_exp - now, 86400))` to the cookie. - [ ] After a successful login + 6 minutes idle, the cookie is still on the browser (not expired); next request triggers server-side refresh path. - [ ] After 25 hours idle (exceeds 24h cap), cookie is gone; user re-authenticates. - [ ] `npm test` exits 0 (existing 25 cases still pass + new cases for `refresh_exp` population). - [ ] No regressions in PR #20's hook ACs. ### Test Expectations - Unit (`npm test`): - 2+ new vitest cases: initial grant populates `refresh_exp`; refresh response populates `refresh_exp`. - Existing 25 cases unchanged. - Manual: log in to staging, idle 6 min, click around — expect no re-login form; idle 25 hr, expect re-login form. ### Constraints - Keep `KeycloakTokens` backward-compatible if possible (additive field), but if not, update all consumers in the same PR. - Do NOT widen the 24h cap without surfacing — pathological refresh tokens (e.g., 30-day) shouldn't bind a cookie that long if the user closes the tab. - Do NOT log `refresh_expires_in` or any token material. ### Checklist - [ ] PR opened - [ ] Tests pass - [ ] No regressions - [ ] No tokens in logs (grep verified) - [ ] PR body includes brief funnel-auth review per `feedback_funnel_requires_auth` (cookie lifetime is part of the auth surface) ### Related - `project-westside-admin` - `forgejo_admin/westside-admin#14` — original lib (the interface this issue extends) - `forgejo_admin/westside-admin#15` — hook that consumes the cookie Max-Age (PR #20) - `review-1135-2026-05-03-iter-qa` — QA review that surfaced this gap (verdict APPROVED-with-followup) - `feedback_funnel_requires_auth`

forgejo_admin referenced this issue 2026-05-03 20:29:32 +00:00

Keycloak cookie SSR auth + admin role gate (hooks.server.ts) #2

forgejo_admin referenced this issue 2026-05-03 20:56:46 +00:00

[BUG] Cookie SSR auth loops infinitely — session cookie value (4719B) exceeds browser per-cookie limit #24

forgejo_admin referenced this issue from a commit 2026-05-03 21:01:51 +00:00

fix(auth): drop id_token from session cookie payload (closes #24)

forgejo_admin referenced this issue 2026-05-03 21:03:33 +00:00

fix(auth): drop id_token from session cookie payload (closes #24) #25

forgejo_admin referenced this issue 2026-05-03 21:07:12 +00:00

fix(auth): drop id_token from session cookie payload (closes #24) #25

forgejo_admin referenced this issue 2026-05-03 21:34:44 +00:00

fix(auth): accept azp claim for client identity (closes #26) #27

Sign in to join this conversation.

No Branch/Tag specified

Branches Tags

main

No results found.

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

forgejo_admin/westside-admin#22

No description provided.