Userauth part4 frontend context#37
Open
parthdude07 wants to merge 7 commits into
Open
Conversation
|
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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
13 issues found across 17 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/config/index.js">
<violation number="1" location="backend/src/config/index.js:69">
P1: Auth is forgeable on any non-production deployment lacking `JWT_SECRET`, since this publicly known fallback signs and verifies every token. Require an explicit secret there too, or generate a cryptographically random per-process development secret rather than using a constant.</violation>
</file>
<file name="backend/src/middleware/auth.js">
<violation number="1" location="backend/src/middleware/auth.js:18">
P3: Valid bearer credentials using a lowercase/mixed-case scheme are rejected, although HTTP auth scheme names are case-insensitive. Match the scheme case-insensitively in both middleware paths before verifying the token.</violation>
</file>
<file name="backend/supabase/migrations/002_auth_users.sql">
<violation number="1" location="backend/supabase/migrations/002_auth_users.sql:8">
P0: On Supabase, this leaves every `users` row—including bcrypt password hashes—readable through the Data API to any role with table grants. Enable RLS immediately after creation and add no public policies; backend DB credentials bypass RLS unless forced.</violation>
<violation number="2" location="backend/supabase/migrations/002_auth_users.sql:21">
P3: This adds a duplicate email index: `UNIQUE` already supplies the lookup index used by the auth queries. Omitting `idx_users_email` avoids needless index storage and write maintenance.</violation>
</file>
<file name="src/components/LoginModal.tsx">
<violation number="1" location="src/components/LoginModal.tsx:23">
P1: Opening `isLoginModalOpen` cannot display a dialog because `LoginModal` is never mounted; mounting it as-is would also throw outside `AuthProvider`. Render the provider and modal in the application tree so `openLoginModal()` has a UI target.</violation>
<violation number="2" location="src/components/LoginModal.tsx:104">
P2: Registration controls become unreachable on short viewports because this fixed dialog clips overflow without a maximum height or scroll container. Bound it to the viewport and allow vertical scrolling.</violation>
<violation number="3" location="src/components/LoginModal.tsx:137">
P3: Failed login or registration errors are not announced to screen-reader users because this dynamically inserted banner lacks live-region semantics. Mark it as an alert.</violation>
<violation number="4" location="src/components/LoginModal.tsx:189">
P3: Keyboard and screen-reader users cannot operate either password visibility toggle because both buttons are removed from tab order and have no accessible name. Keep them focusable and add state-dependent `aria-label`s.</violation>
</file>
<file name="backend/src/services/authService.js">
<violation number="1" location="backend/src/services/authService.js:39">
P1: Concurrent registration attempts for the same email can return a server error instead of `EMAIL_EXISTS`. The separate pre-check plus insert leaves a race window; handling conflict at insert time (or `ON CONFLICT`) keeps duplicate-email responses consistently 409.</violation>
<violation number="2" location="backend/src/services/authService.js:49">
P2: Registration/login logs currently store full email addresses, which persists user PII in log sinks. Prefer masked email or stable internal identifiers (for example `user.id`) in auth log messages.</violation>
</file>
<file name="backend/src/controllers/authController.js">
<violation number="1" location="backend/src/controllers/authController.js:19">
P2: Malformed request types can produce 500s because `register`/`login` validate presence but not string type before calling authService. Adding explicit `typeof email/password === 'string'` (and non-empty trimmed email) checks would keep bad input in the 400-validation path.</violation>
<violation number="2" location="backend/src/controllers/authController.js:38">
P2: Registration can fail with a server error when `name` is sent as a non-string because the controller passes it through without validation. Coercing/validating `name` as an optional string before calling `registerUser` avoids that runtime exception.</violation>
</file>
<file name="src/contexts/AuthContext.tsx">
<violation number="1" location="src/contexts/AuthContext.tsx:66">
P0: The `/auth/me` token validation always fails on page reload because the stored JWT is never sent with the request. The `services/api.ts` fetch client has no interceptor to attach `Authorization: Bearer <token>` from localStorage, and `authApi.me()` does not pass the token explicitly. As a result, every refresh will: read the stored token → call `authApi.me()` with no auth header → get a 401 → clear the stored token and user. The user will never stay logged in across page loads.
**Fix option A (API service level, preferred):** Add a header in the API `request()` function that reads the token from localStorage under `btc-auth-token` and sets the `Authorization` header.
**Fix option B (caller level):** Refactor `authApi.me()` (and future authenticated calls) to accept and forward a token header, then pass it in AuthContext from state.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| -- ============================================================ | ||
|
|
||
| -- Users table | ||
| CREATE TABLE IF NOT EXISTS users ( |
There was a problem hiding this comment.
P0: On Supabase, this leaves every users row—including bcrypt password hashes—readable through the Data API to any role with table grants. Enable RLS immediately after creation and add no public policies; backend DB credentials bypass RLS unless forced.
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 8:
<comment>On Supabase, this leaves every `users` row—including bcrypt password hashes—readable through the Data API to any role with table grants. Enable RLS immediately after creation and add no public policies; backend DB credentials bypass RLS unless forced.</comment>
<file context>
@@ -0,0 +1,36 @@
+-- ============================================================
+
+-- Users table
+CREATE TABLE IF NOT EXISTS users (
+ id UUID PRIMARY KEY DEFAULT gen_random_uuid(),
+ email TEXT UNIQUE NOT NULL,
</file context>
|
|
||
| try { | ||
| // Temporarily inject the token header for this request | ||
| const data = await authApi.me(); |
There was a problem hiding this comment.
P0: The /auth/me token validation always fails on page reload because the stored JWT is never sent with the request. The services/api.ts fetch client has no interceptor to attach Authorization: Bearer <token> from localStorage, and authApi.me() does not pass the token explicitly. As a result, every refresh will: read the stored token → call authApi.me() with no auth header → get a 401 → clear the stored token and user. The user will never stay logged in across page loads.
Fix option A (API service level, preferred): Add a header in the API request() function that reads the token from localStorage under btc-auth-token and sets the Authorization header.
Fix option B (caller level): Refactor authApi.me() (and future authenticated calls) to accept and forward a token header, then pass it in AuthContext from state.
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>The `/auth/me` token validation always fails on page reload because the stored JWT is never sent with the request. The `services/api.ts` fetch client has no interceptor to attach `Authorization: Bearer <token>` from localStorage, and `authApi.me()` does not pass the token explicitly. As a result, every refresh will: read the stored token → call `authApi.me()` with no auth header → get a 401 → clear the stored token and user. The user will never stay logged in across page loads.
**Fix option A (API service level, preferred):** Add a header in the API `request()` function that reads the token from localStorage under `btc-auth-token` and sets the `Authorization` header.
**Fix option B (caller level):** Refactor `authApi.me()` (and future authenticated calls) to accept and forward a token header, then pass it in AuthContext from state.</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>
|
|
||
| // Authentication (JWT) | ||
| auth: { | ||
| jwtSecret: process.env.JWT_SECRET || 'dev-secret-change-in-production', |
There was a problem hiding this comment.
P1: Auth is forgeable on any non-production deployment lacking JWT_SECRET, since this publicly known fallback signs and verifies every token. Require an explicit secret there too, or generate a cryptographically random per-process development secret rather than using a constant.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/src/config/index.js, line 69:
<comment>Auth is forgeable on any non-production deployment lacking `JWT_SECRET`, since this publicly known fallback signs and verifies every token. Require an explicit secret there too, or generate a cryptographically random per-process development secret rather than using a constant.</comment>
<file context>
@@ -64,6 +64,13 @@ const config = {
+ // Authentication (JWT)
+ auth: {
+ jwtSecret: process.env.JWT_SECRET || 'dev-secret-change-in-production',
+ jwtExpiresIn: process.env.JWT_EXPIRES_IN || '7d',
+ bcryptRounds: 12,
</file context>
| import { Label } from '@/components/ui/label'; | ||
| import { useAuth } from '@/hooks/useAuth'; | ||
|
|
||
| export function LoginModal() { |
There was a problem hiding this comment.
P1: Opening isLoginModalOpen cannot display a dialog because LoginModal is never mounted; mounting it as-is would also throw outside AuthProvider. Render the provider and modal in the application tree so openLoginModal() has a UI target.
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 23:
<comment>Opening `isLoginModalOpen` cannot display a dialog because `LoginModal` is never mounted; mounting it as-is would also throw outside `AuthProvider`. Render the provider and modal in the application tree so `openLoginModal()` has a UI target.</comment>
<file context>
@@ -0,0 +1,324 @@
+import { Label } from '@/components/ui/label';
+import { useAuth } from '@/hooks/useAuth';
+
+export function LoginModal() {
+ const { isLoginModalOpen, closeLoginModal, login, register } = useAuth();
+
</file context>
| const hashedPassword = await bcrypt.hash(password, config.auth.bcryptRounds); | ||
|
|
||
| // Insert user | ||
| const result = await query( |
There was a problem hiding this comment.
P1: Concurrent registration attempts for the same email can return a server error instead of EMAIL_EXISTS. The separate pre-check plus insert leaves a race window; handling conflict at insert time (or ON CONFLICT) keeps duplicate-email responses consistently 409.
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 39:
<comment>Concurrent registration attempts for the same email can return a server error instead of `EMAIL_EXISTS`. The separate pre-check plus insert leaves a race window; handling conflict at insert time (or `ON CONFLICT`) keeps duplicate-email responses consistently 409.</comment>
<file context>
@@ -0,0 +1,139 @@
+ const hashedPassword = await bcrypt.hash(password, config.auth.bcryptRounds);
+
+ // Insert user
+ const result = await query(
+ `INSERT INTO users (email, password, name)
+ VALUES ($1, $2, $3)
</file context>
| @@ -0,0 +1,84 @@ | |||
| /** | |||
There was a problem hiding this comment.
P2: Malformed request types can produce 500s because register/login validate presence but not string type before calling authService. Adding explicit typeof email/password === 'string' (and non-empty trimmed email) checks would keep bad input in the 400-validation path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/src/controllers/authController.js, line 19:
<comment>Malformed request types can produce 500s because `register`/`login` validate presence but not string type before calling authService. Adding explicit `typeof email/password === 'string'` (and non-empty trimmed email) checks would keep bad input in the 400-validation path.</comment>
<file context>
@@ -0,0 +1,84 @@
+ const { email, password, name } = req.body;
+
+ // Input validation
+ if (!email || !password) {
+ throw new APIError('Email and password are required', 400, 'VALIDATION_ERROR');
+ }
</file context>
| export const requireAuth = (req, _res, next) => { | ||
| const authHeader = req.headers.authorization; | ||
|
|
||
| if (!authHeader || !authHeader.startsWith('Bearer ')) { |
There was a problem hiding this comment.
P3: Valid bearer credentials using a lowercase/mixed-case scheme are rejected, although HTTP auth scheme names are case-insensitive. Match the scheme case-insensitively in both middleware paths before verifying the token.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/src/middleware/auth.js, line 18:
<comment>Valid bearer credentials using a lowercase/mixed-case scheme are rejected, although HTTP auth scheme names are case-insensitive. Match the scheme case-insensitively in both middleware paths before verifying the token.</comment>
<file context>
@@ -0,0 +1,59 @@
+export const requireAuth = (req, _res, next) => {
+ const authHeader = req.headers.authorization;
+
+ if (!authHeader || !authHeader.startsWith('Bearer ')) {
+ throw new APIError('Authentication required', 401, 'UNAUTHORIZED');
+ }
</file context>
| ); | ||
|
|
||
| -- Indexes | ||
| CREATE INDEX IF NOT EXISTS idx_users_email ON users(email); |
There was a problem hiding this comment.
P3: This adds a duplicate email index: UNIQUE already supplies the lookup index used by the auth queries. Omitting idx_users_email avoids needless index storage and write maintenance.
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>This adds a duplicate email index: `UNIQUE` already supplies the lookup index used by the auth queries. Omitting `idx_users_email` avoids needless index storage and write maintenance.</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>
| {/* Error banner */} | ||
| <AnimatePresence mode="wait"> | ||
| {error && ( | ||
| <motion.div |
There was a problem hiding this comment.
P3: Failed login or registration errors are not announced to screen-reader users because this dynamically inserted banner lacks live-region semantics. Mark it as an alert.
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>Failed login or registration errors are not announced to screen-reader users because this dynamically inserted banner lacks live-region semantics. Mark it as an alert.</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 and screen-reader users cannot operate either password visibility toggle because both buttons are removed from tab order and have no accessible name. Keep them focusable and add state-dependent aria-labels.
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 and screen-reader users cannot operate either password visibility toggle because both buttons are removed from tab order and have no accessible name. Keep them focusable and add state-dependent `aria-label`s.</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>
- 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
parthdude07
force-pushed
the
userauth-part4-frontend-context
branch
from
July 21, 2026 09:53
efc52dc to
dbd89e0
Compare
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
1 participant
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 by cubic
Adds JWT auth and a full notes + whiteboard system integrated into transcripts and the library. Users can register/login, take rich notes with tags/colors, and build concept maps; the API exposes
/api/v1/authroutes.New Features
POST /auth/register,POST /auth/login,GET /auth/mewith JWT viajsonwebtoken, bcrypt hashing viabcryptjs, andrequireAuth.AuthContextwith token persistence and auto-validate,useAuth, and a tabbedLoginModal.useNotes,NoteEditor/NoteCard/TranscriptNotes, and HighlightToolbar “Add Note” support; library now shows notes.@xyflow/react(TranscriptWhiteboard,ConceptNode), persisted per transcript; highlights support color/underline and TranscriptChat accepts a prefilled prompt.Migration
JWT_SECRET(and optionalJWT_EXPIRES_IN) to your environment.002_auth_users.sqlto create theuserstable.@xyflow/react, then restart the backend after setting env vars and running the migration.Written for commit dbd89e0. Summary will update on new commits.