diff --git a/server/channels/api4/apitestlib.go b/server/channels/api4/apitestlib.go index 9890b90675..a939075e91 100644 --- a/server/channels/api4/apitestlib.go +++ b/server/channels/api4/apitestlib.go @@ -620,6 +620,17 @@ func (th *TestHelper) CreateUser() *model.User { return th.CreateUserWithClient(th.Client) } +func (th *TestHelper) CreateGuestUser(tb testing.TB) *model.User { + tb.Helper() + + guestUser := th.CreateUserWithClient(th.Client) + + _, appErr := th.App.UpdateUserRoles(th.Context, guestUser.Id, model.SystemGuestRoleId, false) + require.Nil(tb, appErr) + + return guestUser +} + func (th *TestHelper) CreateTeam() *model.Team { return th.CreateTeamWithClient(th.Client) } diff --git a/server/channels/api4/team.go b/server/channels/api4/team.go index 8e3d0797d3..767580218e 100644 --- a/server/channels/api4/team.go +++ b/server/channels/api4/team.go @@ -695,7 +695,6 @@ func addTeamMember(c *Context, w http.ResponseWriter, r *http.Request) { return } - var err *model.AppError var member model.TeamMember if jsonErr := json.NewDecoder(r.Body).Decode(&member); jsonErr != nil { c.Err = model.NewAppError("addTeamMember", "api.team.add_team_member.invalid_body.app_error", nil, "Error in model.TeamMemberFromJSON()", http.StatusBadRequest).Wrap(jsonErr) @@ -717,7 +716,7 @@ func addTeamMember(c *Context, w http.ResponseWriter, r *http.Request) { if member.UserId == c.AppContext.Session().UserId { var team *model.Team - team, err = c.App.GetTeam(member.TeamId) + team, err := c.App.GetTeam(member.TeamId) if err != nil { c.Err = err return @@ -736,6 +735,19 @@ func addTeamMember(c *Context, w http.ResponseWriter, r *http.Request) { c.SetPermissionError(model.PermissionAddUserToTeam) return } + + canInviteGuests := c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), c.Params.TeamId, model.PermissionInviteGuest) + if !canInviteGuests { + user, err := c.App.GetUser(member.UserId) + if err != nil { + c.Err = model.NewAppError("addTeamMembers", "api.team.user.missing_account", nil, "", http.StatusNotFound).Wrap(err) + return + } + if user.IsGuest() { + c.SetPermissionError(model.PermissionInviteGuest) + return + } + } } team, err := c.App.GetTeam(member.TeamId) diff --git a/server/channels/api4/team_test.go b/server/channels/api4/team_test.go index 751980edbe..888c84c70f 100644 --- a/server/channels/api4/team_test.go +++ b/server/channels/api4/team_test.go @@ -2422,6 +2422,45 @@ func TestAddTeamMember(t *testing.T) { }) } +func TestAddTeamMemberGuestPermissions(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + enableGuestAccounts := *th.App.Config().GuestAccountsSettings.Enable + defer func() { + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.GuestAccountsSettings.Enable = enableGuestAccounts }) + appErr := th.App.Srv().RemoveLicense() + require.Nil(t, appErr) + }() + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.GuestAccountsSettings.Enable = true }) + th.App.Srv().SetLicense(model.NewTestLicense()) + + defaultRolePermissions := th.SaveDefaultRolePermissions() + defer func() { + th.RestoreDefaultRolePermissions(defaultRolePermissions) + }() + + t.Run("should be able to add guest user to team when you have permission to", func(t *testing.T) { + th.AddPermissionToRole(model.PermissionInviteGuest.Id, model.TeamUserRoleId) + + guestUser := th.CreateGuestUser(t) + + member, _, err := th.Client.AddTeamMember(context.Background(), th.BasicTeam.Id, guestUser.Id) + assert.NoError(t, err) + assert.NotNil(t, member) + }) + + t.Run("should not be able to add guest user to team when you don't have permissino to", func(t *testing.T) { + th.RemovePermissionFromRole(model.PermissionInviteGuest.Id, model.TeamUserRoleId) + + guestUser := th.CreateGuestUser(t) + + _, resp, err := th.Client.AddTeamMember(context.Background(), th.BasicTeam.Id, guestUser.Id) + assert.Error(t, err) + assert.Equal(t, http.StatusForbidden, resp.StatusCode) + }) +} + func TestAddTeamMemberMyself(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() @@ -2582,9 +2621,7 @@ func TestAddTeamMembers(t *testing.T) { otherUser.Id, } - guestUser := th.CreateUser() - _, appErr := th.App.UpdateUserRoles(th.Context, guestUser.Id, model.SystemGuestRoleId, false) - require.Nil(t, appErr) + guestUser := th.CreateGuestUser(t) guestList := []string{ guestUser.Id, } @@ -2594,7 +2631,7 @@ func TestAddTeamMembers(t *testing.T) { }) bot := th.CreateBotWithSystemAdminClient() - appErr = th.App.RemoveUserFromTeam(th.Context, th.BasicTeam.Id, th.BasicUser2.Id, "") + appErr := th.App.RemoveUserFromTeam(th.Context, th.BasicTeam.Id, th.BasicUser2.Id, "") require.Nil(t, appErr) // Regular user can't add a member to a team they don't belong to. @@ -2734,6 +2771,45 @@ func TestAddTeamMembers(t *testing.T) { require.NoError(t, err) } +func TestAddTeamMembersGuestPermissions(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + enableGuestAccounts := *th.App.Config().GuestAccountsSettings.Enable + defer func() { + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.GuestAccountsSettings.Enable = enableGuestAccounts }) + appErr := th.App.Srv().RemoveLicense() + require.Nil(t, appErr) + }() + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.GuestAccountsSettings.Enable = true }) + th.App.Srv().SetLicense(model.NewTestLicense()) + + defaultRolePermissions := th.SaveDefaultRolePermissions() + defer func() { + th.RestoreDefaultRolePermissions(defaultRolePermissions) + }() + + t.Run("should be able to add guest user to team when you have permission to", func(t *testing.T) { + th.AddPermissionToRole(model.PermissionInviteGuest.Id, model.TeamUserRoleId) + + guestUser := th.CreateGuestUser(t) + + members, _, err := th.Client.AddTeamMembers(context.Background(), th.BasicTeam.Id, []string{guestUser.Id}) + assert.NoError(t, err) + assert.Len(t, members, 1) + }) + + t.Run("should not be able to add guest user to team when you don't have permissino to", func(t *testing.T) { + th.RemovePermissionFromRole(model.PermissionInviteGuest.Id, model.TeamUserRoleId) + + guestUser := th.CreateGuestUser(t) + + _, resp, err := th.Client.AddTeamMembers(context.Background(), th.BasicTeam.Id, []string{guestUser.Id}) + assert.Error(t, err) + assert.Equal(t, http.StatusForbidden, resp.StatusCode) + }) +} + func TestRemoveTeamMember(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() diff --git a/server/channels/api4/user_test.go b/server/channels/api4/user_test.go index 7d62b2543a..66c7211001 100644 --- a/server/channels/api4/user_test.go +++ b/server/channels/api4/user_test.go @@ -6344,9 +6344,7 @@ func TestPromoteGuestToUser(t *testing.T) { th.App.UpdateConfig(func(cfg *model.Config) { *cfg.GuestAccountsSettings.Enable = true }) th.App.Srv().SetLicense(model.NewTestLicense()) - user := th.BasicUser - _, appErr := th.App.UpdateUserRoles(th.Context, user.Id, model.SystemGuestRoleId, false) - require.Nil(t, appErr) + user := th.CreateGuestUser(t) th.TestForSystemAdminAndLocal(t, func(t *testing.T, c *model.Client4) { _, _, err := c.GetUser(context.Background(), user.Id, "")