Skip to content

Commit de1e575

Browse files
wxiaoguangwxiaoguang
committed
fix
1 parent 328d908 commit de1e575

File tree

4 files changed

+145
-93
lines changed

4 files changed

+145
-93
lines changed

‎routers/web/auth/auth.go

Lines changed: 66 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ package auth
77
import (
88
"errors"
99
"fmt"
10+
"html/template"
1011
"net/http"
1112
"strings"
1213

@@ -37,12 +38,10 @@ import (
3738
)
3839

3940
const (
40-
// tplSignIn template for sign in page
41-
tplSignIn base.TplName = "user/auth/signin"
42-
// tplSignUp template path for sign up page
43-
tplSignUp base.TplName = "user/auth/signup"
44-
// TplActivate template path for activate user
45-
TplActivate base.TplName = "user/auth/activate"
41+
tplSignIn base.TplName = "user/auth/signin" // for sign in page
42+
tplSignUp base.TplName = "user/auth/signup" // for sign up page
43+
TplActivate base.TplName = "user/auth/activate" // for activate user
44+
TplActivatePrompt base.TplName = "user/auth/activate_prompt" // for showing a message for user activation
4645
)
4746

4847
// autoSignIn reads cookie and try to auto-login.
@@ -613,72 +612,83 @@ func handleUserCreated(ctx *context.Context, u *user_model.User, gothUser *goth.
613612
}
614613
}
615614

616-
// Send confirmation email
617-
if !u.IsActive && u.ID > 1 {
618-
if setting.Service.RegisterManualConfirm {
619-
ctx.Data["ManualActivationOnly"] = true
620-
ctx.HTML(http.StatusOK, TplActivate)
621-
return false
622-
}
615+
// for active user or the first (admin) user, we don't need to send confirmation email
616+
if u.IsActive || u.ID == 1 {
617+
return true
618+
}
623619

624-
mailer.SendActivateAccountMail(ctx.Locale, u)
620+
if setting.Service.RegisterManualConfirm {
621+
renderActivationPromptMessage(ctx, ctx.Locale.Tr("auth.manual_activation_only"))
622+
return false
623+
}
625624

626-
ctx.Data["IsSendRegisterMail"] = true
627-
ctx.Data["Email"] = u.Email
628-
ctx.Data["ActiveCodeLives"] = timeutil.MinutesToFriendly(setting.Service.ActiveCodeLives, ctx.Locale)
629-
ctx.HTML(http.StatusOK, TplActivate)
625+
sendActivateEmail(ctx, u)
626+
return false
627+
}
630628

631-
if err := ctx.Cache.Put("MailResendLimit_"+u.LowerName, u.LowerName, 180); err != nil {
632-
log.Error("Set cache(MailResendLimit) fail: %v", err)
633-
}
634-
return false
629+
func renderActivationPromptMessage(ctx *context.Context, msg template.HTML) {
630+
ctx.Data["ActivationPromptMessage"] = msg
631+
ctx.HTML(http.StatusOK, TplActivatePrompt)
632+
}
633+
634+
func sendActivateEmail(ctx *context.Context, u *user_model.User) {
635+
if ctx.Cache.IsExist("MailResendLimit_" + u.LowerName) {
636+
renderActivationPromptMessage(ctx, ctx.Locale.Tr("auth.resent_limit_prompt"))
637+
return
635638
}
636639

637-
return true
640+
if err := ctx.Cache.Put("MailResendLimit_"+u.LowerName, u.LowerName, 180); err != nil {
641+
log.Error("Set cache(MailResendLimit) fail: %v", err)
642+
renderActivationPromptMessage(ctx, ctx.Locale.Tr("auth.resent_limit_prompt"))
643+
return
644+
}
645+
646+
mailer.SendActivateAccountMail(ctx.Locale, u)
647+
648+
activeCodeLives := timeutil.MinutesToFriendly(setting.Service.ActiveCodeLives, ctx.Locale)
649+
msgHTML := ctx.Locale.Tr("auth.confirmation_mail_sent_prompt", u.Email, activeCodeLives)
650+
renderActivationPromptMessage(ctx, msgHTML)
651+
}
652+
653+
func renderActivationVerifyPassword(ctx *context.Context, code string) {
654+
ctx.Data["ActivationCode"] = code
655+
ctx.Data["NeedVerifyLocalPassword"] = true
656+
ctx.HTML(http.StatusOK, TplActivate)
638657
}
639658

