fix: make clients shared across all users#458
fix: make clients shared across all users#458devin-ai-integration[bot] wants to merge 1 commit intomainfrom
Conversation
Root cause: The clients table had a user_email column and all client queries filtered by WHERE user_email = ?, scoping clients to the user who created them. Other users could not see those clients. Changes: - Remove user_email column from clients table schema - Remove user_email filtering from all client CRUD queries - Remove user_email filtering from client lookups in reports routes - Remove user_email filtering from client existence checks in work entries - Work entries remain scoped to individual users via their own user_email - Update all tests to reflect the new shared client behavior
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| db.run( | ||
| 'DELETE FROM clients WHERE user_email = ?', | ||
| [req.userEmail], | ||
| 'DELETE FROM clients', | ||
| [], |
There was a problem hiding this comment.
🔴 DELETE /api/clients/ deletes ALL clients globally, cascading to ALL users' work entries
The DELETE / route now executes DELETE FROM clients with no WHERE clause. Previously it was scoped to WHERE user_email = ?, deleting only the authenticated user's clients. Now that clients are shared, any authenticated user hitting this endpoint (used by the frontend at frontend/src/pages/ClientsPage.tsx:84) wipes out every client in the system. Due to the ON DELETE CASCADE foreign key on work_entries.client_id (backend/src/database/init.js:62), this also cascades to delete ALL work entries for ALL users — a catastrophic data loss scenario triggered by a single user.
Prompt for agents
The DELETE / route on clients.js:189-208 now runs DELETE FROM clients with no filter. Since clients are shared across all users, this becomes a system-wide destructive operation that any authenticated user can trigger, also cascading to delete all work entries for all users via the ON DELETE CASCADE constraint.
Possible approaches:
1. Remove the DELETE / route entirely now that clients are shared — a single user should not be able to delete all shared resources.
2. Add an admin authorization check before allowing bulk deletion.
3. If bulk delete must remain, at minimum scope it so it only deletes clients that have no work entries from other users, or require an admin role.
The frontend calls this via apiClient.deleteAllClients() in frontend/src/pages/ClientsPage.tsx:84, so the UI trigger also needs to be reconsidered.
Was this helpful? React with 👍 or 👎 to provide feedback.
| 'SELECT id FROM clients WHERE id = ?', | ||
| [clientId], |
There was a problem hiding this comment.
🔴 DELETE /api/clients/:id cascades deletion of other users' work entries
Any authenticated user can delete any shared client via DELETE /api/clients/:id. Because work_entries.client_id has ON DELETE CASCADE (backend/src/database/init.js:62), deleting a client also deletes all work entries linked to that client — including entries belonging to other users. Previously, the WHERE user_email = ? filter ensured only the owner could delete their own client. Now with shared clients, user A deleting a client causes user B's work entries for that client to be silently destroyed.
Prompt for agents
The DELETE /:id route in clients.js:210-249 allows any authenticated user to delete any shared client. Due to ON DELETE CASCADE on work_entries.client_id, this also destroys all work entries from ALL users that reference that client.
Possible approaches:
1. Add authorization so only an admin or the original creator of the client can delete it (this may require adding a created_by column to the clients table).
2. Before deleting, check if other users have work entries referencing this client and either prevent deletion or warn the user.
3. Change the cascade behavior — instead of ON DELETE CASCADE, use ON DELETE RESTRICT or SET NULL to prevent accidental data loss in work_entries.
Was this helpful? React with 👍 or 👎 to provide feedback.
Summary
Clients created by one user were not visible to other users because the
clientstable had auser_emailcolumn and every client query filtered byWHERE user_email = ?. This PR removesuser_emailfrom the clients table and all client-related queries so that clients are shared globally, while keeping work entries scoped to individual users.Schema change: Removed
user_emailcolumn and its foreign key from theclientstable. Removed theidx_clients_user_emailindex.Query changes across 4 route files:
clients.js— Removeduser_emailfiltering from all CRUD operations (GET, POST, PUT, DELETE)reports.js— Removeduser_emailfrom client lookup queries; work entry queries still filter byuser_emailworkEntries.js— Removeduser_emailfrom client existence checks when creating/updating work entries; work entry ownership unchangedReview & Testing Checklist for Human
DELETE /api/clients(bulk) now deletes ALL clients globally, not just the current user's. Previously this was scoped toWHERE user_email = ?. Verify this is acceptable — any authenticated user can wipe all clients for everyone.PUTandDELETE /api/clients/:idhave no ownership checks. Any authenticated user can now edit or delete any client. Consider whether some form of authorization (e.g., admin-only, or creator-only for destructive ops) is needed.user_emailin client-related API calls or responses. No frontend files were changed in this PR.Notes
Link to Devin session: https://partner-workshops.devinenterprise.com/sessions/57b4fa950fbc4e099e30416513b8873d
Requested by: @VedantKh