Skip to content

Fix: persist refreshed OAuth tokens to session store#266

Merged
bd73-com merged 3 commits intomainfrom
claude/fix-github-issue-x7Lz4
Mar 25, 2026
Merged

Fix: persist refreshed OAuth tokens to session store#266
bd73-com merged 3 commits intomainfrom
claude/fix-github-issue-x7Lz4

Conversation

@bd73-com
Copy link
Owner

@bd73-com bd73-com commented Mar 25, 2026

Summary

When the OIDC access token expires, the isAuthenticated middleware refreshes it via refreshTokenGrant but previously only updated tokens in memory. With resave: false, express-session never persisted the refreshed tokens to the PostgreSQL session store, causing redundant refresh calls on every subsequent request (~100-500ms extra latency each) and potential unexpected logouts if the provider issues one-time-use refresh tokens.

This PR persists refreshed tokens to the session store and hardens the refresh path against several edge cases discovered during review.

Changes

Core fix (server/replit_integrations/auth/replitAuth.ts):

  • After token refresh, re-serialize updated tokens into req.session.passport.user and call req.session.save()
  • Move next() inside the session.save() callback to prevent race condition where the response dispatches before persistence completes
  • Return 500 on session save failure instead of silently proceeding

Hardening:

  • Extract serializeUserPayload() helper to eliminate shape duplication between serializeUser (login) and the refresh path
  • Add null check on req.session.passport before assignment to handle corrupted session rows gracefully
  • Use safe error message extraction (err instanceof Error ? err.message : String(err)) to prevent logging undefined and avoid leaking full error objects

Tests (server/replit_integrations/auth/replitAuth.test.ts):

  • 7 new tests for isAuthenticated middleware covering: unauthenticated, missing expiry, valid token, missing refresh token, successful refresh with session persistence verification, failed refresh grant, and session save failure (500 response)

How to test

  1. npm run check — TypeScript passes
  2. npm run test — All 1764 tests pass (7 new)
  3. npm run build — Production build succeeds
  4. Manual: Log in, wait for token expiry, confirm session store row updates with new tokens
  5. Manual: Confirm subsequent requests after refresh do not trigger additional refreshTokenGrant calls

Related issues

https://claude.ai/code/session_01JUweaU4PunmvR2R3oD9SKD

After token refresh in isAuthenticated middleware, re-serialize the
updated tokens into req.session.passport.user and call session.save()
so they persist to PostgreSQL. Prevents redundant refreshTokenGrant
calls on every request after token expiry.

Fixes #265

https://claude.ai/code/session_01JUweaU4PunmvR2R3oD9SKD
@github-actions github-actions bot added the fix label Mar 25, 2026
@coderabbitai
Copy link

coderabbitai bot commented Mar 25, 2026

📝 Walkthrough

Walkthrough

The PR adds explicit persistence of refreshed OAuth tokens to the Express session store in the isAuthenticated middleware. After updating user session with fresh tokens, the code now re-serializes token data into req.session.passport.user and calls req.session.save() to commit the changes to PostgreSQL, preventing stale tokens from persisting in the session store.

Changes

Cohort / File(s) Summary
OAuth Token Persistence
server/replit_integrations/auth/replitAuth.ts
Added explicit session persistence after token refresh by re-serializing updated user claims, access_token, refresh_token, and expires_at into req.session.passport.user, followed by req.session.save() with error logging. Ensures refreshed tokens are committed to PostgreSQL session store rather than remaining in-memory only.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Possibly related PRs

Suggested labels

fix

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: persisting refreshed OAuth tokens to the session store, which is the core objective of the PR.
Linked Issues check ✅ Passed The code change directly implements the suggested fix from issue #265: it re-serializes updated tokens into req.session.passport.user and calls req.session.save() with error logging.
Out of Scope Changes check ✅ Passed The changes are narrowly scoped to the isAuthenticated middleware in replitAuth.ts and directly address issue #265 with no extraneous modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/fix-github-issue-x7Lz4

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@server/replit_integrations/auth/replitAuth.ts`:
- Around line 221-234: The code assigns refreshed tokens to (req.session as
any).passport.user then calls req.session.save(...) but calls next()
immediately, so move the next() invocation into the req.session.save callback
and handle save failures by forwarding the error to Express (call next(err) when
err is truthy) and only call next() with no args when save succeeds; update the
block around req.session.save(...) / next() accordingly so the request only
proceeds after the session persist completes.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 294071d9-d6a3-431a-91ac-71231146b956

📥 Commits

Reviewing files that changed from the base of the PR and between 5c1b85d and c525a88.

📒 Files selected for processing (1)
  • server/replit_integrations/auth/replitAuth.ts

claude added 2 commits March 25, 2026 17:37
- Move next() inside session.save() callback to prevent race condition
  where response dispatches before session persistence completes
- Return 500 on session save failure instead of proceeding
- Log err.message instead of full error object to prevent token leakage
- Extract serializeUserPayload() helper to eliminate shape duplication
  between serializeUser and the refresh path
- Add 7 tests for isAuthenticated middleware covering all code paths

https://claude.ai/code/session_01JUweaU4PunmvR2R3oD9SKD
…ging

- Add null check for req.session.passport before assignment to prevent
  TypeError on corrupted session rows
- Use safe error message extraction (err instanceof Error) to handle
  non-Error objects in session.save callback

https://claude.ai/code/session_01JUweaU4PunmvR2R3oD9SKD
@bd73-com bd73-com merged commit bb72750 into main Mar 25, 2026
2 checks passed
@github-actions github-actions bot deleted the claude/fix-github-issue-x7Lz4 branch March 25, 2026 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Refreshed OAuth tokens not persisted to session store

2 participants