Userauth part8 schema cleanup#41
Open
parthdude07 wants to merge 18 commits into
Open
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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
40 issues found across 57 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/supabase/migrations/002_audiobook_seed.sql">
<violation number="1" location="backend/supabase/migrations/002_audiobook_seed.sql:56">
P1: The six Bitcoin Fundamentals chapters now all play the Wallets recording because every `audio_url` is the same unrelated file. Each episode should reference its own uploaded asset, or remain `NULL` until that asset exists; otherwise the published learning path serves incorrect content.</violation>
</file>
<file name="backend/src/config/index.js">
<violation number="1" location="backend/src/config/index.js:70">
P1: A deployment with `NODE_ENV` unset or set to anything other than exactly `production` can run the authentication middleware with a publicly known JWT key, so an attacker can forge any user's token. Restrict this fallback to explicit development mode and fail startup whenever a non-development environment lacks `JWT_SECRET`.</violation>
</file>
<file name="backend/.env.example">
<violation number="1" location="backend/.env.example:30">
P1: Production can accept this example value as a real signing key, allowing anyone who knows the repository to forge JWTs for arbitrary users; the value is also shorter than the stated 32-character minimum. Leaving the example empty would make production startup fail until a real secret is supplied, or the config could explicitly reject this placeholder and enforce the minimum length.</violation>
</file>
<file name="src/contexts/AuthContext.tsx">
<violation number="1" location="src/contexts/AuthContext.tsx:70">
P2: A temporary network, timeout, or 5xx failure while validating `/auth/me` deletes an otherwise valid token because this catch clears auth for every error, not only invalid-auth responses. Preserve the token on transient failures and clear it only for a confirmed 401/invalid-token response.</violation>
<violation number="2" location="src/contexts/AuthContext.tsx:77">
P1: A login can complete while the mount validation is in flight, allowing the stale `/auth/me` continuation to overwrite the newly logged-in user or call `persistToken(null)` and erase the new token. Guard auth actions until validation finishes or ignore validation results after a token-changing action.</violation>
<violation number="3" location="src/contexts/AuthContext.tsx:112">
P1: Logging out and signing in as another account can briefly show the previous account's private notes: `logout` only clears AuthContext state, while `useNotes` retains shared `['notes']` data and enables it for the next user before refetch. Removing user-scoped queries on logout or keying them by user ID would prevent this.</violation>
</file>
<file name="backend/supabase/migrations/003_user_data.sql">
<violation number="1" location="backend/supabase/migrations/003_user_data.sql:23">
P1: When this migration is applied to Supabase, unauthenticated Data API clients can read or modify private notes and choose another `user_id`, bypassing the authenticated backend routes. Enabling RLS with no public policies (the backend uses a direct pool) or explicitly revoking Data API privileges would keep notes behind the backend gateway.</violation>
<violation number="2" location="backend/supabase/migrations/003_user_data.sql:43">
P2: Authenticated audiobook progress is not actually linked to users: the referenced table still stores a free-form `TEXT` user ID with no `users(id)` foreign key, so deleting an account leaves orphaned progress and the schema comment is false. Adding the claimed type/FK migration with a plan for existing session IDs, or correcting the comment and explicitly deferring that change, would make the schema contract accurate.</violation>
</file>
<file name="src/components/notes/NoteEditor.tsx">
<violation number="1" location="src/components/notes/NoteEditor.tsx:53">
P1: Switching directly from one note to another keeps the first note's title, content, color, and tags, then can overwrite the newly selected note with those stale values. Synchronizing the local fields whenever `editingNote` changes would keep the editor aligned with the selected note.</violation>
</file>
<file name="src/hooks/useWhiteboard.ts">
<violation number="1" location="src/hooks/useWhiteboard.ts:4">
P2: A user's whiteboard data is not isolated from other accounts on the same browser profile because the local-storage key contains only `transcriptId`. Scope the key by the authenticated user identity (without reading/managing the JWT directly) or clear account-scoped state on logout.</violation>
<violation number="2" location="src/hooks/useWhiteboard.ts:15">
P1: Switching transcripts can display and save the previous transcript's graph under the new transcript ID when the new key has no stored value (or omits one field). Reset both collections to empty/default values before applying the newly loaded snapshot, and treat the load as a new loading cycle.</violation>
<violation number="3" location="src/hooks/useWhiteboard.ts:17">
P2: Corrupt or stale local-storage data can put non-array values into the React Flow collections and break the whiteboard. Validate that each parsed collection is an array before calling its setter, with an empty-array fallback.</violation>
</file>
<file name="src/App.tsx">
<violation number="1" location="src/App.tsx:64">
P2: The new `/audio` route exposes a Generate Audio workflow that can never succeed: every `generateSpeech` call posts to `/api/v1/ai/tts`, whose backend handler currently returns 503 because TTS is disabled. Either enable/implement that endpoint before routing this page, or keep the route unavailable until it is supported.</violation>
</file>
<file name="src/components/ProtectedRoute.tsx">
<violation number="1" location="src/components/ProtectedRoute.tsx:24">
P2: Unauthenticated navigation is redirected to `redirectTo` immediately when the route first opens the login modal, so the requested protected page is lost before the login flow completes. Tracking whether this guard opened the modal (or otherwise skipping the initial redirect) would make the redirect run only after the modal is actually closed without authentication.</violation>
</file>
<file name="services/api.ts">
<violation number="1" location="services/api.ts:114">
P2: A non-JSON 401 never reaches this block, leaving `btc-auth-token` in storage and causing subsequent requests to reuse the invalid token. Handle the 401 status before parsing the body, or clear the token in the parse-failure path, so the documented any-401 cleanup is reliable.</violation>
<violation number="2" location="services/api.ts:115">
P2: After a later 401 on any non-`/audio` route, the app removes the token but keeps `AuthContext.user`, so the header still presents the session as authenticated and `ProtectedRoute`/the login modal is not triggered. Route the expiry through `AuthContext` (for example, via an auth-expired event that calls `persistToken(null)` and clears `user`) instead of relying on an `/audio`-only reload.</violation>
</file>
<file name="backend/src/controllers/notesController.js">
<violation number="1" location="backend/src/controllers/notesController.js:42">
P2: Concept extraction always fails because the existing frontend flow sends an empty `content` while providing the selected text separately. Using `selected_text` as the concept content (or changing the client payload) would preserve this workflow while still rejecting genuinely empty notes.</violation>
</file>
<file name="backend/src/routes/authRoutes.js">
<violation number="1" location="backend/src/routes/authRoutes.js:18">
P2: The shared brute-force limiter currently protects `/me` as well as the public auth endpoints, allowing harmless unauthenticated `/me` requests to block login or registration for that IP. Scoping `authLimiter` to `/register` and `/login` would preserve the brute-force protection without rate-limiting the profile endpoint with the wrong bucket.</violation>
<violation number="2" location="backend/src/routes/authRoutes.js:21">
P2: Malformed or unsanitized auth input reaches the controllers without the repository-required express-validator middleware. Adding auth-specific rules for string/email/password/name constraints plus `trim()`/`escape()` to both public routes would keep invalid requests at the documented 400 validation response instead of allowing type errors or unconstrained data into the service.</violation>
</file>
<file name="backend/src/controllers/authController.js">
<violation number="1" location="backend/src/controllers/authController.js:33">
P2: Passwords accepted by this check can exceed bcryptjs's 72-byte input limit, making the suffix ineffective and allowing the registered password's first 72 bytes to authenticate by themselves. Enforce the bcrypt byte limit (or pre-hash before bcrypt) and apply the same rule during login.</violation>
</file>
<file name="backend/src/services/notesService.js">
<violation number="1" location="backend/src/services/notesService.js:82">
P2: Note creation accepts unbounded client-controlled text, arrays, and flags, so a user can repeatedly store very large rows or trigger database errors with malformed field types. Field-specific type and size validation before the insert would keep storage and response costs bounded.</violation>
</file>
<file name="src/components/Layout.tsx">
<violation number="1" location="src/components/Layout.tsx:277">
P2: The Profile option is a selectable no-op, so users receive no profile page or other result after choosing it. Connecting it to an implemented destination or removing the item until profile support exists would avoid presenting a broken workflow.</violation>
<violation number="2" location="src/components/Layout.tsx:289">
P2: Unauthenticated users on narrow screens cannot open the login modal: this Sign In button is hidden below `sm`, and the mobile menu has no replacement auth action. Exposing the button in the mobile layout or adding Sign In to the mobile nav would keep authentication reachable.</violation>
</file>
<file name="src/pages/TranscriptDetail.tsx">
<violation number="1" location="src/pages/TranscriptDetail.tsx:44">
P2: The `?tab=notes` deep link does not open Notes: `activeTab` becomes `"notes"`, but the Notes panel is only configured by the click handler that sets `rightTab` and `focusMode`. Synchronizing the query-derived state on mount and search changes, mapping `notes` to the transcript plus the Notes side panel, would make Library note links open the expected editor.</violation>
<violation number="2" location="src/pages/TranscriptDetail.tsx:143">
P2: Concept extraction reports success even when note creation fails: `addNote` is a fire-and-forget mutation, but this unconditional toast and the following Canvas transition run immediately. Gating anonymous users and showing success or switching to Canvas only from the mutation success path would prevent contradictory feedback and a false result.</violation>
</file>
<file name="src/components/notes/TranscriptNotes.tsx">
<violation number="1" location="src/components/notes/TranscriptNotes.tsx:65">
P2: Selecting text for a note updates the parent and child during `TranscriptNotes` render, which triggers React's cross-component render update warning and can produce inconsistent render timing. Moving this auto-open/consume sequence into an effect keyed by `pendingSelectedText` would preserve the behavior without render-phase side effects.</violation>
<violation number="2" location="src/components/notes/TranscriptNotes.tsx:85">
P2: If the notes API rejects a create or update, the editor is already unmounted, so the user's draft/edit is lost and recovery is limited to a generic error toast. Keeping the editor open until mutation success, or restoring it on error, would preserve the user's input.</violation>
</file>
<file name="backend/src/services/authService.js">
<violation number="1" location="backend/src/services/authService.js:28">
P2: Concurrent registrations for the same normalized email can return 500 instead of the documented 409: both requests can pass the pre-check, then the unique constraint rejects one insert. An atomic `INSERT ... ON CONFLICT (email) DO NOTHING RETURNING ...` with an empty-result `EMAIL_EXISTS` response would preserve the expected behavior.</violation>
</file>
<file name="backend/src/controllers/audiobookController.js">
<violation number="1" location="backend/src/controllers/audiobookController.js:205">
P2: Malformed authenticated identities can now reach the progress write because this line bypasses the removed UUID validation, while `requireAuth` only verifies the JWT signature. Validating `req.user?.id` with `isUUID` and returning an authentication error before calling the service would prevent bad rows and database 500s.</violation>
</file>
<file name="src/components/LoginModal.tsx">
<violation number="1" location="src/components/LoginModal.tsx:55">
P2: Closing the dialog during an in-flight authentication request can repopulate `error` after `resetForms()` has run, so the next open may show a stale failure and inherit the old submission state. Prevent closing while `isSubmitting` or cancel/ignore stale request completions before updating this component’s state.</violation>
<violation number="2" location="src/components/LoginModal.tsx:189">
P2: The password visibility controls are inaccessible to keyboard and screen-reader users because `tabIndex={-1}` removes both buttons from the tab order and the icon-only buttons have no accessible name. Keeping them focusable and adding dynamic `aria-label` values such as “Show password”/“Hide password” would make both toggles operable.</violation>
</file>
<file name="src/components/notes/NoteTagInput.tsx">
<violation number="1" location="src/components/notes/NoteTagInput.tsx:15">
P2: Ctrl/Cmd+Enter in the tag field saves the note without the tag currently being entered. Excluding modified Enter events here lets NoteEditor retain its documented save shortcut.</violation>
<violation number="2" location="src/components/notes/NoteTagInput.tsx:42">
P2: Screen readers expose each remove control without a meaningful name, so users cannot tell which tag it removes. Adding an accessible label that includes the tag text makes tag removal operable.</violation>
</file>
<file name="backend/src/routes/notesRoutes.js">
<violation number="1" location="backend/src/routes/notesRoutes.js:23">
P2: Malformed note requests can bypass the API's required validation layer and reach database operations; a POST without a parsed body can even throw while destructuring `req.body`. Adding note-specific express-validator rules for query, body, and path values and attaching `validate` before these handlers would make invalid input return the documented 400 response.</violation>
</file>
<file name="services/notesService.ts">
<violation number="1" location="services/notesService.ts:31">
P2: A note with a null database title can crash the notes search because `rowToNote` exposes `row.title` unchanged and the UI calls `.toLowerCase()` on it. Applying the same default used by the backend (`row.title ?? 'Untitled Note'`) keeps the frontend `Note` shape safe.</violation>
<violation number="2" location="services/notesService.ts:35">
P2: Selected note colors are discarded on every API round trip: `create`/`update` never send `color`, and `rowToNote` always restores `slate`. Persist the color through the notes schema/API, or remove the UI's color-editing behavior so users are not shown a setting that cannot be saved.</violation>
</file>
<file name="src/components/whiteboard/TranscriptWhiteboard.tsx">
<violation number="1" location="src/components/whiteboard/TranscriptWhiteboard.tsx:68">
P2: Deleting a selected concept removes its node but leaves connected edges in the whiteboard state, so stale connections can reappear when that node ID is restored and invalid edge records remain persisted. The node-sync/delete path should also remove edges whose `source` or `target` is no longer present.</violation>
</file>
<file name="src/pages/Audiobooks.tsx">
<violation number="1" location="src/pages/Audiobooks.tsx:16">
P2: Published `collection` playlists will now appear in the Audiobook Learning Paths grid because omitting the parameter removes the server-side `playlist_type = 'series'` filter. Keeping the filter preserves this page's prior series-only behavior, unless collections are intentionally meant to be shown here.</violation>
</file>
<file name="backend/src/routes/audiobookRoutes.js">
<violation number="1" location="backend/src/routes/audiobookRoutes.js:85">
P2: Progress saves are now authentication-gated, but this endpoint is still documented as public and as accepting a client-supplied `user_id`, causing documented anonymous/client-session calls to fail with 401. Updating the route/controller contract to require the Bearer token and identify the authenticated user from `req.user.id` would keep the API documentation aligned with this middleware change.</violation>
</file>
<file name="backend/supabase/migrations/002_auth_users.sql">
<violation number="1" location="backend/supabase/migrations/002_auth_users.sql:33">
P2: Rerunning this migration fails because `trg_users_updated_at` already exists, despite the surrounding migration being written as rerunnable. Dropping the trigger first, as the other user-data migrations do, keeps manual migration retries safe.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| 'How it all began — Satoshi Nakamoto''s whitepaper, the cypherpunk movement, and the creation of the first decentralized digital currency.', | ||
| 1, | ||
| NULL, | ||
| 'https://duqjdsbziertijeycmbk.supabase.co/storage/v1/object/public/audiobooks/Bitcoin%20Beginners%20Guide/Why_Bitcoin_wallets_hold_no_digital_coins%20(1).m4a', |
There was a problem hiding this comment.
P1: The six Bitcoin Fundamentals chapters now all play the Wallets recording because every audio_url is the same unrelated file. Each episode should reference its own uploaded asset, or remain NULL until that asset exists; otherwise the published learning path serves incorrect content.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/supabase/migrations/002_audiobook_seed.sql, line 56:
<comment>The six Bitcoin Fundamentals chapters now all play the Wallets recording because every `audio_url` is the same unrelated file. Each episode should reference its own uploaded asset, or remain `NULL` until that asset exists; otherwise the published learning path serves incorrect content.</comment>
<file context>
@@ -53,7 +53,7 @@ VALUES
'How it all began — Satoshi Nakamoto''s whitepaper, the cypherpunk movement, and the creation of the first decentralized digital currency.',
1,
- NULL,
+ 'https://duqjdsbziertijeycmbk.supabase.co/storage/v1/object/public/audiobooks/Bitcoin%20Beginners%20Guide/Why_Bitcoin_wallets_hold_no_digital_coins%20(1).m4a',
372,
'pending',
</file context>
|
|
||
| // Authentication (JWT) | ||
| auth: { | ||
| jwtSecret: process.env.JWT_SECRET || 'dev-secret-change-in-production', |
There was a problem hiding this comment.
P1: A deployment with NODE_ENV unset or set to anything other than exactly production can run the authentication middleware with a publicly known JWT key, so an attacker can forge any user's token. Restrict this fallback to explicit development mode and fail startup whenever a non-development environment lacks JWT_SECRET.
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 70:
<comment>A deployment with `NODE_ENV` unset or set to anything other than exactly `production` can run the authentication middleware with a publicly known JWT key, so an attacker can forge any user's token. Restrict this fallback to explicit development mode and fail startup whenever a non-development environment lacks `JWT_SECRET`.</comment>
<file context>
@@ -65,6 +65,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>
|
|
||
| # Authentication (JWT) | ||
| # Generate a strong secret: node -e "console.log(require('crypto').randomBytes(64).toString('hex'))" | ||
| JWT_SECRET=your-jwt-secret-min-32-chars |
There was a problem hiding this comment.
P1: Production can accept this example value as a real signing key, allowing anyone who knows the repository to forge JWTs for arbitrary users; the value is also shorter than the stated 32-character minimum. Leaving the example empty would make production startup fail until a real secret is supplied, or the config could explicitly reject this placeholder and enforce the minimum length.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/.env.example, line 30:
<comment>Production can accept this example value as a real signing key, allowing anyone who knows the repository to forge JWTs for arbitrary users; the value is also shorter than the stated 32-character minimum. Leaving the example empty would make production startup fail until a real secret is supplied, or the config could explicitly reject this placeholder and enforce the minimum length.</comment>
<file context>
@@ -24,3 +24,8 @@ RATE_LIMIT_MAX_REQUESTS=100
+
+# Authentication (JWT)
+# Generate a strong secret: node -e "console.log(require('crypto').randomBytes(64).toString('hex'))"
+JWT_SECRET=your-jwt-secret-min-32-chars
+JWT_EXPIRES_IN=7d
</file context>
| /** | ||
| * Log out — clear token and user state | ||
| */ | ||
| const logout = useCallback(() => { |
There was a problem hiding this comment.
P1: Logging out and signing in as another account can briefly show the previous account's private notes: logout only clears AuthContext state, while useNotes retains shared ['notes'] data and enables it for the next user before refetch. Removing user-scoped queries on logout or keying them by user ID would prevent this.
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 112:
<comment>Logging out and signing in as another account can briefly show the previous account's private notes: `logout` only clears AuthContext state, while `useNotes` retains shared `['notes']` data and enables it for the next user before refetch. Removing user-scoped queries on logout or keying them by user ID would prevent this.</comment>
<file context>
@@ -0,0 +1,137 @@
+ /**
+ * Log out — clear token and user state
+ */
+ const logout = useCallback(() => {
+ persistToken(null);
+ setUser(null);
</file context>
| ); | ||
|
|
||
| -- Index for fast lookup by user | ||
| CREATE INDEX IF NOT EXISTS idx_notes_user_id ON notes(user_id); |
There was a problem hiding this comment.
P1: When this migration is applied to Supabase, unauthenticated Data API clients can read or modify private notes and choose another user_id, bypassing the authenticated backend routes. Enabling RLS with no public policies (the backend uses a direct pool) or explicitly revoking Data API privileges would keep notes behind the backend gateway.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/supabase/migrations/003_user_data.sql, line 23:
<comment>When this migration is applied to Supabase, unauthenticated Data API clients can read or modify private notes and choose another `user_id`, bypassing the authenticated backend routes. Enabling RLS with no public policies (the backend uses a direct pool) or explicitly revoking Data API privileges would keep notes behind the backend gateway.</comment>
<file context>
@@ -0,0 +1,43 @@
+);
+
+-- Index for fast lookup by user
+CREATE INDEX IF NOT EXISTS idx_notes_user_id ON notes(user_id);
+CREATE INDEX IF NOT EXISTS idx_notes_transcript_id ON notes(transcript_id);
+
</file context>
|
|
||
| const Audiobooks = () => { | ||
| const { data: response, isLoading } = usePlaylists({ playlist_type: 'series' }); | ||
| const { data: response, isLoading } = usePlaylists(); |
There was a problem hiding this comment.
P2: Published collection playlists will now appear in the Audiobook Learning Paths grid because omitting the parameter removes the server-side playlist_type = 'series' filter. Keeping the filter preserves this page's prior series-only behavior, unless collections are intentionally meant to be shown here.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/pages/Audiobooks.tsx, line 16:
<comment>Published `collection` playlists will now appear in the Audiobook Learning Paths grid because omitting the parameter removes the server-side `playlist_type = 'series'` filter. Keeping the filter preserves this page's prior series-only behavior, unless collections are intentionally meant to be shown here.</comment>
<file context>
@@ -13,7 +13,7 @@ import { inferDifficulty, getTopicImageUrl } from "@/components/audiobook/topicU
const Audiobooks = () => {
- const { data: response, isLoading } = usePlaylists({ playlist_type: 'series' });
+ const { data: response, isLoading } = usePlaylists();
const audiobooks = response?.data || [];
</file context>
| */ | ||
| router.post( | ||
| '/progress', | ||
| requireAuth, |
There was a problem hiding this comment.
P2: Progress saves are now authentication-gated, but this endpoint is still documented as public and as accepting a client-supplied user_id, causing documented anonymous/client-session calls to fail with 401. Updating the route/controller contract to require the Bearer token and identify the authenticated user from req.user.id would keep the API documentation aligned with this middleware change.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/src/routes/audiobookRoutes.js, line 85:
<comment>Progress saves are now authentication-gated, but this endpoint is still documented as public and as accepting a client-supplied `user_id`, causing documented anonymous/client-session calls to fail with 401. Updating the route/controller contract to require the Bearer token and identify the authenticated user from `req.user.id` would keep the API documentation aligned with this middleware change.</comment>
<file context>
@@ -79,6 +82,7 @@ router.get(
*/
router.post(
'/progress',
+ requireAuth,
asyncHandler(audiobookController.saveProgress)
);
</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.
P2: The password visibility controls are inaccessible to keyboard and screen-reader users because tabIndex={-1} removes both buttons from the tab order and the icon-only buttons have no accessible name. Keeping them focusable and adding dynamic aria-label values such as “Show password”/“Hide password” would make both toggles operable.
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>The password visibility controls are inaccessible to keyboard and screen-reader users because `tabIndex={-1}` removes both buttons from the tab order and the icon-only buttons have no accessible name. Keeping them focusable and adding dynamic `aria-label` values such as “Show password”/“Hide password” would make both toggles operable.</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>
Comment on lines
+42
to
+46
| <button | ||
| type="button" | ||
| onClick={() => removeTag(tag)} | ||
| className="hover:bg-background/50 rounded-full p-0.5" | ||
| > |
There was a problem hiding this comment.
P2: Screen readers expose each remove control without a meaningful name, so users cannot tell which tag it removes. Adding an accessible label that includes the tag text makes tag removal operable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/components/notes/NoteTagInput.tsx, line 42:
<comment>Screen readers expose each remove control without a meaningful name, so users cannot tell which tag it removes. Adding an accessible label that includes the tag text makes tag removal operable.</comment>
<file context>
@@ -0,0 +1,61 @@
+ className="inline-flex items-center gap-1 px-2 py-0.5 rounded bg-secondary text-secondary-foreground text-xs font-medium"
+ >
+ {tag}
+ <button
+ type="button"
+ onClick={() => removeTag(tag)}
</file context>
Suggested change
| <button | |
| type="button" | |
| onClick={() => removeTag(tag)} | |
| className="hover:bg-background/50 rounded-full p-0.5" | |
| > | |
| <button | |
| type="button" | |
| aria-label={`Remove tag ${tag}`} | |
| onClick={() => removeTag(tag)} | |
| className="hover:bg-background/50 rounded-full p-0.5" | |
| > |
| END; | ||
| $$ LANGUAGE plpgsql; | ||
|
|
||
| CREATE TRIGGER trg_users_updated_at |
There was a problem hiding this comment.
P2: Rerunning this migration fails because trg_users_updated_at already exists, despite the surrounding migration being written as rerunnable. Dropping the trigger first, as the other user-data migrations do, keeps manual migration retries safe.
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 33:
<comment>Rerunning this migration fails because `trg_users_updated_at` already exists, despite the surrounding migration being written as rerunnable. Dropping the trigger first, as the other user-data migrations do, keeps manual migration retries safe.</comment>
<file context>
@@ -0,0 +1,36 @@
+END;
+$$ LANGUAGE plpgsql;
+
+CREATE TRIGGER trg_users_updated_at
+ BEFORE UPDATE ON users
+ FOR EACH ROW
</file context>
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-based user authentication and a full notes feature, backed by new DB tables. Audiobook progress and notes now persist per user and protected routes are enforced.
New Features
/api/v1/auth/{register,login,me}with JWT (requireAuth,optionalAuth) and strictauthLimiter./api/v1/notes(protected) with user-scoped queries; added user-data limiter.x-user-id.AuthContext,LoginModal, andProtectedRoute;services/api.tsauto-attaches Authorization.@xyflow/react; updates to Transcript and Library pages.jsonwebtoken,bcryptjs,@xyflow/react.Migration
JWT_SECRET(and optionalJWT_EXPIRES_IN) in backend env; deploy with the new config.002_auth_users.sql,003_user_data.sql,004_bookmarks_highlights.sql.Authorization: Bearer <token>; thex-user-idheader is no longer used.Written for commit c492e79. Summary will update on new commits.