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 <joao.guerreiro@grafana.com> * Update docs/sources/setup-grafana/configure-security/configure-authentication/grafana-com/index.md Co-authored-by: Jo <joao.guerreiro@grafana.com> * Update docs/sources/setup-grafana/configure-security/configure-authentication/grafana-com/index.md Co-authored-by: Jo <joao.guerreiro@grafana.com> * add blankline * rephrase of doc improvements for explaing of the settings * Update docs/sources/setup-grafana/configure-grafana/_index.md Co-authored-by: Jo <joao.guerreiro@grafana.com> * 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 <ieva.vasiljeva@grafana.com> * Update docs/sources/setup-grafana/configure-grafana/_index.md Co-authored-by: Ieva <ieva.vasiljeva@grafana.com> * Update docs/sources/setup-grafana/configure-grafana/_index.md Co-authored-by: Ieva <ieva.vasiljeva@grafana.com> * Update docs/sources/setup-grafana/configure-grafana/_index.md Co-authored-by: Ieva <ieva.vasiljeva@grafana.com> * Update docs/sources/setup-grafana/configure-grafana/_index.md Co-authored-by: Ieva <ieva.vasiljeva@grafana.com> Co-authored-by: Christopher Moyer <35463610+chri2547@users.noreply.github.com> Co-authored-by: Jo <joao.guerreiro@grafana.com> Co-authored-by: Ieva <ieva.vasiljeva@grafana.com>
2025-02-25 18:55:37 -06:00 · 2023-01-12 16:44:08 +01:00 · 2023-01-12 16:44:08 +01:00 · 91322bebb5
commit 91322bebb5
parent e7b8b82c14
12 changed files with 278 additions and 16 deletions
--- 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]
--- 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 =
+; proxy_address =
--- 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).
--- 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
@ -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
+```
--- 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;
 }
--- 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{}{
--- 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)
+	}
+}
--- 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) {
--- a/pkg/login/social/grafana_com_oauth_test.go
+++ 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)
+		})
+	}
+}
--- 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,
 			}
 		}
 	}
--- 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
 }

--- a/public/app/features/admin/UserAdminPage.tsx
+++ b/public/app/features/admin/UserAdminPage.tsx
@ -113,13 +113,25 @@ export class UserAdminPage extends PureComponent<Props> {
    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 ?? '',