fix(auth): OAuth httpOnly session sync; exclude intelligent-dashboard from CDN build; domain/OAuth docs#236
Merged
seanhanca merged 4 commits intoApr 7, 2026
Conversation
…N build - Call /api/v1/auth/me with credentials when no JS-visible token; return token from /me for client sync - Exclude examples/intelligent-dashboard from build-plugins and examples manifest; drop middleware route - Add vercel-oauth.env.template, push-oauth-env-to-vercel.sh, expand .env.local.example for host A/B Made-with: Cursor
Contributor
|
@seanhanca is attempting to deploy a commit to the Livepeer Foundation Team on Vercel. A member of the Team first needs to authorize it. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR removes the Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Browser/Client
participant AuthAPI as Auth API
participant Storage as Storage<br/>(localStorage/<br/>sessionStorage)
Client->>AuthAPI: GET /api/v1/auth/me<br/>(no Authorization header)
alt Initial request succeeds
AuthAPI-->>Client: 200 OK + user data + csrfToken
Client->>Storage: Store csrfToken to sessionStorage
else 401 Unauthorized
alt Token exists in localStorage
Client->>AuthAPI: Retry with<br/>Authorization: Bearer {token}
AuthAPI-->>Client: 200 OK + user data + csrfToken
Client->>Storage: Store csrfToken to sessionStorage
else No token exists
Client->>Storage: Call fetchAndStoreCsrfToken()
end
else 5xx Server Error
Client->>Client: Wait (short delay)
Client->>AuthAPI: Retry GET /api/v1/auth/me
AuthAPI-->>Client: Response
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
apps/web-next/src/contexts/auth-context.tsx (1)
84-85: Pre-existing issue: CSRF token removal targets wrong storage.Line 85 removes
STORAGE_KEYS.CSRF_TOKENfromlocalStorage, but the CSRF token is consistently stored insessionStoragethroughout this file (lines 68, 163). This removal is a no-op.The token is still correctly cleared via
sessionStorage.clear()on line 89, so there's no functional bug—but this line is dead code and could mislead future maintainers.🧹 Proposed cleanup
// Clear localStorage tokens localStorage.removeItem(STORAGE_KEYS.AUTH_TOKEN); - localStorage.removeItem(STORAGE_KEYS.CSRF_TOKEN);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web-next/src/contexts/auth-context.tsx` around lines 84 - 85, The line calling localStorage.removeItem(STORAGE_KEYS.CSRF_TOKEN) is incorrect because CSRF tokens are stored in sessionStorage in this module; replace that call with sessionStorage.removeItem(STORAGE_KEYS.CSRF_TOKEN) (or remove the line entirely since sessionStorage.clear() already runs) so STORAGE_KEYS.CSRF_TOKEN is actually removed and the code remains consistent with the rest of the auth context.bin/generate-examples-manifest.cjs (1)
25-27: Centralize exclusion config to avoid drift across scripts.
intelligent-dashboardis now hardcoded here and also inbin/build-plugins.sh. Please move this list to a shared source (JSON/env/sourced shell+node-readable file) so manifest generation and build discovery cannot diverge later.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bin/generate-examples-manifest.cjs` around lines 25 - 27, Replace the hardcoded Set EXCLUDE_FROM_EXAMPLES_MANIFEST with a shared configuration source (JSON or shell+node-readable file) and load it at runtime instead of inlining values; update the code that constructs CDN_DIR/manifest generation (reference symbol EXCLUDE_FROM_EXAMPLES_MANIFEST) to require/parse that shared file and build the Set from it, and update the build script that currently duplicates the list to source the same shared file so both discovery and manifest generation use one canonical list.apps/web-next/scripts/push-oauth-env-to-vercel.sh (1)
63-63: Make redeploy guidance match the selected target.At Line 63, the message always recommends a production deploy (
--prod) even when target ispreviewordevelopment.Proposed fix
-echo "Done. Redeploy from the Vercel dashboard or: npx vercel --prod" +if [[ "$TARGET" == "production" ]]; then + echo "Done. Redeploy from the Vercel dashboard or: npx vercel --prod" +else + echo "Done. Trigger a new $TARGET deployment from Vercel dashboard (or run: npx vercel)." +fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web-next/scripts/push-oauth-env-to-vercel.sh` at line 63, The redeploy message always suggests "npx vercel --prod" regardless of the selected target; update the echo to use the script's target variable (e.g., VERCEL_TARGET or TARGET) so it prints the correct flag (for example "--prod" for production, "--previews" or "--target preview"/"--target development" as your workflow requires). Locate the echo statement that prints "Done. Redeploy..." and replace the hardcoded "--prod" with logic that maps the script's target variable (used earlier in the script) to the appropriate vercel CLI flag and injects that into the message.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web-next/.env.local.example`:
- Around line 140-142: The PLAYWRIGHT_BASE_URL default in
apps/web-next/.env.local.example is inconsistent with the playwright.config.ts
webServer setting; update the example to use http://localhost:3001 (or change
the webServer port to 3000) so they match. Locate the PLAYWRIGHT_BASE_URL entry
in .env.local.example and change its commented default to http://localhost:3001
to align with the webServer configuration in playwright.config.ts (the webServer
configuration block that defines the local server URL/port).
In `@apps/web-next/scripts/push-oauth-env-to-vercel.sh`:
- Around line 37-40: The APP_URL can contain a trailing slash causing double
slashes in callback URIs (GOOGLE_CB, GITHUB_CB); normalize and validate APP_URL
after reading input by trimming any trailing slash and ensuring it has a valid
scheme (http:// or https://), then rebuild GOOGLE_CB and GITHUB_CB using the
normalized APP_URL so callbacks become "${APP_URL}/api/v1/auth/callback/google"
and "${APP_URL}/api/v1/auth/callback/github" without duplicate slashes; also
fallback to the default when APP_URL is empty.
In `@bin/build-plugins.sh`:
- Around line 150-158: The current example-exclusion check compares the raw
config path and misses cases where plugins/ entries are symlinks to examples;
fix by canonicalizing the config path before the match (e.g.,
resolved_config=$(readlink -f "$config") or realpath) and compare that against
the canonical examples directory (e.g., resolved_examples=$(readlink -f
"$ROOT_DIR/examples") and test prefix match like [[ "$resolved_config" ==
"$resolved_examples"/* ]]); keep the existing EXCLUDE_EXAMPLE_PLUGINS /
plugin_name loop and skip logic but use resolved_config for the if check so
symlinked example plugins are correctly excluded.
---
Nitpick comments:
In `@apps/web-next/scripts/push-oauth-env-to-vercel.sh`:
- Line 63: The redeploy message always suggests "npx vercel --prod" regardless
of the selected target; update the echo to use the script's target variable
(e.g., VERCEL_TARGET or TARGET) so it prints the correct flag (for example
"--prod" for production, "--previews" or "--target preview"/"--target
development" as your workflow requires). Locate the echo statement that prints
"Done. Redeploy..." and replace the hardcoded "--prod" with logic that maps the
script's target variable (used earlier in the script) to the appropriate vercel
CLI flag and injects that into the message.
In `@apps/web-next/src/contexts/auth-context.tsx`:
- Around line 84-85: The line calling
localStorage.removeItem(STORAGE_KEYS.CSRF_TOKEN) is incorrect because CSRF
tokens are stored in sessionStorage in this module; replace that call with
sessionStorage.removeItem(STORAGE_KEYS.CSRF_TOKEN) (or remove the line entirely
since sessionStorage.clear() already runs) so STORAGE_KEYS.CSRF_TOKEN is
actually removed and the code remains consistent with the rest of the auth
context.
In `@bin/generate-examples-manifest.cjs`:
- Around line 25-27: Replace the hardcoded Set EXCLUDE_FROM_EXAMPLES_MANIFEST
with a shared configuration source (JSON or shell+node-readable file) and load
it at runtime instead of inlining values; update the code that constructs
CDN_DIR/manifest generation (reference symbol EXCLUDE_FROM_EXAMPLES_MANIFEST) to
require/parse that shared file and build the Set from it, and update the build
script that currently duplicates the list to source the same shared file so both
discovery and manifest generation use one canonical list.
🪄 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: CHILL
Plan: Pro
Run ID: 9b4c2308-815c-4147-886c-f1191e9e3680
⛔ Files ignored due to path filters (1)
apps/web-next/src/generated/examples-manifest.tsis excluded by!**/generated/**
📒 Files selected for processing (9)
apps/web-next/.env.local.exampleapps/web-next/examples-manifest.jsonapps/web-next/scripts/push-oauth-env-to-vercel.shapps/web-next/src/app/api/v1/auth/me/route.tsapps/web-next/src/contexts/auth-context.tsxapps/web-next/src/middleware.tsapps/web-next/vercel-oauth.env.templatebin/build-plugins.shbin/generate-examples-manifest.cjs
💤 Files with no reviewable changes (2)
- apps/web-next/src/middleware.ts
- apps/web-next/examples-manifest.json
Comment on lines
+140
to
+142
| # Target deployment (default local: http://localhost:3000): | ||
| # PLAYWRIGHT_BASE_URL=https://naap-platform.vercel.app | ||
| # |
There was a problem hiding this comment.
Playwright local default URL is inconsistent with config.
At Line 140-Line 142, docs say local default is http://localhost:3000, but apps/web-next/playwright.config.ts:55-64 uses http://localhost:3001 for local webServer. This can break local E2E setup.
Proposed fix
-# Target deployment (default local: http://localhost:3000):
+# Target deployment (default local: http://localhost:3001):
# PLAYWRIGHT_BASE_URL=https://naap-platform.vercel.app📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # Target deployment (default local: http://localhost:3000): | |
| # PLAYWRIGHT_BASE_URL=https://naap-platform.vercel.app | |
| # | |
| # Target deployment (default local: http://localhost:3001): | |
| # PLAYWRIGHT_BASE_URL=https://naap-platform.vercel.app | |
| # |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web-next/.env.local.example` around lines 140 - 142, The
PLAYWRIGHT_BASE_URL default in apps/web-next/.env.local.example is inconsistent
with the playwright.config.ts webServer setting; update the example to use
http://localhost:3001 (or change the webServer port to 3000) so they match.
Locate the PLAYWRIGHT_BASE_URL entry in .env.local.example and change its
commented default to http://localhost:3001 to align with the webServer
configuration in playwright.config.ts (the webServer configuration block that
defines the local server URL/port).
|
This PR has conflicts with the base branch. Please rebase to resolve them: git fetch origin
git rebase origin/main
# resolve conflicts, then:
git push --force-with-leaseThe |
… cookie; robust cookie parse - Use getAuthApiBase() so login/me/oauth/logout always hit app origin when NEXT_PUBLIC_API_URL is a different host (httpOnly cookies were not sent) - Stop writing naap_auth_token via document.cookie (conflicts with httpOnly session cookie parsing) - Always POST logout with credentials so httpOnly session clears even without localStorage token - Retry /auth/me once on 5xx; parse naap_auth_token cookie value with decodeURIComponent Made-with: Cursor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web-next/src/contexts/auth-context.tsx`:
- Around line 183-186: The code rehydrates the primary session token returned by
/auth/me into localStorage (see sessionToken and setTokenStorage), which exposes
the httpOnly cookie-derived bearer token to JS; stop writing tokens coming from
the /auth/me response into local storage—only persist tokens when they originate
from explicit authentication flows that should be JS-readable (e.g.,
login/signup handlers), or instead have the server return a separate
scoped/short-lived client token if client code needs one; remove or gate the
setTokenStorage call that uses data.data?.token or data.token when this function
is invoked for the /auth/me route and ensure any new client-readable credential
is issued and consumed via a distinct endpoint/field.
- Around line 141-147: fetchUser() is currently adding an Authorization header
from the client-side token before the httpOnly cookie is considered, causing
stale localStorage tokens to override the fresh server cookie; update the header
construction in auth-context.tsx (the headers object near the token assignment
and the same logic used around lines 150-163) so Authorization is only set if
there is no valid server cookie (naap_auth_token) present or if getAuthToken()
explicitly indicates the cookie is absent—i.e., check for the server cookie
first (via getAuthToken() or document.cookie lookup) and only fall back to
token/localStorage for headers.Authorization when the cookie is missing.
🪄 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: CHILL
Plan: Pro
Run ID: b05fc373-754d-4ecf-8a07-26267479164e
📒 Files selected for processing (2)
apps/web-next/src/contexts/auth-context.tsxapps/web-next/src/lib/api/response.ts
Contributor
|
@seanhanca It looks like this branch was pushed to production from vercel, but it is two commits behind main and the regression is a performance fix. Please update your branch asap 🙏 |
…red excludes) - Prefer naap_auth_token cookie over Authorization in getAuthToken - Stop echoing session token from GET /api/v1/auth/me - fetchUser: cookie-first /me, retry with Bearer only on 401 if LS token exists - refreshSession: allow cookie-only refresh without localStorage token - clearAllAuthStorage: drop redundant CSRF localStorage remove (CSRF in sessionStorage) - examples/.cdn-build-exclude as single source for bash + manifest generator - build-plugins: resolve realpath so symlinked example plugins respect excludes - push-oauth-env: validate origin URL, strip trailing slash, target-aware redeploy hint - .env.local.example: Playwright base URL note aligned with playwright port 3000 Made-with: Cursor
Resolve examples-manifest: keep intelligent-dashboard excluded per .cdn-build-exclude; regenerate manifest and examples-manifest.ts. Made-with: Cursor
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
naap_auth_token, so the client never called/api/v1/auth/meandRequireAuthbounced with middleware → endless loading on/dashboard.fetchUsernow always usescredentials: 'include';/auth/meechoestoken(and existingcsrfToken) so client storage stays in sync.intelligent-dashboardunderexamples/was auto-discovered bybuild-plugins.shand failed on missing@naap/plugin-sdkfrom npm. It is excluded from default CDN builds and from the examples manifest; middleware no longer rewrites/intelligent-dashboard.vercel-oauth.env.template+scripts/push-oauth-env-to-vercel.shand expanded.env.local.examplefor Vercel OAuth env vars.When the app domain changes (e.g. to
network.livepeer.org)Do all of the following so sign-in and redirects keep working. URLs must match exactly (scheme, host, path; no stray slash).
1. Vercel — environment variables (Production, and Preview if OAuth is used there)
Use one canonical public origin at a time.
NEXT_PUBLIC_APP_URLhttps://<your-new-domain>GOOGLE_REDIRECT_URIhttps://<your-new-domain>/api/v1/auth/callback/googleGITHUB_REDIRECT_URIhttps://<your-new-domain>/api/v1/auth/callback/githubRemove or replace the old host’s values so there is no conflict.
GOOGLE_CLIENT_*/GITHUB_CLIENT_*stay the same unless you create new OAuth clients.Toggle Host A vs Host B in
apps/web-next/vercel-oauth.env.template, then paste into Vercel (or runscripts/push-oauth-env-to-vercel.sh).Redeploy after saving env vars.
2. Google Cloud Console
APIs & Services → Credentials → your OAuth 2.0 Web client → Authorized redirect URIs
https://<your-new-domain>/api/v1/auth/callback/google3. GitHub
Settings → Developer settings → OAuth Apps → your app → Authorization callback URL
https://<your-new-domain>/api/v1/auth/callback/github(GitHub allows one callback per app; for parallel environments use a second OAuth app.)
4. Optional checks
PLAYWRIGHT_BASE_URLto the new origin if you test against that deployment.NEXT_PUBLIC_APP_URLmust use the same canonical URL.Test plan
/dashboardloads (no login/dashboard loop).GET /api/v1/auth/providerslistsgoogleandgithubwhen configured.intelligent-dashboardin auto-build list).Summary by CodeRabbit
Release Notes
Removed Features
Improvements