From a648ced22150754e589a5e6742dca698c9298b95 Mon Sep 17 00:00:00 2001 From: cyrilzhang-mm <112951043+cyrilzhang-mm@users.noreply.github.com> Date: Tue, 25 Oct 2022 11:54:51 -0400 Subject: [PATCH] [MM-42421] Prevent guests from seeing users through groups API (#21151) --- api4/group.go | 53 ++++++-- api4/group_test.go | 2 +- api4/user.go | 6 +- api4/user_test.go | 37 +++++- app/app_iface.go | 10 +- app/group.go | 24 ++-- app/group_test.go | 10 +- app/notification.go | 2 +- app/opentracing/opentracing_layer.go | 20 ++-- app/plugin_api.go | 4 +- store/opentracinglayer/opentracinglayer.go | 30 ++++- store/retrylayer/retrylayer.go | 33 ++++- store/sqlstore/group_store.go | 122 +++++++++++-------- store/store.go | 7 +- store/storetest/group_store.go | 133 +++++++++++++++------ store/storetest/mocks/GroupStore.go | 63 ++++++---- store/timerlayer/timerlayer.go | 28 ++++- 17 files changed, 406 insertions(+), 178 deletions(-) diff --git a/api4/group.go b/api4/group.go index abcde27459..da17d31ce3 100644 --- a/api4/group.go +++ b/api4/group.go @@ -105,9 +105,15 @@ func getGroup(c *Context, w http.ResponseWriter, r *http.Request) { return } + restrictions, appErr := c.App.GetViewUsersRestrictions(c.AppContext.Session().UserId) + if appErr != nil { + c.Err = appErr + return + } + group, appErr := c.App.GetGroup(c.Params.GroupId, &model.GetGroupOpts{ IncludeMemberCount: c.Params.IncludeMemberCount, - }) + }, restrictions) if appErr != nil { c.Err = appErr return @@ -206,7 +212,7 @@ func patchGroup(c *Context, w http.ResponseWriter, r *http.Request) { return } - group, appErr := c.App.GetGroup(c.Params.GroupId, nil) + group, appErr := c.App.GetGroup(c.Params.GroupId, nil, nil) if appErr != nil { c.Err = appErr return @@ -321,7 +327,7 @@ func linkGroupSyncable(c *Context, w http.ResponseWriter, r *http.Request) { return } - group, appErr := c.App.GetGroup(c.Params.GroupId, nil) + group, appErr := c.App.GetGroup(c.Params.GroupId, nil, nil) if appErr != nil { c.Err = appErr return @@ -657,7 +663,7 @@ func getGroupMembers(c *Context, w http.ResponseWriter, r *http.Request) { return } - group, appErr := c.App.GetGroup(c.Params.GroupId, nil) + group, appErr := c.App.GetGroup(c.Params.GroupId, nil, nil) if appErr != nil { c.Err = appErr return @@ -675,7 +681,13 @@ func getGroupMembers(c *Context, w http.ResponseWriter, r *http.Request) { return } - members, count, appErr := c.App.GetGroupMemberUsersPage(c.Params.GroupId, c.Params.Page, c.Params.PerPage) + restrictions, appErr := c.App.GetViewUsersRestrictions(c.AppContext.Session().UserId) + if appErr != nil { + c.Err = appErr + return + } + + members, count, appErr := c.App.GetGroupMemberUsersPage(c.Params.GroupId, c.Params.Page, c.Params.PerPage, restrictions) if appErr != nil { c.Err = appErr return @@ -718,7 +730,7 @@ func getGroupStats(c *Context, w http.ResponseWriter, r *http.Request) { } groupID := c.Params.GroupId - count, appErr := c.App.GetGroupMemberCount(groupID) + count, appErr := c.App.GetGroupMemberCount(groupID, nil) if appErr != nil { c.Err = appErr return @@ -1011,12 +1023,33 @@ func getGroups(c *Context, w http.ResponseWriter, r *http.Request) { opts.Since = since } - groups, appErr := c.App.GetGroups(c.Params.Page, c.Params.PerPage, opts) + restrictions, appErr := c.App.GetViewUsersRestrictions(c.AppContext.Session().UserId) if appErr != nil { c.Err = appErr return } + var ( + groups = []*model.Group{} + canSee bool = true + ) + + if opts.FilterHasMember != "" { + canSee, appErr = c.App.UserCanSeeOtherUser(c.AppContext.Session().UserId, opts.FilterHasMember) + if appErr != nil { + c.Err = appErr + return + } + } + + if canSee { + groups, appErr = c.App.GetGroups(c.Params.Page, c.Params.PerPage, opts, restrictions) + if appErr != nil { + c.Err = appErr + return + } + } + var ( b []byte err error @@ -1055,7 +1088,7 @@ func deleteGroup(c *Context, w http.ResponseWriter, r *http.Request) { return } - group, err := c.App.GetGroup(c.Params.GroupId, nil) + group, err := c.App.GetGroup(c.Params.GroupId, nil, nil) if err != nil { c.Err = err return @@ -1103,7 +1136,7 @@ func addGroupMembers(c *Context, w http.ResponseWriter, r *http.Request) { return } - group, appErr := c.App.GetGroup(c.Params.GroupId, nil) + group, appErr := c.App.GetGroup(c.Params.GroupId, nil, nil) if appErr != nil { c.Err = appErr return @@ -1162,7 +1195,7 @@ func deleteGroupMembers(c *Context, w http.ResponseWriter, r *http.Request) { return } - group, appErr := c.App.GetGroup(c.Params.GroupId, nil) + group, appErr := c.App.GetGroup(c.Params.GroupId, nil, nil) if appErr != nil { c.Err = appErr return diff --git a/api4/group_test.go b/api4/group_test.go index b6f155ad1c..bbe4fa33c4 100644 --- a/api4/group_test.go +++ b/api4/group_test.go @@ -1518,7 +1518,7 @@ func TestAddMembersToGroup(t *testing.T) { assert.Len(t, groupMembers, 2) - count, countErr := th.App.GetGroupMemberCount(group.Id) + count, countErr := th.App.GetGroupMemberCount(group.Id, nil) assert.Nil(t, countErr) assert.Equal(t, count, int64(2)) diff --git a/api4/user.go b/api4/user.go index 423edc755c..69a7c6214a 100644 --- a/api4/user.go +++ b/api4/user.go @@ -829,7 +829,7 @@ func getUsers(c *Context, w http.ResponseWriter, r *http.Request) { return } - profiles, _, appErr = c.App.GetGroupMemberUsersPage(inGroupId, c.Params.Page, c.Params.PerPage) + profiles, _, appErr = c.App.GetGroupMemberUsersPage(inGroupId, c.Params.Page, c.Params.PerPage, userGetOptions.ViewRestrictions) if appErr != nil { c.Err = appErr return @@ -842,7 +842,7 @@ func getUsers(c *Context, w http.ResponseWriter, r *http.Request) { return } - profiles, appErr = c.App.GetUsersNotInGroupPage(notInGroupId, c.Params.Page, c.Params.PerPage) + profiles, appErr = c.App.GetUsersNotInGroupPage(notInGroupId, c.Params.Page, c.Params.PerPage, userGetOptions.ViewRestrictions) if appErr != nil { c.Err = appErr return @@ -876,7 +876,7 @@ func getUsers(c *Context, w http.ResponseWriter, r *http.Request) { } func requireGroupAccess(c *web.Context, groupID string) *model.AppError { - group, err := c.App.GetGroup(groupID, nil) + group, err := c.App.GetGroup(groupID, nil, nil) if err != nil { return err } diff --git a/api4/user_test.go b/api4/user_test.go index 7f937fa7fc..80bdea263e 100644 --- a/api4/user_test.go +++ b/api4/user_test.go @@ -2717,13 +2717,26 @@ func TestGetUsersInGroup(t *testing.T) { }) assert.Nil(t, appErr) + cid := model.NewId() + customGroup, appErr := th.App.CreateGroup(&model.Group{ + DisplayName: "dn-foo_" + cid, + Name: model.NewString("name" + cid), + Source: model.GroupSourceCustom, + Description: "description_" + cid, + RemoteId: model.NewString(model.NewId()), + }) + assert.Nil(t, appErr) + + user1, err := th.App.CreateUser(th.Context, &model.User{Email: th.GenerateTestEmail(), Nickname: "test user1", Password: "test-password-1", Username: "test-user-1", Roles: model.SystemUserRoleId}) + assert.Nil(t, err) + t.Run("Requires ldap license", func(t *testing.T) { _, response, err := th.SystemAdminClient.GetUsersInGroup(group.Id, 0, 60, "") require.Error(t, err) CheckForbiddenStatus(t, response) }) - th.App.Srv().SetLicense(model.NewTestLicense("ldap")) + th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuProfessional)) t.Run("Requires manage system permission to access users in group", func(t *testing.T) { th.Client.Login(th.BasicUser.Email, th.BasicUser.Password) @@ -2732,8 +2745,6 @@ func TestGetUsersInGroup(t *testing.T) { CheckForbiddenStatus(t, response) }) - user1, err := th.App.CreateUser(th.Context, &model.User{Email: th.GenerateTestEmail(), Nickname: "test user1", Password: "test-password-1", Username: "test-user-1", Roles: model.SystemUserRoleId}) - assert.Nil(t, err) _, err = th.App.UpsertGroupMember(group.Id, user1.Id) assert.Nil(t, err) @@ -2748,6 +2759,26 @@ func TestGetUsersInGroup(t *testing.T) { require.NoError(t, err) assert.Empty(t, users) }) + + _, err = th.App.UpsertGroupMember(customGroup.Id, user1.Id) + assert.Nil(t, err) + + t.Run("Returns users in custom group when called by regular user", func(t *testing.T) { + th.Client.Login(th.BasicUser.Email, th.BasicUser.Password) + users, _, err := th.Client.GetUsersInGroup(customGroup.Id, 0, 60, "") + require.NoError(t, err) + assert.Equal(t, users[0].Id, user1.Id) + }) + + t.Run("Returns no users in custom group when called by guest user", func(t *testing.T) { + th.Client.Login(th.BasicUser.Email, th.BasicUser.Password) + th.App.DemoteUserToGuest(th.Context, th.BasicUser) + + users, _, err := th.Client.GetUsersInGroup(customGroup.Id, 0, 60, "") + require.NoError(t, err) + assert.Equal(t, len(users), 0) + }) + } func TestUpdateUserMfa(t *testing.T) { diff --git a/app/app_iface.go b/app/app_iface.go index f3ac3315f7..a113169c5c 100644 --- a/app/app_iface.go +++ b/app/app_iface.go @@ -639,16 +639,16 @@ type AppIface interface { GetFlaggedPostsForChannel(userID, channelID string, offset int, limit int) (*model.PostList, *model.AppError) GetFlaggedPostsForTeam(userID, teamID string, offset int, limit int) (*model.PostList, *model.AppError) GetGlobalRetentionPolicy() (*model.GlobalRetentionPolicy, *model.AppError) - GetGroup(id string, opts *model.GetGroupOpts) (*model.Group, *model.AppError) + GetGroup(id string, opts *model.GetGroupOpts, viewRestrictions *model.ViewUsersRestrictions) (*model.Group, *model.AppError) GetGroupByName(name string, opts model.GroupSearchOpts) (*model.Group, *model.AppError) GetGroupByRemoteID(remoteID string, groupSource model.GroupSource) (*model.Group, *model.AppError) GetGroupChannel(c request.CTX, userIDs []string) (*model.Channel, *model.AppError) - GetGroupMemberCount(groupID string) (int64, *model.AppError) + GetGroupMemberCount(groupID string, viewRestrictions *model.ViewUsersRestrictions) (int64, *model.AppError) GetGroupMemberUsers(groupID string) ([]*model.User, *model.AppError) - GetGroupMemberUsersPage(groupID string, page int, perPage int) ([]*model.User, int, *model.AppError) + GetGroupMemberUsersPage(groupID string, page int, perPage int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, int, *model.AppError) GetGroupSyncable(groupID string, syncableID string, syncableType model.GroupSyncableType) (*model.GroupSyncable, *model.AppError) GetGroupSyncables(groupID string, syncableType model.GroupSyncableType) ([]*model.GroupSyncable, *model.AppError) - GetGroups(page, perPage int, opts model.GroupSearchOpts) ([]*model.Group, *model.AppError) + GetGroups(page, perPage int, opts model.GroupSearchOpts, viewRestrictions *model.ViewUsersRestrictions) ([]*model.Group, *model.AppError) GetGroupsAssociatedToChannelsByTeam(teamID string, opts model.GroupSearchOpts) (map[string][]*model.GroupWithSchemeAdmin, *model.AppError) GetGroupsByChannel(channelID string, opts model.GroupSearchOpts) ([]*model.GroupWithSchemeAdmin, int, *model.AppError) GetGroupsByIDs(groupIDs []string) ([]*model.Group, *model.AppError) @@ -831,7 +831,7 @@ type AppIface interface { GetUsersNotInChannel(teamID string, channelID string, groupConstrained bool, offset int, limit int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, *model.AppError) GetUsersNotInChannelMap(teamID string, channelID string, groupConstrained bool, offset int, limit int, asAdmin bool, viewRestrictions *model.ViewUsersRestrictions) (map[string]*model.User, *model.AppError) GetUsersNotInChannelPage(teamID string, channelID string, groupConstrained bool, page int, perPage int, asAdmin bool, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, *model.AppError) - GetUsersNotInGroupPage(groupID string, page int, perPage int) ([]*model.User, *model.AppError) + GetUsersNotInGroupPage(groupID string, page int, perPage int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, *model.AppError) GetUsersNotInTeam(teamID string, groupConstrained bool, offset int, limit int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, *model.AppError) GetUsersNotInTeamEtag(teamID string, restrictionsHash string) string GetUsersNotInTeamPage(teamID string, groupConstrained bool, page int, perPage int, asAdmin bool, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, *model.AppError) diff --git a/app/group.go b/app/group.go index beead51982..209c9e28c4 100644 --- a/app/group.go +++ b/app/group.go @@ -13,7 +13,7 @@ import ( "github.com/mattermost/mattermost-server/v6/store" ) -func (a *App) GetGroup(id string, opts *model.GetGroupOpts) (*model.Group, *model.AppError) { +func (a *App) GetGroup(id string, opts *model.GetGroupOpts, viewRestrictions *model.ViewUsersRestrictions) (*model.Group, *model.AppError) { group, err := a.Srv().Store().Group().Get(id) if err != nil { var nfErr *store.ErrNotFound @@ -26,7 +26,7 @@ func (a *App) GetGroup(id string, opts *model.GetGroupOpts) (*model.Group, *mode } if opts != nil && opts.IncludeMemberCount { - memberCount, err := a.Srv().Store().Group().GetMemberCount(id) + memberCount, err := a.Srv().Store().Group().GetMemberCountWithRestrictions(id, viewRestrictions) if err != nil { return nil, model.NewAppError("GetGroup", "app.member_count", nil, "", http.StatusInternalServerError).Wrap(err) } @@ -147,7 +147,6 @@ func (a *App) CreateGroupWithUserIds(group *model.GroupWithUserIds) (*model.Grou messageWs := model.NewWebSocketEvent(model.WebsocketEventReceivedGroup, "", "", "", nil, "") count, err := a.Srv().Store().Group().GetMemberCount(newGroup.Id) - if err != nil { return nil, model.NewAppError("CreateGroupWithUserIds", "app.group.id.app_error", nil, "", http.StatusBadRequest).Wrap(err) } @@ -218,8 +217,8 @@ func (a *App) DeleteGroup(groupID string) (*model.Group, *model.AppError) { return deletedGroup, nil } -func (a *App) GetGroupMemberCount(groupID string) (int64, *model.AppError) { - count, err := a.Srv().Store().Group().GetMemberCount(groupID) +func (a *App) GetGroupMemberCount(groupID string, viewRestrictions *model.ViewUsersRestrictions) (int64, *model.AppError) { + count, err := a.Srv().Store().Group().GetMemberCountWithRestrictions(groupID, viewRestrictions) if err != nil { return 0, model.NewAppError("GetGroupMemberCount", "app.select_error", nil, "", http.StatusInternalServerError).Wrap(err) } @@ -236,20 +235,21 @@ func (a *App) GetGroupMemberUsers(groupID string) ([]*model.User, *model.AppErro return users, nil } -func (a *App) GetGroupMemberUsersPage(groupID string, page int, perPage int) ([]*model.User, int, *model.AppError) { - members, err := a.Srv().Store().Group().GetMemberUsersPage(groupID, page, perPage) +func (a *App) GetGroupMemberUsersPage(groupID string, page int, perPage int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, int, *model.AppError) { + members, err := a.Srv().Store().Group().GetMemberUsersPage(groupID, page, perPage, viewRestrictions) if err != nil { return nil, 0, model.NewAppError("GetGroupMemberUsersPage", "app.select_error", nil, "", http.StatusInternalServerError).Wrap(err) } - count, appErr := a.GetGroupMemberCount(groupID) + count, appErr := a.GetGroupMemberCount(groupID, viewRestrictions) if appErr != nil { return nil, 0, appErr } return a.sanitizeProfiles(members, false), int(count), nil } -func (a *App) GetUsersNotInGroupPage(groupID string, page int, perPage int) ([]*model.User, *model.AppError) { - members, err := a.Srv().Store().Group().GetNonMemberUsersPage(groupID, page, perPage) + +func (a *App) GetUsersNotInGroupPage(groupID string, page int, perPage int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, *model.AppError) { + members, err := a.Srv().Store().Group().GetNonMemberUsersPage(groupID, page, perPage, viewRestrictions) if err != nil { return nil, model.NewAppError("GetUsersNotInGroupPage", "app.select_error", nil, "", http.StatusInternalServerError).Wrap(err) } @@ -580,8 +580,8 @@ func (a *App) GetGroupsAssociatedToChannelsByTeam(teamID string, opts model.Grou return groupsAssociatedByChannelId, nil } -func (a *App) GetGroups(page, perPage int, opts model.GroupSearchOpts) ([]*model.Group, *model.AppError) { - groups, err := a.Srv().Store().Group().GetGroups(page, perPage, opts) +func (a *App) GetGroups(page, perPage int, opts model.GroupSearchOpts, viewRestrictions *model.ViewUsersRestrictions) ([]*model.Group, *model.AppError) { + groups, err := a.Srv().Store().Group().GetGroups(page, perPage, opts, viewRestrictions) if err != nil { return nil, model.NewAppError("GetGroups", "app.select_error", nil, "", http.StatusInternalServerError).Wrap(err) } diff --git a/app/group_test.go b/app/group_test.go index 9a1084cfec..3225d881b7 100644 --- a/app/group_test.go +++ b/app/group_test.go @@ -17,19 +17,19 @@ func TestGetGroup(t *testing.T) { defer th.TearDown() group := th.CreateGroup() - group, err := th.App.GetGroup(group.Id, nil) + group, err := th.App.GetGroup(group.Id, nil, nil) require.Nil(t, err) require.NotNil(t, group) - nilGroup, err := th.App.GetGroup(model.NewId(), nil) + nilGroup, err := th.App.GetGroup(model.NewId(), nil, nil) require.NotNil(t, err) require.Nil(t, nilGroup) - group, err = th.App.GetGroup(group.Id, &model.GetGroupOpts{IncludeMemberCount: false}) + group, err = th.App.GetGroup(group.Id, &model.GetGroupOpts{IncludeMemberCount: false}, nil) require.Nil(t, err) require.Nil(t, group.MemberCount) - group, err = th.App.GetGroup(group.Id, &model.GetGroupOpts{IncludeMemberCount: true}) + group, err = th.App.GetGroup(group.Id, &model.GetGroupOpts{IncludeMemberCount: true}, nil) require.Nil(t, err) require.NotNil(t, group.MemberCount) } @@ -369,7 +369,7 @@ func TestGetGroups(t *testing.T) { defer th.TearDown() group := th.CreateGroup() - groups, err := th.App.GetGroups(0, 60, model.GroupSearchOpts{}) + groups, err := th.App.GetGroups(0, 60, model.GroupSearchOpts{}, nil) require.Nil(t, err) require.ElementsMatch(t, []*model.Group{group}, groups) } diff --git a/app/notification.go b/app/notification.go index f143789f4b..c5d36e14b8 100644 --- a/app/notification.go +++ b/app/notification.go @@ -1082,7 +1082,7 @@ func (a *App) getGroupsAllowedForReferenceInChannel(channel *model.Channel, team return groupsMap, nil } - groups, err := a.Srv().Store().Group().GetGroups(0, 0, opts) + groups, err := a.Srv().Store().Group().GetGroups(0, 0, opts, nil) if err != nil { return nil, errors.Wrap(err, "unable to get groups") } diff --git a/app/opentracing/opentracing_layer.go b/app/opentracing/opentracing_layer.go index 2e86193b34..3c5b181160 100644 --- a/app/opentracing/opentracing_layer.go +++ b/app/opentracing/opentracing_layer.go @@ -6151,7 +6151,7 @@ func (a *OpenTracingAppLayer) GetGlobalRetentionPolicy() (*model.GlobalRetention return resultVar0, resultVar1 } -func (a *OpenTracingAppLayer) GetGroup(id string, opts *model.GetGroupOpts) (*model.Group, *model.AppError) { +func (a *OpenTracingAppLayer) GetGroup(id string, opts *model.GetGroupOpts, viewRestrictions *model.ViewUsersRestrictions) (*model.Group, *model.AppError) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetGroup") @@ -6163,7 +6163,7 @@ func (a *OpenTracingAppLayer) GetGroup(id string, opts *model.GetGroupOpts) (*mo }() defer span.Finish() - resultVar0, resultVar1 := a.app.GetGroup(id, opts) + resultVar0, resultVar1 := a.app.GetGroup(id, opts, viewRestrictions) if resultVar1 != nil { span.LogFields(spanlog.Error(resultVar1)) @@ -6239,7 +6239,7 @@ func (a *OpenTracingAppLayer) GetGroupChannel(c request.CTX, userIDs []string) ( return resultVar0, resultVar1 } -func (a *OpenTracingAppLayer) GetGroupMemberCount(groupID string) (int64, *model.AppError) { +func (a *OpenTracingAppLayer) GetGroupMemberCount(groupID string, viewRestrictions *model.ViewUsersRestrictions) (int64, *model.AppError) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetGroupMemberCount") @@ -6251,7 +6251,7 @@ func (a *OpenTracingAppLayer) GetGroupMemberCount(groupID string) (int64, *model }() defer span.Finish() - resultVar0, resultVar1 := a.app.GetGroupMemberCount(groupID) + resultVar0, resultVar1 := a.app.GetGroupMemberCount(groupID, viewRestrictions) if resultVar1 != nil { span.LogFields(spanlog.Error(resultVar1)) @@ -6283,7 +6283,7 @@ func (a *OpenTracingAppLayer) GetGroupMemberUsers(groupID string) ([]*model.User return resultVar0, resultVar1 } -func (a *OpenTracingAppLayer) GetGroupMemberUsersPage(groupID string, page int, perPage int) ([]*model.User, int, *model.AppError) { +func (a *OpenTracingAppLayer) GetGroupMemberUsersPage(groupID string, page int, perPage int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, int, *model.AppError) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetGroupMemberUsersPage") @@ -6295,7 +6295,7 @@ func (a *OpenTracingAppLayer) GetGroupMemberUsersPage(groupID string, page int, }() defer span.Finish() - resultVar0, resultVar1, resultVar2 := a.app.GetGroupMemberUsersPage(groupID, page, perPage) + resultVar0, resultVar1, resultVar2 := a.app.GetGroupMemberUsersPage(groupID, page, perPage, viewRestrictions) if resultVar2 != nil { span.LogFields(spanlog.Error(resultVar2)) @@ -6349,7 +6349,7 @@ func (a *OpenTracingAppLayer) GetGroupSyncables(groupID string, syncableType mod return resultVar0, resultVar1 } -func (a *OpenTracingAppLayer) GetGroups(page int, perPage int, opts model.GroupSearchOpts) ([]*model.Group, *model.AppError) { +func (a *OpenTracingAppLayer) GetGroups(page int, perPage int, opts model.GroupSearchOpts, viewRestrictions *model.ViewUsersRestrictions) ([]*model.Group, *model.AppError) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetGroups") @@ -6361,7 +6361,7 @@ func (a *OpenTracingAppLayer) GetGroups(page int, perPage int, opts model.GroupS }() defer span.Finish() - resultVar0, resultVar1 := a.app.GetGroups(page, perPage, opts) + resultVar0, resultVar1 := a.app.GetGroups(page, perPage, opts, viewRestrictions) if resultVar1 != nil { span.LogFields(spanlog.Error(resultVar1)) @@ -10801,7 +10801,7 @@ func (a *OpenTracingAppLayer) GetUsersNotInChannelPage(teamID string, channelID return resultVar0, resultVar1 } -func (a *OpenTracingAppLayer) GetUsersNotInGroupPage(groupID string, page int, perPage int) ([]*model.User, *model.AppError) { +func (a *OpenTracingAppLayer) GetUsersNotInGroupPage(groupID string, page int, perPage int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, *model.AppError) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetUsersNotInGroupPage") @@ -10813,7 +10813,7 @@ func (a *OpenTracingAppLayer) GetUsersNotInGroupPage(groupID string, page int, p }() defer span.Finish() - resultVar0, resultVar1 := a.app.GetUsersNotInGroupPage(groupID, page, perPage) + resultVar0, resultVar1 := a.app.GetUsersNotInGroupPage(groupID, page, perPage, viewRestrictions) if resultVar1 != nil { span.LogFields(spanlog.Error(resultVar1)) diff --git a/app/plugin_api.go b/app/plugin_api.go index ad33a4d4d9..a9a3c67907 100644 --- a/app/plugin_api.go +++ b/app/plugin_api.go @@ -608,7 +608,7 @@ func (api *PluginAPI) DeleteChannelMember(channelID, userID string) *model.AppEr } func (api *PluginAPI) GetGroup(groupId string) (*model.Group, *model.AppError) { - return api.app.GetGroup(groupId, nil) + return api.app.GetGroup(groupId, nil, nil) } func (api *PluginAPI) GetGroupByName(name string) (*model.Group, *model.AppError) { @@ -616,7 +616,7 @@ func (api *PluginAPI) GetGroupByName(name string) (*model.Group, *model.AppError } func (api *PluginAPI) GetGroupMemberUsers(groupID string, page, perPage int) ([]*model.User, *model.AppError) { - users, _, err := api.app.GetGroupMemberUsersPage(groupID, page, perPage) + users, _, err := api.app.GetGroupMemberUsersPage(groupID, page, perPage, nil) return users, err } diff --git a/store/opentracinglayer/opentracinglayer.go b/store/opentracinglayer/opentracinglayer.go index 44f7d09e86..f66f3352b4 100644 --- a/store/opentracinglayer/opentracinglayer.go +++ b/store/opentracinglayer/opentracinglayer.go @@ -4152,7 +4152,7 @@ func (s *OpenTracingLayerGroupStore) GetGroupSyncable(groupID string, syncableID return result, err } -func (s *OpenTracingLayerGroupStore) GetGroups(page int, perPage int, opts model.GroupSearchOpts) ([]*model.Group, error) { +func (s *OpenTracingLayerGroupStore) GetGroups(page int, perPage int, opts model.GroupSearchOpts, viewRestrictions *model.ViewUsersRestrictions) ([]*model.Group, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "GroupStore.GetGroups") s.Root.Store.SetContext(newCtx) @@ -4161,7 +4161,7 @@ func (s *OpenTracingLayerGroupStore) GetGroups(page int, perPage int, opts model }() defer span.Finish() - result, err := s.GroupStore.GetGroups(page, perPage, opts) + result, err := s.GroupStore.GetGroups(page, perPage, opts, viewRestrictions) if err != nil { span.LogFields(spanlog.Error(err)) ext.Error.Set(span, true) @@ -4260,6 +4260,24 @@ func (s *OpenTracingLayerGroupStore) GetMemberCount(groupID string) (int64, erro return result, err } +func (s *OpenTracingLayerGroupStore) GetMemberCountWithRestrictions(groupID string, viewRestrictions *model.ViewUsersRestrictions) (int64, error) { + origCtx := s.Root.Store.Context() + span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "GroupStore.GetMemberCountWithRestrictions") + s.Root.Store.SetContext(newCtx) + defer func() { + s.Root.Store.SetContext(origCtx) + }() + + defer span.Finish() + result, err := s.GroupStore.GetMemberCountWithRestrictions(groupID, viewRestrictions) + if err != nil { + span.LogFields(spanlog.Error(err)) + ext.Error.Set(span, true) + } + + return result, err +} + func (s *OpenTracingLayerGroupStore) GetMemberUsers(groupID string) ([]*model.User, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "GroupStore.GetMemberUsers") @@ -4314,7 +4332,7 @@ func (s *OpenTracingLayerGroupStore) GetMemberUsersNotInChannel(groupID string, return result, err } -func (s *OpenTracingLayerGroupStore) GetMemberUsersPage(groupID string, page int, perPage int) ([]*model.User, error) { +func (s *OpenTracingLayerGroupStore) GetMemberUsersPage(groupID string, page int, perPage int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "GroupStore.GetMemberUsersPage") s.Root.Store.SetContext(newCtx) @@ -4323,7 +4341,7 @@ func (s *OpenTracingLayerGroupStore) GetMemberUsersPage(groupID string, page int }() defer span.Finish() - result, err := s.GroupStore.GetMemberUsersPage(groupID, page, perPage) + result, err := s.GroupStore.GetMemberUsersPage(groupID, page, perPage, viewRestrictions) if err != nil { span.LogFields(spanlog.Error(err)) ext.Error.Set(span, true) @@ -4332,7 +4350,7 @@ func (s *OpenTracingLayerGroupStore) GetMemberUsersPage(groupID string, page int return result, err } -func (s *OpenTracingLayerGroupStore) GetNonMemberUsersPage(groupID string, page int, perPage int) ([]*model.User, error) { +func (s *OpenTracingLayerGroupStore) GetNonMemberUsersPage(groupID string, page int, perPage int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "GroupStore.GetNonMemberUsersPage") s.Root.Store.SetContext(newCtx) @@ -4341,7 +4359,7 @@ func (s *OpenTracingLayerGroupStore) GetNonMemberUsersPage(groupID string, page }() defer span.Finish() - result, err := s.GroupStore.GetNonMemberUsersPage(groupID, page, perPage) + result, err := s.GroupStore.GetNonMemberUsersPage(groupID, page, perPage, viewRestrictions) if err != nil { span.LogFields(spanlog.Error(err)) ext.Error.Set(span, true) diff --git a/store/retrylayer/retrylayer.go b/store/retrylayer/retrylayer.go index 292924476a..c28caa0d31 100644 --- a/store/retrylayer/retrylayer.go +++ b/store/retrylayer/retrylayer.go @@ -4677,11 +4677,11 @@ func (s *RetryLayerGroupStore) GetGroupSyncable(groupID string, syncableID strin } -func (s *RetryLayerGroupStore) GetGroups(page int, perPage int, opts model.GroupSearchOpts) ([]*model.Group, error) { +func (s *RetryLayerGroupStore) GetGroups(page int, perPage int, opts model.GroupSearchOpts, viewRestrictions *model.ViewUsersRestrictions) ([]*model.Group, error) { tries := 0 for { - result, err := s.GroupStore.GetGroups(page, perPage, opts) + result, err := s.GroupStore.GetGroups(page, perPage, opts, viewRestrictions) if err == nil { return result, nil } @@ -4803,6 +4803,27 @@ func (s *RetryLayerGroupStore) GetMemberCount(groupID string) (int64, error) { } +func (s *RetryLayerGroupStore) GetMemberCountWithRestrictions(groupID string, viewRestrictions *model.ViewUsersRestrictions) (int64, error) { + + tries := 0 + for { + result, err := s.GroupStore.GetMemberCountWithRestrictions(groupID, viewRestrictions) + if err == nil { + return result, nil + } + if !isRepeatableError(err) { + return result, err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return result, err + } + timepkg.Sleep(100 * timepkg.Millisecond) + } + +} + func (s *RetryLayerGroupStore) GetMemberUsers(groupID string) ([]*model.User, error) { tries := 0 @@ -4866,11 +4887,11 @@ func (s *RetryLayerGroupStore) GetMemberUsersNotInChannel(groupID string, channe } -func (s *RetryLayerGroupStore) GetMemberUsersPage(groupID string, page int, perPage int) ([]*model.User, error) { +func (s *RetryLayerGroupStore) GetMemberUsersPage(groupID string, page int, perPage int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, error) { tries := 0 for { - result, err := s.GroupStore.GetMemberUsersPage(groupID, page, perPage) + result, err := s.GroupStore.GetMemberUsersPage(groupID, page, perPage, viewRestrictions) if err == nil { return result, nil } @@ -4887,11 +4908,11 @@ func (s *RetryLayerGroupStore) GetMemberUsersPage(groupID string, page int, perP } -func (s *RetryLayerGroupStore) GetNonMemberUsersPage(groupID string, page int, perPage int) ([]*model.User, error) { +func (s *RetryLayerGroupStore) GetNonMemberUsersPage(groupID string, page int, perPage int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, error) { tries := 0 for { - result, err := s.GroupStore.GetNonMemberUsersPage(groupID, page, perPage) + result, err := s.GroupStore.GetNonMemberUsersPage(groupID, page, perPage, viewRestrictions) if err == nil { return result, nil } diff --git a/store/sqlstore/group_store.go b/store/sqlstore/group_store.go index 8a560395fa..c3341dc9d6 100644 --- a/store/sqlstore/group_store.go +++ b/store/sqlstore/group_store.go @@ -401,58 +401,59 @@ func (s *SqlGroupStore) GetMemberUsers(groupID string) ([]*model.User, error) { return groupMembers, nil } -func (s *SqlGroupStore) GetMemberUsersPage(groupID string, page int, perPage int) ([]*model.User, error) { +func (s *SqlGroupStore) GetMemberUsersPage(groupID string, page int, perPage int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, error) { groupMembers := []*model.User{} - query := ` - SELECT - Users.* - FROM - GroupMembers - JOIN Users ON Users.Id = GroupMembers.UserId - WHERE - GroupMembers.DeleteAt = 0 - AND Users.DeleteAt = 0 - AND GroupId = ? - ORDER BY - GroupMembers.CreateAt DESC - LIMIT - ? - OFFSET - ?` + query := s.getQueryBuilder(). + Select("u.*"). + From("GroupMembers"). + Join("Users u ON u.Id = GroupMembers.UserId"). + Where(sq.Eq{"GroupMembers.DeleteAt": 0}). + Where(sq.Eq{"u.DeleteAt": 0}). + Where(sq.Eq{"GroupId": groupID}). + Limit(uint64(perPage)). + Offset(uint64(page * perPage)). + OrderBy("u.CreateAt DESC") - if err := s.GetReplicaX().Select(&groupMembers, query, groupID, perPage, page*perPage); err != nil { + query = applyViewRestrictionsFilter(query, viewRestrictions, true) + + queryString, args, err := query.ToSql() + if err != nil { + return nil, errors.Wrap(err, "") + } + + if err := s.GetReplicaX().Select(&groupMembers, queryString, args...); err != nil { return nil, errors.Wrapf(err, "failed to find member Users for Group with id=%s", groupID) } return groupMembers, nil } -func (s *SqlGroupStore) GetNonMemberUsersPage(groupID string, page int, perPage int) ([]*model.User, error) { +func (s *SqlGroupStore) GetNonMemberUsersPage(groupID string, page int, perPage int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, error) { groupMembers := []*model.User{} if err := s.GetReplicaX().Get(&model.Group{}, "SELECT * FROM UserGroups WHERE Id = ?", groupID); err != nil { return nil, errors.Wrap(err, "GetNonMemberUsersPage") } - query := ` - SELECT - Users.* - FROM - Users - LEFT JOIN - GroupMembers ON (GroupMembers.UserId = Users.Id AND GroupMembers.GroupId = ?) - WHERE - Users.DeleteAt = 0 - AND ( GroupMembers.UserId IS NULL OR GroupMembers.DeleteAt != 0) - ORDER BY - GroupMembers.CreateAt DESC - LIMIT - ? - OFFSET - ?` + query := s.getQueryBuilder(). + Select("u.*"). + From("Users u"). + LeftJoin("GroupMembers ON (GroupMembers.UserId = u.Id AND GroupMembers.GroupId = ?)", groupID). + Where(sq.Eq{"u.DeleteAt": 0}). + Where("(GroupMembers.UserID IS NULL OR GroupMembers.DeleteAt != 0)"). + Limit(uint64(perPage)). + Offset(uint64(page * perPage)). + OrderBy("u.CreateAt DESC") - if err := s.GetReplicaX().Select(&groupMembers, query, groupID, perPage, page*perPage); err != nil { + query = applyViewRestrictionsFilter(query, viewRestrictions, true) + + queryString, args, err := query.ToSql() + if err != nil { + return nil, errors.Wrap(err, "") + } + + if err := s.GetReplicaX().Select(&groupMembers, queryString, args...); err != nil { return nil, errors.Wrapf(err, "failed to find member Users for Group with id=%s", groupID) } @@ -460,19 +461,27 @@ func (s *SqlGroupStore) GetNonMemberUsersPage(groupID string, page int, perPage } func (s *SqlGroupStore) GetMemberCount(groupID string) (int64, error) { - query := ` - SELECT - count(*) - FROM - GroupMembers - JOIN Users ON Users.Id = GroupMembers.UserId - WHERE - GroupMembers.GroupId = ? - AND Users.DeleteAt = 0 - AND GroupMembers.DeleteAt = 0` + return s.GetMemberCountWithRestrictions(groupID, nil) +} + +func (s *SqlGroupStore) GetMemberCountWithRestrictions(groupID string, viewRestrictions *model.ViewUsersRestrictions) (int64, error) { + query := s.getQueryBuilder(). + Select("COUNT(DISTINCT u.Id)"). + From("GroupMembers"). + Join("Users u ON u.Id = GroupMembers.UserId"). + Where(sq.Eq{"GroupMembers.GroupId": groupID}). + Where(sq.Eq{"u.DeleteAt": 0}). + Where(sq.Eq{"GroupMembers.DeleteAt": 0}) + + query = applyViewRestrictionsFilter(query, viewRestrictions, false) + + queryString, args, err := query.ToSql() + if err != nil { + return int64(0), errors.Wrap(err, "") + } var count int64 - err := s.GetReplicaX().Get(&count, query, groupID) + err = s.GetReplicaX().Get(&count, queryString, args...) if err != nil { return int64(0), errors.Wrapf(err, "failed to count member Users for Group with id=%s", groupID) } @@ -1385,15 +1394,30 @@ func (s *SqlGroupStore) GetGroupsAssociatedToChannelsByTeam(teamId string, opts return groups, nil } -func (s *SqlGroupStore) GetGroups(page, perPage int, opts model.GroupSearchOpts) ([]*model.Group, error) { +func (s *SqlGroupStore) GetGroups(page, perPage int, opts model.GroupSearchOpts, viewRestrictions *model.ViewUsersRestrictions) ([]*model.Group, error) { groupsVar := groups{} groupsQuery := s.getQueryBuilder().Select("g.*") if opts.IncludeMemberCount { + countQuery := s.getQueryBuilder(). + Select("GroupMembers.GroupId, COUNT(DISTINCT u.Id) AS MemberCount"). + From("GroupMembers"). + LeftJoin("Users u ON u.Id = GroupMembers.UserId"). + Where(sq.Eq{"GroupMembers.DeleteAt": 0}). + Where(sq.Eq{"u.DeleteAt": 0}). + GroupBy("GroupId") + + countQuery = applyViewRestrictionsFilter(countQuery, viewRestrictions, false) + + countString, params, err := countQuery.PlaceholderFormat(sq.Question).ToSql() + if err != nil { + return nil, errors.Wrap(err, "get_groups_tosql") + } + groupsQuery = s.getQueryBuilder(). Select("g.*, coalesce(Members.MemberCount, 0) AS MemberCount"). - LeftJoin("(SELECT GroupMembers.GroupId, COUNT(*) AS MemberCount FROM GroupMembers LEFT JOIN Users ON Users.Id = GroupMembers.UserId WHERE GroupMembers.DeleteAt = 0 AND Users.DeleteAt = 0 GROUP BY GroupId) AS Members ON Members.GroupId = g.Id") + LeftJoin("("+countString+") AS Members ON Members.GroupId = g.Id", params...) } if opts.FilterHasMember != "" { diff --git a/store/store.go b/store/store.go index f7c33a938e..5b4ca3dc6d 100644 --- a/store/store.go +++ b/store/store.go @@ -833,10 +833,11 @@ type GroupStore interface { Delete(groupID string) (*model.Group, error) GetMemberUsers(groupID string) ([]*model.User, error) - GetMemberUsersPage(groupID string, page int, perPage int) ([]*model.User, error) + GetMemberUsersPage(groupID string, page int, perPage int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, error) + GetMemberCountWithRestrictions(groupID string, viewRestrictions *model.ViewUsersRestrictions) (int64, error) GetMemberCount(groupID string) (int64, error) - GetNonMemberUsersPage(groupID string, page int, perPage int) ([]*model.User, error) + GetNonMemberUsersPage(groupID string, page int, perPage int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, error) GetMemberUsersInTeam(groupID string, teamID string) ([]*model.User, error) GetMemberUsersNotInChannel(groupID string, channelID string) ([]*model.User, error) @@ -880,7 +881,7 @@ type GroupStore interface { GetGroupsAssociatedToChannelsByTeam(teamID string, opts model.GroupSearchOpts) (map[string][]*model.GroupWithSchemeAdmin, error) CountGroupsByTeam(teamID string, opts model.GroupSearchOpts) (int64, error) - GetGroups(page, perPage int, opts model.GroupSearchOpts) ([]*model.Group, error) + GetGroups(page, perPage int, opts model.GroupSearchOpts, viewRestrictions *model.ViewUsersRestrictions) ([]*model.Group, error) TeamMembersMinusGroupMembers(teamID string, groupIDs []string, page, perPage int) ([]*model.UserWithGroups, error) CountTeamMembersMinusGroupMembers(teamID string, groupIDs []string) (int64, error) diff --git a/store/storetest/group_store.go b/store/storetest/group_store.go index e7a199da24..8696e15bf1 100644 --- a/store/storetest/group_store.go +++ b/store/storetest/group_store.go @@ -836,25 +836,25 @@ func testGroupGetMemberUsersPage(t *testing.T, ss store.Store) { require.NoError(t, err) // Check returns members - groupMembers, err := ss.Group().GetMemberUsersPage(group.Id, 0, 100) + groupMembers, err := ss.Group().GetMemberUsersPage(group.Id, 0, 100, nil) require.NoError(t, err) require.Equal(t, 3, len(groupMembers)) // Check page 1 - groupMembers, err = ss.Group().GetMemberUsersPage(group.Id, 0, 2) + groupMembers, err = ss.Group().GetMemberUsersPage(group.Id, 0, 2, nil) require.NoError(t, err) require.Equal(t, 2, len(groupMembers)) require.Equal(t, user3.Id, groupMembers[0].Id) require.Equal(t, user2.Id, groupMembers[1].Id) // Check page 2 - groupMembers, err = ss.Group().GetMemberUsersPage(group.Id, 1, 2) + groupMembers, err = ss.Group().GetMemberUsersPage(group.Id, 1, 2, nil) require.NoError(t, err) require.Equal(t, 1, len(groupMembers)) require.Equal(t, user1.Id, groupMembers[0].Id) // Check madeup id - groupMembers, err = ss.Group().GetMemberUsersPage(model.NewId(), 0, 100) + groupMembers, err = ss.Group().GetMemberUsersPage(model.NewId(), 0, 100, nil) require.NoError(t, err) require.Equal(t, 0, len(groupMembers)) @@ -863,7 +863,7 @@ func testGroupGetMemberUsersPage(t *testing.T, ss store.Store) { require.NoError(t, err) // Should not return deleted members - groupMembers, err = ss.Group().GetMemberUsersPage(group.Id, 0, 100) + groupMembers, err = ss.Group().GetMemberUsersPage(group.Id, 0, 100, nil) require.NoError(t, err) require.Equal(t, 2, len(groupMembers)) } @@ -3476,6 +3476,13 @@ func testGetGroups(t *testing.T, ss store.Store) { user2, err := ss.User().Save(u2) require.NoError(t, err) + u3 := &model.User{ + Email: MakeEmail(), + Username: model.NewId(), + } + user3, err := ss.User().Save(u3) + require.NoError(t, err) + _, err = ss.Group().UpsertMember(group1.Id, user1.Id) require.NoError(t, err) @@ -3485,9 +3492,20 @@ func testGetGroups(t *testing.T, ss store.Store) { _, err = ss.Group().UpsertMember(group2.Id, user2.Id) require.NoError(t, err) + _, err = ss.Group().UpsertMember(group2.Id, user3.Id) + require.NoError(t, err) + _, err = ss.Group().UpsertMember(deletedGroup.Id, user1.Id) require.NoError(t, err) + m1 := model.ChannelMember{ + ChannelId: channel1.Id, + UserId: user1.Id, + NotifyProps: model.GetDefaultChannelNotifyProps(), + } + _, err = ss.Channel().SaveMember(&m1) + require.NoError(t, err) + user2.DeleteAt = 1 u2Update, _ := ss.User().Update(user2, true) @@ -3519,39 +3537,44 @@ func testGetGroups(t *testing.T, ss store.Store) { require.NoError(t, nErr) testCases := []struct { - Name string - Page int - PerPage int - Opts model.GroupSearchOpts - Resultf func([]*model.Group) bool + Name string + Page int + PerPage int + Opts model.GroupSearchOpts + Resultf func([]*model.Group) bool + Restrictions *model.ViewUsersRestrictions }{ { - Name: "Get all the Groups", - Opts: model.GroupSearchOpts{}, - Page: 0, - PerPage: 3, - Resultf: func(groups []*model.Group) bool { return len(groups) == 3 }, + Name: "Get all the Groups", + Opts: model.GroupSearchOpts{}, + Page: 0, + PerPage: 3, + Resultf: func(groups []*model.Group) bool { return len(groups) == 3 }, + Restrictions: nil, }, { - Name: "Get first Group with page 0 with 1 element", - Opts: model.GroupSearchOpts{}, - Page: 0, - PerPage: 1, - Resultf: func(groups []*model.Group) bool { return len(groups) == 1 }, + Name: "Get first Group with page 0 with 1 element", + Opts: model.GroupSearchOpts{}, + Page: 0, + PerPage: 1, + Resultf: func(groups []*model.Group) bool { return len(groups) == 1 }, + Restrictions: nil, }, { - Name: "Get single result from page 1", - Opts: model.GroupSearchOpts{}, - Page: 1, - PerPage: 1, - Resultf: func(groups []*model.Group) bool { return len(groups) == 1 }, + Name: "Get single result from page 1", + Opts: model.GroupSearchOpts{}, + Page: 1, + PerPage: 1, + Resultf: func(groups []*model.Group) bool { return len(groups) == 1 }, + Restrictions: nil, }, { - Name: "Get multiple results from page 1", - Opts: model.GroupSearchOpts{}, - Page: 1, - PerPage: 2, - Resultf: func(groups []*model.Group) bool { return len(groups) == 2 }, + Name: "Get multiple results from page 1", + Opts: model.GroupSearchOpts{}, + Page: 1, + PerPage: 2, + Resultf: func(groups []*model.Group) bool { return len(groups) == 2 }, + Restrictions: nil, }, { Name: "Get group matching name", @@ -3566,6 +3589,7 @@ func testGetGroups(t *testing.T, ss store.Store) { } return true }, + Restrictions: nil, }, { Name: "Get group matching display name", @@ -3580,6 +3604,7 @@ func testGetGroups(t *testing.T, ss store.Store) { } return true }, + Restrictions: nil, }, { Name: "Get group matching multiple display names", @@ -3594,6 +3619,7 @@ func testGetGroups(t *testing.T, ss store.Store) { } return true }, + Restrictions: nil, }, { Name: "Include member counts", @@ -3605,7 +3631,7 @@ func testGetGroups(t *testing.T, ss store.Store) { if g.MemberCount == nil { return false } - if g.Id == group1.Id && *g.MemberCount != 1 { + if (g.Id == group1.Id || g.Id == group2.Id) && *g.MemberCount != 1 { return false } if g.DeleteAt != 0 { @@ -3614,6 +3640,31 @@ func testGetGroups(t *testing.T, ss store.Store) { } return true }, + Restrictions: nil, + }, + { + Name: "Include member counts with restrictions", + Opts: model.GroupSearchOpts{IncludeMemberCount: true}, + Page: 0, + PerPage: 100, + Resultf: func(groups []*model.Group) bool { + for _, g := range groups { + if g.MemberCount == nil { + return false + } + if g.Id == group1.Id && *g.MemberCount != 1 { + return false + } + if g.Id == group2.Id && *g.MemberCount != 0 { + return false + } + if g.DeleteAt != 0 { + return false + } + } + return true + }, + Restrictions: &model.ViewUsersRestrictions{Channels: []string{channel1.Id}}, }, { Name: "Not associated to team", @@ -3634,6 +3685,7 @@ func testGetGroups(t *testing.T, ss store.Store) { } return true }, + Restrictions: nil, }, { Name: "Not associated to other team", @@ -3654,6 +3706,7 @@ func testGetGroups(t *testing.T, ss store.Store) { } return true }, + Restrictions: nil, }, { Name: "Include allow reference", @@ -3674,6 +3727,7 @@ func testGetGroups(t *testing.T, ss store.Store) { } return true }, + Restrictions: nil, }, { Name: "Use Since return all", @@ -3691,6 +3745,7 @@ func testGetGroups(t *testing.T, ss store.Store) { } return true }, + Restrictions: nil, }, { Name: "Use Since return none", @@ -3700,6 +3755,7 @@ func testGetGroups(t *testing.T, ss store.Store) { Resultf: func(groups []*model.Group) bool { return len(groups) == 0 }, + Restrictions: nil, }, { Name: "Filter groups from group-constrained teams", @@ -3709,6 +3765,7 @@ func testGetGroups(t *testing.T, ss store.Store) { Resultf: func(groups []*model.Group) bool { return len(groups) == 2 && groups[0].Id == group1.Id && groups[1].Id == group2.Id }, + Restrictions: nil, }, { Name: "Filter groups from group-constrained page 0", @@ -3718,6 +3775,7 @@ func testGetGroups(t *testing.T, ss store.Store) { Resultf: func(groups []*model.Group) bool { return groups[0].Id == group1.Id }, + Restrictions: nil, }, { Name: "Filter groups from group-constrained page 1", @@ -3727,6 +3785,7 @@ func testGetGroups(t *testing.T, ss store.Store) { Resultf: func(groups []*model.Group) bool { return groups[0].Id == group2.Id }, + Restrictions: nil, }, { Name: "Non-group constrained team with no associated groups still returns groups for the child channel", @@ -3736,6 +3795,7 @@ func testGetGroups(t *testing.T, ss store.Store) { Resultf: func(groups []*model.Group) bool { return len(groups) > 0 }, + Restrictions: nil, }, { Name: "Filter by group member", @@ -3745,6 +3805,7 @@ func testGetGroups(t *testing.T, ss store.Store) { Resultf: func(groups []*model.Group) bool { return len(groups) == 1 && groups[0].Id == group1.Id }, + Restrictions: nil, }, { Name: "Filter by non-existent group member", @@ -3754,6 +3815,7 @@ func testGetGroups(t *testing.T, ss store.Store) { Resultf: func(groups []*model.Group) bool { return len(groups) == 0 }, + Restrictions: nil, }, { Name: "Filter by non-member member", @@ -3763,12 +3825,13 @@ func testGetGroups(t *testing.T, ss store.Store) { Resultf: func(groups []*model.Group) bool { return len(groups) == 2 }, + Restrictions: nil, }, } for _, tc := range testCases { t.Run(tc.Name, func(t *testing.T) { - groups, err := ss.Group().GetGroups(tc.Page, tc.PerPage, tc.Opts) + groups, err := ss.Group().GetGroups(tc.Page, tc.PerPage, tc.Opts, tc.Restrictions) require.NoError(t, err) require.True(t, tc.Resultf(groups)) }) @@ -5048,7 +5111,7 @@ func groupTestGetNonMemberUsersPage(t *testing.T, ss store.Store) { _, nErr = ss.User().Save(u2) require.NoError(t, nErr) - users, err := ss.Group().GetNonMemberUsersPage(group.Id, 0, 1000) + users, err := ss.Group().GetNonMemberUsersPage(group.Id, 0, 1000, nil) require.NoError(t, err) originalLen := len(users) @@ -5056,11 +5119,11 @@ func groupTestGetNonMemberUsersPage(t *testing.T, ss store.Store) { _, err = ss.Group().UpsertMember(group.Id, user1.Id) require.NoError(t, err) - users, err = ss.Group().GetNonMemberUsersPage(group.Id, 0, 1000) + users, err = ss.Group().GetNonMemberUsersPage(group.Id, 0, 1000, nil) require.NoError(t, err) require.Len(t, users, originalLen-1) - users, err = ss.Group().GetNonMemberUsersPage(model.NewId(), 0, 1000) + users, err = ss.Group().GetNonMemberUsersPage(model.NewId(), 0, 1000, nil) require.Error(t, err) require.Nil(t, users) } diff --git a/store/storetest/mocks/GroupStore.go b/store/storetest/mocks/GroupStore.go index f3f2f839a9..80d6fdd492 100644 --- a/store/storetest/mocks/GroupStore.go +++ b/store/storetest/mocks/GroupStore.go @@ -577,13 +577,13 @@ func (_m *GroupStore) GetGroupSyncable(groupID string, syncableID string, syncab return r0, r1 } -// GetGroups provides a mock function with given fields: page, perPage, opts -func (_m *GroupStore) GetGroups(page int, perPage int, opts model.GroupSearchOpts) ([]*model.Group, error) { - ret := _m.Called(page, perPage, opts) +// GetGroups provides a mock function with given fields: page, perPage, opts, viewRestrictions +func (_m *GroupStore) GetGroups(page int, perPage int, opts model.GroupSearchOpts, viewRestrictions *model.ViewUsersRestrictions) ([]*model.Group, error) { + ret := _m.Called(page, perPage, opts, viewRestrictions) var r0 []*model.Group - if rf, ok := ret.Get(0).(func(int, int, model.GroupSearchOpts) []*model.Group); ok { - r0 = rf(page, perPage, opts) + if rf, ok := ret.Get(0).(func(int, int, model.GroupSearchOpts, *model.ViewUsersRestrictions) []*model.Group); ok { + r0 = rf(page, perPage, opts, viewRestrictions) } else { if ret.Get(0) != nil { r0 = ret.Get(0).([]*model.Group) @@ -591,8 +591,8 @@ func (_m *GroupStore) GetGroups(page int, perPage int, opts model.GroupSearchOpt } var r1 error - if rf, ok := ret.Get(1).(func(int, int, model.GroupSearchOpts) error); ok { - r1 = rf(page, perPage, opts) + if rf, ok := ret.Get(1).(func(int, int, model.GroupSearchOpts, *model.ViewUsersRestrictions) error); ok { + r1 = rf(page, perPage, opts, viewRestrictions) } else { r1 = ret.Error(1) } @@ -713,6 +713,27 @@ func (_m *GroupStore) GetMemberCount(groupID string) (int64, error) { return r0, r1 } +// GetMemberCountWithRestrictions provides a mock function with given fields: groupID, viewRestrictions +func (_m *GroupStore) GetMemberCountWithRestrictions(groupID string, viewRestrictions *model.ViewUsersRestrictions) (int64, error) { + ret := _m.Called(groupID, viewRestrictions) + + var r0 int64 + if rf, ok := ret.Get(0).(func(string, *model.ViewUsersRestrictions) int64); ok { + r0 = rf(groupID, viewRestrictions) + } else { + r0 = ret.Get(0).(int64) + } + + var r1 error + if rf, ok := ret.Get(1).(func(string, *model.ViewUsersRestrictions) error); ok { + r1 = rf(groupID, viewRestrictions) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // GetMemberUsers provides a mock function with given fields: groupID func (_m *GroupStore) GetMemberUsers(groupID string) ([]*model.User, error) { ret := _m.Called(groupID) @@ -782,13 +803,13 @@ func (_m *GroupStore) GetMemberUsersNotInChannel(groupID string, channelID strin return r0, r1 } -// GetMemberUsersPage provides a mock function with given fields: groupID, page, perPage -func (_m *GroupStore) GetMemberUsersPage(groupID string, page int, perPage int) ([]*model.User, error) { - ret := _m.Called(groupID, page, perPage) +// GetMemberUsersPage provides a mock function with given fields: groupID, page, perPage, viewRestrictions +func (_m *GroupStore) GetMemberUsersPage(groupID string, page int, perPage int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, error) { + ret := _m.Called(groupID, page, perPage, viewRestrictions) var r0 []*model.User - if rf, ok := ret.Get(0).(func(string, int, int) []*model.User); ok { - r0 = rf(groupID, page, perPage) + if rf, ok := ret.Get(0).(func(string, int, int, *model.ViewUsersRestrictions) []*model.User); ok { + r0 = rf(groupID, page, perPage, viewRestrictions) } else { if ret.Get(0) != nil { r0 = ret.Get(0).([]*model.User) @@ -796,8 +817,8 @@ func (_m *GroupStore) GetMemberUsersPage(groupID string, page int, perPage int) } var r1 error - if rf, ok := ret.Get(1).(func(string, int, int) error); ok { - r1 = rf(groupID, page, perPage) + if rf, ok := ret.Get(1).(func(string, int, int, *model.ViewUsersRestrictions) error); ok { + r1 = rf(groupID, page, perPage, viewRestrictions) } else { r1 = ret.Error(1) } @@ -805,13 +826,13 @@ func (_m *GroupStore) GetMemberUsersPage(groupID string, page int, perPage int) return r0, r1 } -// GetNonMemberUsersPage provides a mock function with given fields: groupID, page, perPage -func (_m *GroupStore) GetNonMemberUsersPage(groupID string, page int, perPage int) ([]*model.User, error) { - ret := _m.Called(groupID, page, perPage) +// GetNonMemberUsersPage provides a mock function with given fields: groupID, page, perPage, viewRestrictions +func (_m *GroupStore) GetNonMemberUsersPage(groupID string, page int, perPage int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, error) { + ret := _m.Called(groupID, page, perPage, viewRestrictions) var r0 []*model.User - if rf, ok := ret.Get(0).(func(string, int, int) []*model.User); ok { - r0 = rf(groupID, page, perPage) + if rf, ok := ret.Get(0).(func(string, int, int, *model.ViewUsersRestrictions) []*model.User); ok { + r0 = rf(groupID, page, perPage, viewRestrictions) } else { if ret.Get(0) != nil { r0 = ret.Get(0).([]*model.User) @@ -819,8 +840,8 @@ func (_m *GroupStore) GetNonMemberUsersPage(groupID string, page int, perPage in } var r1 error - if rf, ok := ret.Get(1).(func(string, int, int) error); ok { - r1 = rf(groupID, page, perPage) + if rf, ok := ret.Get(1).(func(string, int, int, *model.ViewUsersRestrictions) error); ok { + r1 = rf(groupID, page, perPage, viewRestrictions) } else { r1 = ret.Error(1) } diff --git a/store/timerlayer/timerlayer.go b/store/timerlayer/timerlayer.go index 0558fa66c1..cec6251fc7 100644 --- a/store/timerlayer/timerlayer.go +++ b/store/timerlayer/timerlayer.go @@ -3781,10 +3781,10 @@ func (s *TimerLayerGroupStore) GetGroupSyncable(groupID string, syncableID strin return result, err } -func (s *TimerLayerGroupStore) GetGroups(page int, perPage int, opts model.GroupSearchOpts) ([]*model.Group, error) { +func (s *TimerLayerGroupStore) GetGroups(page int, perPage int, opts model.GroupSearchOpts, viewRestrictions *model.ViewUsersRestrictions) ([]*model.Group, error) { start := time.Now() - result, err := s.GroupStore.GetGroups(page, perPage, opts) + result, err := s.GroupStore.GetGroups(page, perPage, opts, viewRestrictions) elapsed := float64(time.Since(start)) / float64(time.Second) if s.Root.Metrics != nil { @@ -3877,6 +3877,22 @@ func (s *TimerLayerGroupStore) GetMemberCount(groupID string) (int64, error) { return result, err } +func (s *TimerLayerGroupStore) GetMemberCountWithRestrictions(groupID string, viewRestrictions *model.ViewUsersRestrictions) (int64, error) { + start := time.Now() + + result, err := s.GroupStore.GetMemberCountWithRestrictions(groupID, viewRestrictions) + + elapsed := float64(time.Since(start)) / float64(time.Second) + if s.Root.Metrics != nil { + success := "false" + if err == nil { + success = "true" + } + s.Root.Metrics.ObserveStoreMethodDuration("GroupStore.GetMemberCountWithRestrictions", success, elapsed) + } + return result, err +} + func (s *TimerLayerGroupStore) GetMemberUsers(groupID string) ([]*model.User, error) { start := time.Now() @@ -3925,10 +3941,10 @@ func (s *TimerLayerGroupStore) GetMemberUsersNotInChannel(groupID string, channe return result, err } -func (s *TimerLayerGroupStore) GetMemberUsersPage(groupID string, page int, perPage int) ([]*model.User, error) { +func (s *TimerLayerGroupStore) GetMemberUsersPage(groupID string, page int, perPage int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, error) { start := time.Now() - result, err := s.GroupStore.GetMemberUsersPage(groupID, page, perPage) + result, err := s.GroupStore.GetMemberUsersPage(groupID, page, perPage, viewRestrictions) elapsed := float64(time.Since(start)) / float64(time.Second) if s.Root.Metrics != nil { @@ -3941,10 +3957,10 @@ func (s *TimerLayerGroupStore) GetMemberUsersPage(groupID string, page int, perP return result, err } -func (s *TimerLayerGroupStore) GetNonMemberUsersPage(groupID string, page int, perPage int) ([]*model.User, error) { +func (s *TimerLayerGroupStore) GetNonMemberUsersPage(groupID string, page int, perPage int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, error) { start := time.Now() - result, err := s.GroupStore.GetNonMemberUsersPage(groupID, page, perPage) + result, err := s.GroupStore.GetNonMemberUsersPage(groupID, page, perPage, viewRestrictions) elapsed := float64(time.Since(start)) / float64(time.Second) if s.Root.Metrics != nil {