MM-24176: Regenerate invite ID on team privacy change. (#14529)

* MM-24176: Regenerate invite ID on team privacy change. * Fix the other place this setting can be changed. * Fix tests. * Satisfy the tyrannical golangci-lint.
2020-05-19 12:04:51 +01:00
--- 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)
+			}
 		})
 	}

--- 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()
--- 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")
--- 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) {