Userauth part6 api token#39
Conversation
- Create 002_auth_users.sql: users table with email/password, provider columns for future GitHub OAuth, auto-update trigger - Add auth config block (JWT_SECRET, JWT_EXPIRES_IN, bcryptRounds) - Add JWT_SECRET to production required env vars - Install jsonwebtoken and bcryptjs dependencies - Update .env.example with auth variables
- auth.js middleware: requireAuth (hard 401) and optionalAuth (soft attach) for JWT verification from Authorization: Bearer header - authService.js: registerUser, loginUser, getUserById with bcrypt hashing, email normalization, and sanitized user output - authController.js: HTTP handlers with input validation (email format, password length 8–128), proper error code propagation - Export query() from supabaseService.js for use by authService
- Create authRoutes.js: POST /register, POST /login, GET /me - Mount auth routes at /api/v1/auth in route index - Add auth endpoints to API documentation endpoint - Export requireAuth and optionalAuth from middleware index
- Add auth types and API service - Add AuthContext for managing user state and modal visibility - Add useAuth hook - Add LoginModal component with tabs for login and registration
- Create ProtectedRoute component for guarding routes - Create LoginPrompt component for inline auth gating - Guard the /audio route in App.tsx - Guard the notes and canvas panels in TranscriptDetail.tsx - Update Layout.tsx to show user avatar and logout button
- Auto-attach Authorization header in API service - Intercept 401 Unauthorized responses to clear token - Add auth endpoints to frontend config - Add authLimiter to protect backend auth routes
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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.
26 issues found across 25 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="backend/src/middleware/rateLimiter.js">
<violation number="1" location="backend/src/middleware/rateLimiter.js:81">
P3: Backend rate-limit documentation now omits the auth tier and incorrectly describes all tiers as one-minute windows. Update the README with the 20-per-15-minute authentication limit.</violation>
</file>
<file name="backend/.env.example">
<violation number="1" location="backend/.env.example:30">
P3: Placeholder value `your-jwt-secret-min-32-chars` is 28 characters, but its name suggests a minimum of 32. A user who copies this file literally might not notice the discrepancy. Either pad the placeholder to 32+ characters (e.g. `your-jwt-secret-min-32-chars-here`) or rename it to match.</violation>
</file>
<file name="src/components/Layout.tsx">
<violation number="1" location="src/components/Layout.tsx:276">
P3: The new Profile menu option is selectable but performs no action, so users receive a dead control. Remove it until a profile destination exists, or wire it to the intended route.</violation>
<violation number="2" location="src/components/Layout.tsx:288">
P2: Unauthenticated mobile visitors have no way to open the login modal from the navigation because this is hidden below `sm` and the mobile menu has no replacement. Keep a visible mobile sign-in entry or add it to the mobile menu.</violation>
</file>
<file name="src/components/LoginPrompt.tsx">
<violation number="1" location="src/components/LoginPrompt.tsx:26">
P2: Use the project's Button component from @/components/ui/button instead of a raw <button> element. The manual button reproduces the default variant styling but misses accessibility features (focus-visible rings), disabled state handling, and consistent SVG icon sizing that the shared Button provides.</violation>
</file>
<file name="backend/src/middleware/auth.js">
<violation number="1" location="backend/src/middleware/auth.js:18">
P3: Guest-capable routes silently ignore valid JWTs when clients use a lowercase `bearer` scheme. Parse the authorization scheme case-insensitively here as well.</violation>
</file>
<file name="services/api.ts">
<violation number="1" location="services/api.ts:60">
P2: The localStorage key `'btc-auth-token'` is hardcoded in `getAuthHeader()` in `api.ts`, duplicating the `AUTH_TOKEN_KEY` constant already defined in `AuthContext.tsx`. If the key ever changes in AuthContext, this function will silently stop reading the token and all authenticated requests will fail. Consider defining the key in a shared location (e.g., exporting it from a constants file, adding it to `config.ts`, or re-exporting from the api module) and importing it in both places.</violation>
<violation number="2" location="services/api.ts:115">
P2: After a 401 outside `/audio`, the app continues displaying the expired user as signed in because this bypasses AuthContext's `persistToken(null)`/`setUser(null)`. Notify the auth provider to clear its in-memory session (and open the login flow where appropriate) when the client receives an unauthorized response.</violation>
<violation number="3" location="services/api.ts:115">
P2: The `localStorage.removeItem('btc-auth-token')` on a 401 response runs before the guard excludes auth endpoints. If the login or register endpoint returns 401 for invalid credentials (a common API pattern), any previously valid session token is unnecessarily destroyed before the error surfaces. The removal should be moved inside the inner `if` block, guarded by the same conditions, so that failed login attempts don't wipe out an existing valid session.</violation>
</file>
<file name="src/contexts/AuthContext.tsx">
<violation number="1" location="src/contexts/AuthContext.tsx:46">
P1: A same-origin script injection can read and replay this bearer credential for its seven-day default lifetime. Prefer an HttpOnly, Secure, SameSite session cookie with CSRF protection, or keep access tokens only in memory with a refresh flow.</violation>
<violation number="2" location="src/contexts/AuthContext.tsx:66">
P1: Signing in before startup validation settles can overwrite or clear the newly created session. Associate validation with its original token and ignore/cancel stale completions; API 401 cleanup should remove a token only when it matches the request's credential.</violation>
<violation number="3" location="src/contexts/AuthContext.tsx:70">
P2: Temporary API failures erase a valid saved session, forcing users to authenticate again after offline, timeout, rate-limit, or server errors. Clear credentials only for an invalid-token 401; retain them and retry/surface validation failures otherwise.</violation>
</file>
<file name="backend/src/services/authService.js">
<violation number="1" location="backend/src/services/authService.js:25">
P3: Malformed login bodies currently produce a 500 because truthy non-string `email` values reach `.toLowerCase()`. Add typed auth validation middleware (and equivalent password/name constraints) before the controller so invalid requests return `VALIDATION_ERROR`.</violation>
<violation number="2" location="backend/src/services/authService.js:39">
P2: **Concurrent registration with the same email returns 500 instead of 409.**
When two registration requests arrive simultaneously with the same email, both pass the application-level SELECT check (no email exists yet), then the first INSERT succeeds and the second INSERT triggers a PostgreSQL unique-constraint violation (error code 23505). This raw database error has no `statusCode` property, so the controller's catch block (`err.statusCode === 409` is false) lets it fall through to the global error handler, which returns a **500 Internal Server Error**. The second user sees a server error rather than a clear 'Email already registered' message.
**Suggestion:** Wrap the INSERT in a try/catch that intercepts PostgreSQL error code `'23505'` (unique_violation) and re-throws a proper 409 error, similar to how the existing pattern handles errors with `err.statusCode`.
This is a low-probability race condition, but the impact is a degraded UX (500 vs. 409) that could confuse users during sign-up spikes.</violation>
<violation number="3" location="backend/src/services/authService.js:70">
P2: Login requests can enumerate registered emails by timing: nonexistent users return immediately, while existing users always perform `bcrypt.compare`. Perform a dummy bcrypt comparison before returning invalid credentials for a missing user.</violation>
</file>
<file name="backend/src/controllers/authController.js">
<violation number="1" location="backend/src/controllers/authController.js:19">
P2: Malformed registration bodies return 500 instead of `VALIDATION_ERROR`: an object `name` reaches `name?.trim()` in `registerUser`, and non-string passwords reach bcrypt. Validate field types before delegating.</violation>
<violation number="2" location="backend/src/controllers/authController.js:33">
P2: Password length validation allows up to 128 characters, but bcrypt silently truncates at 72 bytes. A password between 73–128 characters passes validation, yet only the first 72 bytes are hashed and verified. Consider capping the password at 72 characters or adding a warning so users aren't misled about the effective strength of longer passwords.</violation>
</file>
<file name="services/config.ts">
<violation number="1" location="services/config.ts:15">
P2: New auth endpoint constants in config.endpoints.auth are unused — authService.ts hardcodes the same path strings instead of referencing them. This creates a maintainability risk where the two sources could diverge. All other services (dataService, geminiService) consume config.endpoints.*; authService should follow the same pattern.</violation>
</file>
<file name="src/components/LoginModal.tsx">
<violation number="1" location="src/components/LoginModal.tsx:42">
P3: After dismissing Create Account, the next use of the existing Sign In entry point reopens the registration tab. Reset `activeTab` with the rest of the modal state so that entry point consistently shows sign-in.</violation>
<violation number="2" location="src/components/LoginModal.tsx:55">
P2: Closing a modal during a pending request leaves a reopened form disabled and can still sign the user in after they dismissed it. Veto close events while submitting, or cancel the request before resetting state.</violation>
<violation number="3" location="src/components/LoginModal.tsx:137">
P3: Login and registration failures are not announced to screen-reader users because the asynchronously inserted error banner is not a live region. Mark it as an alert so the failure is conveyed without moving focus.</violation>
<violation number="4" location="src/components/LoginModal.tsx:189">
P3: Keyboard users cannot reach the password visibility control because `tabIndex={-1}` removes it from tab order. Leave the button focusable and give the icon-only control an accessible name.</violation>
</file>
<file name="backend/supabase/migrations/002_auth_users.sql">
<violation number="1" location="backend/supabase/migrations/002_auth_users.sql:21">
P3: Writes to `users` will maintain two indexes for `email` even though the UNIQUE constraint already provides one. Removing the extra non-unique index avoids unnecessary storage and insert/update overhead.</violation>
<violation number="2" location="backend/supabase/migrations/002_auth_users.sql:33">
P2: This migration can fail on re-apply because trigger creation is not guarded while the rest of the file is idempotent. Dropping the trigger if it exists before creating it keeps reruns and local reset workflows reliable.</violation>
</file>
<file name="backend/src/routes/authRoutes.js">
<violation number="1" location="backend/src/routes/authRoutes.js:18">
P2: `GET /me` is now rate-limited with the same strict brute-force policy as login/register, so normal session checks can start returning 429 after repeated profile fetches from one IP. Scoping `authLimiter` to `/register` and `/login` keeps brute-force protection without throttling authenticated profile reads.</violation>
</file>
<file name="src/components/ProtectedRoute.tsx">
<violation number="1" location="src/components/ProtectedRoute.tsx:22">
P0: Protected route redirects immediately to `/` without ever showing the login modal when an unauthenticated user visits `/audio`.
The two `useEffect` hooks fire in the same render cycle — one opens the login modal (queues a state update), the other calls `navigate('/', {replace: true})`. Since the state update from opening the modal hasn't flushed yet, both conditions evaluate to `true`, and the navigation wins. The user is silently redirected to the homepage and never prompted to log in.
The fix is to merge the two effects into one and use a ref or previous-value check to distinguish "first time — show modal" from "modal was dismissed — redirect":
```tsx
const prevIsOpen = useRef(isLoginModalOpen);
useEffect(() => {
if (isLoading || user) return;
if (!isLoginModalOpen && prevIsOpen.current) {
navigate(redirectTo, { replace: true });
} else if (!isLoginModalOpen && !prevIsOpen.current) {
openLoginModal();
}
prevIsOpen.current = isLoginModalOpen;
}, [isLoading, user, isLoginModalOpen, openLoginModal, navigate, redirectTo]);
```</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| const { user, isLoading, openLoginModal, isLoginModalOpen } = useAuth(); | ||
| const navigate = useNavigate(); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
P0: Protected route redirects immediately to / without ever showing the login modal when an unauthenticated user visits /audio.
The two useEffect hooks fire in the same render cycle — one opens the login modal (queues a state update), the other calls navigate('/', {replace: true}). Since the state update from opening the modal hasn't flushed yet, both conditions evaluate to true, and the navigation wins. The user is silently redirected to the homepage and never prompted to log in.
The fix is to merge the two effects into one and use a ref or previous-value check to distinguish "first time — show modal" from "modal was dismissed — redirect":
const prevIsOpen = useRef(isLoginModalOpen);
useEffect(() => {
if (isLoading || user) return;
if (!isLoginModalOpen && prevIsOpen.current) {
navigate(redirectTo, { replace: true });
} else if (!isLoginModalOpen && !prevIsOpen.current) {
openLoginModal();
}
prevIsOpen.current = isLoginModalOpen;
}, [isLoading, user, isLoginModalOpen, openLoginModal, navigate, redirectTo]);Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/components/ProtectedRoute.tsx, line 22:
<comment>Protected route redirects immediately to `/` without ever showing the login modal when an unauthenticated user visits `/audio`.
The two `useEffect` hooks fire in the same render cycle — one opens the login modal (queues a state update), the other calls `navigate('/', {replace: true})`. Since the state update from opening the modal hasn't flushed yet, both conditions evaluate to `true`, and the navigation wins. The user is silently redirected to the homepage and never prompted to log in.
The fix is to merge the two effects into one and use a ref or previous-value check to distinguish "first time — show modal" from "modal was dismissed — redirect":
```tsx
const prevIsOpen = useRef(isLoginModalOpen);
useEffect(() => {
if (isLoading || user) return;
if (!isLoginModalOpen && prevIsOpen.current) {
navigate(redirectTo, { replace: true });
} else if (!isLoginModalOpen && !prevIsOpen.current) {
openLoginModal();
}
prevIsOpen.current = isLoginModalOpen;
}, [isLoading, user, isLoginModalOpen, openLoginModal, navigate, redirectTo]);
```</comment>
<file context>
@@ -0,0 +1,50 @@
+ const { user, isLoading, openLoginModal, isLoginModalOpen } = useAuth();
+ const navigate = useNavigate();
+
+ useEffect(() => {
+ // If we've finished loading and there is no user, trigger the login modal
+ if (!isLoading && !user && !isLoginModalOpen) {
</file context>
|
|
||
| try { | ||
| // Temporarily inject the token header for this request | ||
| const data = await authApi.me(); |
There was a problem hiding this comment.
P1: Signing in before startup validation settles can overwrite or clear the newly created session. Associate validation with its original token and ignore/cancel stale completions; API 401 cleanup should remove a token only when it matches the request's credential.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/contexts/AuthContext.tsx, line 66:
<comment>Signing in before startup validation settles can overwrite or clear the newly created session. Associate validation with its original token and ignore/cancel stale completions; API 401 cleanup should remove a token only when it matches the request's credential.</comment>
<file context>
@@ -0,0 +1,137 @@
+
+ try {
+ // Temporarily inject the token header for this request
+ const data = await authApi.me();
+ setUser(data.user);
+ } catch {
</file context>
| */ | ||
| const persistToken = useCallback((newToken: string | null) => { | ||
| if (newToken) { | ||
| localStorage.setItem(AUTH_TOKEN_KEY, newToken); |
There was a problem hiding this comment.
P1: A same-origin script injection can read and replay this bearer credential for its seven-day default lifetime. Prefer an HttpOnly, Secure, SameSite session cookie with CSRF protection, or keep access tokens only in memory with a refresh flow.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/contexts/AuthContext.tsx, line 46:
<comment>A same-origin script injection can read and replay this bearer credential for its seven-day default lifetime. Prefer an HttpOnly, Secure, SameSite session cookie with CSRF protection, or keep access tokens only in memory with a refresh flow.</comment>
<file context>
@@ -0,0 +1,137 @@
+ */
+ const persistToken = useCallback((newToken: string | null) => {
+ if (newToken) {
+ localStorage.setItem(AUTH_TOKEN_KEY, newToken);
+ } else {
+ localStorage.removeItem(AUTH_TOKEN_KEY);
</file context>
| ) : ( | ||
| <button | ||
| onClick={openLoginModal} | ||
| className="hidden sm:inline-flex items-center justify-center h-9 px-4 rounded-lg bg-primary text-primary-foreground font-medium text-sm hover:bg-primary/90 transition-colors whitespace-nowrap" |
There was a problem hiding this comment.
P2: Unauthenticated mobile visitors have no way to open the login modal from the navigation because this is hidden below sm and the mobile menu has no replacement. Keep a visible mobile sign-in entry or add it to the mobile menu.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/components/Layout.tsx, line 288:
<comment>Unauthenticated mobile visitors have no way to open the login modal from the navigation because this is hidden below `sm` and the mobile menu has no replacement. Keep a visible mobile sign-in entry or add it to the mobile menu.</comment>
<file context>
@@ -240,11 +250,52 @@ export const Layout = ({ children }: { children: React.ReactNode }) => {
+ ) : (
+ <button
+ onClick={openLoginModal}
+ className="hidden sm:inline-flex items-center justify-center h-9 px-4 rounded-lg bg-primary text-primary-foreground font-medium text-sm hover:bg-primary/90 transition-colors whitespace-nowrap"
+ >
+ Sign In
</file context>
| <p className="text-muted-foreground text-sm max-w-sm mb-6"> | ||
| {message} | ||
| </p> | ||
| <button |
There was a problem hiding this comment.
P2: Use the project's Button component from @/components/ui/button instead of a raw element. The manual button reproduces the default variant styling but misses accessibility features (focus-visible rings), disabled state handling, and consistent SVG icon sizing that the shared Button provides.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/components/LoginPrompt.tsx, line 26:
<comment>Use the project's Button component from @/components/ui/button instead of a raw <button> element. The manual button reproduces the default variant styling but misses accessibility features (focus-visible rings), disabled state handling, and consistent SVG icon sizing that the shared Button provides.</comment>
<file context>
@@ -0,0 +1,34 @@
+ <p className="text-muted-foreground text-sm max-w-sm mb-6">
+ {message}
+ </p>
+ <button
+ onClick={openLoginModal}
+ className="px-6 py-2.5 rounded-md bg-primary text-primary-foreground font-medium text-sm hover:bg-primary/90 transition-colors"
</file context>
| // Check if user already exists | ||
| const existing = await query( | ||
| 'SELECT id FROM users WHERE email = $1', | ||
| [email.toLowerCase().trim()] |
There was a problem hiding this comment.
P3: Malformed login bodies currently produce a 500 because truthy non-string email values reach .toLowerCase(). Add typed auth validation middleware (and equivalent password/name constraints) before the controller so invalid requests return VALIDATION_ERROR.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/src/services/authService.js, line 25:
<comment>Malformed login bodies currently produce a 500 because truthy non-string `email` values reach `.toLowerCase()`. Add typed auth validation middleware (and equivalent password/name constraints) before the controller so invalid requests return `VALIDATION_ERROR`.</comment>
<file context>
@@ -0,0 +1,139 @@
+ // Check if user already exists
+ const existing = await query(
+ 'SELECT id FROM users WHERE email = $1',
+ [email.toLowerCase().trim()]
+ );
+
</file context>
| const [regConfirm, setRegConfirm] = useState(''); | ||
| const [showRegPassword, setShowRegPassword] = useState(false); | ||
|
|
||
| const resetForms = () => { |
There was a problem hiding this comment.
P3: After dismissing Create Account, the next use of the existing Sign In entry point reopens the registration tab. Reset activeTab with the rest of the modal state so that entry point consistently shows sign-in.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/components/LoginModal.tsx, line 42:
<comment>After dismissing Create Account, the next use of the existing Sign In entry point reopens the registration tab. Reset `activeTab` with the rest of the modal state so that entry point consistently shows sign-in.</comment>
<file context>
@@ -0,0 +1,324 @@
+ const [regConfirm, setRegConfirm] = useState('');
+ const [showRegPassword, setShowRegPassword] = useState(false);
+
+ const resetForms = () => {
+ setLoginEmail('');
+ setLoginPassword('');
</file context>
| {/* Error banner */} | ||
| <AnimatePresence mode="wait"> | ||
| {error && ( | ||
| <motion.div |
There was a problem hiding this comment.
P3: Login and registration failures are not announced to screen-reader users because the asynchronously inserted error banner is not a live region. Mark it as an alert so the failure is conveyed without moving focus.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/components/LoginModal.tsx, line 137:
<comment>Login and registration failures are not announced to screen-reader users because the asynchronously inserted error banner is not a live region. Mark it as an alert so the failure is conveyed without moving focus.</comment>
<file context>
@@ -0,0 +1,324 @@
+ {/* Error banner */}
+ <AnimatePresence mode="wait">
+ {error && (
+ <motion.div
+ initial={{ opacity: 0, y: -8 }}
+ animate={{ opacity: 1, y: 0 }}
</file context>
| type="button" | ||
| onClick={() => setShowLoginPassword(!showLoginPassword)} | ||
| className="absolute right-3 top-1/2 -translate-y-1/2 text-muted-foreground hover:text-foreground transition-colors" | ||
| tabIndex={-1} |
There was a problem hiding this comment.
P3: Keyboard users cannot reach the password visibility control because tabIndex={-1} removes it from tab order. Leave the button focusable and give the icon-only control an accessible name.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/components/LoginModal.tsx, line 189:
<comment>Keyboard users cannot reach the password visibility control because `tabIndex={-1}` removes it from tab order. Leave the button focusable and give the icon-only control an accessible name.</comment>
<file context>
@@ -0,0 +1,324 @@
+ type="button"
+ onClick={() => setShowLoginPassword(!showLoginPassword)}
+ className="absolute right-3 top-1/2 -translate-y-1/2 text-muted-foreground hover:text-foreground transition-colors"
+ tabIndex={-1}
+ >
+ {showLoginPassword ? <EyeOff className="w-4 h-4" /> : <Eye className="w-4 h-4" />}
</file context>
| ); | ||
|
|
||
| -- Indexes | ||
| CREATE INDEX IF NOT EXISTS idx_users_email ON users(email); |
There was a problem hiding this comment.
P3: Writes to users will maintain two indexes for email even though the UNIQUE constraint already provides one. Removing the extra non-unique index avoids unnecessary storage and insert/update overhead.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/supabase/migrations/002_auth_users.sql, line 21:
<comment>Writes to `users` will maintain two indexes for `email` even though the UNIQUE constraint already provides one. Removing the extra non-unique index avoids unnecessary storage and insert/update overhead.</comment>
<file context>
@@ -0,0 +1,36 @@
+);
+
+-- Indexes
+CREATE INDEX IF NOT EXISTS idx_users_email ON users(email);
+CREATE INDEX IF NOT EXISTS idx_users_provider ON users(provider, provider_id);
+
</file context>
Summary by cubic
Adds end-to-end email/password authentication with JWT. Includes backend auth endpoints and middleware, plus a frontend AuthContext, login modal, and route guards.
New Features
POST /api/v1/auth/register,POST /api/v1/auth/login,GET /api/v1/auth/me.jsonwebtoken; password hashing viabcryptjs.requireAuthandoptionalAuthmiddleware; strictauthLimiterfor brute-force protection.useAuthhook, and a LoginModal with sign-in/register tabs.ProtectedRoutefor/audio; login prompts for Notes/Canvas.Migration
JWT_SECRET(generate with: node -e "console.log(require('crypto').randomBytes(64).toString('hex'))") and optionallyJWT_EXPIRES_INinbackend/.env.backend/supabase/migrations/002_auth_users.sqlto create theuserstable.Written for commit 3796592. Summary will update on new commits.