From ac76bce5cd0f99de8d96b1d67946685e0a6481dd Mon Sep 17 00:00:00 2001 From: kolaente Date: Fri, 3 Apr 2026 21:30:24 +0200 Subject: [PATCH] fix: use recursive CTE in accessibleProjectIDsSubquery for inherited project permissions Resolves #2490. Users with team access on a parent project were not seeing subtask relations for tasks in child projects because getUserProjectsStatement does not walk the project hierarchy. The fix wraps the base query in a recursive CTE that traverses child projects via parent_project_id. --- pkg/db/fixtures/project_views.yml | 50 ++++++++++++++++++ pkg/db/fixtures/projects.yml | 21 ++++++++ pkg/db/fixtures/task_relations.yml | 13 +++++ pkg/db/fixtures/tasks.yml | 18 +++++++ pkg/db/fixtures/team_members.yml | 3 ++ pkg/db/fixtures/team_projects.yml | 7 +++ pkg/db/fixtures/teams.yml | 3 ++ pkg/models/project.go | 29 ++++++++--- pkg/models/project_test.go | 2 +- pkg/models/task_relation_authz_test.go | 70 ++++++++++++++++++++++++++ 10 files changed, 209 insertions(+), 7 deletions(-) diff --git a/pkg/db/fixtures/project_views.yml b/pkg/db/fixtures/project_views.yml index a8fada26a..918b1595f 100644 --- a/pkg/db/fixtures/project_views.yml +++ b/pkg/db/fixtures/project_views.yml @@ -957,3 +957,53 @@ updated: '2024-03-18 15:14:13' created: '2018-03-18 15:14:13' bucket_configuration_mode: 1 +- id: 153 + title: List + project_id: 41 + view_kind: 0 + updated: '2024-03-18 15:14:13' + created: '2018-03-18 15:14:13' +- id: 154 + title: Gantt + project_id: 41 + view_kind: 1 + updated: '2024-03-18 15:14:13' + created: '2018-03-18 15:14:13' +- id: 155 + title: Table + project_id: 41 + view_kind: 2 + updated: '2024-03-18 15:14:13' + created: '2018-03-18 15:14:13' +- id: 156 + title: Kanban + project_id: 41 + view_kind: 3 + updated: '2024-03-18 15:14:13' + created: '2018-03-18 15:14:13' + bucket_configuration_mode: 1 +- id: 157 + title: List + project_id: 42 + view_kind: 0 + updated: '2024-03-18 15:14:13' + created: '2018-03-18 15:14:13' +- id: 158 + title: Gantt + project_id: 42 + view_kind: 1 + updated: '2024-03-18 15:14:13' + created: '2018-03-18 15:14:13' +- id: 159 + title: Table + project_id: 42 + view_kind: 2 + updated: '2024-03-18 15:14:13' + created: '2018-03-18 15:14:13' +- id: 160 + title: Kanban + project_id: 42 + view_kind: 3 + updated: '2024-03-18 15:14:13' + created: '2018-03-18 15:14:13' + bucket_configuration_mode: 1 diff --git a/pkg/db/fixtures/projects.yml b/pkg/db/fixtures/projects.yml index 65809edff..29c745d71 100644 --- a/pkg/db/fixtures/projects.yml +++ b/pkg/db/fixtures/projects.yml @@ -361,3 +361,24 @@ position: 40 updated: 2018-12-02 15:13:12 created: 2018-12-01 15:13:12 +# Parent project for hierarchy permission test (issue #2490) +- + id: 41 + title: HierarchyParent + description: Parent project for subtask permission hierarchy test + identifier: hier1 + owner_id: 6 + position: 41 + updated: 2018-12-02 15:13:12 + created: 2018-12-01 15:13:12 +# Child project — inherits access from parent 41 +- + id: 42 + title: HierarchyChild + description: Child project for subtask permission hierarchy test + identifier: hier2 + owner_id: 6 + parent_project_id: 41 + position: 42 + updated: 2018-12-02 15:13:12 + created: 2018-12-01 15:13:12 diff --git a/pkg/db/fixtures/task_relations.yml b/pkg/db/fixtures/task_relations.yml index af3818ed3..8e14c38d5 100644 --- a/pkg/db/fixtures/task_relations.yml +++ b/pkg/db/fixtures/task_relations.yml @@ -84,3 +84,16 @@ relation_kind: 'related' created_by_id: 1 created: 2018-12-01 15:13:12 +# Subtask relation: task 49 -> task 50 (cross parent/child project, hierarchy test) +- id: 15 + task_id: 49 + other_task_id: 50 + relation_kind: 'subtask' + created_by_id: 6 + created: 2018-12-01 15:13:12 +- id: 16 + task_id: 50 + other_task_id: 49 + relation_kind: 'parenttask' + created_by_id: 6 + created: 2018-12-01 15:13:12 diff --git a/pkg/db/fixtures/tasks.yml b/pkg/db/fixtures/tasks.yml index e8298d7ed..741f87802 100644 --- a/pkg/db/fixtures/tasks.yml +++ b/pkg/db/fixtures/tasks.yml @@ -429,3 +429,21 @@ index: 33 created: 2018-12-01 01:12:04 updated: 2018-12-01 01:12:04 +# Task in parent project 41 (hierarchy permission test) +- id: 49 + title: 'task #49 in hierarchy parent' + done: false + created_by_id: 6 + project_id: 41 + index: 1 + created: 2018-12-01 01:12:04 + updated: 2018-12-01 01:12:04 +# Task in child project 42 (hierarchy permission test) +- id: 50 + title: 'task #50 in hierarchy child' + done: false + created_by_id: 6 + project_id: 42 + index: 1 + created: 2018-12-01 01:12:04 + updated: 2018-12-01 01:12:04 diff --git a/pkg/db/fixtures/team_members.yml b/pkg/db/fixtures/team_members.yml index dfff71105..8d3d366a3 100644 --- a/pkg/db/fixtures/team_members.yml +++ b/pkg/db/fixtures/team_members.yml @@ -50,3 +50,6 @@ - team_id: 14 user_id: 11 created: 2018-12-01 15:13:12 +- team_id: 16 + user_id: 14 + created: 2018-12-01 15:13:12 diff --git a/pkg/db/fixtures/team_projects.yml b/pkg/db/fixtures/team_projects.yml index 45ecb541e..d42eda411 100644 --- a/pkg/db/fixtures/team_projects.yml +++ b/pkg/db/fixtures/team_projects.yml @@ -94,3 +94,10 @@ permission: 2 updated: 2018-12-02 15:13:12 created: 2018-12-01 15:13:12 +# Team 16 has read access on project 41 (the parent) — hierarchy permission test +- id: 16 + team_id: 16 + project_id: 41 + permission: 0 + updated: 2018-12-02 15:13:12 + created: 2018-12-01 15:13:12 diff --git a/pkg/db/fixtures/teams.yml b/pkg/db/fixtures/teams.yml index abe00c28b..b03c7d624 100644 --- a/pkg/db/fixtures/teams.yml +++ b/pkg/db/fixtures/teams.yml @@ -42,3 +42,6 @@ issuer: "https://some.issuer" is_public: true description: "This is a public team" +- id: 16 + name: testteam16_hierarchy_test + created_by_id: 6 diff --git a/pkg/models/project.go b/pkg/models/project.go index 03d8165fa..5d075780e 100644 --- a/pkg/models/project.go +++ b/pkg/models/project.go @@ -455,8 +455,6 @@ type projectOptions struct { } func getUserProjectsStatement(userID int64, search string) *builder.Builder { - dialect := db.GetDialect() - conds := []builder.Cond{ builder.Or( builder.Eq{"tm2.user_id": userID}, @@ -507,7 +505,7 @@ func getUserProjectsStatement(userID int64, search string) *builder.Builder { conds = append(conds, filterCond, parentCondition) } - return builder.Dialect(dialect). + return builder. Select("l.id, l.title, l.description, l.identifier, l.hex_color, l.owner_id, l.parent_project_id, l.is_archived, l.background_file_id, l.background_blur_hash, l.position, l.created, l.updated"). From("projects", "l"). Join("LEFT", "team_projects tl", "tl.project_id = l.id"). @@ -532,9 +530,28 @@ func accessibleProjectIDsSubquery(a web.Auth, column string) builder.Cond { return builder.Expr("1 = 0") } - return builder.In(column, - getUserProjectsStatement(u.ID, "").Select("l.id"), - ) + // Build the base query SQL from getUserProjectsStatement + baseQuery := getUserProjectsStatement(u.ID, "") + baseSQLStr, baseArgs, err := baseQuery.Select("l.id").ToSQL() + if err != nil { + return builder.Expr("1 = 0") + } + + // Wrap in a recursive CTE that walks child projects down the hierarchy. + // This ensures that if a user has access to a parent project, they also + // have access to all its child projects (matching the permission model + // used in getAllProjectsForUser). + recursiveSQL := column + ` IN ( + WITH RECURSIVE accessible_projects AS ( + ` + baseSQLStr + ` + UNION ALL + SELECT p.id FROM projects p + INNER JOIN accessible_projects ap ON p.parent_project_id = ap.id + ) + SELECT id FROM accessible_projects + )` + + return builder.Expr(recursiveSQL, baseArgs...) } func getAllProjectsForUser(s *xorm.Session, userID int64, opts *projectOptions) (projects []*Project, totalCount int64, err error) { diff --git a/pkg/models/project_test.go b/pkg/models/project_test.go index 88286cdd7..5a8ef1347 100644 --- a/pkg/models/project_test.go +++ b/pkg/models/project_test.go @@ -491,7 +491,7 @@ func TestProject_ReadAll(t *testing.T) { defer s.Close() projects, _, err := getAllProjectsForUser(s, 6, &projectOptions{}) require.NoError(t, err) - assert.Len(t, projects, 25) + assert.Len(t, projects, 27) }) t.Run("all projects for user", func(t *testing.T) { db.LoadAndAssertFixtures(t) diff --git a/pkg/models/task_relation_authz_test.go b/pkg/models/task_relation_authz_test.go index 03e2b70e0..720b8bcf2 100644 --- a/pkg/models/task_relation_authz_test.go +++ b/pkg/models/task_relation_authz_test.go @@ -26,6 +26,76 @@ import ( "github.com/stretchr/testify/require" ) +func TestAddRelatedTasksToTasks_InheritedProjectAccess(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + // User 14 has access to project 41 via team 16. + // Project 42 is a child of project 41. + // Task 49 is in project 41, task 50 is in project 42. + // Task 49 has a subtask relation to task 50. + // User 14 should see task 50 as a related task of task 49 + // because they inherit access to project 42 through project 41. + u := &user.User{ID: 14} + + taskMap := map[int64]*Task{ + 49: { + ID: 49, + ProjectID: 41, + RelatedTasks: make(RelatedTaskMap), + }, + } + taskIDs := []int64{49} + + err := addRelatedTasksToTasks(s, taskIDs, taskMap, u) + require.NoError(t, err) + + foundTask50 := false + for _, relatedTasks := range taskMap[49].RelatedTasks { + for _, rt := range relatedTasks { + if rt.ID == 50 { + foundTask50 = true + } + } + } + + assert.True(t, foundTask50, "Task 50 (child project 42) should be visible via inherited access from parent project 41") +} + +func TestAddRelatedTasksToTasks_NoAccessToHierarchy(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + // User 13 has NO access to project 41 or 42. + // They should NOT see task 50 even though a relation exists. + u := &user.User{ID: 13} + + taskMap := map[int64]*Task{ + 49: { + ID: 49, + ProjectID: 41, + RelatedTasks: make(RelatedTaskMap), + }, + } + taskIDs := []int64{49} + + err := addRelatedTasksToTasks(s, taskIDs, taskMap, u) + require.NoError(t, err) + + foundTask50 := false + for _, relatedTasks := range taskMap[49].RelatedTasks { + for _, rt := range relatedTasks { + if rt.ID == 50 { + foundTask50 = true + } + } + } + + assert.False(t, foundTask50, "Task 50 should NOT be visible to user 13 who has no access to the hierarchy") +} + func TestAddRelatedTasksToTasks_FiltersInaccessibleProjects(t *testing.T) { db.LoadAndAssertFixtures(t) s := db.NewSession()