Skip to content

Commit 2d1a171

Browse files
Zettat123wxiaoguang
Zettat123
and
wxiaoguang
authored
Support for email addresses containing uppercase characters when activating user account (#32998)
Fix #32807 --------- Co-authored-by: wxiaoguang <[email protected]>
1 parent 3c00e89 commit 2d1a171

File tree

7 files changed

+61
-35
lines changed

7 files changed

+61
-35
lines changed

Diff for: ‎models/user/email_address.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -357,8 +357,8 @@ func VerifyActiveEmailCode(ctx context.Context, code, email string) *EmailAddres
357357
if user := GetVerifyUser(ctx, code); user != nil {
358358
// time limit code
359359
prefix := code[:base.TimeLimitCodeLength]
360-
data := fmt.Sprintf("%d%s%s%s%s", user.ID, email, user.LowerName, user.Passwd, user.Rands)
361-
360+
opts := &TimeLimitCodeOptions{Purpose: TimeLimitCodeActivateEmail, NewEmail: email}
361+
data := makeTimeLimitCodeHashData(opts, user)
362362
if base.VerifyTimeLimitCode(time.Now(), data, setting.Service.ActiveCodeLives, prefix) {
363363
emailAddress := &EmailAddress{UID: user.ID, Email: email}
364364
if has, _ := db.GetEngine(ctx).Get(emailAddress); has {
@@ -486,10 +486,10 @@ func ActivateUserEmail(ctx context.Context, userID int64, email string, activate
486486

487487
// Activate/deactivate a user's primary email address and account
488488
if addr.IsPrimary {
489-
user, exist, err := db.Get[User](ctx, builder.Eq{"id": userID, "email": email})
489+
user, exist, err := db.Get[User](ctx, builder.Eq{"id": userID})
490490
if err != nil {
491491
return err
492-
} else if !exist {
492+
} else if !exist || !strings.EqualFold(user.Email, email) {
493493
return fmt.Errorf("no user with ID: %d and Email: %s", userID, email)
494494
}
495495

Diff for: ‎models/user/user.go

+31-15
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,8 @@ func (u *User) BeforeUpdate() {
181181
u.MaxRepoCreation = -1
182182
}
183183

184-
// Organization does not need email
184+
// FIXME: this email doesn't need to be in lowercase, because the emails are mainly managed by the email table with lower_email field
185+
// This trick could be removed in new releases to display the user inputed email as-is.
185186
u.Email = strings.ToLower(u.Email)
186187
if !u.IsOrganization() {
187188
if len(u.AvatarEmail) == 0 {
@@ -310,17 +311,6 @@ func (u *User) OrganisationLink() string {
310311
return setting.AppSubURL + "/org/" + url.PathEscape(u.Name)
311312
}
312313

313-
// GenerateEmailActivateCode generates an activate code based on user information and given e-mail.
314-
func (u *User) GenerateEmailActivateCode(email string) string {
315-
code := base.CreateTimeLimitCode(
316-
fmt.Sprintf("%d%s%s%s%s", u.ID, email, u.LowerName, u.Passwd, u.Rands),
317-
setting.Service.ActiveCodeLives, time.Now(), nil)
318-
319-
// Add tail hex username
320-
code += hex.EncodeToString([]byte(u.LowerName))
321-
return code
322-
}
323-
324314
// GetUserFollowers returns range of user's followers.
325315
func GetUserFollowers(ctx context.Context, u, viewer *User, listOptions db.ListOptions) ([]*User, int64, error) {
326316
sess := db.GetEngine(ctx).
@@ -863,12 +853,38 @@ func GetVerifyUser(ctx context.Context, code string) (user *User) {
863853
return nil
864854
}
865855

866-
// VerifyUserActiveCode verifies active code when active account
867-
func VerifyUserActiveCode(ctx context.Context, code string) (user *User) {
856+
type TimeLimitCodePurpose string
857+
858+
const (
859+
TimeLimitCodeActivateAccount TimeLimitCodePurpose = "activate_account"
860+
TimeLimitCodeActivateEmail TimeLimitCodePurpose = "activate_email"
861+
TimeLimitCodeResetPassword TimeLimitCodePurpose = "reset_password"
862+
)
863+
864+
type TimeLimitCodeOptions struct {
865+
Purpose TimeLimitCodePurpose
866+
NewEmail string
867+
}
868+
869+
func makeTimeLimitCodeHashData(opts *TimeLimitCodeOptions, u *User) string {
870+
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)
871+
}
872+
873+
// GenerateUserTimeLimitCode generates a time-limit code based on user information and given e-mail.
874+
// TODO: need to use cache or db to store it to make sure a code can only be consumed once
875+
func GenerateUserTimeLimitCode(opts *TimeLimitCodeOptions, u *User) string {
876+
data := makeTimeLimitCodeHashData(opts, u)
877+
code := base.CreateTimeLimitCode(data, setting.Service.ActiveCodeLives, time.Now(), nil)
878+
code += hex.EncodeToString([]byte(u.LowerName)) // Add tail hex username
879+
return code
880+
}
881+
882+
// VerifyUserTimeLimitCode verifies the time-limit code
883+
func VerifyUserTimeLimitCode(ctx context.Context, opts *TimeLimitCodeOptions, code string) (user *User) {
868884
if user = GetVerifyUser(ctx, code); user != nil {
869885
// time limit code
870886
prefix := code[:base.TimeLimitCodeLength]
871-
data := fmt.Sprintf("%d%s%s%s%s", user.ID, user.Email, user.LowerName, user.Passwd, user.Rands)
887+
data := makeTimeLimitCodeHashData(opts, user)
872888
if base.VerifyTimeLimitCode(time.Now(), data, setting.Service.ActiveCodeLives, prefix) {
873889
return user
874890
}

Diff for: ‎routers/web/auth/auth.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -689,7 +689,7 @@ func Activate(ctx *context.Context) {
689689
}
690690

691691
// TODO: ctx.Doer/ctx.Data["SignedUser"] could be nil or not the same user as the one being activated
692-
user := user_model.VerifyUserActiveCode(ctx, code)
692+
user := user_model.VerifyUserTimeLimitCode(ctx, &user_model.TimeLimitCodeOptions{Purpose: user_model.TimeLimitCodeActivateAccount}, code)
693693
if user == nil { // if code is wrong
694694
renderActivationPromptMessage(ctx, ctx.Locale.Tr("auth.invalid_code"))
695695
return
@@ -734,7 +734,7 @@ func ActivatePost(ctx *context.Context) {
734734
}
735735

736736
// TODO: ctx.Doer/ctx.Data["SignedUser"] could be nil or not the same user as the one being activated
737-
user := user_model.VerifyUserActiveCode(ctx, code)
737+
user := user_model.VerifyUserTimeLimitCode(ctx, &user_model.TimeLimitCodeOptions{Purpose: user_model.TimeLimitCodeActivateAccount}, code)
738738
if user == nil { // if code is wrong
739739
renderActivationPromptMessage(ctx, ctx.Locale.Tr("auth.invalid_code"))
740740
return

Diff for: ‎routers/web/auth/password.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ func commonResetPassword(ctx *context.Context) (*user_model.User, *auth.TwoFacto
113113
}
114114

115115
// Fail early, don't frustrate the user
116-
u := user_model.VerifyUserActiveCode(ctx, code)
116+
u := user_model.VerifyUserTimeLimitCode(ctx, &user_model.TimeLimitCodeOptions{Purpose: user_model.TimeLimitCodeResetPassword}, code)
117117
if u == nil {
118118
ctx.Flash.Error(ctx.Tr("auth.invalid_code_forgot_password", fmt.Sprintf("%s/user/forgot_password", setting.AppSubURL)), true)
119119
return nil, nil

Diff for: ‎services/mailer/mail.go

+6-3
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,8 @@ func SendActivateAccountMail(locale translation.Locale, u *user_model.User) {
9393
// No mail service configured
9494
return
9595
}
96-
sendUserMail(locale.Language(), u, mailAuthActivate, u.GenerateEmailActivateCode(u.Email), locale.TrString("mail.activate_account"), "activate account")
96+
opts := &user_model.TimeLimitCodeOptions{Purpose: user_model.TimeLimitCodeActivateAccount}
97+
sendUserMail(locale.Language(), u, mailAuthActivate, user_model.GenerateUserTimeLimitCode(opts, u), locale.TrString("mail.activate_account"), "activate account")
9798
}
9899

99100
// SendResetPasswordMail sends a password reset mail to the user
@@ -103,7 +104,8 @@ func SendResetPasswordMail(u *user_model.User) {
103104
return
104105
}
105106
locale := translation.NewLocale(u.Language)
106-
sendUserMail(u.Language, u, mailAuthResetPassword, u.GenerateEmailActivateCode(u.Email), locale.TrString("mail.reset_password"), "recover account")
107+
opts := &user_model.TimeLimitCodeOptions{Purpose: user_model.TimeLimitCodeResetPassword}
108+
sendUserMail(u.Language, u, mailAuthResetPassword, user_model.GenerateUserTimeLimitCode(opts, u), locale.TrString("mail.reset_password"), "recover account")
107109
}
108110

109111
// SendActivateEmailMail sends confirmation email to confirm new email address
@@ -113,11 +115,12 @@ func SendActivateEmailMail(u *user_model.User, email string) {
113115
return
114116
}
115117
locale := translation.NewLocale(u.Language)
118+
opts := &user_model.TimeLimitCodeOptions{Purpose: user_model.TimeLimitCodeActivateEmail, NewEmail: email}
116119
data := map[string]any{
117120
"locale": locale,
118121
"DisplayName": u.DisplayName(),
119122
"ActiveCodeLives": timeutil.MinutesToFriendly(setting.Service.ActiveCodeLives, locale),
120-
"Code": u.GenerateEmailActivateCode(email),
123+
"Code": user_model.GenerateUserTimeLimitCode(opts, u),
121124
"Email": email,
122125
"Language": locale.Language(),
123126
}

Diff for: ‎tests/integration/org_team_invite_test.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,8 @@ func TestOrgTeamEmailInviteRedirectsNewUserWithActivation(t *testing.T) {
274274
user, err := user_model.GetUserByName(db.DefaultContext, "doesnotexist")
275275
assert.NoError(t, err)
276276

277-
activateURL := fmt.Sprintf("/user/activate?code=%s", user.GenerateEmailActivateCode("[email protected]"))
277+
activationCode := user_model.GenerateUserTimeLimitCode(&user_model.TimeLimitCodeOptions{Purpose: user_model.TimeLimitCodeActivateAccount}, user)
278+
activateURL := fmt.Sprintf("/user/activate?code=%s", activationCode)
278279
req = NewRequestWithValues(t, "POST", activateURL, map[string]string{
279280
"password": "examplePassword!1",
280281
})

Diff for: ‎tests/integration/signup_test.go

+15-9
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"strings"
1010
"testing"
1111

12+
"code.gitea.io/gitea/models/db"
1213
"code.gitea.io/gitea/models/unittest"
1314
user_model "code.gitea.io/gitea/models/user"
1415
"code.gitea.io/gitea/modules/setting"
@@ -99,34 +100,39 @@ func TestSignupEmailActive(t *testing.T) {
99100

100101
// try to sign up and send the activation email
101102
req := NewRequestWithValues(t, "POST", "/user/sign_up", map[string]string{
102-
"user_name": "test-user-1",
103-
"email": "email[email protected]",
103+
"user_name": "Test-User-1",
104+
"email": "EmAiL[email protected]",
104105
"password": "password1",
105106
"retype": "password1",
106107
})
107108
resp := MakeRequest(t, req, http.StatusOK)
108-
assert.Contains(t, resp.Body.String(), `A new confirmation email has been sent to <b>email[email protected]</b>.`)
109+
assert.Contains(t, resp.Body.String(), `A new confirmation email has been sent to <b>EmAiL[email protected]</b>.`)
109110

110111
// access "user/activate" means trying to re-send the activation email
111112
session := loginUserWithPassword(t, "test-user-1", "password1")
112113
resp = session.MakeRequest(t, NewRequest(t, "GET", "/user/activate"), http.StatusOK)
113114
assert.Contains(t, resp.Body.String(), "You have already requested an activation email recently")
114115

115-
// access anywhere else will see a "Activate Your Account" prompt, and there is a chance to change email
116+
// access anywhere else will see an "Activate Your Account" prompt, and there is a chance to change email
116117
resp = session.MakeRequest(t, NewRequest(t, "GET", "/user/issues"), http.StatusOK)
117118
assert.Contains(t, resp.Body.String(), `<input id="change-email" name="change_email" `)
118119

119120
// post to "user/activate" with a new email
120121
session.MakeRequest(t, NewRequestWithValues(t, "POST", "/user/activate", map[string]string{"change_email": "[email protected]"}), http.StatusSeeOther)
121-
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "test-user-1"})
122+
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "Test-User-1"})
122123
assert.Equal(t, "[email protected]", user.Email)
123124
email := unittest.AssertExistsAndLoadBean(t, &user_model.EmailAddress{Email: "[email protected]"})
124125
assert.False(t, email.IsActivated)
125126
assert.True(t, email.IsPrimary)
126127

128+
// generate an activation code from lower-cased email
129+
activationCode := user_model.GenerateUserTimeLimitCode(&user_model.TimeLimitCodeOptions{Purpose: user_model.TimeLimitCodeActivateAccount}, user)
130+
// and update the user email to case-sensitive, it shouldn't affect the verification later
131+
_, _ = db.Exec(db.DefaultContext, "UPDATE `user` SET email=? WHERE id=?", "[email protected]", user.ID)
132+
user = unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "Test-User-1"})
133+
assert.Equal(t, "[email protected]", user.Email)
134+
127135
// access "user/activate" with a valid activation code, then get the "verify password" page
128-
user = unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "test-user-1"})
129-
activationCode := user.GenerateEmailActivateCode(user.Email)
130136
resp = session.MakeRequest(t, NewRequest(t, "GET", "/user/activate?code="+activationCode), http.StatusOK)
131137
assert.Contains(t, resp.Body.String(), `<input id="verify-password"`)
132138

@@ -138,7 +144,7 @@ func TestSignupEmailActive(t *testing.T) {
138144
resp = session.MakeRequest(t, req, http.StatusOK)
139145
assert.Contains(t, resp.Body.String(), `Your password does not match`)
140146
assert.Contains(t, resp.Body.String(), `<input id="verify-password"`)
141-
user = unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "test-user-1"})
147+
user = unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "Test-User-1"})
142148
assert.False(t, user.IsActive)
143149

144150
// then use a correct password, the user should be activated
@@ -148,6 +154,6 @@ func TestSignupEmailActive(t *testing.T) {
148154
})
149155
resp = session.MakeRequest(t, req, http.StatusSeeOther)
150156
assert.Equal(t, "/", test.RedirectURL(resp))
151-
user = unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "test-user-1"})
157+
user = unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "Test-User-1"})
152158
assert.True(t, user.IsActive)
153159
}

0 commit comments

Comments
 (0)