fix(labels): report owner-level max_permission
Label writes/deletes are owner-only (CanUpdate/CanDelete), but hasAccessToLabel derived max_permission from the accessible task's permission with a read fallback for the creator branch — so owners showed as read-only and a task-admin reading a label via that task showed as a label admin. Derive it from ownership instead: owner -> admin, otherwise read. Corrects the value CanRead returns for both v1's x-max-permission header and the new v2 max_permission body field.
This commit is contained in:
parent
e22e169fb9
commit
af2482aab2
|
|
@ -107,38 +107,15 @@ func (l *Label) hasAccessToLabel(s *xorm.Session, a web.Auth) (has bool, maxPerm
|
|||
return
|
||||
}
|
||||
|
||||
// maxPermission is derived only from label_tasks rows whose task is
|
||||
// actually accessible. The pre-fix code used Get(ll) against the
|
||||
// unrestricted LEFT JOIN, so it could return an inaccessible row and
|
||||
// yield a wrong (or errored) permission.
|
||||
accessibleTaskIDs := []int64{}
|
||||
err = s.Table("label_tasks").
|
||||
Join("INNER", "tasks", "tasks.id = label_tasks.task_id").
|
||||
Where(builder.And(
|
||||
builder.Eq{"label_tasks.label_id": l.ID},
|
||||
accessibleProjects,
|
||||
)).
|
||||
Cols("label_tasks.task_id").
|
||||
Find(&accessibleTaskIDs)
|
||||
// Writes and deletes are owner-only (CanUpdate/CanDelete), so the caller's
|
||||
// max permission is admin for the owner and read for anyone else who can see it.
|
||||
owner, err := l.isLabelOwner(s, a)
|
||||
if err != nil {
|
||||
return
|
||||
}
|
||||
|
||||
for _, taskID := range accessibleTaskIDs {
|
||||
t := &Task{ID: taskID}
|
||||
_, taskPermission, tErr := t.CanRead(s, a)
|
||||
if tErr != nil {
|
||||
err = tErr
|
||||
return
|
||||
}
|
||||
if taskPermission > maxPermission {
|
||||
maxPermission = taskPermission
|
||||
}
|
||||
}
|
||||
|
||||
// Creator-branch fallback: access came from created_by_id with no
|
||||
// accessible task to derive a permission from.
|
||||
if len(accessibleTaskIDs) == 0 {
|
||||
if owner {
|
||||
maxPermission = int(PermissionAdmin)
|
||||
} else {
|
||||
maxPermission = int(PermissionRead)
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -228,7 +228,7 @@ func TestLabel_ReadOne(t *testing.T) {
|
|||
},
|
||||
auth: &user.User{ID: 1},
|
||||
assertMaxPermission: true,
|
||||
wantMaxPermission: int(PermissionRead),
|
||||
wantMaxPermission: int(PermissionAdmin),
|
||||
},
|
||||
{
|
||||
name: "Get nonexistant label",
|
||||
|
|
@ -249,8 +249,8 @@ func TestLabel_ReadOne(t *testing.T) {
|
|||
auth: &user.User{ID: 1},
|
||||
},
|
||||
{
|
||||
// Label 4 is attached to tasks in project 1 (user 1 is admin),
|
||||
// so the accessible-tasks iteration must yield PermissionAdmin.
|
||||
// Label 4 is owned by user 2; user 1 can read it via a shared task
|
||||
// but is not the owner, so max permission is read.
|
||||
name: "Get label #4 - other user",
|
||||
fields: fields{
|
||||
ID: 4,
|
||||
|
|
@ -276,7 +276,7 @@ func TestLabel_ReadOne(t *testing.T) {
|
|||
},
|
||||
auth: &user.User{ID: 1},
|
||||
assertMaxPermission: true,
|
||||
wantMaxPermission: int(PermissionAdmin),
|
||||
wantMaxPermission: int(PermissionRead),
|
||||
},
|
||||
{
|
||||
// PoC for GHSA-hj5c-mhh2-g7jq: label 6 is reachable only via task
|
||||
|
|
@ -304,12 +304,12 @@ func TestLabel_ReadOne(t *testing.T) {
|
|||
},
|
||||
auth: &user.User{ID: 1},
|
||||
assertMaxPermission: true,
|
||||
wantMaxPermission: int(PermissionRead),
|
||||
wantMaxPermission: int(PermissionAdmin),
|
||||
},
|
||||
{
|
||||
// Label 8's only label_tasks row points at inaccessible task 34,
|
||||
// so access must come from the creator branch and the
|
||||
// maxPermission fallback to PermissionRead must kick in.
|
||||
// Label 8's only label_tasks row points at inaccessible task 34, so
|
||||
// access comes from the creator branch; as the owner, user 1's max
|
||||
// permission is admin.
|
||||
name: "creator can read own label only attached to inaccessible task",
|
||||
fields: fields{
|
||||
ID: 8,
|
||||
|
|
@ -324,7 +324,7 @@ func TestLabel_ReadOne(t *testing.T) {
|
|||
},
|
||||
auth: &user.User{ID: 1},
|
||||
assertMaxPermission: true,
|
||||
wantMaxPermission: int(PermissionRead),
|
||||
wantMaxPermission: int(PermissionAdmin),
|
||||
},
|
||||
{
|
||||
// Non-creator must not be able to read an unattached label owned
|
||||
|
|
|
|||
Loading…
Reference in New Issue