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.
This commit is contained in:
parent
856011f132
commit
ac76bce5cd
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
Loading…
Reference in New Issue