diff --git a/models/user/email_address.go b/models/user/email_address.go index 5c04909ed7c..74ba5f617a5 100644 --- a/models/user/email_address.go +++ b/models/user/email_address.go @@ -357,8 +357,8 @@ func VerifyActiveEmailCode(ctx context.Context, code, email string) *EmailAddres if user := GetVerifyUser(ctx, code); user != nil { // time limit code prefix := code[:base.TimeLimitCodeLength] - data := fmt.Sprintf("%d%s%s%s%s", user.ID, email, user.LowerName, user.Passwd, user.Rands) - + opts := &TimeLimitCodeOptions{Purpose: TimeLimitCodeActivateEmail, NewEmail: email} + data := makeTimeLimitCodeHashData(opts, user) if base.VerifyTimeLimitCode(time.Now(), data, setting.Service.ActiveCodeLives, prefix) { emailAddress := &EmailAddress{UID: user.ID, Email: email} if has, _ := db.GetEngine(ctx).Get(emailAddress); has { @@ -486,10 +486,10 @@ func ActivateUserEmail(ctx context.Context, userID int64, email string, activate // Activate/deactivate a user's primary email address and account if addr.IsPrimary { - user, exist, err := db.Get[User](ctx, builder.Eq{"id": userID, "email": email}) + user, exist, err := db.Get[User](ctx, builder.Eq{"id": userID}) if err != nil { return err - } else if !exist { + } else if !exist || !strings.EqualFold(user.Email, email) { return fmt.Errorf("no user with ID: %d and Email: %s", userID, email) } diff --git a/models/user/user.go b/models/user/user.go index cf08d26498e..19879fbcc79 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -181,7 +181,8 @@ func (u *User) BeforeUpdate() { u.MaxRepoCreation = -1 } - // Organization does not need email + // FIXME: this email doesn't need to be in lowercase, because the emails are mainly managed by the email table with lower_email field + // This trick could be removed in new releases to display the user inputed email as-is. u.Email = strings.ToLower(u.Email) if !u.IsOrganization() { if len(u.AvatarEmail) == 0 { @@ -310,17 +311,6 @@ func (u *User) OrganisationLink() string { return setting.AppSubURL + "/org/" + url.PathEscape(u.Name) } -// GenerateEmailActivateCode generates an activate code based on user information and given e-mail. -func (u *User) GenerateEmailActivateCode(email string) string { - code := base.CreateTimeLimitCode( - fmt.Sprintf("%d%s%s%s%s", u.ID, email, u.LowerName, u.Passwd, u.Rands), - setting.Service.ActiveCodeLives, time.Now(), nil) - - // Add tail hex username - code += hex.EncodeToString([]byte(u.LowerName)) - return code -} - // GetUserFollowers returns range of user's followers. func GetUserFollowers(ctx context.Context, u, viewer *User, listOptions db.ListOptions) ([]*User, int64, error) { sess := db.GetEngine(ctx). @@ -863,12 +853,38 @@ func GetVerifyUser(ctx context.Context, code string) (user *User) { return nil } -// VerifyUserActiveCode verifies active code when active account -func VerifyUserActiveCode(ctx context.Context, code string) (user *User) { +type TimeLimitCodePurpose string + +const ( + TimeLimitCodeActivateAccount TimeLimitCodePurpose = "activate_account" + TimeLimitCodeActivateEmail TimeLimitCodePurpose = "activate_email" + TimeLimitCodeResetPassword TimeLimitCodePurpose = "reset_password" +) + +type TimeLimitCodeOptions struct { + Purpose TimeLimitCodePurpose + NewEmail string +} + +func makeTimeLimitCodeHashData(opts *TimeLimitCodeOptions, u *User) string { + return fmt.Sprintf("%s|%d|%s|%s|%s|%s", opts.Purpose, u.ID, strings.ToLower(util.IfZero(opts.NewEmail, u.Email)), u.LowerName, u.Passwd, u.Rands) +} + +// GenerateUserTimeLimitCode generates a time-limit code based on user information and given e-mail. +// TODO: need to use cache or db to store it to make sure a code can only be consumed once +func GenerateUserTimeLimitCode(opts *TimeLimitCodeOptions, u *User) string { + data := makeTimeLimitCodeHashData(opts, u) + code := base.CreateTimeLimitCode(data, setting.Service.ActiveCodeLives, time.Now(), nil) + code += hex.EncodeToString([]byte(u.LowerName)) // Add tail hex username + return code +} + +// VerifyUserTimeLimitCode verifies the time-limit code +func VerifyUserTimeLimitCode(ctx context.Context, opts *TimeLimitCodeOptions, code string) (user *User) { if user = GetVerifyUser(ctx, code); user != nil { // time limit code prefix := code[:base.TimeLimitCodeLength] - data := fmt.Sprintf("%d%s%s%s%s", user.ID, user.Email, user.LowerName, user.Passwd, user.Rands) + data := makeTimeLimitCodeHashData(opts, user) if base.VerifyTimeLimitCode(time.Now(), data, setting.Service.ActiveCodeLives, prefix) { return user } diff --git a/routers/web/auth/auth.go b/routers/web/auth/auth.go index 42736a423f8..3fe1d5970e8 100644 --- a/routers/web/auth/auth.go +++ b/routers/web/auth/auth.go @@ -689,7 +689,7 @@ func Activate(ctx *context.Context) { } // TODO: ctx.Doer/ctx.Data["SignedUser"] could be nil or not the same user as the one being activated - user := user_model.VerifyUserActiveCode(ctx, code) + user := user_model.VerifyUserTimeLimitCode(ctx, &user_model.TimeLimitCodeOptions{Purpose: user_model.TimeLimitCodeActivateAccount}, code) if user == nil { // if code is wrong renderActivationPromptMessage(ctx, ctx.Locale.Tr("auth.invalid_code")) return @@ -734,7 +734,7 @@ func ActivatePost(ctx *context.Context) { } // TODO: ctx.Doer/ctx.Data["SignedUser"] could be nil or not the same user as the one being activated - user := user_model.VerifyUserActiveCode(ctx, code) + user := user_model.VerifyUserTimeLimitCode(ctx, &user_model.TimeLimitCodeOptions{Purpose: user_model.TimeLimitCodeActivateAccount}, code) if user == nil { // if code is wrong renderActivationPromptMessage(ctx, ctx.Locale.Tr("auth.invalid_code")) return diff --git a/routers/web/auth/password.go b/routers/web/auth/password.go index 3812d582e53..614e086f773 100644 --- a/routers/web/auth/password.go +++ b/routers/web/auth/password.go @@ -113,7 +113,7 @@ func commonResetPassword(ctx *context.Context) (*user_model.User, *auth.TwoFacto } // Fail early, don't frustrate the user - u := user_model.VerifyUserActiveCode(ctx, code) + u := user_model.VerifyUserTimeLimitCode(ctx, &user_model.TimeLimitCodeOptions{Purpose: user_model.TimeLimitCodeResetPassword}, code) if u == nil { ctx.Flash.Error(ctx.Tr("auth.invalid_code_forgot_password", fmt.Sprintf("%s/user/forgot_password", setting.AppSubURL)), true) return nil, nil diff --git a/services/mailer/mail.go b/services/mailer/mail.go index a6763e4f03c..52e19bde6f2 100644 --- a/services/mailer/mail.go +++ b/services/mailer/mail.go @@ -93,7 +93,8 @@ func SendActivateAccountMail(locale translation.Locale, u *user_model.User) { // No mail service configured return } - sendUserMail(locale.Language(), u, mailAuthActivate, u.GenerateEmailActivateCode(u.Email), locale.TrString("mail.activate_account"), "activate account") + opts := &user_model.TimeLimitCodeOptions{Purpose: user_model.TimeLimitCodeActivateAccount} + sendUserMail(locale.Language(), u, mailAuthActivate, user_model.GenerateUserTimeLimitCode(opts, u), locale.TrString("mail.activate_account"), "activate account") } // SendResetPasswordMail sends a password reset mail to the user @@ -103,7 +104,8 @@ func SendResetPasswordMail(u *user_model.User) { return } locale := translation.NewLocale(u.Language) - sendUserMail(u.Language, u, mailAuthResetPassword, u.GenerateEmailActivateCode(u.Email), locale.TrString("mail.reset_password"), "recover account") + opts := &user_model.TimeLimitCodeOptions{Purpose: user_model.TimeLimitCodeResetPassword} + sendUserMail(u.Language, u, mailAuthResetPassword, user_model.GenerateUserTimeLimitCode(opts, u), locale.TrString("mail.reset_password"), "recover account") } // SendActivateEmailMail sends confirmation email to confirm new email address @@ -113,11 +115,12 @@ func SendActivateEmailMail(u *user_model.User, email string) { return } locale := translation.NewLocale(u.Language) + opts := &user_model.TimeLimitCodeOptions{Purpose: user_model.TimeLimitCodeActivateEmail, NewEmail: email} data := map[string]any{ "locale": locale, "DisplayName": u.DisplayName(), "ActiveCodeLives": timeutil.MinutesToFriendly(setting.Service.ActiveCodeLives, locale), - "Code": u.GenerateEmailActivateCode(email), + "Code": user_model.GenerateUserTimeLimitCode(opts, u), "Email": email, "Language": locale.Language(), } diff --git a/tests/integration/org_team_invite_test.go b/tests/integration/org_team_invite_test.go index 274fde40850..4c1053702e6 100644 --- a/tests/integration/org_team_invite_test.go +++ b/tests/integration/org_team_invite_test.go @@ -274,7 +274,8 @@ func TestOrgTeamEmailInviteRedirectsNewUserWithActivation(t *testing.T) { user, err := user_model.GetUserByName(db.DefaultContext, "doesnotexist") assert.NoError(t, err) - activateURL := fmt.Sprintf("/user/activate?code=%s", user.GenerateEmailActivateCode("doesnotexist@example.com")) + activationCode := user_model.GenerateUserTimeLimitCode(&user_model.TimeLimitCodeOptions{Purpose: user_model.TimeLimitCodeActivateAccount}, user) + activateURL := fmt.Sprintf("/user/activate?code=%s", activationCode) req = NewRequestWithValues(t, "POST", activateURL, map[string]string{ "password": "examplePassword!1", }) diff --git a/tests/integration/signup_test.go b/tests/integration/signup_test.go index e9a05201eef..e86851352e0 100644 --- a/tests/integration/signup_test.go +++ b/tests/integration/signup_test.go @@ -9,6 +9,7 @@ import ( "strings" "testing" + "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/setting" @@ -99,34 +100,39 @@ func TestSignupEmailActive(t *testing.T) { // try to sign up and send the activation email req := NewRequestWithValues(t, "POST", "/user/sign_up", map[string]string{ - "user_name": "test-user-1", - "email": "email-1@example.com", + "user_name": "Test-User-1", + "email": "EmAiL-1@example.com", "password": "password1", "retype": "password1", }) resp := MakeRequest(t, req, http.StatusOK) - assert.Contains(t, resp.Body.String(), `A new confirmation email has been sent to email-1@example.com.`) + assert.Contains(t, resp.Body.String(), `A new confirmation email has been sent to EmAiL-1@example.com.`) // access "user/activate" means trying to re-send the activation email session := loginUserWithPassword(t, "test-user-1", "password1") resp = session.MakeRequest(t, NewRequest(t, "GET", "/user/activate"), http.StatusOK) assert.Contains(t, resp.Body.String(), "You have already requested an activation email recently") - // access anywhere else will see a "Activate Your Account" prompt, and there is a chance to change email + // access anywhere else will see an "Activate Your Account" prompt, and there is a chance to change email resp = session.MakeRequest(t, NewRequest(t, "GET", "/user/issues"), http.StatusOK) assert.Contains(t, resp.Body.String(), `