Skip to content

Commit

Permalink
Support for email addresses containing uppercase characters when acti…
Browse files Browse the repository at this point in the history
…vating user account (#32998)

Fix #32807

---------

Co-authored-by: wxiaoguang <[email protected]>
  • Loading branch information
Zettat123 and wxiaoguang authored Dec 27, 2024
1 parent 3c00e89 commit 2d1a171
Show file tree
Hide file tree
Showing 7 changed files with 61 additions and 35 deletions.
8 changes: 4 additions & 4 deletions models/user/email_address.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}

Expand Down
46 changes: 31 additions & 15 deletions models/user/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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).
Expand Down Expand Up @@ -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
}
Expand Down
4 changes: 2 additions & 2 deletions routers/web/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion routers/web/auth/password.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 6 additions & 3 deletions services/mailer/mail.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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(),
}
Expand Down
3 changes: 2 additions & 1 deletion tests/integration/org_team_invite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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("[email protected]"))
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",
})
Expand Down
24 changes: 15 additions & 9 deletions tests/integration/signup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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[email protected]",
"user_name": "Test-User-1",
"email": "EmAiL[email protected]",
"password": "password1",
"retype": "password1",
})
resp := MakeRequest(t, req, http.StatusOK)
assert.Contains(t, resp.Body.String(), `A new confirmation email has been sent to <b>email[email protected]</b>.`)
assert.Contains(t, resp.Body.String(), `A new confirmation email has been sent to <b>EmAiL[email protected]</b>.`)

// 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(), `<input id="change-email" name="change_email" `)

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

// generate an activation code from lower-cased email
activationCode := user_model.GenerateUserTimeLimitCode(&user_model.TimeLimitCodeOptions{Purpose: user_model.TimeLimitCodeActivateAccount}, user)
// and update the user email to case-sensitive, it shouldn't affect the verification later
_, _ = db.Exec(db.DefaultContext, "UPDATE `user` SET email=? WHERE id=?", "[email protected]", user.ID)
user = unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "Test-User-1"})
assert.Equal(t, "[email protected]", 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"})
activationCode := user.GenerateEmailActivateCode(user.Email)
resp = session.MakeRequest(t, NewRequest(t, "GET", "/user/activate?code="+activationCode), http.StatusOK)
assert.Contains(t, resp.Body.String(), `<input id="verify-password"`)

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

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

0 comments on commit 2d1a171

Please sign in to comment.