Auth: Add skip_org_role_sync setting for github (#61673)

* add: skip_org_role_sync setting for github * fix: frontend * rearranged tests * refactor: assignGrafanaAdmin skip also * Add: tests for allowGrafanaAdmin - both for the case when both settings are set and the setting for only allowGrafanaAdmin * Apply suggestions from code review Co-authored-by: Christopher Moyer <35463610+chri2547@users.noreply.github.com> * Update docs/sources/setup-grafana/configure-grafana/_index.md Co-authored-by: Christopher Moyer <35463610+chri2547@users.noreply.github.com> * Update pkg/login/social/github_oauth.go Co-authored-by: Ieva <ieva.vasiljeva@grafana.com> * added vairable inside scope * Update docs/sources/setup-grafana/configure-security/configure-authentication/github/index.md * Update docs/sources/setup-grafana/configure-security/configure-authentication/github/index.md Co-authored-by: Christopher Moyer <35463610+chri2547@users.noreply.github.com> Co-authored-by: Ieva <ieva.vasiljeva@grafana.com>
2025-02-10 23:55:47 -06:00 · 2023-01-25 15:16:08 +01:00 · 2023-01-25 15:16:08 +01:00 · 6bd11e0ebf
commit 6bd11e0ebf
parent 529e6c379f
9 changed files with 103 additions and 7 deletions
--- a/docs/sources/setup-grafana/configure-grafana/_index.md
+++ b/docs/sources/setup-grafana/configure-grafana/_index.md
@ -904,6 +904,21 @@ The following table shows the OAuth provider's setting with the default value an
 | Google | false | true | User organization roles are set with `defaultRole` and the org role can be changed for Google synced users. |
 | Google | true | true | User organization roles are set with `defaultRole` for Google. For other providers, the synchronization will be skipped, and the org role can be changed, along with other OAuth provider users' org roles. |

+### [auth.github] skip_org_role_sync
+
+When a user logs in the first time, Grafana sets the organization role based on the value specified in `AutoAssignOrgRole`. If you want to manage organization roles, set the `skip_org_role_sync` option to `true`. GitHub syncs organization roles and sets Grafana Admins.
+This also impacts `allow_assign_grafana_admin` setting, by not syncing the grafana admin role from GitHub.
+
+> **Note:** There is 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 following table shows the OAuth provider's 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 |
+| --- | --- | --- | --- |
+| GitHub | false | false | User organization roles are set with `defaultRole` and cannot be changed |
+| Github | true | false | User organization roles are set with `defaultRole` for GitHub, and Grafana Admins are set. For other providers, the synchronization is skipped, and the org role can be changed, along with other OAuth provider users' org roles. |
+| GitHub | false | true | User organization roles are set with `defaultRole`, and the organization role can be changed for GitHub synced users. |
+| GitHub | true | true | User organization roles are set with `defaultRole` for Google. For other providers, the synchronization is skipped, and the org role can be changed, along with other OAuth provider users' org roles. |
+
 ### [auth.gitlab] skip_org_role_sync

 When a user logs in the first time, Grafana sets the organization role based on the value specified in `AutoAssignOrgRole`. If you want to manage organization roles, set the `skip_org_role_sync` option to `true`. GitLab syncs organization roles and sets Grafana Admins.
--- a/docs/sources/setup-grafana/configure-security/configure-authentication/github/index.md
+++ b/docs/sources/setup-grafana/configure-security/configure-authentication/github/index.md
@ -203,3 +203,16 @@ Your GitHub teams can be referenced in two ways:
 Example: `@grafana/developers`

 [Learn more about Team Sync]({{< relref "../../configure-team-sync/" >}})
+
+## Skip organization role sync
+
+To prevent the sync of organization roles from GitHub, set `skip_org_role_sync` to `true`. This is useful if you want to manage the organization roles for your users from within Grafana.
+This also impacts the `allow_assign_grafana_admin` setting by not syncing the Grafana admin role from GitHub.
+
+```ini
+[auth.github]
+# ..
+# prevents the sync of org roles from Github
+skip_org_role_sync = true
+``
+```
--- a/packages/grafana-data/src/types/config.ts
+++ b/packages/grafana-data/src/types/config.ts
@ -226,6 +226,7 @@ export interface AuthSettings {
  LDAPSkipOrgRoleSync?: boolean;
  JWTAuthSkipOrgRoleSync?: boolean;
  GrafanaComSkipOrgRoleSync?: boolean;
+  GithubSkipOrgRoleSync?: boolean;
  GitLabSkipOrgRoleSync?: boolean;
  AzureADSkipOrgRoleSync?: boolean;
  GoogleSkipOrgRoleSync?: 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,
+			"GithubSkipOrgRoleSync":      hs.Cfg.GithubSkipOrgRoleSync,
 			"GoogleSkipOrgRoleSync":      hs.Cfg.GoogleSkipOrgRoleSync,
 			"JWTAuthSkipOrgRoleSync":     hs.Cfg.JWTAuthSkipOrgRoleSync,
 			"GrafanaComSkipOrgRoleSync":  hs.Cfg.GrafanaComSkipOrgRoleSync,
--- a/pkg/login/social/github_oauth.go
+++ b/pkg/login/social/github_oauth.go
@ -8,6 +8,8 @@ import (
 	"regexp"

 	"golang.org/x/oauth2"
+
+	"github.com/grafana/grafana/pkg/models/roletype"
 )

 type SocialGithub struct {
@ -15,6 +17,7 @@ type SocialGithub struct {
 	allowedOrganizations []string
 	apiUrl               string
 	teamIds              []int
+	skipOrgRoleSync      bool
 }

 type GithubTeam struct {
@ -201,14 +204,25 @@ func (s *SocialGithub) UserInfo(client *http.Client, token *oauth2.Token) (*Basi

 	teams := convertToGroupList(teamMemberships)

-	role, grafanaAdmin := s.extractRoleAndAdmin(response.Body, teams, true)
-	if s.roleAttributeStrict && !role.IsValid() {
-		return nil, &InvalidBasicRoleError{idP: "Github", assignedRole: string(role)}
+	var role roletype.RoleType
+	var isGrafanaAdmin *bool = nil
+
+	if !s.skipOrgRoleSync {
+		var grafanaAdmin bool
+		role, grafanaAdmin = s.extractRoleAndAdmin(response.Body, teams, true)
+
+		if s.roleAttributeStrict && !role.IsValid() {
+			return nil, &InvalidBasicRoleError{idP: "Github", assignedRole: string(role)}
+		}
+
+		if s.allowAssignGrafanaAdmin {
+			isGrafanaAdmin = &grafanaAdmin
+		}
 	}

-	var isGrafanaAdmin *bool = nil
-	if s.allowAssignGrafanaAdmin {
-		isGrafanaAdmin = &grafanaAdmin
+	// we skip allowing assignment of GrafanaAdmin if skipOrgRoleSync is present
+	if s.allowAssignGrafanaAdmin && s.skipOrgRoleSync {
+		s.log.Debug("allowAssignGrafanaAdmin and skipOrgRoleSync are both set, Grafana Admin role will not be synced, consider setting one or the other")
 	}

 	userInfo := &BasicUserInfo{
--- a/pkg/login/social/github_oauth_test.go
+++ b/pkg/login/social/github_oauth_test.go
@ -112,11 +112,14 @@ const testGHUserJSON = `{
 }`

 func TestSocialGitHub_UserInfo(t *testing.T) {
+	var boolPointer *bool
 	tests := []struct {
 		name                     string
 		userRawJSON              string
 		userTeamsRawJSON         string
 		settingAutoAssignOrgRole string
+		settingAllowGrafanaAdmin bool
+		settingSkipOrgRoleSync   bool
 		roleAttributePath        string
 		autoAssignOrgRole        string
 		want                     *BasicUserInfo
@ -167,6 +170,38 @@ func TestSocialGitHub_UserInfo(t *testing.T) {
 				Groups: []string{"https://github.com/orgs/github/teams/justice-league", "@github/justice-league"},
 			},
 		},
+		{
+			name:                   "Should be empty role if setting skipOrgRoleSync is set to true",
+			roleAttributePath:      "contains(groups[*], '@github/justice-league') && 'Editor' || 'Viewer'",
+			settingSkipOrgRoleSync: true,
+			userRawJSON:            testGHUserJSON,
+			userTeamsRawJSON:       testGHUserTeamsJSON,
+			want: &BasicUserInfo{
+				Id:     "1",
+				Name:   "monalisa octocat",
+				Email:  "octocat@github.com",
+				Login:  "octocat",
+				Role:   "",
+				Groups: []string{"https://github.com/orgs/github/teams/justice-league", "@github/justice-league"},
+			},
+		},
+		{
+			name:                     "Should return nil pointer if allowGrafanaAdmin and skipOrgRoleSync setting is set to true",
+			roleAttributePath:        "contains(groups[*], '@github/justice-league') && 'Editor' || 'Viewer'",
+			settingSkipOrgRoleSync:   true,
+			settingAllowGrafanaAdmin: true,
+			userRawJSON:              testGHUserJSON,
+			userTeamsRawJSON:         testGHUserTeamsJSON,
+			want: &BasicUserInfo{
+				Id:             "1",
+				Name:           "monalisa octocat",
+				Email:          "octocat@github.com",
+				Login:          "octocat",
+				Role:           "",
+				Groups:         []string{"https://github.com/orgs/github/teams/justice-league", "@github/justice-league"},
+				IsGrafanaAdmin: boolPointer,
+			},
+		},
 		{ // Case that's going to change with Grafana 10
 			name:              "No fallback to default org role (will change in Grafana 10)",
 			roleAttributePath: "",
@ -208,6 +243,7 @@ func TestSocialGitHub_UserInfo(t *testing.T) {
 				allowedOrganizations: []string{},
 				apiUrl:               server.URL + "/user",
 				teamIds:              []int{},
+				skipOrgRoleSync:      tt.settingSkipOrgRoleSync,
 			}

 			token := &oauth2.Token{
--- a/pkg/login/social/social.go
+++ b/pkg/login/social/social.go
@ -146,6 +146,7 @@ func ProvideService(cfg *setting.Cfg, features *featuremgmt.FeatureManager) *Soc
 				apiUrl:               info.ApiUrl,
 				teamIds:              sec.Key("team_ids").Ints(","),
 				allowedOrganizations: util.SplitString(sec.Key("allowed_organizations").String()),
+				skipOrgRoleSync:      cfg.GithubSkipOrgRoleSync,
 			}
 		}

--- a/pkg/setting/setting.go
+++ b/pkg/setting/setting.go
@ -467,6 +467,9 @@ type Cfg struct {
 	// then Live uses AppURL as the only allowed origin.
 	LiveAllowedOrigins []string

+	// Github OAuth
+	GithubSkipOrgRoleSync bool
+
 	// Grafana.com URL, used for OAuth redirect.
 	GrafanaComURL string
 	// Grafana.com API URL. Can be set separately to GrafanaComURL
@ -1375,6 +1378,11 @@ func readAuthGrafanaComSettings(iniFile *ini.File, cfg *Cfg) {
 	cfg.GrafanaComSkipOrgRoleSync = sec.Key("skip_org_role_sync").MustBool(false)
 }

+func readAuthGithubSettings(iniFile *ini.File, cfg *Cfg) {
+	sec := iniFile.Section("auth.github")
+	cfg.GithubSkipOrgRoleSync = sec.Key("skip_org_role_sync").MustBool(false)
+}
+
 func readAuthGoogleSettings(iniFile *ini.File, cfg *Cfg) {
 	sec := iniFile.Section("auth.google")
 	cfg.GoogleSkipOrgRoleSync = sec.Key("skip_org_role_sync").MustBool(false)
@ -1501,7 +1509,11 @@ func readAuthSettings(iniFile *ini.File, cfg *Cfg) (err error) {

 	cfg.AuthProxyHeadersEncoded = authProxy.Key("headers_encoded").MustBool(false)

+	// GrafanaCom
 	readAuthGrafanaComSettings(iniFile, cfg)
+
+	// Github
+	readAuthGithubSettings(iniFile, cfg)
 	return nil
 }

--- a/public/app/features/admin/UserAdminPage.tsx
+++ b/public/app/features/admin/UserAdminPage.tsx
@ -39,7 +39,7 @@ interface OwnProps extends GrafanaRouteComponentProps<{ id: string }> {
  error?: UserAdminError;
 }

-const SyncedOAuthLabels: string[] = ['GitHub', 'OAuth'];
+const SyncedOAuthLabels: string[] = ['OAuth'];

 export class UserAdminPage extends PureComponent<Props> {
  async componentDidMount() {
@ -113,6 +113,7 @@ export class UserAdminPage extends PureComponent<Props> {
      user?.isExternal && user?.authLabels?.some((r) => SyncedOAuthLabels.includes(r));
    const isSAMLUser = user?.isExternal && user?.authLabels?.includes('SAML');
    const isGoogleUser = user?.isExternal && user?.authLabels?.includes('Google');
+    const isGithubUser = user?.isExternal && user?.authLabels?.includes('GitHub');
    const isGitLabUser = user?.isExternal && user?.authLabels?.includes('GitLab');
    const isAuthProxyUser = user?.isExternal && user?.authLabels?.includes('Auth Proxy');
    const isAzureADUser = user?.isExternal && user?.authLabels?.includes('AzureAD');
@ -127,6 +128,7 @@ export class UserAdminPage extends PureComponent<Props> {
          isOAuthUserWithSkippableSync ||
          isSAMLUser ||
          isLDAPUser ||
+          isGithubUser ||
          isAzureADUser ||
          isJWTUser ||
          isGrafanaComUser
@ -137,6 +139,7 @@ export class UserAdminPage extends PureComponent<Props> {
        (!config.auth.JWTAuthSkipOrgRoleSync && isJWTUser) ||
        // both OAuthSkipOrgRoleUpdateSync and specific provider settings needs to be false for a user to be synced
        (!config.auth.OAuthSkipOrgRoleUpdateSync && !config.auth.GrafanaComSkipOrgRoleSync && isGrafanaComUser) ||
+        (!config.auth.OAuthSkipOrgRoleUpdateSync && !config.auth.GithubSkipOrgRoleSync && isGithubUser) ||
        (!config.auth.OAuthSkipOrgRoleUpdateSync && !config.auth.AzureADSkipOrgRoleSync && isAzureADUser) ||
        (!config.auth.OAuthSkipOrgRoleUpdateSync && !config.auth.GitLabSkipOrgRoleSync && isGitLabUser) ||
        (!config.auth.OAuthSkipOrgRoleUpdateSync && !config.auth.GoogleSkipOrgRoleSync && isGoogleUser));