From ea86dc9a627ae8f2a75a32d0f4c8f42d53b327fb Mon Sep 17 00:00:00 2001 From: Ashish Bhate Date: Fri, 22 May 2020 15:52:57 +0530 Subject: [PATCH] MM-24872/MM-24873: local mode support for addTeamMember and removeTeamMember (#14534) * Add local mode handler for addTeamMember * Add local mode handler for remoteTeamMember * short circuit session team permission for local mode Co-authored-by: Ibrahim Serdar Acikgoz Co-authored-by: Miguel de la Cruz --- api4/api.go | 3 ++ api4/team_local.go | 2 ++ api4/team_test.go | 69 +++++++++++++++++++++++++++++--------------- app/authorization.go | 3 ++ 4 files changed, 54 insertions(+), 23 deletions(-) diff --git a/api4/api.go b/api4/api.go index 1d015cb53b..7cea10c6e0 100644 --- a/api4/api.go +++ b/api4/api.go @@ -270,6 +270,9 @@ func InitLocal(configservice configservice.ConfigService, globalOptionsFunc app. api.BaseRoutes.ApiRoot = root.PathPrefix(model.API_URL_SUFFIX).Subrouter() api.BaseRoutes.Teams = api.BaseRoutes.ApiRoot.PathPrefix("/teams").Subrouter() + api.BaseRoutes.Team = api.BaseRoutes.Teams.PathPrefix("/{team_id:[A-Za-z0-9]+}").Subrouter() + api.BaseRoutes.TeamMembers = api.BaseRoutes.Team.PathPrefix("/members").Subrouter() + api.BaseRoutes.TeamMember = api.BaseRoutes.TeamMembers.PathPrefix("/{user_id:[A-Za-z0-9]+}").Subrouter() api.BaseRoutes.Channels = api.BaseRoutes.ApiRoot.PathPrefix("/channels").Subrouter() diff --git a/api4/team_local.go b/api4/team_local.go index 184e1eccd5..340d72f6ae 100644 --- a/api4/team_local.go +++ b/api4/team_local.go @@ -5,4 +5,6 @@ package api4 func (api *API) InitTeamLocal() { api.BaseRoutes.Teams.Handle("", api.ApiLocal(getAllTeams)).Methods("GET") + api.BaseRoutes.TeamMembers.Handle("", api.ApiLocal(addTeamMember)).Methods("POST") + api.BaseRoutes.TeamMember.Handle("", api.ApiLocal(removeTeamMember)).Methods("DELETE") } diff --git a/api4/team_test.go b/api4/team_test.go index 7d7b6e09c6..dc860389a5 100644 --- a/api4/team_test.go +++ b/api4/team_test.go @@ -1654,6 +1654,15 @@ func TestAddTeamMember(t *testing.T) { require.NotNil(t, resp.Error, "Error is nil") Client.Logout() + // SystemAdmin and mode can add member to a team + th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) { + tm, r := client.AddTeamMember(team.Id, otherUser.Id) + CheckNoError(t, r) + CheckCreatedStatus(t, r) + require.Equal(t, tm.UserId, otherUser.Id, "user ids should have matched") + require.Equal(t, tm.TeamId, team.Id, "team ids should have matched") + }) + // Regular user can add a member to a team they belong to. th.LoginBasic() tm, resp := Client.AddTeamMember(team.Id, otherUser.Id) @@ -1817,8 +1826,10 @@ func TestAddTeamMember(t *testing.T) { require.Equal(t, "app.team.invite_id.group_constrained.error", resp.Error.Id) // User is not in associated groups so shouldn't be allowed - _, resp = th.SystemAdminClient.AddTeamMember(team.Id, otherUser.Id) - CheckErrorMessage(t, resp, "api.team.add_members.user_denied") + th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) { + _, resp = client.AddTeamMember(team.Id, otherUser.Id) + CheckErrorMessage(t, resp, "api.team.add_members.user_denied") + }) // Associate group to team _, err = th.App.UpsertGroupSyncable(&model.GroupSyncable{ @@ -1832,8 +1843,10 @@ func TestAddTeamMember(t *testing.T) { _, err = th.App.UpsertGroupMember(th.Group.Id, otherUser.Id) require.Nil(t, err) - _, resp = th.SystemAdminClient.AddTeamMember(team.Id, otherUser.Id) - CheckNoError(t, resp) + th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) { + _, resp = client.AddTeamMember(team.Id, otherUser.Id) + CheckNoError(t, resp) + }) } func TestAddTeamMemberMyself(t *testing.T) { @@ -2123,28 +2136,31 @@ func TestRemoveTeamMember(t *testing.T) { }) bot := th.CreateBotWithSystemAdminClient() - pass, resp := Client.RemoveTeamMember(th.BasicTeam.Id, th.BasicUser.Id) - CheckNoError(t, resp) + th.TestForAllClients(t, func(t *testing.T, client *model.Client4) { + pass, resp := client.RemoveTeamMember(th.BasicTeam.Id, th.BasicUser.Id) + CheckNoError(t, resp) - require.True(t, pass, "should have passed") + require.True(t, pass, "should have passed") - _, resp = th.SystemAdminClient.AddTeamMember(th.BasicTeam.Id, th.BasicUser.Id) - CheckNoError(t, resp) + _, resp = th.SystemAdminClient.AddTeamMember(th.BasicTeam.Id, th.BasicUser.Id) + CheckNoError(t, resp) + }) - _, resp = Client.RemoveTeamMember(th.BasicTeam.Id, "junk") - CheckBadRequestStatus(t, resp) + th.TestForAllClients(t, func(t *testing.T, client *model.Client4) { + _, resp := client.RemoveTeamMember(th.BasicTeam.Id, "junk") + CheckBadRequestStatus(t, resp) - _, resp = Client.RemoveTeamMember("junk", th.BasicUser2.Id) - CheckBadRequestStatus(t, resp) + _, resp = client.RemoveTeamMember("junk", th.BasicUser2.Id) + CheckBadRequestStatus(t, resp) + }) - _, resp = Client.RemoveTeamMember(th.BasicTeam.Id, th.BasicUser2.Id) + _, resp := Client.RemoveTeamMember(th.BasicTeam.Id, th.BasicUser2.Id) CheckForbiddenStatus(t, resp) - _, resp = Client.RemoveTeamMember(model.NewId(), th.BasicUser.Id) - CheckNotFoundStatus(t, resp) - - _, resp = th.SystemAdminClient.RemoveTeamMember(th.BasicTeam.Id, th.BasicUser.Id) - CheckNoError(t, resp) + th.TestForAllClients(t, func(t *testing.T, client *model.Client4) { + _, resp = client.RemoveTeamMember(model.NewId(), th.BasicUser.Id) + CheckNotFoundStatus(t, resp) + }) _, resp = th.SystemAdminClient.AddTeamMember(th.BasicTeam.Id, th.SystemAdminUser.Id) CheckNoError(t, resp) @@ -2156,12 +2172,19 @@ func TestRemoveTeamMember(t *testing.T) { th.BasicTeam.GroupConstrained = model.NewBool(true) _, err := th.App.UpdateTeam(th.BasicTeam) require.Nil(t, err) - _, resp = th.SystemAdminClient.RemoveTeamMember(th.BasicTeam.Id, th.BasicUser.Id) - require.Equal(t, "api.team.remove_member.group_constrained.app_error", resp.Error.Id) + th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) { + _, resp = client.RemoveTeamMember(th.BasicTeam.Id, th.BasicUser.Id) + require.Equal(t, "api.team.remove_member.group_constrained.app_error", resp.Error.Id) + }) // Can remove a bot even if team is group-constrained - _, resp = th.SystemAdminClient.RemoveTeamMember(th.BasicTeam.Id, bot.UserId) - CheckNoError(t, resp) + + th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) { + _, resp = client.RemoveTeamMember(th.BasicTeam.Id, bot.UserId) + CheckNoError(t, resp) + _, resp = client.AddTeamMember(th.BasicTeam.Id, bot.UserId) + CheckNoError(t, resp) + }) // Can remove self even if team is group-constrained _, resp = th.SystemAdminClient.RemoveTeamMember(th.BasicTeam.Id, th.SystemAdminUser.Id) diff --git a/app/authorization.go b/app/authorization.go index 74ea109a9a..d219276e81 100644 --- a/app/authorization.go +++ b/app/authorization.go @@ -26,6 +26,9 @@ func (a *App) SessionHasPermissionToTeam(session model.Session, teamId string, p if teamId == "" { return false } + if session.IsUnrestricted() { + return true + } teamMember := session.GetTeamByTeamId(teamId) if teamMember != nil {