Skip to content

Allow ENABLE_OPENID_SIGNUP to depend on DISABLE_REGISTRATION #1369

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Mar 29, 2017
Merged

Allow ENABLE_OPENID_SIGNUP to depend on DISABLE_REGISTRATION #1369

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
721d88e
Allow ENABLE_OPENID_SIGNUP to depend on DISABLE_REGISTRATION
strk Mar 22, 2017
797268d
Move OpenID settings under Service object
strk Mar 22, 2017
a8ca869
Show OpenID SignUp and SignIn status in admin panel / configuration
strk Mar 22, 2017
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
4 changes: 2 additions & 2 deletions cmd/web.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ func runWeb(ctx *cli.Context) error {
m.Group("/user", func() {
m.Get("/login", user.SignIn)
m.Post("/login", bindIgnErr(auth.SignInForm{}), user.SignInPost)
if setting.EnableOpenIDSignIn {
if setting.Service.EnableOpenIDSignIn {
m.Combo("/login/openid").
Get(user.SignInOpenID).
Post(bindIgnErr(auth.SignInOpenIDForm{}), user.SignInOpenIDPost)
Expand Down Expand Up @@ -243,7 +243,7 @@ func runWeb(ctx *cli.Context) error {
m.Post("/email/delete", user.DeleteEmail)
m.Get("/password", user.SettingsPassword)
m.Post("/password", bindIgnErr(auth.ChangePasswordForm{}), user.SettingsPasswordPost)
if setting.EnableOpenIDSignIn {
if setting.Service.EnableOpenIDSignIn {
m.Group("/openid", func() {
m.Combo("").Get(user.SettingsOpenID).
Post(bindIgnErr(auth.AddOpenIDForm{}), user.SettingsOpenIDPost)
Expand Down
3 changes: 2 additions & 1 deletion conf/app.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,8 @@ IMPORT_LOCAL_PATHS = false
; Whether to allow signin in via OpenID
ENABLE_OPENID_SIGNIN = true
; Whether to allow registering via OpenID
ENABLE_OPENID_SIGNUP = true
; Do not include to rely on DISABLE_REGISTRATION setting
;ENABLE_OPENID_SIGNUP = true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we not default this to %(DISABLE_REGISTRATION) ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would only work if it's renamed to DISABLE_OPENID_SIGNUP, which kinda makes sense anyhow.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uhm, I didn't know about %(DISABLE_REGISTRATION) - would that use the value of DISABLE_REGISTRATION found under custom dir too ?

Renaming a variable would be a compatibility break, then we'd need to migrate config files ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Migrating variablenames could be done by removing the old default from conf/app.ini but load it from custom/conf/app.ini if it exists. then cfg = oldVar; if exists(newVar) { cfg = newVar } in LoadConfig. Maybe throw a deprication warning. So no migration needed. And would make the config defautls easier. Since ENABLE_OPENID_SIGNUP = ! DISABLE_REGISTRATION which I don't think you can do in ini format 😒

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are migration scripts allowed to modify stuff under custom/ ?
In general, is Gitea binary supposed to have write privilege in that dir ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bkcsoft I just realized it doesn't take write access to custom/ to do what you're asking, which is basically still support the old variable in custom if present. Sounds a bit complex though, is that still worth the advantage being the possibility to write DISABLE_OPENID_REGISTRATION = %DISABLE_REGISTRATION in custom/conf/app.ini ?

; Allowed URI patterns (POSIX regexp).
; Space separated.
; Only these would be allowed if non-blank.
Expand Down
2 changes: 1 addition & 1 deletion modules/context/context.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ func Contexter() macaron.Handler {
ctx.Data["ShowRegistrationButton"] = setting.Service.ShowRegistrationButton
ctx.Data["ShowFooterBranding"] = setting.ShowFooterBranding
ctx.Data["ShowFooterVersion"] = setting.ShowFooterVersion
ctx.Data["EnableOpenIDSignIn"] = setting.EnableOpenIDSignIn
ctx.Data["EnableOpenIDSignIn"] = setting.Service.EnableOpenIDSignIn

c.Map(ctx)
}
Expand Down
50 changes: 26 additions & 24 deletions modules/setting/setting.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,12 +121,6 @@ var (
MinPasswordLength int
ImportLocalPaths bool

// OpenID settings
EnableOpenIDSignIn bool
EnableOpenIDSignUp bool
OpenIDWhitelist []*regexp.Regexp
OpenIDBlacklist []*regexp.Regexp

// Database settings
UseSQLite3 bool
UseMySQL bool
Expand Down Expand Up @@ -758,24 +752,6 @@ please consider changing to GITEA_CUSTOM`)
MinPasswordLength = sec.Key("MIN_PASSWORD_LENGTH").MustInt(6)
ImportLocalPaths = sec.Key("IMPORT_LOCAL_PATHS").MustBool(false)

sec = Cfg.Section("openid")
EnableOpenIDSignIn = sec.Key("ENABLE_OPENID_SIGNIN").MustBool(true)
EnableOpenIDSignUp = sec.Key("ENABLE_OPENID_SIGNUP").MustBool(true)
pats := sec.Key("WHITELISTED_URIS").Strings(" ")
if len(pats) != 0 {
OpenIDWhitelist = make([]*regexp.Regexp, len(pats))
for i, p := range pats {
OpenIDWhitelist[i] = regexp.MustCompilePOSIX(p)
}
}
pats = sec.Key("BLACKLISTED_URIS").Strings(" ")
if len(pats) != 0 {
OpenIDBlacklist = make([]*regexp.Regexp, len(pats))
for i, p := range pats {
OpenIDBlacklist[i] = regexp.MustCompilePOSIX(p)
}
}

sec = Cfg.Section("attachment")
AttachmentPath = sec.Key("PATH").MustString(path.Join(AppDataPath, "attachments"))
if !filepath.IsAbs(AttachmentPath) {
Expand Down Expand Up @@ -939,6 +915,13 @@ var Service struct {
EnableCaptcha bool
DefaultKeepEmailPrivate bool
NoReplyAddress string

// OpenID settings
EnableOpenIDSignIn bool
EnableOpenIDSignUp bool
OpenIDWhitelist []*regexp.Regexp
OpenIDBlacklist []*regexp.Regexp

}

func newService() {
Expand All @@ -953,6 +936,25 @@ func newService() {
Service.EnableCaptcha = sec.Key("ENABLE_CAPTCHA").MustBool()
Service.DefaultKeepEmailPrivate = sec.Key("DEFAULT_KEEP_EMAIL_PRIVATE").MustBool()
Service.NoReplyAddress = sec.Key("NO_REPLY_ADDRESS").MustString("noreply.example.org")

sec = Cfg.Section("openid")
Service.EnableOpenIDSignIn = sec.Key("ENABLE_OPENID_SIGNIN").MustBool(true)
Service.EnableOpenIDSignUp = sec.Key("ENABLE_OPENID_SIGNUP").MustBool(!Service.DisableRegistration)
pats := sec.Key("WHITELISTED_URIS").Strings(" ")
if len(pats) != 0 {
Service.OpenIDWhitelist = make([]*regexp.Regexp, len(pats))
for i, p := range pats {
Service.OpenIDWhitelist[i] = regexp.MustCompilePOSIX(p)
}
}
pats = sec.Key("BLACKLISTED_URIS").Strings(" ")
if len(pats) != 0 {
Service.OpenIDBlacklist = make([]*regexp.Regexp, len(pats))
for i, p := range pats {
Service.OpenIDBlacklist[i] = regexp.MustCompilePOSIX(p)
}
}

}

var logLevels = map[string]string{
Expand Down
2 changes: 2 additions & 0 deletions options/locale/locale_en-US.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1209,6 +1209,8 @@ config.db_path_helper = (for "sqlite3" and "tidb")
config.service_config = Service Configuration
config.register_email_confirm = Require Email Confirmation
config.disable_register = Disable Registration
config.enable_openid_signup = Enable Registration via OpenID
config.enable_openid_signin = Enable OpenID Sign In
config.show_registration_button = Show Register Button
config.require_sign_in_view = Require Sign In View
config.mail_notify = Mail Notification
Expand Down
20 changes: 10 additions & 10 deletions routers/user/auth_openid.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ func allowedOpenIDURI(uri string) (err error) {

// In case a Whitelist is present, URI must be in it
// in order to be accepted
if len(setting.OpenIDWhitelist) != 0 {
for _, pat := range setting.OpenIDWhitelist {
if len(setting.Service.OpenIDWhitelist) != 0 {
for _, pat := range setting.Service.OpenIDWhitelist {
if pat.MatchString(uri) {
return nil // pass
}
Expand All @@ -79,7 +79,7 @@ func allowedOpenIDURI(uri string) (err error) {
}

// A blacklist match expliclty forbids
for _, pat := range setting.OpenIDBlacklist {
for _, pat := range setting.Service.OpenIDBlacklist {
if pat.MatchString(uri) {
return fmt.Errorf("URI forbidden by blacklist")
}
Expand Down Expand Up @@ -231,7 +231,7 @@ func signInOpenIDVerify(ctx *context.Context) {

ctx.Session.Set("openid_determined_username", nickname)

if u != nil || !setting.EnableOpenIDSignUp {
if u != nil || !setting.Service.EnableOpenIDSignUp {
ctx.Redirect(setting.AppSubURL + "/user/openid/connect")
} else {
ctx.Redirect(setting.AppSubURL + "/user/openid/register")
Expand All @@ -248,7 +248,7 @@ func ConnectOpenID(ctx *context.Context) {
ctx.Data["Title"] = "OpenID connect"
ctx.Data["PageIsSignIn"] = true
ctx.Data["PageIsOpenIDConnect"] = true
ctx.Data["EnableOpenIDSignUp"] = setting.EnableOpenIDSignUp
ctx.Data["EnableOpenIDSignUp"] = setting.Service.EnableOpenIDSignUp
ctx.Data["OpenID"] = oid
userName, _ := ctx.Session.Get("openid_determined_username").(string)
if userName != "" {
Expand All @@ -267,7 +267,7 @@ func ConnectOpenIDPost(ctx *context.Context, form auth.ConnectOpenIDForm) {
ctx.Data["Title"] = "OpenID connect"
ctx.Data["PageIsSignIn"] = true
ctx.Data["PageIsOpenIDConnect"] = true
ctx.Data["EnableOpenIDSignUp"] = setting.EnableOpenIDSignUp
ctx.Data["EnableOpenIDSignUp"] = setting.Service.EnableOpenIDSignUp
ctx.Data["OpenID"] = oid

u, err := models.UserSignIn(form.UserName, form.Password)
Expand Down Expand Up @@ -300,7 +300,7 @@ func ConnectOpenIDPost(ctx *context.Context, form auth.ConnectOpenIDForm) {

// RegisterOpenID shows a form to create a new user authenticated via an OpenID URI
func RegisterOpenID(ctx *context.Context) {
if !setting.EnableOpenIDSignUp {
if !setting.Service.EnableOpenIDSignUp {
ctx.Error(403)
return
}
Expand All @@ -312,7 +312,7 @@ func RegisterOpenID(ctx *context.Context) {
ctx.Data["Title"] = "OpenID signup"
ctx.Data["PageIsSignIn"] = true
ctx.Data["PageIsOpenIDRegister"] = true
ctx.Data["EnableOpenIDSignUp"] = setting.EnableOpenIDSignUp
ctx.Data["EnableOpenIDSignUp"] = setting.Service.EnableOpenIDSignUp
ctx.Data["EnableCaptcha"] = setting.Service.EnableCaptcha
ctx.Data["OpenID"] = oid
userName, _ := ctx.Session.Get("openid_determined_username").(string)
Expand All @@ -328,7 +328,7 @@ func RegisterOpenID(ctx *context.Context) {

// RegisterOpenIDPost handles submission of a form to create a new user authenticated via an OpenID URI
func RegisterOpenIDPost(ctx *context.Context, cpt *captcha.Captcha, form auth.SignUpOpenIDForm) {
if !setting.EnableOpenIDSignUp {
if !setting.Service.EnableOpenIDSignUp {
ctx.Error(403)
return
}
Expand All @@ -341,7 +341,7 @@ func RegisterOpenIDPost(ctx *context.Context, cpt *captcha.Captcha, form auth.Si
ctx.Data["Title"] = "OpenID signup"
ctx.Data["PageIsSignIn"] = true
ctx.Data["PageIsOpenIDRegister"] = true
ctx.Data["EnableOpenIDSignUp"] = setting.EnableOpenIDSignUp
ctx.Data["EnableOpenIDSignUp"] = setting.Service.EnableOpenIDSignUp
ctx.Data["EnableCaptcha"] = setting.Service.EnableCaptcha
ctx.Data["OpenID"] = oid

Expand Down
4 changes: 4 additions & 0 deletions templates/admin/config.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,10 @@
<dd><i class="fa fa{{if .Service.DisableRegistration}}-check{{end}}-square-o"></i></dd>
<dt>{{.i18n.Tr "admin.config.show_registration_button"}}</dt>
<dd><i class="fa fa{{if .Service.ShowRegistrationButton}}-check{{end}}-square-o"></i></dd>
<dt>{{.i18n.Tr "admin.config.enable_openid_signup"}}</dt>
<dd><i class="fa fa{{if .Service.EnableOpenIDSignUp}}-check{{end}}-square-o"></i></dd>
<dt>{{.i18n.Tr "admin.config.enable_openid_signin"}}</dt>
<dd><i class="fa fa{{if .Service.EnableOpenIDSignIn}}-check{{end}}-square-o"></i></dd>
<dt>{{.i18n.Tr "admin.config.require_sign_in_view"}}</dt>
<dd><i class="fa fa{{if .Service.RequireSignInView}}-check{{end}}-square-o"></i></dd>
<dt>{{.i18n.Tr "admin.config.mail_notify"}}</dt>
Expand Down