[MM-13671] Rework Team InviteId Creation and Updates (#10536)

* Add regenerate invite ID endpoint; Dont allow inviteID updates via other methods; Remove unrequired checks in get handler

* Fix tests; Dont accept TeamId as invite ID

* Ensure all teams have an InviteID set

* Custom Selector to get empty teams; dont crash when inviteid set fails

* Remote InviteId from TeamPatch

* Add missing translation

* Translation string order

* Use sync store

* gofmt

api4/team.go

@@ -33,6 +33,7 @@ func (api *API) InitTeam() {
 	api.BaseRoutes.Team.Handle("", api.ApiSessionRequired(deleteTeam)).Methods("DELETE")
 	api.BaseRoutes.Team.Handle("/patch", api.ApiSessionRequired(patchTeam)).Methods("PUT")
 	api.BaseRoutes.Team.Handle("/stats", api.ApiSessionRequired(getTeamStats)).Methods("GET")
+	api.BaseRoutes.Team.Handle("/regenerate_invite_id", api.ApiSessionRequired(regenerateTeamInviteId)).Methods("POST")
 
 	api.BaseRoutes.Team.Handle("/image", api.ApiSessionRequiredTrustRequester(getTeamIcon)).Methods("GET")
 	api.BaseRoutes.Team.Handle("/image", api.ApiSessionRequired(setTeamIcon)).Methods("POST")
@@ -190,6 +191,29 @@ func patchTeam(c *Context, w http.ResponseWriter, r *http.Request) {
 	w.Write([]byte(patchedTeam.ToJson()))
 }
 
+func regenerateTeamInviteId(c *Context, w http.ResponseWriter, r *http.Request) {
+	c.RequireTeamId()
+	if c.Err != nil {
+		return
+	}
+
+	if !c.App.SessionHasPermissionToTeam(c.App.Session, c.Params.TeamId, model.PERMISSION_MANAGE_TEAM) {
+		c.SetPermissionError(model.PERMISSION_MANAGE_TEAM)
+		return
+	}
+
+	patchedTeam, err := c.App.RegenerateTeamInviteId(c.Params.TeamId)
+	if err != nil {
+		c.Err = err
+		return
+	}
+
+	c.App.SanitizeTeam(c.App.Session, patchedTeam)
+
+	c.LogAudit("")
+	w.Write([]byte(patchedTeam.ToJson()))
+}
+
 func deleteTeam(c *Context, w http.ResponseWriter, r *http.Request) {
 	c.RequireTeamId()
 	if c.Err != nil {

api4/team_test.go

@@ -307,8 +307,8 @@ func TestUpdateTeam(t *testing.T) {
 	uteam, resp = Client.UpdateTeam(team)
 	CheckNoError(t, resp)
 
-	if uteam.InviteId != "inviteid1" {
+	if uteam.InviteId == "inviteid1" {
-		t.Fatal("Update failed")
+		t.Fatal("InviteID should not be updated")
 	}
 
 	team.AllowedDomains = "domain"
@@ -411,7 +411,6 @@ func TestPatchTeam(t *testing.T) {
 	patch.DisplayName = model.NewString("Other name")
 	patch.Description = model.NewString("Other description")
 	patch.CompanyName = model.NewString("Other company name")
-	patch.InviteId = model.NewString("inviteid1")
 	patch.AllowOpenInvite = model.NewBool(true)
 
 	rteam, resp := Client.PatchTeam(team.Id, patch)
@@ -426,8 +425,8 @@ func TestPatchTeam(t *testing.T) {
 	if rteam.CompanyName != "Other company name" {
 		t.Fatal("CompanyName did not update properly")
 	}
-	if rteam.InviteId != "inviteid1" {
+	if rteam.InviteId == "inviteid1" {
-		t.Fatal("InviteId did not update properly")
+		t.Fatal("InviteId should not update")
 	}
 	if !rteam.AllowOpenInvite {
 		t.Fatal("AllowOpenInvite did not update properly")
@@ -504,6 +503,24 @@ func TestPatchTeamSanitization(t *testing.T) {
 	})
 }
 
+func TestRegenerateTeamInviteId(t *testing.T) {
+	th := Setup().InitBasic()
+	defer th.TearDown()
+	Client := th.Client
+
+	team := &model.Team{DisplayName: "Name", Description: "Some description", CompanyName: "Some company name", AllowOpenInvite: false, InviteId: "inviteid0", Name: "z-z-" + model.NewId() + "a", Email: "success+" + model.NewId() + "@simulator.amazonses.com", Type: model.TEAM_OPEN}
+	team, _ = Client.CreateTeam(team)
+
+	assert.NotEqual(t, team.InviteId, "")
+	assert.NotEqual(t, team.InviteId, "inviteid0")
+
+	rteam, resp := Client.RegenerateTeamInviteId(team.Id)
+	CheckNoError(t, resp)
+
+	assert.NotEqual(t, team.InviteId, rteam.InviteId)
+	assert.NotEqual(t, team.InviteId, "")
+}
+
 func TestSoftDeleteTeam(t *testing.T) {
 	th := Setup().InitBasic()
 	defer th.TearDown()

api4/user_test.go

@@ -283,8 +283,7 @@ func TestCreateUserWithInviteId(t *testing.T) {
 
 		inviteId := th.BasicTeam.InviteId
 
-		th.BasicTeam.InviteId = model.NewId()
+		_, resp := th.SystemAdminClient.RegenerateTeamInviteId(th.BasicTeam.Id)
-		_, resp := th.SystemAdminClient.UpdateTeam(th.BasicTeam)
 		CheckNoError(t, resp)
 
 		_, resp = th.Client.CreateUserWithInviteId(&user, inviteId)
@@ -319,7 +318,9 @@ func TestCreateUserWithInviteId(t *testing.T) {
 
 		th.App.UpdateConfig(func(cfg *model.Config) { *cfg.TeamSettings.EnableOpenServer = false })
 
-		inviteId := th.BasicTeam.InviteId
+		team, res := th.SystemAdminClient.RegenerateTeamInviteId(th.BasicTeam.Id)
+		assert.Nil(t, res.Error)
+		inviteId := team.InviteId
 
 		ruser, resp := th.Client.CreateUserWithInviteId(&user, inviteId)
 		CheckNoError(t, resp)

app/team.go

@@ -23,6 +23,7 @@ import (
 )
 
 func (a *App) CreateTeam(team *model.Team) (*model.Team, *model.AppError) {
+	team.InviteId = ""
 	result := <-a.Srv.Store.Team().Save(team)
 	if result.Err != nil {
 		return nil, result.Err
@@ -118,7 +119,6 @@ func (a *App) UpdateTeam(team *model.Team) (*model.Team, *model.AppError) {
 
 	oldTeam.DisplayName = team.DisplayName
 	oldTeam.Description = team.Description
-	oldTeam.InviteId = team.InviteId
 	oldTeam.AllowOpenInvite = team.AllowOpenInvite
 	oldTeam.CompanyName = team.CompanyName
 	oldTeam.AllowedDomains = team.AllowedDomains
@@ -202,6 +202,24 @@ func (a *App) PatchTeam(teamId string, patch *model.TeamPatch) (*model.Team, *mo
 	return updatedTeam, nil
 }
 
+func (a *App) RegenerateTeamInviteId(teamId string) (*model.Team, *model.AppError) {
+	team, err := a.GetTeam(teamId)
+	if err != nil {
+		return nil, err
+	}
+
+	team.InviteId = model.NewId()
+
+	updatedTeam, err := a.Srv.Store.Team().Update(team)
+	if err != nil {
+		return nil, err
+	}
+
+	a.sendTeamEvent(updatedTeam, model.WEBSOCKET_EVENT_UPDATE_TEAM)
+
+	return updatedTeam, nil
+}
+
 func (a *App) sendTeamEvent(team *model.Team, event string) {
 	sanitizedTeam := &model.Team{}
 	*sanitizedTeam = *team

i18n/en.json

@@ -5002,6 +5002,10 @@
     "id": "model.team.is_valid.id.app_error",
     "translation": "Invalid Id"
   },
+  {
+    "id": "model.team.is_valid.invite_id.app_error",
+    "translation": "Invalid invite id"
+  },
   {
     "id": "model.team.is_valid.name.app_error",
     "translation": "Invalid name"

model/client4.go

@@ -1577,6 +1577,16 @@ func (c *Client4) PatchTeam(teamId string, patch *TeamPatch) (*Team, *Response)
 	return TeamFromJson(r.Body), BuildResponse(r)
 }
 
+// RegenerateTeamInviteId requests a new invite ID to be generated.
+func (c *Client4) RegenerateTeamInviteId(teamId string) (*Team, *Response) {
+	r, err := c.DoApiPost(c.GetTeamRoute(teamId)+"/regenerate_invite_id", "")
+	if err != nil {
+		return nil, BuildErrorResponse(r, err)
+	}
+	defer closeBody(r)
+	return TeamFromJson(r.Body), BuildResponse(r)
+}
+
 // SoftDeleteTeam deletes the team softly (archive only, not permanent delete).
 func (c *Client4) SoftDeleteTeam(teamId string) (bool, *Response) {
 	r, err := c.DoApiDelete(c.GetTeamRoute(teamId))

model/team.go

@@ -49,7 +49,6 @@ type TeamPatch struct {
 	Description      *string `json:"description"`
 	CompanyName      *string `json:"company_name"`
 	AllowedDomains   *string `json:"allowed_domains"`
-	InviteId         *string `json:"invite_id"`
 	AllowOpenInvite  *bool   `json:"allow_open_invite"`
 	GroupConstrained *bool   `json:"group_constrained"`
 }
@@ -153,6 +152,10 @@ func (o *Team) IsValid() *AppError {
 		return NewAppError("Team.IsValid", "model.team.is_valid.description.app_error", nil, "id="+o.Id, http.StatusBadRequest)
 	}
 
+	if len(o.InviteId) == 0 {
+		return NewAppError("Team.IsValid", "model.team.is_valid.invite_id.app_error", nil, "id="+o.Id, http.StatusBadRequest)
+	}
+
 	if IsReservedTeamName(o.Name) {
 		return NewAppError("Team.IsValid", "model.team.is_valid.reserved.app_error", nil, "id="+o.Id, http.StatusBadRequest)
 	}
@@ -268,10 +271,6 @@ func (t *Team) Patch(patch *TeamPatch) {
 		t.AllowedDomains = *patch.AllowedDomains
 	}
 
-	if patch.InviteId != nil {
-		t.InviteId = *patch.InviteId
-	}
-
 	if patch.AllowOpenInvite != nil {
 		t.AllowOpenInvite = *patch.AllowOpenInvite
 	}

model/team_test.go

@@ -59,6 +59,7 @@ func TestTeamIsValid(t *testing.T) {
 
 	o.Name = "zzzzz"
 	o.Type = TEAM_OPEN
+	o.InviteId = NewId()
 	if err := o.IsValid(); err != nil {
 		t.Fatal(err)
 	}
@@ -137,7 +138,6 @@ func TestTeamPatch(t *testing.T) {
 		Description:      new(string),
 		CompanyName:      new(string),
 		AllowedDomains:   new(string),
-		InviteId:         new(string),
 		AllowOpenInvite:  new(bool),
 		GroupConstrained: new(bool),
 	}
@@ -146,7 +146,6 @@ func TestTeamPatch(t *testing.T) {
 	*p.Description = NewId()
 	*p.CompanyName = NewId()
 	*p.AllowedDomains = NewId()
-	*p.InviteId = NewId()
 	*p.AllowOpenInvite = true
 	*p.GroupConstrained = true
 
@@ -165,9 +164,6 @@ func TestTeamPatch(t *testing.T) {
 	if *p.AllowedDomains != o.AllowedDomains {
 		t.Fatal("AllowedDomains did not update")
 	}
-	if *p.InviteId != o.InviteId {
-		t.Fatal("InviteId did not update")
-	}
 	if *p.AllowOpenInvite != o.AllowOpenInvite {
 		t.Fatal("AllowOpenInvite did not update")
 	}

store/sqlstore/team_store.go

@@ -251,9 +251,6 @@ func (s SqlTeamStore) Get(id string) store.StoreChannel {
 		}
 
 		team := obj.(*model.Team)
-		if len(team.InviteId) == 0 {
-			team.InviteId = team.Id
-		}
 
 		result.Data = team
 	})
@@ -263,15 +260,11 @@ func (s SqlTeamStore) GetByInviteId(inviteId string) store.StoreChannel {
 	return store.Do(func(result *store.StoreResult) {
 		team := model.Team{}
 
-		if err := s.GetReplica().SelectOne(&team, "SELECT * FROM Teams WHERE Id = :InviteId OR InviteId = :InviteId", map[string]interface{}{"InviteId": inviteId}); err != nil {
+		if err := s.GetReplica().SelectOne(&team, "SELECT * FROM Teams WHERE InviteId = :InviteId", map[string]interface{}{"InviteId": inviteId}); err != nil {
 			result.Err = model.NewAppError("SqlTeamStore.GetByInviteId", "store.sql_team.get_by_invite_id.finding.app_error", nil, "inviteId="+inviteId+", "+err.Error(), http.StatusNotFound)
 			return
 		}
 
-		if len(team.InviteId) == 0 {
-			team.InviteId = team.Id
-		}
-
 		if len(inviteId) == 0 || team.InviteId != inviteId {
 			result.Err = model.NewAppError("SqlTeamStore.GetByInviteId", "store.sql_team.get_by_invite_id.find.app_error", nil, "inviteId="+inviteId, http.StatusNotFound)
 			return
@@ -290,10 +283,6 @@ func (s SqlTeamStore) GetByName(name string) store.StoreChannel {
 			return
 		}
 
-		if len(team.InviteId) == 0 {
-			team.InviteId = team.Id
-		}
-
 		result.Data = &team
 	})
 }
@@ -358,12 +347,6 @@ func (s SqlTeamStore) GetAll() store.StoreChannel {
 			return
 		}
 
-		for _, team := range data {
-			if len(team.InviteId) == 0 {
-				team.InviteId = team.Id
-			}
-		}
-
 		result.Data = data
 	})
 }
@@ -376,12 +359,6 @@ func (s SqlTeamStore) GetAllPage(offset int, limit int) store.StoreChannel {
 			return
 		}
 
-		for _, team := range data {
-			if len(team.InviteId) == 0 {
-				team.InviteId = team.Id
-			}
-		}
-
 		result.Data = data
 	})
 }
@@ -394,12 +371,6 @@ func (s SqlTeamStore) GetTeamsByUserId(userId string) store.StoreChannel {
 			return
 		}
 
-		for _, team := range data {
-			if len(team.InviteId) == 0 {
-				team.InviteId = team.Id
-			}
-		}
-
 		result.Data = data
 	})
 }
@@ -418,12 +389,6 @@ func (s SqlTeamStore) GetAllPrivateTeamListing() store.StoreChannel {
 			return
 		}
 
-		for _, team := range data {
-			if len(team.InviteId) == 0 {
-				team.InviteId = team.Id
-			}
-		}
-
 		result.Data = data
 	})
 }
@@ -442,12 +407,6 @@ func (s SqlTeamStore) GetAllPrivateTeamPageListing(offset int, limit int) store.
 			return
 		}
 
-		for _, team := range data {
-			if len(team.InviteId) == 0 {
-				team.InviteId = team.Id
-			}
-		}
-
 		result.Data = data
 	})
 }
@@ -466,12 +425,6 @@ func (s SqlTeamStore) GetAllTeamListing() store.StoreChannel {
 			return
 		}
 
-		for _, team := range data {
-			if len(team.InviteId) == 0 {
-				team.InviteId = team.Id
-			}
-		}
-
 		result.Data = data
 	})
 }
