diff --git a/api4/team_test.go b/api4/team_test.go index 7e4dcfbc96..7d7b6e09c6 100644 --- a/api4/team_test.go +++ b/api4/team_test.go @@ -395,6 +395,36 @@ func TestPatchTeam(t *testing.T) { require.NotEqual(t, rteam.InviteId, "inviteid1", "InviteId should not update") require.True(t, rteam.AllowOpenInvite, "AllowOpenInvite did not update properly") + t.Run("Changing AllowOpenInvite to false regenerates InviteID", func(t *testing.T) { + team2 := &model.Team{DisplayName: "Name2", Description: "Some description", CompanyName: "Some company name", AllowOpenInvite: true, InviteId: model.NewId(), Name: "z-z-" + model.NewRandomTeamName() + "a", Email: "success+" + model.NewId() + "@simulator.amazonses.com", Type: model.TEAM_OPEN} + team2, _ = Client.CreateTeam(team2) + + patch2 := &model.TeamPatch{ + AllowOpenInvite: model.NewBool(false), + } + + rteam2, resp2 := Client.PatchTeam(team2.Id, patch2) + CheckNoError(t, resp2) + require.Equal(t, team2.Id, rteam2.Id) + require.False(t, rteam2.AllowOpenInvite) + require.NotEqual(t, team2.InviteId, rteam2.InviteId) + }) + + t.Run("Changing AllowOpenInvite to true doesn't regenerate InviteID", func(t *testing.T) { + team2 := &model.Team{DisplayName: "Name3", Description: "Some description", CompanyName: "Some company name", AllowOpenInvite: false, InviteId: model.NewId(), Name: "z-z-" + model.NewRandomTeamName() + "a", Email: "success+" + model.NewId() + "@simulator.amazonses.com", Type: model.TEAM_OPEN} + team2, _ = Client.CreateTeam(team2) + + patch2 := &model.TeamPatch{ + AllowOpenInvite: model.NewBool(true), + } + + rteam2, resp2 := Client.PatchTeam(team2.Id, patch2) + CheckNoError(t, resp2) + require.Equal(t, team2.Id, rteam2.Id) + require.True(t, rteam2.AllowOpenInvite) + require.Equal(t, team2.InviteId, rteam2.InviteId) + }) + // Test GroupConstrained flag patch.GroupConstrained = model.NewBool(true) rteam, resp = Client.PatchTeam(team.Id, patch) @@ -555,19 +585,21 @@ func TestUpdateTeamPrivacy(t *testing.T) { teamPrivate2 := createTeam(model.TEAM_INVITE, false) tests := []struct { - name string - team *model.Team - privacy string - errChecker func(t *testing.T, resp *model.Response) - wantType string - wantOpenInvite bool + name string + team *model.Team + privacy string + errChecker func(t *testing.T, resp *model.Response) + wantType string + wantOpenInvite bool + wantInviteIdChanged bool + originalInviteId string }{ {name: "bad privacy", team: teamPublic, privacy: "blap", errChecker: CheckBadRequestStatus, wantType: model.TEAM_OPEN, wantOpenInvite: true}, {name: "bad team", team: &model.Team{Id: model.NewId()}, privacy: model.TEAM_OPEN, errChecker: CheckForbiddenStatus, wantType: model.TEAM_OPEN, wantOpenInvite: true}, - {name: "public to private", team: teamPublic, privacy: model.TEAM_INVITE, errChecker: nil, wantType: model.TEAM_INVITE, wantOpenInvite: false}, - {name: "private to public", team: teamPrivate, privacy: model.TEAM_OPEN, errChecker: nil, wantType: model.TEAM_OPEN, wantOpenInvite: true}, - {name: "public to public", team: teamPublic2, privacy: model.TEAM_OPEN, errChecker: nil, wantType: model.TEAM_OPEN, wantOpenInvite: true}, - {name: "private to private", team: teamPrivate2, privacy: model.TEAM_INVITE, errChecker: nil, wantType: model.TEAM_INVITE, wantOpenInvite: false}, + {name: "public to private", team: teamPublic, privacy: model.TEAM_INVITE, errChecker: nil, wantType: model.TEAM_INVITE, wantOpenInvite: false, originalInviteId: teamPublic.InviteId, wantInviteIdChanged: true}, + {name: "private to public", team: teamPrivate, privacy: model.TEAM_OPEN, errChecker: nil, wantType: model.TEAM_OPEN, wantOpenInvite: true, originalInviteId: teamPrivate.InviteId, wantInviteIdChanged: false}, + {name: "public to public", team: teamPublic2, privacy: model.TEAM_OPEN, errChecker: nil, wantType: model.TEAM_OPEN, wantOpenInvite: true, originalInviteId: teamPublic2.InviteId, wantInviteIdChanged: false}, + {name: "private to private", team: teamPrivate2, privacy: model.TEAM_INVITE, errChecker: nil, wantType: model.TEAM_INVITE, wantOpenInvite: false, originalInviteId: teamPrivate2.InviteId, wantInviteIdChanged: false}, } for _, test := range tests { @@ -582,6 +614,11 @@ func TestUpdateTeamPrivacy(t *testing.T) { } require.Equal(t, test.wantType, team.Type) require.Equal(t, test.wantOpenInvite, team.AllowOpenInvite) + if test.wantInviteIdChanged { + require.NotEqual(t, test.originalInviteId, team.InviteId) + } else { + require.Equal(t, test.originalInviteId, team.InviteId) + } }) } diff --git a/app/app_iface.go b/app/app_iface.go index c17a2593e7..44fccfe578 100644 --- a/app/app_iface.go +++ b/app/app_iface.go @@ -376,6 +376,7 @@ type AppIface interface { CheckUserMfa(user *model.User, token string) *model.AppError CheckUserPostflightAuthenticationCriteria(user *model.User) *model.AppError CheckUserPreflightAuthenticationCriteria(user *model.User, mfaToken string) *model.AppError + CheckValidDomains(team *model.Team) *model.AppError ClearChannelMembersCache(channelID string) ClearSessionCacheForAllUsers() ClearSessionCacheForAllUsersSkipClusterSend() diff --git a/app/opentracing_layer.go b/app/opentracing_layer.go index 52114f272e..9868b7fbf5 100644 --- a/app/opentracing_layer.go +++ b/app/opentracing_layer.go @@ -1111,6 +1111,28 @@ func (a *OpenTracingAppLayer) CheckUserPreflightAuthenticationCriteria(user *mod return resultVar0 } +func (a *OpenTracingAppLayer) CheckValidDomains(team *model.Team) *model.AppError { + origCtx := a.ctx + span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.CheckValidDomains") + + a.ctx = newCtx + a.app.Srv().Store.SetContext(newCtx) + defer func() { + a.app.Srv().Store.SetContext(origCtx) + a.ctx = origCtx + }() + + defer span.Finish() + resultVar0 := a.app.CheckValidDomains(team) + + if resultVar0 != nil { + span.LogFields(spanlog.Error(resultVar0)) + ext.Error.Set(span, true) + } + + return resultVar0 +} + func (a *OpenTracingAppLayer) ClearChannelMembersCache(channelID string) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.ClearChannelMembersCache") diff --git a/app/team.go b/app/team.go index ab999884fa..8c3c943fc9 100644 --- a/app/team.go +++ b/app/team.go @@ -103,12 +103,7 @@ func (a *App) getAllowedDomains(user *model.User, team *model.Team) []string { return []string{team.AllowedDomains, *a.Config().TeamSettings.RestrictCreationToDomains} } -func (a *App) UpdateTeam(team *model.Team) (*model.Team, *model.AppError) { - oldTeam, err := a.GetTeam(team.Id) - if err != nil { - return nil, err - } - +func (a *App) CheckValidDomains(team *model.Team) *model.AppError { validDomains := a.normalizeDomains(*a.Config().TeamSettings.RestrictCreationToDomains) if len(validDomains) > 0 { for _, domain := range a.normalizeDomains(team.AllowedDomains) { @@ -120,12 +115,25 @@ func (a *App) UpdateTeam(team *model.Team) (*model.Team, *model.AppError) { } } if !matched { - err = model.NewAppError("UpdateTeam", "api.team.update_restricted_domains.mismatch.app_error", map[string]interface{}{"Domain": domain}, "", http.StatusBadRequest) - return nil, err + err := model.NewAppError("UpdateTeam", "api.team.update_restricted_domains.mismatch.app_error", map[string]interface{}{"Domain": domain}, "", http.StatusBadRequest) + return err } } } + return nil +} + +func (a *App) UpdateTeam(team *model.Team) (*model.Team, *model.AppError) { + oldTeam, err := a.GetTeam(team.Id) + if err != nil { + return nil, err + } + + if err = a.CheckValidDomains(team); err != nil { + return nil, err + } + oldTeam.DisplayName = team.DisplayName oldTeam.Description = team.Description oldTeam.AllowOpenInvite = team.AllowOpenInvite @@ -199,6 +207,11 @@ func (a *App) UpdateTeamPrivacy(teamId string, teamType string, allowOpenInvite return err } + // Force a regeneration of the invite token if changing a team to restricted. + if (allowOpenInvite != oldTeam.AllowOpenInvite || teamType != oldTeam.Type) && (!allowOpenInvite || teamType == model.TEAM_INVITE) { + oldTeam.InviteId = model.NewId() + } + oldTeam.Type = teamType oldTeam.AllowOpenInvite = allowOpenInvite @@ -218,15 +231,22 @@ func (a *App) PatchTeam(teamId string, patch *model.TeamPatch) (*model.Team, *mo } team.Patch(patch) + if patch.AllowOpenInvite != nil && !*patch.AllowOpenInvite { + team.InviteId = model.NewId() + } - updatedTeam, err := a.UpdateTeam(team) - if err != nil { + if err = a.CheckValidDomains(team); err != nil { return nil, err } - a.sendTeamEvent(updatedTeam, model.WEBSOCKET_EVENT_UPDATE_TEAM) + team, err = a.updateTeamUnsanitized(team) + if err != nil { + return team, err + } - return updatedTeam, nil + a.sendTeamEvent(team, model.WEBSOCKET_EVENT_UPDATE_TEAM) + + return team, nil } func (a *App) RegenerateTeamInviteId(teamId string) (*model.Team, *model.AppError) {