From 0910eae31de8ed7b409654515dbd11f5c86dbf71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Espino?= Date: Wed, 18 Apr 2018 22:46:10 +0200 Subject: MM-9779: Incorporate a Token into the invitations system (#8604) * Incorporate a Token into the invitations system * Adding unit tests * Fixing some api4 client tests * Removing unnecesary hash validation * Change the Hash concept on invitations with tokenId * Not send invitation if it wasn't able to create the Token * Fixing some naming problems * Changing the hash query params received from the client side * Removed unneded data param in the token usage --- api/team.go | 7 ++- api/user.go | 6 +-- api/user_test.go | 16 ++++--- api4/team.go | 7 ++- api4/team_test.go | 56 ++++++++++++---------- api4/user.go | 6 +-- api4/user_test.go | 137 +++++++++++++++++++++++++----------------------------- app/email.go | 15 ++++-- app/team.go | 52 +++++++++++++-------- app/team_test.go | 78 +++++++++++++++++++++++++++++++ app/user.go | 31 ++++++++---- app/user_test.go | 70 ++++++++++++++++++++++++++++ i18n/en.json | 4 +- model/client.go | 20 ++++---- model/client4.go | 18 +++---- 15 files changed, 351 insertions(+), 172 deletions(-) diff --git a/api/team.go b/api/team.go index b1d8086d3..c3eaab128 100644 --- a/api/team.go +++ b/api/team.go @@ -182,15 +182,14 @@ func removeUserFromTeam(c *Context, w http.ResponseWriter, r *http.Request) { func addUserToTeamFromInvite(c *Context, w http.ResponseWriter, r *http.Request) { params := model.MapFromJson(r.Body) - hash := params["hash"] - data := params["data"] + tokenId := params["token"] inviteId := params["invite_id"] var team *model.Team var err *model.AppError - if len(hash) > 0 { - team, err = c.App.AddUserToTeamByHash(c.Session.UserId, hash, data) + if len(tokenId) > 0 { + team, err = c.App.AddUserToTeamByToken(c.Session.UserId, tokenId) } else if len(inviteId) > 0 { team, err = c.App.AddUserToTeamByInviteId(inviteId, c.Session.UserId) } else { diff --git a/api/user.go b/api/user.go index 560d722a4..35a3687b9 100644 --- a/api/user.go +++ b/api/user.go @@ -76,13 +76,13 @@ func createUser(c *Context, w http.ResponseWriter, r *http.Request) { return } - hash := r.URL.Query().Get("h") + tokenId := r.URL.Query().Get("t") inviteId := r.URL.Query().Get("iid") var ruser *model.User var err *model.AppError - if len(hash) > 0 { - ruser, err = c.App.CreateUserWithHash(user, hash, r.URL.Query().Get("d")) + if len(tokenId) > 0 { + ruser, err = c.App.CreateUserWithToken(user, tokenId) } else if len(inviteId) > 0 { ruser, err = c.App.CreateUserWithInviteId(user, inviteId) } else { diff --git a/api/user_test.go b/api/user_test.go index 518379305..05ec0e096 100644 --- a/api/user_test.go +++ b/api/user_test.go @@ -5,7 +5,6 @@ package api import ( "bytes" - "fmt" "image" "image/color" "io" @@ -176,21 +175,26 @@ func TestLogin(t *testing.T) { t.Fatal("Should have errored, signed up without hashed email") } + token := model.NewToken( + app.TOKEN_TYPE_TEAM_INVITATION, + model.MapToJson(map[string]string{"teamId": rteam2.Data.(*model.Team).Id, "email": user2.Email}), + ) + <-th.App.Srv.Store.Token().Save(token) props := make(map[string]string) props["email"] = user2.Email - props["id"] = rteam2.Data.(*model.Team).Id props["display_name"] = rteam2.Data.(*model.Team).DisplayName - props["time"] = fmt.Sprintf("%v", model.GetMillis()) data := model.MapToJson(props) - hash := utils.HashSha256(fmt.Sprintf("%v:%v", data, th.App.Config().EmailSettings.InviteSalt)) - ruser2, err := Client.CreateUserFromSignup(&user2, data, hash) + ruser2, err := Client.CreateUserFromSignup(&user2, data, token.Token) if err != nil { t.Fatal(err) } + if result := <-th.App.Srv.Store.Token().GetByToken(token.Token); result.Err == nil { + t.Fatal("The token must be deleted after be used") + } if _, err := Client.Login(ruser2.Data.(*model.User).Email, user2.Password); err != nil { - t.Fatal("From verified hash") + t.Fatal("From verified token") } Client.AuthToken = authToken diff --git a/api4/team.go b/api4/team.go index 33cd57fbb..94035a770 100644 --- a/api4/team.go +++ b/api4/team.go @@ -376,15 +376,14 @@ func addTeamMember(c *Context, w http.ResponseWriter, r *http.Request) { } func addUserToTeamFromInvite(c *Context, w http.ResponseWriter, r *http.Request) { - hash := r.URL.Query().Get("hash") - data := r.URL.Query().Get("data") + tokenId := r.URL.Query().Get("token") inviteId := r.URL.Query().Get("invite_id") var member *model.TeamMember var err *model.AppError - if len(hash) > 0 && len(data) > 0 { - member, err = c.App.AddTeamMemberByHash(c.Session.UserId, hash, data) + if len(tokenId) > 0 { + member, err = c.App.AddTeamMemberByToken(c.Session.UserId, tokenId) } else if len(inviteId) > 0 { member, err = c.App.AddTeamMemberByInviteId(inviteId, c.Session.UserId) } else { diff --git a/api4/team_test.go b/api4/team_test.go index 991dee148..cdf201771 100644 --- a/api4/team_test.go +++ b/api4/team_test.go @@ -13,6 +13,7 @@ import ( "encoding/base64" + "github.com/mattermost/mattermost-server/app" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/utils" "github.com/stretchr/testify/assert" @@ -1361,17 +1362,16 @@ func TestAddTeamMember(t *testing.T) { _, resp = Client.AddTeamMember(team.Id, otherUser.Id) CheckNoError(t, resp) - // by hash and data + // by token Client.Login(otherUser.Email, otherUser.Password) - dataObject := make(map[string]string) - dataObject["time"] = fmt.Sprintf("%v", model.GetMillis()) - dataObject["id"] = team.Id + token := model.NewToken( + app.TOKEN_TYPE_TEAM_INVITATION, + model.MapToJson(map[string]string{"teamId": team.Id}), + ) + <-th.App.Srv.Store.Token().Save(token) - data := model.MapToJson(dataObject) - hashed := utils.HashSha256(fmt.Sprintf("%v:%v", data, th.App.Config().EmailSettings.InviteSalt)) - - tm, resp = Client.AddTeamMemberFromInvite(hashed, data, "") + tm, resp = Client.AddTeamMemberFromInvite(token.Token, "") CheckNoError(t, resp) if tm == nil { @@ -1386,36 +1386,42 @@ func TestAddTeamMember(t *testing.T) { t.Fatal("team ids should have matched") } - tm, resp = Client.AddTeamMemberFromInvite("junk", data, "") + if result := <-th.App.Srv.Store.Token().GetByToken(token.Token); result.Err == nil { + t.Fatal("The token must be deleted after be used") + } + + tm, resp = Client.AddTeamMemberFromInvite("junk", "") CheckBadRequestStatus(t, resp) if tm != nil { t.Fatal("should have not returned team member") } - _, resp = Client.AddTeamMemberFromInvite(hashed, "junk", "") - CheckBadRequestStatus(t, resp) - - // expired data of more than 50 hours - dataObject["time"] = fmt.Sprintf("%v", model.GetMillis()-1000*60*60*50) - data = model.MapToJson(dataObject) - hashed = utils.HashSha256(fmt.Sprintf("%v:%v", data, th.App.Config().EmailSettings.InviteSalt)) + // expired token of more than 50 hours + token = model.NewToken(app.TOKEN_TYPE_TEAM_INVITATION, "") + token.CreateAt = model.GetMillis() - 1000*60*60*50 + <-th.App.Srv.Store.Token().Save(token) - tm, resp = Client.AddTeamMemberFromInvite(hashed, data, "") + tm, resp = Client.AddTeamMemberFromInvite(token.Token, "") CheckBadRequestStatus(t, resp) + th.App.DeleteToken(token) // invalid team id - dataObject["id"] = GenerateTestId() - data = model.MapToJson(dataObject) - hashed = utils.HashSha256(fmt.Sprintf("%v:%v", data, th.App.Config().EmailSettings.InviteSalt)) - - tm, resp = Client.AddTeamMemberFromInvite(hashed, data, "") - CheckBadRequestStatus(t, resp) + testId := GenerateTestId() + token = model.NewToken( + app.TOKEN_TYPE_TEAM_INVITATION, + model.MapToJson(map[string]string{"teamId": testId}), + ) + <-th.App.Srv.Store.Token().Save(token) + + tm, resp = Client.AddTeamMemberFromInvite(token.Token, "") + CheckNotFoundStatus(t, resp) + th.App.DeleteToken(token) // by invite_id Client.Login(otherUser.Email, otherUser.Password) - tm, resp = Client.AddTeamMemberFromInvite("", "", team.InviteId) + tm, resp = Client.AddTeamMemberFromInvite("", team.InviteId) CheckNoError(t, resp) if tm == nil { @@ -1430,7 +1436,7 @@ func TestAddTeamMember(t *testing.T) { t.Fatal("team ids should have matched") } - tm, resp = Client.AddTeamMemberFromInvite("", "", "junk") + tm, resp = Client.AddTeamMemberFromInvite("", "junk") CheckNotFoundStatus(t, resp) if tm != nil { diff --git a/api4/user.go b/api4/user.go index f5c56656c..e13bf9448 100644 --- a/api4/user.go +++ b/api4/user.go @@ -73,15 +73,15 @@ func createUser(c *Context, w http.ResponseWriter, r *http.Request) { return } - hash := r.URL.Query().Get("h") + tokenId := r.URL.Query().Get("t") inviteId := r.URL.Query().Get("iid") // No permission check required var ruser *model.User var err *model.AppError - if len(hash) > 0 { - ruser, err = c.App.CreateUserWithHash(user, hash, r.URL.Query().Get("d")) + if len(tokenId) > 0 { + ruser, err = c.App.CreateUserWithToken(user, tokenId) } else if len(inviteId) > 0 { ruser, err = c.App.CreateUserWithInviteId(user, inviteId) } else if c.IsSystemAdmin() { diff --git a/api4/user_test.go b/api4/user_test.go index 27219726b..5b4d8890a 100644 --- a/api4/user_test.go +++ b/api4/user_test.go @@ -4,15 +4,14 @@ package api4 import ( - "fmt" "net/http" "strconv" "testing" "time" + "github.com/mattermost/mattermost-server/app" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/store" - "github.com/mattermost/mattermost-server/utils" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -81,23 +80,20 @@ func TestCreateUser(t *testing.T) { } } -func TestCreateUserWithHash(t *testing.T) { +func TestCreateUserWithToken(t *testing.T) { th := Setup().InitBasic().InitSystemAdmin() defer th.TearDown() Client := th.Client - t.Run("CreateWithHashHappyPath", func(t *testing.T) { + t.Run("CreateWithTokenHappyPath", func(t *testing.T) { user := model.User{Email: th.GenerateTestEmail(), Nickname: "Corey Hulen", Password: "hello1", Username: GenerateTestUsername(), Roles: model.SYSTEM_ADMIN_ROLE_ID + " " + model.SYSTEM_USER_ROLE_ID} - props := make(map[string]string) - props["email"] = user.Email - props["id"] = th.BasicTeam.Id - props["display_name"] = th.BasicTeam.DisplayName - props["name"] = th.BasicTeam.Name - props["time"] = fmt.Sprintf("%v", model.GetMillis()) - data := model.MapToJson(props) - hash := utils.HashSha256(fmt.Sprintf("%v:%v", data, th.App.Config().EmailSettings.InviteSalt)) - - ruser, resp := Client.CreateUserWithHash(&user, hash, data) + token := model.NewToken( + app.TOKEN_TYPE_TEAM_INVITATION, + model.MapToJson(map[string]string{"teamId": th.BasicTeam.Id, "email": user.Email}), + ) + <-th.App.Srv.Store.Token().Save(token) + + ruser, resp := Client.CreateUserWithToken(&user, token.Token) CheckNoError(t, resp) CheckCreatedStatus(t, resp) @@ -110,78 +106,73 @@ func TestCreateUserWithHash(t *testing.T) { t.Fatal("did not clear roles") } CheckUserSanitization(t, ruser) + if result := <-th.App.Srv.Store.Token().GetByToken(token.Token); result.Err == nil { + t.Fatal("The token must be deleted after be used") + } + + if result := <-th.App.Srv.Store.Token().GetByToken(token.Token); result.Err == nil { + t.Fatal("The token must be deleted after be used") + } + + if teams, err := th.App.GetTeamsForUser(ruser.Id); err != nil || len(teams) == 0 { + t.Fatal("The user must have teams") + } else if teams[0].Id != th.BasicTeam.Id { + t.Fatal("The user joined team must be the team provided.") + } }) - t.Run("NoHashAndNoData", func(t *testing.T) { + t.Run("NoToken", func(t *testing.T) { user := model.User{Email: th.GenerateTestEmail(), Nickname: "Corey Hulen", Password: "hello1", Username: GenerateTestUsername(), Roles: model.SYSTEM_ADMIN_ROLE_ID + " " + model.SYSTEM_USER_ROLE_ID} - props := make(map[string]string) - props["email"] = user.Email - props["id"] = th.BasicTeam.Id - props["display_name"] = th.BasicTeam.DisplayName - props["name"] = th.BasicTeam.Name - props["time"] = fmt.Sprintf("%v", model.GetMillis()) - data := model.MapToJson(props) - hash := utils.HashSha256(fmt.Sprintf("%v:%v", data, th.App.Config().EmailSettings.InviteSalt)) - - _, resp := Client.CreateUserWithHash(&user, "", data) + token := model.NewToken( + app.TOKEN_TYPE_TEAM_INVITATION, + model.MapToJson(map[string]string{"teamId": th.BasicTeam.Id, "email": user.Email}), + ) + <-th.App.Srv.Store.Token().Save(token) + defer th.App.DeleteToken(token) + + _, resp := Client.CreateUserWithToken(&user, "") CheckBadRequestStatus(t, resp) - CheckErrorMessage(t, resp, "api.user.create_user.missing_hash_or_data.app_error") - - _, resp = Client.CreateUserWithHash(&user, hash, "") - CheckBadRequestStatus(t, resp) - CheckErrorMessage(t, resp, "api.user.create_user.missing_hash_or_data.app_error") + CheckErrorMessage(t, resp, "api.user.create_user.missing_token.app_error") }) - t.Run("HashExpired", func(t *testing.T) { + t.Run("TokenExpired", func(t *testing.T) { user := model.User{Email: th.GenerateTestEmail(), Nickname: "Corey Hulen", Password: "hello1", Username: GenerateTestUsername(), Roles: model.SYSTEM_ADMIN_ROLE_ID + " " + model.SYSTEM_USER_ROLE_ID} timeNow := time.Now() past49Hours := timeNow.Add(-49*time.Hour).UnixNano() / int64(time.Millisecond) - - props := make(map[string]string) - props["email"] = user.Email - props["id"] = th.BasicTeam.Id - props["display_name"] = th.BasicTeam.DisplayName - props["name"] = th.BasicTeam.Name - props["time"] = fmt.Sprintf("%v", past49Hours) - data := model.MapToJson(props) - hash := utils.HashSha256(fmt.Sprintf("%v:%v", data, th.App.Config().EmailSettings.InviteSalt)) - - _, resp := Client.CreateUserWithHash(&user, hash, data) - CheckInternalErrorStatus(t, resp) + token := model.NewToken( + app.TOKEN_TYPE_TEAM_INVITATION, + model.MapToJson(map[string]string{"teamId": th.BasicTeam.Id, "email": user.Email}), + ) + token.CreateAt = past49Hours + <-th.App.Srv.Store.Token().Save(token) + defer th.App.DeleteToken(token) + + _, resp := Client.CreateUserWithToken(&user, token.Token) + CheckBadRequestStatus(t, resp) CheckErrorMessage(t, resp, "api.user.create_user.signup_link_expired.app_error") }) - t.Run("WrongHash", func(t *testing.T) { + t.Run("WrongToken", func(t *testing.T) { user := model.User{Email: th.GenerateTestEmail(), Nickname: "Corey Hulen", Password: "hello1", Username: GenerateTestUsername(), Roles: model.SYSTEM_ADMIN_ROLE_ID + " " + model.SYSTEM_USER_ROLE_ID} - props := make(map[string]string) - props["email"] = user.Email - props["id"] = th.BasicTeam.Id - props["display_name"] = th.BasicTeam.DisplayName - props["name"] = th.BasicTeam.Name - props["time"] = fmt.Sprintf("%v", model.GetMillis()) - data := model.MapToJson(props) - hash := utils.HashSha256(fmt.Sprintf("%v:%v", data, "WrongHash")) - - _, resp := Client.CreateUserWithHash(&user, hash, data) - CheckInternalErrorStatus(t, resp) + + _, resp := Client.CreateUserWithToken(&user, "wrong") + CheckBadRequestStatus(t, resp) CheckErrorMessage(t, resp, "api.user.create_user.signup_link_invalid.app_error") }) t.Run("EnableUserCreationDisable", func(t *testing.T) { user := model.User{Email: th.GenerateTestEmail(), Nickname: "Corey Hulen", Password: "hello1", Username: GenerateTestUsername(), Roles: model.SYSTEM_ADMIN_ROLE_ID + " " + model.SYSTEM_USER_ROLE_ID} - props := make(map[string]string) - props["email"] = user.Email - props["id"] = th.BasicTeam.Id - props["display_name"] = th.BasicTeam.DisplayName - props["name"] = th.BasicTeam.Name - props["time"] = fmt.Sprintf("%v", model.GetMillis()) - data := model.MapToJson(props) - hash := utils.HashSha256(fmt.Sprintf("%v:%v", data, th.App.Config().EmailSettings.InviteSalt)) + token := model.NewToken( + app.TOKEN_TYPE_TEAM_INVITATION, + model.MapToJson(map[string]string{"teamId": th.BasicTeam.Id, "email": user.Email}), + ) + <-th.App.Srv.Store.Token().Save(token) + defer th.App.DeleteToken(token) th.App.UpdateConfig(func(cfg *model.Config) { cfg.TeamSettings.EnableUserCreation = false }) - _, resp := Client.CreateUserWithHash(&user, hash, data) + _, resp := Client.CreateUserWithToken(&user, token.Token) CheckNotImplementedStatus(t, resp) CheckErrorMessage(t, resp, "api.user.create_user.signup_email_disabled.app_error") @@ -191,18 +182,15 @@ func TestCreateUserWithHash(t *testing.T) { t.Run("EnableOpenServerDisable", func(t *testing.T) { user := model.User{Email: th.GenerateTestEmail(), Nickname: "Corey Hulen", Password: "hello1", Username: GenerateTestUsername(), Roles: model.SYSTEM_ADMIN_ROLE_ID + " " + model.SYSTEM_USER_ROLE_ID} - props := make(map[string]string) - props["email"] = user.Email - props["id"] = th.BasicTeam.Id - props["display_name"] = th.BasicTeam.DisplayName - props["name"] = th.BasicTeam.Name - props["time"] = fmt.Sprintf("%v", model.GetMillis()) - data := model.MapToJson(props) - hash := utils.HashSha256(fmt.Sprintf("%v:%v", data, th.App.Config().EmailSettings.InviteSalt)) + token := model.NewToken( + app.TOKEN_TYPE_TEAM_INVITATION, + model.MapToJson(map[string]string{"teamId": th.BasicTeam.Id, "email": user.Email}), + ) + <-th.App.Srv.Store.Token().Save(token) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.TeamSettings.EnableOpenServer = false }) - ruser, resp := Client.CreateUserWithHash(&user, hash, data) + ruser, resp := Client.CreateUserWithToken(&user, token.Token) CheckNoError(t, resp) CheckCreatedStatus(t, resp) @@ -215,6 +203,9 @@ func TestCreateUserWithHash(t *testing.T) { t.Fatal("did not clear roles") } CheckUserSanitization(t, ruser) + if result := <-th.App.Srv.Store.Token().GetByToken(token.Token); result.Err == nil { + t.Fatal("The token must be deleted after be used") + } }) } diff --git a/app/email.go b/app/email.go index 7676dfe13..7a50fd82a 100644 --- a/app/email.go +++ b/app/email.go @@ -270,15 +270,22 @@ func (a *App) SendInviteEmails(team *model.Team, senderName string, invites []st bodyPage.Html["ExtraInfo"] = utils.TranslateAsHtml(utils.T, "api.templates.invite_body.extra_info", map[string]interface{}{"TeamDisplayName": team.DisplayName, "TeamURL": siteURL + "/" + team.Name}) + token := model.NewToken( + TOKEN_TYPE_TEAM_INVITATION, + model.MapToJson(map[string]string{"teamId": team.Id, "email": invite}), + ) + props := make(map[string]string) props["email"] = invite - props["id"] = team.Id props["display_name"] = team.DisplayName props["name"] = team.Name - props["time"] = fmt.Sprintf("%v", model.GetMillis()) data := model.MapToJson(props) - hash := utils.HashSha256(fmt.Sprintf("%v:%v", data, a.Config().EmailSettings.InviteSalt)) - bodyPage.Props["Link"] = fmt.Sprintf("%s/signup_user_complete/?d=%s&h=%s", siteURL, url.QueryEscape(data), url.QueryEscape(hash)) + + if result := <-a.Srv.Store.Token().Save(token); result.Err != nil { + l4g.Error(utils.T("api.team.invite_members.send.error"), result.Err) + continue + } + bodyPage.Props["Link"] = fmt.Sprintf("%s/signup_user_complete/?d=%s&t=%s", siteURL, url.QueryEscape(data), url.QueryEscape(token.Token)) if !a.Config().EmailSettings.SendEmailNotifications { l4g.Info(utils.T("api.team.invite_members.sending.info"), invite, bodyPage.Props["Link"]) diff --git a/app/team.go b/app/team.go index de71ed796..47e28f2ed 100644 --- a/app/team.go +++ b/app/team.go @@ -11,7 +11,6 @@ import ( "mime/multipart" "net/http" "net/url" - "strconv" "strings" l4g "github.com/alecthomas/log4go" @@ -216,19 +215,25 @@ func (a *App) AddUserToTeamByTeamId(teamId string, user *model.User) *model.AppE } } -func (a *App) AddUserToTeamByHash(userId string, hash string, data string) (*model.Team, *model.AppError) { - props := model.MapFromJson(strings.NewReader(data)) +func (a *App) AddUserToTeamByToken(userId string, tokenId string) (*model.Team, *model.AppError) { + result := <-a.Srv.Store.Token().GetByToken(tokenId) + if result.Err != nil { + return nil, model.NewAppError("AddUserToTeamByToken", "api.user.create_user.signup_link_invalid.app_error", nil, result.Err.Error(), http.StatusBadRequest) + } - if hash != utils.HashSha256(fmt.Sprintf("%v:%v", data, a.Config().EmailSettings.InviteSalt)) { - return nil, model.NewAppError("JoinUserToTeamByHash", "api.user.create_user.signup_link_invalid.app_error", nil, "", http.StatusBadRequest) + token := result.Data.(*model.Token) + if token.Type != TOKEN_TYPE_TEAM_INVITATION { + return nil, model.NewAppError("AddUserToTeamByToken", "api.user.create_user.signup_link_invalid.app_error", nil, "", http.StatusBadRequest) } - t, timeErr := strconv.ParseInt(props["time"], 10, 64) - if timeErr != nil || model.GetMillis()-t > 1000*60*60*48 { // 48 hours - return nil, model.NewAppError("JoinUserToTeamByHash", "api.user.create_user.signup_link_expired.app_error", nil, "", http.StatusBadRequest) + if model.GetMillis()-token.CreateAt >= TEAM_INVITATION_EXPIRY_TIME { + a.DeleteToken(token) + return nil, model.NewAppError("AddUserToTeamByToken", "api.user.create_user.signup_link_expired.app_error", nil, "", http.StatusBadRequest) } - tchan := a.Srv.Store.Team().Get(props["id"]) + tokenData := model.MapFromJson(strings.NewReader(token.Extra)) + + tchan := a.Srv.Store.Team().Get(tokenData["teamId"]) uchan := a.Srv.Store.User().Get(userId) var team *model.Team @@ -249,6 +254,10 @@ func (a *App) AddUserToTeamByHash(userId string, hash string, data string) (*mod return nil, err } + if err := a.DeleteToken(token); err != nil { + return nil, err + } + return team, nil } @@ -510,11 +519,11 @@ func (a *App) AddTeamMembers(teamId string, userIds []string, userRequestorId st return members, nil } -func (a *App) AddTeamMemberByHash(userId, hash, data string) (*model.TeamMember, *model.AppError) { +func (a *App) AddTeamMemberByToken(userId, tokenId string) (*model.TeamMember, *model.AppError) { var team *model.Team var err *model.AppError - if team, err = a.AddUserToTeamByHash(userId, hash, data); err != nil { + if team, err = a.AddUserToTeamByToken(userId, tokenId); err != nil { return nil, err } @@ -874,23 +883,28 @@ func (a *App) GetTeamStats(teamId string) (*model.TeamStats, *model.AppError) { } func (a *App) GetTeamIdFromQuery(query url.Values) (string, *model.AppError) { - hash := query.Get("h") + tokenId := query.Get("t") inviteId := query.Get("id") - if len(hash) > 0 { - data := query.Get("d") - props := model.MapFromJson(strings.NewReader(data)) + if len(tokenId) > 0 { + result := <-a.Srv.Store.Token().GetByToken(tokenId) + if result.Err != nil { + return "", model.NewAppError("GetTeamIdFromQuery", "api.oauth.singup_with_oauth.invalid_link.app_error", nil, "", http.StatusBadRequest) + } - if hash != utils.HashSha256(fmt.Sprintf("%v:%v", data, a.Config().EmailSettings.InviteSalt)) { + token := result.Data.(*model.Token) + if token.Type != TOKEN_TYPE_TEAM_INVITATION { return "", model.NewAppError("GetTeamIdFromQuery", "api.oauth.singup_with_oauth.invalid_link.app_error", nil, "", http.StatusBadRequest) } - t, err := strconv.ParseInt(props["time"], 10, 64) - if err != nil || model.GetMillis()-t > 1000*60*60*48 { // 48 hours + if model.GetMillis()-token.CreateAt >= TEAM_INVITATION_EXPIRY_TIME { + a.DeleteToken(token) return "", model.NewAppError("GetTeamIdFromQuery", "api.oauth.singup_with_oauth.expired_link.app_error", nil, "", http.StatusBadRequest) } - return props["id"], nil + tokenData := model.MapFromJson(strings.NewReader(token.Extra)) + + return tokenData["teamId"], nil } else if len(inviteId) > 0 { if result := <-a.Srv.Store.Team().GetByInviteId(inviteId); result.Err != nil { // soft fail, so we still create user but don't auto-join team diff --git a/app/team_test.go b/app/team_test.go index 95f4b83d6..7ebfb8166 100644 --- a/app/team_test.go +++ b/app/team_test.go @@ -105,6 +105,84 @@ func TestAddUserToTeam(t *testing.T) { } } +func TestAddUserToTeamByToken(t *testing.T) { + th := Setup().InitBasic() + defer th.TearDown() + + user := model.User{Email: strings.ToLower(model.NewId()) + "success+test@example.com", Nickname: "Darth Vader", Username: "vader" + model.NewId(), Password: "passwd1", AuthService: ""} + ruser, _ := th.App.CreateUser(&user) + + t.Run("invalid token", func(t *testing.T) { + if _, err := th.App.AddUserToTeamByToken(ruser.Id, "123"); err == nil { + t.Fatal("Should fail on unexisting token") + } + }) + + t.Run("invalid token type", func(t *testing.T) { + token := model.NewToken( + TOKEN_TYPE_VERIFY_EMAIL, + model.MapToJson(map[string]string{"teamId": th.BasicTeam.Id}), + ) + <-th.App.Srv.Store.Token().Save(token) + defer th.App.DeleteToken(token) + if _, err := th.App.AddUserToTeamByToken(ruser.Id, token.Token); err == nil { + t.Fatal("Should fail on bad token type") + } + }) + + t.Run("expired token", func(t *testing.T) { + token := model.NewToken( + TOKEN_TYPE_TEAM_INVITATION, + model.MapToJson(map[string]string{"teamId": th.BasicTeam.Id}), + ) + token.CreateAt = model.GetMillis() - TEAM_INVITATION_EXPIRY_TIME - 1 + <-th.App.Srv.Store.Token().Save(token) + defer th.App.DeleteToken(token) + if _, err := th.App.AddUserToTeamByToken(ruser.Id, token.Token); err == nil { + t.Fatal("Should fail on expired token") + } + }) + + t.Run("invalid team id", func(t *testing.T) { + token := model.NewToken( + TOKEN_TYPE_TEAM_INVITATION, + model.MapToJson(map[string]string{"teamId": model.NewId()}), + ) + <-th.App.Srv.Store.Token().Save(token) + defer th.App.DeleteToken(token) + if _, err := th.App.AddUserToTeamByToken(ruser.Id, token.Token); err == nil { + t.Fatal("Should fail on bad team id") + } + }) + + t.Run("invalid user id", func(t *testing.T) { + token := model.NewToken( + TOKEN_TYPE_TEAM_INVITATION, + model.MapToJson(map[string]string{"teamId": th.BasicTeam.Id}), + ) + <-th.App.Srv.Store.Token().Save(token) + defer th.App.DeleteToken(token) + if _, err := th.App.AddUserToTeamByToken(model.NewId(), token.Token); err == nil { + t.Fatal("Should fail on bad user id") + } + }) + + t.Run("valid request", func(t *testing.T) { + token := model.NewToken( + TOKEN_TYPE_TEAM_INVITATION, + model.MapToJson(map[string]string{"teamId": th.BasicTeam.Id}), + ) + <-th.App.Srv.Store.Token().Save(token) + if _, err := th.App.AddUserToTeamByToken(ruser.Id, token.Token); err != nil { + t.Log(err) + t.Fatal("Should add user to the team") + } + if result := <-th.App.Srv.Store.Token().GetByToken(token.Token); result.Err == nil { + t.Fatal("The token must be deleted after be used") + } + }) +} + func TestAddUserToTeamByTeamId(t *testing.T) { th := Setup().InitBasic() defer th.TearDown() diff --git a/app/user.go b/app/user.go index 21165fdba..80c8b6ef2 100644 --- a/app/user.go +++ b/app/user.go @@ -34,35 +34,42 @@ import ( const ( TOKEN_TYPE_PASSWORD_RECOVERY = "password_recovery" TOKEN_TYPE_VERIFY_EMAIL = "verify_email" - PASSWORD_RECOVER_EXPIRY_TIME = 1000 * 60 * 60 // 1 hour + TOKEN_TYPE_TEAM_INVITATION = "team_invitation" + PASSWORD_RECOVER_EXPIRY_TIME = 1000 * 60 * 60 // 1 hour + TEAM_INVITATION_EXPIRY_TIME = 1000 * 60 * 60 * 48 // 48 hours IMAGE_PROFILE_PIXEL_DIMENSION = 128 ) -func (a *App) CreateUserWithHash(user *model.User, hash string, data string) (*model.User, *model.AppError) { +func (a *App) CreateUserWithToken(user *model.User, tokenId string) (*model.User, *model.AppError) { if err := a.IsUserSignUpAllowed(); err != nil { return nil, err } - props := model.MapFromJson(strings.NewReader(data)) + result := <-a.Srv.Store.Token().GetByToken(tokenId) + if result.Err != nil { + return nil, model.NewAppError("CreateUserWithToken", "api.user.create_user.signup_link_invalid.app_error", nil, result.Err.Error(), http.StatusBadRequest) + } - if hash != utils.HashSha256(fmt.Sprintf("%v:%v", data, a.Config().EmailSettings.InviteSalt)) { - return nil, model.NewAppError("CreateUserWithHash", "api.user.create_user.signup_link_invalid.app_error", nil, "", http.StatusInternalServerError) + token := result.Data.(*model.Token) + if token.Type != TOKEN_TYPE_TEAM_INVITATION { + return nil, model.NewAppError("CreateUserWithToken", "api.user.create_user.signup_link_invalid.app_error", nil, "", http.StatusBadRequest) } - if t, err := strconv.ParseInt(props["time"], 10, 64); err != nil || model.GetMillis()-t > 1000*60*60*48 { // 48 hours - return nil, model.NewAppError("CreateUserWithHash", "api.user.create_user.signup_link_expired.app_error", nil, "", http.StatusInternalServerError) + if model.GetMillis()-token.CreateAt >= TEAM_INVITATION_EXPIRY_TIME { + a.DeleteToken(token) + return nil, model.NewAppError("CreateUserWithToken", "api.user.create_user.signup_link_expired.app_error", nil, "", http.StatusBadRequest) } - teamId := props["id"] + tokenData := model.MapFromJson(strings.NewReader(token.Extra)) var team *model.Team - if result := <-a.Srv.Store.Team().Get(teamId); result.Err != nil { + if result := <-a.Srv.Store.Team().Get(tokenData["teamId"]); result.Err != nil { return nil, result.Err } else { team = result.Data.(*model.Team) } - user.Email = props["email"] + user.Email = tokenData["email"] user.EmailVerified = true var ruser *model.User @@ -77,6 +84,10 @@ func (a *App) CreateUserWithHash(user *model.User, hash string, data string) (*m a.AddDirectChannels(team.Id, ruser) + if err := a.DeleteToken(token); err != nil { + return nil, err + } + return ruser, nil } diff --git a/app/user_test.go b/app/user_test.go index 94052da61..20dafd826 100644 --- a/app/user_test.go +++ b/app/user_test.go @@ -428,3 +428,73 @@ func TestGetUsersByStatus(t *testing.T) { } }) } + +func TestCreateUserWithToken(t *testing.T) { + th := Setup().InitBasic() + defer th.TearDown() + + user := model.User{Email: strings.ToLower(model.NewId()) + "success+test@example.com", Nickname: "Darth Vader", Username: "vader" + model.NewId(), Password: "passwd1", AuthService: ""} + + t.Run("invalid token", func(t *testing.T) { + if _, err := th.App.CreateUserWithToken(&user, "123"); err == nil { + t.Fatal("Should fail on unexisting token") + } + }) + + t.Run("invalid token type", func(t *testing.T) { + token := model.NewToken( + TOKEN_TYPE_VERIFY_EMAIL, + model.MapToJson(map[string]string{"teamId": th.BasicTeam.Id, "email": user.Email}), + ) + <-th.App.Srv.Store.Token().Save(token) + defer th.App.DeleteToken(token) + if _, err := th.App.CreateUserWithToken(&user, token.Token); err == nil { + t.Fatal("Should fail on bad token type") + } + }) + + t.Run("expired token", func(t *testing.T) { + token := model.NewToken( + TOKEN_TYPE_TEAM_INVITATION, + model.MapToJson(map[string]string{"teamId": th.BasicTeam.Id, "email": user.Email}), + ) + token.CreateAt = model.GetMillis() - TEAM_INVITATION_EXPIRY_TIME - 1 + <-th.App.Srv.Store.Token().Save(token) + defer th.App.DeleteToken(token) + if _, err := th.App.CreateUserWithToken(&user, token.Token); err == nil { + t.Fatal("Should fail on expired token") + } + }) + + t.Run("invalid team id", func(t *testing.T) { + token := model.NewToken( + TOKEN_TYPE_TEAM_INVITATION, + model.MapToJson(map[string]string{"teamId": model.NewId(), "email": user.Email}), + ) + <-th.App.Srv.Store.Token().Save(token) + defer th.App.DeleteToken(token) + if _, err := th.App.CreateUserWithToken(&user, token.Token); err == nil { + t.Fatal("Should fail on bad team id") + } + }) + + t.Run("valid request", func(t *testing.T) { + invitationEmail := model.NewId() + "other-email@test.com" + token := model.NewToken( + TOKEN_TYPE_TEAM_INVITATION, + model.MapToJson(map[string]string{"teamId": th.BasicTeam.Id, "email": invitationEmail}), + ) + <-th.App.Srv.Store.Token().Save(token) + newUser, err := th.App.CreateUserWithToken(&user, token.Token) + if err != nil { + t.Log(err) + t.Fatal("Should add user to the team") + } + if newUser.Email != invitationEmail { + t.Fatal("The user email must be the invitation one") + } + if result := <-th.App.Srv.Store.Token().GetByToken(token.Token); result.Err == nil { + t.Fatal("The token must be deleted after be used") + } + }) +} diff --git a/i18n/en.json b/i18n/en.json index d71ad8009..ea6314107 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -2851,8 +2851,8 @@ "translation": "Encountered an issue joining default channels user_id=%s, team_id=%s, err=%v" }, { - "id": "api.user.create_user.missing_hash_or_data.app_error", - "translation": "Missing Hash or URL query data." + "id": "api.user.create_user.missing_token.app_error", + "translation": "Missing Token." }, { "id": "api.user.create_user.missing_invite_id.app_error", diff --git a/model/client.go b/model/client.go index b56043a92..b48d1ac37 100644 --- a/model/client.go +++ b/model/client.go @@ -377,11 +377,11 @@ func (c *Client) AddUserToTeam(teamId string, userId string) (*Result, *AppError } // AddUserToTeamFromInvite adds a user to a team based off data provided in an invite link. -// Either hash and dataToHash are required or inviteId is required. -func (c *Client) AddUserToTeamFromInvite(hash, dataToHash, inviteId string) (*Result, *AppError) { +// Either token and data are required or inviteId is required. +func (c *Client) AddUserToTeamFromInvite(token, inviteData, inviteId string) (*Result, *AppError) { data := make(map[string]string) - data["hash"] = hash - data["data"] = dataToHash + data["token"] = token + data["data"] = inviteData data["invite_id"] = inviteId if r, err := c.DoApiPost("/teams/add_user_to_team_from_invite", MapToJson(data)); err != nil { return nil, err @@ -438,7 +438,7 @@ func (c *Client) UpdateTeam(team *Team) (*Result, *AppError) { // User Routes Section // CreateUser creates a user in the system based on the provided user struct. -func (c *Client) CreateUser(user *User, hash string) (*Result, *AppError) { +func (c *Client) CreateUser(user *User, token string) (*Result, *AppError) { if r, err := c.DoApiPost("/users/create", user.ToJson()); err != nil { return nil, err } else { @@ -448,11 +448,11 @@ func (c *Client) CreateUser(user *User, hash string) (*Result, *AppError) { } } -// CreateUserWithInvite creates a user based on the provided user struct. Either the hash and +// CreateUserWithInvite creates a user based on the provided user struct. Either the token and // data strings or the inviteId is required from the invite. -func (c *Client) CreateUserWithInvite(user *User, hash string, data string, inviteId string) (*Result, *AppError) { +func (c *Client) CreateUserWithInvite(user *User, token string, data string, inviteId string) (*Result, *AppError) { - url := "/users/create?d=" + url.QueryEscape(data) + "&h=" + url.QueryEscape(hash) + "&iid=" + url.QueryEscape(inviteId) + url := "/users/create?d=" + url.QueryEscape(data) + "&t=" + url.QueryEscape(token) + "&iid=" + url.QueryEscape(inviteId) if r, err := c.DoApiPost(url, user.ToJson()); err != nil { return nil, err @@ -463,8 +463,8 @@ func (c *Client) CreateUserWithInvite(user *User, hash string, data string, invi } } -func (c *Client) CreateUserFromSignup(user *User, data string, hash string) (*Result, *AppError) { - if r, err := c.DoApiPost("/users/create?d="+url.QueryEscape(data)+"&h="+hash, user.ToJson()); err != nil { +func (c *Client) CreateUserFromSignup(user *User, data string, token string) (*Result, *AppError) { + if r, err := c.DoApiPost("/users/create?d="+url.QueryEscape(data)+"&t="+token, user.ToJson()); err != nil { return nil, err } else { defer closeBody(r) diff --git a/model/client4.go b/model/client4.go index f1feb1bfe..870e440e8 100644 --- a/model/client4.go +++ b/model/client4.go @@ -530,13 +530,13 @@ func (c *Client4) CreateUser(user *User) (*User, *Response) { } } -// CreateUserWithHash creates a user in the system based on the provided user struct and hash created. -func (c *Client4) CreateUserWithHash(user *User, hash, data string) (*User, *Response) { +// CreateUserWithToken creates a user in the system based on the provided tokenId. +func (c *Client4) CreateUserWithToken(user *User, tokenId string) (*User, *Response) { var query string - if hash != "" && data != "" { - query = fmt.Sprintf("?d=%v&h=%v", url.QueryEscape(data), hash) + if tokenId != "" { + query = fmt.Sprintf("?t=%v", tokenId) } else { - err := NewAppError("MissingHashOrData", "api.user.create_user.missing_hash_or_data.app_error", nil, "", http.StatusBadRequest) + err := NewAppError("MissingHashOrData", "api.user.create_user.missing_token.app_error", nil, "", http.StatusBadRequest) return nil, &Response{StatusCode: err.StatusCode, Error: err} } if r, err := c.DoApiPost(c.GetUsersRoute()+query, user.ToJson()); err != nil { @@ -1340,16 +1340,16 @@ func (c *Client4) AddTeamMember(teamId, userId string) (*TeamMember, *Response) } // AddTeamMemberFromInvite adds a user to a team and return a team member using an invite id -// or an invite hash/data pair. -func (c *Client4) AddTeamMemberFromInvite(hash, dataToHash, inviteId string) (*TeamMember, *Response) { +// or an invite token/data pair. +func (c *Client4) AddTeamMemberFromInvite(token, inviteId string) (*TeamMember, *Response) { var query string if inviteId != "" { query += fmt.Sprintf("?invite_id=%v", inviteId) } - if hash != "" && dataToHash != "" { - query += fmt.Sprintf("?hash=%v&data=%v", hash, dataToHash) + if token != "" { + query += fmt.Sprintf("?token=%v", token) } if r, err := c.DoApiPost(c.GetTeamsRoute()+"/members/invite"+query, ""); err != nil { -- cgit v1.2.3-1-g7c22