Fix critical security issue where admin users could delete other users'
saved filters due to incorrect error handling in DELETE query.
Database Schema Changes:
- Change DeleteSavedFilter from :exec to :one (queries.sql:1747-1750)
- Add RETURNING * to return deleted row for proper error detection
- Regenerate querier.go and queries.sql.go with updated signature
Service Layer (internal/services/filters.go):
- Update DeleteSavedFilter to capture returned row (using _ to discard)
- Properly propagate pgx.ErrNoRows when no rows are deleted
- Error wrapping preserves original error for handler detection
Handler Layer (internal/handlers/filters.go):
- Add errors.Is() check for pgx.ErrNoRows (line 148)
- Return 404 Not Found when filter doesn't exist or belongs to different user
- Return 500 Internal Server Error for other database errors
- Add "errors" import (line 8)
Security Fix Details:
Before: Admin could delete user's filter → 204 No Content (SUCCESS)
After: Admin tries to delete user's filter → 404 Not Found (DENIED)
The DELETE query uses WHERE id = @id AND user_id = @user_id, which matches
0 rows when attempting to delete another user's filter. The old :exec query
didn't return row count, so 0 affected rows looked like success. The new :one
query with RETURNING * returns pgx.ErrNoRows when no rows match, allowing
the handler to return proper 404 error.
Test Impact:
- TestSavedFilters/User_cannot_access_another_user's_filter now passes
- All 6 integration tests pass with proper user isolation enforcement
Pattern Consistency:
- Matches DeleteLibraryFolder pattern (line 99 in queries.sql)
- Uses same error handling as media handlers (errors.Is + pgx.ErrNoRows)
- Follows user-scoping pattern used throughout codebase
Related: Saved filters implementation user isolation
Security: Prevents unauthorized deletion of user data
Add complete backend implementation for saved filters CRUD operations
with proper service layer architecture and RESTful API endpoints.
Service Layer (internal/services/filters.go):
- NewFiltersService() constructor following project patterns
- GetSavedFilters(): Retrieve all filters for user + resource type
- CreateSavedFilter(): Create filter with duplicate name validation
- UpdateSavedFilter(): Update filter with ownership verification
- DeleteSavedFilter(): Delete filter with user scoping
Business Logic:
- Filter name uniqueness enforced per user + resource type
- User ownership validation on all operations (JWT user_id)
- JSONB marshaling/unmarshaling for flexible filter storage
- Proper error wrapping with context messages
Handler Layer (internal/handlers/filters.go):
- NewFiltersHandler() constructor (receives db.Queries)
- GetSavedFilters: GET /api/saved-filters?resource_type=X
- CreateSavedFilter: POST /api/saved-filters
- UpdateSavedFilter: PUT /api/saved-filters/:id
- DeleteSavedFilter: DELETE /api/saved-filters/:id
Content Negotiation:
- Supports both JSON (API clients) and HTML (HTMX) responses
- wantsHTML() helper checks Accept header
- HX-Redirect header for HTMX form submissions
- Proper status codes (200, 201, 204, 400, 401, 404, 409)
Router Configuration:
- registerFiltersRoutes() function in internal/router/filters.go
- JWT middleware protection on all endpoints
- RESTful route structure: /api/saved-filters
- Registered in main router.go RegisterRoutes() function
- Added FiltersHandler to router.Config struct
Test Infrastructure:
- Added FiltersHandler to test server setup (test_helpers_test.go)
- FiltersHandler initialized in setupTestServer() function
- Router.Config includes FiltersHandler for integration tests
Code Quality:
- Follows PROJECT_GUIDELINES.md service layer patterns
- Uses database models (not custom domain models)
- JSONB returned as []byte (matches collections pattern)
- All errors wrapped with context using fmt.Errorf
- Handlers create services internally (not dependency injection)
Part of: Saved Filters Implementation (Phase 2: Backend)
Related: #saved-filters-feature
john-okeefe