diff --git a/backend/internal/db/postgres/category_repo.go b/backend/internal/db/postgres/category_repo.go index ea97bbd..509ac2a 100644 --- a/backend/internal/db/postgres/category_repo.go +++ b/backend/internal/db/postgres/category_repo.go @@ -116,6 +116,12 @@ func (r *CategoryRepo) List(ctx context.Context, params port.OffsetParams) (*dom args = append(args, "%"+params.Search+"%") 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 := "" if len(conditions) > 0 { diff --git a/backend/internal/db/postgres/file_repo.go b/backend/internal/db/postgres/file_repo.go index 57e11ee..3a227c9 100644 --- a/backend/internal/db/postgres/file_repo.go +++ b/backend/internal/db/postgres/file_repo.go @@ -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 if hasCursor { ksWhere, ksOrder, nextN, ksArgs := buildKeysetCond( diff --git a/backend/internal/db/postgres/pool_repo.go b/backend/internal/db/postgres/pool_repo.go index e5b373e..e1fc848 100644 --- a/backend/internal/db/postgres/pool_repo.go +++ b/backend/internal/db/postgres/pool_repo.go @@ -183,6 +183,12 @@ func (r *PoolRepo) List(ctx context.Context, params port.OffsetParams) (*domain. args = append(args, "%"+params.Search+"%") 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 := "" if len(conditions) > 0 { diff --git a/backend/internal/db/postgres/postgres.go b/backend/internal/db/postgres/postgres.go index c8c4bdd..86e9d11 100644 --- a/backend/internal/db/postgres/postgres.go +++ b/backend/internal/db/postgres/postgres.go @@ -65,3 +65,29 @@ func connOrTx(ctx context.Context, pool *pgxpool.Pool) db.Querier { } 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 .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) +} diff --git a/backend/internal/db/postgres/tag_repo.go b/backend/internal/db/postgres/tag_repo.go index 42dd990..742e552 100644 --- a/backend/internal/db/postgres/tag_repo.go +++ b/backend/internal/db/postgres/tag_repo.go @@ -169,6 +169,12 @@ func (r *TagRepo) listTags(ctx context.Context, params port.OffsetParams, catego args = append(args, *categoryID) 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 := "" if len(conditions) > 0 { diff --git a/backend/internal/domain/file.go b/backend/internal/domain/file.go index 45f72f9..d5569e4 100644 --- a/backend/internal/domain/file.go +++ b/backend/internal/domain/file.go @@ -49,6 +49,12 @@ type FileListParams struct { Filter string // filter DSL expression Search string // substring match on original_name 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. diff --git a/backend/internal/integration/server_test.go b/backend/internal/integration/server_test.go index 27556f5..aa50b3b 100644 --- a/backend/internal/integration/server_test.go +++ b/backend/internal/integration/server_test.go @@ -828,6 +828,121 @@ func TestBlockRevokesActiveSessions(t *testing.T) { 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 // --------------------------------------------------------------------------- diff --git a/backend/internal/port/repository.go b/backend/internal/port/repository.go index a7d3649..e578aa2 100644 --- a/backend/internal/port/repository.go +++ b/backend/internal/port/repository.go @@ -22,6 +22,13 @@ type OffsetParams struct { Search string Offset 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. diff --git a/backend/internal/service/category_service.go b/backend/internal/service/category_service.go index 2034245..31c9809 100644 --- a/backend/internal/service/category_service.go +++ b/backend/internal/service/category_service.go @@ -49,14 +49,27 @@ func NewCategoryService( // 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) { + params.ViewerID, params.ViewerIsAdmin, _ = domain.UserFromContext(ctx) 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) { - 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. @@ -158,7 +171,9 @@ func (s *CategoryService) Delete(ctx context.Context, id uuid.UUID) error { // 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) { + params.ViewerID, params.ViewerIsAdmin, _ = domain.UserFromContext(ctx) return s.tags.ListByCategory(ctx, categoryID, params) } \ No newline at end of file diff --git a/backend/internal/service/file_service.go b/backend/internal/service/file_service.go index 5c56ce7..05acd10 100644 --- a/backend/internal/service/file_service.go +++ b/backend/internal/service/file_service.go @@ -406,8 +406,10 @@ func (s *FileService) Replace(ctx context.Context, id uuid.UUID, p UploadParams) 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) { + params.ViewerID, params.ViewerIsAdmin, _ = domain.UserFromContext(ctx) return s.files.List(ctx, params) } diff --git a/backend/internal/service/pool_service.go b/backend/internal/service/pool_service.go index 98552df..d1890f0 100644 --- a/backend/internal/service/pool_service.go +++ b/backend/internal/service/pool_service.go @@ -41,14 +41,63 @@ func NewPoolService( // 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) { + params.ViewerID, params.ViewerIsAdmin, _ = domain.UserFromContext(ctx) 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) { - 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. @@ -146,13 +195,21 @@ func (s *PoolService) Delete(ctx context.Context, id uuid.UUID) error { // Pool–file 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) { + if err := s.authorizeView(ctx, poolID); err != nil { + return nil, err + } 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 { + if err := s.authorizeEdit(ctx, poolID); err != nil { + return err + } if err := s.pools.AddFiles(ctx, poolID, fileIDs, position); err != nil { return err } @@ -161,8 +218,11 @@ func (s *PoolService) AddFiles(ctx context.Context, poolID uuid.UUID, fileIDs [] 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 { + if err := s.authorizeEdit(ctx, poolID); err != nil { + return err + } if err := s.pools.RemoveFiles(ctx, poolID, fileIDs); err != nil { return err } @@ -171,7 +231,11 @@ func (s *PoolService) RemoveFiles(ctx context.Context, poolID uuid.UUID, fileIDs 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 { + if err := s.authorizeEdit(ctx, poolID); err != nil { + return err + } return s.pools.Reorder(ctx, poolID, fileIDs) } \ No newline at end of file diff --git a/backend/internal/service/tag_service.go b/backend/internal/service/tag_service.go index 9de7e9a..195d6fd 100644 --- a/backend/internal/service/tag_service.go +++ b/backend/internal/service/tag_service.go @@ -54,14 +54,27 @@ func NewTagService( // 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) { + params.ViewerID, params.ViewerIsAdmin, _ = domain.UserFromContext(ctx) 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) { - 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.