From 143ee0c49f0d56bdca0849df10acfd03471faf05 Mon Sep 17 00:00:00 2001 From: Eric Leijonmarck Date: Wed, 25 Jan 2023 13:39:54 +0100 Subject: [PATCH] Auth: Add skip_org_role_sync to GitLab OAuth (#62055) * Auth: Add skip_org_role_sync to GitLab OAuth - add: tests - docs added * Update pkg/login/social/gitlab_oauth.go Co-authored-by: Karl Persson * fix: for import Co-authored-by: Karl Persson --- conf/defaults.ini | 1 + conf/sample.ini | 1 + .../setup-grafana/configure-grafana/_index.md | 15 ++++++++++ .../configure-authentication/gitlab/index.md | 13 +++++++++ packages/grafana-data/src/types/config.ts | 1 + pkg/api/frontendsettings.go | 1 + pkg/login/social/gitlab_oauth.go | 28 +++++++++++++------ pkg/login/social/gitlab_oauth_test.go | 15 ++++++++++ pkg/login/social/social.go | 7 +++-- pkg/setting/setting.go | 11 ++++++++ public/app/features/admin/UserAdminPage.tsx | 5 +++- 11 files changed, 85 insertions(+), 13 deletions(-) diff --git a/conf/defaults.ini b/conf/defaults.ini index 2a5884efc9e..56fe0791c0d 100644 --- a/conf/defaults.ini +++ b/conf/defaults.ini @@ -546,6 +546,7 @@ allowed_groups = role_attribute_path = role_attribute_strict = false allow_assign_grafana_admin = false +skip_org_role_sync = false #################################### Google Auth ######################### [auth.google] diff --git a/conf/sample.ini b/conf/sample.ini index f509496244d..799151eb864 100644 --- a/conf/sample.ini +++ b/conf/sample.ini @@ -550,6 +550,7 @@ ;role_attribute_path = ;role_attribute_strict = false ;allow_assign_grafana_admin = false +;skip_org_role_sync = false #################################### Google Auth ########################## [auth.google] diff --git a/docs/sources/setup-grafana/configure-grafana/_index.md b/docs/sources/setup-grafana/configure-grafana/_index.md index dc936faa815..47f502a7da5 100644 --- 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.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. +This also impacts `allow_assign_grafana_admin` setting, by not syncing the grafana admin role from GitLab. + +> **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 | +| --- | --- | --- | --- | +| GitLab | false | false | User organization roles are set with `defaultRole` and cannot be changed | +| Github | true | false | User organization roles are set with `defaultRole` for GitLab, 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. | +| GitLab | false | true | User organization roles are set with `defaultRole`, and the organization role can be changed for GitLab synced users. | +| GitLab | true | true | User organization roles are set with `defaultRole` for GitLab. For other providers, the synchronization is skipped, and the org role can be changed, along with other OAuth provider users' org roles. | + ### 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/gitlab/index.md b/docs/sources/setup-grafana/configure-security/configure-authentication/gitlab/index.md index 333e190083f..2fe7302ab1d 100644 --- a/docs/sources/setup-grafana/configure-security/configure-authentication/gitlab/index.md +++ b/docs/sources/setup-grafana/configure-security/configure-authentication/gitlab/index.md @@ -223,3 +223,16 @@ the correct teams. Your GitLab groups can be referenced in the same way as `allowed_groups`, like `example` or `foo/bar`. [Learn more about Team Sync]({{< relref "../../configure-team-sync/" >}}) + +## Skip organization role sync + +To prevent the sync of organization roles from GitLab, 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 GitLab. + +```ini +[auth.gitlab] +# .. +# prevents the sync of org roles from Github +skip_org_role_sync = true +`` +``` diff --git a/packages/grafana-data/src/types/config.ts b/packages/grafana-data/src/types/config.ts index a9668155e67..5ef7e3943d6 100644 --- 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; + GitLabSkipOrgRoleSync?: boolean; AzureADSkipOrgRoleSync?: boolean; GoogleSkipOrgRoleSync?: boolean; DisableSyncLock?: boolean; diff --git a/pkg/api/frontendsettings.go b/pkg/api/frontendsettings.go index 725b488acc9..9ceb984e53b 100644 --- a/pkg/api/frontendsettings.go +++ b/pkg/api/frontendsettings.go @@ -151,6 +151,7 @@ func (hs *HTTPServer) getFrontendSettingsMap(c *models.ReqContext) (map[string]i "GoogleSkipOrgRoleSync": hs.Cfg.GoogleSkipOrgRoleSync, "JWTAuthSkipOrgRoleSync": hs.Cfg.JWTAuthSkipOrgRoleSync, "GrafanaComSkipOrgRoleSync": hs.Cfg.GrafanaComSkipOrgRoleSync, + "GitLabSkipOrgRoleSync": hs.Cfg.GitLabSkipOrgRoleSync, "AzureADSkipOrgRoleSync": hs.Cfg.AzureADSkipOrgRoleSync, "DisableSyncLock": hs.Cfg.DisableSyncLock, }, diff --git a/pkg/login/social/gitlab_oauth.go b/pkg/login/social/gitlab_oauth.go index c7cd9d51d13..5f0f48ee960 100644 --- a/pkg/login/social/gitlab_oauth.go +++ b/pkg/login/social/gitlab_oauth.go @@ -7,12 +7,15 @@ import ( "regexp" "golang.org/x/oauth2" + + "github.com/grafana/grafana/pkg/models/roletype" ) type SocialGitlab struct { *SocialBase - allowedGroups []string - apiUrl string + allowedGroups []string + apiUrl string + skipOrgRoleSync bool } func (s *SocialGitlab) IsGroupMember(groups []string) bool { @@ -107,14 +110,21 @@ func (s *SocialGitlab) UserInfo(client *http.Client, _ *oauth2.Token) (*BasicUse groups := s.GetGroups(client) - role, grafanaAdmin := s.extractRoleAndAdmin(response.Body, groups, true) - if s.roleAttributeStrict && !role.IsValid() { - return nil, &InvalidBasicRoleError{idP: "Gitlab", assignedRole: string(role)} - } - + var role roletype.RoleType var isGrafanaAdmin *bool = nil - if s.allowAssignGrafanaAdmin { - isGrafanaAdmin = &grafanaAdmin + if !s.skipOrgRoleSync { + var grafanaAdmin bool + role, grafanaAdmin = s.extractRoleAndAdmin(response.Body, groups, true) + if s.roleAttributeStrict && !role.IsValid() { + return nil, &InvalidBasicRoleError{idP: "Gitlab", assignedRole: string(role)} + } + + if s.allowAssignGrafanaAdmin { + isGrafanaAdmin = &grafanaAdmin + } + } + 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{ diff --git a/pkg/login/social/gitlab_oauth_test.go b/pkg/login/social/gitlab_oauth_test.go index e95b4ed326e..188b23e7d44 100644 --- a/pkg/login/social/gitlab_oauth_test.go +++ b/pkg/login/social/gitlab_oauth_test.go @@ -27,16 +27,19 @@ const ( ) func TestSocialGitlab_UserInfo(t *testing.T) { + var nilPointer *bool provider := SocialGitlab{ SocialBase: &SocialBase{ log: newLogger("gitlab_oauth_test", "debug"), }, + skipOrgRoleSync: false, } type conf struct { AllowAssignGrafanaAdmin bool RoleAttributeStrict bool AutoAssignOrgRole org.RoleType + SkipOrgRoleSync bool } tests := []struct { @@ -83,6 +86,17 @@ func TestSocialGitlab_UserInfo(t *testing.T) { ExpectedRole: "Editor", ExpectedGrafanaAdmin: falseBoolPtr(), }, + { + Name: "Should not sync role, return empty role and nil pointer for GrafanaAdmin for skip org role sync set to true", + Cfg: conf{SkipOrgRoleSync: true}, + UserRespBody: editorUserRespBody, + GroupsRespBody: "[" + strings.Join([]string{viewerGroup, editorGroup}, ",") + "]", + RoleAttributePath: gitlabAttrPath, + ExpectedLogin: "gitlab-editor", + ExpectedEmail: "gitlab-editor@example.org", + ExpectedRole: "", + ExpectedGrafanaAdmin: nilPointer, + }, { // Case that's going to change with Grafana 10 Name: "No fallback to default org role (will change in Grafana 10)", Cfg: conf{AutoAssignOrgRole: org.RoleViewer}, @@ -126,6 +140,7 @@ func TestSocialGitlab_UserInfo(t *testing.T) { provider.allowAssignGrafanaAdmin = test.Cfg.AllowAssignGrafanaAdmin provider.autoAssignOrgRole = string(test.Cfg.AutoAssignOrgRole) provider.roleAttributeStrict = test.Cfg.RoleAttributeStrict + 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) { diff --git a/pkg/login/social/social.go b/pkg/login/social/social.go index bc3f68eaea1..5139360bab7 100644 --- a/pkg/login/social/social.go +++ b/pkg/login/social/social.go @@ -152,9 +152,10 @@ func ProvideService(cfg *setting.Cfg, features *featuremgmt.FeatureManager) *Soc // GitLab. if name == "gitlab" { ss.socialMap["gitlab"] = &SocialGitlab{ - SocialBase: newSocialBase(name, &config, info, cfg.AutoAssignOrgRole, cfg.OAuthSkipOrgRoleUpdateSync, *features), - apiUrl: info.ApiUrl, - allowedGroups: util.SplitString(sec.Key("allowed_groups").String()), + SocialBase: newSocialBase(name, &config, info, cfg.AutoAssignOrgRole, cfg.OAuthSkipOrgRoleUpdateSync, *features), + apiUrl: info.ApiUrl, + allowedGroups: util.SplitString(sec.Key("allowed_groups").String()), + skipOrgRoleSync: cfg.GitLabSkipOrgRoleSync, } } diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index 8fac999f44a..e351a070ce5 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -430,6 +430,9 @@ type Cfg struct { // Google GoogleSkipOrgRoleSync bool + // Gitlab + GitLabSkipOrgRoleSync bool + // LDAP LDAPEnabled bool LDAPSkipOrgRoleSync bool @@ -1377,6 +1380,11 @@ func readAuthGoogleSettings(iniFile *ini.File, cfg *Cfg) { cfg.GoogleSkipOrgRoleSync = sec.Key("skip_org_role_sync").MustBool(false) } +func readAuthGitlabSettings(iniFile *ini.File, cfg *Cfg) { + sec := iniFile.Section("auth.gitlab") + cfg.GitLabSkipOrgRoleSync = sec.Key("skip_org_role_sync").MustBool(false) +} + func readAuthSettings(iniFile *ini.File, cfg *Cfg) (err error) { auth := iniFile.Section("auth") @@ -1434,6 +1442,9 @@ func readAuthSettings(iniFile *ini.File, cfg *Cfg) (err error) { // Google Auth readAuthGoogleSettings(iniFile, cfg) + // GitLab Auth + readAuthGitlabSettings(iniFile, cfg) + // anonymous access AnonymousEnabled = iniFile.Section("auth.anonymous").Key("enabled").MustBool(false) cfg.AnonymousEnabled = AnonymousEnabled diff --git a/public/app/features/admin/UserAdminPage.tsx b/public/app/features/admin/UserAdminPage.tsx index 73b38f7c180..302e2ad96ca 100644 --- 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', 'GitLab', 'OAuth']; +const SyncedOAuthLabels: string[] = ['GitHub', 'OAuth']; export class UserAdminPage extends PureComponent { async componentDidMount() { @@ -113,6 +113,7 @@ export class UserAdminPage extends PureComponent { 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 isGitLabUser = user?.isExternal && user?.authLabels?.includes('GitLab'); const isAuthProxyUser = user?.isExternal && user?.authLabels?.includes('Auth Proxy'); const isAzureADUser = user?.isExternal && user?.authLabels?.includes('AzureAD'); const isGrafanaComUser = user?.isExternal && user?.authLabels?.includes('grafana.com'); @@ -122,6 +123,7 @@ export class UserAdminPage extends PureComponent { !( isAuthProxyUser || isGoogleUser || + isGitLabUser || isOAuthUserWithSkippableSync || isSAMLUser || isLDAPUser || @@ -136,6 +138,7 @@ export class UserAdminPage extends PureComponent { // 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.AzureADSkipOrgRoleSync && isAzureADUser) || + (!config.auth.OAuthSkipOrgRoleUpdateSync && !config.auth.GitLabSkipOrgRoleSync && isGitLabUser) || (!config.auth.OAuthSkipOrgRoleUpdateSync && !config.auth.GoogleSkipOrgRoleSync && isGoogleUser)); const pageNav: NavModelItem = {