From 05f64efba75312d36dcd5ac7185a7252e044093f Mon Sep 17 00:00:00 2001 From: Zettat123 Date: Fri, 27 Dec 2024 12:02:24 +0800 Subject: [PATCH 1/6] support uppercase email --- models/user/email_address.go | 2 +- models/user/user.go | 1 + tests/integration/signup_test.go | 25 +++++++++++++++++++++++++ 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/models/user/email_address.go b/models/user/email_address.go index 5c04909ed7c59..c23b4d47ab5fd 100644 --- a/models/user/email_address.go +++ b/models/user/email_address.go @@ -486,7 +486,7 @@ 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, "email": strings.ToLower(email)}) if err != nil { return err } else if !exist { diff --git a/models/user/user.go b/models/user/user.go index cf08d26498ec1..7d0deb676bc67 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -728,6 +728,7 @@ func createUser(ctx context.Context, u *User, meta *Meta, createdByAdmin bool, o u.LowerName = strings.ToLower(u.Name) u.AvatarEmail = u.Email + u.Email = strings.ToLower(u.Email) if u.Rands, err = GetUserSalt(); err != nil { return err } diff --git a/tests/integration/signup_test.go b/tests/integration/signup_test.go index e9a05201eefbd..47e9e83369eaa 100644 --- a/tests/integration/signup_test.go +++ b/tests/integration/signup_test.go @@ -151,3 +151,28 @@ func TestSignupEmailActive(t *testing.T) { user = unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "test-user-1"}) assert.True(t, user.IsActive) } + +func TestSignUpWithUppercaseEmail(t *testing.T) { + defer tests.PrepareTestEnv(t)() + defer test.MockVariableValue(&setting.Service.RegisterEmailConfirm, true)() + + req := NewRequestWithValues(t, "POST", "/user/sign_up", map[string]string{ + "user_name": "Upper-user-1", + "email": "Upper-email-1@example.com", + "password": "password1", + "retype": "password1", + }) + MakeRequest(t, req, http.StatusOK) + + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "Upper-user-1"}) + session := loginUserWithPassword(t, "Upper-user-1", "password1") + activationCode := user.GenerateEmailActivateCode(user.Email) + req = NewRequestWithValues(t, "POST", "/user/activate", map[string]string{ + "code": activationCode, + "password": "password1", + }) + resp := session.MakeRequest(t, req, http.StatusSeeOther) + assert.Equal(t, "/", test.RedirectURL(resp)) + user = unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "Upper-user-1"}) + assert.True(t, user.IsActive) +} From 4a03afc086db511fb4f230b250824d3414bc9306 Mon Sep 17 00:00:00 2001 From: Zettat123 Date: Fri, 27 Dec 2024 12:23:43 +0800 Subject: [PATCH 2/6] follow suggestions --- models/user/email_address.go | 4 ++-- models/user/user.go | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/models/user/email_address.go b/models/user/email_address.go index c23b4d47ab5fd..d35e96d770dc4 100644 --- a/models/user/email_address.go +++ b/models/user/email_address.go @@ -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": strings.ToLower(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 7d0deb676bc67..cf08d26498ec1 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -728,7 +728,6 @@ func createUser(ctx context.Context, u *User, meta *Meta, createdByAdmin bool, o u.LowerName = strings.ToLower(u.Name) u.AvatarEmail = u.Email - u.Email = strings.ToLower(u.Email) if u.Rands, err = GetUserSalt(); err != nil { return err } From 7a5c8082361573927f919187dfe725993cecf98c Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 27 Dec 2024 12:31:29 +0800 Subject: [PATCH 3/6] Update user.go --- models/user/user.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/models/user/user.go b/models/user/user.go index cf08d26498ec1..bde6291065826 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 { From d7608e59c18dda6c5f6296f5df3b0fb976af0b90 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 27 Dec 2024 13:05:24 +0800 Subject: [PATCH 4/6] fix email in token --- models/user/email_address.go | 4 ++-- models/user/user.go | 40 +++++++++++++++++++++++------------- routers/web/auth/auth.go | 4 ++-- routers/web/auth/password.go | 2 +- services/mailer/mail.go | 9 +++++--- 5 files changed, 37 insertions(+), 22 deletions(-) diff --git a/models/user/email_address.go b/models/user/email_address.go index d35e96d770dc4..74ba5f617a55d 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 { diff --git a/models/user/user.go b/models/user/user.go index bde6291065826..6780432fdfe1f 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -311,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). @@ -864,12 +853,35 @@ 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" +const TimeLimitCodeActivateEmail TimeLimitCodePurpose = "activate_email" +const 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 an activate code based on user information and given e-mail. +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 active code when active account +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 42736a423f80d..3fe1d5970e816 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 3812d582e53fc..614e086f773b0 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 a6763e4f03c09..52e19bde6f261 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(), } From de36d2f53eac027f9520055e66988ea61cc10df3 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 27 Dec 2024 13:26:30 +0800 Subject: [PATCH 5/6] fix tests --- models/user/user.go | 5 ++- tests/integration/org_team_invite_test.go | 3 +- tests/integration/signup_test.go | 50 ++++++++--------------- 3 files changed, 21 insertions(+), 37 deletions(-) diff --git a/models/user/user.go b/models/user/user.go index 6780432fdfe1f..70bd739e8733d 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -868,7 +868,8 @@ 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 an activate code based on user information and given e-mail. +// 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) @@ -876,7 +877,7 @@ func GenerateUserTimeLimitCode(opts *TimeLimitCodeOptions, u *User) string { return code } -// VerifyUserTimeLimitCode verifies active code when active account +// 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 diff --git a/tests/integration/org_team_invite_test.go b/tests/integration/org_team_invite_test.go index 274fde4085054..4c1053702e652 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 47e9e83369eaa..9afe4cf337fc6 100644 --- a/tests/integration/signup_test.go +++ b/tests/integration/signup_test.go @@ -4,6 +4,7 @@ package integration import ( + "code.gitea.io/gitea/models/db" "fmt" "net/http" "strings" @@ -99,34 +100,40 @@ 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(), ` Date: Fri, 27 Dec 2024 13:33:21 +0800 Subject: [PATCH 6/6] fix lint --- models/user/user.go | 8 +++++--- tests/integration/signup_test.go | 3 +-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/models/user/user.go b/models/user/user.go index 70bd739e8733d..19879fbcc7915 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -855,9 +855,11 @@ func GetVerifyUser(ctx context.Context, code string) (user *User) { type TimeLimitCodePurpose string -const TimeLimitCodeActivateAccount TimeLimitCodePurpose = "activate_account" -const TimeLimitCodeActivateEmail TimeLimitCodePurpose = "activate_email" -const TimeLimitCodeResetPassword TimeLimitCodePurpose = "reset_password" +const ( + TimeLimitCodeActivateAccount TimeLimitCodePurpose = "activate_account" + TimeLimitCodeActivateEmail TimeLimitCodePurpose = "activate_email" + TimeLimitCodeResetPassword TimeLimitCodePurpose = "reset_password" +) type TimeLimitCodeOptions struct { Purpose TimeLimitCodePurpose diff --git a/tests/integration/signup_test.go b/tests/integration/signup_test.go index 9afe4cf337fc6..e86851352e090 100644 --- a/tests/integration/signup_test.go +++ b/tests/integration/signup_test.go @@ -4,12 +4,12 @@ package integration import ( - "code.gitea.io/gitea/models/db" "fmt" "net/http" "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" @@ -133,7 +133,6 @@ func TestSignupEmailActive(t *testing.T) { assert.Equal(t, "EmAiL-changed@example.com", user.Email) // access "user/activate" with a valid activation code, then get the "verify password" page - user = unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "Test-User-1"}) resp = session.MakeRequest(t, NewRequest(t, "GET", "/user/activate?code="+activationCode), http.StatusOK) assert.Contains(t, resp.Body.String(), `