From 720ee81113ac7a7dd062271c3d6cdf58ce8e044a Mon Sep 17 00:00:00 2001 From: George Goldberg Date: Sun, 26 Mar 2017 14:37:39 +0100 Subject: PLT-6063: AddUserToTeam permission depends on policy. (#5869) Uses same policy setting as InviteUserToTeam. --- api/team_test.go | 80 ++++++++++++++++++++++++++++++++++++++++++-- api4/team_test.go | 90 +++++++++++++++++++++++++++++++++++++++++++++++--- model/authorization.go | 2 +- utils/authorization.go | 29 ++++++++++------ 4 files changed, 183 insertions(+), 18 deletions(-) diff --git a/api/team_test.go b/api/team_test.go index 2166c004b..9dd598834 100644 --- a/api/team_test.go +++ b/api/team_test.go @@ -58,19 +58,28 @@ func TestCreateTeam(t *testing.T) { func TestAddUserToTeam(t *testing.T) { th := Setup().InitSystemAdmin().InitBasic() th.BasicClient.Logout() + + // Test adding a user to a team you are not a member of. + th.SystemAdminClient.SetTeamId(th.BasicTeam.Id) + th.SystemAdminClient.Must(th.SystemAdminClient.RemoveUserFromTeam(th.BasicTeam.Id, th.BasicUser2.Id)) + th.LoginBasic2() user2 := th.CreateUser(th.BasicClient) if _, err := th.BasicClient.AddUserToTeam(th.BasicTeam.Id, user2.Id); err == nil { - t.Fatal("Should have failed because of permissions") + t.Fatal("Should have failed because of not being a team member") } - th.SystemAdminClient.SetTeamId(th.BasicTeam.Id) - if _, err := th.SystemAdminClient.UpdateTeamRoles(th.BasicUser2.Id, "team_user team_admin"); err != nil { + // Test adding a user to a team you are a member of. + th.BasicClient.Logout() + th.LoginBasic() + + if _, err := th.BasicClient.AddUserToTeam(th.BasicTeam.Id, user2.Id); err != nil { t.Fatal(err) } + // Check it worked properly. if result, err := th.BasicClient.AddUserToTeam(th.BasicTeam.Id, user2.Id); err != nil { t.Fatal(err) } else { @@ -83,6 +92,69 @@ func TestAddUserToTeam(t *testing.T) { if _, err := th.BasicClient.GetTeamMember(th.BasicTeam.Id, user2.Id); err != nil { t.Fatal(err) } + + // Restore config/license at end of test case. + restrictTeamInvite := *utils.Cfg.TeamSettings.RestrictTeamInvite + isLicensed := utils.IsLicensed + license := utils.License + defer func() { + *utils.Cfg.TeamSettings.RestrictTeamInvite = restrictTeamInvite + utils.IsLicensed = isLicensed + utils.License = license + utils.SetDefaultRolesBasedOnConfig() + }() + + // Set the config so that only team admins can add a user to a team. + *utils.Cfg.TeamSettings.RestrictTeamInvite = model.PERMISSIONS_TEAM_ADMIN + utils.SetDefaultRolesBasedOnConfig() + + // Test without the EE license to see that the permission restriction is ignored. + user3 := th.CreateUser(th.BasicClient) + if _, err := th.BasicClient.AddUserToTeam(th.BasicTeam.Id, user3.Id); err != nil { + t.Fatal(err) + } + + // Add an EE license. + utils.IsLicensed = true + utils.License = &model.License{Features: &model.Features{}} + utils.License.Features.SetDefaults() + utils.SetDefaultRolesBasedOnConfig() + + // Check that a regular user can't add someone to the team. + user4 := th.CreateUser(th.BasicClient) + if _, err := th.BasicClient.AddUserToTeam(th.BasicTeam.Id, user4.Id); err == nil { + t.Fatal("should have failed due to permissions error") + } + + // Should work as team admin. + UpdateUserToTeamAdmin(th.BasicUser, th.BasicTeam) + app.InvalidateAllCaches() + *utils.Cfg.TeamSettings.RestrictTeamInvite = model.PERMISSIONS_TEAM_ADMIN + utils.IsLicensed = true + utils.License = &model.License{Features: &model.Features{}} + utils.License.Features.SetDefaults() + utils.SetDefaultRolesBasedOnConfig() + + user5 := th.CreateUser(th.BasicClient) + if _, err := th.BasicClient.AddUserToTeam(th.BasicTeam.Id, user5.Id); err != nil { + t.Fatal(err) + } + + // Change permission level to System Admin + *utils.Cfg.TeamSettings.RestrictTeamInvite = model.PERMISSIONS_SYSTEM_ADMIN + utils.SetDefaultRolesBasedOnConfig() + + // Should not work as team admin. + user6 := th.CreateUser(th.BasicClient) + if _, err := th.BasicClient.AddUserToTeam(th.BasicTeam.Id, user6.Id); err == nil { + t.Fatal("should have failed due to permissions error") + } + + // Should work as system admin. + user7 := th.CreateUser(th.BasicClient) + if _, err := th.SystemAdminClient.AddUserToTeam(th.BasicTeam.Id, user7.Id); err != nil { + t.Fatal(err) + } } func TestRemoveUserFromTeam(t *testing.T) { @@ -314,10 +386,12 @@ func TestInviteMembers(t *testing.T) { defer func() { utils.IsLicensed = isLicensed utils.License = license + utils.SetDefaultRolesBasedOnConfig() }() utils.IsLicensed = true utils.License = &model.License{Features: &model.Features{}} utils.License.Features.SetDefaults() + utils.SetDefaultRolesBasedOnConfig() if _, err := Client.InviteMembers(invites); err == nil { t.Fatal("should have errored not team admin and licensed") diff --git a/api4/team_test.go b/api4/team_test.go index 36d6df147..40f5ec778 100644 --- a/api4/team_test.go +++ b/api4/team_test.go @@ -9,6 +9,7 @@ import ( "strconv" "testing" + "github.com/mattermost/platform/app" "github.com/mattermost/platform/model" "github.com/mattermost/platform/utils" ) @@ -617,12 +618,25 @@ func TestAddTeamMember(t *testing.T) { team := th.BasicTeam otherUser := th.CreateUser() - // by user_id - th.LoginTeamAdmin() + if err := app.RemoveUserFromTeam(th.BasicTeam.Id, th.BasicUser2.Id); err != nil { + t.Fatalf(err.Error()) + } + // Regular user can't add a member to a team they don't belong to. + th.LoginBasic2() tm, resp := Client.AddTeamMember(team.Id, otherUser.Id, "", "", "") + CheckForbiddenStatus(t, resp) + if resp.Error == nil { + t.Fatalf("ERror is nhul") + } + Client.Logout() + + // Regular user can add a member to a team they belong to. + th.LoginBasic() + tm, resp = Client.AddTeamMember(team.Id, otherUser.Id, "", "", "") CheckNoError(t, resp) + // Check all the returned data. if tm == nil { t.Fatal("should have returned team member") } @@ -635,6 +649,7 @@ func TestAddTeamMember(t *testing.T) { t.Fatal("team ids should have matched") } + // Check with various invalid requests. tm, resp = Client.AddTeamMember(team.Id, "junk", "", "", "") CheckBadRequestStatus(t, resp) @@ -651,19 +666,86 @@ func TestAddTeamMember(t *testing.T) { _, resp = Client.AddTeamMember(team.Id, GenerateTestId(), "", "", "") CheckNotFoundStatus(t, resp) + Client.Logout() + + // Check effects of config and license changes. + restrictTeamInvite := *utils.Cfg.TeamSettings.RestrictTeamInvite + isLicensed := utils.IsLicensed + license := utils.License + defer func() { + *utils.Cfg.TeamSettings.RestrictTeamInvite = restrictTeamInvite + utils.IsLicensed = isLicensed + utils.License = license + utils.SetDefaultRolesBasedOnConfig() + }() + + // Set the config so that only team admins can add a user to a team. + *utils.Cfg.TeamSettings.RestrictTeamInvite = model.PERMISSIONS_TEAM_ADMIN + utils.SetDefaultRolesBasedOnConfig() + th.LoginBasic() + + // Test without the EE license to see that the permission restriction is ignored. + _, resp = Client.AddTeamMember(team.Id, otherUser.Id, "", "", "") + CheckNoError(t, resp) + + // Add an EE license. + utils.IsLicensed = true + utils.License = &model.License{Features: &model.Features{}} + utils.License.Features.SetDefaults() + utils.SetDefaultRolesBasedOnConfig() th.LoginBasic() + // Check that a regular user can't add someone to the team. _, resp = Client.AddTeamMember(team.Id, otherUser.Id, "", "", "") CheckForbiddenStatus(t, resp) - Client.Logout() + // Update user to team admin + UpdateUserToTeamAdmin(th.BasicUser, th.BasicTeam) + app.InvalidateAllCaches() + *utils.Cfg.TeamSettings.RestrictTeamInvite = model.PERMISSIONS_TEAM_ADMIN + utils.IsLicensed = true + utils.License = &model.License{Features: &model.Features{}} + utils.License.Features.SetDefaults() + utils.SetDefaultRolesBasedOnConfig() + th.LoginBasic() + // Should work as a team admin. _, resp = Client.AddTeamMember(team.Id, otherUser.Id, "", "", "") - CheckUnauthorizedStatus(t, resp) + CheckNoError(t, resp) + + // Change permission level to System Admin + *utils.Cfg.TeamSettings.RestrictTeamInvite = model.PERMISSIONS_SYSTEM_ADMIN + utils.SetDefaultRolesBasedOnConfig() + + // Should not work as team admin. + _, resp = Client.AddTeamMember(team.Id, otherUser.Id, "", "", "") + CheckForbiddenStatus(t, resp) + // Should work as system admin. _, resp = th.SystemAdminClient.AddTeamMember(team.Id, otherUser.Id, "", "", "") CheckNoError(t, resp) + // Change permission level to All + UpdateUserToNonTeamAdmin(th.BasicUser, th.BasicTeam) + app.InvalidateAllCaches() + *utils.Cfg.TeamSettings.RestrictTeamInvite = model.PERMISSIONS_ALL + utils.IsLicensed = true + utils.License = &model.License{Features: &model.Features{}} + utils.License.Features.SetDefaults() + utils.SetDefaultRolesBasedOnConfig() + th.LoginBasic() + + // Should work as a regular user. + _, resp = Client.AddTeamMember(team.Id, otherUser.Id, "", "", "") + CheckNoError(t, resp) + + // Reset config and license. + *utils.Cfg.TeamSettings.RestrictTeamInvite = restrictTeamInvite + utils.IsLicensed = isLicensed + utils.License = license + utils.SetDefaultRolesBasedOnConfig() + th.LoginBasic() + // by hash and data Client.Login(otherUser.Email, otherUser.Password) diff --git a/model/authorization.go b/model/authorization.go index eec0f79ee..d49406e47 100644 --- a/model/authorization.go +++ b/model/authorization.go @@ -343,7 +343,6 @@ func InitalizeRoles() { "authentication.roles.team_admin.description", []string{ PERMISSION_EDIT_OTHERS_POSTS.Id, - PERMISSION_ADD_USER_TO_TEAM.Id, PERMISSION_REMOVE_USER_FROM_TEAM.Id, PERMISSION_MANAGE_TEAM.Id, PERMISSION_IMPORT_TEAM.Id, @@ -400,6 +399,7 @@ func InitalizeRoles() { PERMISSION_DELETE_POST.Id, PERMISSION_DELETE_OTHERS_POSTS.Id, PERMISSION_CREATE_TEAM.Id, + PERMISSION_ADD_USER_TO_TEAM.Id, }, ROLE_TEAM_USER.Permissions..., ), diff --git a/utils/authorization.go b/utils/authorization.go index 2c7f35164..086caa565 100644 --- a/utils/authorization.go +++ b/utils/authorization.go @@ -195,17 +195,26 @@ func SetDefaultRolesBasedOnConfig() { ) } - // If team admins are given permission - if *Cfg.TeamSettings.RestrictTeamInvite == model.PERMISSIONS_TEAM_ADMIN { - model.ROLE_TEAM_ADMIN.Permissions = append( - model.ROLE_TEAM_ADMIN.Permissions, - model.PERMISSION_INVITE_USER.Id, - ) - // If it's not restricted to system admin or team admin, then give all users permission - } else if *Cfg.TeamSettings.RestrictTeamInvite != model.PERMISSIONS_SYSTEM_ADMIN { - model.ROLE_SYSTEM_USER.Permissions = append( - model.ROLE_SYSTEM_USER.Permissions, + // Grant permissions for inviting and adding users to a team. + if IsLicensed { + if *Cfg.TeamSettings.RestrictTeamInvite == model.PERMISSIONS_TEAM_ADMIN { + model.ROLE_TEAM_ADMIN.Permissions = append( + model.ROLE_TEAM_ADMIN.Permissions, + model.PERMISSION_INVITE_USER.Id, + model.PERMISSION_ADD_USER_TO_TEAM.Id, + ) + } else if *Cfg.TeamSettings.RestrictTeamInvite == model.PERMISSIONS_ALL { + model.ROLE_SYSTEM_USER.Permissions = append( + model.ROLE_SYSTEM_USER.Permissions, + model.PERMISSION_INVITE_USER.Id, + model.PERMISSION_ADD_USER_TO_TEAM.Id, + ) + } + } else { + model.ROLE_TEAM_USER.Permissions = append( + model.ROLE_TEAM_USER.Permissions, model.PERMISSION_INVITE_USER.Id, + model.PERMISSION_ADD_USER_TO_TEAM.Id, ) } -- cgit v1.2.3-1-g7c22