From ceaf11cbe329a9884f96fee64c0f88c9463e7dd1 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sat, 21 Aug 2021 16:18:18 +0100 Subject: [PATCH 1/2] Add setting to OAuth handlers to override local 2FA settings This PR adds a setting to OAuth and OpenID login sources to allow the source to override local 2FA requirements. Fix #13939 Signed-off-by: Andrew Thornton --- cmd/admin.go | 5 +++++ options/locale/locale_en-US.ini | 2 ++ routers/web/admin/auths.go | 1 + routers/web/user/auth.go | 20 ++++++++++++-------- services/auth/source/oauth2/source.go | 1 + services/forms/auth_form.go | 1 + templates/admin/auth/edit.tmpl | 7 +++++++ templates/admin/auth/source/oauth.tmpl | 7 +++++++ 8 files changed, 36 insertions(+), 8 deletions(-) diff --git a/cmd/admin.go b/cmd/admin.go index 94e78186c902f..ab0d3ce95056f 100644 --- a/cmd/admin.go +++ b/cmd/admin.go @@ -288,6 +288,10 @@ var ( Value: "", Usage: "Custom icon URL for OAuth2 login source", }, + cli.BoolFlag{ + Name: "override-local-2fa", + Usage: "Set to true to override local 2fa settings", + }, } microcmdAuthUpdateOauth = cli.Command{ @@ -616,6 +620,7 @@ func parseOAuth2Config(c *cli.Context) *oauth2.Source { OpenIDConnectAutoDiscoveryURL: c.String("auto-discover-url"), CustomURLMapping: customURLMapping, IconURL: c.String("icon-url"), + OverrideLocalTwoFA: c.Bool("override-local-2fa"), } } diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 4715afcd3e54d..ccee3104df50f 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -2448,6 +2448,8 @@ auths.oauth2_tokenURL = Token URL auths.oauth2_authURL = Authorize URL auths.oauth2_profileURL = Profile URL auths.oauth2_emailURL = Email URL +auths.override_local_two_fa = Override local 2FA +auths.override_local_two_fa_helper = Leaving unset means local users with 2FA set will still have to pass 2FA to log on auths.oauth2_tenant = Tenant auths.enable_auto_register = Enable Auto Registration auths.sspi_auto_create_users = Automatically create users diff --git a/routers/web/admin/auths.go b/routers/web/admin/auths.go index 342318e04e9b5..04143e3610615 100644 --- a/routers/web/admin/auths.go +++ b/routers/web/admin/auths.go @@ -181,6 +181,7 @@ func parseOAuth2Config(form forms.AuthenticationForm) *oauth2.Source { OpenIDConnectAutoDiscoveryURL: form.OpenIDConnectAutoDiscoveryURL, CustomURLMapping: customURLMapping, IconURL: form.Oauth2IconURL, + OverrideLocalTwoFA: form.OverrideLocalTwoFA, } } diff --git a/routers/web/user/auth.go b/routers/web/user/auth.go index 313a583004a57..9b8de29802940 100644 --- a/routers/web/user/auth.go +++ b/routers/web/user/auth.go @@ -574,7 +574,7 @@ func SignInOAuth(ctx *context.Context) { user, gothUser, err := oAuth2UserLoginCallback(loginSource, ctx.Req, ctx.Resp) if err == nil && user != nil { // we got the user without going through the whole OAuth2 authentication flow again - handleOAuth2SignIn(ctx, user, gothUser) + handleOAuth2SignIn(ctx, loginSource, user, gothUser) return } @@ -660,7 +660,7 @@ func SignInOAuthCallback(ctx *context.Context) { } } - handleOAuth2SignIn(ctx, u, gothUser) + handleOAuth2SignIn(ctx, loginSource, u, gothUser) } func getUserName(gothUser *goth.User) string { @@ -702,18 +702,22 @@ func updateAvatarIfNeed(url string, u *models.User) { } } -func handleOAuth2SignIn(ctx *context.Context, u *models.User, gothUser goth.User) { +func handleOAuth2SignIn(ctx *context.Context, source *models.LoginSource, u *models.User, gothUser goth.User) { updateAvatarIfNeed(gothUser.AvatarURL, u) - // If this user is enrolled in 2FA, we can't sign the user in just yet. - // Instead, redirect them to the 2FA authentication page. - _, err := models.GetTwoFactorByUID(u.ID) - if err != nil { - if !models.IsErrTwoFactorNotEnrolled(err) { + needs2FA := false + if !source.Cfg.(*oauth2.Source).OverrideLocalTwoFA { + _, err := models.GetTwoFactorByUID(u.ID) + if err != nil && !models.IsErrTwoFactorNotEnrolled(err) { ctx.ServerError("UserSignIn", err) return } + needs2FA = err == nil + } + // If this user is enrolled in 2FA and this source doesn't override it, + // we can't sign the user in just yet. Instead, redirect them to the 2FA authentication page. + if !needs2FA { if err := ctx.Session.Set("uid", u.ID); err != nil { log.Error("Error setting uid in session: %v", err) } diff --git a/services/auth/source/oauth2/source.go b/services/auth/source/oauth2/source.go index 40d8973b4b2ce..542a0b57aec3c 100644 --- a/services/auth/source/oauth2/source.go +++ b/services/auth/source/oauth2/source.go @@ -24,6 +24,7 @@ type Source struct { OpenIDConnectAutoDiscoveryURL string CustomURLMapping *CustomURLMapping IconURL string + OverrideLocalTwoFA bool // reference to the loginSource loginSource *models.LoginSource diff --git a/services/forms/auth_form.go b/services/forms/auth_form.go index b45ea6ea124f7..288b87b45b208 100644 --- a/services/forms/auth_form.go +++ b/services/forms/auth_form.go @@ -66,6 +66,7 @@ type AuthenticationForm struct { Oauth2EmailURL string Oauth2IconURL string Oauth2Tenant string + OverrideLocalTwoFA bool SSPIAutoCreateUsers bool SSPIAutoActivateUsers bool SSPIStripDomainNames bool diff --git a/templates/admin/auth/edit.tmpl b/templates/admin/auth/edit.tmpl index 109186a17821f..6e1a785908f55 100644 --- a/templates/admin/auth/edit.tmpl +++ b/templates/admin/auth/edit.tmpl @@ -255,6 +255,13 @@ +
+
+ + +

