fix(backend): enforce private-by-default visibility and pool-op ACL

Listings returned every row regardless of ownership: GET /files, /tags,
/pools and /categories exposed other users' private items (while the
single-item GET correctly returned 403), and the pool file operations
(GET /pools/:id, /pools/:id/files, add/remove/reorder) skipped ACL
entirely, so any authenticated user could read and rewrite anyone's
private pool.

- List queries now filter to rows the caller may see (public, owned, or
  granted can_view) via a shared SQL condition; admins bypass. The viewer
  identity is taken from the request context by the service and passed to
  the repository in the list params.
- Tag/Category/Pool single-item Get now enforce CanView (File already did).
- Pool Get/ListFiles require pool view; AddFiles/RemoveFiles/Reorder
  require pool edit.

Adds regression tests for private-by-default listing (hidden / public /
granted / admin) and for pool operations rejecting a non-owner.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
2026-06-10 15:07:17 +03:00
parent 2af3c481bb
commit 89ba6bae82
12 changed files with 288 additions and 15 deletions
@@ -116,6 +116,12 @@ func (r *CategoryRepo) List(ctx context.Context, params port.OffsetParams) (*dom
args = append(args, "%"+params.Search+"%") args = append(args, "%"+params.Search+"%")
n++ n++
} }
// Restrict to categories the viewer may see (private-by-default), unless admin.
if !params.ViewerIsAdmin {
var aclCond string
aclCond, n, args = aclVisibilityCond("c", objTypeCategory, params.ViewerID, n, args)
conditions = append(conditions, aclCond)
}
where := "" where := ""
if len(conditions) > 0 { if len(conditions) > 0 {
@@ -607,6 +607,13 @@ func (r *FileRepo) List(ctx context.Context, params domain.FileListParams) (*dom
} }
} }
// Restrict to files the viewer may see (private-by-default), unless admin.
if !params.ViewerIsAdmin {
var aclCond string
aclCond, n, args = aclVisibilityCond("f", objTypeFile, params.ViewerID, n, args)
conds = append(conds, aclCond)
}
var orderBy string var orderBy string
if hasCursor { if hasCursor {
ksWhere, ksOrder, nextN, ksArgs := buildKeysetCond( ksWhere, ksOrder, nextN, ksArgs := buildKeysetCond(
@@ -183,6 +183,12 @@ func (r *PoolRepo) List(ctx context.Context, params port.OffsetParams) (*domain.
args = append(args, "%"+params.Search+"%") args = append(args, "%"+params.Search+"%")
n++ n++
} }
// Restrict to pools the viewer may see (private-by-default), unless admin.
if !params.ViewerIsAdmin {
var aclCond string
aclCond, n, args = aclVisibilityCond("p", objTypePool, params.ViewerID, n, args)
conditions = append(conditions, aclCond)
}
where := "" where := ""
if len(conditions) > 0 { if len(conditions) > 0 {
+26
View File
@@ -65,3 +65,29 @@ func connOrTx(ctx context.Context, pool *pgxpool.Pool) db.Querier {
} }
return pool return pool
} }
// Object type IDs as seeded in core.object_types (007_seed_data.sql).
const (
objTypeFile int16 = 1
objTypeTag int16 = 2
objTypeCategory int16 = 3
objTypePool int16 = 4
)
// aclVisibilityCond returns a SQL boolean fragment that is true when the viewer
// may see the row at <alias>.id of the given object type under the
// private-by-default model: the row is public, the viewer created it, or the
// viewer holds an explicit can_view grant. objectTypeID is a trusted constant
// and is inlined; viewerID is bound as $n (referenced twice). Returns the
// fragment, the next free parameter index, and the extended args.
//
// Callers skip this entirely for admins (who bypass ACL).
func aclVisibilityCond(alias string, objectTypeID int16, viewerID int16, n int, args []any) (string, int, []any) {
cond := fmt.Sprintf(
"(%[1]s.is_public OR %[1]s.creator_id = $%[2]d OR EXISTS ("+
"SELECT 1 FROM acl.permissions p "+
"WHERE p.object_type_id = %[3]d AND p.object_id = %[1]s.id "+
"AND p.user_id = $%[2]d AND p.can_view))",
alias, n, objectTypeID)
return cond, n + 1, append(args, viewerID)
}
+6
View File
@@ -169,6 +169,12 @@ func (r *TagRepo) listTags(ctx context.Context, params port.OffsetParams, catego
args = append(args, *categoryID) args = append(args, *categoryID)
n++ n++
} }
// Restrict to tags the viewer may see (private-by-default), unless admin.
if !params.ViewerIsAdmin {
var aclCond string
aclCond, n, args = aclVisibilityCond("t", objTypeTag, params.ViewerID, n, args)
conditions = append(conditions, aclCond)
}
where := "" where := ""
if len(conditions) > 0 { if len(conditions) > 0 {
+6
View File
@@ -49,6 +49,12 @@ type FileListParams struct {
Filter string // filter DSL expression Filter string // filter DSL expression
Search string // substring match on original_name Search string // substring match on original_name
Trash bool // if true, return only soft-deleted files Trash bool // if true, return only soft-deleted files
// Visibility — populated by the service from the request context. When
// ViewerIsAdmin is false the repository restricts results to files the
// viewer may see (public, owned, or explicitly granted).
ViewerID int16
ViewerIsAdmin bool
} }
// FilePage is the result of a cursor-based file listing. // FilePage is the result of a cursor-based file listing.
+115
View File
@@ -828,6 +828,121 @@ func TestBlockRevokesActiveSessions(t *testing.T) {
assert.Equal(t, http.StatusUnauthorized, resp.StatusCode, resp.String()) assert.Equal(t, http.StatusUnauthorized, resp.StatusCode, resp.String())
} }
// fileListIDs returns the set of file IDs visible to token via GET /files.
func (h *harness) fileListIDs(token string) map[string]bool {
h.t.Helper()
resp := h.doJSON("GET", "/files", nil, token)
require.Equal(h.t, http.StatusOK, resp.StatusCode, resp.String())
var page map[string]any
resp.decode(h.t, &page)
ids := map[string]bool{}
if items, ok := page["items"].([]any); ok {
for _, it := range items {
if m, ok := it.(map[string]any); ok {
if id, ok := m["id"].(string); ok {
ids[id] = true
}
}
}
}
return ids
}
// TestPrivateByDefaultVisibility verifies that listings only return rows the
// caller may see: private files are hidden from non-owners, public files are
// visible to all, an explicit grant reveals a private file, and admins see all.
func TestPrivateByDefaultVisibility(t *testing.T) {
if testing.Short() {
t.Skip("skipping integration test in short mode")
}
h := setupSuite(t)
adminToken := h.login("admin", "admin")
resp := h.doJSON("POST", "/users", map[string]any{"name": "alice", "password": "alicepass"}, adminToken)
require.Equal(t, http.StatusCreated, resp.StatusCode, resp.String())
resp = h.doJSON("POST", "/users", map[string]any{"name": "bob", "password": "bobpass"}, adminToken)
require.Equal(t, http.StatusCreated, resp.StatusCode, resp.String())
var bob map[string]any
resp.decode(t, &bob)
bobID := bob["id"].(float64)
aliceToken := h.login("alice", "alicepass")
bobToken := h.login("bob", "bobpass")
file := h.uploadJPEG(aliceToken, "alice-secret.jpg")
fileID := file["id"].(string)
// Owner and admin see it; the unrelated user does not.
assert.True(t, h.fileListIDs(aliceToken)[fileID], "owner should see own file")
assert.True(t, h.fileListIDs(adminToken)[fileID], "admin should see all files")
assert.False(t, h.fileListIDs(bobToken)[fileID], "private file must not appear for a non-owner")
// Making it public reveals it to everyone.
resp = h.doJSON("PATCH", "/files/"+fileID, map[string]any{"is_public": true}, aliceToken)
require.Equal(t, http.StatusOK, resp.StatusCode, resp.String())
assert.True(t, h.fileListIDs(bobToken)[fileID], "public file should be visible to all")
// Private again → hidden; an explicit view grant reveals it.
resp = h.doJSON("PATCH", "/files/"+fileID, map[string]any{"is_public": false}, aliceToken)
require.Equal(t, http.StatusOK, resp.StatusCode, resp.String())
assert.False(t, h.fileListIDs(bobToken)[fileID])
resp = h.doJSON("PUT", "/acl/file/"+fileID, map[string]any{
"permissions": []map[string]any{{"user_id": bobID, "can_view": true, "can_edit": false}},
}, aliceToken)
require.Equal(t, http.StatusOK, resp.StatusCode, resp.String())
assert.True(t, h.fileListIDs(bobToken)[fileID], "granted file should be visible in the listing")
}
// TestPoolOperationsRequireACL verifies that a non-owner cannot view or modify
// another user's private pool's contents.
func TestPoolOperationsRequireACL(t *testing.T) {
if testing.Short() {
t.Skip("skipping integration test in short mode")
}
h := setupSuite(t)
adminToken := h.login("admin", "admin")
resp := h.doJSON("POST", "/users", map[string]any{"name": "alice", "password": "alicepass"}, adminToken)
require.Equal(t, http.StatusCreated, resp.StatusCode, resp.String())
resp = h.doJSON("POST", "/users", map[string]any{"name": "bob", "password": "bobpass"}, adminToken)
require.Equal(t, http.StatusCreated, resp.StatusCode, resp.String())
aliceToken := h.login("alice", "alicepass")
bobToken := h.login("bob", "bobpass")
file := h.uploadJPEG(aliceToken, "f.jpg")
fileID := file["id"].(string)
resp = h.doJSON("POST", "/pools", map[string]any{"name": "alice pool", "is_public": false}, aliceToken)
require.Equal(t, http.StatusCreated, resp.StatusCode, resp.String())
var pool map[string]any
resp.decode(t, &pool)
poolID := pool["id"].(string)
resp = h.doJSON("POST", "/pools/"+poolID+"/files", map[string]any{"file_ids": []string{fileID}}, aliceToken)
require.Equal(t, http.StatusCreated, resp.StatusCode, resp.String())
// Bob cannot view the pool, list its files, or mutate its membership.
for _, c := range []struct {
method, path string
body any
}{
{"GET", "/pools/" + poolID, nil},
{"GET", "/pools/" + poolID + "/files", nil},
{"POST", "/pools/" + poolID + "/files", map[string]any{"file_ids": []string{fileID}}},
{"POST", "/pools/" + poolID + "/files/remove", map[string]any{"file_ids": []string{fileID}}},
{"PUT", "/pools/" + poolID + "/files/reorder", map[string]any{"file_ids": []string{fileID}}},
} {
resp = h.doJSON(c.method, c.path, c.body, bobToken)
assert.Equal(t, http.StatusForbidden, resp.StatusCode, "%s %s: %s", c.method, c.path, resp)
}
// The owner can still list the pool's files.
resp = h.doJSON("GET", "/pools/"+poolID+"/files", nil, aliceToken)
assert.Equal(t, http.StatusOK, resp.StatusCode, resp.String())
}
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
// Test helpers // Test helpers
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
+7
View File
@@ -22,6 +22,13 @@ type OffsetParams struct {
Search string Search string
Offset int Offset int
Limit int Limit int
// Visibility — populated by the service from the request context. When
// ViewerIsAdmin is false the repository restricts results to rows the viewer
// may see (public, owned, or explicitly granted). Ignored by user listing,
// which is admin-only.
ViewerID int16
ViewerIsAdmin bool
} }
// PoolFileListParams holds parameters for listing files inside a pool. // PoolFileListParams holds parameters for listing files inside a pool.
+19 -4
View File
@@ -49,14 +49,27 @@ func NewCategoryService(
// CRUD // CRUD
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
// List returns a paginated, optionally filtered list of categories. // List returns a paginated list of categories the caller may see.
func (s *CategoryService) List(ctx context.Context, params port.OffsetParams) (*domain.CategoryOffsetPage, error) { func (s *CategoryService) List(ctx context.Context, params port.OffsetParams) (*domain.CategoryOffsetPage, error) {
params.ViewerID, params.ViewerIsAdmin, _ = domain.UserFromContext(ctx)
return s.categories.List(ctx, params) return s.categories.List(ctx, params)
} }
// Get returns a category by ID. // Get returns a category by ID, enforcing view ACL.
func (s *CategoryService) Get(ctx context.Context, id uuid.UUID) (*domain.Category, error) { func (s *CategoryService) Get(ctx context.Context, id uuid.UUID) (*domain.Category, error) {
return s.categories.GetByID(ctx, id) userID, isAdmin, _ := domain.UserFromContext(ctx)
c, err := s.categories.GetByID(ctx, id)
if err != nil {
return nil, err
}
ok, err := s.acl.CanView(ctx, userID, isAdmin, c.CreatorID, c.IsPublic, categoryObjectTypeID, id)
if err != nil {
return nil, err
}
if !ok {
return nil, domain.ErrForbidden
}
return c, nil
} }
// Create inserts a new category record. // Create inserts a new category record.
@@ -158,7 +171,9 @@ func (s *CategoryService) Delete(ctx context.Context, id uuid.UUID) error {
// Tags in category // Tags in category
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
// ListTags returns a paginated list of tags belonging to this category. // ListTags returns a paginated list of tags in this category that the caller
// may see.
func (s *CategoryService) ListTags(ctx context.Context, categoryID uuid.UUID, params port.OffsetParams) (*domain.TagOffsetPage, error) { func (s *CategoryService) ListTags(ctx context.Context, categoryID uuid.UUID, params port.OffsetParams) (*domain.TagOffsetPage, error) {
params.ViewerID, params.ViewerIsAdmin, _ = domain.UserFromContext(ctx)
return s.tags.ListByCategory(ctx, categoryID, params) return s.tags.ListByCategory(ctx, categoryID, params)
} }
+3 -1
View File
@@ -406,8 +406,10 @@ func (s *FileService) Replace(ctx context.Context, id uuid.UUID, p UploadParams)
return updated, nil return updated, nil
} }
// List delegates to FileRepo with the given params. // List delegates to FileRepo with the given params, restricting results to
// files the caller may see (unless they are an admin).
func (s *FileService) List(ctx context.Context, params domain.FileListParams) (*domain.FilePage, error) { func (s *FileService) List(ctx context.Context, params domain.FileListParams) (*domain.FilePage, error) {
params.ViewerID, params.ViewerIsAdmin, _ = domain.UserFromContext(ctx)
return s.files.List(ctx, params) return s.files.List(ctx, params)
} }
+71 -7
View File
@@ -41,14 +41,63 @@ func NewPoolService(
// CRUD // CRUD
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
// List returns a paginated list of pools. // List returns a paginated list of pools the caller may see.
func (s *PoolService) List(ctx context.Context, params port.OffsetParams) (*domain.PoolOffsetPage, error) { func (s *PoolService) List(ctx context.Context, params port.OffsetParams) (*domain.PoolOffsetPage, error) {
params.ViewerID, params.ViewerIsAdmin, _ = domain.UserFromContext(ctx)
return s.pools.List(ctx, params) return s.pools.List(ctx, params)
} }
// Get returns a pool by ID. // Get returns a pool by ID, enforcing view ACL.
func (s *PoolService) Get(ctx context.Context, id uuid.UUID) (*domain.Pool, error) { func (s *PoolService) Get(ctx context.Context, id uuid.UUID) (*domain.Pool, error) {
return s.pools.GetByID(ctx, id) userID, isAdmin, _ := domain.UserFromContext(ctx)
p, err := s.pools.GetByID(ctx, id)
if err != nil {
return nil, err
}
ok, err := s.acl.CanView(ctx, userID, isAdmin, p.CreatorID, p.IsPublic, poolObjectTypeID, id)
if err != nil {
return nil, err
}
if !ok {
return nil, domain.ErrForbidden
}
return p, nil
}
// authorizeView returns nil if the caller may view the pool, else ErrForbidden
// (or ErrNotFound if the pool does not exist).
func (s *PoolService) authorizeView(ctx context.Context, poolID uuid.UUID) error {
userID, isAdmin, _ := domain.UserFromContext(ctx)
p, err := s.pools.GetByID(ctx, poolID)
if err != nil {
return err
}
ok, err := s.acl.CanView(ctx, userID, isAdmin, p.CreatorID, p.IsPublic, poolObjectTypeID, poolID)
if err != nil {
return err
}
if !ok {
return domain.ErrForbidden
}
return nil
}
// authorizeEdit returns nil if the caller may edit the pool, else ErrForbidden
// (or ErrNotFound if the pool does not exist).
func (s *PoolService) authorizeEdit(ctx context.Context, poolID uuid.UUID) error {
userID, isAdmin, _ := domain.UserFromContext(ctx)
p, err := s.pools.GetByID(ctx, poolID)
if err != nil {
return err
}
ok, err := s.acl.CanEdit(ctx, userID, isAdmin, p.CreatorID, poolObjectTypeID, poolID)
if err != nil {
return err
}
if !ok {
return domain.ErrForbidden
}
return nil
} }
// Create inserts a new pool. // Create inserts a new pool.
@@ -146,13 +195,21 @@ func (s *PoolService) Delete(ctx context.Context, id uuid.UUID) error {
// Poolfile operations // Poolfile operations
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
// ListFiles returns cursor-paginated files within a pool ordered by position. // ListFiles returns cursor-paginated files within a pool ordered by position,
// enforcing view ACL on the pool.
func (s *PoolService) ListFiles(ctx context.Context, poolID uuid.UUID, params port.PoolFileListParams) (*domain.PoolFilePage, error) { func (s *PoolService) ListFiles(ctx context.Context, poolID uuid.UUID, params port.PoolFileListParams) (*domain.PoolFilePage, error) {
if err := s.authorizeView(ctx, poolID); err != nil {
return nil, err
}
return s.pools.ListFiles(ctx, poolID, params) return s.pools.ListFiles(ctx, poolID, params)
} }
// AddFiles adds files to a pool at the given position (nil = append). // AddFiles adds files to a pool at the given position (nil = append), enforcing
// edit ACL on the pool.
func (s *PoolService) AddFiles(ctx context.Context, poolID uuid.UUID, fileIDs []uuid.UUID, position *int) error { func (s *PoolService) AddFiles(ctx context.Context, poolID uuid.UUID, fileIDs []uuid.UUID, position *int) error {
if err := s.authorizeEdit(ctx, poolID); err != nil {
return err
}
if err := s.pools.AddFiles(ctx, poolID, fileIDs, position); err != nil { if err := s.pools.AddFiles(ctx, poolID, fileIDs, position); err != nil {
return err return err
} }
@@ -161,8 +218,11 @@ func (s *PoolService) AddFiles(ctx context.Context, poolID uuid.UUID, fileIDs []
return nil return nil
} }
// RemoveFiles removes files from a pool. // RemoveFiles removes files from a pool, enforcing edit ACL on the pool.
func (s *PoolService) RemoveFiles(ctx context.Context, poolID uuid.UUID, fileIDs []uuid.UUID) error { func (s *PoolService) RemoveFiles(ctx context.Context, poolID uuid.UUID, fileIDs []uuid.UUID) error {
if err := s.authorizeEdit(ctx, poolID); err != nil {
return err
}
if err := s.pools.RemoveFiles(ctx, poolID, fileIDs); err != nil { if err := s.pools.RemoveFiles(ctx, poolID, fileIDs); err != nil {
return err return err
} }
@@ -171,7 +231,11 @@ func (s *PoolService) RemoveFiles(ctx context.Context, poolID uuid.UUID, fileIDs
return nil return nil
} }
// Reorder sets the full ordered sequence of file IDs within a pool. // Reorder sets the ordered sequence of file IDs within a pool, enforcing edit
// ACL on the pool.
func (s *PoolService) Reorder(ctx context.Context, poolID uuid.UUID, fileIDs []uuid.UUID) error { func (s *PoolService) Reorder(ctx context.Context, poolID uuid.UUID, fileIDs []uuid.UUID) error {
if err := s.authorizeEdit(ctx, poolID); err != nil {
return err
}
return s.pools.Reorder(ctx, poolID, fileIDs) return s.pools.Reorder(ctx, poolID, fileIDs)
} }
+16 -3
View File
@@ -54,14 +54,27 @@ func NewTagService(
// Tag CRUD // Tag CRUD
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
// List returns a paginated, optionally filtered list of tags. // List returns a paginated, optionally filtered list of tags the caller may see.
func (s *TagService) List(ctx context.Context, params port.OffsetParams) (*domain.TagOffsetPage, error) { func (s *TagService) List(ctx context.Context, params port.OffsetParams) (*domain.TagOffsetPage, error) {
params.ViewerID, params.ViewerIsAdmin, _ = domain.UserFromContext(ctx)
return s.tags.List(ctx, params) return s.tags.List(ctx, params)
} }
// Get returns a tag by ID. // Get returns a tag by ID, enforcing view ACL.
func (s *TagService) Get(ctx context.Context, id uuid.UUID) (*domain.Tag, error) { func (s *TagService) Get(ctx context.Context, id uuid.UUID) (*domain.Tag, error) {
return s.tags.GetByID(ctx, id) userID, isAdmin, _ := domain.UserFromContext(ctx)
t, err := s.tags.GetByID(ctx, id)
if err != nil {
return nil, err
}
ok, err := s.acl.CanView(ctx, userID, isAdmin, t.CreatorID, t.IsPublic, tagObjectTypeID, id)
if err != nil {
return nil, err
}
if !ok {
return nil, domain.ErrForbidden
}
return t, nil
} }
// Create inserts a new tag record. // Create inserts a new tag record.