fix(api): fail fast when Google OAuth is not configured#70
Conversation
google_login/google_callback read GOOGLE_CLIENT_ID/SECRET with unwrap_or_default(), so an unconfigured deployment redirected users to Google with an empty client_id (a confusing Google error page) and the callback attempted a token exchange with empty credentials. Return a clear error when the credentials are missing, and document the vars in .env.example. Closes #51
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughConfiguration documentation and early validation guards are added to the Google OAuth flow. ChangesGoogle OAuth Configuration and Validation
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~10 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 docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/api/src/api/handlers/auth_handler.rs (1)
206-210: 💤 Low valueConsider a more semantically accurate HTTP status code.
The validation correctly fails fast when Google OAuth is unconfigured. However, HTTP 400 Bad Request typically indicates a client error in the request payload. Server misconfiguration might be better represented by 503 Service Unavailable or 501 Not Implemented, which the PR objectives mentioned.
Looking at the
AppErrorenum, these variants aren't available. You could consider:
- Using
AppError::InternalError(maps to 500), though that's less descriptive- Adding a new
ServiceUnavailablevariant toAppErrorfor features disabled by missing configurationThat said,
BadRequestwith a clear message is acceptable and meets the fail-fast requirement.🤖 Prompt for 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. In `@backend/api/src/api/handlers/auth_handler.rs` around lines 206 - 210, The current check returns AppError::BadRequest when client_id is empty; change this to a more semantically accurate error: either return AppError::InternalError (to map to 500) or add a new AppError::ServiceUnavailable (or ServiceDisabled) variant and return that here to represent missing server configuration; update the AppError enum and its HTTP mapping accordingly and replace the return in auth_handler.rs (the client_id validation block) to use the chosen variant so the response code reflects server misconfiguration rather than a client payload error.
🤖 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.
Nitpick comments:
In `@backend/api/src/api/handlers/auth_handler.rs`:
- Around line 206-210: The current check returns AppError::BadRequest when
client_id is empty; change this to a more semantically accurate error: either
return AppError::InternalError (to map to 500) or add a new
AppError::ServiceUnavailable (or ServiceDisabled) variant and return that here
to represent missing server configuration; update the AppError enum and its HTTP
mapping accordingly and replace the return in auth_handler.rs (the client_id
validation block) to use the chosen variant so the response code reflects server
misconfiguration rather than a client payload error.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 24147bcc-4e57-4f33-8eb9-3f937186edc1
📒 Files selected for processing (2)
.env.examplebackend/api/src/api/handlers/auth_handler.rs
fix(api): fail fast when Google OAuth is not configured
Summary
google_loginandgoogle_callbackreadGOOGLE_CLIENT_ID/GOOGLE_CLIENT_SECRETwithunwrap_or_default(). When unset (the default — these aren't in.env.example/compose),/auth/google/loginredirected users to...?client_id=&...(a Google error page) and the callback attempted a token exchange with empty credentials, instead of signalling a clear configuration error.Changes
Google OAuth is not configurederror from both handlers when the client id/secret are missing.GOOGLE_CLIENT_ID,GOOGLE_CLIENT_SECRET,GOOGLE_REDIRECT_URL.Testing
cargo check -p txio-apipasses.Closes #51
Summary by CodeRabbit