diff --git a/pkg/models/project_permissions.go b/pkg/models/project_permissions.go index 213c9559f..ea22983af 100644 --- a/pkg/models/project_permissions.go +++ b/pkg/models/project_permissions.go @@ -250,7 +250,6 @@ func checkPermissionsForProjects(s *xorm.Session, u *user.User, projectIDs []int u.ID, u.ID, u.ID, - u.ID, } err = s.SQL(` @@ -274,12 +273,21 @@ WITH RECURSIVE FROM projects p INNER JOIN project_hierarchy ph ON p.id = ph.parent_project_id), + -- Calculate max team permission for each project/user combination + max_team_permissions AS ( + SELECT tl.project_id, + MAX(tl.permission) AS max_team_permission + FROM team_projects tl + INNER JOIN team_members tm ON tm.team_id = tl.team_id AND tm.user_id = ? + GROUP BY tl.project_id + ), + project_permissions AS (SELECT ph.id, ph.original_project_id, CASE WHEN p.owner_id = ? THEN 2 - WHEN COALESCE(ul.permission, 0) > COALESCE(tl.permission, 0) THEN ul.permission - ELSE COALESCE(tl.permission, 0) + WHEN COALESCE(ul.permission, 0) > COALESCE(mtp.max_team_permission, 0) THEN ul.permission + ELSE COALESCE(mtp.max_team_permission, 0) END AS project_permission, CASE WHEN p.owner_id = ? THEN 1 -- Direct project ownership @@ -289,9 +297,8 @@ WITH RECURSIVE LEFT JOIN projects p ON ph.id = p.id LEFT JOIN users_projects ul ON ul.project_id = ph.id AND ul.user_id = ? - LEFT JOIN team_projects tl ON tl.project_id = ph.id - LEFT JOIN team_members tm ON tm.team_id = tl.team_id AND tm.user_id = ? - WHERE p.owner_id = ? OR ul.user_id = ? OR tm.user_id = ?) + LEFT JOIN max_team_permissions mtp ON mtp.project_id = ph.id + WHERE p.owner_id = ? OR ul.user_id = ? OR mtp.max_team_permission IS NOT NULL) SELECT ph.original_project_id AS id, COALESCE(MAX(pp.project_permission), -1) AS max_permission diff --git a/pkg/models/project_permissions_multiple_teams_test.go b/pkg/models/project_permissions_multiple_teams_test.go new file mode 100644 index 000000000..04efaaeeb --- /dev/null +++ b/pkg/models/project_permissions_multiple_teams_test.go @@ -0,0 +1,702 @@ +// 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" + + "code.vikunja.io/api/pkg/db" + "code.vikunja.io/api/pkg/user" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "xorm.io/xorm" +) + +// TestProjectPermissions_OwnerInMultipleTeamsWithDifferentPermissions tests the scenario where: +// - User A creates a project (becomes owner) +// - User A is part of two teams Y and Z +// - Project is shared with team Y with admin permissions +// - Project is shared with team Z with read-only permissions +// - User A should still have owner/admin permissions (highest permission should apply) +func TestProjectPermissions_OwnerInMultipleTeamsWithDifferentPermissions(t *testing.T) { + // Setup: Load fixtures and create a database session + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + // User A (we'll use user 1 from fixtures - needs Username for GetFromAuth) + userA := &user.User{ + ID: 1, + Username: "user1", + } + + // Create a new project owned by User A + project := &Project{ + Title: "Test Project for Multiple Teams", + Description: "Testing permissions when owner is in multiple teams", + } + err := project.Create(s, userA) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + + // Verify User A has admin permissions as the owner + t.Run("owner has admin permissions before sharing", func(t *testing.T) { + s = db.NewSession() + defer s.Close() + + canRead, maxPerm, err := project.CanRead(s, userA) + require.NoError(t, err) + assert.True(t, canRead, "Owner should be able to read their project") + assert.Equal(t, int(PermissionAdmin), maxPerm, "Owner should have admin permission") + + canWrite, err := project.CanWrite(s, userA) + require.NoError(t, err) + assert.True(t, canWrite, "Owner should be able to write to their project") + + isAdmin, err := project.IsAdmin(s, userA) + require.NoError(t, err) + assert.True(t, isAdmin, "Owner should be admin of their project") + }) + + prepareSharingY := func(sess *xorm.Session) { + // Create Team Y - User A will be automatically added as a member when creating the team + teamY := &Team{ + Name: "Team Y", + Description: "Team with admin permissions", + } + err = teamY.Create(sess, userA) + require.NoError(t, err) + require.NoError(t, sess.Commit()) + + // Share project with Team Y (admin permissions) + teamProjectY := &TeamProject{ + TeamID: teamY.ID, + ProjectID: project.ID, + Permission: PermissionAdmin, + } + err = teamProjectY.Create(sess, userA) + require.NoError(t, err) + require.NoError(t, sess.Commit()) + } + + // Setup sharing with Team Y + s = db.NewSession() + prepareSharingY(s) + s.Close() + + // Verify User A still has admin permissions after sharing with Team Y + t.Run("owner has admin permissions after sharing with team Y (admin)", func(t *testing.T) { + s = db.NewSession() + defer s.Close() + + canRead, maxPerm, err := project.CanRead(s, userA) + require.NoError(t, err) + assert.True(t, canRead, "Owner should be able to read their project after sharing with Team Y") + assert.Equal(t, int(PermissionAdmin), maxPerm, "Owner should still have admin permission after sharing with Team Y") + + canWrite, err := project.CanWrite(s, userA) + require.NoError(t, err) + assert.True(t, canWrite, "Owner should be able to write to their project after sharing with Team Y") + + isAdmin, err := project.IsAdmin(s, userA) + require.NoError(t, err) + assert.True(t, isAdmin, "Owner should still be admin after sharing with Team Y") + }) + + prepareTeamSharedMultiple := func(sess *xorm.Session) { + // Create Team Z - User A will be automatically added as a member when creating the team + teamZ := &Team{ + Name: "Team Z", + Description: "Team with read-only permissions", + } + err = teamZ.Create(sess, userA) + require.NoError(t, err) + require.NoError(t, sess.Commit()) + + // Share project with Team Z (read-only permissions) + teamProjectZ := &TeamProject{ + TeamID: teamZ.ID, + ProjectID: project.ID, + Permission: PermissionRead, + } + err = teamProjectZ.Create(sess, userA) + require.NoError(t, err) + require.NoError(t, sess.Commit()) + } + + // Setup sharing with Team Z + s = db.NewSession() + prepareTeamSharedMultiple(s) + s.Close() + + // Verify User A STILL has admin permissions after sharing with Team Z + // user should retain highest permission (owner/admin), not be downgraded to read-only + t.Run("owner has admin permissions after sharing with team Z (read-only)", func(t *testing.T) { + s = db.NewSession() + defer s.Close() + + canRead, maxPerm, err := project.CanRead(s, userA) + require.NoError(t, err) + assert.True(t, canRead, "Owner should be able to read their project after sharing with Team Z") + assert.Equal(t, int(PermissionAdmin), maxPerm, "Owner should STILL have admin permission (not downgraded to read-only)") + + canWrite, err := project.CanWrite(s, userA) + require.NoError(t, err) + assert.True(t, canWrite, "Owner should STILL be able to write to their project (not downgraded to read-only)") + + isAdmin, err := project.IsAdmin(s, userA) + require.NoError(t, err) + assert.True(t, isAdmin, "Owner should STILL be admin (not downgraded to read-only)") + }) +} + +// TestProjectPermissions_OwnerInMultipleTeamsAdminThenWrite tests the variant where: +// - User A creates a project +// - Project is shared with team Y with admin permissions +// - Project is shared with team Z with write permissions +// - User A should still have admin permissions (not downgraded to write) +func TestProjectPermissions_OwnerInMultipleTeamsAdminThenWrite(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + userA := &user.User{ + ID: 1, + Username: "user1", + } + + // Create a new project owned by User A + project := &Project{ + Title: "Test Project Admin Then Write", + Description: "Testing permissions when owner is in teams with admin then write", + } + err := project.Create(s, userA) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + + // Create Team Y with admin permissions - User A will be automatically added as a member + teamY := &Team{ + Name: "Team Y Admin", + } + err = teamY.Create(s, userA) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + + teamProjectY := &TeamProject{ + TeamID: teamY.ID, + ProjectID: project.ID, + Permission: PermissionAdmin, + } + err = teamProjectY.Create(s, userA) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + + // Create Team Z with write permissions - User A will be automatically added as a member + teamZ := &Team{ + Name: "Team Z Write", + } + err = teamZ.Create(s, userA) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + + teamProjectZ := &TeamProject{ + TeamID: teamZ.ID, + ProjectID: project.ID, + Permission: PermissionWrite, + } + err = teamProjectZ.Create(s, userA) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + + // Test: User A should still have admin permissions (not downgraded to write) + t.Run("owner has admin permissions after sharing with admin team then write team", func(t *testing.T) { + s = db.NewSession() + defer s.Close() + + canRead, maxPerm, err := project.CanRead(s, userA) + require.NoError(t, err) + assert.True(t, canRead) + assert.Equal(t, int(PermissionAdmin), maxPerm, "Owner should have admin permission, not downgraded to write") + + isAdmin, err := project.IsAdmin(s, userA) + require.NoError(t, err) + assert.True(t, isAdmin, "Owner should still be admin, not downgraded to write") + }) +} + +// TestProjectPermissions_NonOwnerInMultipleTeams tests the scenario where: +// - User B (not the owner) is part of two teams +// - Team Y has admin permissions on a project +// - Team Z has read-only permissions on the same project +// - User B should have admin permissions (highest of the two) +func TestProjectPermissions_NonOwnerInMultipleTeams(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + // User 1 is the owner + owner := &user.User{ + ID: 1, + Username: "user1", + } + // User 2 will be in multiple teams + userB := &user.User{ + ID: 2, + Username: "user2", + } + + // Create a new project owned by User 1 + project := &Project{ + Title: "Test Project Non-Owner Multiple Teams", + Description: "Testing permissions for non-owner in multiple teams", + } + err := project.Create(s, owner) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + + // Create Team Y with admin permissions + teamY := &Team{ + Name: "Team Y Admin for Non-Owner", + } + err = teamY.Create(s, owner) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + + // Add User B to Team Y + teamMemberY := &TeamMember{ + TeamID: teamY.ID, + Username: "user2", + } + err = teamMemberY.Create(s, owner) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + + // Share project with Team Y (admin) + teamProjectY := &TeamProject{ + TeamID: teamY.ID, + ProjectID: project.ID, + Permission: PermissionAdmin, + } + err = teamProjectY.Create(s, owner) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + + // Verify User B has admin permissions through Team Y + t.Run("non-owner has admin through team Y", func(t *testing.T) { + s = db.NewSession() + defer s.Close() + + canRead, maxPerm, err := project.CanRead(s, userB) + require.NoError(t, err) + assert.True(t, canRead) + assert.Equal(t, int(PermissionAdmin), maxPerm) + + isAdmin, err := project.IsAdmin(s, userB) + require.NoError(t, err) + assert.True(t, isAdmin) + }) + + // Create Team Z with read-only permissions + teamZ := &Team{ + Name: "Team Z Read for Non-Owner", + } + err = teamZ.Create(s, owner) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + + // Add User B to Team Z + teamMemberZ := &TeamMember{ + TeamID: teamZ.ID, + Username: "user2", + } + err = teamMemberZ.Create(s, owner) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + + // Share project with Team Z (read-only) + teamProjectZ := &TeamProject{ + TeamID: teamZ.ID, + ProjectID: project.ID, + Permission: PermissionRead, + } + err = teamProjectZ.Create(s, owner) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + + // Test: User B should still have admin permissions (highest of Team Y and Team Z) + t.Run("non-owner retains admin after adding to read-only team Z", func(t *testing.T) { + s = db.NewSession() + defer s.Close() + + canRead, maxPerm, err := project.CanRead(s, userB) + require.NoError(t, err) + assert.True(t, canRead) + assert.Equal(t, int(PermissionAdmin), maxPerm, "Non-owner should have admin (highest permission), not downgraded to read") + + isAdmin, err := project.IsAdmin(s, userB) + require.NoError(t, err) + assert.True(t, isAdmin, "Non-owner should still have admin permission, not downgraded to read") + }) +} + +// TestProjectPermissions_OrderOfSharingDoesNotMatter tests that the order in which +// teams are granted access doesn't affect the final permission level +func TestProjectPermissions_OrderOfSharingDoesNotMatter(t *testing.T) { + db.LoadAndAssertFixtures(t) + + owner := &user.User{ + ID: 1, + Username: "user1", + } + userB := &user.User{ + ID: 2, + Username: "user2", + } + + // Test 1: Share read-only first, then admin + t.Run("share read-only first then admin", func(t *testing.T) { + s := db.NewSession() + defer s.Close() + + project := &Project{ + Title: "Test Order: Read then Admin", + } + err := project.Create(s, owner) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + + // Create and share with read-only team first + teamRead := &Team{Name: "Read First Team"} + err = teamRead.Create(s, owner) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + + teamMemberRead := &TeamMember{TeamID: teamRead.ID, Username: "user2"} + err = teamMemberRead.Create(s, owner) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + + teamProjectRead := &TeamProject{ + TeamID: teamRead.ID, + ProjectID: project.ID, + Permission: PermissionRead, + } + err = teamProjectRead.Create(s, owner) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + + // Now share with admin team + teamAdmin := &Team{Name: "Admin Second Team"} + err = teamAdmin.Create(s, owner) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + + teamMemberAdmin := &TeamMember{TeamID: teamAdmin.ID, Username: "user2"} + err = teamMemberAdmin.Create(s, owner) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + + teamProjectAdmin := &TeamProject{ + TeamID: teamAdmin.ID, + ProjectID: project.ID, + Permission: PermissionAdmin, + } + err = teamProjectAdmin.Create(s, owner) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + + // Verify user has admin permission + s = db.NewSession() + defer s.Close() + + canRead, maxPerm, err := project.CanRead(s, userB) + require.NoError(t, err) + assert.True(t, canRead) + assert.Equal(t, int(PermissionAdmin), maxPerm, "User should have admin even though read-only was shared first") + + isAdmin, err := project.IsAdmin(s, userB) + require.NoError(t, err) + assert.True(t, isAdmin) + }) + + // Test 2: Share admin first, then read-only (the reported bug scenario) + t.Run("share admin first then read-only - REPORTED BUG", func(t *testing.T) { + s := db.NewSession() + defer s.Close() + + project := &Project{ + Title: "Test Order: Admin then Read", + } + err := project.Create(s, owner) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + + // Create and share with admin team first + teamAdmin := &Team{Name: "Admin First Team"} + err = teamAdmin.Create(s, owner) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + + teamMemberAdmin := &TeamMember{TeamID: teamAdmin.ID, Username: "user2"} + err = teamMemberAdmin.Create(s, owner) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + + teamProjectAdmin := &TeamProject{ + TeamID: teamAdmin.ID, + ProjectID: project.ID, + Permission: PermissionAdmin, + } + err = teamProjectAdmin.Create(s, owner) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + + // Now share with read-only team + teamRead := &Team{Name: "Read Second Team"} + err = teamRead.Create(s, owner) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + + teamMemberRead := &TeamMember{TeamID: teamRead.ID, Username: "user2"} + err = teamMemberRead.Create(s, owner) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + + teamProjectRead := &TeamProject{ + TeamID: teamRead.ID, + ProjectID: project.ID, + Permission: PermissionRead, + } + err = teamProjectRead.Create(s, owner) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + + // Verify user STILL has admin permission (not downgraded to read) + s = db.NewSession() + defer s.Close() + + canRead, maxPerm, err := project.CanRead(s, userB) + require.NoError(t, err) + assert.True(t, canRead) + assert.Equal(t, int(PermissionAdmin), maxPerm, "User should STILL have admin even though read-only was shared last") + + isAdmin, err := project.IsAdmin(s, userB) + require.NoError(t, err) + assert.True(t, isAdmin, "User should STILL be admin even though read-only was shared last") + }) +} + +// TestProjectPermissions_UserInMultipleTeamsWithSamePermission tests the edge case where: +// - A user belongs to multiple teams that all grant the same permission level +// - The effective permission should equal that permission level (not more, not less) +// - This verifies the MAX function works correctly when all permissions are equal +func TestProjectPermissions_UserInMultipleTeamsWithSamePermission(t *testing.T) { + db.LoadAndAssertFixtures(t) + + owner := &user.User{ + ID: 1, + Username: "user1", + } + userB := &user.User{ + ID: 2, + Username: "user2", + } + + // Test: User in two teams, both with read-only permission + t.Run("user in multiple teams with same read-only permission", func(t *testing.T) { + s := db.NewSession() + defer s.Close() + + // Create project + project := &Project{ + Title: "Test Multiple Teams Same Permission", + } + err := project.Create(s, owner) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + + // Create first team with read-only permission + team1 := &Team{Name: "Team 1 Read"} + err = team1.Create(s, owner) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + + teamMember1 := &TeamMember{TeamID: team1.ID, Username: "user2"} + err = teamMember1.Create(s, owner) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + + teamProject1 := &TeamProject{ + TeamID: team1.ID, + ProjectID: project.ID, + Permission: PermissionRead, + } + err = teamProject1.Create(s, owner) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + + // Create second team with read-only permission + team2 := &Team{Name: "Team 2 Read"} + err = team2.Create(s, owner) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + + teamMember2 := &TeamMember{TeamID: team2.ID, Username: "user2"} + err = teamMember2.Create(s, owner) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + + teamProject2 := &TeamProject{ + TeamID: team2.ID, + ProjectID: project.ID, + Permission: PermissionRead, + } + err = teamProject2.Create(s, owner) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + + // Verify user has read permission (not more, not less) + s = db.NewSession() + defer s.Close() + + canRead, maxPerm, err := project.CanRead(s, userB) + require.NoError(t, err) + assert.True(t, canRead, "User should be able to read") + assert.Equal(t, int(PermissionRead), maxPerm, "User should have exactly read permission when all teams grant read-only") + + canWrite, err := project.CanWrite(s, userB) + require.NoError(t, err) + assert.False(t, canWrite, "User should NOT be able to write with only read permissions") + + isAdmin, err := project.IsAdmin(s, userB) + require.NoError(t, err) + assert.False(t, isAdmin, "User should NOT be admin with only read permissions") + }) + + // Test: User in two teams, both with write permission + t.Run("user in multiple teams with same write permission", func(t *testing.T) { + s := db.NewSession() + defer s.Close() + + // Create project + project := &Project{ + Title: "Test Multiple Teams Same Write Permission", + } + err := project.Create(s, owner) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + + // Create first team with write permission + team1 := &Team{Name: "Team 1 Write"} + err = team1.Create(s, owner) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + + teamMember1 := &TeamMember{TeamID: team1.ID, Username: "user2"} + err = teamMember1.Create(s, owner) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + + teamProject1 := &TeamProject{ + TeamID: team1.ID, + ProjectID: project.ID, + Permission: PermissionWrite, + } + err = teamProject1.Create(s, owner) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + + // Create second team with write permission + team2 := &Team{Name: "Team 2 Write"} + err = team2.Create(s, owner) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + + teamMember2 := &TeamMember{TeamID: team2.ID, Username: "user2"} + err = teamMember2.Create(s, owner) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + + teamProject2 := &TeamProject{ + TeamID: team2.ID, + ProjectID: project.ID, + Permission: PermissionWrite, + } + err = teamProject2.Create(s, owner) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + + // Verify user has write permission + s = db.NewSession() + defer s.Close() + + canRead, maxPerm, err := project.CanRead(s, userB) + require.NoError(t, err) + assert.True(t, canRead, "User should be able to read") + assert.Equal(t, int(PermissionWrite), maxPerm, "User should have exactly write permission when all teams grant write") + + canWrite, err := project.CanWrite(s, userB) + require.NoError(t, err) + assert.True(t, canWrite, "User should be able to write") + + isAdmin, err := project.IsAdmin(s, userB) + require.NoError(t, err) + assert.False(t, isAdmin, "User should NOT be admin with only write permissions") + }) +}