Commit a675f71b authored by Daniel Schalla's avatar Daniel Schalla

[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
parent 857e822c
......@@ -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 {
......
......@@ -285,8 +285,8 @@ func TestUpdateTeam(t *testing.T) {
uteam, resp = Client.UpdateTeam(team)
CheckNoError(t, resp)
if uteam.InviteId != "inviteid1" {
t.Fatal("Update failed")
if uteam.InviteId == "inviteid1" {
t.Fatal("InviteID should not be updated")
}
team.AllowedDomains = "domain"
......@@ -389,7 +389,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)
......@@ -404,8 +403,8 @@ func TestPatchTeam(t *testing.T) {
if rteam.CompanyName != "Other company name" {
t.Fatal("CompanyName did not update properly")
}
if rteam.InviteId != "inviteid1" {
t.Fatal("InviteId did not update properly")
if rteam.InviteId == "inviteid1" {
t.Fatal("InviteId should not update")
}
if !rteam.AllowOpenInvite {
t.Fatal("AllowOpenInvite did not update properly")
......@@ -471,6 +470,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()
......
......@@ -283,8 +283,7 @@ func TestCreateUserWithInviteId(t *testing.T) {
inviteId := th.BasicTeam.InviteId
th.BasicTeam.InviteId = model.NewId()
_, resp := th.SystemAdminClient.UpdateTeam(th.BasicTeam)
_, resp := th.SystemAdminClient.RegenerateTeamInviteId(th.BasicTeam.Id)
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)
......
......@@ -22,6 +22,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
......@@ -117,7 +118,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
......@@ -205,6 +205,25 @@ 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()
res := <-a.Srv.Store.Team().Update(team)
if res.Err != nil {
return nil, res.Err
}
updatedTeam := res.Data.(*model.Team)
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
......
......@@ -4870,6 +4870,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"
......
......@@ -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))
......
......@@ -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
}
......
......@@ -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")
}
......
......@@ -255,9 +255,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
})
......@@ -267,15 +264,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
......@@ -294,10 +287,6 @@ func (s SqlTeamStore) GetByName(name string) store.StoreChannel {
return
}
if len(team.InviteId) == 0 {
team.InviteId = team.Id
}
result.Data = &team
})
}
......@@ -362,12 +351,6 @@ func (s SqlTeamStore) GetAll() store.StoreChannel {
return
}
for _, team := range data {
if len(team.InviteId) == 0 {
team.InviteId = team.Id
}
}
result.Data = data
})
}
......@@ -380,12 +363,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
})
}
......@@ -398,12 +375,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
})
}
......@@ -422,12 +393,6 @@ func (s SqlTeamStore) GetAllPrivateTeamListing() store.StoreChannel {
return
}
for _, team := range data {
if len(team.InviteId) == 0 {
team.InviteId = team.Id
}
}
result.Data = data
})
}
......@@ -446,12 +411,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
})
}
......@@ -470,12 +429,6 @@ func (s SqlTeamStore) GetAllTeamListing() store.StoreChannel {
return
}
for _, team := range data {
if len(team.InviteId) == 0 {
team.InviteId = team.Id
}
}
result.Data = data
})
}
......@@ -494,12 +447,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
})
}
......@@ -1003,12 +950,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
})
}
......
......@@ -60,8 +60,9 @@ const (
)
const (
EXIT_VERSION_SAVE = 1003
EXIT_THEME_MIGRATION = 1004
EXIT_VERSION_SAVE = 1003
EXIT_THEME_MIGRATION = 1004
EXIT_TEAM_INVITEID_MIGRATION_FAILED = 1006
)
// UpgradeDatabase attempts to migrate the schema to the latest supported version.
......@@ -650,3 +651,24 @@ func UpgradeDatabaseToVersion510(sqlStore SqlStore) {
saveSchemaVersion(sqlStore, VERSION_5_10_0)
}
}
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 res := <-sqlStore.Team().Update(team); res.Err != nil {
mlog.Error("Error updating Team InviteIDs: " + res.Err.Error())
}
}
}
// saveSchemaVersion(sqlStore, VERSION_5_11_0)
// }
}
......@@ -393,7 +393,9 @@ func testTeamStoreGetByIniviteId(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 testTeamStoreGetByIniviteId(t *testing.T, ss store.Store) {
o2.Email = MakeEmail()
o2.Type = model.TEAM_OPEN
if err := (<-ss.Team().Save(&o2)).Err; err != nil {
t.Fatal(err)
}
if r1 := <-ss.Team().GetByInviteId(o1.InviteId); r1.Err != nil {
if r1 := <-ss.Team().GetByInviteId(save1.Data.(*model.Team).InviteId); r1.Err != nil {
t.Fatal(r1.Err)
} else {
if r1.Data.(*model.Team).ToJson() != o1.ToJson() {
......@@ -415,17 +413,6 @@ func testTeamStoreGetByIniviteId(t *testing.T, ss store.Store) {
}
}
o2.InviteId = ""
<-ss.Team().Update(&o2)
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")
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment