feat(rls): add owner-write RLS flow and move dashboard RLS config t…#51
Conversation
…o modal - add collection-level RLS config support in project model/api - add public-api auth-context + write authorization middleware - allow PATCH in public data routes - replace inline RLS controls with compact RLS button + settings dialog - clarify ownerField help text in English - update README/docs for RLS publishable write behavior and PATCH endpoint - fix users default RLS ownerField consistency to _id
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (2)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR introduces Row Level Security (RLS) functionality across the platform. It adds per-collection RLS configuration with owner-based access control, new middleware for JWT-based user context resolution and write authorization, refactors user model creation to be schema-flexible, and provides dashboard UI for RLS management. Changes
Sequence DiagramsequenceDiagram
participant Client
participant resolvePublicAuthContext
participant authorizeWriteOperation
participant ProjectModel
participant UserModel
participant Database
Client->>ProjectModel: PATCH /api/data/posts (publishable key + Bearer JWT)
ProjectModel->>resolvePublicAuthContext: Extract JWT from Authorization header
resolvePublicAuthContext->>resolvePublicAuthContext: Verify JWT against jwtSecret
resolvePublicAuthContext->>ProjectModel: Populate req.authUser with userId
ProjectModel->>authorizeWriteOperation: Validate write operation with RLS
alt RLS Enabled for Collection
authorizeWriteOperation->>authorizeWriteOperation: Fetch collection RLS config
alt ownerField in Payload and Matches authUserId
authorizeWriteOperation->>Database: Proceed with update
Database->>Client: 200 Success
else ownerField Mismatch
authorizeWriteOperation->>Client: 403 Forbidden (ownership violation)
end
else RLS Disabled
authorizeWriteOperation->>Client: 403 Forbidden (publishable key cannot write)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements Row Level Security (RLS) for collections, enabling secure write operations via publishable keys when a valid user JWT is provided. The changes include new backend middlewares for JWT verification and write authorization, a new controller for RLS management, and a dashboard UI for configuration. Feedback identifies a critical potential crash in the authorization middleware if authentication is bypassed while RLS is enabled, along with several instances of code duplication and redundant logic in the project controllers and frontend state initialization that should be refactored for better maintainability.
| if (rls.requireAuthForWrite && !req.authUser?.userId) { | ||
| return res.status(401).json({ | ||
| error: 'Authentication required', | ||
| message: 'Provide a valid user Bearer token for write operations.' | ||
| }); | ||
| } | ||
|
|
||
| if ((rls.mode || 'owner-write-only') !== 'owner-write-only') { | ||
| return res.status(403).json({ error: 'Unsupported RLS mode' }); | ||
| } | ||
|
|
||
| const ownerField = rls.ownerField || 'userId'; | ||
| const authUserId = String(req.authUser.userId); |
There was a problem hiding this comment.
This block of code contains a potential server crash. If rls.enabled is true but rls.requireAuthForWrite is false, and no auth token is provided, req.authUser will be null. The code then proceeds to line 39, where String(req.authUser.userId) will throw a TypeError and crash the process.
For an "owner-write-only" policy, the user's identity is essential. Therefore, authentication should be mandatory whenever RLS is enabled for writes.
I suggest refactoring this section to always require authentication when RLS is enabled. This resolves the crash and strengthens the security model.
| if (rls.requireAuthForWrite && !req.authUser?.userId) { | |
| return res.status(401).json({ | |
| error: 'Authentication required', | |
| message: 'Provide a valid user Bearer token for write operations.' | |
| }); | |
| } | |
| if ((rls.mode || 'owner-write-only') !== 'owner-write-only') { | |
| return res.status(403).json({ error: 'Unsupported RLS mode' }); | |
| } | |
| const ownerField = rls.ownerField || 'userId'; | |
| const authUserId = String(req.authUser.userId); | |
| if (!req.authUser?.userId) { | |
| return res.status(401).json({ | |
| error: 'Authentication required', | |
| message: 'Provide a valid user Bearer token for write operations on this RLS-protected collection.' | |
| }); | |
| } | |
| if ((rls.mode || 'owner-write-only') !== 'owner-write-only') { | |
| return res.status(403).json({ error: 'Unsupported RLS mode' }); | |
| } | |
| const ownerField = rls.ownerField || 'userId'; | |
| const authUserId = String(req.authUser.userId); |
| } else if (keys.includes('userId')) { | ||
| ownerField = 'userId'; |
There was a problem hiding this comment.
This else if block is redundant because ownerField is already initialized to 'userId'. You can safely remove these lines to simplify the code and improve readability.
| if (col.name === 'users' && col.model) { | ||
| return { | ||
| ...col, | ||
| model: col.model.filter(m => m.key !== 'password') | ||
| model: col.model.filter(m => m.key !== 'password'), | ||
| rls: col.rls || { | ||
| enabled: false, | ||
| mode: 'owner-write-only', | ||
| ownerField: '_id', | ||
| requireAuthForWrite: true | ||
| } | ||
| }; | ||
| } | ||
| return col; | ||
| return { | ||
| ...col, | ||
| rls: col.rls || { | ||
| enabled: false, | ||
| mode: 'owner-write-only', | ||
| ownerField: 'userId', | ||
| requireAuthForWrite: true | ||
| } | ||
| }; |
There was a problem hiding this comment.
There's significant code duplication here for setting default RLS configurations. This logic is also slightly different from the getDefaultRlsForCollection helper function you've added, which could lead to inconsistencies.
You can refactor this to remove duplication and reuse the helper function, making the code more maintainable.
const sanitizedCol = { ...col };
if (sanitizedCol.name === 'users' && sanitizedCol.model) {
sanitizedCol.model = sanitizedCol.model.filter(m => m.key !== 'password');
}
sanitizedCol.rls = sanitizedCol.rls || getDefaultRlsForCollection(sanitizedCol.name, sanitizedCol.model);
return sanitizedCol;| if (col.name === 'users' && col.model) { | ||
| return { | ||
| ...col, | ||
| model: col.model.filter(m => m.key !== 'password') | ||
| model: col.model.filter(m => m.key !== 'password'), | ||
| rls: col.rls || { | ||
| enabled: false, | ||
| mode: 'owner-write-only', | ||
| ownerField: '_id', | ||
| requireAuthForWrite: true | ||
| } | ||
| }; | ||
| } | ||
| return col; | ||
| return { | ||
| ...col, | ||
| rls: col.rls || { | ||
| enabled: false, | ||
| mode: 'owner-write-only', | ||
| ownerField: 'userId', | ||
| requireAuthForWrite: true | ||
| } | ||
| }; |
There was a problem hiding this comment.
Similar to getSingleProject, there's duplicated code here for setting default RLS configurations. Reusing the getDefaultRlsForCollection helper function would make this code more maintainable and consistent with other parts of the controller.
const sanitizedCol = { ...col };
if (sanitizedCol.name === 'users' && sanitizedCol.model) {
sanitizedCol.model = sanitizedCol.model.filter(m => m.key !== 'password');
}
sanitizedCol.rls = sanitizedCol.rls || getDefaultRlsForCollection(sanitizedCol.name, sanitizedCol.model);
return sanitizedCol;| const withRlsDefaults = (res.data.collections || []).map(c => ({ | ||
| ...c, | ||
| rls: c.rls || { | ||
| enabled: false, | ||
| mode: "owner-write-only", | ||
| ownerField: c.name === 'users' ? '_id' : 'userId', | ||
| requireAuthForWrite: true | ||
| } | ||
| })); | ||
|
|
||
| setProject(res.data); | ||
| setCollections(res.data.collections || []); | ||
| setCollections(withRlsDefaults); | ||
|
|
||
| const queryCollection = searchParams.get("collection"); | ||
| if (queryCollection) { | ||
| const found = res.data.collections.find( | ||
| const found = withRlsDefaults.find( | ||
| (c) => c.name === queryCollection | ||
| ); | ||
| if (found) setActiveCollection(found); | ||
| } else { | ||
| const filtered = res.data.collections.filter(c => c.name !== 'users'); | ||
| const filtered = withRlsDefaults.filter(c => c.name !== 'users'); | ||
| if (filtered.length > 0) { | ||
| setActiveCollection(filtered[0]); | ||
| } else if (res.data.collections.length > 0) { | ||
| setActiveCollection(res.data.collections[0]); | ||
| } else if (withRlsDefaults.length > 0) { | ||
| setActiveCollection(withRlsDefaults[0]); | ||
| } | ||
| } |
There was a problem hiding this comment.
This client-side logic to add default RLS settings to collections is redundant. The backend API (/api/projects/:projectId) already ensures that every collection has an rls object with appropriate defaults.
Relying on the backend as the single source of truth is safer and more maintainable. If the backend's default logic changes (e.g., to prefer ownerId over userId), the frontend would become inconsistent. Please remove this mapping and rely on the data provided by the API.
const collections = res.data.collections || [];
setProject(res.data);
setCollections(collections);
const queryCollection = searchParams.get("collection");
if (queryCollection) {
const found = collections.find(
(c) => c.name === queryCollection
);
if (found) setActiveCollection(found);
} else {
const filtered = collections.filter(c => c.name !== 'users');
if (filtered.length > 0) {
setActiveCollection(filtered[0]);
} else if (collections.length > 0) {
setActiveCollection(collections[0]);
}
}There was a problem hiding this comment.
Pull request overview
Completes the RLS V1 rollout by introducing collection-level RLS configuration in the project model, enforcing owner-write authorization for publishable-key writes in public-api, and adding dashboard UX + docs updates for configuring and understanding RLS.
Changes:
- Added per-collection RLS config (
enabled,mode,ownerField,requireAuthForWrite) and a dashboard API endpoint to update it. - Replaced public-api secret-key-only write gating with an RLS-aware middleware flow and added
PATCHsupport. - Updated the web dashboard Database page UX to configure RLS via a modal, plus documentation updates for the new behavior/endpoints.
Reviewed changes
Copilot reviewed 14 out of 16 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/common/src/models/Project.js | Persists collection-level rls configuration in the Project schema. |
| apps/dashboard-api/src/controllers/project.controller.js | Injects RLS defaults into project responses; adds endpoint to update a collection’s RLS settings. |
| apps/dashboard-api/src/routes/projects.js | Exposes the new PATCH /collections/:collectionName/rls route. |
| apps/public-api/src/routes/data.js | Routes write ops through new auth-context + RLS authorization middleware; adds PATCH. |
| apps/public-api/src/middlewares/resolvePublicAuthContext.js | Resolves req.authUser from Authorization: Bearer <token>. |
| apps/public-api/src/middlewares/authorizeWriteOperation.js | Enforces owner-write-only policy for publishable-key writes. |
| apps/web-dashboard/src/pages/Database.jsx | Adds RLS button + settings modal; PATCHes dashboard API to save per-collection RLS config. |
| apps/web-dashboard/dist/index.html | Updates built asset hash reference. |
| apps/public-api/src/controllers/userAuth.controller.js | Refactors user payload creation + verification-field handling for signup/admin flows. |
| apps/dashboard-api/src/controllers/userAuth.controller.js | Mirrors the user payload + verification-field refactor for dashboard-api. |
| docs/getting-started.md | Updates publishable key guidance to reflect RLS-enabled writes. |
| docs/database.md | Documents RLS write requirements and new PATCH endpoint section. |
| docs/api-reference.md | Adds PATCH endpoint + clarifies 403 semantics for RLS. |
| docs/rls-implementation-roadmap.md | Adds a detailed RLS roadmap/reference doc. |
| README.md | Adds a brief note about RLS-enabled publishable-key writes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const rlsOwnerFieldOptions = (() => { | ||
| if (!activeCollection) return []; | ||
| const modelKeys = (activeCollection.model || []).map((f) => f.key).filter(Boolean); | ||
| return [...new Set(["_id", ...modelKeys, "userId", "ownerId"])]; | ||
| })(); |
There was a problem hiding this comment.
rlsOwnerFieldOptions always includes _id, userId, and ownerId even when those fields aren’t present in the collection schema. Since the backend validates ownerField must exist in the schema (except _id), this UI can offer choices that are guaranteed to fail on save. Consider generating options from the schema keys only (and include _id only for the users collection), so users can’t select an invalid owner field.
| <div className="rls-dialog slide-up" onClick={(e) => e.stopPropagation()}> | ||
| <div className="rls-dialog-header"> | ||
| <div className="rls-dialog-title-wrap"> | ||
| <Shield size={18} /> | ||
| <h3>RLS Settings</h3> |
There was a problem hiding this comment.
The RLS dialog markup is missing basic dialog accessibility semantics (e.g., role="dialog", aria-modal="true", and an aria-labelledby pointing to the title). Without this, screen readers may not announce it as a modal and focus isn’t constrained. Add appropriate ARIA attributes and ensure focus is moved into the dialog on open and restored on close.
| const authHeader = req.header('Authorization'); | ||
| req.authUser = null; | ||
|
|
There was a problem hiding this comment.
resolvePublicAuthContext runs JWT verification whenever an Authorization header is present, even for secret-key requests where RLS is bypassed. To avoid unnecessary JWT work and potential confusion, consider short-circuiting when req.keyRole === 'secret' (or only resolving req.authUser when the key role is publishable).
| const authHeader = req.header('Authorization'); | |
| req.authUser = null; | |
| req.authUser = null; | |
| if (req.keyRole === 'secret') { | |
| return next(); | |
| } | |
| const authHeader = req.header('Authorization'); |
| const ownerField = rls.ownerField || 'userId'; | ||
| const authUserId = String(req.authUser.userId); | ||
| const method = String(req.method || '').toUpperCase(); |
There was a problem hiding this comment.
authorizeWriteOperation computes authUserId = String(req.authUser.userId) unconditionally. If a collection is configured with rls.requireAuthForWrite: false, req.authUser may be null and this will throw, resulting in a 500 instead of a controlled 401/403. Either enforce auth for owner-write-only mode regardless of requireAuthForWrite, or guard this path so anonymous writes don’t crash the middleware.
| rls: col.rls || { | ||
| enabled: false, | ||
| mode: 'owner-write-only', | ||
| ownerField: 'userId', | ||
| requireAuthForWrite: true |
There was a problem hiding this comment.
getSingleProject injects a default rls.ownerField: 'userId' for non-users collections regardless of the collection schema. If the schema doesn’t contain userId (e.g., it uses ownerId), the dashboard will show an invalid default and updateCollectionRls will reject saves with “ownerField not found in collection schema”. Use getDefaultRlsForCollection(col.name, col.model) (already defined) or derive the default from col.model here so the default ownerField is always schema-valid.
| rls: col.rls || { | ||
| enabled: false, | ||
| mode: 'owner-write-only', | ||
| ownerField: 'userId', | ||
| requireAuthForWrite: true |
There was a problem hiding this comment.
Same issue as getSingleProject: toggleAuth returns rls.ownerField: 'userId' as the default for all non-users collections, even when that field isn’t present in the collection schema. This can surface an invalid default in the dashboard and cause the RLS save call to fail. Reuse getDefaultRlsForCollection(col.name, col.model) (or schema-derived defaults) when populating col.rls here.
|
|
||
| const modelKeys = (collection.model || []).map(f => f.key); | ||
| const nextOwnerField = ownerField || collection?.rls?.ownerField || 'userId'; | ||
|
|
There was a problem hiding this comment.
updateCollectionRls allows ownerField: '_id' for any collection (it bypasses the schema-key check). For non-users collections, selecting _id will effectively block inserts (public-api middleware explicitly denies POST when ownerField === '_id') and likely make updates/deletes impossible unless document _id equals the user id. Consider restricting _id to the users collection (or returning a clear 400 explaining why _id isn’t a valid ownerField for general collections).
| // Restrict use of '_id' as ownerField to the 'users' collection only. | |
| if (nextOwnerField === '_id' && collection.name !== 'users') { | |
| return res.status(400).json({ | |
| error: "Invalid owner field", | |
| message: "ownerField '_id' is only allowed for the 'users' collection" | |
| }); | |
| } |
| rls: c.rls || { | ||
| enabled: false, | ||
| mode: "owner-write-only", | ||
| ownerField: c.name === 'users' ? '_id' : 'userId', | ||
| requireAuthForWrite: true |
There was a problem hiding this comment.
Dashboard-side RLS defaults hardcode ownerField to 'userId' for every non-users collection when c.rls is missing. If the collection schema doesn’t include userId, this default is invalid and will cause the save request to fail. Derive the default ownerField from c.model (e.g., prefer userId, else ownerId, else force the user to choose) so the initial value is always valid for the schema.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (9)
apps/web-dashboard/src/pages/Database.jsx (4)
768-774: Minor: Raw backticks in JSX render as literal text.The help text uses backticks that render as literal characters rather than styled code. Consider using
<code>elements for better presentation.📝 Suggested fix
<p className="rls-help-text"> - `userId` means the field in each document that stores the logged-in user's id. - Example: `posts.userId` or `orders.ownerId`. For the users collection, `_id` is usually the correct owner field. + <code>userId</code> means the field in each document that stores the logged-in user's id. + Example: <code>posts.userId</code> or <code>orders.ownerId</code>. For the users collection, <code>_id</code> is usually the correct owner field. </p>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web-dashboard/src/pages/Database.jsx` around lines 768 - 774, The help text in Database.jsx (the JSX paragraph elements with className "rls-help-text") contains raw backticks that render literally; replace inline backticks with semantic <code> elements around identifiers like userId, posts.userId, orders.ownerId and _id, and similarly wrap Authorization: Bearer <user_jwt> in a <code> element so the text renders as code/monospace (update the two <p className="rls-help-text"> blocks accordingly).
173-181: Fallback logic differs from initial load defaults.The effect derives
fallbackOwnerby checkingmodelKeys.includes('userId')then'ownerId', but the initial collection mapping at lines 98-106 always defaults to'userId'for non-users collections. This inconsistency could cause the UI to show different values depending on whether the data came from initial load vs. subsequent activeCollection changes.♻️ Suggested approach
Extract the fallback derivation into a shared helper to ensure consistency:
const deriveDefaultOwnerField = (collectionName, modelKeys) => { if (collectionName === 'users') return '_id'; if (modelKeys.includes('userId')) return 'userId'; if (modelKeys.includes('ownerId')) return 'ownerId'; return 'userId'; };Then use this helper in both the initial load mapping and the effect.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web-dashboard/src/pages/Database.jsx` around lines 173 - 181, The fallback owner-field logic is inconsistent between the initial collection mapping and the useEffect; extract the logic into a single helper (e.g., deriveDefaultOwnerField(collectionName, modelKeys)) that returns '_id' for collectionName === 'users', 'userId' if modelKeys includes 'userId', 'ownerId' if modelKeys includes 'ownerId', otherwise 'userId', then replace the inline fallback computation in the useEffect and the initial collection mapping to call deriveDefaultOwnerField and use its result when calling setRlsOwnerField or when building the initial collection objects (referencing activeCollection, setRlsOwnerField, and the initial mapping code path).
98-106: Potential inconsistency in default ownerField derivation between frontend and backend.The frontend defaults
ownerFieldto'userId'for non-users collections without checking if the schema containsuserIdorownerId. However, the backend helpergetDefaultRlsForCollection(inproject.controller.jslines 28-47) checks if schema keys include'userId'first, then'ownerId', then falls back to'userId'.This could lead to a mismatch where the frontend shows
'userId'but the backend persisted'ownerId'if that was present in the schema. Consider aligning the logic or relying solely on the server response.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web-dashboard/src/pages/Database.jsx` around lines 98 - 106, The frontend's withRlsDefaults logic sets ownerField to 'userId' unconditionally for non-'users' collections which can diverge from the backend getDefaultRlsForCollection behavior; update withRlsDefaults to mirror the backend: inspect the collection schema (e.g., c.schema or c.fields) and set ownerField to 'userId' if schema contains 'userId', otherwise 'ownerId' if present, otherwise fallback to 'userId', or alternatively stop deriving and use the server-provided RLS defaults directly from the response to ensure consistency with getDefaultRlsForCollection in project.controller.js.
727-794: Missing keyboard trap and focus management for modal accessibility.The RLS dialog lacks proper focus management. When opened, focus should move to the dialog, and pressing Escape should close it. Currently, only clicking the overlay or buttons closes it.
♿ Suggested accessibility improvements
+import { useEffect, useRef } from "react"; // Inside the component, add: +const rlsDialogRef = useRef(null); + +useEffect(() => { + if (isRlsDialogOpen && rlsDialogRef.current) { + rlsDialogRef.current.focus(); + } +}, [isRlsDialogOpen]); // In the dialog div: -<div className="rls-dialog slide-up" onClick={(e) => e.stopPropagation()}> +<div + ref={rlsDialogRef} + className="rls-dialog slide-up" + onClick={(e) => e.stopPropagation()} + onKeyDown={(e) => { if (e.key === 'Escape') setIsRlsDialogOpen(false); }} + tabIndex={-1} + role="dialog" + aria-modal="true" + aria-labelledby="rls-dialog-title" +>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web-dashboard/src/pages/Database.jsx` around lines 727 - 794, The RLS modal (rendered when isRlsDialogOpen) needs proper focus management and Escape handling: when isRlsDialogOpen transitions true, move focus into the dialog (use a ref for the dialog container and call focus on mount), trap tab focus inside the dialog (ensure first/last tabbable wrap or use a small focus-trap utility) and restore focus to the element that opened the dialog on close via setIsRlsDialogOpen(false); also add a keydown handler on the dialog that listens for Escape to call setIsRlsDialogOpen(false) and prevent propagation, and ensure all interactive elements (Close button, Cancel, Save RLS) are reachable by keyboard; use the existing dialog container element (class "rls-dialog") and state functions (isRlsDialogOpen, setIsRlsDialogOpen, handleSaveRls) to implement these changes.docs/rls-implementation-roadmap.md (1)
180-182: Minor: Add period after "etc" for American English style.📝 Fix
- Support column-level restrictions: - - Publishable writes cannot modify protected fields (`role`, `plan`, `isAdmin`, etc). + - Publishable writes cannot modify protected fields (`role`, `plan`, `isAdmin`, etc.).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/rls-implementation-roadmap.md` around lines 180 - 182, Update the markdown bullet "Support column-level restrictions" so the phrase "`role`, `plan`, `isAdmin`, etc" ends with a period; locate the line containing that bullet (the text "Publishable writes cannot modify protected fields (`role`, `plan`, `isAdmin`, etc).") and ensure there is a period after "etc" to match American English style.apps/public-api/src/middlewares/authorizeWriteOperation.js (2)
72-74: Performance consideration: Document fetch on every write.Each PUT/PATCH/DELETE requires a database query to verify ownership. For high-frequency updates, this adds latency.
Consider documenting this as a known V1 limitation in the roadmap. Future versions could explore caching strategies or optimistic checks for frequently updated documents.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/public-api/src/middlewares/authorizeWriteOperation.js` around lines 72 - 74, The middleware currently performs a Model.findById(id).select(ownerField).lean() on every write (via getConnection and getCompiledModel), which adds latency for high-frequency PUT/PATCH/DELETE operations; add a short note in the project roadmap/docs (and a TODO comment in authorizeWriteOperation.js near the getConnection/getCompiledModel/Model.findById usage) marking this as a known V1 limitation and propose future mitigations (caching owner checks, optimistic concurrency, or batched ownership validation) so reviewers know this is intentional and slated for improvement.
85-87: Edge case:ownerFieldimmutability check doesn't apply to DELETE.The check for
ownerFieldtampering inreq.bodyis inside thePUT/PATCH/DELETEblock, but DELETE requests typically don't have a body. While not a bug (the condition just won't trigger for DELETE), consider restructuring for clarity:♻️ Clearer structure
if (method === 'PUT' || method === 'PATCH' || method === 'DELETE') { // ... validation and doc fetch ... - if (ownerField === '_id') { + const isOwner = ownerField === '_id' + ? String(doc._id) === authUserId + : String(doc[ownerField]) === authUserId; + + if (!isOwner) { + return res.status(403).json({ error: 'RLS owner mismatch', message: 'You can only modify your own document.' }); + } + + // Only check ownerField immutability on updates with body + if ((method === 'PUT' || method === 'PATCH') && ownerField !== '_id') { + if (req.body && Object.prototype.hasOwnProperty.call(req.body, ownerField) && String(req.body[ownerField]) !== String(doc[ownerField])) { + return res.status(403).json({ error: 'Owner field immutable', message: `${ownerField} cannot be changed under RLS.` }); + } - if (String(doc._id) !== authUserId) { - return res.status(403).json({ error: 'RLS owner mismatch', message: 'You can only modify your own document.' }); - } - } else { - if (req.body && Object.prototype.hasOwnProperty.call(req.body, ownerField) && String(req.body[ownerField]) !== String(doc[ownerField])) { - return res.status(403).json({ error: 'Owner field immutable', message: `${ownerField} cannot be changed under RLS.` }); - } - - if (doc[ownerField] === undefined || doc[ownerField] === null || String(doc[ownerField]) !== authUserId) { - return res.status(403).json({ error: 'RLS owner mismatch', message: 'You can only modify your own document.' }); - } } return next(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/public-api/src/middlewares/authorizeWriteOperation.js` around lines 85 - 87, The ownerField immutability check currently lives inside the PUT/PATCH/DELETE branch and should be applied only for methods that can carry a body; update authorizeWriteOperation so the ownerField tampering check (the condition using req.body, ownerField and doc[ownerField]) runs only for PUT and PATCH (or when req.method !== 'DELETE'), or alternatively gate it explicitly by checking req.method === 'PUT' || req.method === 'PATCH' before evaluating req.body; this clarifies intent and avoids implying DELETE should be subject to a body-based check while keeping the same protection for updates.apps/dashboard-api/src/controllers/project.controller.js (2)
968-990: Same code duplication in toggleAuth response sanitization.The
toggleAuthfunction also has inline RLS defaults rather than using the helper function. Apply the same refactor as suggested forgetSingleProject.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard-api/src/controllers/project.controller.js` around lines 968 - 990, The toggleAuth response still duplicates inline RLS/defaults and model sanitization; replace that block inside toggleAuth with the same helper used by getSingleProject (the project collections sanitization helper used in getSingleProject) so you don't recreate the default rls object or the users model password filter inline. Specifically, refactor the mapping inside toggleAuth to call the shared helper (the function getSingleProject uses for applying default rls and stripping user password fields) and ensure toggleAuth returns the sanitized projectObj rather than building RLS/model defaults in-place.
143-166: Code duplication: RLS defaults are inline here instead of using the helper.The
getSingleProjectfunction constructs RLS defaults inline rather than usinggetDefaultRlsForCollection. This creates maintenance burden and potential inconsistency if defaults change.♻️ Suggested refactor
if (projectObj.collections && Array.isArray(projectObj.collections)) { projectObj.collections = projectObj.collections.map(col => { + const defaultRls = getDefaultRlsForCollection(col.name, col.model); if (col.name === 'users' && col.model) { return { ...col, model: col.model.filter(m => m.key !== 'password'), - rls: col.rls || { - enabled: false, - mode: 'owner-write-only', - ownerField: '_id', - requireAuthForWrite: true - } + rls: col.rls || defaultRls }; } - return { - ...col, - rls: col.rls || { - enabled: false, - mode: 'owner-write-only', - ownerField: 'userId', - requireAuthForWrite: true - } - }; + return { ...col, rls: col.rls || defaultRls }; }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard-api/src/controllers/project.controller.js` around lines 143 - 166, In getSingleProject, remove the duplicated inline RLS default objects inside the projectObj.collections mapping and instead call the existing helper getDefaultRlsForCollection to supply defaults; specifically, in the mapping for each col replace rls: col.rls || { ...inline... } with rls: col.rls || getDefaultRlsForCollection(col.name), and for the special-case when col.name === 'users' keep the model filtering (col.model.filter(m => m.key !== 'password')) but still set rls via getDefaultRlsForCollection('users') (or merge/override only specific fields if needed) so all RLS defaults come from the single helper.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/public-api/src/controllers/userAuth.controller.js`:
- Around line 46-48: The fallback username generation in the payload branch
using payload.name || email.split('@')[0] can produce values outside the API's
3-50 character rule and isn't propagated to the value returned by
createAdminUser(); update the logic so the generated username is normalized to
the same validation rules (trim, enforce min length 3 and max length 50, and
sanitize/remove invalid chars per the API's username policy), set
payload.username to that canonicalized value, and ensure createAdminUser()
returns this same canonical username variable (not the original raw value) so
stored data and the response body remain consistent with the profile endpoint
checks; look for hasRequiredField, payload.username usage, and the
createAdminUser() return path to apply the fix.
- Around line 17-23: getVerificationField currently defaults to 'emailVerified'
even when usersColConfig.model (the declared field list) doesn't include any
supported verification key; change getVerificationField to return null (or
undefined) instead of 'emailVerified' when no matching keys are found so callers
don't act on an undeclared field. Update the function (getVerificationField) to
compute modelKeys from usersColConfig.model and if none of 'emailVerified',
'isVerified', or 'isverified' are present return null/undefined; then audit
callers (signup/admin-create/verifyEmail) to handle a null/undefined
verificationField gracefully (e.g., skip verification logic or surface an
explicit error).
In `@apps/public-api/src/middlewares/authorizeWriteOperation.js`:
- Around line 27-39: The code currently assumes req.authUser exists when setting
authUserId which can throw if no token is provided; modify the assignment of
authUserId in authorizeWriteOperation middleware to safely read req.authUser
(use optional chaining or a conditional) so authUserId becomes undefined/null
when req.authUser is missing, and ensure downstream logic that uses ownerField
and authUserId (rls.ownerField, authUserId) handles the absent user case (e.g.,
treat as anonymous or reject based on rls.requireAuthForWrite). Specifically,
update the use of req.authUser.userId to a guarded read (req.authUser?.userId)
and adjust any subsequent checks that assume authUserId is always a string.
In `@apps/public-api/src/middlewares/resolvePublicAuthContext.js`:
- Around line 4-12: Authorization header check currently requires 'Bearer ' with
exact casing; update the middleware to accept the scheme case-insensitively by
matching the header against a case-insensitive pattern (e.g., use a regex with
the i flag) to extract the token. Locate the authHeader handling in
resolvePublicAuthContext.js (variables/authHeader, token) and replace the
startsWith/slice logic with a case-insensitive match like /^\s*Bearer\s+(.+)$/i,
then set req.authUser processing to use the captured token only when the regex
matches; otherwise call next().
In `@docs/database.md`:
- Around line 18-24: Update the wording to make clear that the owner check under
RLS applies only to publishable-key writes: state that when using a publishable
key (x-api-key: pk_live_...) plus an Authorization: Bearer <user_jwt>, writes
are restricted to documents owned by the authenticated user based on the
configured ownerField; explicitly note that requests authenticated with a secret
key remain administrative and are not subject to the ownerField restriction so
secret-key behavior is preserved.
In `@packages/common/src/models/Project.js`:
- Around line 29-37: The rls.ownerField value is stored as an arbitrary string
which can alter query semantics; update the Project model to validate and
normalize ownerField before persistence by enforcing a safe field-name pattern
(e.g., allow only alphanumeric/underscore, no operator characters) and rejecting
or sanitizing invalid values, and implement logic in the Project schema
save/update path (e.g., a pre-save/pre-validate hook on the Project model) to
map the special-case collection ownerField value to '_id' where appropriate
(apply the users collection override at persistence time rather than only in
response shaping) so stored ownerField is always a validated, normalized
identifier used safely when building ownership filters.
---
Nitpick comments:
In `@apps/dashboard-api/src/controllers/project.controller.js`:
- Around line 968-990: The toggleAuth response still duplicates inline
RLS/defaults and model sanitization; replace that block inside toggleAuth with
the same helper used by getSingleProject (the project collections sanitization
helper used in getSingleProject) so you don't recreate the default rls object or
the users model password filter inline. Specifically, refactor the mapping
inside toggleAuth to call the shared helper (the function getSingleProject uses
for applying default rls and stripping user password fields) and ensure
toggleAuth returns the sanitized projectObj rather than building RLS/model
defaults in-place.
- Around line 143-166: In getSingleProject, remove the duplicated inline RLS
default objects inside the projectObj.collections mapping and instead call the
existing helper getDefaultRlsForCollection to supply defaults; specifically, in
the mapping for each col replace rls: col.rls || { ...inline... } with rls:
col.rls || getDefaultRlsForCollection(col.name), and for the special-case when
col.name === 'users' keep the model filtering (col.model.filter(m => m.key !==
'password')) but still set rls via getDefaultRlsForCollection('users') (or
merge/override only specific fields if needed) so all RLS defaults come from the
single helper.
In `@apps/public-api/src/middlewares/authorizeWriteOperation.js`:
- Around line 72-74: The middleware currently performs a
Model.findById(id).select(ownerField).lean() on every write (via getConnection
and getCompiledModel), which adds latency for high-frequency PUT/PATCH/DELETE
operations; add a short note in the project roadmap/docs (and a TODO comment in
authorizeWriteOperation.js near the
getConnection/getCompiledModel/Model.findById usage) marking this as a known V1
limitation and propose future mitigations (caching owner checks, optimistic
concurrency, or batched ownership validation) so reviewers know this is
intentional and slated for improvement.
- Around line 85-87: The ownerField immutability check currently lives inside
the PUT/PATCH/DELETE branch and should be applied only for methods that can
carry a body; update authorizeWriteOperation so the ownerField tampering check
(the condition using req.body, ownerField and doc[ownerField]) runs only for PUT
and PATCH (or when req.method !== 'DELETE'), or alternatively gate it explicitly
by checking req.method === 'PUT' || req.method === 'PATCH' before evaluating
req.body; this clarifies intent and avoids implying DELETE should be subject to
a body-based check while keeping the same protection for updates.
In `@apps/web-dashboard/src/pages/Database.jsx`:
- Around line 768-774: The help text in Database.jsx (the JSX paragraph elements
with className "rls-help-text") contains raw backticks that render literally;
replace inline backticks with semantic <code> elements around identifiers like
userId, posts.userId, orders.ownerId and _id, and similarly wrap Authorization:
Bearer <user_jwt> in a <code> element so the text renders as
code/monospace (update the two <p className="rls-help-text"> blocks
accordingly).
- Around line 173-181: The fallback owner-field logic is inconsistent between
the initial collection mapping and the useEffect; extract the logic into a
single helper (e.g., deriveDefaultOwnerField(collectionName, modelKeys)) that
returns '_id' for collectionName === 'users', 'userId' if modelKeys includes
'userId', 'ownerId' if modelKeys includes 'ownerId', otherwise 'userId', then
replace the inline fallback computation in the useEffect and the initial
collection mapping to call deriveDefaultOwnerField and use its result when
calling setRlsOwnerField or when building the initial collection objects
(referencing activeCollection, setRlsOwnerField, and the initial mapping code
path).
- Around line 98-106: The frontend's withRlsDefaults logic sets ownerField to
'userId' unconditionally for non-'users' collections which can diverge from the
backend getDefaultRlsForCollection behavior; update withRlsDefaults to mirror
the backend: inspect the collection schema (e.g., c.schema or c.fields) and set
ownerField to 'userId' if schema contains 'userId', otherwise 'ownerId' if
present, otherwise fallback to 'userId', or alternatively stop deriving and use
the server-provided RLS defaults directly from the response to ensure
consistency with getDefaultRlsForCollection in project.controller.js.
- Around line 727-794: The RLS modal (rendered when isRlsDialogOpen) needs
proper focus management and Escape handling: when isRlsDialogOpen transitions
true, move focus into the dialog (use a ref for the dialog container and call
focus on mount), trap tab focus inside the dialog (ensure first/last tabbable
wrap or use a small focus-trap utility) and restore focus to the element that
opened the dialog on close via setIsRlsDialogOpen(false); also add a keydown
handler on the dialog that listens for Escape to call setIsRlsDialogOpen(false)
and prevent propagation, and ensure all interactive elements (Close button,
Cancel, Save RLS) are reachable by keyboard; use the existing dialog container
element (class "rls-dialog") and state functions (isRlsDialogOpen,
setIsRlsDialogOpen, handleSaveRls) to implement these changes.
In `@docs/rls-implementation-roadmap.md`:
- Around line 180-182: Update the markdown bullet "Support column-level
restrictions" so the phrase "`role`, `plan`, `isAdmin`, etc" ends with a period;
locate the line containing that bullet (the text "Publishable writes cannot
modify protected fields (`role`, `plan`, `isAdmin`, etc).") and ensure there is
a period after "etc" to match American English style.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c3487b1d-b602-4f99-83c5-a4461cf62b35
⛔ Files ignored due to path filters (2)
apps/web-dashboard/dist/assets/index-Bu_-Dcm1.jsis excluded by!**/dist/**apps/web-dashboard/dist/index.htmlis excluded by!**/dist/**
📒 Files selected for processing (14)
README.mdapps/dashboard-api/src/controllers/project.controller.jsapps/dashboard-api/src/controllers/userAuth.controller.jsapps/dashboard-api/src/routes/projects.jsapps/public-api/src/controllers/userAuth.controller.jsapps/public-api/src/middlewares/authorizeWriteOperation.jsapps/public-api/src/middlewares/resolvePublicAuthContext.jsapps/public-api/src/routes/data.jsapps/web-dashboard/src/pages/Database.jsxdocs/api-reference.mddocs/database.mddocs/getting-started.mddocs/rls-implementation-roadmap.mdpackages/common/src/models/Project.js
| const getVerificationField = (usersColConfig) => { | ||
| const modelKeys = (usersColConfig?.model || []).map((f) => f?.key); | ||
| if (modelKeys.includes('emailVerified')) return 'emailVerified'; | ||
| if (modelKeys.includes('isVerified')) return 'isVerified'; | ||
| if (modelKeys.includes('isverified')) return 'isverified'; | ||
| return 'emailVerified'; | ||
| }; |
There was a problem hiding this comment.
Don't fall back to an undeclared verification field.
packages/common/src/models/Project.js, Lines 26-39 makes usersColConfig.model the declared field list. If none of the supported keys exist, returning 'emailVerified' here lets signup/admin-create/verifyEmail target a path the collection never defined; on strict models that can turn email verification into a false-positive success.
Proposed fix
const getVerificationField = (usersColConfig) => {
- const modelKeys = (usersColConfig?.model || []).map((f) => f?.key);
- if (modelKeys.includes('emailVerified')) return 'emailVerified';
- if (modelKeys.includes('isVerified')) return 'isVerified';
- if (modelKeys.includes('isverified')) return 'isverified';
- return 'emailVerified';
+ const verificationField = (usersColConfig?.model || []).find(
+ (f) =>
+ ['emailVerified', 'isVerified', 'isverified'].includes(f?.key) &&
+ f?.type === 'Boolean'
+ );
+ if (!verificationField) {
+ throw new Error("Users collection must define a boolean verification field.");
+ }
+ return verificationField.key;
};📝 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.
| const getVerificationField = (usersColConfig) => { | |
| const modelKeys = (usersColConfig?.model || []).map((f) => f?.key); | |
| if (modelKeys.includes('emailVerified')) return 'emailVerified'; | |
| if (modelKeys.includes('isVerified')) return 'isVerified'; | |
| if (modelKeys.includes('isverified')) return 'isverified'; | |
| return 'emailVerified'; | |
| }; | |
| const getVerificationField = (usersColConfig) => { | |
| const verificationField = (usersColConfig?.model || []).find( | |
| (f) => | |
| ['emailVerified', 'isVerified', 'isverified'].includes(f?.key) && | |
| f?.type === 'Boolean' | |
| ); | |
| if (!verificationField) { | |
| throw new Error("Users collection must define a boolean verification field."); | |
| } | |
| return verificationField.key; | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/public-api/src/controllers/userAuth.controller.js` around lines 17 - 23,
getVerificationField currently defaults to 'emailVerified' even when
usersColConfig.model (the declared field list) doesn't include any supported
verification key; change getVerificationField to return null (or undefined)
instead of 'emailVerified' when no matching keys are found so callers don't act
on an undeclared field. Update the function (getVerificationField) to compute
modelKeys from usersColConfig.model and if none of 'emailVerified',
'isVerified', or 'isverified' are present return null/undefined; then audit
callers (signup/admin-create/verifyEmail) to handle a null/undefined
verificationField gracefully (e.g., skip verification logic or surface an
explicit error).
| if (hasRequiredField(usersColConfig, 'username') && (payload.username === undefined || payload.username === null || payload.username === '')) { | ||
| payload.username = payload.name || email.split('@')[0]; | ||
| } |
There was a problem hiding this comment.
Keep generated usernames consistent with the rest of the API.
This fallback can produce values outside the 3-50 character rule enforced in Lines 389-393 (a@... -> a), and createAdminUser() still returns the raw username variable on Line 220 instead of the generated one. That leaves you with stored data the profile endpoint would later reject and a response body that can disagree with the document.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/public-api/src/controllers/userAuth.controller.js` around lines 46 - 48,
The fallback username generation in the payload branch using payload.name ||
email.split('@')[0] can produce values outside the API's 3-50 character rule and
isn't propagated to the value returned by createAdminUser(); update the logic so
the generated username is normalized to the same validation rules (trim, enforce
min length 3 and max length 50, and sanitize/remove invalid chars per the API's
username policy), set payload.username to that canonicalized value, and ensure
createAdminUser() returns this same canonical username variable (not the
original raw value) so stored data and the response body remain consistent with
the profile endpoint checks; look for hasRequiredField, payload.username usage,
and the createAdminUser() return path to apply the fix.
| if (rls.requireAuthForWrite && !req.authUser?.userId) { | ||
| return res.status(401).json({ | ||
| error: 'Authentication required', | ||
| message: 'Provide a valid user Bearer token for write operations.' | ||
| }); | ||
| } | ||
|
|
||
| if ((rls.mode || 'owner-write-only') !== 'owner-write-only') { | ||
| return res.status(403).json({ error: 'Unsupported RLS mode' }); | ||
| } | ||
|
|
||
| const ownerField = rls.ownerField || 'userId'; | ||
| const authUserId = String(req.authUser.userId); |
There was a problem hiding this comment.
Critical: Potential crash when requireAuthForWrite is false but RLS is enabled.
If rls.requireAuthForWrite is false, the auth check on line 27 is skipped, but line 39 still accesses req.authUser.userId unconditionally. This will throw a TypeError if no Bearer token was provided.
🔧 Proposed fix
if (rls.requireAuthForWrite && !req.authUser?.userId) {
return res.status(401).json({
error: 'Authentication required',
message: 'Provide a valid user Bearer token for write operations.'
});
}
+// Even if requireAuthForWrite is false, owner-write-only mode needs a userId
+if (!req.authUser?.userId) {
+ return res.status(401).json({
+ error: 'Authentication required',
+ message: 'Owner-write-only mode requires a valid user Bearer token.'
+ });
+}
+
if ((rls.mode || 'owner-write-only') !== 'owner-write-only') {
return res.status(403).json({ error: 'Unsupported RLS mode' });
}
const ownerField = rls.ownerField || 'userId';
const authUserId = String(req.authUser.userId);Alternatively, if the intent is to allow anonymous writes when requireAuthForWrite: false, then the entire owner-write-only logic should be conditional, which would be a design change.
📝 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.
| if (rls.requireAuthForWrite && !req.authUser?.userId) { | |
| return res.status(401).json({ | |
| error: 'Authentication required', | |
| message: 'Provide a valid user Bearer token for write operations.' | |
| }); | |
| } | |
| if ((rls.mode || 'owner-write-only') !== 'owner-write-only') { | |
| return res.status(403).json({ error: 'Unsupported RLS mode' }); | |
| } | |
| const ownerField = rls.ownerField || 'userId'; | |
| const authUserId = String(req.authUser.userId); | |
| if (rls.requireAuthForWrite && !req.authUser?.userId) { | |
| return res.status(401).json({ | |
| error: 'Authentication required', | |
| message: 'Provide a valid user Bearer token for write operations.' | |
| }); | |
| } | |
| // Even if requireAuthForWrite is false, owner-write-only mode needs a userId | |
| if (!req.authUser?.userId) { | |
| return res.status(401).json({ | |
| error: 'Authentication required', | |
| message: 'Owner-write-only mode requires a valid user Bearer token.' | |
| }); | |
| } | |
| if ((rls.mode || 'owner-write-only') !== 'owner-write-only') { | |
| return res.status(403).json({ error: 'Unsupported RLS mode' }); | |
| } | |
| const ownerField = rls.ownerField || 'userId'; | |
| const authUserId = String(req.authUser.userId); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/public-api/src/middlewares/authorizeWriteOperation.js` around lines 27 -
39, The code currently assumes req.authUser exists when setting authUserId which
can throw if no token is provided; modify the assignment of authUserId in
authorizeWriteOperation middleware to safely read req.authUser (use optional
chaining or a conditional) so authUserId becomes undefined/null when
req.authUser is missing, and ensure downstream logic that uses ownerField and
authUserId (rls.ownerField, authUserId) handles the absent user case (e.g.,
treat as anonymous or reject based on rls.requireAuthForWrite). Specifically,
update the use of req.authUser.userId to a guarded read (req.authUser?.userId)
and adjust any subsequent checks that assume authUserId is always a string.
| const authHeader = req.header('Authorization'); | ||
| req.authUser = null; | ||
|
|
||
| if (!authHeader || !authHeader.startsWith('Bearer ')) { | ||
| return next(); | ||
| } | ||
|
|
||
| const token = authHeader.slice(7).trim(); | ||
| if (!token) return next(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/public-api/src/middlewares/resolvePublicAuthContext.js | head -30Repository: yash-pouranik/urBackend
Length of output: 834
🌐 Web query:
HTTP Authorization header scheme case sensitivity RFC
💡 Result:
The HTTP Authorization header scheme (auth-scheme) is case-insensitive according to the relevant RFCs. RFC 7235 Section 2.1 (HTTP/1.1 Authentication) explicitly states: "It uses a case-insensitive token as a means to identify the authentication scheme". This is reaffirmed in RFC 9110 (HTTP Semantics, June 2022), Section 11.1: "It uses a case-insensitive token to identify the authentication scheme: auth-scheme = token". Parameter names within authentication are also matched case-insensitively. HTTP header field names themselves are case-insensitive per RFC 7230/9112. This applies to both WWW-Authenticate challenges and Authorization credentials. Specific schemes like Basic (RFC 7617) and Bearer note case-insensitive matching for scheme and parameters. Despite this, some implementations incorrectly treat it as case-sensitive, leading to interoperability issues.
Citations:
- 1: https://www.rfc-editor.org/rfc/rfc7235
- 2: https://datatracker.ietf.org/doc/html/rfc7235
- 3: https://rfcinfo.com/rfc-9110/11
- 4: https://auth.ietf.org/doc/html/rfc7235
- 5: https://rfc-editor.org/rfc/rfc7235.html
- 6: https://datatracker.ietf.org/doc/html/rfc7617
- 7: https://httpwg.org/specs/rfc7235.html
Accept case-insensitive Bearer schemes.
Authorization schemes are case-insensitive per RFC 7235/9110, so bearer <token> or other case variations currently fall through as anonymous and can break otherwise valid authentication flows depending on the client.
♻️ Proposed fix
module.exports = (req, res, next) => {
const authHeader = req.header('Authorization');
req.authUser = null;
- if (!authHeader || !authHeader.startsWith('Bearer ')) {
+ if (!authHeader) {
return next();
}
- const token = authHeader.slice(7).trim();
- if (!token) return next();
+ const [scheme, token = ''] = authHeader.trim().split(/\s+/, 2);
+ if (!/^Bearer$/i.test(scheme) || !token) {
+ return next();
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/public-api/src/middlewares/resolvePublicAuthContext.js` around lines 4 -
12, Authorization header check currently requires 'Bearer ' with exact casing;
update the middleware to accept the scheme case-insensitively by matching the
header against a case-insensitive pattern (e.g., use a regex with the i flag) to
extract the token. Locate the authHeader handling in resolvePublicAuthContext.js
(variables/authHeader, token) and replace the startsWith/slice logic with a
case-insensitive match like /^\s*Bearer\s+(.+)$/i, then set req.authUser
processing to use the captured token only when the regex matches; otherwise call
next().
| If you enable **RLS (owner-write-only)** for a collection from the dashboard, publishable-key writes are also allowed but must include a valid user token: | ||
|
|
||
| - `x-api-key: pk_live_...` | ||
| - `Authorization: Bearer <user_jwt>` | ||
|
|
||
| Under RLS, writes are permitted only for documents owned by the authenticated user (based on configured `ownerField`). | ||
|
|
There was a problem hiding this comment.
Scope the owner check to the publishable-key exception path.
Lines 18-24 currently read as if all writes become owner-restricted once RLS is enabled, which conflicts with the PR goal of preserving secret-key admin behavior. Please clarify that this owner check applies to publishable-key writes, while secret-key writes remain administrative.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/database.md` around lines 18 - 24, Update the wording to make clear that
the owner check under RLS applies only to publishable-key writes: state that
when using a publishable key (x-api-key: pk_live_...) plus an Authorization:
Bearer <user_jwt>, writes are restricted to documents owned by the authenticated
user based on the configured ownerField; explicitly note that requests
authenticated with a secret key remain administrative and are not subject to the
ownerField restriction so secret-key behavior is preserved.
| rls: { | ||
| enabled: { type: Boolean, default: false }, | ||
| mode: { | ||
| type: String, | ||
| enum: ['owner-write-only'], | ||
| default: 'owner-write-only' | ||
| }, | ||
| ownerField: { type: String, default: 'userId' }, | ||
| requireAuthForWrite: { type: Boolean, default: true } |
There was a problem hiding this comment.
Normalize and validate ownerField before storing it.
ownerField is part of the RLS security boundary, because it is later reused to build the ownership filter. Leaving it as an arbitrary string here means operator-like keys can change query semantics, and the generic 'userId' default still doesn't cover the users collection special case called out in this PR. Please enforce a safe field name and apply the _id override at persistence time instead of only in response shaping.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/common/src/models/Project.js` around lines 29 - 37, The
rls.ownerField value is stored as an arbitrary string which can alter query
semantics; update the Project model to validate and normalize ownerField before
persistence by enforcing a safe field-name pattern (e.g., allow only
alphanumeric/underscore, no operator characters) and rejecting or sanitizing
invalid values, and implement logic in the Project schema save/update path
(e.g., a pre-save/pre-validate hook on the Project model) to map the
special-case collection ownerField value to '_id' where appropriate (apply the
users collection override at persistence time rather than only in response
shaping) so stored ownerField is always a validated, normalized identifier used
safely when building ownership filters.
|
@copilot apply changes based on the comments in this thread |
…rd, accessibility Agent-Logs-Url: https://github.com/yash-pouranik/urBackend/sessions/b3c82297-16e2-48e7-9af8-ba7992227aa8 Co-authored-by: yash-pouranik <172860064+yash-pouranik@users.noreply.github.com>
Applied all review feedback in commit
|
|
@coderabbitai now please test all updates may i merge? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainLet me examine the PR review comments and the current state of the code before giving you a merge recommendation. 🏁 Script executed: Length of output: 2014 🏁 Script executed: Length of output: 12355 Let me check the current state of the critical files to see if the flagged issues were addressed. 🏁 Script executed: Length of output: 10824 🏁 Script executed: Length of output: 1879 🏁 Script executed: Length of output: 1894
✅ Issues That Were Fixed
|
@coderabbitai, yes create a followup issue for item 2 and 3, till then I fix 1 |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainSure, 🏁 Script executed: Length of output: 109
Once you've fixed item 1 ( |
|
@copilot apply changes based on the comment in #51 (review) work on the last comment by @coderabbitai and the first one only, i mentioned '
// Line 22 in both files const getVerificationField = (usersColConfig) => {
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainTask: Fix
|
| Location | Change | Why |
|---|---|---|
getVerificationField fallback |
return null |
Stops returning a phantom field name for schemas that define none of the known verification keys |
buildAuthUserPayload |
Skip payload[verificationField] assignment when null |
Prevents writing payload['null'] = false/true during signup/admin-create |
verifyEmail |
Return 500 when verificationField is null |
Prevents silently writing { $set: { null: true } } to the database; gives a clear error instead |
Failed to handle agent chat message. Please try again.
…ema field Agent-Logs-Url: https://github.com/yash-pouranik/urBackend/sessions/8ffa16aa-260a-441d-a417-eb2734631188 Co-authored-by: yash-pouranik <172860064+yash-pouranik@users.noreply.github.com>
yash-pouranik
temporarily deployed
to
rls/v1-write-on-ur-own-doc - urBackend-frankfrut PR #51
Render
Destroyed
Fixed in commit
|
@coderabbitai check if the issue one is solved? and i can merge? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 3850
All previously flagged issues are now addressed:
✅ This PR is ready to merge. The two remaining minor issues (Bearer case-sensitivity |
resolvePublicAuthContext.js: short-circuit for secret key to skip unnecessary JWT workauthorizeWriteOperation.js: guardreq.authUsernull crash whenrequireAuthForWrite: falseproject.controller.js getSingleProject: usegetDefaultRlsForCollectionfor schema-derived defaultsproject.controller.js toggleAuth: usegetDefaultRlsForCollectionfor schema-derived defaultsproject.controller.js updateCollectionRls: restrict_idownerField touserscollection onlyDatabase.jsxRLS defaults: derive ownerField from schema (userId > ownerId > first key)Database.jsxrlsOwnerFieldOptions: generate options from schema keys only (not hardcoded)Database.jsxRLS dialog: addrole="dialog",aria-modal,aria-labelledbyfor accessibilitygetVerificationFieldin bothuserAuth.controller.jsfiles: returnnullinstead of hardcoded'emailVerified'fallback; guard both call sites (buildAuthUserPayload skips field, verifyEmail returns 500 with clear error)⚡ Quickly spin up Copilot coding agent tasks from anywhere on your macOS or Windows machine with Raycast.