From 11c46668a13823525f236f947f954373e6b693a2 Mon Sep 17 00:00:00 2001 From: JG Heithcock Date: Wed, 8 Jul 2026 14:27:44 -0700 Subject: [PATCH] [MM-69603] Enforce board creation permissions on archive import (#219) * MM-69603: Enforce board creation permissions on archive import. Apply the same team permission and import validation checks used by direct board creation when parsing boards from archive JSONL, including modern "board" records that previously skipped IsValidForImport. * MM-69603: Reduce ImportBoardJSONL cyclomatic complexity. Extract validateBoardForImport helper to satisfy gocyclo lint limit. (cherry picked from commit ab5935a144c45915ecd1c27143a412c2e2dc8791) --- server/app/helper_test.go | 8 +++ server/app/import.go | 5 +- server/app/import_test.go | 104 ++++++++++++++++++++++++++++++++++---- server/app/permissions.go | 24 +++++++++ 4 files changed, 129 insertions(+), 12 deletions(-) diff --git a/server/app/helper_test.go b/server/app/helper_test.go index 9094a10a5..cb29e3a57 100644 --- a/server/app/helper_test.go +++ b/server/app/helper_test.go @@ -103,3 +103,11 @@ func (th *TestHelper) expectBoardEditor(userID, boardID, teamID string) { }, nil) th.API.EXPECT().HasPermissionToTeam(userID, teamID, model.PermissionManageTeam).Return(false) } + +func (th *TestHelper) expectBoardImportPermissions(userID, teamID string, boardType model.BoardType) { + if boardType == model.BoardTypeOpen { + th.API.EXPECT().HasPermissionToTeam(userID, teamID, model.PermissionCreatePublicChannel).Return(true) + return + } + th.API.EXPECT().HasPermissionToTeam(userID, teamID, model.PermissionCreatePrivateChannel).Return(true) +} diff --git a/server/app/import.go b/server/app/import.go index 23b637a39..177da2b0a 100644 --- a/server/app/import.go +++ b/server/app/import.go @@ -206,6 +206,9 @@ func (a *App) ImportBoardJSONL(r io.Reader, opt model.ImportArchiveOptions) (*mo board.ModifiedBy = userID board.UpdateAt = now board.TeamID = opt.TeamID + if err := a.validateBoardForImport(userID, opt.TeamID, &board); err != nil { + return nil, err + } boardsAndBlocks.Boards = append(boardsAndBlocks.Boards, &board) boardID = board.ID case "board_block": @@ -220,7 +223,7 @@ func (a *App) ImportBoardJSONL(r io.Reader, opt model.ImportArchiveOptions) (*mo if err != nil { return nil, fmt.Errorf("cannot convert archive line %d to block: %w", lineNum, err) } - if err := board.IsValidForImport(); err != nil { + if err := a.validateBoardForImport(userID, opt.TeamID, board); err != nil { return nil, err } boardsAndBlocks.Boards = append(boardsAndBlocks.Boards, board) diff --git a/server/app/import_test.go b/server/app/import_test.go index f892aed6c..672312405 100644 --- a/server/app/import_test.go +++ b/server/app/import_test.go @@ -14,13 +14,15 @@ import ( "github.com/stretchr/testify/require" ) +const importTestTeamID = "y5tuzz9yb3y99gmobyc4hg5wnr" + func TestApp_ImportArchive(t *testing.T) { th, tearDown := SetupTestHelper(t) defer tearDown() board := &model.Board{ ID: "d14b9df9-1f31-4732-8a64-92bc7162cd28", - TeamID: "test-team", + TeamID: importTestTeamID, Title: "Cross-Functional Project Plan", IsTemplate: false, } @@ -49,11 +51,13 @@ func TestApp_ImportArchive(t *testing.T) { ModifiedBy: "user", } + th.expectBoardImportPermissions("user", opts.TeamID, model.BoardTypePrivate) + th.Store.EXPECT().CreateBoardsAndBlocks(gomock.AssignableToTypeOf(&model.BoardsAndBlocks{}), "user").Return(babs, nil) th.Store.EXPECT().GetMembersForBoard(board.ID).AnyTimes().Return([]*model.BoardMember{boardMember}, nil) th.Store.EXPECT().GetBoard(board.ID).Return(board, nil) th.Store.EXPECT().GetMemberForBoard(board.ID, "user").Return(boardMember, nil) - th.Store.EXPECT().GetUserCategoryBoards("user", "test-team").Return([]model.CategoryBoards{ + th.Store.EXPECT().GetUserCategoryBoards("user", opts.TeamID).Return([]model.CategoryBoards{ { Category: model.Category{ Type: "default", @@ -62,13 +66,13 @@ func TestApp_ImportArchive(t *testing.T) { }, }, }, nil) - th.Store.EXPECT().GetUserCategoryBoards("user", "test-team") + th.Store.EXPECT().GetUserCategoryBoards("user", opts.TeamID) th.Store.EXPECT().CreateCategory(utils.Anything).Return(nil) th.Store.EXPECT().GetCategory(utils.Anything).Return(&model.Category{ ID: "boards_category_id", Name: "Boards", }, nil) - th.Store.EXPECT().GetBoardsForUserAndTeam("user", "test-team", false).Return([]*model.Board{}, nil) + th.Store.EXPECT().GetBoardsForUserAndTeam("user", opts.TeamID, false).Return([]*model.Board{}, nil) th.Store.EXPECT().GetMembersForUser("user").Return([]*model.BoardMember{}, nil) th.Store.EXPECT().AddUpdateCategoryBoard("user", utils.Anything, utils.Anything).Return(nil) @@ -79,10 +83,12 @@ func TestApp_ImportArchive(t *testing.T) { t.Run("import board archive", func(t *testing.T) { r := bytes.NewReader([]byte(boardArchive)) opts := model.ImportArchiveOptions{ - TeamID: "test-team", + TeamID: importTestTeamID, ModifiedBy: "f1tydgc697fcbp8ampr6881jea", } + th.expectBoardImportPermissions(opts.ModifiedBy, opts.TeamID, model.BoardTypePrivate) + bm1 := &model.BoardMember{ BoardID: board.ID, UserID: "f1tydgc697fcbp8ampr6881jea", @@ -112,8 +118,8 @@ func TestApp_ImportArchive(t *testing.T) { th.Store.EXPECT().CreateBoardsAndBlocks(gomock.AssignableToTypeOf(&model.BoardsAndBlocks{}), "f1tydgc697fcbp8ampr6881jea").Return(babs, nil) th.Store.EXPECT().GetMembersForBoard(board.ID).AnyTimes().Return([]*model.BoardMember{bm1, bm2, bm3}, nil) - th.Store.EXPECT().GetUserCategoryBoards("f1tydgc697fcbp8ampr6881jea", "test-team").Return([]model.CategoryBoards{}, nil) - th.Store.EXPECT().GetUserCategoryBoards("f1tydgc697fcbp8ampr6881jea", "test-team").Return([]model.CategoryBoards{ + th.Store.EXPECT().GetUserCategoryBoards("f1tydgc697fcbp8ampr6881jea", importTestTeamID).Return([]model.CategoryBoards{}, nil) + th.Store.EXPECT().GetUserCategoryBoards("f1tydgc697fcbp8ampr6881jea", importTestTeamID).Return([]model.CategoryBoards{ { Category: model.Category{ ID: "boards_category_id", @@ -128,7 +134,7 @@ func TestApp_ImportArchive(t *testing.T) { Name: "Boards", }, nil) th.Store.EXPECT().GetMembersForUser("f1tydgc697fcbp8ampr6881jea").Return([]*model.BoardMember{}, nil) - th.Store.EXPECT().GetBoardsForUserAndTeam("f1tydgc697fcbp8ampr6881jea", "test-team", false).Return([]*model.Board{}, nil) + th.Store.EXPECT().GetBoardsForUserAndTeam("f1tydgc697fcbp8ampr6881jea", importTestTeamID, false).Return([]*model.Board{}, nil) th.Store.EXPECT().AddUpdateCategoryBoard("f1tydgc697fcbp8ampr6881jea", utils.Anything, utils.Anything).Return(nil) th.Store.EXPECT().GetBoard(board.ID).AnyTimes().Return(board, nil) th.Store.EXPECT().GetMemberForBoard(board.ID, "f1tydgc697fcbp8ampr6881jea").AnyTimes().Return(bm1, nil) @@ -149,13 +155,15 @@ func TestApp_ImportArchive(t *testing.T) { r := bytes.NewReader([]byte(boardArchive)) opts := model.ImportArchiveOptions{ - TeamID: "test-team", + TeamID: importTestTeamID, ModifiedBy: importerID, } + th.expectBoardImportPermissions(importerID, opts.TeamID, model.BoardTypePrivate) + th.Store.EXPECT().CreateBoardsAndBlocks(gomock.AssignableToTypeOf(&model.BoardsAndBlocks{}), importerID).Return(babs, nil) th.Store.EXPECT().GetMembersForBoard(board.ID).AnyTimes().Return([]*model.BoardMember{}, nil) - th.Store.EXPECT().GetUserCategoryBoards(importerID, "test-team").AnyTimes().Return([]model.CategoryBoards{ + th.Store.EXPECT().GetUserCategoryBoards(importerID, importTestTeamID).AnyTimes().Return([]model.CategoryBoards{ { Category: model.Category{ ID: "boards_category_id", @@ -165,7 +173,7 @@ func TestApp_ImportArchive(t *testing.T) { }, }, nil) th.Store.EXPECT().GetMembersForUser(importerID).AnyTimes().Return([]*model.BoardMember{}, nil) - th.Store.EXPECT().GetBoardsForUserAndTeam(importerID, "test-team", false).AnyTimes().Return([]*model.Board{}, nil) + th.Store.EXPECT().GetBoardsForUserAndTeam(importerID, importTestTeamID, false).AnyTimes().Return([]*model.Board{}, nil) th.Store.EXPECT().AddUpdateCategoryBoard(importerID, utils.Anything, utils.Anything).AnyTimes().Return(nil) th.Store.EXPECT().GetBoard(board.ID).AnyTimes().Return(board, nil) th.Store.EXPECT().GetMemberForBoard(board.ID, gomock.Any()).AnyTimes().Return(nil, nil) @@ -245,8 +253,82 @@ func TestApp_ImportArchive(t *testing.T) { th.Store.EXPECT().PatchBlocks(gomock.Any(), "my-userid").Return(nil) th.App.fixImagesAttachments(boardMap, fileMap, "test-team", "my-userid") }) + + t.Run("import private board archive requires create private channel permission", func(t *testing.T) { + const importerID = "f1tydgc697fcbp8ampr6881jea" + + r := bytes.NewReader([]byte(boardArchive)) + opts := model.ImportArchiveOptions{ + TeamID: importTestTeamID, + ModifiedBy: importerID, + } + + th.API.EXPECT().HasPermissionToTeam(importerID, opts.TeamID, model.PermissionCreatePrivateChannel).Return(false) + + _, err := th.App.ImportBoardJSONL(r, opts) + require.Error(t, err) + var permErr *model.ErrPermission + require.ErrorAs(t, err, &permErr) + }) + + t.Run("import open board archive requires create public channel permission", func(t *testing.T) { + const importerID = "f1tydgc697fcbp8ampr6881jea" + + r := bytes.NewReader([]byte(openBoardArchive)) + opts := model.ImportArchiveOptions{ + TeamID: importTestTeamID, + ModifiedBy: importerID, + } + + th.API.EXPECT().HasPermissionToTeam(importerID, opts.TeamID, model.PermissionCreatePublicChannel).Return(false) + + _, err := th.App.ImportBoardJSONL(r, opts) + require.Error(t, err) + var permErr *model.ErrPermission + require.ErrorAs(t, err, &permErr) + }) + + t.Run("import open board archive succeeds with create public channel permission", func(t *testing.T) { + const importerID = "f1tydgc697fcbp8ampr6881jea" + + r := bytes.NewReader([]byte(openBoardArchive)) + opts := model.ImportArchiveOptions{ + TeamID: importTestTeamID, + ModifiedBy: importerID, + } + + th.expectBoardImportPermissions(importerID, opts.TeamID, model.BoardTypeOpen) + th.Store.EXPECT().CreateBoardsAndBlocks(gomock.AssignableToTypeOf(&model.BoardsAndBlocks{}), importerID).Return(babs, nil) + th.Store.EXPECT().GetMembersForBoard(board.ID).AnyTimes().Return([]*model.BoardMember{}, nil) + th.Store.EXPECT().GetUserCategoryBoards(importerID, importTestTeamID).AnyTimes().Return([]model.CategoryBoards{ + { + Category: model.Category{ + ID: "boards_category_id", + Name: "Boards", + Type: model.CategoryTypeSystem, + }, + }, + }, nil) + th.Store.EXPECT().GetMembersForUser(importerID).AnyTimes().Return([]*model.BoardMember{}, nil) + th.Store.EXPECT().GetBoardsForUserAndTeam(importerID, importTestTeamID, false).AnyTimes().Return([]*model.Board{}, nil) + th.Store.EXPECT().AddUpdateCategoryBoard(importerID, utils.Anything, utils.Anything).AnyTimes().Return(nil) + th.Store.EXPECT().GetBoard(board.ID).AnyTimes().Return(board, nil) + th.Store.EXPECT().GetMemberForBoard(board.ID, gomock.Any()).AnyTimes().Return(nil, nil) + th.Store.EXPECT().GetUserByID(gomock.Any()).AnyTimes().DoAndReturn(func(id string) (*model.User, error) { + return &model.User{ID: id, IsGuest: false}, nil + }) + + newBoard, err := th.App.ImportBoardJSONL(r, opts) + require.NoError(t, err) + require.Equal(t, board.ID, newBoard.ID) + }) } +//nolint:lll +const openBoardArchive = `{"type":"board","data":{"id":"bfoi6yy6pa3yzika53spj7pq9ee","teamId":"wsmqbtwb5jb35jb3mtp85c8a9h","createdBy":"f1tydgc697fcbp8ampr6881jea","modifiedBy":"f1tydgc697fcbp8ampr6881jea","type":"O","minimumRole":"","title":"Open Import Test","createAt":1672750481591,"updateAt":1672750481591}} +{"type":"block","data":{"id":"ckpc3b1dp3pbw7bqntfryy9jbzo","parentId":"bjaqxtbyqz3bu7pgyddpgpms74a","createdBy":"f1tydgc697fcbp8ampr6881jea","modifiedBy":"f1tydgc697fcbp8ampr6881jea","schema":1,"type":"card","title":"Test","fields":{"contentOrder":[],"icon":"","isTemplate":false,"properties":{}},"createAt":1672750481612,"updateAt":1672845003530,"deleteAt":0,"boardId":"bfoi6yy6pa3yzika53spj7pq9ee"}} +` + //nolint:lll const asana = `{"version":1,"date":1614714686842} {"type":"block","data":{"id":"d14b9df9-1f31-4732-8a64-92bc7162cd28","fields":{"icon":"","description":"","cardProperties":[{"id":"3bdcbaeb-bc78-4884-8531-a0323b74676a","name":"Section","type":"select","options":[{"id":"d8d94ef1-5e74-40bb-8be5-fc0eb3f47732","value":"Planning","color":"propColorGray"},{"id":"454559bb-b788-4ff6-873e-04def8491d2c","value":"Milestones","color":"propColorBrown"},{"id":"deaab476-c690-48df-828f-725b064dc476","value":"Next steps","color":"propColorOrange"},{"id":"2138305a-3157-461c-8bbe-f19ebb55846d","value":"Comms Plan","color":"propColorYellow"}]}]},"createAt":1614714686836,"updateAt":1614714686836,"deleteAt":0,"schema":1,"parentId":"","rootId":"d14b9df9-1f31-4732-8a64-92bc7162cd28","modifiedBy":"","type":"board","title":"Cross-Functional Project Plan"}} diff --git a/server/app/permissions.go b/server/app/permissions.go index ac2169414..e07c0da88 100644 --- a/server/app/permissions.go +++ b/server/app/permissions.go @@ -4,9 +4,33 @@ package app import ( + "github.com/mattermost/mattermost-plugin-boards/server/model" + mm_model "github.com/mattermost/mattermost/server/public/model" ) func (a *App) HasPermissionToBoard(userID, boardID string, permission *mm_model.Permission) bool { return a.permissions.HasPermissionToBoard(userID, boardID, permission) } + +func (a *App) checkBoardCreationPermission(userID, teamID string, boardType model.BoardType) error { + if userID == model.SystemUserID { + return nil + } + + if boardType == model.BoardTypeOpen { + if !a.permissions.HasPermissionToTeam(userID, teamID, model.PermissionCreatePublicChannel) { + return model.NewErrPermission("access denied to create public boards") + } + } else if !a.permissions.HasPermissionToTeam(userID, teamID, model.PermissionCreatePrivateChannel) { + return model.NewErrPermission("access denied to create private boards") + } + return nil +} + +func (a *App) validateBoardForImport(userID, teamID string, board *model.Board) error { + if err := board.IsValidForImport(); err != nil { + return err + } + return a.checkBoardCreationPermission(userID, teamID, board.Type) +}