From 977c791e5b54bc14fdb96a2b3dacc85da5d8c623 Mon Sep 17 00:00:00 2001 From: Nick Misasi Date: Tue, 5 May 2026 02:23:37 -0400 Subject: [PATCH] MM-68382: Align team creation invite permission checks (#36188) (#36402) Automatic Merge --- server/channels/api4/team.go | 36 ++++- server/channels/api4/team_test.go | 209 ++++++++++++++++++++++++++++-- 2 files changed, 232 insertions(+), 13 deletions(-) diff --git a/server/channels/api4/team.go b/server/channels/api4/team.go index 2f44c6dc0b..708a703f2c 100644 --- a/server/channels/api4/team.go +++ b/server/channels/api4/team.go @@ -120,17 +120,20 @@ func createTeam(c *Context, w http.ResponseWriter, r *http.Request) { return } + // Setting AllowOpenInvite or AllowedDomains requires PermissionInviteUser, matching updateTeam/patchTeam. + if (team.AllowOpenInvite || team.AllowedDomains != "") && !creatorCanInviteUsersOnTeam(c, &team) { + c.SetPermissionError(model.PermissionInviteUser) + return + } + rteam, err := c.App.CreateTeamWithUser(c.AppContext, &team, c.AppContext.Session().UserId) if err != nil { c.Err = err return } - // Don't sanitize the team here since the user will be a team admin and their session won't reflect that yet - // instead check the scheme roles for the team and if the user has the permission to invite users - _, schemeUserRole, schemeAdminRole, schemeErr := c.App.GetSchemeRolesForTeam(rteam.Id) - if schemeErr != nil || !c.App.RolesGrantPermission([]string{schemeUserRole, schemeAdminRole}, model.PermissionInviteUser.Id) { - // If we can't check permissions, fail secure by hiding the invite_id because the team is already created above + // The creator's session doesn't yet reflect their team_admin role, so check the team's default roles directly. + if !creatorCanInviteUsersOnTeam(c, rteam) { rteam.InviteId = "" } @@ -144,6 +147,29 @@ func createTeam(c *Context, w http.ResponseWriter, r *http.Request) { } } +// creatorCanInviteUsersOnTeam checks whether the creator will have PermissionInviteUser on the new team, +// using the team's scheme (if any) or the built-in team roles as defaults. +func creatorCanInviteUsersOnTeam(c *Context, team *model.Team) bool { + if c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionInviteUser) { + return true + } + + if team.SchemeId != nil && *team.SchemeId != "" { + scheme, appErr := c.App.GetScheme(*team.SchemeId) + if appErr != nil { + c.Logger.Warn("Failed to fetch scheme while checking invite permission for new team", + mlog.String("scheme_id", *team.SchemeId), + mlog.Err(appErr), + ) + return false + } + + return c.App.RolesGrantPermission([]string{scheme.DefaultTeamUserRole, scheme.DefaultTeamAdminRole}, model.PermissionInviteUser.Id) + } + + return c.App.RolesGrantPermission([]string{model.TeamUserRoleId, model.TeamAdminRoleId}, model.PermissionInviteUser.Id) +} + func getTeam(c *Context, w http.ResponseWriter, r *http.Request) { c.RequireTeamId() if c.Err != nil { diff --git a/server/channels/api4/team_test.go b/server/channels/api4/team_test.go index db6ed64d47..d89d79b87b 100644 --- a/server/channels/api4/team_test.go +++ b/server/channels/api4/team_test.go @@ -242,23 +242,216 @@ func TestCreateTeamInviteIdHiddenWithoutInvitePermission(t *testing.T) { defaultRolePermissions := th.SaveDefaultRolePermissions() defer th.RestoreDefaultRolePermissions(defaultRolePermissions) - // Remove PermissionInviteUser from the default team user role + // team_admin inherits from team_user by default, so removing from team_user is enough. th.RemovePermissionFromRole(model.PermissionInviteUser.Id, model.TeamUserRoleId) - // Regular user creates a team - InviteId should be hidden - // since the team user role lacks invite permission rteam, _, err := th.Client.CreateTeam(context.Background(), &model.Team{ - DisplayName: "Team Without Invite Permission", - Name: GenerateTestTeamName(), - Email: th.GenerateTestEmail(), - Type: model.TeamOpen, - AllowedDomains: "simulator.amazonses.com,localhost", + DisplayName: "Team Without Invite Permission", + Name: GenerateTestTeamName(), + Email: th.GenerateTestEmail(), + Type: model.TeamOpen, }) require.NoError(t, err) require.NotEmpty(t, rteam.Email, "should not have sanitized email") require.Empty(t, rteam.InviteId, "should have hidden invite_id when user lacks invite permission") } +func TestCreateTeamInviteUserPermission(t *testing.T) { + th := Setup(t) + + defaultRolePermissions := th.SaveDefaultRolePermissions() + defer th.RestoreDefaultRolePermissions(defaultRolePermissions) + + th.RemovePermissionFromRole(model.PermissionInviteUser.Id, model.TeamUserRoleId) + th.RemovePermissionFromRole(model.PermissionInviteUser.Id, model.TeamAdminRoleId) + + t.Run("AllowOpenInvite=true is rejected with 403", func(t *testing.T) { + _, resp, err := th.Client.CreateTeam(context.Background(), &model.Team{ + DisplayName: "Open Invite Team Without Permission", + Name: GenerateTestTeamName(), + Email: th.GenerateTestEmail(), + Type: model.TeamOpen, + AllowOpenInvite: true, + }) + require.Error(t, err) + CheckForbiddenStatus(t, resp) + }) + + t.Run("non-empty AllowedDomains is rejected with 403", func(t *testing.T) { + creatorDomain := strings.SplitN(th.BasicUser.Email, "@", 2)[1] + + _, resp, err := th.Client.CreateTeam(context.Background(), &model.Team{ + DisplayName: "Restricted Domains Team Without Permission", + Name: GenerateTestTeamName(), + Email: th.GenerateTestEmail(), + Type: model.TeamOpen, + AllowedDomains: creatorDomain, + }) + require.Error(t, err) + CheckForbiddenStatus(t, resp) + }) + + t.Run("team without invite-restricted fields is still created", func(t *testing.T) { + createdTeam, resp, err := th.Client.CreateTeam(context.Background(), &model.Team{ + DisplayName: "Plain Team Without Invite Permission", + Name: GenerateTestTeamName(), + Email: th.GenerateTestEmail(), + Type: model.TeamOpen, + }) + require.NoError(t, err) + CheckCreatedStatus(t, resp) + + assert.False(t, createdTeam.AllowOpenInvite) + assert.Empty(t, createdTeam.AllowedDomains) + assert.Empty(t, createdTeam.InviteId, "InviteId should be hidden from creators that can't invite users") + }) +} + +func TestCreateTeamInviteUserPermissionSystemAdmin(t *testing.T) { + th := Setup(t) + creatorDomain := strings.SplitN(th.SystemAdminUser.Email, "@", 2)[1] + + defaultRolePermissions := th.SaveDefaultRolePermissions() + defer th.RestoreDefaultRolePermissions(defaultRolePermissions) + + th.RemovePermissionFromRole(model.PermissionInviteUser.Id, model.TeamUserRoleId) + th.RemovePermissionFromRole(model.PermissionInviteUser.Id, model.TeamAdminRoleId) + + createdTeam, resp, err := th.SystemAdminClient.CreateTeam(context.Background(), &model.Team{ + DisplayName: "System Admin Team With Invite Permission", + Name: GenerateTestTeamName(), + Email: th.GenerateTestEmail(), + Type: model.TeamOpen, + AllowOpenInvite: true, + AllowedDomains: creatorDomain, + }) + require.NoError(t, err) + CheckCreatedStatus(t, resp) + + assert.True(t, createdTeam.AllowOpenInvite, "system admins should still be able to create open invite teams") + assert.Equal(t, creatorDomain, createdTeam.AllowedDomains, "system admins should still be able to set allowed domains") + require.NotEmpty(t, createdTeam.InviteId, "system admins should receive the invite_id when they can invite users") + + persistedTeam, _, err := th.SystemAdminClient.GetTeam(context.Background(), createdTeam.Id, "") + require.NoError(t, err) + + assert.True(t, persistedTeam.AllowOpenInvite, "system admins should persist open invite team settings") + assert.Equal(t, creatorDomain, persistedTeam.AllowedDomains, "system admins should persist allowed domains") +} + +// Exercises the scheme branch of creatorCanInviteUsersOnTeam. +func TestCreateTeamInviteUserPermissionScheme(t *testing.T) { + th := Setup(t) + th.App.Srv().SetLicense(model.NewTestLicense("custom_permissions_schemes")) + err := th.App.SetPhase2PermissionsMigrationStatus(true) + require.NoError(t, err) + + defaultRolePermissions := th.SaveDefaultRolePermissions() + defer th.RestoreDefaultRolePermissions(defaultRolePermissions) + + // Remove InviteUser from the built-in team roles; new schemes inherit from these at creation, so their defaults start without it too. + th.RemovePermissionFromRole(model.PermissionInviteUser.Id, model.TeamUserRoleId) + th.RemovePermissionFromRole(model.PermissionInviteUser.Id, model.TeamAdminRoleId) + + // SystemManager has SchemeWrite but not system-level InviteUser, so it exercises the scheme branch. + th.LoginSystemManager() + managerClient := th.SystemManagerClient + + t.Run("scheme admin role grants InviteUser - create succeeds", func(t *testing.T) { + scheme, _, err := th.SystemAdminClient.CreateScheme(context.Background(), &model.Scheme{ + DisplayName: "dn_" + model.NewId(), + Name: model.NewId(), + Scope: model.SchemeScopeTeam, + }) + require.NoError(t, err) + require.NotEmpty(t, scheme.DefaultTeamAdminRole) + require.NotEmpty(t, scheme.DefaultTeamUserRole) + + th.AddPermissionToRole(model.PermissionInviteUser.Id, scheme.DefaultTeamAdminRole) + + rteam, resp, err := managerClient.CreateTeam(context.Background(), &model.Team{ + DisplayName: "Scheme Team With Invite " + model.NewId(), + Name: GenerateTestTeamName(), + Email: th.GenerateTestEmail(), + Type: model.TeamOpen, + SchemeId: &scheme.Id, + AllowOpenInvite: true, + }) + require.NoError(t, err) + CheckCreatedStatus(t, resp) + + assert.True(t, rteam.AllowOpenInvite, "AllowOpenInvite should be preserved when scheme grants InviteUser") + assert.NotEmpty(t, rteam.InviteId, "InviteId should be returned when scheme grants InviteUser") + }) + + t.Run("scheme roles do not grant InviteUser - create is rejected", func(t *testing.T) { + scheme, _, err := th.SystemAdminClient.CreateScheme(context.Background(), &model.Scheme{ + DisplayName: "dn_" + model.NewId(), + Name: model.NewId(), + Scope: model.SchemeScopeTeam, + }) + require.NoError(t, err) + + _, resp, err := managerClient.CreateTeam(context.Background(), &model.Team{ + DisplayName: "Scheme Team Without Invite " + model.NewId(), + Name: GenerateTestTeamName(), + Email: th.GenerateTestEmail(), + Type: model.TeamOpen, + SchemeId: &scheme.Id, + AllowOpenInvite: true, + }) + require.Error(t, err) + CheckForbiddenStatus(t, resp) + }) + + t.Run("scheme roles do not grant InviteUser but no invite-restricted fields - create succeeds with hidden invite_id", func(t *testing.T) { + scheme, _, err := th.SystemAdminClient.CreateScheme(context.Background(), &model.Scheme{ + DisplayName: "dn_" + model.NewId(), + Name: model.NewId(), + Scope: model.SchemeScopeTeam, + }) + require.NoError(t, err) + + rteam, resp, err := managerClient.CreateTeam(context.Background(), &model.Team{ + DisplayName: "Scheme Team Without Invite Fields " + model.NewId(), + Name: GenerateTestTeamName(), + Email: th.GenerateTestEmail(), + Type: model.TeamOpen, + SchemeId: &scheme.Id, + }) + require.NoError(t, err) + CheckCreatedStatus(t, resp) + + assert.Empty(t, rteam.InviteId, "InviteId should be hidden when scheme does not grant InviteUser") + }) + + t.Run("scheme admin role grants InviteUser but no invite-restricted fields - create succeeds with invite_id", func(t *testing.T) { + scheme, _, err := th.SystemAdminClient.CreateScheme(context.Background(), &model.Scheme{ + DisplayName: "dn_" + model.NewId(), + Name: model.NewId(), + Scope: model.SchemeScopeTeam, + }) + require.NoError(t, err) + require.NotEmpty(t, scheme.DefaultTeamAdminRole) + + th.AddPermissionToRole(model.PermissionInviteUser.Id, scheme.DefaultTeamAdminRole) + + rteam, resp, err := managerClient.CreateTeam(context.Background(), &model.Team{ + DisplayName: "Scheme Team Invite Via Defaults Only " + model.NewId(), + Name: GenerateTestTeamName(), + Email: th.GenerateTestEmail(), + Type: model.TeamOpen, + SchemeId: &scheme.Id, + }) + require.NoError(t, err) + CheckCreatedStatus(t, resp) + + assert.False(t, rteam.AllowOpenInvite) + assert.Empty(t, rteam.AllowedDomains) + assert.NotEmpty(t, rteam.InviteId, "InviteId should be returned when scheme grants InviteUser without open invite or domain restrictions") + }) +} + func TestGetTeam(t *testing.T) { mainHelper.Parallel(t) th := Setup(t).InitBasic()