feat: add AUTH_DISABLED environment variable to disable authentication#443
Draft
iamriajul wants to merge 2 commits intositeboon:mainfrom
Draft
feat: add AUTH_DISABLED environment variable to disable authentication#443iamriajul wants to merge 2 commits intositeboon:mainfrom
iamriajul wants to merge 2 commits intositeboon:mainfrom
Conversation
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR introduces an Changes
Sequence DiagramsequenceDiagram
participant Client
participant AuthMiddleware
participant AuthContext
participant ProtectedRoute
rect rgba(100, 150, 200, 0.5)
Note over Client,ProtectedRoute: AUTH_DISABLED=true Path
Client->>AuthMiddleware: API request
AuthMiddleware->>AuthMiddleware: Check AUTH_DISABLED flag
AuthMiddleware->>AuthMiddleware: Assign fallback user
AuthMiddleware->>Client: Return response with authDisabled: true
Client->>AuthContext: Fetch status
AuthContext->>AuthContext: Detect authDisabled in response
AuthContext->>AuthContext: Set authDisabled state
AuthContext->>ProtectedRoute: Provide authDisabled flag
ProtectedRoute->>ProtectedRoute: Check authDisabled
ProtectedRoute->>Client: Render protected content
end
rect rgba(200, 100, 100, 0.5)
Note over Client,ProtectedRoute: Normal Auth Path
Client->>AuthMiddleware: API request with token
AuthMiddleware->>AuthMiddleware: Validate JWT token
AuthMiddleware->>Client: Return response with authDisabled: false
Client->>AuthContext: Fetch status
AuthContext->>AuthContext: Normal auth processing
AuthContext->>ProtectedRoute: authDisabled: false
ProtectedRoute->>AuthMiddleware: Verify authentication
ProtectedRoute->>Client: Render protected content
end
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
server/routes/auth.js (1)
132-139: Avoid duplicating auth-disabled fallback user construction.Line 134 redefines a fallback user even though
authenticateTokenalready setsreq.userin auth-disabled mode. Keeping one source of truth will prevent drift.♻️ Simplify `/user` auth-disabled response
if (isAuthDisabled()) { return res.json({ - user: req.user || { - id: 1, - username: 'admin' - }, + user: req.user, authDisabled: true }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/routes/auth.js` around lines 132 - 139, The /user route is constructing a hardcoded fallback user even though authenticateToken already sets req.user when auth is disabled; remove the duplicated object and return the existing req.user instead (e.g., return res.json({ user: req.user, authDisabled: true })), or if authenticateToken doesn't already set req.user in auth-disabled mode, update authenticateToken to populate req.user and then reference req.user here (use isAuthDisabled(), authenticateToken, and req.user to locate the relevant code).
🤖 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/middleware/auth.js`:
- Around line 8-10: The isAuthDisabled function in server/middleware/auth.js
currently allows auth bypass in any environment; update it to enforce a
hard-stop when running in production by checking process.env.NODE_ENV ===
'production' and, if AUTH_DISABLED is truthy, throw a descriptive Error (or call
process.exit(1)) to fail startup rather than returning true; keep the existing
case-insensitive check for AUTH_DISABLED but add the NODE_ENV guard and message
so the app cannot run with auth disabled in production.
In `@src/contexts/AuthContext.jsx`:
- Line 81: Replace the hardcoded fallback user in AuthContext.jsx — currently
setUser(statusData.user || { id: 1, username: 'admin' }) — with a non-deceptive
fallback (e.g., setUser(statusData.user ?? null) or undefined) so the client
doesn't fabricate an "admin" identity; update any downstream consumers to handle
a nullable user value. Target the setUser and statusData usage in the
AuthContext.jsx file and remove the hardcoded { id: 1, username: 'admin' }
fallback.
---
Nitpick comments:
In `@server/routes/auth.js`:
- Around line 132-139: The /user route is constructing a hardcoded fallback user
even though authenticateToken already sets req.user when auth is disabled;
remove the duplicated object and return the existing req.user instead (e.g.,
return res.json({ user: req.user, authDisabled: true })), or if
authenticateToken doesn't already set req.user in auth-disabled mode, update
authenticateToken to populate req.user and then reference req.user here (use
isAuthDisabled(), authenticateToken, and req.user to locate the relevant code).
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.env.exampleserver/middleware/auth.jsserver/routes/auth.jssrc/components/ProtectedRoute.jsxsrc/contexts/AuthContext.jsx
Comment on lines
+8
to
+10
| const isAuthDisabled = () => { | ||
| return String(process.env.AUTH_DISABLED || '').toLowerCase() === 'true'; | ||
| }; |
Contributor
There was a problem hiding this comment.
Add a production hard-stop for AUTH_DISABLED.
Line 8 currently allows full auth bypass in any environment if the env var is set. That makes accidental production exposure too easy. Please enforce a runtime guard (or startup failure) when NODE_ENV=production.
🔧 Suggested hardening
-const isAuthDisabled = () => {
- return String(process.env.AUTH_DISABLED || '').toLowerCase() === 'true';
-};
+const isAuthDisabled = () => {
+ const requested = String(process.env.AUTH_DISABLED || '').toLowerCase() === 'true';
+ if (requested && process.env.NODE_ENV === 'production') {
+ console.error('AUTH_DISABLED is blocked in production');
+ return false;
+ }
+ return requested;
+};📝 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
| const isAuthDisabled = () => { | |
| return String(process.env.AUTH_DISABLED || '').toLowerCase() === 'true'; | |
| }; | |
| const isAuthDisabled = () => { | |
| const requested = String(process.env.AUTH_DISABLED || '').toLowerCase() === 'true'; | |
| if (requested && process.env.NODE_ENV === 'production') { | |
| console.error('AUTH_DISABLED is blocked in production'); | |
| return false; | |
| } | |
| return requested; | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/middleware/auth.js` around lines 8 - 10, The isAuthDisabled function
in server/middleware/auth.js currently allows auth bypass in any environment;
update it to enforce a hard-stop when running in production by checking
process.env.NODE_ENV === 'production' and, if AUTH_DISABLED is truthy, throw a
descriptive Error (or call process.exit(1)) to fail startup rather than
returning true; keep the existing case-insensitive check for AUTH_DISABLED but
add the NODE_ENV guard and message so the app cannot run with auth disabled in
production.
src/contexts/AuthContext.jsx
Outdated
| setNeedsSetup(false); | ||
| setToken(compatibilityToken); | ||
| localStorage.setItem('auth-token', compatibilityToken); | ||
| setUser(statusData.user || { id: 1, username: 'admin' }); |
Contributor
There was a problem hiding this comment.
Hardcoded fallback user can desync displayed identity.
Line 81 forces { id: 1, username: 'admin' } when statusData.user is absent. In auth-disabled mode this can show the wrong user identity versus server-side fallback resolution.
🔧 Safer client fallback flow
if (statusData.authDisabled) {
const compatibilityToken = 'auth-disabled-token';
setAuthDisabled(true);
setNeedsSetup(false);
setToken(compatibilityToken);
localStorage.setItem('auth-token', compatibilityToken);
- setUser(statusData.user || { id: 1, username: 'admin' });
+ if (statusData.user) {
+ setUser(statusData.user);
+ } else {
+ const userResponse = await api.auth.user();
+ const userData = userResponse.ok ? await userResponse.json() : null;
+ setUser(userData?.user || { id: 1, username: 'admin' });
+ }
setHasCompletedOnboarding(true);
setIsLoading(false);
return;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/contexts/AuthContext.jsx` at line 81, Replace the hardcoded fallback user
in AuthContext.jsx — currently setUser(statusData.user || { id: 1, username:
'admin' }) — with a non-deceptive fallback (e.g., setUser(statusData.user ??
null) or undefined) so the client doesn't fabricate an "admin" identity; update
any downstream consumers to handle a nullable user value. Target the setUser and
statusData usage in the AuthContext.jsx file and remove the hardcoded { id: 1,
username: 'admin' } fallback.
Collaborator
|
@iamriajul, this PR is awesome. Do you have any new updates? |
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.
Recreates #175 on top of latest
main.This adds
AUTH_DISABLED=truesupport so authentication can be bypassed in private/local environments.What changed
AUTH_DISABLEDto .env.example.authDisabledstate.Behavior when AUTH_DISABLED=true
Closes #175
Summary by CodeRabbit