Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make captcha and password optional for external accounts #6606

Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion docs/content/doc/advanced/config-cheat-sheet.en-us.md
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,9 @@ Values containing `#` or `;` must be quoted using `` ` `` or `"""`.
Requires `Mailer` to be enabled.
- `DISABLE_REGISTRATION`: **false**: Disable registration, after which only admin can create
accounts for users.
- `REQUIRE_EXTERNAL_REGISTRATION_PASSWORD`: **false**: Enable this to force externally created
accounts (via GitHub, OpenID Connect, etc) to create a password. Warning: enabling this will
decrease security, so you should only enable it if you know what you're doing.
- `REQUIRE_SIGNIN_VIEW`: **false**: Enable this to force users to log in to view any page.
- `ENABLE_NOTIFY_MAIL`: **false**: Enable this to send e-mail to watchers of a repository when
something happens, like creating issues. Requires `Mailer` to be enabled.
Expand All @@ -225,6 +228,8 @@ Values containing `#` or `;` must be quoted using `` ` `` or `"""`.
- `ENABLE_REVERSE_PROXY_EMAIL`: **false**: Enable this to allow to auto-registration with a
provided email rather than a generated email.
- `ENABLE_CAPTCHA`: **false**: Enable this to use captcha validation for registration.
- `REQUIRE_EXTERNAL_REGISTRATION_CAPTCHA`: **false**: Enable this to force captcha validation
coolaj86 marked this conversation as resolved.
Show resolved Hide resolved
even for External Accounts (i.e. GitHub, OpenID Connect, etc). You must `ENABLE_CAPTCHA` also.
- `CAPTCHA_TYPE`: **image**: \[image, recaptcha\]
- `RECAPTCHA_SECRET`: **""**: Go to https://www.google.com/recaptcha/admin to get a secret for recaptcha.
- `RECAPTCHA_SITEKEY`: **""**: Go to https://www.google.com/recaptcha/admin to get a sitekey for recaptcha.
Expand Down Expand Up @@ -419,7 +424,7 @@ NB: You must `REDIRECT_MACARON_LOG` and have `DISABLE_ROUTER_LOG` set to `false`

## Metrics (`metrics`)

- `ENABLED`: **false**: Enables /metrics endpoint for prometheus.
- `ENABLED`: **false**: Enables /metrics endpoint for prometheus.
- `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`.

## API (`api`)
Expand Down
3 changes: 2 additions & 1 deletion modules/auth/user_form.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func (f *InstallForm) Validate(ctx *macaron.Context, errs binding.Errors) bindin
type RegisterForm struct {
UserName string `binding:"Required;AlphaDashDot;MaxSize(40)"`
Email string `binding:"Required;Email;MaxSize(254)"`
Password string `binding:"Required;MaxSize(255)"`
Password string `binding:"MaxSize(255)"`
techknowlogick marked this conversation as resolved.
Show resolved Hide resolved
Retype string
GRecaptchaResponse string `form:"g-recaptcha-response"`
}
Expand Down Expand Up @@ -129,6 +129,7 @@ func (f *MustChangePasswordForm) Validate(ctx *macaron.Context, errs binding.Err
// SignInForm form for signing in with user/password
type SignInForm struct {
UserName string `binding:"Required;MaxSize(254)"`
// TODO remove required from password for SecondFactorAuthentication
Password string `binding:"Required;MaxSize(255)"`
Remember bool
}
Expand Down
4 changes: 4 additions & 0 deletions modules/setting/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ var Service struct {
EnableReverseProxyAutoRegister bool
EnableReverseProxyEmail bool
EnableCaptcha bool
RequireExternalRegistrationCaptcha bool
RequireExternalRegistrationPassword bool
CaptchaType string
RecaptchaSecret string
RecaptchaSitekey string
Expand Down Expand Up @@ -61,6 +63,8 @@ func newService() {
Service.EnableReverseProxyAutoRegister = sec.Key("ENABLE_REVERSE_PROXY_AUTO_REGISTRATION").MustBool()
Service.EnableReverseProxyEmail = sec.Key("ENABLE_REVERSE_PROXY_EMAIL").MustBool()
Service.EnableCaptcha = sec.Key("ENABLE_CAPTCHA").MustBool(false)
Service.RequireExternalRegistrationCaptcha = sec.Key("REQUIRE_EXTERNAL_REGISTRATION_CAPTCHA").MustBool(Service.EnableCaptcha)
Service.RequireExternalRegistrationPassword = sec.Key("REQUIRE_EXTERNAL_REGISTRATION_PASSWORD").MustBool()
Service.CaptchaType = sec.Key("CAPTCHA_TYPE").MustString(ImageCaptcha)
Service.RecaptchaSecret = sec.Key("RECAPTCHA_SECRET").MustString("")
Service.RecaptchaSitekey = sec.Key("RECAPTCHA_SITEKEY").MustString("")
Expand Down
82 changes: 56 additions & 26 deletions routers/user/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
package user

import (
"crypto/rand"
"encoding/hex"
"errors"
"fmt"
"net/http"
Expand Down Expand Up @@ -697,9 +699,10 @@ func oAuth2UserLoginCallback(loginSource *models.LoginSource, request *http.Requ

// LinkAccount shows the page where the user can decide to login or create a new account
func LinkAccount(ctx *context.Context) {
ctx.Data["DisablePassword"] = !setting.Service.RequireExternalRegistrationCaptcha || setting.Service.AllowOnlyExternalRegistration
ctx.Data["Title"] = ctx.Tr("link_account")
ctx.Data["LinkAccountMode"] = true
ctx.Data["EnableCaptcha"] = setting.Service.EnableCaptcha
ctx.Data["EnableCaptcha"] = setting.Service.EnableCaptcha && setting.Service.RequireExternalRegistrationCaptcha
ctx.Data["CaptchaType"] = setting.Service.CaptchaType
ctx.Data["RecaptchaURL"] = setting.Service.RecaptchaURL
ctx.Data["RecaptchaSitekey"] = setting.Service.RecaptchaSitekey
Expand Down Expand Up @@ -746,10 +749,11 @@ func LinkAccount(ctx *context.Context) {

// LinkAccountPostSignIn handle the coupling of external account with another account using signIn
func LinkAccountPostSignIn(ctx *context.Context, signInForm auth.SignInForm) {
ctx.Data["DisablePassword"] = setting.Service.AllowOnlyExternalRegistration
ctx.Data["Title"] = ctx.Tr("link_account")
ctx.Data["LinkAccountMode"] = true
ctx.Data["LinkAccountModeSignIn"] = true
ctx.Data["EnableCaptcha"] = setting.Service.EnableCaptcha
ctx.Data["EnableCaptcha"] = setting.Service.EnableCaptcha && setting.Service.RequireExternalRegistrationCaptcha
ctx.Data["RecaptchaURL"] = setting.Service.RecaptchaURL
ctx.Data["CaptchaType"] = setting.Service.CaptchaType
ctx.Data["RecaptchaSitekey"] = setting.Service.RecaptchaSitekey
Expand Down Expand Up @@ -824,10 +828,13 @@ func LinkAccountPostSignIn(ctx *context.Context, signInForm auth.SignInForm) {

// LinkAccountPostRegister handle the creation of a new account for an external account using signUp
func LinkAccountPostRegister(ctx *context.Context, cpt *captcha.Captcha, form auth.RegisterForm) {
// TODO Make insecure passwords optional for local accounts also,
// once email-based Second-Factor Auth is available
ctx.Data["DisablePassword"] = !setting.Service.RequireExternalRegistrationCaptcha || setting.Service.AllowOnlyExternalRegistration
ctx.Data["Title"] = ctx.Tr("link_account")
ctx.Data["LinkAccountMode"] = true
ctx.Data["LinkAccountModeRegister"] = true
ctx.Data["EnableCaptcha"] = setting.Service.EnableCaptcha
ctx.Data["EnableCaptcha"] = setting.Service.EnableCaptcha && setting.Service.RequireExternalRegistrationCaptcha
ctx.Data["RecaptchaURL"] = setting.Service.RecaptchaURL
ctx.Data["CaptchaType"] = setting.Service.CaptchaType
ctx.Data["RecaptchaSitekey"] = setting.Service.RecaptchaSitekey
Expand All @@ -854,30 +861,49 @@ func LinkAccountPostRegister(ctx *context.Context, cpt *captcha.Captcha, form au
return
}

if setting.Service.EnableCaptcha && setting.Service.CaptchaType == setting.ImageCaptcha && !cpt.VerifyReq(ctx.Req) {
ctx.Data["Err_Captcha"] = true
ctx.RenderWithErr(ctx.Tr("form.captcha_incorrect"), tplLinkAccount, &form)
return
}
if setting.Service.EnableCaptcha && setting.Service.RequireExternalRegistrationCaptcha {
var valid bool
switch setting.Service.CaptchaType {
case setting.ImageCaptcha:
valid = cpt.VerifyReq(ctx.Req)
case setting.ReCaptcha:
valid, _ = recaptcha.Verify(form.GRecaptchaResponse)
default:
ctx.ServerError("Unknown Captcha Type", fmt.Errorf("Unknown Captcha Type: %s", setting.Service.CaptchaType))
return
}

if setting.Service.EnableCaptcha && setting.Service.CaptchaType == setting.ReCaptcha {
valid, _ := recaptcha.Verify(form.GRecaptchaResponse)
if !valid {
ctx.Data["Err_Captcha"] = true
ctx.RenderWithErr(ctx.Tr("form.captcha_incorrect"), tplLinkAccount, &form)
return
}
}

if (len(strings.TrimSpace(form.Password)) > 0 || len(strings.TrimSpace(form.Retype)) > 0) && form.Password != form.Retype {
ctx.Data["Err_Password"] = true
ctx.RenderWithErr(ctx.Tr("form.password_not_match"), tplLinkAccount, &form)
return
}
if len(strings.TrimSpace(form.Password)) > 0 && len(form.Password) < setting.MinPasswordLength {
ctx.Data["Err_Password"] = true
ctx.RenderWithErr(ctx.Tr("auth.password_too_short", setting.MinPasswordLength), tplLinkAccount, &form)
return
if setting.Service.AllowOnlyExternalRegistration || !setting.Service.RequireExternalRegistrationPassword {
// Generating a random password a stop-gap shim to get around the password requirement.
// Eventually the database should be changed to indicate "Second Factor"-enabled accounts
// (accounts that do not introduce the security vulnerabilities of a password).
// If a user decides to circumvent second-factor security, and purposefully create a password,
// they can still do so using the "Recover Account" option.
zeripath marked this conversation as resolved.
Show resolved Hide resolved
bytes := make([]byte, 16)
_, err := rand.Read(bytes)
if nil != err {
ctx.ServerError("CreateUser", err)
return
}
form.Password = hex.EncodeToString(bytes)
} else {
if (len(strings.TrimSpace(form.Password)) > 0 || len(strings.TrimSpace(form.Retype)) > 0) && form.Password != form.Retype {
ctx.Data["Err_Password"] = true
ctx.RenderWithErr(ctx.Tr("form.password_not_match"), tplLinkAccount, &form)
return
}
if len(strings.TrimSpace(form.Password)) > 0 && len(form.Password) < setting.MinPasswordLength {
ctx.Data["Err_Password"] = true
ctx.RenderWithErr(ctx.Tr("auth.password_too_short", setting.MinPasswordLength), tplLinkAccount, &form)
return
}
}

loginSource, err := models.GetActiveOAuth2LoginSourceByName(gothUser.(goth.User).Provider)
Expand Down Expand Up @@ -1000,14 +1026,18 @@ func SignUpPost(ctx *context.Context, cpt *captcha.Captcha, form auth.RegisterFo
return
}

if setting.Service.EnableCaptcha && setting.Service.CaptchaType == setting.ImageCaptcha && !cpt.VerifyReq(ctx.Req) {
ctx.Data["Err_Captcha"] = true
ctx.RenderWithErr(ctx.Tr("form.captcha_incorrect"), tplSignUp, &form)
return
}
if setting.Service.EnableCaptcha {
var valid bool
switch setting.Service.CaptchaType {
case setting.ImageCaptcha:
valid = cpt.VerifyReq(ctx.Req)
case setting.ReCaptcha:
valid, _ = recaptcha.Verify(form.GRecaptchaResponse)
default:
ctx.ServerError("Unknown Captcha Type", fmt.Errorf("Unknown Captcha Type: %s", setting.Service.CaptchaType))
return
}

if setting.Service.EnableCaptcha && setting.Service.CaptchaType == setting.ReCaptcha {
valid, _ := recaptcha.Verify(form.GRecaptchaResponse)
if !valid {
ctx.Data["Err_Captcha"] = true
ctx.RenderWithErr(ctx.Tr("form.captcha_incorrect"), tplSignUp, &form)
Expand Down
26 changes: 15 additions & 11 deletions routers/user/auth_openid.go
Original file line number Diff line number Diff line change
Expand Up @@ -357,19 +357,23 @@ func RegisterOpenIDPost(ctx *context.Context, cpt *captcha.Captcha, form auth.Si
ctx.Data["RecaptchaSitekey"] = setting.Service.RecaptchaSitekey
ctx.Data["OpenID"] = oid

if setting.Service.EnableCaptcha && setting.Service.CaptchaType == setting.ImageCaptcha && !cpt.VerifyReq(ctx.Req) {
ctx.Data["Err_Captcha"] = true
ctx.RenderWithErr(ctx.Tr("form.captcha_incorrect"), tplSignUpOID, &form)
return
}

if setting.Service.EnableCaptcha && setting.Service.CaptchaType == setting.ReCaptcha {
err := ctx.Req.ParseForm()
if err != nil {
ctx.ServerError("", err)
if setting.Service.EnableCaptcha {
var valid bool
switch setting.Service.CaptchaType {
case setting.ImageCaptcha:
valid = cpt.VerifyReq(ctx.Req)
case setting.ReCaptcha:
err := ctx.Req.ParseForm()
if err != nil {
ctx.ServerError("", err)
return
}
valid, _ = recaptcha.Verify(form.GRecaptchaResponse)
default:
ctx.ServerError("Unknown Captcha Type", fmt.Errorf("Unknown Captcha Type: %s", setting.Service.CaptchaType))
return
}
valid, _ := recaptcha.Verify(form.GRecaptchaResponse)

if !valid {
ctx.Data["Err_Captcha"] = true
ctx.RenderWithErr(ctx.Tr("form.captcha_incorrect"), tplSignUpOID, &form)
Expand Down
2 changes: 2 additions & 0 deletions templates/user/auth/signin_inner.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,12 @@
<label for="user_name">{{.i18n.Tr "home.uname_holder"}}</label>
<input id="user_name" name="user_name" value="{{.user_name}}" autofocus required>
</div>
{{if not .DisablePassword}}
<div class="required inline field {{if and (.Err_Password) (or (not .LinkAccountMode) (and .LinkAccountMode .LinkAccountModeSignIn))}}error{{end}}">
<label for="password">{{.i18n.Tr "password"}}</label>
<input id="password" name="password" type="password" value="{{.password}}" autocomplete="off" required>
</div>
{{end}}
{{if not .LinkAccountMode}}
<div class="inline field">
<label></label>
Expand Down
19 changes: 11 additions & 8 deletions templates/user/auth/signup_inner.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,17 @@
<label for="email">{{.i18n.Tr "email"}}</label>
<input id="email" name="email" type="email" value="{{.email}}" required>
</div>
<div class="required inline field {{if and (.Err_Password) (or (not .LinkAccountMode) (and .LinkAccountMode .LinkAccountModeRegister))}}error{{end}}">
<label for="password">{{.i18n.Tr "password"}}</label>
<input id="password" name="password" type="password" value="{{.password}}" autocomplete="off" required>
</div>
<div class="required inline field {{if and (.Err_Password) (or (not .LinkAccountMode) (and .LinkAccountMode .LinkAccountModeRegister))}}error{{end}}">
<label for="retype">{{.i18n.Tr "re_type"}}</label>
<input id="retype" name="retype" type="password" value="{{.retype}}" autocomplete="off" required>
</div>

{{if not .DisablePassword}}
<div class="required inline field {{if and (.Err_Password) (or (not .LinkAccountMode) (and .LinkAccountMode .LinkAccountModeRegister))}}error{{end}}">
<label for="password">{{.i18n.Tr "password"}}</label>
<input id="password" name="password" type="password" value="{{.password}}" autocomplete="off" required>
</div>
<div class="required inline field {{if and (.Err_Password) (or (not .LinkAccountMode) (and .LinkAccountMode .LinkAccountModeRegister))}}error{{end}}">
<label for="retype">{{.i18n.Tr "re_type"}}</label>
<input id="retype" name="retype" type="password" value="{{.retype}}" autocomplete="off" required>
</div>
{{end}}
{{if and .EnableCaptcha (eq .CaptchaType "image")}}
<div class="inline field">
<label></label>
Expand Down