Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automated cherry pick of #4884 #4889

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion server/api/users.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,15 +95,25 @@ func (a *API) handleGetUsersList(w http.ResponseWriter, r *http.Request) {
session := ctx.Value(sessionContextKey).(*model.Session)
isSystemAdmin := a.permissions.HasPermissionTo(session.UserID, mmModel.PermissionManageSystem)

sanitizedUsers := make([]*model.User, 0)
for _, user := range users {
canSeeUser, err2 := a.app.CanSeeUser(session.UserID, user.ID)
if err2 != nil {
a.errorResponse(w, r, err2)
return
}
if !canSeeUser {
continue
}
if user.ID == session.UserID {
user.Sanitize(map[string]bool{})
} else {
a.app.SanitizeProfile(user, isSystemAdmin)
}
sanitizedUsers = append(sanitizedUsers, user)
}

usersList, err := json.Marshal(users)
usersList, err := json.Marshal(sanitizedUsers)
if err != nil {
a.errorResponse(w, r, err)
return
Expand Down
20 changes: 20 additions & 0 deletions server/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,26 @@ func (c *Client) GetUser(id string) (*model.User, *Response) {
return user, BuildResponse(r)
}

func (c *Client) GetUserList(ids []string) ([]model.User, *Response) {
r, err := c.DoAPIPost("/users", toJSON(ids))
if err != nil {
return nil, BuildErrorResponse(r, err)
}
defer closeBody(r)

requestBody, err := io.ReadAll(r.Body)
if err != nil {
return nil, BuildErrorResponse(r, err)
}

var users []model.User
err = json.Unmarshal(requestBody, &users)
if err != nil {
return nil, BuildErrorResponse(r, err)
}
return users, BuildResponse(r)
}

func (c *Client) GetUserChangePasswordRoute(id string) string {
return fmt.Sprintf("/users/%s/changepassword", id)
}
Expand Down
11 changes: 11 additions & 0 deletions server/integrationtests/pluginteststore.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,17 @@ func (s *PluginTestStore) GetUserByID(userID string) (*model.User, error) {
return user, nil
}

func (s *PluginTestStore) GetUsersList(userIDs []string, showEmail, showName bool) ([]*model.User, error) {
var users []*model.User
for _, id := range userIDs {
user := s.users[id]
if user != nil {
users = append(users, user)
}
}
return users, nil
}

func (s *PluginTestStore) GetUserByEmail(email string) (*model.User, error) {
for _, user := range s.users {
if user.Email == email {
Expand Down
73 changes: 73 additions & 0 deletions server/integrationtests/user_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,79 @@ func TestGetUser(t *testing.T) {
})
}

func TestGetUserList(t *testing.T) {
th := SetupTestHelperPluginMode(t)
defer th.TearDown()
clients := setupClients(th)
th.Client = clients.TeamMember
th.Client2 = clients.Editor

me, resp := th.Client.GetMe()
require.NoError(t, resp.Error)
require.NotNil(t, me)

userID1 := me.ID
userID2 := th.GetUser2().ID

// Admin user should return both
returnUsers, resp := clients.Admin.GetUserList([]string{userID1, userID2})
require.NoError(t, resp.Error)
require.NotNil(t, returnUsers)
require.Equal(t, 2, len(returnUsers))

// Guest user should return none
returnUsers2, resp := clients.Guest.GetUserList([]string{userID1, userID2})
require.NoError(t, resp.Error)
require.NotNil(t, returnUsers2)
require.Equal(t, 0, len(returnUsers2))

newBoard := &model.Board{
Title: "title",
Type: model.BoardTypeOpen,
TeamID: testTeamID,
}
board, err := th.Server.App().CreateBoard(newBoard, userID1, true)
require.NoError(t, err)

// add Guest as board member
newGuestMember := &model.BoardMember{
UserID: userGuestID,
BoardID: board.ID,
SchemeViewer: true,
SchemeCommenter: true,
SchemeEditor: true,
SchemeAdmin: false,
}
guestMember, err := th.Server.App().AddMemberToBoard(newGuestMember)
require.NoError(t, err)
require.NotNil(t, guestMember)

// Guest user should now return one of members
guestUsers, resp := clients.Guest.GetUserList([]string{th.GetUser1().ID, th.GetUser2().ID})
require.NoError(t, resp.Error)
require.NotNil(t, guestUsers)
require.Equal(t, 1, len(guestUsers))

// add other user as board member
newBoardMember := &model.BoardMember{
UserID: userID2,
BoardID: board.ID,
SchemeViewer: true,
SchemeCommenter: true,
SchemeEditor: true,
SchemeAdmin: false,
}
newMember, err := th.Server.App().AddMemberToBoard(newBoardMember)
require.NoError(t, err)
require.NotNil(t, newMember)

// Guest user should now return both
guestUsers, resp = clients.Guest.GetUserList([]string{th.GetUser1().ID, th.GetUser2().ID})
require.NoError(t, resp.Error)
require.NotNil(t, guestUsers)
require.Equal(t, 2, len(guestUsers))
}

func TestUserChangePassword(t *testing.T) {
th := SetupTestHelper(t).Start()
defer th.TearDown()
Expand Down
26 changes: 12 additions & 14 deletions server/services/store/mattermostauthlayer/mattermostauthlayer.go
Original file line number Diff line number Diff line change
Expand Up @@ -1297,17 +1297,16 @@ func (s *MattermostAuthLayer) CanSeeUser(seerID string, seenID string) (bool, er

query := s.getQueryBuilder().
Select("1").
From(s.tablePrefix + "board_members AS BM1").
Join(s.tablePrefix + "board_members AS BM2 ON BM1.BoardID=BM2.BoardID").
LeftJoin("Bots b ON ( b.UserId = u.id )").
From(s.tablePrefix + "board_members AS bm1").
Join(s.tablePrefix + "board_members AS bm2 ON bm1.board_id=bm2.board_id").
Where(sq.Or{
sq.And{
sq.Eq{"BM1.UserID": seerID},
sq.Eq{"BM2.UserID": seenID},
sq.Eq{"bm1.user_id": seerID},
sq.Eq{"bm2.user_id": seenID},
},
sq.And{
sq.Eq{"BM1.UserID": seenID},
sq.Eq{"BM2.UserID": seerID},
sq.Eq{"bm1.user_id": seenID},
sq.Eq{"bm2.user_id": seerID},
},
}).Limit(1)

Expand All @@ -1323,17 +1322,16 @@ func (s *MattermostAuthLayer) CanSeeUser(seerID string, seenID string) (bool, er

query = s.getQueryBuilder().
Select("1").
From("ChannelMembers AS CM1").
Join("ChannelMembers AS CM2 ON CM1.BoardID=CM2.BoardID").
LeftJoin("Bots b ON ( b.UserId = u.id )").
From("channelmembers AS cm1").
Join("channelmembers AS cm2 ON cm1.channelid=cm2.channelid").
Where(sq.Or{
sq.And{
sq.Eq{"CM1.UserID": seerID},
sq.Eq{"CM2.UserID": seenID},
sq.Eq{"cm1.userid": seerID},
sq.Eq{"cm2.userid": seenID},
},
sq.And{
sq.Eq{"CM1.UserID": seenID},
sq.Eq{"CM2.UserID": seerID},
sq.Eq{"cm1.userid": seenID},
sq.Eq{"cm2.userid": seerID},
},
}).Limit(1)

Expand Down