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
123 lines
4.0 KiB
Go
123 lines
4.0 KiB
Go
package services
|
|
|
|
import (
|
|
"bookhoard/internal/database"
|
|
"context"
|
|
"encoding/json"
|
|
"fmt"
|
|
|
|
"github.com/google/uuid"
|
|
"github.com/jackc/pgx/v5/pgtype"
|
|
)
|
|
|
|
type FiltersService struct {
|
|
db *database.Queries
|
|
}
|
|
|
|
func NewFiltersService(db *database.Queries) *FiltersService {
|
|
return &FiltersService{db: db}
|
|
}
|
|
|
|
// GetSavedFilters - Retrieve all saved filters for a user + resource type
|
|
func (s *FiltersService) GetSavedFilters(ctx context.Context, userID uuid.UUID, resourceType string) ([]database.SavedFilters, error) {
|
|
filters, err := s.db.GetSavedFilters(ctx, database.GetSavedFiltersParams{
|
|
UserID: pgtype.UUID{Bytes: userID, Valid: true},
|
|
ResourceType: resourceType,
|
|
})
|
|
if err != nil {
|
|
return nil, fmt.Errorf("failed to get saved filters: %w", err)
|
|
}
|
|
|
|
return filters, nil
|
|
}
|
|
|
|
// CreateSavedFilter - Create a new saved filter
|
|
func (s *FiltersService) CreateSavedFilter(ctx context.Context, userID uuid.UUID, name string, resourceType string, filters map[string]string) (database.SavedFilters, error) {
|
|
// Business logic: Validate filter name uniqueness per user + resource type
|
|
existing, err := s.db.GetSavedFilters(ctx, database.GetSavedFiltersParams{
|
|
UserID: pgtype.UUID{Bytes: userID, Valid: true},
|
|
ResourceType: resourceType,
|
|
})
|
|
if err == nil {
|
|
for _, f := range existing {
|
|
if f.Name == name {
|
|
return database.SavedFilters{}, fmt.Errorf("filter with name '%s' already exists for this resource type", name)
|
|
}
|
|
}
|
|
}
|
|
|
|
// Convert filters map to JSONB ([]byte)
|
|
filtersJSON, err := json.Marshal(filters)
|
|
if err != nil {
|
|
return database.SavedFilters{}, fmt.Errorf("failed to marshal filters: %w", err)
|
|
}
|
|
|
|
filter, err := s.db.CreateSavedFilter(ctx, database.CreateSavedFilterParams{
|
|
UserID: pgtype.UUID{Bytes: userID, Valid: true},
|
|
Name: name,
|
|
ResourceType: resourceType,
|
|
Filters: filtersJSON,
|
|
})
|
|
if err != nil {
|
|
return database.SavedFilters{}, fmt.Errorf("failed to create saved filter: %w", err)
|
|
}
|
|
|
|
return filter, nil
|
|
}
|
|
|
|
// UpdateSavedFilter - Update an existing saved filter
|
|
func (s *FiltersService) UpdateSavedFilter(ctx context.Context, userID uuid.UUID, filterID uuid.UUID, name string, filters map[string]string) (database.SavedFilters, error) {
|
|
// Business logic: Verify filter exists and belongs to user
|
|
existing, err := s.db.GetSavedFilterByID(ctx, database.GetSavedFilterByIDParams{
|
|
ID: pgtype.UUID{Bytes: filterID, Valid: true},
|
|
UserID: pgtype.UUID{Bytes: userID, Valid: true},
|
|
})
|
|
if err != nil {
|
|
return database.SavedFilters{}, fmt.Errorf("filter not found or access denied: %w", err)
|
|
}
|
|
|
|
// Business logic: Check name uniqueness (excluding current filter)
|
|
allFilters, err := s.db.GetSavedFilters(ctx, database.GetSavedFiltersParams{
|
|
UserID: pgtype.UUID{Bytes: userID, Valid: true},
|
|
ResourceType: existing.ResourceType,
|
|
})
|
|
if err == nil {
|
|
for _, f := range allFilters {
|
|
existingID := uuid.Must(uuid.FromBytes(f.ID.Bytes[:]))
|
|
if f.Name == name && existingID != filterID {
|
|
return database.SavedFilters{}, fmt.Errorf("filter with name '%s' already exists for this resource type", name)
|
|
}
|
|
}
|
|
}
|
|
|
|
// Convert filters to JSONB
|
|
filtersJSON, err := json.Marshal(filters)
|
|
if err != nil {
|
|
return database.SavedFilters{}, fmt.Errorf("failed to marshal filters: %w", err)
|
|
}
|
|
|
|
updated, err := s.db.UpdateSavedFilter(ctx, database.UpdateSavedFilterParams{
|
|
ID: pgtype.UUID{Bytes: filterID, Valid: true},
|
|
UserID: pgtype.UUID{Bytes: userID, Valid: true},
|
|
Name: name,
|
|
Filters: filtersJSON,
|
|
})
|
|
if err != nil {
|
|
return database.SavedFilters{}, fmt.Errorf("failed to update saved filter: %w", err)
|
|
}
|
|
|
|
return updated, nil
|
|
}
|
|
|
|
// DeleteSavedFilter - Delete a saved filter
|
|
func (s *FiltersService) DeleteSavedFilter(ctx context.Context, userID uuid.UUID, filterID uuid.UUID) error {
|
|
_, err := s.db.DeleteSavedFilter(ctx, database.DeleteSavedFilterParams{
|
|
ID: pgtype.UUID{Bytes: filterID, Valid: true},
|
|
UserID: pgtype.UUID{Bytes: userID, Valid: true},
|
|
})
|
|
if err != nil {
|
|
return fmt.Errorf("failed to delete saved filter: %w", err)
|
|
}
|
|
return nil
|
|
}
|