{{.i18n.Tr "admin.auths.override_local_two_fa_helper"}}

+
+
diff --git a/templates/admin/auth/source/oauth.tmpl b/templates/admin/auth/source/oauth.tmpl index b19fe3d42825d..07077471b235d 100644 --- a/templates/admin/auth/source/oauth.tmpl +++ b/templates/admin/auth/source/oauth.tmpl @@ -28,6 +28,13 @@
+
+
+ + +

{{.i18n.Tr "admin.auths.override_local_two_fa_helper"}}

+
+
From cb6b4c23abb6b4e77008b3df52c1337f6429d0ba Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sat, 4 Sep 2021 11:43:09 +0100 Subject: [PATCH 2/2] Change to Skip Local 2fa instead of Override Local 2fa Signed-off-by: Andrew Thornton --- cmd/admin.go | 6 +++--- options/locale/locale_en-US.ini | 4 ++-- routers/web/admin/auths.go | 2 +- routers/web/user/auth.go | 2 +- services/auth/source/oauth2/source.go | 2 +- services/forms/auth_form.go | 2 +- templates/admin/auth/edit.tmpl | 6 +++--- templates/admin/auth/source/oauth.tmpl | 6 +++--- 8 files changed, 15 insertions(+), 15 deletions(-) diff --git a/cmd/admin.go b/cmd/admin.go index ab0d3ce95056f..cfc297c474646 100644 --- a/cmd/admin.go +++ b/cmd/admin.go @@ -289,8 +289,8 @@ var ( Usage: "Custom icon URL for OAuth2 login source", }, cli.BoolFlag{ - Name: "override-local-2fa", - Usage: "Set to true to override local 2fa settings", + Name: "skip-local-2fa", + Usage: "Set to true to skip local 2fa for users authenticated by this source", }, } @@ -620,7 +620,7 @@ func parseOAuth2Config(c *cli.Context) *oauth2.Source { OpenIDConnectAutoDiscoveryURL: c.String("auto-discover-url"), CustomURLMapping: customURLMapping, IconURL: c.String("icon-url"), - OverrideLocalTwoFA: c.Bool("override-local-2fa"), + SkipLocalTwoFA: c.Bool("skip-local-2fa"), } } diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index ccee3104df50f..f1c3ba7e2a858 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -2448,8 +2448,8 @@ auths.oauth2_tokenURL = Token URL auths.oauth2_authURL = Authorize URL auths.oauth2_profileURL = Profile URL auths.oauth2_emailURL = Email URL -auths.override_local_two_fa = Override local 2FA -auths.override_local_two_fa_helper = Leaving unset means local users with 2FA set will still have to pass 2FA to log on +auths.skip_local_two_fa = Skip local 2FA +auths.skip_local_two_fa_helper = Leaving unset means local users with 2FA set will still have to pass 2FA to log on auths.oauth2_tenant = Tenant auths.enable_auto_register = Enable Auto Registration auths.sspi_auto_create_users = Automatically create users diff --git a/routers/web/admin/auths.go b/routers/web/admin/auths.go index 04143e3610615..b2879d7c4f66c 100644 --- a/routers/web/admin/auths.go +++ b/routers/web/admin/auths.go @@ -181,7 +181,7 @@ func parseOAuth2Config(form forms.AuthenticationForm) *oauth2.Source { OpenIDConnectAutoDiscoveryURL: form.OpenIDConnectAutoDiscoveryURL, CustomURLMapping: customURLMapping, IconURL: form.Oauth2IconURL, - OverrideLocalTwoFA: form.OverrideLocalTwoFA, + SkipLocalTwoFA: form.SkipLocalTwoFA, } } diff --git a/routers/web/user/auth.go b/routers/web/user/auth.go index 9b8de29802940..38e0d989b8d56 100644 --- a/routers/web/user/auth.go +++ b/routers/web/user/auth.go @@ -706,7 +706,7 @@ func handleOAuth2SignIn(ctx *context.Context, source *models.LoginSource, u *mod updateAvatarIfNeed(gothUser.AvatarURL, u) needs2FA := false - if !source.Cfg.(*oauth2.Source).OverrideLocalTwoFA { + if !source.Cfg.(*oauth2.Source).SkipLocalTwoFA { _, err := models.GetTwoFactorByUID(u.ID) if err != nil && !models.IsErrTwoFactorNotEnrolled(err) { ctx.ServerError("UserSignIn", err) diff --git a/services/auth/source/oauth2/source.go b/services/auth/source/oauth2/source.go index 542a0b57aec3c..7b22383d7ed6d 100644 --- a/services/auth/source/oauth2/source.go +++ b/services/auth/source/oauth2/source.go @@ -24,7 +24,7 @@ type Source struct { OpenIDConnectAutoDiscoveryURL string CustomURLMapping *CustomURLMapping IconURL string - OverrideLocalTwoFA bool + SkipLocalTwoFA bool // reference to the loginSource loginSource *models.LoginSource diff --git a/services/forms/auth_form.go b/services/forms/auth_form.go index 288b87b45b208..229728cf7daf3 100644 --- a/services/forms/auth_form.go +++ b/services/forms/auth_form.go @@ -66,7 +66,7 @@ type AuthenticationForm struct { Oauth2EmailURL string Oauth2IconURL string Oauth2Tenant string - OverrideLocalTwoFA bool + SkipLocalTwoFA bool SSPIAutoCreateUsers bool SSPIAutoActivateUsers bool SSPIStripDomainNames bool diff --git a/templates/admin/auth/edit.tmpl b/templates/admin/auth/edit.tmpl index 6e1a785908f55..3e21710353e35 100644 --- a/templates/admin/auth/edit.tmpl +++ b/templates/admin/auth/edit.tmpl @@ -257,9 +257,9 @@
- - -

{{.i18n.Tr "admin.auths.override_local_two_fa_helper"}}

+ + +

{{.i18n.Tr "admin.auths.skip_local_two_fa_helper"}}

diff --git a/templates/admin/auth/source/oauth.tmpl b/templates/admin/auth/source/oauth.tmpl index 07077471b235d..6e91da14e24f0 100644 --- a/templates/admin/auth/source/oauth.tmpl +++ b/templates/admin/auth/source/oauth.tmpl @@ -30,9 +30,9 @@
- - -

{{.i18n.Tr "admin.auths.override_local_two_fa_helper"}}

+ + +

{{.i18n.Tr "admin.auths.skip_local_two_fa_helper"}}