From 098868387e20065085d9cf373708088a9eb5d360 Mon Sep 17 00:00:00 2001 From: Scott Bishel Date: Tue, 14 Feb 2023 09:17:33 -0700 Subject: [PATCH] initial implementation of SysAdmin/TeamAdmin feature (#4537) * initial implementation of SysAdmin/TeamAdmin feature * fix adminBadge tests * updating tests * more fixes for unit tests * lint fixes * update snapshots * update cypress test for call change * add additional unit tests * update test for lint errors * fix reviews implement tests * fix for merge, reset dialog before redirection * remove unused test code * fix more tests * fix swagger doc for missing parameters --------- Co-authored-by: Mattermost Build --- server/api/members.go | 19 +- server/api/teams.go | 105 +++++++ server/api/users.go | 15 + server/app/boards.go | 49 +++- server/app/boards_test.go | 116 +++++++- server/app/category_boards_test.go | 4 + server/app/helper_test.go | 10 + server/app/onboarding_test.go | 2 +- server/app/user.go | 15 +- server/app/user_test.go | 64 +++++ server/integrationtests/clienttestlib.go | 3 + server/model/permission.go | 2 + server/model/user.go | 3 + .../localpermissions/localpermissions.go | 3 + .../localpermissions/localpermissions_test.go | 28 ++ .../permissions/mmpermissions/helpers_test.go | 14 + .../mmpermissions/mmpermissions.go | 8 +- .../mmpermissions/mmpermissions_test.go | 21 ++ .../__snapshots__/shareBoard.test.tsx.snap | 18 ++ .../userPermissionsRow.test.tsx.snap | 260 ++++++++++++++++++ .../components/shareBoard/shareBoard.test.tsx | 4 +- .../src/components/shareBoard/shareBoard.tsx | 2 + .../shareBoard/userPermissionsRow.test.tsx | 32 +++ .../shareBoard/userPermissionsRow.tsx | 2 + webapp/src/octoClient.ts | 32 ++- webapp/src/pages/boardPage/boardPage.tsx | 154 +++++++---- webapp/src/store/boards.ts | 12 +- webapp/src/user.tsx | 1 + .../__snapshots__/adminBadge.test.tsx.snap | 29 ++ webapp/src/widgets/adminBadge/adminBadge.scss | 16 ++ .../widgets/adminBadge/adminBadge.test.tsx | 32 +++ webapp/src/widgets/adminBadge/adminBadge.tsx | 36 +++ 32 files changed, 1031 insertions(+), 80 deletions(-) create mode 100644 server/app/user_test.go create mode 100644 webapp/src/widgets/adminBadge/__snapshots__/adminBadge.test.tsx.snap create mode 100644 webapp/src/widgets/adminBadge/adminBadge.scss create mode 100644 webapp/src/widgets/adminBadge/adminBadge.test.tsx create mode 100644 webapp/src/widgets/adminBadge/adminBadge.tsx diff --git a/server/api/members.go b/server/api/members.go index 9937fd9dc..f60d00db9 100644 --- a/server/api/members.go +++ b/server/api/members.go @@ -206,6 +206,11 @@ func (a *API) handleJoinBoard(w http.ResponseWriter, r *http.Request) { // description: Board ID // required: true // type: string + // - name: allow_admin + // in: path + // description: allows admin users to join private boards + // required: false + // type: boolean // security: // - BearerAuth: [] // responses: @@ -222,6 +227,9 @@ func (a *API) handleJoinBoard(w http.ResponseWriter, r *http.Request) { // schema: // "$ref": "#/definitions/ErrorResponse" + query := r.URL.Query() + allowAdmin := query.Has("allow_admin") + userID := getUserID(r) if userID == "" { a.errorResponse(w, r, model.NewErrBadRequest("missing user ID")) @@ -234,9 +242,14 @@ func (a *API) handleJoinBoard(w http.ResponseWriter, r *http.Request) { a.errorResponse(w, r, err) return } + + isAdmin := false if board.Type != model.BoardTypeOpen { - a.errorResponse(w, r, model.NewErrPermission("cannot join a non Open board")) - return + if !allowAdmin || !a.permissions.HasPermissionToTeam(userID, board.TeamID, model.PermissionManageTeam) { + a.errorResponse(w, r, model.NewErrPermission("cannot join a non Open board")) + return + } + isAdmin = true } if !a.permissions.HasPermissionToTeam(userID, board.TeamID, model.PermissionViewTeam) { @@ -257,7 +270,7 @@ func (a *API) handleJoinBoard(w http.ResponseWriter, r *http.Request) { newBoardMember := &model.BoardMember{ UserID: userID, BoardID: boardID, - SchemeAdmin: board.MinimumRole == model.BoardRoleAdmin, + SchemeAdmin: board.MinimumRole == model.BoardRoleAdmin || isAdmin, SchemeEditor: board.MinimumRole == model.BoardRoleNone || board.MinimumRole == model.BoardRoleEditor, SchemeCommenter: board.MinimumRole == model.BoardRoleCommenter, SchemeViewer: board.MinimumRole == model.BoardRoleViewer, diff --git a/server/api/teams.go b/server/api/teams.go index 2d18c86f8..50c36ac1a 100644 --- a/server/api/teams.go +++ b/server/api/teams.go @@ -2,6 +2,7 @@ package api import ( "encoding/json" + "io" "net/http" "github.com/gorilla/mux" @@ -15,6 +16,7 @@ func (a *API) registerTeamsRoutes(r *mux.Router) { r.HandleFunc("/teams", a.sessionRequired(a.handleGetTeams)).Methods("GET") r.HandleFunc("/teams/{teamID}", a.sessionRequired(a.handleGetTeam)).Methods("GET") r.HandleFunc("/teams/{teamID}/users", a.sessionRequired(a.handleGetTeamUsers)).Methods("GET") + r.HandleFunc("/teams/{teamID}/users", a.sessionRequired(a.handleGetTeamUsersByID)).Methods("POST") r.HandleFunc("/teams/{teamID}/archive/export", a.sessionRequired(a.handleArchiveExportTeam)).Methods("GET") } @@ -257,3 +259,106 @@ func (a *API) handleGetTeamUsers(w http.ResponseWriter, r *http.Request) { auditRec.AddMeta("userCount", len(users)) auditRec.Success() } + +func (a *API) handleGetTeamUsersByID(w http.ResponseWriter, r *http.Request) { + // swagger:operation POST /teams/{teamID}/users getTeamUsersByID + // + // Returns a user[] + // + // --- + // produces: + // - application/json + // parameters: + // - name: teamID + // in: path + // description: Team ID + // required: true + // type: string + // - name: Body + // in: body + // description: []UserIDs to return + // required: true + // type: []string + // security: + // - BearerAuth: [] + // responses: + // '200': + // description: success + // schema: + // type: array + // items: + // "$ref": "#/definitions/User" + // default: + // description: internal error + // schema: + // "$ref": "#/definitions/ErrorResponse" + + requestBody, err := io.ReadAll(r.Body) + if err != nil { + a.errorResponse(w, r, err) + return + } + + var userIDs []string + if err = json.Unmarshal(requestBody, &userIDs); err != nil { + a.errorResponse(w, r, err) + return + } + + auditRec := a.makeAuditRecord(r, "getTeamUsersByID", audit.Fail) + defer a.audit.LogRecord(audit.LevelRead, auditRec) + + vars := mux.Vars(r) + teamID := vars["teamID"] + userID := getUserID(r) + + if !a.permissions.HasPermissionToTeam(userID, teamID, model.PermissionViewTeam) { + a.errorResponse(w, r, model.NewErrPermission("access denied to team")) + return + } + + var users []*model.User + var error error + + if len(userIDs) == 0 { + a.errorResponse(w, r, model.NewErrBadRequest("User IDs are empty")) + return + } + + if userIDs[0] == model.SingleUser { + ws, _ := a.app.GetRootTeam() + now := utils.GetMillis() + user := &model.User{ + ID: model.SingleUser, + Username: model.SingleUser, + Email: model.SingleUser, + CreateAt: ws.UpdateAt, + UpdateAt: now, + } + users = append(users, user) + } else { + users, error = a.app.GetUsersList(userIDs) + if error != nil { + a.errorResponse(w, r, error) + return + } + + for i, u := range users { + if a.permissions.HasPermissionToTeam(u.ID, teamID, model.PermissionManageTeam) { + users[i].Permissions = append(users[i].Permissions, model.PermissionManageTeam.Id) + } + if a.permissions.HasPermissionTo(u.ID, model.PermissionManageSystem) { + users[i].Permissions = append(users[i].Permissions, model.PermissionManageSystem.Id) + } + } + } + + usersList, err := json.Marshal(users) + if err != nil { + a.errorResponse(w, r, err) + return + } + + jsonStringResponse(w, http.StatusOK, string(usersList)) + auditRec.Success() +} diff --git a/server/api/users.go b/server/api/users.go index 90932d4d6..08d447187 100644 --- a/server/api/users.go +++ b/server/api/users.go @@ -107,6 +107,12 @@ func (a *API) handleGetMe(w http.ResponseWriter, r *http.Request) { // --- // produces: // - application/json + // parameters: + // - name: teamID + // in: path + // description: Team ID + // required: false + // type: string // security: // - BearerAuth: [] // responses: @@ -118,6 +124,8 @@ func (a *API) handleGetMe(w http.ResponseWriter, r *http.Request) { // description: internal error // schema: // "$ref": "#/definitions/ErrorResponse" + query := r.URL.Query() + teamID := query.Get("teamID") userID := getUserID(r) @@ -146,6 +154,13 @@ func (a *API) handleGetMe(w http.ResponseWriter, r *http.Request) { } } + if teamID != "" && a.permissions.HasPermissionToTeam(userID, teamID, model.PermissionManageTeam) { + user.Permissions = append(user.Permissions, model.PermissionManageTeam.Id) + } + if a.permissions.HasPermissionTo(userID, model.PermissionManageSystem) { + user.Permissions = append(user.Permissions, model.PermissionManageSystem.Id) + } + userData, err := json.Marshal(user) if err != nil { a.errorResponse(w, r, err) diff --git a/server/app/boards.go b/server/app/boards.go index bb5d3ff34..2ddee918f 100644 --- a/server/app/boards.go +++ b/server/app/boards.go @@ -502,11 +502,48 @@ func (a *App) DeleteBoard(boardID, userID string) error { } func (a *App) GetMembersForBoard(boardID string) ([]*model.BoardMember, error) { - return a.store.GetMembersForBoard(boardID) + members, err := a.store.GetMembersForBoard(boardID) + if err != nil { + return nil, err + } + + board, err := a.store.GetBoard(boardID) + if err != nil && !model.IsErrNotFound(err) { + return nil, err + } + if board != nil { + for i, m := range members { + if !m.SchemeAdmin { + if a.permissions.HasPermissionToTeam(m.UserID, board.TeamID, model.PermissionManageTeam) { + members[i].SchemeAdmin = true + } + } + } + } + return members, nil } func (a *App) GetMembersForUser(userID string) ([]*model.BoardMember, error) { - return a.store.GetMembersForUser(userID) + members, err := a.store.GetMembersForUser(userID) + if err != nil { + return nil, err + } + + for i, m := range members { + if !m.SchemeAdmin { + board, err := a.store.GetBoard(m.BoardID) + if err != nil && !model.IsErrNotFound(err) { + return nil, err + } + if board != nil { + if a.permissions.HasPermissionToTeam(m.UserID, board.TeamID, model.PermissionManageTeam) { + // if system/team admin + members[i].SchemeAdmin = true + } + } + } + } + return members, nil } func (a *App) GetMemberForBoard(boardID string, userID string) (*model.BoardMember, error) { @@ -536,6 +573,14 @@ func (a *App) AddMemberToBoard(member *model.BoardMember) (*model.BoardMember, e return nil, err } + if !newMember.SchemeAdmin { + if board != nil { + if a.permissions.HasPermissionToTeam(newMember.UserID, board.TeamID, model.PermissionManageTeam) { + newMember.SchemeAdmin = true + } + } + } + if !board.IsTemplate { if err = a.addBoardsToDefaultCategory(member.UserID, board.TeamID, []*model.Board{board}); err != nil { return nil, err diff --git a/server/app/boards_test.go b/server/app/boards_test.go index 51bb88296..fc9771e54 100644 --- a/server/app/boards_test.go +++ b/server/app/boards_test.go @@ -127,6 +127,7 @@ func TestAddMemberToBoard(t *testing.T) { }, }, nil).Times(2) th.Store.EXPECT().AddUpdateCategoryBoard("user_id_1", "default_category_id", []string{"board_id_1"}).Return(nil) + th.API.EXPECT().HasPermissionToTeam("user_id_1", "team_id_1", model.PermissionManageTeam).Return(false).Times(1) addedBoardMember, err := th.App.AddMemberToBoard(boardMember) require.NoError(t, err) @@ -180,7 +181,7 @@ func TestPatchBoard(t *testing.T) { ID: boardID, TeamID: teamID, IsTemplate: true, - }, nil) + }, nil).Times(2) // Type not null will retrieve team members th.Store.EXPECT().GetUsersByTeam(teamID, "", false, false).Return([]*model.User{}, nil) @@ -218,7 +219,7 @@ func TestPatchBoard(t *testing.T) { ID: boardID, TeamID: teamID, IsTemplate: true, - }, nil) + }, nil).Times(2) // Type not null will retrieve team members th.Store.EXPECT().GetUsersByTeam(teamID, "", false, false).Return([]*model.User{}, nil) @@ -256,7 +257,7 @@ func TestPatchBoard(t *testing.T) { ID: boardID, TeamID: teamID, IsTemplate: true, - }, nil) + }, nil).Times(2) // Type not null will retrieve team members th.Store.EXPECT().GetUsersByTeam(teamID, "", false, false).Return([]*model.User{{ID: userID}}, nil) @@ -294,7 +295,7 @@ func TestPatchBoard(t *testing.T) { ID: boardID, TeamID: teamID, IsTemplate: true, - }, nil) + }, nil).Times(2) // Type not null will retrieve team members th.Store.EXPECT().GetUsersByTeam(teamID, "", false, false).Return([]*model.User{{ID: userID}}, nil) @@ -332,7 +333,10 @@ func TestPatchBoard(t *testing.T) { ID: boardID, TeamID: teamID, IsTemplate: true, - }, nil) + }, nil).Times(3) + + th.API.EXPECT().HasPermissionToTeam(userID, teamID, model.PermissionManageTeam).Return(false).Times(1) + // Type not null will retrieve team members th.Store.EXPECT().GetUsersByTeam(teamID, "", false, false).Return([]*model.User{{ID: userID}}, nil) @@ -370,7 +374,11 @@ func TestPatchBoard(t *testing.T) { ID: boardID, TeamID: teamID, IsTemplate: true, - }, nil) + ChannelID: "", + }, nil).Times(1) + + th.API.EXPECT().HasPermissionToTeam(userID, teamID, model.PermissionManageTeam).Return(false).Times(1) + // Type not null will retrieve team members th.Store.EXPECT().GetUsersByTeam(teamID, "", false, false).Return([]*model.User{{ID: userID}}, nil) @@ -566,3 +574,99 @@ func TestDuplicateBoard(t *testing.T) { assert.NotNil(t, members) }) } + +func TestGetMembersForBoard(t *testing.T) { + th, tearDown := SetupTestHelper(t) + defer tearDown() + + const boardID = "board_id_1" + const userID = "user_id_1" + const teamID = "team_id_1" + + th.Store.EXPECT().GetMembersForBoard(boardID).Return([]*model.BoardMember{ + { + BoardID: boardID, + UserID: userID, + SchemeEditor: true, + }, + }, nil).Times(3) + th.Store.EXPECT().GetBoard(boardID).Return(nil, nil).Times(1) + t.Run("-base case", func(t *testing.T) { + members, err := th.App.GetMembersForBoard(boardID) + assert.NoError(t, err) + assert.NotNil(t, members) + assert.False(t, members[0].SchemeAdmin) + }) + + board := &model.Board{ + ID: boardID, + TeamID: teamID, + } + th.Store.EXPECT().GetBoard(boardID).Return(board, nil).Times(2) + th.API.EXPECT().HasPermissionToTeam(userID, teamID, model.PermissionManageTeam).Return(false).Times(1) + + t.Run("-team check false ", func(t *testing.T) { + members, err := th.App.GetMembersForBoard(boardID) + assert.NoError(t, err) + assert.NotNil(t, members) + + assert.False(t, members[0].SchemeAdmin) + }) + + th.API.EXPECT().HasPermissionToTeam(userID, teamID, model.PermissionManageTeam).Return(true).Times(1) + t.Run("-team check true", func(t *testing.T) { + members, err := th.App.GetMembersForBoard(boardID) + assert.NoError(t, err) + assert.NotNil(t, members) + + assert.True(t, members[0].SchemeAdmin) + }) +} + +func TestGetMembersForUser(t *testing.T) { + th, tearDown := SetupTestHelper(t) + defer tearDown() + + const boardID = "board_id_1" + const userID = "user_id_1" + const teamID = "team_id_1" + + th.Store.EXPECT().GetMembersForUser(userID).Return([]*model.BoardMember{ + { + BoardID: boardID, + UserID: userID, + SchemeEditor: true, + }, + }, nil).Times(3) + th.Store.EXPECT().GetBoard(boardID).Return(nil, nil) + t.Run("-base case", func(t *testing.T) { + members, err := th.App.GetMembersForUser(userID) + assert.NoError(t, err) + assert.NotNil(t, members) + assert.False(t, members[0].SchemeAdmin) + }) + + board := &model.Board{ + ID: boardID, + TeamID: teamID, + } + th.Store.EXPECT().GetBoard(boardID).Return(board, nil).Times(2) + + th.API.EXPECT().HasPermissionToTeam(userID, teamID, model.PermissionManageTeam).Return(false).Times(1) + t.Run("-team check false ", func(t *testing.T) { + members, err := th.App.GetMembersForUser(userID) + assert.NoError(t, err) + assert.NotNil(t, members) + + assert.False(t, members[0].SchemeAdmin) + }) + + th.API.EXPECT().HasPermissionToTeam(userID, teamID, model.PermissionManageTeam).Return(true).Times(1) + t.Run("-team check true", func(t *testing.T) { + members, err := th.App.GetMembersForUser(userID) + assert.NoError(t, err) + assert.NotNil(t, members) + + assert.True(t, members[0].SchemeAdmin) + }) +} diff --git a/server/app/category_boards_test.go b/server/app/category_boards_test.go index 1ff0c2bf1..02a7d930e 100644 --- a/server/app/category_boards_test.go +++ b/server/app/category_boards_test.go @@ -58,6 +58,7 @@ func TestGetUserCategoryBoards(t *testing.T) { Synthetic: false, }, }, nil) + th.Store.EXPECT().GetBoard(utils.Anything).Return(nil, nil).Times(3) th.Store.EXPECT().AddUpdateCategoryBoard("user_id", "boards_category_id", []string{"board_id_1", "board_id_2", "board_id_3"}).Return(nil) categoryBoards, err := th.App.GetUserCategoryBoards("user_id", "team_id") @@ -151,6 +152,7 @@ func TestCreateBoardsCategory(t *testing.T) { Synthetic: true, }, }, nil) + th.Store.EXPECT().GetBoard(utils.Anything).Return(nil, nil).Times(3) existingCategoryBoards := []model.CategoryBoards{} boardsCategory, err := th.App.createBoardsCategory("user_id", "team_id", existingCategoryBoards) @@ -195,6 +197,7 @@ func TestCreateBoardsCategory(t *testing.T) { Synthetic: false, }, }, nil) + th.Store.EXPECT().GetBoard(utils.Anything).Return(nil, nil).Times(3) th.Store.EXPECT().AddUpdateCategoryBoard("user_id", "boards_category_id", []string{"board_id_1", "board_id_2", "board_id_3"}).Return(nil) th.Store.EXPECT().GetUserCategoryBoards("user_id", "team_id").Return([]model.CategoryBoards{ @@ -244,6 +247,7 @@ func TestCreateBoardsCategory(t *testing.T) { Synthetic: true, }, }, nil) + th.Store.EXPECT().GetBoard(utils.Anything).Return(nil, nil).Times(3) th.Store.EXPECT().AddUpdateCategoryBoard("user_id", "boards_category_id", []string{"board_id_1"}).Return(nil) th.Store.EXPECT().GetUserCategoryBoards("user_id", "team_id").Return([]model.CategoryBoards{ diff --git a/server/app/helper_test.go b/server/app/helper_test.go index b471ace1e..df80aba57 100644 --- a/server/app/helper_test.go +++ b/server/app/helper_test.go @@ -10,6 +10,9 @@ import ( "github.com/mattermost/focalboard/server/auth" "github.com/mattermost/focalboard/server/services/config" "github.com/mattermost/focalboard/server/services/metrics" + "github.com/mattermost/focalboard/server/services/permissions/mmpermissions" + mmpermissionsMocks "github.com/mattermost/focalboard/server/services/permissions/mmpermissions/mocks" + permissionsMocks "github.com/mattermost/focalboard/server/services/permissions/mocks" "github.com/mattermost/focalboard/server/services/store/mockstore" "github.com/mattermost/focalboard/server/services/webhook" "github.com/mattermost/focalboard/server/ws" @@ -23,6 +26,7 @@ type TestHelper struct { Store *mockstore.MockStore FilesBackend *mocks.FileBackend logger mlog.LoggerIFace + API *mmpermissionsMocks.MockAPI } func SetupTestHelper(t *testing.T) (*TestHelper, func()) { @@ -37,6 +41,10 @@ func SetupTestHelper(t *testing.T) (*TestHelper, func()) { webhook := webhook.NewClient(&cfg, logger) metricsService := metrics.NewMetrics(metrics.InstanceInfo{}) + mockStore := permissionsMocks.NewMockStore(ctrl) + mockAPI := mmpermissionsMocks.NewMockAPI(ctrl) + permissions := mmpermissions.New(mockStore, mockAPI, mlog.CreateConsoleTestLogger(true, mlog.LvlError)) + appServices := Services{ Auth: auth, Store: store, @@ -45,6 +53,7 @@ func SetupTestHelper(t *testing.T) (*TestHelper, func()) { Metrics: metricsService, Logger: logger, SkipTemplateInit: true, + Permissions: permissions, } app2 := New(&cfg, wsserver, appServices) @@ -60,5 +69,6 @@ func SetupTestHelper(t *testing.T) (*TestHelper, func()) { Store: store, FilesBackend: filesBackend, logger: logger, + API: mockAPI, }, tearDown } diff --git a/server/app/onboarding_test.go b/server/app/onboarding_test.go index 90a77170d..a3f41715e 100644 --- a/server/app/onboarding_test.go +++ b/server/app/onboarding_test.go @@ -39,7 +39,7 @@ func TestPrepareOnboardingTour(t *testing.T) { nil, nil) th.Store.EXPECT().GetMembersForBoard(welcomeBoard.ID).Return([]*model.BoardMember{}, nil).Times(2) th.Store.EXPECT().GetMembersForBoard("board_id_2").Return([]*model.BoardMember{}, nil).Times(1) - th.Store.EXPECT().GetBoard(welcomeBoard.ID).Return(&welcomeBoard, nil).Times(1) + th.Store.EXPECT().GetBoard(welcomeBoard.ID).Return(&welcomeBoard, nil).Times(2) th.Store.EXPECT().GetBoard("board_id_2").Return(&welcomeBoard, nil).Times(1) th.Store.EXPECT().GetUsersByTeam("0", "", false, false).Return([]*model.User{}, nil) diff --git a/server/app/user.go b/server/app/user.go index af409187e..56d628510 100644 --- a/server/app/user.go +++ b/server/app/user.go @@ -10,7 +10,20 @@ func (a *App) GetTeamUsers(teamID string, asGuestID string) ([]*model.User, erro } func (a *App) SearchTeamUsers(teamID string, searchQuery string, asGuestID string, excludeBots bool) ([]*model.User, error) { - return a.store.SearchUsersByTeam(teamID, searchQuery, asGuestID, excludeBots, a.config.ShowEmailAddress, a.config.ShowFullName) + users, err := a.store.SearchUsersByTeam(teamID, searchQuery, asGuestID, excludeBots, a.config.ShowEmailAddress, a.config.ShowFullName) + if err != nil { + return nil, err + } + + for i, u := range users { + if a.permissions.HasPermissionToTeam(u.ID, teamID, model.PermissionManageTeam) { + users[i].Permissions = append(users[i].Permissions, model.PermissionManageTeam.Id) + } + if a.permissions.HasPermissionTo(u.ID, model.PermissionManageSystem) { + users[i].Permissions = append(users[i].Permissions, model.PermissionManageSystem.Id) + } + } + return users, nil } func (a *App) UpdateUserConfig(userID string, patch model.UserPreferencesPatch) ([]mmModel.Preference, error) { diff --git a/server/app/user_test.go b/server/app/user_test.go new file mode 100644 index 000000000..668282247 --- /dev/null +++ b/server/app/user_test.go @@ -0,0 +1,64 @@ +package app + +import ( + "testing" + + "github.com/mattermost/focalboard/server/model" + "github.com/stretchr/testify/assert" +) + +func TestSearchUsers(t *testing.T) { + th, tearDown := SetupTestHelper(t) + defer tearDown() + th.App.config.ShowEmailAddress = false + th.App.config.ShowFullName = false + + teamID := "team-id-1" + userID := "user-id-1" + + t.Run("return empty users", func(t *testing.T) { + th.Store.EXPECT().SearchUsersByTeam(teamID, "", "", true, false, false).Return([]*model.User{}, nil) + + users, err := th.App.SearchTeamUsers(teamID, "", "", true) + assert.NoError(t, err) + assert.Equal(t, 0, len(users)) + }) + + t.Run("return user", func(t *testing.T) { + th.Store.EXPECT().SearchUsersByTeam(teamID, "", "", true, false, false).Return([]*model.User{{ID: userID}}, nil) + th.API.EXPECT().HasPermissionToTeam(userID, teamID, model.PermissionManageTeam).Return(false).Times(1) + th.API.EXPECT().HasPermissionTo(userID, model.PermissionManageSystem).Return(false).Times(1) + + users, err := th.App.SearchTeamUsers(teamID, "", "", true) + assert.NoError(t, err) + assert.Equal(t, 1, len(users)) + assert.Equal(t, 0, len(users[0].Permissions)) + }) + + t.Run("return team admin", func(t *testing.T) { + th.Store.EXPECT().SearchUsersByTeam(teamID, "", "", true, false, false).Return([]*model.User{{ID: userID}}, nil) + th.App.config.ShowEmailAddress = false + th.App.config.ShowFullName = false + th.API.EXPECT().HasPermissionToTeam(userID, teamID, model.PermissionManageTeam).Return(true).Times(1) + th.API.EXPECT().HasPermissionTo(userID, model.PermissionManageSystem).Return(false).Times(1) + + users, err := th.App.SearchTeamUsers(teamID, "", "", true) + assert.NoError(t, err) + assert.Equal(t, 1, len(users)) + assert.Equal(t, users[0].Permissions[0], model.PermissionManageTeam.Id) + }) + + t.Run("return system admin", func(t *testing.T) { + th.Store.EXPECT().SearchUsersByTeam(teamID, "", "", true, false, false).Return([]*model.User{{ID: userID}}, nil) + th.App.config.ShowEmailAddress = false + th.App.config.ShowFullName = false + th.API.EXPECT().HasPermissionToTeam(userID, teamID, model.PermissionManageTeam).Return(true).Times(1) + th.API.EXPECT().HasPermissionTo(userID, model.PermissionManageSystem).Return(true).Times(1) + + users, err := th.App.SearchTeamUsers(teamID, "", "", true) + assert.NoError(t, err) + assert.Equal(t, 1, len(users)) + assert.Equal(t, users[0].Permissions[0], model.PermissionManageTeam.Id) + assert.Equal(t, users[0].Permissions[1], model.PermissionManageSystem.Id) + }) +} diff --git a/server/integrationtests/clienttestlib.go b/server/integrationtests/clienttestlib.go index 5cbe18bb9..87884d24f 100644 --- a/server/integrationtests/clienttestlib.go +++ b/server/integrationtests/clienttestlib.go @@ -78,6 +78,9 @@ func (*FakePermissionPluginAPI) HasPermissionTo(userID string, permission *mmMod } func (*FakePermissionPluginAPI) HasPermissionToTeam(userID string, teamID string, permission *mmModel.Permission) bool { + if permission.Id == model.PermissionManageTeam.Id { + return false + } if userID == userNoTeamMember { return false } diff --git a/server/model/permission.go b/server/model/permission.go index 7d3d773f1..f503a0f3d 100644 --- a/server/model/permission.go +++ b/server/model/permission.go @@ -6,6 +6,8 @@ import ( var ( PermissionViewTeam = mmModel.PermissionViewTeam + PermissionManageTeam = mmModel.PermissionManageTeam + PermissionManageSystem = mmModel.PermissionManageSystem PermissionReadChannel = mmModel.PermissionReadChannel PermissionViewMembers = mmModel.PermissionViewMembers PermissionCreatePublicChannel = mmModel.PermissionCreatePublicChannel diff --git a/server/model/user.go b/server/model/user.go index 7b48cb0a4..37c1f946d 100644 --- a/server/model/user.go +++ b/server/model/user.go @@ -66,6 +66,9 @@ type User struct { // required: true IsGuest bool `json:"is_guest"` + // Special Permissions the user may have + Permissions []string `json:"permissions,omitempty"` + Roles string `json:"roles"` } diff --git a/server/services/permissions/localpermissions/localpermissions.go b/server/services/permissions/localpermissions/localpermissions.go index c45412870..e71894ff0 100644 --- a/server/services/permissions/localpermissions/localpermissions.go +++ b/server/services/permissions/localpermissions/localpermissions.go @@ -31,6 +31,9 @@ func (s *Service) HasPermissionToTeam(userID, teamID string, permission *mmModel if userID == "" || teamID == "" || permission == nil { return false } + if permission.Id == model.PermissionManageTeam.Id { + return false + } return true } diff --git a/server/services/permissions/localpermissions/localpermissions_test.go b/server/services/permissions/localpermissions/localpermissions_test.go index 81a9e5677..dc7db8148 100644 --- a/server/services/permissions/localpermissions/localpermissions_test.go +++ b/server/services/permissions/localpermissions/localpermissions_test.go @@ -27,6 +27,11 @@ func TestHasPermissionToTeam(t *testing.T) { hasPermission := th.permissions.HasPermissionToTeam("user-id", "team-id", model.PermissionManageBoardCards) assert.True(t, hasPermission) }) + + t.Run("no users have PermissionManageTeam on teams", func(t *testing.T) { + hasPermission := th.permissions.HasPermissionToTeam("user-id", "team-id", model.PermissionManageTeam) + assert.False(t, hasPermission) + }) } func TestHasPermissionToBoard(t *testing.T) { @@ -141,4 +146,27 @@ func TestHasPermissionToBoard(t *testing.T) { th.checkBoardPermissions("viewer", member, hasPermissionTo, hasNotPermissionTo) }) + + t.Run("Manage Team Permission ", func(t *testing.T) { + member := &model.BoardMember{ + UserID: "user-id", + BoardID: "board-id", + SchemeViewer: true, + } + + hasPermissionTo := []*mmModel.Permission{ + model.PermissionViewBoard, + } + + hasNotPermissionTo := []*mmModel.Permission{ + model.PermissionManageBoardType, + model.PermissionDeleteBoard, + model.PermissionManageBoardRoles, + model.PermissionShareBoard, + model.PermissionManageBoardCards, + model.PermissionManageBoardProperties, + } + + th.checkBoardPermissions("viewer", member, hasPermissionTo, hasNotPermissionTo) + }) } diff --git a/server/services/permissions/mmpermissions/helpers_test.go b/server/services/permissions/mmpermissions/helpers_test.go index fc4117e7d..ec022bc01 100644 --- a/server/services/permissions/mmpermissions/helpers_test.go +++ b/server/services/permissions/mmpermissions/helpers_test.go @@ -58,6 +58,13 @@ func (th *TestHelper) checkBoardPermissions(roleName string, member *model.Board Return(member, nil). Times(1) + if !member.SchemeAdmin { + th.api.EXPECT(). + HasPermissionToTeam(member.UserID, teamID, model.PermissionManageTeam). + Return(roleName == "elevated-admin"). + Times(1) + } + hasPermission := th.permissions.HasPermissionToBoard(member.UserID, member.BoardID, p) assert.True(t, hasPermission) }) @@ -80,6 +87,13 @@ func (th *TestHelper) checkBoardPermissions(roleName string, member *model.Board Return(member, nil). Times(1) + if !member.SchemeAdmin { + th.api.EXPECT(). + HasPermissionToTeam(member.UserID, teamID, model.PermissionManageTeam). + Return(roleName == "elevated-admin"). + Times(1) + } + hasPermission := th.permissions.HasPermissionToBoard(member.UserID, member.BoardID, p) assert.False(t, hasPermission) }) diff --git a/server/services/permissions/mmpermissions/mmpermissions.go b/server/services/permissions/mmpermissions/mmpermissions.go index 8239a31b8..71aaec1b6 100644 --- a/server/services/permissions/mmpermissions/mmpermissions.go +++ b/server/services/permissions/mmpermissions/mmpermissions.go @@ -82,7 +82,6 @@ func (s *Service) HasPermissionToBoard(userID, boardID string, permission *mmMod if !s.HasPermissionToTeam(userID, board.TeamID, model.PermissionViewTeam) { return false } - member, err := s.store.GetMemberForBoard(boardID, userID) if model.IsErrNotFound(err) { return false @@ -107,6 +106,13 @@ func (s *Service) HasPermissionToBoard(userID, boardID string, permission *mmMod member.SchemeViewer = true } + // Admins become member of boards, but get minimal role + // if they are a System/Team Admin (model.PermissionManageTeam) + // elevate their permissions + if !member.SchemeAdmin && s.HasPermissionToTeam(userID, board.TeamID, model.PermissionManageTeam) { + return true + } + switch permission { case model.PermissionManageBoardType, model.PermissionDeleteBoard, model.PermissionManageBoardRoles, model.PermissionShareBoard, model.PermissionDeleteOthersComments: return member.SchemeAdmin diff --git a/server/services/permissions/mmpermissions/mmpermissions_test.go b/server/services/permissions/mmpermissions/mmpermissions_test.go index 01a31b95f..ff14baec7 100644 --- a/server/services/permissions/mmpermissions/mmpermissions_test.go +++ b/server/services/permissions/mmpermissions/mmpermissions_test.go @@ -219,4 +219,25 @@ func TestHasPermissionToBoard(t *testing.T) { th.checkBoardPermissions("viewer", member, teamID, hasPermissionTo, hasNotPermissionTo) }) + + t.Run("elevate board viewer permissions", func(t *testing.T) { + member := &model.BoardMember{ + UserID: userID, + BoardID: boardID, + SchemeViewer: true, + } + + hasPermissionTo := []*mmModel.Permission{ + model.PermissionManageBoardType, + model.PermissionDeleteBoard, + model.PermissionManageBoardRoles, + model.PermissionShareBoard, + model.PermissionManageBoardCards, + model.PermissionViewBoard, + model.PermissionManageBoardProperties, + } + + hasNotPermissionTo := []*mmModel.Permission{} + th.checkBoardPermissions("elevated-admin", member, teamID, hasPermissionTo, hasNotPermissionTo) + }) } diff --git a/webapp/src/components/shareBoard/__snapshots__/shareBoard.test.tsx.snap b/webapp/src/components/shareBoard/__snapshots__/shareBoard.test.tsx.snap index 78765f630..7e76ccb8b 100644 --- a/webapp/src/components/shareBoard/__snapshots__/shareBoard.test.tsx.snap +++ b/webapp/src/components/shareBoard/__snapshots__/shareBoard.test.tsx.snap @@ -1991,6 +1991,15 @@ exports[`src/components/shareBoard/shareBoard return shareBoard and click Select > @username_1 +
+
+ Team Admin +
+
@@ -2017,6 +2026,15 @@ exports[`src/components/shareBoard/shareBoard return shareBoard and click Select > @username_2 +
+
+ Admin +
+
diff --git a/webapp/src/components/shareBoard/__snapshots__/userPermissionsRow.test.tsx.snap b/webapp/src/components/shareBoard/__snapshots__/userPermissionsRow.test.tsx.snap index eeae83108..8745f3ea7 100644 --- a/webapp/src/components/shareBoard/__snapshots__/userPermissionsRow.test.tsx.snap +++ b/webapp/src/components/shareBoard/__snapshots__/userPermissionsRow.test.tsx.snap @@ -728,3 +728,263 @@ exports[`src/components/shareBoard/userPermissionsRow should match snapshot in t `; + +exports[`src/components/shareBoard/userPermissionsRow should match snapshot-admin 1`] = ` +
+
+
+
+ + + @username_1 + + + (You) + +
+
+ Admin +
+
+
+
+
+