From 91322bebb564b1a8d0bccc860f04078d60a62dff Mon Sep 17 00:00:00 2001 From: Eric Leijonmarck Date: Thu, 12 Jan 2023 16:44:08 +0100 Subject: [PATCH] Auth: Add skip_org_role_sync setting for GrafanaCom (#60553) * add frontend settings and setting for grafanacom * removed println * add skip-org-role-sync on login * add deprecation notice for this field * remove println * remove newline * change and renamed variables * fix for reconfiguring the settings for grafanacom * add documentationf or grafanacom setup * WIP tests * added tests * Update docs/sources/setup-grafana/configure-security/configure-authentication/grafana-com/index.md Co-authored-by: Christopher Moyer <35463610+chri2547@users.noreply.github.com> * Update docs/sources/setup-grafana/configure-security/configure-authentication/grafana-com/index.md Co-authored-by: Christopher Moyer <35463610+chri2547@users.noreply.github.com> * Update docs/sources/setup-grafana/configure-security/configure-authentication/grafana-com/index.md Co-authored-by: Christopher Moyer <35463610+chri2547@users.noreply.github.com> * Update docs/sources/setup-grafana/configure-security/configure-authentication/grafana-com/index.md Co-authored-by: Christopher Moyer <35463610+chri2547@users.noreply.github.com> * Update docs/sources/setup-grafana/configure-security/configure-authentication/grafana-com/index.md Co-authored-by: Christopher Moyer <35463610+chri2547@users.noreply.github.com> * Update docs/sources/setup-grafana/configure-security/configure-authentication/grafana-com/index.md Co-authored-by: Christopher Moyer <35463610+chri2547@users.noreply.github.com> * Update docs/sources/setup-grafana/configure-security/configure-authentication/grafana-com/index.md Co-authored-by: Christopher Moyer <35463610+chri2547@users.noreply.github.com> * Update docs/sources/setup-grafana/configure-security/configure-authentication/grafana-com/index.md Co-authored-by: Christopher Moyer <35463610+chri2547@users.noreply.github.com> * updated steps * Update docs/sources/setup-grafana/configure-security/configure-authentication/grafana-com/index.md Co-authored-by: Christopher Moyer <35463610+chri2547@users.noreply.github.com> * Update docs/sources/setup-grafana/configure-security/configure-authentication/grafana-com/index.md Co-authored-by: Christopher Moyer <35463610+chri2547@users.noreply.github.com> * doc: updated the docs to reflect what happens to grafana.com users * Update docs/sources/setup-grafana/configure-grafana/_index.md Co-authored-by: Jo * Update docs/sources/setup-grafana/configure-security/configure-authentication/grafana-com/index.md Co-authored-by: Jo * Update docs/sources/setup-grafana/configure-security/configure-authentication/grafana-com/index.md Co-authored-by: Jo * add blankline * rephrase of doc improvements for explaing of the settings * Update docs/sources/setup-grafana/configure-grafana/_index.md Co-authored-by: Jo * add frontend setting for grafanacom. * WIP tests * refactor docs * frontend to adhere to skipping org role sync for GrafanaCom users * update docs to reflect desired behavior * tests: added test for skip and nonskip * Update docs/sources/setup-grafana/configure-grafana/_index.md Co-authored-by: Ieva * Update docs/sources/setup-grafana/configure-grafana/_index.md Co-authored-by: Ieva * Update docs/sources/setup-grafana/configure-grafana/_index.md Co-authored-by: Ieva * Update docs/sources/setup-grafana/configure-grafana/_index.md Co-authored-by: Ieva * Update docs/sources/setup-grafana/configure-grafana/_index.md Co-authored-by: Ieva Co-authored-by: Christopher Moyer <35463610+chri2547@users.noreply.github.com> Co-authored-by: Jo Co-authored-by: Ieva --- conf/defaults.ini | 1 + conf/sample.ini | 5 +- .../setup-grafana/configure-grafana/_index.md | 16 ++++ .../grafana-com/index.md | 46 +++++++++ packages/grafana-data/src/types/config.ts | 1 + pkg/api/frontendsettings.go | 1 + pkg/api/login_oauth_test.go | 93 +++++++++++++++--- pkg/login/social/grafana_com_oauth.go | 10 +- pkg/login/social/grafana_com_oauth_test.go | 94 +++++++++++++++++++ pkg/login/social/social.go | 1 + pkg/setting/setting.go | 10 ++ public/app/features/admin/UserAdminPage.tsx | 16 +++- 12 files changed, 278 insertions(+), 16 deletions(-) create mode 100644 docs/sources/setup-grafana/configure-security/configure-authentication/grafana-com/index.md create mode 100644 pkg/login/social/grafana_com_oauth_test.go diff --git a/conf/defaults.ini b/conf/defaults.ini index 922cb5985aa..eeca9aa13d6 100644 --- a/conf/defaults.ini +++ b/conf/defaults.ini @@ -574,6 +574,7 @@ client_id = some_id client_secret = scopes = user:email allowed_organizations = +skip_org_role_sync = false #################################### Azure AD OAuth ####################### [auth.azuread] diff --git a/conf/sample.ini b/conf/sample.ini index 30e4e65cdcc..9fef1b9c5a4 100644 --- a/conf/sample.ini +++ b/conf/sample.ini @@ -566,6 +566,7 @@ ;client_secret = some_secret ;scopes = user:email ;allowed_organizations = +;skip_org_role_sync = false #################################### Azure AD OAuth ####################### [auth.azuread] @@ -1308,7 +1309,7 @@ ; enabled = false ; root_ca_cert = ; client_key = -; client_cert = +; client_cert = ; server_name = # The address of the socks5 proxy datasources should connect to -; proxy_address = \ No newline at end of file +; proxy_address = diff --git a/docs/sources/setup-grafana/configure-grafana/_index.md b/docs/sources/setup-grafana/configure-grafana/_index.md index 95b07406cb4..7c8883d7c9d 100644 --- a/docs/sources/setup-grafana/configure-grafana/_index.md +++ b/docs/sources/setup-grafana/configure-grafana/_index.md @@ -849,6 +849,8 @@ Administrators can increase this if they experience OAuth login state mismatch e ### oauth_skip_org_role_update_sync +> **Note**: This option will soon be a legacy option in favor of OAuth provider specific `skip_org_role_sync` settings. + Skip forced assignment of OrgID `1` or `auto_assign_org_id` for external logins. Default is `false`. Use this setting to allow users with external login to be manually assigned to multiple organizations. @@ -858,6 +860,20 @@ By default, the users' organization and role is reset on every new login. > With Grafana 10, if `oauth_skip_org_role_update_sync` option is set to `false`, users with no mapping will be > reset to the default organization role on every login. [See `auto_assign_org_role` option]({{< relref ".#auto_assign_org_role" >}}). +### [auth.grafana_com] skip_org_role_update_sync + +To prevent synchronization of organization roles for a specific OAuth integration, you can set the `skip_org_role_sync` option to `true`. Please note that there is also a separate setting called `oauth_skip_org_role_update_sync` which has a different scope. While `skip_org_role_sync` only applies to the specific OAuth provider, `oauth_skip_org_role_update_sync` is a generic setting that affects all configured OAuth providers. + +The setting `oauth_skip_org_role_update_sync` will be deprecated in favor of provider-specific settings. + +The table below shows the available OAuth providers and their setting with the default value and the skip org role sync setting. +| OAuth Provider | `oauth_skip_org_role_sync_update` | `skip_org_role_sync` | Behavior | +| --- | --- | --- | --- | +| Grafana.com | false | false | will sync with Grafana.com roles | +| Grafana.com | true | false | skip org role sync for OAuth providers including Grafana.com users | +| Grafana.com | false | true | skip org role sync for grafana.com users | +| Grafana.com | true | true | skip org role sync for Grafana.com users and all other OAuth providers | + ### api_key_max_seconds_to_live Limit of API key seconds to live before expiration. Default is -1 (unlimited). diff --git a/docs/sources/setup-grafana/configure-security/configure-authentication/grafana-com/index.md b/docs/sources/setup-grafana/configure-security/configure-authentication/grafana-com/index.md new file mode 100644 index 00000000000..f53c3448ce2 --- /dev/null +++ b/docs/sources/setup-grafana/configure-security/configure-authentication/grafana-com/index.md @@ -0,0 +1,46 @@ +--- +aliases: + - ../../../auth/grafana-com/ +description: Grafana Com Authentication +title: Configure Grafana Com authentication +weight: 500 +--- + +# Configure Grafana Com authentication + +To enable GrafanaCom as your authentication provider, you configure it to generate a client ID and a secret key. + +## Create GrafanaCom OAuth keys + +To use GrafanaCom authentication: + +1. Log in to [GrafanaCom](https://grafana.com). +1. To create an OAuth client, locate your organization and click **OAuth Clients**. +1. Click **Add OAuth Client Application**. +1. Add the name and URL of your running Grafana instance. +1. Click **Add OAuth Client**. +1. Copy the client ID and secret key or the configuration that has been generated. + +The following snippet shows an example configuration: + +```ini +[auth.grafana_com] +enabled = true +allow_sign_up = true +client_id = 450bc21c10dc2194879d +client_secret = eyJ0Ijoib2F1dGgyYyIhlmlkIjoiNzUwYmMzM2MxMGRjMjE6NDh3OWQiLCJ2IjoiZmI1YzVlYmIwYzFmN2ZhYzZmNjIwOGI1NmVkYTRlNWYxMzgwM2NkMiJ9 +scopes = user:email +allowed_organizations = sampleorganization +enabled = true +``` + +## Skip organization role sync + +To prevent the sync of org roles from Grafana.com, set `skip_org_role_sync` to `true`. This is useful if you want to manage the organization roles for your users from within Grafana. + +```ini +[auth.grafana_com] +# .. +# prevents the sync of org roles from Grafana.com +skip_org_role_sync = true +``` diff --git a/packages/grafana-data/src/types/config.ts b/packages/grafana-data/src/types/config.ts index c6f708056b0..739bde84869 100644 --- a/packages/grafana-data/src/types/config.ts +++ b/packages/grafana-data/src/types/config.ts @@ -223,5 +223,6 @@ export interface AuthSettings { OAuthSkipOrgRoleUpdateSync?: boolean; SAMLSkipOrgRoleSync?: boolean; LDAPSkipOrgRoleSync?: boolean; + GrafanaComSkipOrgRoleSync?: boolean; DisableSyncLock?: boolean; } diff --git a/pkg/api/frontendsettings.go b/pkg/api/frontendsettings.go index 1db72e6ef2e..6e49a583422 100644 --- a/pkg/api/frontendsettings.go +++ b/pkg/api/frontendsettings.go @@ -148,6 +148,7 @@ func (hs *HTTPServer) getFrontendSettingsMap(c *models.ReqContext) (map[string]i "OAuthSkipOrgRoleUpdateSync": hs.Cfg.OAuthSkipOrgRoleUpdateSync, "SAMLSkipOrgRoleSync": hs.Cfg.SectionWithEnvOverrides("auth.saml").Key("skip_org_role_sync").MustBool(false), "LDAPSkipOrgRoleSync": hs.Cfg.LDAPSkipOrgRoleSync, + "GrafanaComSkipOrgRoleSync": hs.Cfg.GrafanaComSkipOrgRoleSync, "DisableSyncLock": hs.Cfg.DisableSyncLock, }, "buildInfo": map[string]interface{}{ diff --git a/pkg/api/login_oauth_test.go b/pkg/api/login_oauth_test.go index f39c5cda96c..b8b798e1deb 100644 --- a/pkg/api/login_oauth_test.go +++ b/pkg/api/login_oauth_test.go @@ -14,14 +14,29 @@ import ( "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/login/social" + "github.com/grafana/grafana/pkg/models/roletype" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/hooks" "github.com/grafana/grafana/pkg/services/licensing" + "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/secrets/fakes" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/web" ) +func setupSocialHTTPServerWithConfig(t *testing.T, cfg *setting.Cfg) *HTTPServer { + sqlStore := db.InitTestDB(t) + + return &HTTPServer{ + Cfg: cfg, + License: &licensing.OSSLicensingService{Cfg: cfg}, + SQLStore: sqlStore, + SocialService: social.ProvideService(cfg, featuremgmt.WithFeatures()), + HooksService: hooks.ProvideService(), + SecretsService: fakes.NewFakeSecretsService(), + } +} + func setupOAuthTest(t *testing.T, cfg *setting.Cfg) *web.Mux { t.Helper() @@ -29,17 +44,7 @@ func setupOAuthTest(t *testing.T, cfg *setting.Cfg) *web.Mux { cfg = setting.NewCfg() } cfg.ErrTemplateName = "error-template" - - sqlStore := db.InitTestDB(t) - - hs := &HTTPServer{ - Cfg: cfg, - License: &licensing.OSSLicensingService{Cfg: cfg}, - SQLStore: sqlStore, - SocialService: social.ProvideService(cfg, featuremgmt.WithFeatures()), - HooksService: hooks.ProvideService(), - SecretsService: fakes.NewFakeSecretsService(), - } + hs := setupSocialHTTPServerWithConfig(t, cfg) m := web.New() m.Use(getContextHandler(t, cfg).Middleware) @@ -159,3 +164,69 @@ func TestOAuthLogin_UsePKCE(t *testing.T) { base64.RawURLEncoding.EncodeToString(shasum[:]), ) } + +func TestOAuthLogin_BuildExternalUserInfo(t *testing.T) { + t.Helper() + cfgOAuthSkipRoleSync := setting.NewCfg() + authOAuthSec := cfgOAuthSkipRoleSync.Raw.Section("auth") + _, err := authOAuthSec.NewKey("oauth_skip_org_role_update_sync", "true") + require.NoError(t, err) + cfgOAuthSkipRoleSync.ErrTemplateName = "error-template" + + cfgOAuthOrgRoleSync := setting.NewCfg() + authOAutoWithoutSec := cfgOAuthOrgRoleSync.Raw.Section("auth") + _, err = authOAutoWithoutSec.NewKey("oauth_skip_org_role_update_sync", "false") + require.NoError(t, err) + cfgOAuthOrgRoleSync.ErrTemplateName = "error-template" + + testcases := []struct { + name string + cfg *setting.Cfg + basicUser *social.BasicUserInfo + expectedOrgRoles map[int64]org.RoleType + }{ + { + name: "should return empty map of org role mapping if the role for the basic info is empty", + cfg: cfgOAuthOrgRoleSync, + basicUser: &social.BasicUserInfo{ + Id: "1", + Name: "first lastname", + Email: "example@github.com", + Login: "example", + Role: "", + }, + expectedOrgRoles: map[int64]org.RoleType{}, + }, + { + name: "should set internal role if role exists and we are skipping org role sync", + cfg: cfgOAuthSkipRoleSync, + basicUser: &social.BasicUserInfo{ + Id: "1", + Name: "first lastname", + Email: "example@github.com", + Login: "example", + Role: roletype.RoleAdmin, + }, + expectedOrgRoles: map[int64]org.RoleType{1: roletype.RoleAdmin}, + }, + { + name: "should return empty external role, if the role for the basic info is empty", + cfg: cfgOAuthSkipRoleSync, + basicUser: &social.BasicUserInfo{ + Id: "1", + Name: "first lastname", + Email: "example@github.com", + Login: "example", + Role: "", + }, + expectedOrgRoles: map[int64]org.RoleType{}, + }, + } + for _, tc := range testcases { + t.Logf("%s", tc.name) + cfg := tc.cfg + hs := setupSocialHTTPServerWithConfig(t, cfg) + externalUser := hs.buildExternalUserInfo(nil, tc.basicUser, "") + require.Equal(t, tc.expectedOrgRoles, externalUser.OrgRoles) + } +} diff --git a/pkg/login/social/grafana_com_oauth.go b/pkg/login/social/grafana_com_oauth.go index 24391871645..7db69cf0aa3 100644 --- a/pkg/login/social/grafana_com_oauth.go +++ b/pkg/login/social/grafana_com_oauth.go @@ -5,6 +5,7 @@ import ( "fmt" "net/http" + "github.com/grafana/grafana/pkg/models/roletype" "github.com/grafana/grafana/pkg/services/org" "golang.org/x/oauth2" @@ -14,6 +15,7 @@ type SocialGrafanaCom struct { *SocialBase url string allowedOrganizations []string + skipOrgRoleSync bool } type OrgRecord struct { @@ -40,6 +42,7 @@ func (s *SocialGrafanaCom) IsOrganizationMember(organizations []OrgRecord) bool return false } +// UserInfo is used for login credentials for the user func (s *SocialGrafanaCom) UserInfo(client *http.Client, _ *oauth2.Token) (*BasicUserInfo, error) { var data struct { Id int `json:"id"` @@ -60,12 +63,17 @@ func (s *SocialGrafanaCom) UserInfo(client *http.Client, _ *oauth2.Token) (*Basi return nil, fmt.Errorf("Error getting user info: %s", err) } + // on login we do not want to display the role from the external provider + var role roletype.RoleType + if !s.skipOrgRoleSync { + role = org.RoleType(data.Role) + } userInfo := &BasicUserInfo{ Id: fmt.Sprintf("%d", data.Id), Name: data.Name, Login: data.Login, Email: data.Email, - Role: org.RoleType(data.Role), + Role: role, } if !s.IsOrganizationMember(data.Orgs) { diff --git a/pkg/login/social/grafana_com_oauth_test.go b/pkg/login/social/grafana_com_oauth_test.go new file mode 100644 index 00000000000..3f322951cb3 --- /dev/null +++ b/pkg/login/social/grafana_com_oauth_test.go @@ -0,0 +1,94 @@ +package social + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/require" +) + +const ( + UserInfoURL = "/api/oauth2/user" + userResponse = `{ + "id": 123, + "name": "grafana name", + "login": "grafanalogin", + "username": "grafanalogin", + "email": "grafana@grafana.com", + "role": "Admin", + "orgs": [ { "login": "grafana", "role": "Admin" } ] + }` +) + +func TestSocialGrafanaCom_UserInfo(t *testing.T) { + provider := SocialGrafanaCom{ + SocialBase: &SocialBase{ + log: newLogger("grafana_com_oauth_test", "debug"), + }, + } + + type conf struct { + skipOrgRoleSync bool + } + + tests := []struct { + Name string + Cfg conf + userInfoResp string + want *BasicUserInfo + ExpectedError error + }{ + { + Name: "should return empty role as userInfo when Skip Org Role Sync Enabled", + userInfoResp: userResponse, + Cfg: conf{skipOrgRoleSync: true}, + want: &BasicUserInfo{ + Id: "1", + Name: "Eric Leijonmarck", + Email: "octocat@github.com", + Login: "octocat", + Role: "", + }, + }, + { + Name: "should return role as userInfo when Skip Org Role Sync Enabled", + userInfoResp: userResponse, + Cfg: conf{skipOrgRoleSync: false}, + want: &BasicUserInfo{ + Id: "1", + Name: "Eric Leijonmarck", + Email: "octocat@github.com", + Login: "octocat", + Role: "Admin", + }, + }, + } + + for _, test := range tests { + provider.skipOrgRoleSync = test.Cfg.skipOrgRoleSync + + t.Run(test.Name, func(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + w.Header().Set("Content-Type", "application/json") + switch r.RequestURI { + case UserInfoURL: + _, err := w.Write([]byte(test.userInfoResp)) + require.NoError(t, err) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + provider.url = ts.URL + actualResult, err := provider.UserInfo(ts.Client(), nil) + if test.ExpectedError != nil { + require.Equal(t, err, test.ExpectedError) + return + } + + require.NoError(t, err) + require.Equal(t, test.want.Role, actualResult.Role) + }) + } +} diff --git a/pkg/login/social/social.go b/pkg/login/social/social.go index fad3ed7095b..19ac4b311de 100644 --- a/pkg/login/social/social.go +++ b/pkg/login/social/social.go @@ -218,6 +218,7 @@ func ProvideService(cfg *setting.Cfg, features *featuremgmt.FeatureManager) *Soc SocialBase: newSocialBase(name, &config, info, cfg.AutoAssignOrgRole, cfg.OAuthSkipOrgRoleUpdateSync, *features), url: cfg.GrafanaComURL, allowedOrganizations: util.SplitString(sec.Key("allowed_organizations").String()), + skipOrgRoleSync: cfg.GrafanaComSkipOrgRoleSync, } } } diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index f9a6e1f96e1..921fdadadaf 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -462,6 +462,10 @@ type Cfg struct { // in case API is not publicly accessible. // Defaults to GrafanaComURL setting + "/api" if unset. GrafanaComAPIURL string + // GrafanaComSkipOrgRoleSync can be set for + // letting users set org roles from within Grafana and + // skip the org roles coming from GrafanaCom + GrafanaComSkipOrgRoleSync bool // Geomap base layer config GeomapDefaultBaseLayerConfig map[string]interface{} @@ -1351,6 +1355,11 @@ func readSecuritySettings(iniFile *ini.File, cfg *Cfg) error { return nil } +func readAuthGrafanaComSettings(iniFile *ini.File, cfg *Cfg) { + sec := iniFile.Section("auth.grafana_com") + cfg.GrafanaComSkipOrgRoleSync = sec.Key("skip_org_role_sync").MustBool(false) +} + func readAuthSettings(iniFile *ini.File, cfg *Cfg) (err error) { auth := iniFile.Section("auth") @@ -1453,6 +1462,7 @@ func readAuthSettings(iniFile *ini.File, cfg *Cfg) (err error) { cfg.AuthProxyHeadersEncoded = authProxy.Key("headers_encoded").MustBool(false) + readAuthGrafanaComSettings(iniFile, cfg) return nil } diff --git a/public/app/features/admin/UserAdminPage.tsx b/public/app/features/admin/UserAdminPage.tsx index 25fb7f584ea..155adb3abd5 100644 --- a/public/app/features/admin/UserAdminPage.tsx +++ b/public/app/features/admin/UserAdminPage.tsx @@ -113,13 +113,25 @@ export class UserAdminPage extends PureComponent { const isSAMLUser = user?.isExternal && user?.authLabels?.includes('SAML'); const isGoogleUser = user?.isExternal && user?.authLabels?.includes('Google'); const isAuthProxyUser = user?.isExternal && user?.authLabels?.includes('Auth Proxy'); + const isGrafanaComUser = user?.isExternal && user?.authLabels?.includes('grafana.com'); + // isGrafanaComUser true + // isOAuthUserWithSkippableSync true const isUserSynced = !config.auth.DisableSyncLock && ((user?.isExternal && - !(isAuthProxyUser || isGoogleUser || isOAuthUserWithSkippableSync || isSAMLUser || isLDAPUser)) || + !( + isAuthProxyUser || + isGoogleUser || + isOAuthUserWithSkippableSync || + isSAMLUser || + isLDAPUser || + isGrafanaComUser + )) || (!config.auth.OAuthSkipOrgRoleUpdateSync && isOAuthUserWithSkippableSync) || (!config.auth.SAMLSkipOrgRoleSync && isSAMLUser) || - (!config.auth.LDAPSkipOrgRoleSync && isLDAPUser)); + (!config.auth.LDAPSkipOrgRoleSync && isLDAPUser) || + // both OAuthSkipOrgRoleUpdateSync and GrafanaComSkipOrgRoleSync needs to be false for a GrafanaComUser to be synced + (!config.auth.OAuthSkipOrgRoleUpdateSync && !config.auth.GrafanaComSkipOrgRoleSync && isGrafanaComUser)); const pageNav: NavModelItem = { text: user?.login ?? '',