From 54c7c4aef2fbdf7d4c04630d75cd36a0d121daec Mon Sep 17 00:00:00 2001 From: kolaente Date: Wed, 4 Mar 2026 20:13:59 +0100 Subject: [PATCH] refactor: move ListUsers tests from pkg/user to pkg/models The ListUsers function now references team_members and teams tables via a subquery for external team discoverability. The pkg/user test environment only syncs user tables, so these tests need to run in pkg/models which has the full schema and all fixtures. Also adds new tests for the external team discoverability bypass directly in the models package alongside the moved tests. --- pkg/models/user_list_test.go | 242 +++++++++++++++++++++++++++++++++++ pkg/user/user_test.go | 148 --------------------- 2 files changed, 242 insertions(+), 148 deletions(-) create mode 100644 pkg/models/user_list_test.go diff --git a/pkg/models/user_list_test.go b/pkg/models/user_list_test.go new file mode 100644 index 000000000..cc2301dd5 --- /dev/null +++ b/pkg/models/user_list_test.go @@ -0,0 +1,242 @@ +// 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/builder" +) + +func TestListUsers(t *testing.T) { + user1 := &user.User{ID: 1} + user10 := &user.User{ID: 10} + + t.Run("normal", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + all, err := user.ListUsers(s, "user1", user1, nil) + require.NoError(t, err) + assert.NotEmpty(t, all) + assert.Equal(t, "user1", all[0].Username) + }) + t.Run("case insensitive", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + all, err := user.ListUsers(s, "uSEr1", user1, nil) + require.NoError(t, err) + assert.NotEmpty(t, all) + assert.Equal(t, "user1", all[0].Username) + }) + t.Run("all users", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + all, err := user.ListAllUsers(s) + require.NoError(t, err) + assert.Len(t, all, 16) + }) + t.Run("no search term", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + all, err := user.ListUsers(s, "", user1, nil) + require.NoError(t, err) + assert.Empty(t, all) + }) + t.Run("not discoverable by email", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + all, err := user.ListUsers(s, "user1@example.com", user1, nil) + require.NoError(t, err) + assert.Empty(t, all) + db.AssertExists(t, "users", map[string]interface{}{ + "email": "user1@example.com", + "discoverable_by_email": false, + }, false) + }) + t.Run("not discoverable by name", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + all, err := user.ListUsers(s, "one else", user1, nil) + require.NoError(t, err) + assert.Empty(t, all) + db.AssertExists(t, "users", map[string]interface{}{ + "name": "Some one else", + "discoverable_by_name": false, + }, false) + }) + t.Run("discoverable by email", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + all, err := user.ListUsers(s, "user7@example.com", user1, nil) + require.NoError(t, err) + assert.Len(t, all, 1) + assert.Equal(t, int64(7), all[0].ID) + db.AssertExists(t, "users", map[string]interface{}{ + "email": "user7@example.com", + "discoverable_by_email": true, + }, false) + }) + t.Run("discoverable by partial name", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + all, err := user.ListUsers(s, "with space", user1, nil) + require.NoError(t, err) + assert.Len(t, all, 1) + assert.Equal(t, int64(12), all[0].ID) + db.AssertExists(t, "users", map[string]interface{}{ + "name": "Name with spaces", + "discoverable_by_name": true, + }, false) + }) + t.Run("discoverable by email with extra condition", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + all, err := user.ListUsers(s, "user7@example.com", user1, &user.ProjectUserOpts{AdditionalCond: builder.In("id", 7)}) + require.NoError(t, err) + assert.Len(t, all, 1) + assert.Equal(t, int64(7), all[0].ID) + db.AssertExists(t, "users", map[string]interface{}{ + "email": "user7@example.com", + "discoverable_by_email": true, + }, false) + }) + t.Run("discoverable by exact username", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + all, err := user.ListUsers(s, "user7", user1, nil) + require.NoError(t, err) + assert.Len(t, all, 1) + assert.Equal(t, int64(7), all[0].ID) + db.AssertExists(t, "users", map[string]interface{}{ + "username": "user7", + }, false) + }) + t.Run("not discoverable by partial username", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + all, err := user.ListUsers(s, "user", user1, nil) + require.NoError(t, err) + assert.Empty(t, all) + db.AssertExists(t, "users", map[string]interface{}{ + "username": "user7", + }, false) + }) + t.Run("discoverable by partial username, email and name when matching fuzzily", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + all, err := user.ListUsers(s, "user", user1, &user.ProjectUserOpts{ + MatchFuzzily: true, + }) + require.NoError(t, err) + assert.Len(t, all, 16) + }) + + // External team discoverability bypass tests + // User 10 and user 11 share external team 14 (has external_id). + // User 11 has discoverable_by_name=false and discoverable_by_email=false. + t.Run("external team member discoverable by name", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + all, err := user.ListUsers(s, "Some one else", user10, nil) + require.NoError(t, err) + assert.Len(t, all, 1) + assert.Equal(t, int64(11), all[0].ID) + }) + t.Run("external team member discoverable by email", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + all, err := user.ListUsers(s, "user11@example.com", user10, nil) + require.NoError(t, err) + assert.Len(t, all, 1) + assert.Equal(t, int64(11), all[0].ID) + }) + t.Run("non-external-team user cannot discover by name", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + // User 1 does NOT share an external team with user 11. + all, err := user.ListUsers(s, "Some one else", user1, nil) + require.NoError(t, err) + assert.Empty(t, all) + }) + t.Run("non-external-team user cannot discover by email", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + // User 1 does NOT share an external team with user 11. + all, err := user.ListUsers(s, "user11@example.com", user1, nil) + require.NoError(t, err) + assert.Empty(t, all) + }) + t.Run("regular team does not bypass discoverability", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + // User 1 and user 2 share team 1 (regular team, no external_id). + // User 2 has discoverable_by_email=false. + all, err := user.ListUsers(s, "user2@example.com", user1, nil) + require.NoError(t, err) + assert.Empty(t, all) + }) + t.Run("external team member email masked when searching by name", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + all, err := user.ListUsers(s, "Some one else", user10, nil) + require.NoError(t, err) + require.Len(t, all, 1) + // Email should be masked because the search was by name, not email + assert.Empty(t, all[0].Email) + }) +} diff --git a/pkg/user/user_test.go b/pkg/user/user_test.go index 24aa27c58..03f42aa8e 100644 --- a/pkg/user/user_test.go +++ b/pkg/user/user_test.go @@ -23,7 +23,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "xorm.io/builder" ) func TestCreateUser(t *testing.T) { @@ -398,153 +397,6 @@ func TestUpdateUserPassword(t *testing.T) { }) } -func TestListUsers(t *testing.T) { - user1 := &User{ID: 1} - - t.Run("normal", func(t *testing.T) { - db.LoadAndAssertFixtures(t) - s := db.NewSession() - defer s.Close() - - all, err := ListUsers(s, "user1", user1, nil) - require.NoError(t, err) - assert.NotEmpty(t, all) - assert.Equal(t, "user1", all[0].Username) - }) - t.Run("case insensitive", func(t *testing.T) { - db.LoadAndAssertFixtures(t) - s := db.NewSession() - defer s.Close() - - all, err := ListUsers(s, "uSEr1", user1, nil) - require.NoError(t, err) - assert.NotEmpty(t, all) - assert.Equal(t, "user1", all[0].Username) - }) - t.Run("all users", func(t *testing.T) { - db.LoadAndAssertFixtures(t) - s := db.NewSession() - defer s.Close() - - all, err := ListAllUsers(s) - require.NoError(t, err) - assert.Len(t, all, 16) - }) - t.Run("no search term", func(t *testing.T) { - db.LoadAndAssertFixtures(t) - s := db.NewSession() - defer s.Close() - - all, err := ListUsers(s, "", user1, nil) - require.NoError(t, err) - assert.Empty(t, all) - }) - t.Run("not discoverable by email", func(t *testing.T) { - db.LoadAndAssertFixtures(t) - s := db.NewSession() - defer s.Close() - - all, err := ListUsers(s, "user1@example.com", user1, nil) - require.NoError(t, err) - assert.Empty(t, all) - db.AssertExists(t, "users", map[string]interface{}{ - "email": "user1@example.com", - "discoverable_by_email": false, - }, false) - }) - t.Run("not discoverable by name", func(t *testing.T) { - db.LoadAndAssertFixtures(t) - s := db.NewSession() - defer s.Close() - - all, err := ListUsers(s, "one else", user1, nil) - require.NoError(t, err) - assert.Empty(t, all) - db.AssertExists(t, "users", map[string]interface{}{ - "name": "Some one else", - "discoverable_by_name": false, - }, false) - }) - t.Run("discoverable by email", func(t *testing.T) { - db.LoadAndAssertFixtures(t) - s := db.NewSession() - defer s.Close() - - all, err := ListUsers(s, "user7@example.com", user1, nil) - require.NoError(t, err) - assert.Len(t, all, 1) - assert.Equal(t, int64(7), all[0].ID) - db.AssertExists(t, "users", map[string]interface{}{ - "email": "user7@example.com", - "discoverable_by_email": true, - }, false) - }) - t.Run("discoverable by partial name", func(t *testing.T) { - db.LoadAndAssertFixtures(t) - s := db.NewSession() - defer s.Close() - - all, err := ListUsers(s, "with space", user1, nil) - require.NoError(t, err) - assert.Len(t, all, 1) - assert.Equal(t, int64(12), all[0].ID) - db.AssertExists(t, "users", map[string]interface{}{ - "name": "Name with spaces", - "discoverable_by_name": true, - }, false) - }) - t.Run("discoverable by email with extra condition", func(t *testing.T) { - db.LoadAndAssertFixtures(t) - s := db.NewSession() - defer s.Close() - - all, err := ListUsers(s, "user7@example.com", user1, &ProjectUserOpts{AdditionalCond: builder.In("id", 7)}) - require.NoError(t, err) - assert.Len(t, all, 1) - assert.Equal(t, int64(7), all[0].ID) - db.AssertExists(t, "users", map[string]interface{}{ - "email": "user7@example.com", - "discoverable_by_email": true, - }, false) - }) - t.Run("discoverable by exact username", func(t *testing.T) { - db.LoadAndAssertFixtures(t) - s := db.NewSession() - defer s.Close() - - all, err := ListUsers(s, "user7", user1, nil) - require.NoError(t, err) - assert.Len(t, all, 1) - assert.Equal(t, int64(7), all[0].ID) - db.AssertExists(t, "users", map[string]interface{}{ - "username": "user7", - }, false) - }) - t.Run("not discoverable by partial username", func(t *testing.T) { - db.LoadAndAssertFixtures(t) - s := db.NewSession() - defer s.Close() - - all, err := ListUsers(s, "user", user1, nil) - require.NoError(t, err) - assert.Empty(t, all) - db.AssertExists(t, "users", map[string]interface{}{ - "username": "user7", - }, false) - }) - t.Run("discoverable by partial username, email and name when matching fuzzily", func(t *testing.T) { - db.LoadAndAssertFixtures(t) - s := db.NewSession() - defer s.Close() - - all, err := ListUsers(s, "user", user1, &ProjectUserOpts{ - MatchFuzzily: true, - }) - require.NoError(t, err) - assert.Len(t, all, 16) - }) -} - func TestUserPasswordReset(t *testing.T) { t.Run("normal", func(t *testing.T) { db.LoadAndAssertFixtures(t)