From 7055d7341c3d7b913d8d72bb083e70c6b5f772c6 Mon Sep 17 00:00:00 2001 From: kolaente Date: Mon, 28 Oct 2024 11:08:06 +0100 Subject: [PATCH] feat(sharing): add config so that users only find members of their teams This adds a feature where you can enable users to only find members of teams they're part of. This makes the user search when sharing projects less confusing, because users only see other users they already know. It is still possible to add users to teams with their email address, if they have that enabled in the user settings. --- config-raw.json | 5 ++++ pkg/config/config.go | 34 +++++++++++----------- pkg/models/user_project.go | 4 +-- pkg/models/user_project_test.go | 2 +- pkg/routes/api/v1/user_list.go | 13 +++++++-- pkg/user/user_test.go | 24 ++++++++-------- pkg/user/users_project.go | 50 +++++++++++++++++++++++++++++---- 7 files changed, 93 insertions(+), 39 deletions(-) diff --git a/config-raw.json b/config-raw.json index 4c874d296..d7e110eb2 100644 --- a/config-raw.json +++ b/config-raw.json @@ -132,6 +132,11 @@ "key": "bcryptrounds", "default_value": "11", "comment": "The number of bcrypt rounds to use during registration. Each increment of this number doubles the computational cost. You probably don't need to change this value." + }, + { + "key": "enableopenidteamusersearch", + "default_value": "false", + "comment": "If enabled, users will only find other users who are part of an existing team when they are searching for a user by their partial name. The other existing team may be created from openid. It is still possible to add users to teams with their exact email address even when this is enabled." } ] }, diff --git a/pkg/config/config.go b/pkg/config/config.go index f24deb069..4f5105216 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -50,22 +50,23 @@ const ( ServiceMaxItemsPerPage Key = `service.maxitemsperpage` ServiceDemoMode Key = `service.demomode` // Deprecated: Use metrics.enabled - ServiceEnableMetrics Key = `service.enablemetrics` - ServiceMotd Key = `service.motd` - ServiceEnableLinkSharing Key = `service.enablelinksharing` - ServiceEnableRegistration Key = `service.enableregistration` - ServiceEnableTaskAttachments Key = `service.enabletaskattachments` - ServiceTimeZone Key = `service.timezone` - ServiceEnableTaskComments Key = `service.enabletaskcomments` - ServiceEnableTotp Key = `service.enabletotp` - ServiceTestingtoken Key = `service.testingtoken` - ServiceEnableEmailReminders Key = `service.enableemailreminders` - ServiceEnableUserDeletion Key = `service.enableuserdeletion` - ServiceMaxAvatarSize Key = `service.maxavatarsize` - ServiceAllowIconChanges Key = `service.allowiconchanges` - ServiceCustomLogoURL Key = `service.customlogourl` - ServiceEnablePublicTeams Key = `service.enablepublicteams` - ServiceBcryptRounds Key = `service.bcryptrounds` + ServiceEnableMetrics Key = `service.enablemetrics` + ServiceMotd Key = `service.motd` + ServiceEnableLinkSharing Key = `service.enablelinksharing` + ServiceEnableRegistration Key = `service.enableregistration` + ServiceEnableTaskAttachments Key = `service.enabletaskattachments` + ServiceTimeZone Key = `service.timezone` + ServiceEnableTaskComments Key = `service.enabletaskcomments` + ServiceEnableTotp Key = `service.enabletotp` + ServiceTestingtoken Key = `service.testingtoken` + ServiceEnableEmailReminders Key = `service.enableemailreminders` + ServiceEnableUserDeletion Key = `service.enableuserdeletion` + ServiceMaxAvatarSize Key = `service.maxavatarsize` + ServiceAllowIconChanges Key = `service.allowiconchanges` + ServiceCustomLogoURL Key = `service.customlogourl` + ServiceEnablePublicTeams Key = `service.enablepublicteams` + ServiceBcryptRounds Key = `service.bcryptrounds` + ServiceEnableOpenIDTeamUserOnlySearch Key = `service.enableopenidteamusersearch` SentryEnabled Key = `sentry.enabled` SentryDsn Key = `sentry.dsn` @@ -321,6 +322,7 @@ func InitDefaultConfig() { ServiceAllowIconChanges.setDefault(true) ServiceEnablePublicTeams.setDefault(false) ServiceBcryptRounds.setDefault(11) + ServiceEnableOpenIDTeamUserOnlySearch.setDefault(false) // Sentry SentryDsn.setDefault("https://440eedc957d545a795c17bbaf477497c@o1047380.ingest.sentry.io/4504254983634944") diff --git a/pkg/models/user_project.go b/pkg/models/user_project.go index ff722a4fd..016873ba2 100644 --- a/pkg/models/user_project.go +++ b/pkg/models/user_project.go @@ -30,7 +30,7 @@ type ProjectUIDs struct { } // ListUsersFromProject returns a list with all users who have access to a project, regardless of the method which gave them access -func ListUsersFromProject(s *xorm.Session, l *Project, search string) (users []*user.User, err error) { +func ListUsersFromProject(s *xorm.Session, l *Project, currentUser *user.User, search string) (users []*user.User, err error) { userids := []*ProjectUIDs{} @@ -107,7 +107,7 @@ func ListUsersFromProject(s *xorm.Session, l *Project, search string) (users []* cond = builder.In("id", uids) } - users, err = user.ListUsers(s, search, &user.ProjectUserOpts{ + users, err = user.ListUsers(s, search, currentUser, &user.ProjectUserOpts{ AdditionalCond: cond, ReturnAllIfNoSearchProvided: true, MatchFuzzily: true, diff --git a/pkg/models/user_project_test.go b/pkg/models/user_project_test.go index c74aef497..397f9c17a 100644 --- a/pkg/models/user_project_test.go +++ b/pkg/models/user_project_test.go @@ -235,7 +235,7 @@ func TestListUsersFromProject(t *testing.T) { s := db.NewSession() defer s.Close() - gotUsers, err := ListUsersFromProject(s, tt.args.l, tt.args.search) + gotUsers, err := ListUsersFromProject(s, tt.args.l, testuser1, tt.args.search) if (err != nil) != tt.wantErr { t.Errorf("ListUsersFromProject() error = %v, wantErr %v", err, tt.wantErr) return diff --git a/pkg/routes/api/v1/user_list.go b/pkg/routes/api/v1/user_list.go index 8842c4013..afeb54938 100644 --- a/pkg/routes/api/v1/user_list.go +++ b/pkg/routes/api/v1/user_list.go @@ -47,13 +47,14 @@ func UserList(c echo.Context) error { s := db.NewSession() defer s.Close() - users, err := user.ListUsers(s, search, nil) + currentUser, err := user.GetCurrentUser(c) if err != nil { _ = s.Rollback() return handler.HandleHTTPError(err) } - if err := s.Commit(); err != nil { + users, err := user.ListUsers(s, search, currentUser, nil) + if err != nil { _ = s.Rollback() return handler.HandleHTTPError(err) } @@ -104,8 +105,14 @@ func ListUsersForProject(c echo.Context) error { return echo.ErrForbidden } + currentUser, err := user.GetCurrentUser(c) + if err != nil { + _ = s.Rollback() + return handler.HandleHTTPError(err) + } + search := c.QueryParam("s") - users, err := models.ListUsersFromProject(s, &project, search) + users, err := models.ListUsersFromProject(s, &project, currentUser, search) if err != nil { _ = s.Rollback() return handler.HandleHTTPError(err) diff --git a/pkg/user/user_test.go b/pkg/user/user_test.go index 3dd75d736..e5e33ce44 100644 --- a/pkg/user/user_test.go +++ b/pkg/user/user_test.go @@ -373,12 +373,14 @@ 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", nil) + all, err := ListUsers(s, "user1", user1, nil) require.NoError(t, err) assert.NotEmpty(t, all) assert.Equal(t, "user1", all[0].Username) @@ -388,7 +390,7 @@ func TestListUsers(t *testing.T) { s := db.NewSession() defer s.Close() - all, err := ListUsers(s, "uSEr1", nil) + all, err := ListUsers(s, "uSEr1", user1, nil) require.NoError(t, err) assert.NotEmpty(t, all) assert.Equal(t, "user1", all[0].Username) @@ -407,7 +409,7 @@ func TestListUsers(t *testing.T) { s := db.NewSession() defer s.Close() - all, err := ListUsers(s, "", nil) + all, err := ListUsers(s, "", user1, nil) require.NoError(t, err) assert.Empty(t, all) }) @@ -416,7 +418,7 @@ func TestListUsers(t *testing.T) { s := db.NewSession() defer s.Close() - all, err := ListUsers(s, "user1@example.com", nil) + all, err := ListUsers(s, "user1@example.com", user1, nil) require.NoError(t, err) assert.Empty(t, all) db.AssertExists(t, "users", map[string]interface{}{ @@ -429,7 +431,7 @@ func TestListUsers(t *testing.T) { s := db.NewSession() defer s.Close() - all, err := ListUsers(s, "one else", nil) + all, err := ListUsers(s, "one else", user1, nil) require.NoError(t, err) assert.Empty(t, all) db.AssertExists(t, "users", map[string]interface{}{ @@ -442,7 +444,7 @@ func TestListUsers(t *testing.T) { s := db.NewSession() defer s.Close() - all, err := ListUsers(s, "user7@example.com", nil) + 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) @@ -456,7 +458,7 @@ func TestListUsers(t *testing.T) { s := db.NewSession() defer s.Close() - all, err := ListUsers(s, "with space", nil) + 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) @@ -470,7 +472,7 @@ func TestListUsers(t *testing.T) { s := db.NewSession() defer s.Close() - all, err := ListUsers(s, "user7@example.com", &ProjectUserOpts{AdditionalCond: builder.In("id", 7)}) + 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) @@ -484,7 +486,7 @@ func TestListUsers(t *testing.T) { s := db.NewSession() defer s.Close() - all, err := ListUsers(s, "user7", nil) + all, err := ListUsers(s, "user7", user1, nil) require.NoError(t, err) assert.Len(t, all, 1) assert.Equal(t, int64(7), all[0].ID) @@ -497,7 +499,7 @@ func TestListUsers(t *testing.T) { s := db.NewSession() defer s.Close() - all, err := ListUsers(s, "user", nil) + all, err := ListUsers(s, "user", user1, nil) require.NoError(t, err) assert.Empty(t, all) db.AssertExists(t, "users", map[string]interface{}{ @@ -509,7 +511,7 @@ func TestListUsers(t *testing.T) { s := db.NewSession() defer s.Close() - all, err := ListUsers(s, "user", &ProjectUserOpts{ + all, err := ListUsers(s, "user", user1, &ProjectUserOpts{ MatchFuzzily: true, }) require.NoError(t, err) diff --git a/pkg/user/users_project.go b/pkg/user/users_project.go index 019c137aa..f845d4cb9 100644 --- a/pkg/user/users_project.go +++ b/pkg/user/users_project.go @@ -19,6 +19,8 @@ package user import ( "strings" + "code.vikunja.io/api/pkg/config" + "code.vikunja.io/api/pkg/db" "xorm.io/builder" @@ -33,7 +35,7 @@ type ProjectUserOpts struct { } // ListUsers returns a list with all users, filtered by an optional search string -func ListUsers(s *xorm.Session, search string, opts *ProjectUserOpts) (users []*User, err error) { +func ListUsers(s *xorm.Session, search string, currentUser *User, opts *ProjectUserOpts) (users []*User, err error) { if opts == nil { opts = &ProjectUserOpts{} } @@ -47,8 +49,10 @@ func ListUsers(s *xorm.Session, search string, opts *ProjectUserOpts) (users []* conds := []builder.Cond{} + queryParts := strings.Split(search, ",") + if search != "" { - for _, queryPart := range strings.Split(search, ",") { + for _, queryPart := range queryParts { if opts.MatchFuzzily { conds = append(conds, @@ -69,10 +73,6 @@ func ListUsers(s *xorm.Session, search string, opts *ProjectUserOpts) (users []* conds = append(conds, usernameCond, - builder.And( - builder.Eq{"email": queryPart}, - builder.Eq{"discoverable_by_email": true}, - ), builder.And( db.ILIKE("name", queryPart), builder.Eq{"discoverable_by_name": true}, @@ -81,6 +81,15 @@ func ListUsers(s *xorm.Session, search string, opts *ProjectUserOpts) (users []* } } + if !opts.MatchFuzzily { + conds = append(conds, + builder.And( + builder.In("email", queryParts), + builder.Eq{"discoverable_by_email": true}, + ), + ) + } + cond := builder.Or(conds...) if opts.AdditionalCond != nil { @@ -90,6 +99,35 @@ func ListUsers(s *xorm.Session, search string, opts *ProjectUserOpts) (users []* ) } + if config.ServiceEnableOpenIDTeamUserOnlySearch.GetBool() { + teamMemberCond := builder.In("id", builder.Select("user_id"). + From("team_members"). + Where(builder.In("team_id", + builder.Select("team_id"). + From("team_members"). + Where(builder.Eq{"team_members.user_id": currentUser.ID}), + )), + ) + + if !opts.MatchFuzzily { + cond = builder.And( + cond, + builder.Or( + teamMemberCond, + builder.And( + builder.In("email", queryParts), + builder.Eq{"discoverable_by_email": true}, + ), + ), + ) + } else { + cond = builder.And( + cond, + teamMemberCond, + ) + } + } + err = s. Where(cond). Find(&users)