From 32b7fdd73e93057fd025ee1cee51131d80ffb53e Mon Sep 17 00:00:00 2001 From: kolaente Date: Sun, 30 Aug 2026 17:14:45 +0200 Subject: [PATCH] fix(tasks): validate position target views --- pkg/models/task_position.go | 70 ++++++- pkg/models/task_position_view_test.go | 254 ++++++++++++++++++++++++++ 2 files changed, 323 insertions(+), 1 deletion(-) create mode 100644 pkg/models/task_position_view_test.go diff --git a/pkg/models/task_position.go b/pkg/models/task_position.go index ee6f0b1b7..e168af110 100644 --- a/pkg/models/task_position.go +++ b/pkg/models/task_position.go @@ -29,6 +29,7 @@ import ( "code.vikunja.io/api/pkg/db" "code.vikunja.io/api/pkg/events" "code.vikunja.io/api/pkg/log" + "code.vikunja.io/api/pkg/user" "code.vikunja.io/api/pkg/web" ) @@ -60,7 +61,74 @@ func (tp *TaskPosition) TableName() string { func (tp *TaskPosition) CanUpdate(s *xorm.Session, a web.Auth) (bool, error) { t := &Task{ID: tp.TaskID} - return t.CanUpdate(s, a) + can, err := t.CanUpdate(s, a) + if err != nil { + return false, err + } + if !can { + return false, nil + } + + view, err := GetProjectViewByID(s, tp.ProjectViewID) + if err != nil { + return false, err + } + + // Saved-filter views also require the task to match the filter. + if filterID := GetSavedFilterIDFromProjectID(view.ProjectID); filterID > 0 { + return tp.canPositionTaskInSavedFilterView(s, a, view, filterID) + } + + task, err := GetTaskByIDSimple(s, tp.TaskID) + if err != nil { + return false, err + } + if view.ProjectID != task.ProjectID { + return false, nil + } + canRead, _, err := view.CanRead(s, a) + if err != nil { + return false, err + } + return canRead, nil +} + +// Reuse fetch matching so relative dates use the saved filter owner's timezone. +func (tp *TaskPosition) canPositionTaskInSavedFilterView(s *xorm.Session, a web.Auth, view *ProjectView, filterID int64) (bool, error) { + sf := &SavedFilter{ID: filterID} + canRead, _, err := sf.CanRead(s, a) + if err != nil { + return false, err + } + if !canRead { + return false, nil + } + + filter, err := GetSavedFilterSimpleByID(s, filterID) + if err != nil { + return false, err + } + task, err := GetTaskByIDSimple(s, tp.TaskID) + if err != nil { + return false, err + } + + accessByProject, _, err := getProjectAccessForTasks(s, []*Task{&task}) + if err != nil { + return false, err + } + + owner, err := user.GetUserByID(s, filter.OwnerID) + if err != nil { + return false, err + } + + matched, err := matchTasksToViewsOfFilter(s, []*Task{&task}, filter, []*ProjectView{view}, accessByProject, owner.Timezone) + if err != nil { + return false, err + } + _, matches := matched[task.ID] + return matches, nil } func (tp *TaskPosition) refresh(s *xorm.Session) (err error) { diff --git a/pkg/models/task_position_view_test.go b/pkg/models/task_position_view_test.go new file mode 100644 index 000000000..2a679770f --- /dev/null +++ b/pkg/models/task_position_view_test.go @@ -0,0 +1,254 @@ +// Vikunja is a to-do list application to facilitate your life. +// Copyright 2018-present Vikunja and contributors. All rights reserved. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package models + +import ( + "testing" + "time" + + "code.vikunja.io/api/pkg/db" + "code.vikunja.io/api/pkg/user" + + "xorm.io/xorm" + + "github.com/stretchr/testify/require" +) + +// Guards foreign and unrelated view positioning (GHSA-w39f-h553-h2mx). +func TestTaskPositionCanUpdateValidatesView(t *testing.T) { + setup := func(t *testing.T) (s *xorm.Session, task *Task, view, otherView *ProjectView, writer *user.User) { + t.Helper() + db.LoadAndAssertFixtures(t) + s = db.NewSession() + + owner := &user.User{ID: 1} + writer = &user.User{ID: 2} + + proj := &Project{Title: "position-validation project"} + require.NoError(t, proj.Create(s, owner)) + _, err := s.Insert(&ProjectUser{UserID: writer.ID, ProjectID: proj.ID, Permission: PermissionWrite}) + require.NoError(t, err) + + task = &Task{Title: "position-validation task", ProjectID: proj.ID} + require.NoError(t, task.Create(s, owner)) + + view = &ProjectView{Title: "position-validation view", ProjectID: proj.ID, ViewKind: ProjectViewKindList, Position: 1} + _, err = s.Insert(view) + require.NoError(t, err) + + otherProj := &Project{Title: "position-validation other project"} + require.NoError(t, otherProj.Create(s, owner)) + otherView = &ProjectView{Title: "position-validation other view", ProjectID: otherProj.ID, ViewKind: ProjectViewKindList, Position: 1} + _, err = s.Insert(otherView) + require.NoError(t, err) + + require.NoError(t, s.Commit()) + return + } + + t.Run("valid write to a view of the task's project", func(t *testing.T) { + s, task, view, _, writer := setup(t) + defer s.Close() + + tp := &TaskPosition{TaskID: task.ID, ProjectViewID: view.ID, Position: 123} + can, err := tp.CanUpdate(s, writer) + require.NoError(t, err) + require.True(t, can) + + require.NoError(t, tp.Update(s, writer)) + db.AssertExists(t, "task_positions", map[string]interface{}{ + "task_id": task.ID, + "project_view_id": view.ID, + "position": 123, + }, false) + }) + + t.Run("view of a foreign project is denied and writes no row", func(t *testing.T) { + s, task, _, otherView, writer := setup(t) + defer s.Close() + + tp := &TaskPosition{TaskID: task.ID, ProjectViewID: otherView.ID, Position: 123} + can, err := tp.CanUpdate(s, writer) + require.NoError(t, err) + require.False(t, can, "a view of a project the caller cannot access must be rejected") + + db.AssertMissing(t, "task_positions", map[string]interface{}{ + "task_id": task.ID, + "project_view_id": otherView.ID, + }) + }) + + t.Run("saved filter: matching task of the owner is allowed", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + owner := &user.User{ID: 1} + proj := &Project{Title: "position-validation filter project"} + require.NoError(t, proj.Create(s, owner)) + task := &Task{Title: "position-validation filter task", ProjectID: proj.ID} + require.NoError(t, task.Create(s, owner)) + + sf := &SavedFilter{ + Title: "position-validation filter", + Filters: &TaskCollection{Filter: "title like \"position-validation filter task\""}, + } + require.NoError(t, sf.Create(s, owner)) + require.NoError(t, sf.Update(s, owner)) + + view := &ProjectView{} + exists, err := s.Where("project_id = ?", getProjectIDFromSavedFilterID(sf.ID)).Get(view) + require.NoError(t, err) + require.True(t, exists) + + tp := &TaskPosition{TaskID: task.ID, ProjectViewID: view.ID, Position: 42} + can, err := tp.CanUpdate(s, owner) + require.NoError(t, err) + require.True(t, can, "the owner may position a task which matches the filter") + }) + + t.Run("saved filter: non-owner is denied", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + owner := &user.User{ID: 1} + writer := &user.User{ID: 2} + proj := &Project{Title: "position-validation foreign filter project"} + require.NoError(t, proj.Create(s, owner)) + _, err := s.Insert(&ProjectUser{UserID: writer.ID, ProjectID: proj.ID, Permission: PermissionWrite}) + require.NoError(t, err) + task := &Task{Title: "position-validation foreign filter task", ProjectID: proj.ID} + require.NoError(t, task.Create(s, owner)) + + sf := &SavedFilter{ + Title: "position-validation foreign filter", + Filters: &TaskCollection{Filter: "title like \"position-validation foreign filter task\""}, + } + require.NoError(t, sf.Create(s, owner)) + require.NoError(t, sf.Update(s, owner)) + + view := &ProjectView{} + exists, err := s.Where("project_id = ?", getProjectIDFromSavedFilterID(sf.ID)).Get(view) + require.NoError(t, err) + require.True(t, exists) + + tp := &TaskPosition{TaskID: task.ID, ProjectViewID: view.ID, Position: 42} + can, err := tp.CanUpdate(s, writer) + require.NoError(t, err) + require.False(t, can, "a saved-filter view of another user must be rejected") + }) + + t.Run("saved filter: nonmatching task is denied", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + owner := &user.User{ID: 1} + proj := &Project{Title: "position-validation mismatch project"} + require.NoError(t, proj.Create(s, owner)) + task := &Task{Title: "position-validation mismatch task", ProjectID: proj.ID} + require.NoError(t, task.Create(s, owner)) + + sf := &SavedFilter{ + Title: "position-validation mismatch filter", + Filters: &TaskCollection{Filter: "title like \"something else entirely\""}, + } + require.NoError(t, sf.Create(s, owner)) + require.NoError(t, sf.Update(s, owner)) + + view := &ProjectView{} + exists, err := s.Where("project_id = ?", getProjectIDFromSavedFilterID(sf.ID)).Get(view) + require.NoError(t, err) + require.True(t, exists) + + tp := &TaskPosition{TaskID: task.ID, ProjectViewID: view.ID, Position: 42} + can, err := tp.CanUpdate(s, owner) + require.NoError(t, err) + require.False(t, can, "a task which does not match the filter must be rejected") + }) + + t.Run("saved filter: relative dates resolve in the owner's timezone", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + owner := &user.User{ID: 1} + // UTC+14 makes this match only when the owner's timezone is used. + _, err := s.Cols("timezone").Where("id = ?", owner.ID).Update(&user.User{Timezone: "Pacific/Kiritimati"}) + require.NoError(t, err) + + proj := &Project{Title: "position-validation tz project"} + require.NoError(t, proj.Create(s, owner)) + task := &Task{ + Title: "position-validation tz task", + ProjectID: proj.ID, + DueDate: time.Date(2026, 8, 29, 23, 30, 0, 0, time.UTC), + } + require.NoError(t, task.Create(s, owner)) + + sf := &SavedFilter{ + Title: "position-validation tz filter", + Filters: &TaskCollection{Filter: "due_date > 2026-08-30"}, + } + require.NoError(t, sf.Create(s, owner)) + require.NoError(t, sf.Update(s, owner)) + + view := &ProjectView{} + exists, err := s.Where("project_id = ?", getProjectIDFromSavedFilterID(sf.ID)).Get(view) + require.NoError(t, err) + require.True(t, exists) + + tp := &TaskPosition{TaskID: task.ID, ProjectViewID: view.ID, Position: 42} + can, err := tp.CanUpdate(s, owner) + require.NoError(t, err) + require.True(t, can, + "the filter must be evaluated with the owner's timezone, matching the view the user actually sees") + }) + + t.Run("saved filter: owner lookup errors are returned", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + owner := &user.User{ID: 1} + proj := &Project{Title: "position-validation missing owner"} + require.NoError(t, proj.Create(s, owner)) + task := &Task{Title: "position-validation missing owner task", ProjectID: proj.ID} + require.NoError(t, task.Create(s, owner)) + + sf := &SavedFilter{ + Title: "position-validation missing owner filter", + Filters: &TaskCollection{Filter: "title like \"position-validation missing owner task\""}, + } + require.NoError(t, sf.Create(s, owner)) + require.NoError(t, sf.Update(s, owner)) + + view := &ProjectView{} + exists, err := s.Where("project_id = ?", getProjectIDFromSavedFilterID(sf.ID)).Get(view) + require.NoError(t, err) + require.True(t, exists) + + _, err = s.ID(owner.ID).Delete(&user.User{}) + require.NoError(t, err) + + tp := &TaskPosition{TaskID: task.ID, ProjectViewID: view.ID, Position: 42} + can, err := tp.CanUpdate(s, owner) + require.Error(t, err) + require.False(t, can) + }) +}