Skip to content

Commit 62d6127

Browse files
AJ ONealtechknowlogick
AJ ONeal
authored and
techknowlogick
committed
Make captcha and password optional for external accounts (#6606)
1 parent 337d691 commit 62d6127

File tree

7 files changed

+88
-47
lines changed

7 files changed

+88
-47
lines changed

‎docs/content/doc/advanced/config-cheat-sheet.en-us.md

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,9 @@ Values containing `#` or `;` must be quoted using `` ` `` or `"""`.
216216
Requires `Mailer` to be enabled.
217217
- `DISABLE_REGISTRATION`: **false**: Disable registration, after which only admin can create
218218
accounts for users.
219+
- `REQUIRE_EXTERNAL_REGISTRATION_PASSWORD`: **false**: Enable this to force externally created
220+
accounts (via GitHub, OpenID Connect, etc) to create a password. Warning: enabling this will
221+
decrease security, so you should only enable it if you know what you're doing.
219222
- `REQUIRE_SIGNIN_VIEW`: **false**: Enable this to force users to log in to view any page.
220223
- `ENABLE_NOTIFY_MAIL`: **false**: Enable this to send e-mail to watchers of a repository when
221224
something happens, like creating issues. Requires `Mailer` to be enabled.
@@ -225,6 +228,8 @@ Values containing `#` or `;` must be quoted using `` ` `` or `"""`.
225228
- `ENABLE_REVERSE_PROXY_EMAIL`: **false**: Enable this to allow to auto-registration with a
226229
provided email rather than a generated email.
227230
- `ENABLE_CAPTCHA`: **false**: Enable this to use captcha validation for registration.
231+
- `REQUIRE_EXTERNAL_REGISTRATION_CAPTCHA`: **false**: Enable this to force captcha validation
232+
even for External Accounts (i.e. GitHub, OpenID Connect, etc). You must `ENABLE_CAPTCHA` also.
228233
- `CAPTCHA_TYPE`: **image**: \[image, recaptcha\]
229234
- `RECAPTCHA_SECRET`: **""**: Go to https://www.google.com/recaptcha/admin to get a secret for recaptcha.
230235
- `RECAPTCHA_SITEKEY`: **""**: Go to https://www.google.com/recaptcha/admin to get a sitekey for recaptcha.
@@ -419,7 +424,7 @@ NB: You must `REDIRECT_MACARON_LOG` and have `DISABLE_ROUTER_LOG` set to `false`
419424

420425
## Metrics (`metrics`)
421426

422-
- `ENABLED`: **false**: Enables /metrics endpoint for prometheus.
427+
- `ENABLED`: **false**: Enables /metrics endpoint for prometheus.
423428
- `TOKEN`: **\<empty\>**: You need to specify the token, if you want to include in the authorization the metrics . The same token need to be used in prometheus parameters `bearer_token` or `bearer_token_file`.
424429

425430
## API (`api`)

‎modules/auth/user_form.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ func (f *InstallForm) Validate(ctx *macaron.Context, errs binding.Errors) bindin
7979
type RegisterForm struct {
8080
UserName string `binding:"Required;AlphaDashDot;MaxSize(40)"`
8181
Email string `binding:"Required;Email;MaxSize(254)"`
82-
Password string `binding:"Required;MaxSize(255)"`
82+
Password string `binding:"MaxSize(255)"`
8383
Retype string
8484
GRecaptchaResponse string `form:"g-recaptcha-response"`
8585
}
@@ -129,6 +129,7 @@ func (f *MustChangePasswordForm) Validate(ctx *macaron.Context, errs binding.Err
129129
// SignInForm form for signing in with user/password
130130
type SignInForm struct {
131131
UserName string `binding:"Required;MaxSize(254)"`
132+
// TODO remove required from password for SecondFactorAuthentication
132133
Password string `binding:"Required;MaxSize(255)"`
133134
Remember bool
134135
}

‎modules/setting/service.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ var Service struct {
2727
EnableReverseProxyAutoRegister bool
2828
EnableReverseProxyEmail bool
2929
EnableCaptcha bool
30+
RequireExternalRegistrationCaptcha bool
31+
RequireExternalRegistrationPassword bool
3032
CaptchaType string
3133
RecaptchaSecret string
3234
RecaptchaSitekey string
@@ -61,6 +63,8 @@ func newService() {
6163
Service.EnableReverseProxyAutoRegister = sec.Key("ENABLE_REVERSE_PROXY_AUTO_REGISTRATION").MustBool()
6264
Service.EnableReverseProxyEmail = sec.Key("ENABLE_REVERSE_PROXY_EMAIL").MustBool()
6365
Service.EnableCaptcha = sec.Key("ENABLE_CAPTCHA").MustBool(false)
66+
Service.RequireExternalRegistrationCaptcha = sec.Key("REQUIRE_EXTERNAL_REGISTRATION_CAPTCHA").MustBool(Service.EnableCaptcha)
67+
Service.RequireExternalRegistrationPassword = sec.Key("REQUIRE_EXTERNAL_REGISTRATION_PASSWORD").MustBool()
6468
Service.CaptchaType = sec.Key("CAPTCHA_TYPE").MustString(ImageCaptcha)
6569
Service.RecaptchaSecret = sec.Key("RECAPTCHA_SECRET").MustString("")
6670
Service.RecaptchaSitekey = sec.Key("RECAPTCHA_SITEKEY").MustString("")

‎routers/user/auth.go

Lines changed: 48 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -697,9 +697,10 @@ func oAuth2UserLoginCallback(loginSource *models.LoginSource, request *http.Requ
697697

698698
// LinkAccount shows the page where the user can decide to login or create a new account
699699
func LinkAccount(ctx *context.Context) {
700+
ctx.Data["DisablePassword"] = !setting.Service.RequireExternalRegistrationCaptcha || setting.Service.AllowOnlyExternalRegistration
700701
ctx.Data["Title"] = ctx.Tr("link_account")
701702
ctx.Data["LinkAccountMode"] = true
702-
ctx.Data["EnableCaptcha"] = setting.Service.EnableCaptcha
703+
ctx.Data["EnableCaptcha"] = setting.Service.EnableCaptcha && setting.Service.RequireExternalRegistrationCaptcha
703704
ctx.Data["CaptchaType"] = setting.Service.CaptchaType
704705
ctx.Data["RecaptchaURL"] = setting.Service.RecaptchaURL
705706
ctx.Data["RecaptchaSitekey"] = setting.Service.RecaptchaSitekey
@@ -746,10 +747,11 @@ func LinkAccount(ctx *context.Context) {
746747

747748
// LinkAccountPostSignIn handle the coupling of external account with another account using signIn
748749
func LinkAccountPostSignIn(ctx *context.Context, signInForm auth.SignInForm) {
750+
ctx.Data["DisablePassword"] = setting.Service.AllowOnlyExternalRegistration
749751
ctx.Data["Title"] = ctx.Tr("link_account")
750752
ctx.Data["LinkAccountMode"] = true
751753
ctx.Data["LinkAccountModeSignIn"] = true
752-
ctx.Data["EnableCaptcha"] = setting.Service.EnableCaptcha
754+
ctx.Data["EnableCaptcha"] = setting.Service.EnableCaptcha && setting.Service.RequireExternalRegistrationCaptcha
753755
ctx.Data["RecaptchaURL"] = setting.Service.RecaptchaURL
754756
ctx.Data["CaptchaType"] = setting.Service.CaptchaType
755757
ctx.Data["RecaptchaSitekey"] = setting.Service.RecaptchaSitekey
@@ -824,10 +826,13 @@ func LinkAccountPostSignIn(ctx *context.Context, signInForm auth.SignInForm) {
824826

825827
// LinkAccountPostRegister handle the creation of a new account for an external account using signUp
826828
func LinkAccountPostRegister(ctx *context.Context, cpt *captcha.Captcha, form auth.RegisterForm) {
829+
// TODO Make insecure passwords optional for local accounts also,
830+
// once email-based Second-Factor Auth is available
831+
ctx.Data["DisablePassword"] = !setting.Service.RequireExternalRegistrationCaptcha || setting.Service.AllowOnlyExternalRegistration
827832
ctx.Data["Title"] = ctx.Tr("link_account")
828833
ctx.Data["LinkAccountMode"] = true
829834
ctx.Data["LinkAccountModeRegister"] = true
830-
ctx.Data["EnableCaptcha"] = setting.Service.EnableCaptcha
835+
ctx.Data["EnableCaptcha"] = setting.Service.EnableCaptcha && setting.Service.RequireExternalRegistrationCaptcha
831836
ctx.Data["RecaptchaURL"] = setting.Service.RecaptchaURL
832837
ctx.Data["CaptchaType"] = setting.Service.CaptchaType
833838
ctx.Data["RecaptchaSitekey"] = setting.Service.RecaptchaSitekey
@@ -854,30 +859,43 @@ func LinkAccountPostRegister(ctx *context.Context, cpt *captcha.Captcha, form au
854859
return
855860
}
856861

857-
if setting.Service.EnableCaptcha && setting.Service.CaptchaType == setting.ImageCaptcha && !cpt.VerifyReq(ctx.Req) {
858-
ctx.Data["Err_Captcha"] = true
859-
ctx.RenderWithErr(ctx.Tr("form.captcha_incorrect"), tplLinkAccount, &form)
860-
return
861-
}
862+
if setting.Service.EnableCaptcha && setting.Service.RequireExternalRegistrationCaptcha {
863+
var valid bool
864+
switch setting.Service.CaptchaType {
865+
case setting.ImageCaptcha:
866+
valid = cpt.VerifyReq(ctx.Req)
867+
case setting.ReCaptcha:
868+
valid, _ = recaptcha.Verify(form.GRecaptchaResponse)
869+
default:
870+
ctx.ServerError("Unknown Captcha Type", fmt.Errorf("Unknown Captcha Type: %s", setting.Service.CaptchaType))
871+
return
872+
}
862873

863-
if setting.Service.EnableCaptcha && setting.Service.CaptchaType == setting.ReCaptcha {
864-
valid, _ := recaptcha.Verify(form.GRecaptchaResponse)
865874
if !valid {
866875
ctx.Data["Err_Captcha"] = true
867876
ctx.RenderWithErr(ctx.Tr("form.captcha_incorrect"), tplLinkAccount, &form)
868877
return
869878
}
870879
}
871880

872-
if (len(strings.TrimSpace(form.Password)) > 0 || len(strings.TrimSpace(form.Retype)) > 0) && form.Password != form.Retype {
873-
ctx.Data["Err_Password"] = true
874-
ctx.RenderWithErr(ctx.Tr("form.password_not_match"), tplLinkAccount, &form)
875-
return
876-
}
877-
if len(strings.TrimSpace(form.Password)) > 0 && len(form.Password) < setting.MinPasswordLength {
878-
ctx.Data["Err_Password"] = true
879-
ctx.RenderWithErr(ctx.Tr("auth.password_too_short", setting.MinPasswordLength), tplLinkAccount, &form)
880-
return
881+
if setting.Service.AllowOnlyExternalRegistration || !setting.Service.RequireExternalRegistrationPassword {
882+
// In models.User an empty password is classed as not set, so we set form.Password to empty.
883+
// Eventually the database should be changed to indicate "Second Factor"-enabled accounts
884+
// (accounts that do not introduce the security vulnerabilities of a password).
885+
// If a user decides to circumvent second-factor security, and purposefully create a password,
886+
// they can still do so using the "Recover Account" option.
887+
form.Password = ""
888+
} else {
889+
if (len(strings.TrimSpace(form.Password)) > 0 || len(strings.TrimSpace(form.Retype)) > 0) && form.Password != form.Retype {
890+
ctx.Data["Err_Password"] = true
891+
ctx.RenderWithErr(ctx.Tr("form.password_not_match"), tplLinkAccount, &form)
892+
return
893+
}
894+
if len(strings.TrimSpace(form.Password)) > 0 && len(form.Password) < setting.MinPasswordLength {
895+
ctx.Data["Err_Password"] = true
896+
ctx.RenderWithErr(ctx.Tr("auth.password_too_short", setting.MinPasswordLength), tplLinkAccount, &form)
897+
return
898+
}
881899
}
882900

883901
loginSource, err := models.GetActiveOAuth2LoginSourceByName(gothUser.(goth.User).Provider)
@@ -1000,14 +1018,18 @@ func SignUpPost(ctx *context.Context, cpt *captcha.Captcha, form auth.RegisterFo
10001018
return
10011019
}
10021020

1003-
if setting.Service.EnableCaptcha && setting.Service.CaptchaType == setting.ImageCaptcha && !cpt.VerifyReq(ctx.Req) {
1004-
ctx.Data["Err_Captcha"] = true
1005-
ctx.RenderWithErr(ctx.Tr("form.captcha_incorrect"), tplSignUp, &form)
1006-
return
1007-
}
1021+
if setting.Service.EnableCaptcha {
1022+
var valid bool
1023+
switch setting.Service.CaptchaType {
1024+
case setting.ImageCaptcha:
1025+
valid = cpt.VerifyReq(ctx.Req)
1026+
case setting.ReCaptcha:
1027+
valid, _ = recaptcha.Verify(form.GRecaptchaResponse)
1028+
default:
1029+
ctx.ServerError("Unknown Captcha Type", fmt.Errorf("Unknown Captcha Type: %s", setting.Service.CaptchaType))
1030+
return
1031+
}
10081032

1009-
if setting.Service.EnableCaptcha && setting.Service.CaptchaType == setting.ReCaptcha {
1010-
valid, _ := recaptcha.Verify(form.GRecaptchaResponse)
10111033
if !valid {
10121034
ctx.Data["Err_Captcha"] = true
10131035
ctx.RenderWithErr(ctx.Tr("form.captcha_incorrect"), tplSignUp, &form)

‎routers/user/auth_openid.go

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -357,19 +357,23 @@ func RegisterOpenIDPost(ctx *context.Context, cpt *captcha.Captcha, form auth.Si
357357
ctx.Data["RecaptchaSitekey"] = setting.Service.RecaptchaSitekey
358358
ctx.Data["OpenID"] = oid
359359

360-
if setting.Service.EnableCaptcha && setting.Service.CaptchaType == setting.ImageCaptcha && !cpt.VerifyReq(ctx.Req) {
361-
ctx.Data["Err_Captcha"] = true
362-
ctx.RenderWithErr(ctx.Tr("form.captcha_incorrect"), tplSignUpOID, &form)
363-
return
364-
}
365-
366-
if setting.Service.EnableCaptcha && setting.Service.CaptchaType == setting.ReCaptcha {
367-
err := ctx.Req.ParseForm()
368-
if err != nil {
369-
ctx.ServerError("", err)
360+
if setting.Service.EnableCaptcha {
361+
var valid bool
362+
switch setting.Service.CaptchaType {
363+
case setting.ImageCaptcha:
364+
valid = cpt.VerifyReq(ctx.Req)
365+
case setting.ReCaptcha:
366+
err := ctx.Req.ParseForm()
367+
if err != nil {
368+
ctx.ServerError("", err)
369+
return
370+
}
371+
valid, _ = recaptcha.Verify(form.GRecaptchaResponse)
372+
default:
373+
ctx.ServerError("Unknown Captcha Type", fmt.Errorf("Unknown Captcha Type: %s", setting.Service.CaptchaType))
370374
return
371375
}
372-
valid, _ := recaptcha.Verify(form.GRecaptchaResponse)
376+
373377
if !valid {
374378
ctx.Data["Err_Captcha"] = true
375379
ctx.RenderWithErr(ctx.Tr("form.captcha_incorrect"), tplSignUpOID, &form)

‎templates/user/auth/signin_inner.tmpl

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,12 @@
1515
<label for="user_name">{{.i18n.Tr "home.uname_holder"}}</label>
1616
<input id="user_name" name="user_name" value="{{.user_name}}" autofocus required>
1717
</div>
18+
{{if not .DisablePassword}}
1819
<div class="required inline field {{if and (.Err_Password) (or (not .LinkAccountMode) (and .LinkAccountMode .LinkAccountModeSignIn))}}error{{end}}">
1920
<label for="password">{{.i18n.Tr "password"}}</label>
2021
<input id="password" name="password" type="password" value="{{.password}}" autocomplete="off" required>
2122
</div>
23+
{{end}}
2224
{{if not .LinkAccountMode}}
2325
<div class="inline field">
2426
<label></label>

‎templates/user/auth/signup_inner.tmpl

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,17 @@
2525
<label for="email">{{.i18n.Tr "email"}}</label>
2626
<input id="email" name="email" type="email" value="{{.email}}" required>
2727
</div>
28-
<div class="required inline field {{if and (.Err_Password) (or (not .LinkAccountMode) (and .LinkAccountMode .LinkAccountModeRegister))}}error{{end}}">
29-
<label for="password">{{.i18n.Tr "password"}}</label>
30-
<input id="password" name="password" type="password" value="{{.password}}" autocomplete="off" required>
31-
</div>
32-
<div class="required inline field {{if and (.Err_Password) (or (not .LinkAccountMode) (and .LinkAccountMode .LinkAccountModeRegister))}}error{{end}}">
33-
<label for="retype">{{.i18n.Tr "re_type"}}</label>
34-
<input id="retype" name="retype" type="password" value="{{.retype}}" autocomplete="off" required>
35-
</div>
28+
29+
{{if not .DisablePassword}}
30+
<div class="required inline field {{if and (.Err_Password) (or (not .LinkAccountMode) (and .LinkAccountMode .LinkAccountModeRegister))}}error{{end}}">
31+
<label for="password">{{.i18n.Tr "password"}}</label>
32+
<input id="password" name="password" type="password" value="{{.password}}" autocomplete="off" required>
33+
</div>
34+
<div class="required inline field {{if and (.Err_Password) (or (not .LinkAccountMode) (and .LinkAccountMode .LinkAccountModeRegister))}}error{{end}}">
35+
<label for="retype">{{.i18n.Tr "re_type"}}</label>
36+
<input id="retype" name="retype" type="password" value="{{.retype}}" autocomplete="off" required>
37+
</div>
38+
{{end}}
3639
{{if and .EnableCaptcha (eq .CaptchaType "image")}}
3740
<div class="inline field">
3841
<label></label>

0 commit comments

Comments
 (0)