@@ -490,12 +443,6 @@ func (s SqlTeamStore) GetAllTeamPageListing(offset int, limit int) store.StoreCh
 			return
 		}
 
-		for _, team := range data {
-			if len(team.InviteId) == 0 {
-				team.InviteId = team.Id
-			}
-		}
-
 		result.Data = data
 	})
 }
@@ -999,12 +946,6 @@ func (s SqlTeamStore) GetAllForExportAfter(limit int, afterId string) store.Stor
 			return
 		}
 
-		for _, team := range data {
-			if len(team.InviteId) == 0 {
-				team.InviteId = team.Id
-			}
-		}
-
 		result.Data = data
 	})
 }

store/sqlstore/upgrade.go

@@ -61,8 +61,9 @@ const (
 )
 
 const (
-	EXIT_VERSION_SAVE    = 1003
+	EXIT_VERSION_SAVE                   = 1003
-	EXIT_THEME_MIGRATION = 1004
+	EXIT_THEME_MIGRATION                = 1004
+	EXIT_TEAM_INVITEID_MIGRATION_FAILED = 1006
 )
 
 // UpgradeDatabase attempts to migrate the schema to the latest supported version.
@@ -660,6 +661,19 @@ func UpgradeDatabaseToVersion511(sqlStore SqlStore) {
 	// TODO: Uncomment following condition when version 5.11.0 is released
 	// if shouldPerformUpgrade(sqlStore, VERSION_5_10_0, VERSION_5_11_0) {
 
+	// Enforce all teams have an InviteID set
+	var teams []*model.Team
+	if _, err := sqlStore.GetReplica().Select(&teams, "SELECT * FROM Teams WHERE InviteId = ''"); err != nil {
+		mlog.Error("Error fetching Teams without InviteID: " + err.Error())
+	} else {
+		for _, team := range teams {
+			team.InviteId = model.NewId()
+			if _, err := sqlStore.Team().Update(team); err != nil {
+				mlog.Error("Error updating Team InviteIDs: " + err.Error())
+			}
+		}
+	}
+
 	// 	saveSchemaVersion(sqlStore, VERSION_5_11_0)
 	// }
 }