640659
// Activate render activate user page
641660
func Activate(ctx *context.Context) {
642661
code := ctx.FormString("code")
643662

644-
if len(code) == 0 {
645-
ctx.Data["IsActivatePage"] = true
646-
if ctx.Doer == nil || ctx.Doer.IsActive {
647-
ctx.NotFound("invalid user", nil)
663+
if code == "" {
664+
if ctx.Doer == nil {
665+
ctx.Redirect(setting.AppSubURL + "/user/login")
666+
return
667+
} else if ctx.Doer.IsActive {
668+
ctx.Redirect(setting.AppSubURL + "/")
648669
return
649670
}
650-
// Resend confirmation email.
651-
if setting.Service.RegisterEmailConfirm {
652-
if ctx.Cache.IsExist("MailResendLimit_" + ctx.Doer.LowerName) {
653-
ctx.Data["ResendLimited"] = true
654-
} else {
655-
ctx.Data["ActiveCodeLives"] = timeutil.MinutesToFriendly(setting.Service.ActiveCodeLives, ctx.Locale)
656-
mailer.SendActivateAccountMail(ctx.Locale, ctx.Doer)
657671

658-
if err := ctx.Cache.Put("MailResendLimit_"+ctx.Doer.LowerName, ctx.Doer.LowerName, 180); err != nil {
659-
log.Error("Set cache(MailResendLimit) fail: %v", err)
660-
}
661-
}
662-
} else {
663-
ctx.Data["ServiceNotEnabled"] = true
672+
if setting.MailService == nil || !setting.Service.RegisterEmailConfirm {
673+
renderActivationPromptMessage(ctx, ctx.Tr("auth.disable_register_mail"))
674+
return
664675
}
665-
ctx.HTML(http.StatusOK, TplActivate)
676+
677+
// Resend confirmation email.
678+
sendActivateEmail(ctx, ctx.Doer)
666679
return
667680
}
668681

682+
// TODO: ctx.Doer/ctx.Data["SignedUser"] could be nil or not the same user as the one being activated
669683
user := user_model.VerifyUserActiveCode(ctx, code)
670-
// if code is wrong
671-
if user == nil {
672-
ctx.Data["IsCodeInvalid"] = true
673-
ctx.HTML(http.StatusOK, TplActivate)
684+
if user == nil { // if code is wrong
685+
renderActivationPromptMessage(ctx, ctx.Locale.Tr("auth.invalid_code"))
674686
return
675687
}
676688

677689
// if account is local account, verify password
678690
if user.LoginSource == 0 {
679-
ctx.Data["Code"] = code
680-
ctx.Data["NeedsPassword"] = true
681-
ctx.HTML(http.StatusOK, TplActivate)
691+
renderActivationVerifyPassword(ctx, code)
682692
return
683693
}
684694

@@ -688,31 +698,28 @@ func Activate(ctx *context.Context) {
688698
// ActivatePost handles account activation with password check
689699
func ActivatePost(ctx *context.Context) {
690700
code := ctx.FormString("code")
691-
if len(code) == 0 {
701+
if code == "" || (ctx.Doer != nil && ctx.Doer.IsActive) {
692702
ctx.Redirect(setting.AppSubURL + "/user/activate")
693703
return
694704
}
695705

706+
// TODO: ctx.Doer/ctx.Data["SignedUser"] could be nil or not the same user as the one being activated
696707
user := user_model.VerifyUserActiveCode(ctx, code)
697-
// if code is wrong
698-
if user == nil {
699-
ctx.Data["IsCodeInvalid"] = true
700-
ctx.HTML(http.StatusOK, TplActivate)
708+
if user == nil { // if code is wrong
709+
renderActivationPromptMessage(ctx, ctx.Locale.Tr("auth.invalid_code"))
701710
return
702711
}
703712

704713
// if account is local account, verify password
705714
if user.LoginSource == 0 {
706715
password := ctx.FormString("password")
707-
if len(password) == 0 {
708-
ctx.Data["Code"] = code
709-
ctx.Data["NeedsPassword"] = true
710-
ctx.HTML(http.StatusOK, TplActivate)
716+
if password == "" {
717+
renderActivationVerifyPassword(ctx, code)
711718
return
712719
}
713720
if !user.ValidatePassword(password) {
714-
ctx.Data["IsPasswordInvalid"] = true
715-
ctx.HTML(http.StatusOK, TplActivate)
721+
ctx.Flash.Error(ctx.Locale.Tr("auth.invalid_password"), true)
722+
renderActivationVerifyPassword(ctx, code)
716723
return
717724
}
718725
}

‎templates/user/auth/activate.tmpl

Lines changed: 15 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -9,40 +9,22 @@
99
</h2>
1010
<div class="ui attached segment">
1111
{{template "base/alert" .}}
12-
{{if .IsActivatePage}}
13-
{{if .ServiceNotEnabled}}
14-
<p class="center">{{ctx.Locale.Tr "auth.disable_register_mail"}}</p>
15-
{{else if .ResendLimited}}
16-
<p class="center">{{ctx.Locale.Tr "auth.resent_limit_prompt"}}</p>
17-
{{else}}
18-
<p>{{ctx.Locale.Tr "auth.confirmation_mail_sent_prompt" (.SignedUser.Email|Escape) .ActiveCodeLives}}</p>
19-
{{end}}
12+
{{if .NeedVerifyLocalPassword}}
13+
<div class="required inline field">
14+
<label for="verify-password">{{ctx.Locale.Tr "password"}}</label>
15+
<input id="verify-password" name="password" type="password" autocomplete="off" required>
16+
</div>
17+
<div class="inline field">
18+
<label></label>
19+
<button class="ui primary button">{{ctx.Locale.Tr "install.confirm_password"}}</button>
20+
</div>
21+
<input name="code" type="hidden" value="{{.ActivationCode}}">
2022
{{else}}
21-
{{if .NeedsPassword}}
22-
<div class="required inline field">
23-
<label for="password">{{ctx.Locale.Tr "password"}}</label>
24-
<input id="password" name="password" type="password" autocomplete="off" required>
25-
</div>
26-
<div class="inline field">
27-
<label></label>
28-
<button class="ui primary button">{{ctx.Locale.Tr "install.confirm_password"}}</button>
29-
</div>
30-
<input id="code" name="code" type="hidden" value="{{.Code}}">
31-
{{else if .IsSendRegisterMail}}
32-
<p>{{ctx.Locale.Tr "auth.confirmation_mail_sent_prompt" (.Email|Escape) .ActiveCodeLives}}</p>
33-
{{else if .IsCodeInvalid}}
34-
<p>{{ctx.Locale.Tr "auth.invalid_code"}}</p>
35-
{{else if .IsPasswordInvalid}}
36-
<p>{{ctx.Locale.Tr "auth.invalid_password"}}</p>
37-
{{else if .ManualActivationOnly}}
38-
<p class="center">{{ctx.Locale.Tr "auth.manual_activation_only"}}</p>
39-
{{else}}
40-
<p>{{ctx.Locale.Tr "auth.has_unconfirmed_mail" (.SignedUser.Name|Escape) (.SignedUser.Email|Escape)}}</p>
41-
<div class="divider"></div>
42-
<div class="text right">
43-
<button class="ui primary button">{{ctx.Locale.Tr "auth.resend_mail"}}</button>
44-
</div>
45-
{{end}}
23+
<p>{{ctx.Locale.Tr "auth.has_unconfirmed_mail" .SignedUser.Name .SignedUser.Email}}</p>
24+
<div class="divider"></div>
25+
<div class="text right">
26+
<button class="ui primary button">{{ctx.Locale.Tr "auth.resend_mail"}}</button>
27+
</div>
4628
{{end}}
4729
</div>
4830
</form>

‎templates/user/auth/activate_prompt.tmpl

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
{{template "base/head" .}}
2+
<div role="main" aria-label="{{.Title}}" class="page-content user activate">
3+
<div class="ui middle very relaxed page grid">
4+
<div class="column">
5+
<h2 class="ui top attached header">
6+
{{ctx.Locale.Tr "auth.active_your_account"}}
7+
</h2>
8+
<div class="ui attached segment">
9+
{{template "base/alert" .}}
10+
<p>{{.ActivationPromptMessage}}</p>
11+
</div>
12+
</div>
13+
</div>
14+
</div>
15+
{{template "base/footer" .}}

‎tests/integration/signup_test.go

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"code.gitea.io/gitea/models/unittest"
1313
user_model "code.gitea.io/gitea/models/user"
1414
"code.gitea.io/gitea/modules/setting"
15+
"code.gitea.io/gitea/modules/test"
1516
"code.gitea.io/gitea/modules/translation"
1617
"code.gitea.io/gitea/tests"
1718

@@ -58,7 +59,7 @@ func TestSignupAsRestricted(t *testing.T) {
5859
assert.True(t, user2.IsRestricted)
5960
}
6061

61-
func TestSignupEmail(t *testing.T) {
62+
func TestSignupEmailValidation(t *testing.T) {
6263
defer tests.PrepareTestEnv(t)()
6364

6465
setting.Service.EnableCaptcha = false
@@ -91,3 +92,50 @@ func TestSignupEmail(t *testing.T) {
9192
}
9293
}
9394
}
95+
96+
func TestSignupEmailActive(t *testing.T) {
97+
defer tests.PrepareTestEnv(t)()
98+
defer test.MockVariableValue(&setting.Service.RegisterEmailConfirm, true)()
99+
100+
// try to sign up and send the activation email
101+
req := NewRequestWithValues(t, "POST", "/user/sign_up", map[string]string{
102+
"user_name": "test-user-1",
103+
"email": "[email protected]",
104+
"password": "password1",
105+
"retype": "password1",
106+
})
107+
resp := MakeRequest(t, req, http.StatusOK)
108+
assert.Contains(t, resp.Body.String(), `A new confirmation email has been sent to <b>[email protected]</b>.`)
109+
110+
// access "user/active" means trying to re-send the activation email
111+
session := loginUserWithPassword(t, "test-user-1", "password1")
112+
resp = session.MakeRequest(t, NewRequest(t, "GET", "/user/activate"), http.StatusOK)
113+
assert.Contains(t, resp.Body.String(), "You have already requested an activation email recently")
114+
115+
// access "user/active" with a valid activation code, then get the "verify password" page
116+
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "test-user-1"})
117+
activationCode := user.GenerateEmailActivateCode(user.Email)
118+
resp = session.MakeRequest(t, NewRequest(t, "GET", "/user/activate?code="+activationCode), http.StatusOK)
119+
assert.Contains(t, resp.Body.String(), `<input id="verify-password"`)
120+
121+
// try to use a wrong password, it should fail
122+
req = NewRequestWithValues(t, "POST", "/user/activate", map[string]string{
123+
"code": activationCode,
124+
"password": "password-wrong",
125+
})
126+
resp = session.MakeRequest(t, req, http.StatusOK)
127+
assert.Contains(t, resp.Body.String(), `Your password does not match`)
128+
assert.Contains(t, resp.Body.String(), `<input id="verify-password"`)
129+
user = unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "test-user-1"})
130+
assert.False(t, user.IsActive)
131+
132+
// then use a correct password, the user should be activated
133+
req = NewRequestWithValues(t, "POST", "/user/activate", map[string]string{
134+
"code": activationCode,
135+
"password": "password1",
136+
})
137+
resp = session.MakeRequest(t, req, http.StatusSeeOther)
138+
assert.Equal(t, "/", test.RedirectURL(resp))
139+
user = unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "test-user-1"})
140+
assert.True(t, user.IsActive)
141+
}

0 commit comments

Comments
 (0)