fix(auth): allow dev-impersonation in production builds via runtime o… - #166
Conversation
…pt-in
Dev impersonation was gated three places on import.meta.env.DEV (Vite's
dev-server flag) — oidc-manager.ts (auth bypass when no OIDC config),
use-viewer.ts (isDevImpersonating), fetch-with-auth.ts (devBearer).
Production bundles set DEV=false, so the published ghcr image silently
fell into status=unauthorized,missing_oidc_config when stood up in the
docker-compose dev stack: SPA halts before any /api call, no errors,
black screen.
Replaces the build-mode gate with a runtime opt-in:
* src/auth/types.ts adds window.__DEV_CONFIG__?: { devUserEmail?: string }.
* src/auth/dev-config.ts (new) reads it.
* src/auth/use-viewer.ts resolves the dev email from runtime first,
falling back to import.meta.env.VITE_DEV_USER_EMAIL only when in
Vite dev. isDevImpersonating() drops its DEV gate.
* src/auth/oidc-manager.ts bypasses auth when no OIDC config AND
(DEV OR a runtime dev email is present).
* src/api/fetch-with-auth.ts drops the DEV gate on devBearer().
docker-entrypoint.sh validates a new DEV_USER_EMAIL env var (same
unsafe-char check as OIDC_*), refuses to honor it when OIDC_ISSUER /
OIDC_CLIENT_ID are also set (mutually exclusive), and writes
window.__DEV_CONFIG__ = { devUserEmail: "..." } into oidc-config.js
when only DEV_USER_EMAIL is set.
Safety: a production deploy that doesn't set DEV_USER_EMAIL still
fails closed exactly as before. The dev path is strictly opt-in via
the env var, set only in insight's docker-compose.yml for the
insight-front-ghcr service.
Signed-off-by: Anton Zelenov <antonz@constructor.tech>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a runtime dev impersonation mode controlled by ChangesRuntime Dev Impersonation via DEV_USER_EMAIL
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docker-entrypoint.sh`:
- Around line 55-59: The runtime configuration documentation needs to be updated
to reflect the current implementation that supports both __OIDC_CONFIG__ and
__DEV_CONFIG__ modes. Update the README or relevant documentation to include:
documentation for the __DEV_CONFIG__ variable and DEV_USER_EMAIL parameter,
clarification that __OIDC_CONFIG__ and __DEV_CONFIG__ are mutually exclusive
options, and the production-bundle dev-stack path for development. Remove
references to only the old Vite-dev-only bypass approach to match the current
behavior described in the docker-entrypoint.sh comment block.
- Line 76: The echo statement on the dev config line includes the DEV_USER_EMAIL
variable, which is PII and should not appear in container logs. Remove the
`devUserEmail=$DEV_USER_EMAIL` portion from the echo message while preserving
the informational parts that indicate dev config was written and auth was
bypassed. This maintains the startup signal without retaining user identifiers
in logs.
In `@src/api/fetch-with-auth.ts`:
- Around line 5-10: The devBearer() function currently uses getViewerEmail()
which can return OIDC or override identities, not just legitimate dev
identities, creating a security issue where unsigned bearer tokens could be sent
outside the intended opt-in path. Add a source-aware helper in
src/auth/use-viewer.ts that distinguishes actual dev identities from generic
viewer emails, then replace the devBearer() function logic (lines 5-10 in
src/api/fetch-with-auth.ts) to gate the unsigned JWT generation on this new
source-aware check instead of just checking getViewerEmail(). Apply the same
source-aware gating to the other affected location at lines 27-29 in
src/api/fetch-with-auth.ts where the token ?? devBearer() expression is used.
In `@src/auth/use-viewer.ts`:
- Around line 66-71: The isDevImpersonating() function currently returns true
whenever mocks are enabled or a dev email is configured, but it should only
return true when the active viewer source is actually from dev/mocks. Since
resolve() gives OIDC and overrides precedence over dev sources, the current
logic can incorrectly show impersonation UI even when the active viewer identity
source is not dev. Modify the isDevImpersonating() function to check that the
actual source of the current viewer identity is from dev/mocks before returning
true, rather than just checking if those options exist.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 425ae7a4-90d6-4083-aa76-d09692fa0321
📒 Files selected for processing (6)
docker-entrypoint.shsrc/api/fetch-with-auth.tssrc/auth/dev-config.tssrc/auth/oidc-manager.tssrc/auth/types.tssrc/auth/use-viewer.ts
…urce Addresses CodeRabbit review feedback on PR #166. devBearer() called getViewerEmail(), which can resolve to an OIDC user's email when OIDC is mid-bootstrap (authStore.token still null, but resolve() falls through MOCKS/dev branches). That would mint an unsigned JWT bearing the real user's identity — leak path in any Vite-dev session with both VITE_OIDC_* and VITE_DEV_USER_EMAIL set. Production builds are still safe (entrypoint refuses OIDC+DEV combo) but the source-aware gate is the correct fix at the source level. Adds getDevBearerEmail() in use-viewer.ts that returns the email only when the active viewer source is dev-style ('dev' for runtime/build-time dev email or MOCKS; 'override' for sessionStorage impersonation) — never for 'oidc' or 'none'. devBearer() switches to it. isDevImpersonating() previously returned true whenever a dev email was configured, even if OIDC was the active source — could surface the impersonation banner / hint over a real OIDC session. Now reads the active source from resolve(). docker-entrypoint.sh stops echoing DEV_USER_EMAIL on startup — deployers pointing it at a real mailbox would have leaked PII into container logs. Skipped: CodeRabbit's docs-update suggestion (no separate README in scope; the inline comment block in the entrypoint already describes both __OIDC_CONFIG__ and __DEV_CONFIG__ modes). Signed-off-by: Anton Zelenov <antonz@constructor.tech>
…pt-in
Dev impersonation was gated three places on import.meta.env.DEV (Vite's dev-server flag) — oidc-manager.ts (auth bypass when no OIDC config), use-viewer.ts (isDevImpersonating), fetch-with-auth.ts (devBearer). Production bundles set DEV=false, so the published ghcr image silently fell into status=unauthorized,missing_oidc_config when stood up in the docker-compose dev stack: SPA halts before any /api call, no errors, black screen.
Replaces the build-mode gate with a runtime opt-in:
docker-entrypoint.sh validates a new DEV_USER_EMAIL env var (same unsafe-char check as OIDC_*), refuses to honor it when OIDC_ISSUER / OIDC_CLIENT_ID are also set (mutually exclusive), and writes window.DEV_CONFIG = { devUserEmail: "..." } into oidc-config.js when only DEV_USER_EMAIL is set.
Safety: a production deploy that doesn't set DEV_USER_EMAIL still fails closed exactly as before. The dev path is strictly opt-in via the env var, set only in insight's docker-compose.yml for the insight-front-ghcr service.
Summary by CodeRabbit
New Features
Improvements