store/storetest/team_store.go

@@ -393,7 +393,9 @@ func testTeamStoreGetByInviteId(t *testing.T, ss store.Store) {
 	o1.Type = model.TEAM_OPEN
 	o1.InviteId = model.NewId()
 
-	if err := (<-ss.Team().Save(&o1)).Err; err != nil {
+	save1 := <-ss.Team().Save(&o1)
+
+	if err := save1.Err; err != nil {
 		t.Fatal(err)
 	}
 
@@ -403,11 +405,7 @@ func testTeamStoreGetByInviteId(t *testing.T, ss store.Store) {
 	o2.Email = MakeEmail()
 	o2.Type = model.TEAM_OPEN
 
-	if err := (<-ss.Team().Save(&o2)).Err; err != nil {
+	if r1 := <-ss.Team().GetByInviteId(save1.Data.(*model.Team).InviteId); r1.Err != nil {
-		t.Fatal(err)
-	}
-
-	if r1 := <-ss.Team().GetByInviteId(o1.InviteId); r1.Err != nil {
 		t.Fatal(r1.Err)
 	} else {
 		if r1.Data.(*model.Team).ToJson() != o1.ToJson() {
@@ -415,18 +413,6 @@ func testTeamStoreGetByInviteId(t *testing.T, ss store.Store) {
 		}
 	}
 
-	o2.InviteId = ""
-	_, err := ss.Team().Update(&o2)
-	require.Nil(t, err)
-
-	if r1 := <-ss.Team().GetByInviteId(o2.Id); r1.Err != nil {
-		t.Fatal(r1.Err)
-	} else {
-		if r1.Data.(*model.Team).Id != o2.Id {
-			t.Fatal("invalid returned team")
-		}
-	}
-
 	if err := (<-ss.Team().GetByInviteId("")).Err; err == nil {
 		t.Fatal("Missing id should have failed")
 	}