From 263e5e89458d2ceb0a000757fa7692f69919a7ca Mon Sep 17 00:00:00 2001 From: Ibrahim Serdar Acikgoz Date: Tue, 21 Jul 2020 17:09:49 +0300 Subject: [PATCH] api4/user: convey redirect link through email verification (#15052) --- api4/user.go | 10 ++++++---- app/app_iface.go | 8 ++++---- app/email.go | 10 ++++++++-- app/opentracing/opentracing_layer.go | 16 ++++++++-------- app/user.go | 18 +++++++++--------- app/user_test.go | 6 +++--- 6 files changed, 38 insertions(+), 30 deletions(-) diff --git a/api4/user.go b/api4/user.go index 2a1bc67a03..ecbdcf3a2a 100644 --- a/api4/user.go +++ b/api4/user.go @@ -98,6 +98,7 @@ func createUser(c *Context, w http.ResponseWriter, r *http.Request) { tokenId := r.URL.Query().Get("t") inviteId := r.URL.Query().Get("iid") + redirect := r.URL.Query().Get("r") auditRec := c.MakeAuditRecord("createUser", audit.Fail) defer c.LogAuditRec(auditRec) @@ -135,12 +136,12 @@ func createUser(c *Context, w http.ResponseWriter, r *http.Request) { } ruser, err = c.App.CreateUserWithToken(user, token) } else if len(inviteId) > 0 { - ruser, err = c.App.CreateUserWithInviteId(user, inviteId) + ruser, err = c.App.CreateUserWithInviteId(user, inviteId, redirect) } else if c.IsSystemAdmin() { - ruser, err = c.App.CreateUserAsAdmin(user) + ruser, err = c.App.CreateUserAsAdmin(user, redirect) auditRec.AddMeta("admin", true) } else { - ruser, err = c.App.CreateUserFromSignup(user) + ruser, err = c.App.CreateUserFromSignup(user, redirect) } if err != nil { @@ -1952,6 +1953,7 @@ func sendVerificationEmail(c *Context, w http.ResponseWriter, r *http.Request) { c.SetInvalidParam("email") return } + redirect := r.URL.Query().Get("r") auditRec := c.MakeAuditRecord("sendVerificationEmail", audit.Fail) defer c.LogAuditRec(auditRec) @@ -1965,7 +1967,7 @@ func sendVerificationEmail(c *Context, w http.ResponseWriter, r *http.Request) { } auditRec.AddMeta("user", user) - if err = c.App.SendEmailVerification(user, user.Email); err != nil { + if err = c.App.SendEmailVerification(user, user.Email, redirect); err != nil { // Don't want to leak whether the email is valid or not mlog.Error(err.Error()) ReturnStatusOK(w) diff --git a/app/app_iface.go b/app/app_iface.go index 33381a9823..574ba24a58 100644 --- a/app/app_iface.go +++ b/app/app_iface.go @@ -423,9 +423,9 @@ type AppIface interface { CreateTeamWithUser(team *model.Team, userId string) (*model.Team, *model.AppError) CreateTermsOfService(text, userId string) (*model.TermsOfService, *model.AppError) CreateUserAccessToken(token *model.UserAccessToken) (*model.UserAccessToken, *model.AppError) - CreateUserAsAdmin(user *model.User) (*model.User, *model.AppError) - CreateUserFromSignup(user *model.User) (*model.User, *model.AppError) - CreateUserWithInviteId(user *model.User, inviteId string) (*model.User, *model.AppError) + CreateUserAsAdmin(user *model.User, redirect string) (*model.User, *model.AppError) + CreateUserFromSignup(user *model.User, redirect string) (*model.User, *model.AppError) + CreateUserWithInviteId(user *model.User, inviteId, redirect string) (*model.User, *model.AppError) CreateUserWithToken(user *model.User, token *model.Token) (*model.User, *model.AppError) CreateWebhookPost(userId string, channel *model.Channel, text, overrideUsername, overrideIconUrl, overrideIconEmoji string, props model.StringInterface, postType string, postRootId string) (*model.Post, *model.AppError) DataRetention() einterfaces.DataRetentionInterface @@ -856,7 +856,7 @@ type AppIface interface { SendAckToPushProxy(ack *model.PushNotificationAck) error SendAutoResponse(channel *model.Channel, receiver *model.User) (bool, *model.AppError) SendAutoResponseIfNecessary(channel *model.Channel, sender *model.User) (bool, *model.AppError) - SendEmailVerification(user *model.User, newEmail string) *model.AppError + SendEmailVerification(user *model.User, newEmail, redirect string) *model.AppError SendEphemeralPost(userId string, post *model.Post) *model.Post SendNotifications(post *model.Post, team *model.Team, channel *model.Channel, sender *model.User, parentPostList *model.PostList, setOnline bool) ([]string, error) SendPasswordReset(email string, siteURL string) (bool, *model.AppError) diff --git a/app/email.go b/app/email.go index 327a8f453f..896b1f737a 100644 --- a/app/email.go +++ b/app/email.go @@ -141,10 +141,13 @@ func (es *EmailService) sendEmailChangeEmail(oldEmail, newEmail, locale, siteURL return nil } -func (es *EmailService) sendVerifyEmail(userEmail, locale, siteURL, token string) *model.AppError { +func (es *EmailService) sendVerifyEmail(userEmail, locale, siteURL, token, redirect string) *model.AppError { T := utils.GetUserTranslations(locale) link := fmt.Sprintf("%s/do_verify_email?token=%s&email=%s", siteURL, token, url.QueryEscape(userEmail)) + if redirect != "" { + link += fmt.Sprintf("&redirect_to=%s", redirect) + } serverURL := condenseSiteURL(siteURL) @@ -185,7 +188,7 @@ func (es *EmailService) SendSignInChangeEmail(email, method, locale, siteURL str return nil } -func (es *EmailService) sendWelcomeEmail(userId string, email string, verified bool, locale, siteURL string) *model.AppError { +func (es *EmailService) sendWelcomeEmail(userId string, email string, verified bool, locale, siteURL, redirect string) *model.AppError { if !*es.srv.Config().EmailSettings.SendEmailNotifications && !*es.srv.Config().EmailSettings.RequireEmailVerification { return model.NewAppError("SendWelcomeEmail", "api.user.send_welcome_email_and_forget.failed.error", nil, "Send Email Notifications and Require Email Verification is disabled in the system console", http.StatusInternalServerError) } @@ -218,6 +221,9 @@ func (es *EmailService) sendWelcomeEmail(userId string, email string, verified b return err } link := fmt.Sprintf("%s/do_verify_email?token=%s&email=%s", siteURL, token.Token, url.QueryEscape(email)) + if redirect != "" { + link += fmt.Sprintf("&redirect_to=%s", redirect) + } bodyPage.Props["VerifyUrl"] = link } diff --git a/app/opentracing/opentracing_layer.go b/app/opentracing/opentracing_layer.go index a7c96fc6a6..7c5c54f09d 100644 --- a/app/opentracing/opentracing_layer.go +++ b/app/opentracing/opentracing_layer.go @@ -2158,7 +2158,7 @@ func (a *OpenTracingAppLayer) CreateUserAccessToken(token *model.UserAccessToken return resultVar0, resultVar1 } -func (a *OpenTracingAppLayer) CreateUserAsAdmin(user *model.User) (*model.User, *model.AppError) { +func (a *OpenTracingAppLayer) CreateUserAsAdmin(user *model.User, redirect string) (*model.User, *model.AppError) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.CreateUserAsAdmin") @@ -2170,7 +2170,7 @@ func (a *OpenTracingAppLayer) CreateUserAsAdmin(user *model.User) (*model.User, }() defer span.Finish() - resultVar0, resultVar1 := a.app.CreateUserAsAdmin(user) + resultVar0, resultVar1 := a.app.CreateUserAsAdmin(user, redirect) if resultVar1 != nil { span.LogFields(spanlog.Error(resultVar1)) @@ -2180,7 +2180,7 @@ func (a *OpenTracingAppLayer) CreateUserAsAdmin(user *model.User) (*model.User, return resultVar0, resultVar1 } -func (a *OpenTracingAppLayer) CreateUserFromSignup(user *model.User) (*model.User, *model.AppError) { +func (a *OpenTracingAppLayer) CreateUserFromSignup(user *model.User, redirect string) (*model.User, *model.AppError) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.CreateUserFromSignup") @@ -2192,7 +2192,7 @@ func (a *OpenTracingAppLayer) CreateUserFromSignup(user *model.User) (*model.Use }() defer span.Finish() - resultVar0, resultVar1 := a.app.CreateUserFromSignup(user) + resultVar0, resultVar1 := a.app.CreateUserFromSignup(user, redirect) if resultVar1 != nil { span.LogFields(spanlog.Error(resultVar1)) @@ -2202,7 +2202,7 @@ func (a *OpenTracingAppLayer) CreateUserFromSignup(user *model.User) (*model.Use return resultVar0, resultVar1 } -func (a *OpenTracingAppLayer) CreateUserWithInviteId(user *model.User, inviteId string) (*model.User, *model.AppError) { +func (a *OpenTracingAppLayer) CreateUserWithInviteId(user *model.User, inviteId string, redirect string) (*model.User, *model.AppError) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.CreateUserWithInviteId") @@ -2214,7 +2214,7 @@ func (a *OpenTracingAppLayer) CreateUserWithInviteId(user *model.User, inviteId }() defer span.Finish() - resultVar0, resultVar1 := a.app.CreateUserWithInviteId(user, inviteId) + resultVar0, resultVar1 := a.app.CreateUserWithInviteId(user, inviteId, redirect) if resultVar1 != nil { span.LogFields(spanlog.Error(resultVar1)) @@ -12517,7 +12517,7 @@ func (a *OpenTracingAppLayer) SendAutoResponseIfNecessary(channel *model.Channel return resultVar0, resultVar1 } -func (a *OpenTracingAppLayer) SendEmailVerification(user *model.User, newEmail string) *model.AppError { +func (a *OpenTracingAppLayer) SendEmailVerification(user *model.User, newEmail string, redirect string) *model.AppError { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.SendEmailVerification") @@ -12529,7 +12529,7 @@ func (a *OpenTracingAppLayer) SendEmailVerification(user *model.User, newEmail s }() defer span.Finish() - resultVar0 := a.app.SendEmailVerification(user, newEmail) + resultVar0 := a.app.SendEmailVerification(user, newEmail, redirect) if resultVar0 != nil { span.LogFields(spanlog.Error(resultVar0)) diff --git a/app/user.go b/app/user.go index 9da2e1ce09..95061abfc2 100644 --- a/app/user.go +++ b/app/user.go @@ -108,7 +108,7 @@ func (a *App) CreateUserWithToken(user *model.User, token *model.Token) (*model. return ruser, nil } -func (a *App) CreateUserWithInviteId(user *model.User, inviteId string) (*model.User, *model.AppError) { +func (a *App) CreateUserWithInviteId(user *model.User, inviteId, redirect string) (*model.User, *model.AppError) { if err := a.IsUserSignUpAllowed(); err != nil { return nil, err } @@ -139,27 +139,27 @@ func (a *App) CreateUserWithInviteId(user *model.User, inviteId string) (*model. a.AddDirectChannels(team.Id, ruser) - if err := a.Srv().EmailService.sendWelcomeEmail(ruser.Id, ruser.Email, ruser.EmailVerified, ruser.Locale, a.GetSiteURL()); err != nil { + if err := a.Srv().EmailService.sendWelcomeEmail(ruser.Id, ruser.Email, ruser.EmailVerified, ruser.Locale, a.GetSiteURL(), redirect); err != nil { mlog.Error("Failed to send welcome email on create user with inviteId", mlog.Err(err)) } return ruser, nil } -func (a *App) CreateUserAsAdmin(user *model.User) (*model.User, *model.AppError) { +func (a *App) CreateUserAsAdmin(user *model.User, redirect string) (*model.User, *model.AppError) { ruser, err := a.CreateUser(user) if err != nil { return nil, err } - if err := a.Srv().EmailService.sendWelcomeEmail(ruser.Id, ruser.Email, ruser.EmailVerified, ruser.Locale, a.GetSiteURL()); err != nil { + if err := a.Srv().EmailService.sendWelcomeEmail(ruser.Id, ruser.Email, ruser.EmailVerified, ruser.Locale, a.GetSiteURL(), redirect); err != nil { mlog.Error("Failed to send welcome email on create admin user", mlog.Err(err)) } return ruser, nil } -func (a *App) CreateUserFromSignup(user *model.User) (*model.User, *model.AppError) { +func (a *App) CreateUserFromSignup(user *model.User, redirect string) (*model.User, *model.AppError) { if err := a.IsUserSignUpAllowed(); err != nil { return nil, err } @@ -176,7 +176,7 @@ func (a *App) CreateUserFromSignup(user *model.User) (*model.User, *model.AppErr return nil, err } - if err := a.Srv().EmailService.sendWelcomeEmail(ruser.Id, ruser.Email, ruser.EmailVerified, ruser.Locale, a.GetSiteURL()); err != nil { + if err := a.Srv().EmailService.sendWelcomeEmail(ruser.Id, ruser.Email, ruser.EmailVerified, ruser.Locale, a.GetSiteURL(), redirect); err != nil { mlog.Error("Failed to send welcome email on create user from signup", mlog.Err(err)) } @@ -1159,7 +1159,7 @@ func (a *App) UpdateUser(user *model.User, sendNotifications bool) (*model.User, if userUpdate.New.Email != userUpdate.Old.Email || newEmail != "" { if *a.Config().EmailSettings.RequireEmailVerification { a.Srv().Go(func() { - if err := a.SendEmailVerification(userUpdate.New, newEmail); err != nil { + if err := a.SendEmailVerification(userUpdate.New, newEmail, ""); err != nil { mlog.Error("Failed to send email verification", mlog.Err(err)) } }) @@ -1567,14 +1567,14 @@ func (a *App) PermanentDeleteAllUsers() *model.AppError { return nil } -func (a *App) SendEmailVerification(user *model.User, newEmail string) *model.AppError { +func (a *App) SendEmailVerification(user *model.User, newEmail, redirect string) *model.AppError { token, err := a.Srv().EmailService.CreateVerifyEmailToken(user.Id, newEmail) if err != nil { return err } if _, err := a.GetStatus(user.Id); err != nil { - return a.Srv().EmailService.sendVerifyEmail(newEmail, user.Locale, a.GetSiteURL(), token.Token) + return a.Srv().EmailService.sendVerifyEmail(newEmail, user.Locale, a.GetSiteURL(), token.Token, redirect) } return a.Srv().EmailService.sendEmailChangeVerifyEmail(newEmail, user.Locale, a.GetSiteURL(), token.Token) } diff --git a/app/user_test.go b/app/user_test.go index 6b9d55498f..44612741d7 100644 --- a/app/user_test.go +++ b/app/user_test.go @@ -600,13 +600,13 @@ func TestCreateUserWithInviteId(t *testing.T) { user := model.User{Email: strings.ToLower(model.NewId()) + "success+test@example.com", Nickname: "Darth Vader", Username: "vader" + model.NewId(), Password: "passwd1", AuthService: ""} t.Run("should create a user", func(t *testing.T) { - u, err := th.App.CreateUserWithInviteId(&user, th.BasicTeam.InviteId) + u, err := th.App.CreateUserWithInviteId(&user, th.BasicTeam.InviteId, "") require.Nil(t, err) require.Equal(t, u.Id, user.Id) }) t.Run("invalid invite id", func(t *testing.T) { - _, err := th.App.CreateUserWithInviteId(&user, "") + _, err := th.App.CreateUserWithInviteId(&user, "", "") require.NotNil(t, err) require.Contains(t, err.Id, "store.sql_team.get_by_invite_id") }) @@ -615,7 +615,7 @@ func TestCreateUserWithInviteId(t *testing.T) { th.BasicTeam.AllowedDomains = "mattermost.com" _, err := th.App.Srv().Store.Team().Update(th.BasicTeam) require.Nil(t, err) - _, err = th.App.CreateUserWithInviteId(&user, th.BasicTeam.InviteId) + _, err = th.App.CreateUserWithInviteId(&user, th.BasicTeam.InviteId, "") require.NotNil(t, err) require.Equal(t, "api.team.invite_members.invalid_email.app_error", err.Id) })