From ceaf11cbe329a9884f96fee64c0f88c9463e7dd1 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sat, 21 Aug 2021 16:18:18 +0100 Subject: [PATCH 01/13] 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 ddc19662906b31b305e73c7b3489959c38fd3800 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Fri, 20 Aug 2021 20:27:43 +0100 Subject: [PATCH 02/13] Fix regression from #16544 Signed-off-by: Andrew Thornton --- services/auth/source/oauth2/providers_openid.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/auth/source/oauth2/providers_openid.go b/services/auth/source/oauth2/providers_openid.go index b725cf96059cc..7c3836503ce9c 100644 --- a/services/auth/source/oauth2/providers_openid.go +++ b/services/auth/source/oauth2/providers_openid.go @@ -18,7 +18,7 @@ type OpenIDProvider struct { // Name provides the technical name for this provider func (o *OpenIDProvider) Name() string { - return "openidconnect" + return "openidConnect" } // DisplayName returns the friendly name for this provider From ca74f82564b4a74f2b9a19f4d50a754a928de242 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Fri, 20 Aug 2021 20:28:21 +0100 Subject: [PATCH 03/13] Add scopes settings Signed-off-by: Andrew Thornton --- cmd/admin.go | 10 ++++++ docs/content/doc/usage/command-line.en-us.md | 4 +++ modules/templates/helper.go | 1 + options/locale/locale_en-US.ini | 1 + routers/web/admin/auths.go | 4 ++- .../auth/source/oauth2/providers_custom.go | 32 +++++++++++-------- .../auth/source/oauth2/providers_openid.go | 7 +++- .../auth/source/oauth2/providers_simple.go | 3 +- services/auth/source/oauth2/source.go | 2 ++ services/forms/auth_form.go | 1 + templates/admin/auth/edit.tmpl | 5 +++ templates/admin/auth/source/oauth.tmpl | 5 +++ 12 files changed, 59 insertions(+), 16 deletions(-) diff --git a/cmd/admin.go b/cmd/admin.go index ab0d3ce95056f..260bbd45cafcf 100644 --- a/cmd/admin.go +++ b/cmd/admin.go @@ -292,6 +292,11 @@ var ( Name: "override-local-2fa", Usage: "Set to true to override local 2fa settings", }, + cli.StringSliceFlag{ + Name: "scopes", + Value: nil, + Usage: "Scopes to request when to authenticate against this OAuth2 source", + }, } microcmdAuthUpdateOauth = cli.Command{ @@ -621,6 +626,7 @@ func parseOAuth2Config(c *cli.Context) *oauth2.Source { CustomURLMapping: customURLMapping, IconURL: c.String("icon-url"), OverrideLocalTwoFA: c.Bool("override-local-2fa"), + Scopes: c.StringSlice("scopes"), } } @@ -677,6 +683,10 @@ func runUpdateOauth(c *cli.Context) error { oAuth2Config.IconURL = c.String("icon-url") } + if c.IsSet("scopes") { + oAuth2Config.Scopes = c.StringSlice("scopes") + } + // update custom URL mapping var customURLMapping = &oauth2.CustomURLMapping{} diff --git a/docs/content/doc/usage/command-line.en-us.md b/docs/content/doc/usage/command-line.en-us.md index 0bc8d70fdb53a..eafb26ba4c82c 100644 --- a/docs/content/doc/usage/command-line.en-us.md +++ b/docs/content/doc/usage/command-line.en-us.md @@ -117,6 +117,8 @@ Admin operations: - `--custom-profile-url`: Use a custom Profile URL (option for GitLab/GitHub). - `--custom-email-url`: Use a custom Email URL (option for GitHub). - `--icon-url`: Custom icon URL for OAuth2 login source. + - `--override-local-2fa`: Allow source to override local 2fa. (Optional) + - `--scopes`: Addtional scopes to request for this OAuth2 source. (Optional) - Examples: - `gitea admin auth add-oauth --name external-github --provider github --key OBTAIN_FROM_SOURCE --secret OBTAIN_FROM_SOURCE` - `update-oauth`: @@ -133,6 +135,8 @@ Admin operations: - `--custom-profile-url`: Use a custom Profile URL (option for GitLab/GitHub). - `--custom-email-url`: Use a custom Email URL (option for GitHub). - `--icon-url`: Custom icon URL for OAuth2 login source. + - `--override-local-2fa`: Allow source to override local 2fa. (Optional) + - `--scopes`: Addtional scopes to request for this OAuth2 source. - Examples: - `gitea admin auth update-oauth --id 1 --name external-github-updated` - `add-ldap`: Add new LDAP (via Bind DN) authentication source diff --git a/modules/templates/helper.go b/modules/templates/helper.go index 6517127ebf450..07a4c80d977d3 100644 --- a/modules/templates/helper.go +++ b/modules/templates/helper.go @@ -377,6 +377,7 @@ func NewFuncMap() []template.FuncMap { "MermaidMaxSourceCharacters": func() int { return setting.MermaidMaxSourceCharacters }, + "Join": strings.Join, }} } diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index ccee3104df50f..b9766df0bfe5f 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -2451,6 +2451,7 @@ 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.oauth2_scopes = Additional Scopes auths.enable_auto_register = Enable Auto Registration auths.sspi_auto_create_users = Automatically create users auths.sspi_auto_create_users_helper = Allow SSPI auth method to automatically create new accounts for users that login for the first time diff --git a/routers/web/admin/auths.go b/routers/web/admin/auths.go index 04143e3610615..cb92a484a899e 100644 --- a/routers/web/admin/auths.go +++ b/routers/web/admin/auths.go @@ -9,6 +9,7 @@ import ( "fmt" "net/http" "regexp" + "strings" "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/auth/pam" @@ -182,6 +183,7 @@ func parseOAuth2Config(form forms.AuthenticationForm) *oauth2.Source { CustomURLMapping: customURLMapping, IconURL: form.Oauth2IconURL, OverrideLocalTwoFA: form.OverrideLocalTwoFA, + Scopes: strings.Split(form.Oauth2Scopes, ","), } } @@ -322,8 +324,8 @@ func EditAuthSource(ctx *context.Context) { break } } - } + ctx.HTML(http.StatusOK, tplAuthEdit) } diff --git a/services/auth/source/oauth2/providers_custom.go b/services/auth/source/oauth2/providers_custom.go index f2cff131f4a29..c3ebdf9df0af7 100644 --- a/services/auth/source/oauth2/providers_custom.go +++ b/services/auth/source/oauth2/providers_custom.go @@ -17,7 +17,7 @@ import ( ) // CustomProviderNewFn creates a goth.Provider using a custom url mapping -type CustomProviderNewFn func(clientID, secret, callbackURL string, custom *CustomURLMapping) (goth.Provider, error) +type CustomProviderNewFn func(clientID, secret, callbackURL string, custom *CustomURLMapping, scopes []string) (goth.Provider, error) // CustomProvider is a GothProvider that has CustomURL features type CustomProvider struct { @@ -35,7 +35,7 @@ func (c *CustomProvider) CustomURLSettings() *CustomURLSettings { func (c *CustomProvider) CreateGothProvider(providerName, callbackURL string, source *Source) (goth.Provider, error) { custom := c.customURLSettings.OverrideWith(source.CustomURLMapping) - return c.newFn(source.ClientID, source.ClientSecret, callbackURL, custom) + return c.newFn(source.ClientID, source.ClientSecret, callbackURL, custom, source.Scopes) } // NewCustomProvider is a constructor function for custom providers @@ -60,8 +60,7 @@ func init() { ProfileURL: availableAttribute(github.ProfileURL), EmailURL: availableAttribute(github.EmailURL), }, - func(clientID, secret, callbackURL string, custom *CustomURLMapping) (goth.Provider, error) { - scopes := []string{} + func(clientID, secret, callbackURL string, custom *CustomURLMapping, scopes []string) (goth.Provider, error) { if setting.OAuth2Client.EnableAutoRegistration { scopes = append(scopes, "user:email") } @@ -73,8 +72,9 @@ func init() { AuthURL: availableAttribute(gitlab.AuthURL), TokenURL: availableAttribute(gitlab.TokenURL), ProfileURL: availableAttribute(gitlab.ProfileURL), - }, func(clientID, secret, callbackURL string, custom *CustomURLMapping) (goth.Provider, error) { - return gitlab.NewCustomisedURL(clientID, secret, callbackURL, custom.AuthURL, custom.TokenURL, custom.ProfileURL, "read_user"), nil + }, func(clientID, secret, callbackURL string, custom *CustomURLMapping, scopes []string) (goth.Provider, error) { + scopes = append(scopes, "read_user") + return gitlab.NewCustomisedURL(clientID, secret, callbackURL, custom.AuthURL, custom.TokenURL, custom.ProfileURL, scopes...), nil })) RegisterGothProvider(NewCustomProvider( @@ -83,8 +83,8 @@ func init() { AuthURL: requiredAttribute(gitea.AuthURL), ProfileURL: requiredAttribute(gitea.ProfileURL), }, - func(clientID, secret, callbackURL string, custom *CustomURLMapping) (goth.Provider, error) { - return gitea.NewCustomisedURL(clientID, secret, callbackURL, custom.AuthURL, custom.TokenURL, custom.ProfileURL), nil + func(clientID, secret, callbackURL string, custom *CustomURLMapping, scopes []string) (goth.Provider, error) { + return gitea.NewCustomisedURL(clientID, secret, callbackURL, custom.AuthURL, custom.TokenURL, custom.ProfileURL, scopes...), nil })) RegisterGothProvider(NewCustomProvider( @@ -93,25 +93,31 @@ func init() { AuthURL: requiredAttribute(nextcloud.AuthURL), ProfileURL: requiredAttribute(nextcloud.ProfileURL), }, - func(clientID, secret, callbackURL string, custom *CustomURLMapping) (goth.Provider, error) { - return nextcloud.NewCustomisedURL(clientID, secret, callbackURL, custom.AuthURL, custom.TokenURL, custom.ProfileURL), nil + func(clientID, secret, callbackURL string, custom *CustomURLMapping, scopes []string) (goth.Provider, error) { + return nextcloud.NewCustomisedURL(clientID, secret, callbackURL, custom.AuthURL, custom.TokenURL, custom.ProfileURL, scopes...), nil })) RegisterGothProvider(NewCustomProvider( "mastodon", "Mastodon", &CustomURLSettings{ AuthURL: requiredAttribute(mastodon.InstanceURL), }, - func(clientID, secret, callbackURL string, custom *CustomURLMapping) (goth.Provider, error) { - return mastodon.NewCustomisedURL(clientID, secret, callbackURL, custom.AuthURL), nil + func(clientID, secret, callbackURL string, custom *CustomURLMapping, scopes []string) (goth.Provider, error) { + return mastodon.NewCustomisedURL(clientID, secret, callbackURL, custom.AuthURL, scopes...), nil })) RegisterGothProvider(NewCustomProvider( "azureadv2", "Azure AD v2", &CustomURLSettings{ Tenant: requiredAttribute("organizations"), }, - func(clientID, secret, callbackURL string, custom *CustomURLMapping) (goth.Provider, error) { + func(clientID, secret, callbackURL string, custom *CustomURLMapping, scopes []string) (goth.Provider, error) { + azureScopes := make([]azureadv2.ScopeType, len(scopes)) + for i, scope := range scopes { + azureScopes[i] = azureadv2.ScopeType(scope) + } + return azureadv2.New(clientID, secret, callbackURL, azureadv2.ProviderOptions{ Tenant: azureadv2.TenantType(custom.Tenant), + Scopes: azureScopes, }), nil }, )) diff --git a/services/auth/source/oauth2/providers_openid.go b/services/auth/source/oauth2/providers_openid.go index 7c3836503ce9c..838311b4a17f5 100644 --- a/services/auth/source/oauth2/providers_openid.go +++ b/services/auth/source/oauth2/providers_openid.go @@ -33,7 +33,12 @@ func (o *OpenIDProvider) Image() string { // CreateGothProvider creates a GothProvider from this Provider func (o *OpenIDProvider) CreateGothProvider(providerName, callbackURL string, source *Source) (goth.Provider, error) { - provider, err := openidConnect.New(source.ClientID, source.ClientSecret, callbackURL, source.OpenIDConnectAutoDiscoveryURL, setting.OAuth2Client.OpenIDConnectScopes...) + scopes := setting.OAuth2Client.OpenIDConnectScopes + if len(scopes) == 0 { + scopes = append(scopes, source.Scopes...) + } + + provider, err := openidConnect.New(source.ClientID, source.ClientSecret, callbackURL, source.OpenIDConnectAutoDiscoveryURL, scopes...) if err != nil { log.Warn("Failed to create OpenID Connect Provider with name '%s' with url '%s': %v", providerName, source.OpenIDConnectAutoDiscoveryURL, err) } diff --git a/services/auth/source/oauth2/providers_simple.go b/services/auth/source/oauth2/providers_simple.go index 5a7062e6c3718..21b1e9de7aaed 100644 --- a/services/auth/source/oauth2/providers_simple.go +++ b/services/auth/source/oauth2/providers_simple.go @@ -31,7 +31,8 @@ type SimpleProvider struct { // CreateGothProvider creates a GothProvider from this Provider func (c *SimpleProvider) CreateGothProvider(providerName, callbackURL string, source *Source) (goth.Provider, error) { - return c.newFn(source.ClientID, source.ClientSecret, callbackURL, c.scopes...), nil + scopes := append(c.scopes, source.Scopes...) + return c.newFn(source.ClientID, source.ClientSecret, callbackURL, scopes...), nil } // NewSimpleProvider is a constructor function for simple providers diff --git a/services/auth/source/oauth2/source.go b/services/auth/source/oauth2/source.go index 542a0b57aec3c..c9c8875480802 100644 --- a/services/auth/source/oauth2/source.go +++ b/services/auth/source/oauth2/source.go @@ -26,6 +26,8 @@ type Source struct { IconURL string OverrideLocalTwoFA bool + Scopes []string + // reference to the loginSource loginSource *models.LoginSource } diff --git a/services/forms/auth_form.go b/services/forms/auth_form.go index 288b87b45b208..2ad569e0b0b0c 100644 --- a/services/forms/auth_form.go +++ b/services/forms/auth_form.go @@ -67,6 +67,7 @@ type AuthenticationForm struct { Oauth2IconURL string Oauth2Tenant string OverrideLocalTwoFA bool + Oauth2Scopes string SSPIAutoCreateUsers bool SSPIAutoActivateUsers bool SSPIStripDomainNames bool diff --git a/templates/admin/auth/edit.tmpl b/templates/admin/auth/edit.tmpl index 6e1a785908f55..33cdd0249e5ba 100644 --- a/templates/admin/auth/edit.tmpl +++ b/templates/admin/auth/edit.tmpl @@ -298,6 +298,11 @@ {{end}}{{end}} + +
+ + +
{{end}} diff --git a/templates/admin/auth/source/oauth.tmpl b/templates/admin/auth/source/oauth.tmpl index 07077471b235d..1e26af6a71449 100644 --- a/templates/admin/auth/source/oauth.tmpl +++ b/templates/admin/auth/source/oauth.tmpl @@ -71,4 +71,9 @@ {{end}}{{end}} + +
+ + +
From c282f854f12544e33d6208e274cba5c93709040a Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Fri, 20 Aug 2021 22:03:20 +0100 Subject: [PATCH 04/13] fix trace logging in auth_openid Signed-off-by: Andrew Thornton --- routers/web/user/auth_openid.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/routers/web/user/auth_openid.go b/routers/web/user/auth_openid.go index fc419a7f6ea63..88b43b0b02af9 100644 --- a/routers/web/user/auth_openid.go +++ b/routers/web/user/auth_openid.go @@ -144,10 +144,10 @@ func SignInOpenIDPost(ctx *context.Context) { // signInOpenIDVerify handles response from OpenID provider func signInOpenIDVerify(ctx *context.Context) { - log.Trace("Incoming call to: " + ctx.Req.URL.String()) + log.Trace("Incoming call to: %s", ctx.Req.URL.String()) fullURL := setting.AppURL + ctx.Req.URL.String()[1:] - log.Trace("Full URL: " + fullURL) + log.Trace("Full URL: %s", fullURL) var id, err = openid.Verify(fullURL) if err != nil { @@ -157,7 +157,7 @@ func signInOpenIDVerify(ctx *context.Context) { return } - log.Trace("Verified ID: " + id) + log.Trace("Verified ID: %s", id) /* Now we should seek for the user and log him in, or prompt * to register if not found */ @@ -180,7 +180,7 @@ func signInOpenIDVerify(ctx *context.Context) { return } - log.Trace("User with openid " + id + " does not exist, should connect or register") + log.Trace("User with openid: %s does not exist, should connect or register", id) parsedURL, err := url.Parse(fullURL) if err != nil { @@ -199,7 +199,7 @@ func signInOpenIDVerify(ctx *context.Context) { email := values.Get("openid.sreg.email") nickname := values.Get("openid.sreg.nickname") - log.Trace("User has email=" + email + " and nickname=" + nickname) + log.Trace("User has email=%s and nickname=%s", email, nickname) if email != "" { u, err = models.GetUserByEmail(email) @@ -213,7 +213,7 @@ func signInOpenIDVerify(ctx *context.Context) { log.Error("signInOpenIDVerify: %v", err) } if u != nil { - log.Trace("Local user " + u.LowerName + " has OpenID provided email " + email) + log.Trace("Local user %s has OpenID provided email %s", u.LowerName, email) } } @@ -228,7 +228,7 @@ func signInOpenIDVerify(ctx *context.Context) { } } if u != nil { - log.Trace("Local user " + u.LowerName + " has OpenID provided nickname " + nickname) + log.Trace("Local user %s has OpenID provided nickname %s", u.LowerName, nickname) } } From 0d4874f5b42f2b10f63f0e5f2ce0cfc3f1ccd3ae Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Fri, 20 Aug 2021 22:10:08 +0100 Subject: [PATCH 05/13] add required claim options Signed-off-by: Andrew Thornton --- cmd/admin.go | 20 ++++++++++++++++++++ docs/content/doc/usage/command-line.en-us.md | 4 ++++ options/locale/locale_en-US.ini | 4 ++++ routers/web/admin/auths.go | 2 ++ routers/web/user/auth.go | 11 ++++++++++- services/auth/source/oauth2/source.go | 4 +++- services/forms/auth_form.go | 2 ++ templates/admin/auth/edit.tmpl | 10 ++++++++++ templates/admin/auth/source/oauth.tmpl | 10 ++++++++++ 9 files changed, 65 insertions(+), 2 deletions(-) diff --git a/cmd/admin.go b/cmd/admin.go index 260bbd45cafcf..469e7a0fb342e 100644 --- a/cmd/admin.go +++ b/cmd/admin.go @@ -297,6 +297,16 @@ var ( Value: nil, Usage: "Scopes to request when to authenticate against this OAuth2 source", }, + cli.StringFlag{ + Name: "required-claim-name", + Value: "", + Usage: "Claim name that has to be set to allow users to login with this source", + }, + cli.StringFlag{ + Name: "required-claim-value", + Value: "", + Usage: "Claim value that has to be set to allow users to login with this source", + }, } microcmdAuthUpdateOauth = cli.Command{ @@ -627,6 +637,8 @@ func parseOAuth2Config(c *cli.Context) *oauth2.Source { IconURL: c.String("icon-url"), OverrideLocalTwoFA: c.Bool("override-local-2fa"), Scopes: c.StringSlice("scopes"), + RequiredClaimName: c.String("required-claim-name"), + RequiredClaimValue: c.String("required-claim-value"), } } @@ -687,6 +699,14 @@ func runUpdateOauth(c *cli.Context) error { oAuth2Config.Scopes = c.StringSlice("scopes") } + if c.IsSet("required-claim-name") { + oAuth2Config.RequiredClaimName = c.String("required-claim-name") + + } + if c.IsSet("required-claim-value") { + oAuth2Config.RequiredClaimValue = c.String("required-claim-value") + } + // update custom URL mapping var customURLMapping = &oauth2.CustomURLMapping{} diff --git a/docs/content/doc/usage/command-line.en-us.md b/docs/content/doc/usage/command-line.en-us.md index eafb26ba4c82c..6b032f554a127 100644 --- a/docs/content/doc/usage/command-line.en-us.md +++ b/docs/content/doc/usage/command-line.en-us.md @@ -119,6 +119,8 @@ Admin operations: - `--icon-url`: Custom icon URL for OAuth2 login source. - `--override-local-2fa`: Allow source to override local 2fa. (Optional) - `--scopes`: Addtional scopes to request for this OAuth2 source. (Optional) + - `--required-claim-name`: Claim name that has to be set to allow users to login with this source. (Optional) + - `--required-claim-value`: Claim value that has to be set to allow users to login with this source. (Optional) - Examples: - `gitea admin auth add-oauth --name external-github --provider github --key OBTAIN_FROM_SOURCE --secret OBTAIN_FROM_SOURCE` - `update-oauth`: @@ -137,6 +139,8 @@ Admin operations: - `--icon-url`: Custom icon URL for OAuth2 login source. - `--override-local-2fa`: Allow source to override local 2fa. (Optional) - `--scopes`: Addtional scopes to request for this OAuth2 source. + - `--required-claim-name`: Claim name that has to be set to allow users to login with this source. (Optional) + - `--required-claim-value`: Claim value that has to be set to allow users to login with this source. (Optional) - Examples: - `gitea admin auth update-oauth --id 1 --name external-github-updated` - `add-ldap`: Add new LDAP (via Bind DN) authentication source diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index b9766df0bfe5f..3624c49a6cbd5 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -2452,6 +2452,10 @@ 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.oauth2_scopes = Additional Scopes +auths.oauth2_required_claim_name = Required Claim Name +auths.oauth2_required_claim_name_helper = Set this name to only allow users to login from this source if the user has a claim with this name +auths.oauth2_required_claim_value = Required Claim Value +auths.oauth2_required_claim_value_helper = Set this value to only allow users to login from this source if the user hasthe claim above with this value auths.enable_auto_register = Enable Auto Registration auths.sspi_auto_create_users = Automatically create users auths.sspi_auto_create_users_helper = Allow SSPI auth method to automatically create new accounts for users that login for the first time diff --git a/routers/web/admin/auths.go b/routers/web/admin/auths.go index cb92a484a899e..3d445978d67b5 100644 --- a/routers/web/admin/auths.go +++ b/routers/web/admin/auths.go @@ -184,6 +184,8 @@ func parseOAuth2Config(form forms.AuthenticationForm) *oauth2.Source { IconURL: form.Oauth2IconURL, OverrideLocalTwoFA: form.OverrideLocalTwoFA, Scopes: strings.Split(form.Oauth2Scopes, ","), + RequiredClaimName: form.Oauth2RequiredClaimName, + RequiredClaimValue: form.Oauth2RequiredClaimValue, } } diff --git a/routers/web/user/auth.go b/routers/web/user/auth.go index 9b8de29802940..af0023c5fd725 100644 --- a/routers/web/user/auth.go +++ b/routers/web/user/auth.go @@ -777,7 +777,9 @@ func handleOAuth2SignIn(ctx *context.Context, source *models.LoginSource, u *mod // OAuth2UserLoginCallback attempts to handle the callback from the OAuth2 provider and if successful // login the user func oAuth2UserLoginCallback(loginSource *models.LoginSource, request *http.Request, response http.ResponseWriter) (*models.User, goth.User, error) { - gothUser, err := loginSource.Cfg.(*oauth2.Source).Callback(request, response) + oauth2Source := loginSource.Cfg.(*oauth2.Source) + + gothUser, err := oauth2Source.Callback(request, response) if err != nil { if err.Error() == "securecookie: the value is too long" { log.Error("OAuth2 Provider %s returned too long a token. Current max: %d. Either increase the [OAuth2] MAX_TOKEN_LENGTH or reduce the information returned from the OAuth2 provider", loginSource.Name, setting.OAuth2.MaxTokenLength) @@ -786,6 +788,13 @@ func oAuth2UserLoginCallback(loginSource *models.LoginSource, request *http.Requ return nil, goth.User{}, err } + if oauth2Source.RequiredClaimName != "" { + claimInterface, has := gothUser.RawData[oauth2Source.RequiredClaimName] + if !has || (oauth2Source.RequiredClaimValue != "" && claimInterface.(string) != oauth2Source.RequiredClaimValue) { + return nil, goth.User{}, models.ErrUserProhibitLogin{Name: gothUser.UserID} + } + } + user := &models.User{ LoginName: gothUser.UserID, LoginType: models.LoginOAuth2, diff --git a/services/auth/source/oauth2/source.go b/services/auth/source/oauth2/source.go index c9c8875480802..0b2e658eb20a2 100644 --- a/services/auth/source/oauth2/source.go +++ b/services/auth/source/oauth2/source.go @@ -26,7 +26,9 @@ type Source struct { IconURL string OverrideLocalTwoFA bool - Scopes []string + Scopes []string + RequiredClaimName string + RequiredClaimValue string // reference to the loginSource loginSource *models.LoginSource diff --git a/services/forms/auth_form.go b/services/forms/auth_form.go index 2ad569e0b0b0c..bc56a2bec3cc5 100644 --- a/services/forms/auth_form.go +++ b/services/forms/auth_form.go @@ -68,6 +68,8 @@ type AuthenticationForm struct { Oauth2Tenant string OverrideLocalTwoFA bool Oauth2Scopes string + Oauth2RequiredClaimName string + Oauth2RequiredClaimValue string SSPIAutoCreateUsers bool SSPIAutoActivateUsers bool SSPIStripDomainNames bool diff --git a/templates/admin/auth/edit.tmpl b/templates/admin/auth/edit.tmpl index 33cdd0249e5ba..57f5d7b59cb86 100644 --- a/templates/admin/auth/edit.tmpl +++ b/templates/admin/auth/edit.tmpl @@ -303,6 +303,16 @@
+
+ + +

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

+
+
+ + +

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

+
{{end}} diff --git a/templates/admin/auth/source/oauth.tmpl b/templates/admin/auth/source/oauth.tmpl index 1e26af6a71449..94b196fed4e89 100644 --- a/templates/admin/auth/source/oauth.tmpl +++ b/templates/admin/auth/source/oauth.tmpl @@ -76,4 +76,14 @@
+
+ + +

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

+
+
+ + +

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

+
From bd86307cecf7ca0b89d0f4df23154a0846c5e733 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Fri, 20 Aug 2021 23:42:37 +0100 Subject: [PATCH 06/13] Move UpdateExternalUser to externalaccount Signed-off-by: Andrew Thornton --- models/external_login_user.go | 37 ++++++-------------------------- routers/web/user/auth.go | 24 ++++----------------- services/externalaccount/link.go | 28 ++++++++++++++++++++++++ services/externalaccount/user.go | 26 +++++++++++++++++----- 4 files changed, 59 insertions(+), 56 deletions(-) create mode 100644 services/externalaccount/link.go diff --git a/models/external_login_user.go b/models/external_login_user.go index aa5da8134a3e3..af480ae0b6511 100644 --- a/models/external_login_user.go +++ b/models/external_login_user.go @@ -9,7 +9,6 @@ import ( "code.gitea.io/gitea/modules/structs" - "github.com/markbates/goth" "xorm.io/builder" ) @@ -99,42 +98,18 @@ func GetUserIDByExternalUserID(provider, userID string) (int64, error) { return id, nil } -// UpdateExternalUser updates external user's information -func UpdateExternalUser(user *User, gothUser goth.User) error { - loginSource, err := GetActiveOAuth2LoginSourceByName(gothUser.Provider) - if err != nil { - return err - } - externalLoginUser := &ExternalLoginUser{ - ExternalID: gothUser.UserID, - UserID: user.ID, - LoginSourceID: loginSource.ID, - RawData: gothUser.RawData, - Provider: gothUser.Provider, - Email: gothUser.Email, - Name: gothUser.Name, - FirstName: gothUser.FirstName, - LastName: gothUser.LastName, - NickName: gothUser.NickName, - Description: gothUser.Description, - AvatarURL: gothUser.AvatarURL, - Location: gothUser.Location, - AccessToken: gothUser.AccessToken, - AccessTokenSecret: gothUser.AccessTokenSecret, - RefreshToken: gothUser.RefreshToken, - ExpiresAt: gothUser.ExpiresAt, - } - - has, err := x.Where("external_id=? AND login_source_id=?", gothUser.UserID, loginSource.ID). +// UpdateExternalUserByExternalID updates an external user's information +func UpdateExternalUserByExternalID(external *ExternalLoginUser) error { + has, err := x.Where("external_id=? AND login_source_id=?", external.ExternalID, external.LoginSourceID). NoAutoCondition(). - Exist(externalLoginUser) + Exist(external) if err != nil { return err } else if !has { - return ErrExternalLoginUserNotExist{user.ID, loginSource.ID} + return ErrExternalLoginUserNotExist{external.UserID, external.LoginSourceID} } - _, err = x.Where("external_id=? AND login_source_id=?", gothUser.UserID, loginSource.ID).AllCols().Update(externalLoginUser) + _, err = x.Where("external_id=? AND login_source_id=?", external.ExternalID, external.LoginSourceID).AllCols().Update(external) return err } diff --git a/routers/web/user/auth.go b/routers/web/user/auth.go index af0023c5fd725..a0011f1263969 100644 --- a/routers/web/user/auth.go +++ b/routers/web/user/auth.go @@ -289,16 +289,8 @@ func TwoFactorPost(ctx *context.Context) { } if ctx.Session.Get("linkAccount") != nil { - gothUser := ctx.Session.Get("linkAccountGothUser") - if gothUser == nil { - ctx.ServerError("UserSignIn", errors.New("not in LinkAccount session")) - return - } - - err = externalaccount.LinkAccountToUser(u, gothUser.(goth.User)) - if err != nil { + if err := externalaccount.LinkAccountFromStore(ctx.Session, u); err != nil { ctx.ServerError("UserSignIn", err) - return } } @@ -470,16 +462,8 @@ func U2FSign(ctx *context.Context) { } if ctx.Session.Get("linkAccount") != nil { - gothUser := ctx.Session.Get("linkAccountGothUser") - if gothUser == nil { - ctx.ServerError("UserSignIn", errors.New("not in LinkAccount session")) - return - } - - err = externalaccount.LinkAccountToUser(user, gothUser.(goth.User)) - if err != nil { + if err := externalaccount.LinkAccountFromStore(ctx.Session, user); err != nil { ctx.ServerError("UserSignIn", err) - return } } redirect := handleSignInFull(ctx, user, remember, false) @@ -739,7 +723,7 @@ func handleOAuth2SignIn(ctx *context.Context, source *models.LoginSource, u *mod } // update external user information - if err := models.UpdateExternalUser(u, gothUser); err != nil { + if err := externalaccount.UpdateExternalUser(u, gothUser); err != nil { log.Error("UpdateExternalUser failed: %v", err) } @@ -1321,7 +1305,7 @@ func handleUserCreated(ctx *context.Context, u *models.User, gothUser *goth.User // update external user information if gothUser != nil { - if err := models.UpdateExternalUser(u, *gothUser); err != nil { + if err := externalaccount.UpdateExternalUser(u, *gothUser); err != nil { log.Error("UpdateExternalUser failed: %v", err) } } diff --git a/services/externalaccount/link.go b/services/externalaccount/link.go new file mode 100644 index 0000000000000..109b4523e8919 --- /dev/null +++ b/services/externalaccount/link.go @@ -0,0 +1,28 @@ +// Copyright 2021 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package externalaccount + +import ( + "fmt" + + "code.gitea.io/gitea/models" + + "github.com/markbates/goth" +) + +type Store interface { + Get(interface{}) interface{} + Set(interface{}, interface{}) error + Release() error +} + +func LinkAccountFromStore(store Store, user *models.User) error { + gothUser := store.Get("linkAccountGothUser") + if gothUser == nil { + return fmt.Errorf("not in LinkAccount session") + } + + return LinkAccountToUser(user, gothUser.(goth.User)) +} diff --git a/services/externalaccount/user.go b/services/externalaccount/user.go index 45773fdb127d2..d700920e1a9c3 100644 --- a/services/externalaccount/user.go +++ b/services/externalaccount/user.go @@ -13,14 +13,12 @@ import ( "github.com/markbates/goth" ) -// LinkAccountToUser link the gothUser to the user -func LinkAccountToUser(user *models.User, gothUser goth.User) error { +func toExternalLoginUser(user *models.User, gothUser goth.User) (*models.ExternalLoginUser, error) { loginSource, err := models.GetActiveOAuth2LoginSourceByName(gothUser.Provider) if err != nil { - return err + return nil, err } - - externalLoginUser := &models.ExternalLoginUser{ + return &models.ExternalLoginUser{ ExternalID: gothUser.UserID, UserID: user.ID, LoginSourceID: loginSource.ID, @@ -38,6 +36,14 @@ func LinkAccountToUser(user *models.User, gothUser goth.User) error { AccessTokenSecret: gothUser.AccessTokenSecret, RefreshToken: gothUser.RefreshToken, ExpiresAt: gothUser.ExpiresAt, + }, nil +} + +// LinkAccountToUser link the gothUser to the user +func LinkAccountToUser(user *models.User, gothUser goth.User) error { + externalLoginUser, err := toExternalLoginUser(user, gothUser) + if err != nil { + return err } if err := models.LinkExternalToUser(user, externalLoginUser); err != nil { @@ -60,3 +66,13 @@ func LinkAccountToUser(user *models.User, gothUser goth.User) error { return nil } + +// UpdateExternalUser updates external user's information +func UpdateExternalUser(user *models.User, gothUser goth.User) error { + externalLoginUser, err := toExternalLoginUser(user, gothUser) + if err != nil { + return err + } + + return models.UpdateExternalUserByExternalID(externalLoginUser) +} From 0e27070c6d17ad902f3b47100e89f872d85fe351 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sat, 21 Aug 2021 00:08:50 +0100 Subject: [PATCH 07/13] Allow OAuth2/OIDC to set Admin/Restricted status Signed-off-by: Andrew Thornton --- cmd/admin.go | 28 ++++++++ docs/content/doc/usage/command-line.en-us.md | 6 ++ routers/web/user/auth.go | 72 +++++++++++++++++++- services/auth/source/oauth2/source.go | 3 + services/forms/auth_form.go | 3 + templates/admin/auth/edit.tmpl | 12 ++++ templates/admin/auth/source/oauth.tmpl | 12 ++++ 7 files changed, 135 insertions(+), 1 deletion(-) diff --git a/cmd/admin.go b/cmd/admin.go index 469e7a0fb342e..91efaf3ab8ef1 100644 --- a/cmd/admin.go +++ b/cmd/admin.go @@ -307,6 +307,21 @@ var ( Value: "", Usage: "Claim value that has to be set to allow users to login with this source", }, + cli.StringFlag{ + Name: "group-claim-name", + Value: "", + Usage: "Claim name providing group names for this source", + }, + cli.StringFlag{ + Name: "admin-group", + Value: "", + Usage: "Group Claim value for administrator users", + }, + cli.StringFlag{ + Name: "restricted-group", + Value: "", + Usage: "Group Claim value for restricted users", + }, } microcmdAuthUpdateOauth = cli.Command{ @@ -639,6 +654,9 @@ func parseOAuth2Config(c *cli.Context) *oauth2.Source { Scopes: c.StringSlice("scopes"), RequiredClaimName: c.String("required-claim-name"), RequiredClaimValue: c.String("required-claim-value"), + GroupClaimName: c.String("group-claim-name"), + AdminGroup: c.String("admin-group"), + RestrictedGroup: c.String("restricted-group"), } } @@ -707,6 +725,16 @@ func runUpdateOauth(c *cli.Context) error { oAuth2Config.RequiredClaimValue = c.String("required-claim-value") } + if c.IsSet("group-claim-name") { + oAuth2Config.GroupClaimName = c.String("group-claim-name") + } + if c.IsSet("admin-group") { + oAuth2Config.AdminGroup = c.String("admin-group") + } + if c.IsSet("restricted-group") { + oAuth2Config.RestrictedGroup = c.String("restricted-group") + } + // update custom URL mapping var customURLMapping = &oauth2.CustomURLMapping{} diff --git a/docs/content/doc/usage/command-line.en-us.md b/docs/content/doc/usage/command-line.en-us.md index 6b032f554a127..865f031a19ac2 100644 --- a/docs/content/doc/usage/command-line.en-us.md +++ b/docs/content/doc/usage/command-line.en-us.md @@ -121,6 +121,9 @@ Admin operations: - `--scopes`: Addtional scopes to request for this OAuth2 source. (Optional) - `--required-claim-name`: Claim name that has to be set to allow users to login with this source. (Optional) - `--required-claim-value`: Claim value that has to be set to allow users to login with this source. (Optional) + - `--group-claim-name`: Claim name providing group names for this source. (Optional) + - `--admin-group`: Group Claim value for administrator users. (Optional) + - `--restricted-group`: Group Claim value for restricted users. (Optional) - Examples: - `gitea admin auth add-oauth --name external-github --provider github --key OBTAIN_FROM_SOURCE --secret OBTAIN_FROM_SOURCE` - `update-oauth`: @@ -141,6 +144,9 @@ Admin operations: - `--scopes`: Addtional scopes to request for this OAuth2 source. - `--required-claim-name`: Claim name that has to be set to allow users to login with this source. (Optional) - `--required-claim-value`: Claim value that has to be set to allow users to login with this source. (Optional) + - `--group-claim-name`: Claim name providing group names for this source. (Optional) + - `--admin-group`: Group Claim value for administrator users. (Optional) + - `--restricted-group`: Group Claim value for restricted users. (Optional) - Examples: - `gitea admin auth update-oauth --id 1 --name external-github-updated` - `add-ldap`: Add new LDAP (via Bind DN) authentication source diff --git a/routers/web/user/auth.go b/routers/web/user/auth.go index a0011f1263969..55f0d1f06bb2d 100644 --- a/routers/web/user/auth.go +++ b/routers/web/user/auth.go @@ -597,6 +597,13 @@ func SignInOAuthCallback(ctx *context.Context) { u, gothUser, err := oAuth2UserLoginCallback(loginSource, ctx.Req, ctx.Resp) if err != nil { + if models.IsErrUserProhibitLogin(err) { + uplerr := err.(*models.ErrUserProhibitLogin) + log.Info("Failed authentication attempt for %s from %s: %v", uplerr.Name, ctx.RemoteAddr(), err) + ctx.Data["Title"] = ctx.Tr("auth.prohibit_login") + ctx.HTML(http.StatusOK, "user/auth/prohibit_login") + return + } ctx.ServerError("UserSignIn", err) return } @@ -633,6 +640,8 @@ func SignInOAuthCallback(ctx *context.Context) { LoginName: gothUser.UserID, } + setUserGroupClaims(loginSource, u, &gothUser) + if !createAndHandleCreatedUser(ctx, base.TplName(""), nil, u, &gothUser, setting.OAuth2Client.AccountLinking != setting.OAuth2AccountLinkingDisabled) { // error already handled return @@ -647,6 +656,51 @@ func SignInOAuthCallback(ctx *context.Context) { handleOAuth2SignIn(ctx, loginSource, u, gothUser) } +func setUserGroupClaims(loginSource *models.LoginSource, u *models.User, gothUser *goth.User) bool { + + source := loginSource.Cfg.(*oauth2.Source) + if source.GroupClaimName == "" || (source.AdminGroup == "" && source.RestrictedGroup == "") { + return false + } + + groupClaims, has := gothUser.RawData[source.GroupClaimName] + if !has { + return false + } + + var groups []string + + switch rawGroup := groupClaims.(type) { + case []string: + groups = rawGroup + case string: + if strings.Contains(rawGroup, ",") { + groups = strings.Split(rawGroup, ",") + } else { + groups = []string{rawGroup} + } + } + + wasAdmin, wasRestricted := u.IsAdmin, u.IsRestricted + + if source.AdminGroup != "" { + u.IsAdmin = false + } + if source.RestrictedGroup != "" { + u.IsRestricted = false + } + + for _, g := range groups { + if source.AdminGroup != "" && g == source.AdminGroup { + u.IsAdmin = true + } else if source.RestrictedGroup != "" && g == source.RestrictedGroup { + u.IsRestricted = true + } + } + + return wasAdmin == u.IsAdmin && wasRestricted == u.IsRestricted +} + func getUserName(gothUser *goth.User) string { switch setting.OAuth2Client.Username { case setting.OAuth2UsernameEmail: @@ -717,7 +771,15 @@ func handleOAuth2SignIn(ctx *context.Context, source *models.LoginSource, u *mod // Register last login u.SetLastLogin() - if err := models.UpdateUserCols(u, "last_login_unix"); err != nil { + + // Update GroupClaims + changed := setUserGroupClaims(source, u, &gothUser) + cols := []string{"last_login_unix"} + if changed { + cols = append(cols, "is_admin", "is_restricted") + } + + if err := models.UpdateUserCols(u, cols...); err != nil { ctx.ServerError("UpdateUserCols", err) return } @@ -737,6 +799,14 @@ func handleOAuth2SignIn(ctx *context.Context, source *models.LoginSource, u *mod return } + changed := setUserGroupClaims(source, u, &gothUser) + if changed { + if err := models.UpdateUserCols(u, "is_admin", "is_restricted"); err != nil { + ctx.ServerError("UpdateUserCols", err) + return + } + } + // User needs to use 2FA, save data and redirect to 2FA page. if err := ctx.Session.Set("twofaUid", u.ID); err != nil { log.Error("Error setting twofaUid in session: %v", err) diff --git a/services/auth/source/oauth2/source.go b/services/auth/source/oauth2/source.go index 0b2e658eb20a2..e8184db30f8eb 100644 --- a/services/auth/source/oauth2/source.go +++ b/services/auth/source/oauth2/source.go @@ -29,6 +29,9 @@ type Source struct { Scopes []string RequiredClaimName string RequiredClaimValue string + GroupClaimName string + AdminGroup string + RestrictedGroup string // reference to the loginSource loginSource *models.LoginSource diff --git a/services/forms/auth_form.go b/services/forms/auth_form.go index bc56a2bec3cc5..13ca57d6abb98 100644 --- a/services/forms/auth_form.go +++ b/services/forms/auth_form.go @@ -70,6 +70,9 @@ type AuthenticationForm struct { Oauth2Scopes string Oauth2RequiredClaimName string Oauth2RequiredClaimValue string + Oauth2GroupClaimName string + Oauth2AdminGroup string + Oauth2RestrictedGroup string SSPIAutoCreateUsers bool SSPIAutoActivateUsers bool SSPIStripDomainNames bool diff --git a/templates/admin/auth/edit.tmpl b/templates/admin/auth/edit.tmpl index 57f5d7b59cb86..16ee487c4a694 100644 --- a/templates/admin/auth/edit.tmpl +++ b/templates/admin/auth/edit.tmpl @@ -313,6 +313,18 @@

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

+
+ + +
+
+ + +
+
+ + +
{{end}} diff --git a/templates/admin/auth/source/oauth.tmpl b/templates/admin/auth/source/oauth.tmpl index 94b196fed4e89..222c5f6c7c8a2 100644 --- a/templates/admin/auth/source/oauth.tmpl +++ b/templates/admin/auth/source/oauth.tmpl @@ -86,4 +86,16 @@

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

+
+ + +
+
+ + +
+
+ + +
From b51c09cd6aae33e3ef5ee28b5feb87f46c83cda0 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sat, 21 Aug 2021 16:54:50 +0100 Subject: [PATCH 08/13] Allow use of the same group claim name for the prohibit login value Signed-off-by: Andrew Thornton --- routers/web/user/auth.go | 46 ++++++++++++++++++++++++++++------------ 1 file changed, 33 insertions(+), 13 deletions(-) diff --git a/routers/web/user/auth.go b/routers/web/user/auth.go index 55f0d1f06bb2d..f8fafa0961935 100644 --- a/routers/web/user/auth.go +++ b/routers/web/user/auth.go @@ -656,6 +656,23 @@ func SignInOAuthCallback(ctx *context.Context) { handleOAuth2SignIn(ctx, loginSource, u, gothUser) } +func claimValueToStringSlice(claimValue interface{}) []string { + var groups []string + + switch rawGroup := claimValue.(type) { + case []string: + groups = rawGroup + default: + str := fmt.Sprintf("%s", rawGroup) + if strings.Contains(str, ",") { + groups = strings.Split(str, ",") + } else { + groups = []string{str} + } + } + return groups +} + func setUserGroupClaims(loginSource *models.LoginSource, u *models.User, gothUser *goth.User) bool { source := loginSource.Cfg.(*oauth2.Source) @@ -668,18 +685,7 @@ func setUserGroupClaims(loginSource *models.LoginSource, u *models.User, gothUse return false } - var groups []string - - switch rawGroup := groupClaims.(type) { - case []string: - groups = rawGroup - case string: - if strings.Contains(rawGroup, ",") { - groups = strings.Split(rawGroup, ",") - } else { - groups = []string{rawGroup} - } - } + groups := claimValueToStringSlice(groupClaims) wasAdmin, wasRestricted := u.IsAdmin, u.IsRestricted @@ -844,9 +850,23 @@ func oAuth2UserLoginCallback(loginSource *models.LoginSource, request *http.Requ if oauth2Source.RequiredClaimName != "" { claimInterface, has := gothUser.RawData[oauth2Source.RequiredClaimName] - if !has || (oauth2Source.RequiredClaimValue != "" && claimInterface.(string) != oauth2Source.RequiredClaimValue) { + if !has { return nil, goth.User{}, models.ErrUserProhibitLogin{Name: gothUser.UserID} } + + if oauth2Source.RequiredClaimValue != "" { + groups := claimValueToStringSlice(claimInterface) + found := false + for _, group := range groups { + if group == oauth2Source.RequiredClaimValue { + found = true + break + } + } + if !found { + return nil, goth.User{}, models.ErrUserProhibitLogin{Name: gothUser.UserID} + } + } } user := &models.User{ From 11791e5f36b2c9912dc4e1fe27612b18d52fc7b9 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sat, 21 Aug 2021 19:28:36 +0100 Subject: [PATCH 09/13] fixup! Move UpdateExternalUser to externalaccount --- services/externalaccount/link.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/services/externalaccount/link.go b/services/externalaccount/link.go index 109b4523e8919..f16fa728ec2a6 100644 --- a/services/externalaccount/link.go +++ b/services/externalaccount/link.go @@ -12,12 +12,14 @@ import ( "github.com/markbates/goth" ) +// Store represents a thing that stores things type Store interface { Get(interface{}) interface{} Set(interface{}, interface{}) error Release() error } +// LinkAccountFromStore links the provided user with a stored external user func LinkAccountFromStore(store Store, user *models.User) error { gothUser := store.Get("linkAccountGothUser") if gothUser == nil { From 8b8abaaecc4e1b983e22b9894cfde7fe68b6adc6 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Mon, 27 Sep 2021 10:40:50 +0100 Subject: [PATCH 10/13] as per wxiaoguang Signed-off-by: Andrew Thornton --- routers/web/user/auth.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/routers/web/user/auth.go b/routers/web/user/auth.go index fd755e18637be..4f8bb498d383c 100644 --- a/routers/web/user/auth.go +++ b/routers/web/user/auth.go @@ -674,11 +674,7 @@ func claimValueToStringSlice(claimValue interface{}) []string { groups = rawGroup default: str := fmt.Sprintf("%s", rawGroup) - if strings.Contains(str, ",") { - groups = strings.Split(str, ",") - } else { - groups = []string{str} - } + groups = strings.Split(str, ",") } return groups } @@ -714,7 +710,7 @@ func setUserGroupClaims(loginSource *login.Source, u *models.User, gothUser *got } } - return wasAdmin == u.IsAdmin && wasRestricted == u.IsRestricted + return wasAdmin != u.IsAdmin || wasRestricted != u.IsRestricted } func getUserName(gothUser *goth.User) string { From 7a88d060555ac99af56ef149d639406c17d42028 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Thu, 14 Oct 2021 21:18:35 +0100 Subject: [PATCH 11/13] add label back in Signed-off-by: Andrew Thornton --- templates/admin/auth/edit.tmpl | 1 + 1 file changed, 1 insertion(+) diff --git a/templates/admin/auth/edit.tmpl b/templates/admin/auth/edit.tmpl index 00ec10baa6797..c98205c624cec 100644 --- a/templates/admin/auth/edit.tmpl +++ b/templates/admin/auth/edit.tmpl @@ -282,6 +282,7 @@
+

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

From 7cd84d711354ea53431387001256abc7ede8278b Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Thu, 14 Oct 2021 21:22:57 +0100 Subject: [PATCH 12/13] adjust localisation Signed-off-by: Andrew Thornton --- options/locale/locale_en-US.ini | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 356ea2f24f204..31095fc07c5f9 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -2491,9 +2491,9 @@ auths.skip_local_two_fa_helper = Leaving unset means local users with 2FA set wi auths.oauth2_tenant = Tenant auths.oauth2_scopes = Additional Scopes auths.oauth2_required_claim_name = Required Claim Name -auths.oauth2_required_claim_name_helper = Set this name to only allow users to login from this source if the user has a claim with this name +auths.oauth2_required_claim_name_helper = Set this name to restrict login from this source to users with a claim with this name auths.oauth2_required_claim_value = Required Claim Value -auths.oauth2_required_claim_value_helper = Set this value to only allow users to login from this source if the user hasthe claim above with this value +auths.oauth2_required_claim_value_helper = Set this value to restrict login from this source to users with a claim with this name and value auths.enable_auto_register = Enable Auto Registration auths.sspi_auto_create_users = Automatically create users auths.sspi_auto_create_users_helper = Allow SSPI auth method to automatically create new accounts for users that login for the first time From 47ae8f5567e700a2bda789479a3b9b74a5f8a216 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 28 Nov 2021 20:09:11 +0000 Subject: [PATCH 13/13] placate lint Signed-off-by: Andrew Thornton --- services/auth/source/oauth2/providers_simple.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/services/auth/source/oauth2/providers_simple.go b/services/auth/source/oauth2/providers_simple.go index 21b1e9de7aaed..a4d61eb2f3e8c 100644 --- a/services/auth/source/oauth2/providers_simple.go +++ b/services/auth/source/oauth2/providers_simple.go @@ -31,7 +31,9 @@ type SimpleProvider struct { // CreateGothProvider creates a GothProvider from this Provider func (c *SimpleProvider) CreateGothProvider(providerName, callbackURL string, source *Source) (goth.Provider, error) { - scopes := append(c.scopes, source.Scopes...) + scopes := make([]string, len(c.scopes)+len(source.Scopes)) + copy(scopes, c.scopes) + copy(scopes[len(c.scopes):], source.Scopes) return c.newFn(source.ClientID, source.ClientSecret, callbackURL, scopes...), nil }