From 8ff19bd9019e313d42b9565a1fc5667a3a1739e7 Mon Sep 17 00:00:00 2001 From: Eric Leijonmarck Date: Wed, 1 Feb 2023 16:27:53 +0000 Subject: [PATCH] Auth: Add Generic oauth skip org role sync setting (#62418) * add: generic oauth skip org role sync * add: docs * add: backend login skip sync * fix: docs typo * add: tests * remove public key * fix markdown for generic oauth * add: generic oauth to the configuration * refactor: change debug to warn --- conf/defaults.ini | 1 + .../setup-grafana/configure-grafana/_index.md | 17 +++++++++++++- .../generic-oauth/index.md | 15 +++++++++++- packages/grafana-data/src/types/config.ts | 1 + pkg/api/dtos/frontend_settings.go | 23 ++++++++++--------- pkg/api/frontendsettings.go | 23 ++++++++++--------- pkg/login/social/generic_oauth.go | 17 +++++++++----- pkg/login/social/generic_oauth_test.go | 19 +++++++++++++++ pkg/services/login/authinfo.go | 4 +++- pkg/setting/setting.go | 16 +++++++++++-- public/app/features/admin/UserAdminPage.tsx | 9 +++----- .../app/features/admin/UserListAdminPage.tsx | 2 +- 12 files changed, 107 insertions(+), 40 deletions(-) diff --git a/conf/defaults.ini b/conf/defaults.ini index 539e1bee548..730b7e1c3de 100644 --- a/conf/defaults.ini +++ b/conf/defaults.ini @@ -657,6 +657,7 @@ tls_client_ca = use_pkce = false auth_style = allow_assign_grafana_admin = false +skip_org_role_sync = false #################################### Basic Auth ########################## [auth.basic] diff --git a/docs/sources/setup-grafana/configure-grafana/_index.md b/docs/sources/setup-grafana/configure-grafana/_index.md index bc213e0bd06..ecba4fbc69b 100644 --- a/docs/sources/setup-grafana/configure-grafana/_index.md +++ b/docs/sources/setup-grafana/configure-grafana/_index.md @@ -936,10 +936,25 @@ The following table shows the OAuth provider's setting with the default value an | 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 | 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. | +### [auth.generic_oauth] 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`. the OAuth provider syncs organization roles and sets Grafana Admins. +This also impacts `allow_assign_grafana_admin` setting, by not syncing the grafana admin role from the OAuth provider. + +> **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 | +| --- | --- | --- | --- | +| Generic OAuth | false | false | User organization roles are set with `defaultRole` and cannot be changed | +| Generic OAuth | true | false | User organization roles are set with `defaultRole` for Generic OAuth, 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. | +| Generic OAuth | false | true | User organization roles are set with `defaultRole`, and the organization role can be changed for Generic OAuth synced users. | +| Generic OAuth | true | true | User organization roles are set with `defaultRole` for Generic OAuth. For other providers, the synchronization is skipped, and the org role can be changed, along with other OAuth provider users' org roles. | + ### [auth.okta] 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 through Grafana's UI, set the `skip_org_role_sync` option to `true`. diff --git a/docs/sources/setup-grafana/configure-security/configure-authentication/generic-oauth/index.md b/docs/sources/setup-grafana/configure-security/configure-authentication/generic-oauth/index.md index 91252194de5..9a07e500f11 100644 --- a/docs/sources/setup-grafana/configure-security/configure-authentication/generic-oauth/index.md +++ b/docs/sources/setup-grafana/configure-security/configure-authentication/generic-oauth/index.md @@ -217,7 +217,7 @@ By default, a refresh token is included in the response for the **Authorization allow_sign_up = true auto_login = false client_id = - client_secret = scopes = openid profile email auth_url = https://.my.centrify.com/OAuth2/Authorize/ token_url = https://.my.centrify.com/OAuth2/Token/ @@ -392,3 +392,16 @@ Payload: ... } ``` + +## Skip organization role sync + +To prevent the sync of organization roles from the OAuth provider, 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 the OAuth provider. + +```ini +[auth.generic_oauth] +# .. +# prevents the sync of org roles from the Oauth provider +skip_org_role_sync = true +`` +``` diff --git a/packages/grafana-data/src/types/config.ts b/packages/grafana-data/src/types/config.ts index d4c9381f6f7..7df2e80381e 100644 --- a/packages/grafana-data/src/types/config.ts +++ b/packages/grafana-data/src/types/config.ts @@ -235,5 +235,6 @@ export interface AuthSettings { OktaSkipOrgRoleSync?: boolean; AzureADSkipOrgRoleSync?: boolean; GoogleSkipOrgRoleSync?: boolean; + GenericOAuthSkipOrgRoleSync?: boolean; DisableSyncLock?: boolean; } diff --git a/pkg/api/dtos/frontend_settings.go b/pkg/api/dtos/frontend_settings.go index 778ab9683d3..edd62f527df 100644 --- a/pkg/api/dtos/frontend_settings.go +++ b/pkg/api/dtos/frontend_settings.go @@ -6,17 +6,18 @@ import ( ) type FrontendSettingsAuthDTO struct { - OAuthSkipOrgRoleUpdateSync bool `json:"OAuthSkipOrgRoleUpdateSync"` - SAMLSkipOrgRoleSync bool `json:"SAMLSkipOrgRoleSync"` - LDAPSkipOrgRoleSync bool `json:"LDAPSkipOrgRoleSync"` - GoogleSkipOrgRoleSync bool `json:"GoogleSkipOrgRoleSync"` - JWTAuthSkipOrgRoleSync bool `json:"JWTAuthSkipOrgRoleSync"` - GrafanaComSkipOrgRoleSync bool `json:"GrafanaComSkipOrgRoleSync"` - AzureADSkipOrgRoleSync bool `json:"AzureADSkipOrgRoleSync"` - GithubSkipOrgRoleSync bool `json:"GithubSkipOrgRoleSync"` - GitLabSkipOrgRoleSync bool `json:"GitLabSkipOrgRoleSync"` - OktaSkipOrgRoleSync bool `json:"OktaSkipOrgRoleSync"` - DisableSyncLock bool `json:"DisableSyncLock"` + OAuthSkipOrgRoleUpdateSync bool `json:"OAuthSkipOrgRoleUpdateSync"` + SAMLSkipOrgRoleSync bool `json:"SAMLSkipOrgRoleSync"` + LDAPSkipOrgRoleSync bool `json:"LDAPSkipOrgRoleSync"` + GoogleSkipOrgRoleSync bool `json:"GoogleSkipOrgRoleSync"` + GenericOAuthSkipOrgRoleSync bool `json:"GenericOAuthSkipOrgRoleSync"` + JWTAuthSkipOrgRoleSync bool `json:"JWTAuthSkipOrgRoleSync"` + GrafanaComSkipOrgRoleSync bool `json:"GrafanaComSkipOrgRoleSync"` + AzureADSkipOrgRoleSync bool `json:"AzureADSkipOrgRoleSync"` + GithubSkipOrgRoleSync bool `json:"GithubSkipOrgRoleSync"` + GitLabSkipOrgRoleSync bool `json:"GitLabSkipOrgRoleSync"` + OktaSkipOrgRoleSync bool `json:"OktaSkipOrgRoleSync"` + DisableSyncLock bool `json:"DisableSyncLock"` } type FrontendSettingsBuildInfoDTO struct { diff --git a/pkg/api/frontendsettings.go b/pkg/api/frontendsettings.go index ebdb2c50308..108ad16e7e7 100644 --- a/pkg/api/frontendsettings.go +++ b/pkg/api/frontendsettings.go @@ -148,17 +148,18 @@ func (hs *HTTPServer) getFrontendSettings(c *contextmodel.ReqContext) (*dtos.Fro DateFormats: hs.Cfg.DateFormats, Auth: dtos.FrontendSettingsAuthDTO{ - OAuthSkipOrgRoleUpdateSync: hs.Cfg.OAuthSkipOrgRoleUpdateSync, - SAMLSkipOrgRoleSync: hs.Cfg.SectionWithEnvOverrides("auth.saml").Key("skip_org_role_sync").MustBool(false), - LDAPSkipOrgRoleSync: hs.Cfg.LDAPSkipOrgRoleSync, - GoogleSkipOrgRoleSync: hs.Cfg.GoogleSkipOrgRoleSync, - JWTAuthSkipOrgRoleSync: hs.Cfg.JWTAuthSkipOrgRoleSync, - GrafanaComSkipOrgRoleSync: hs.Cfg.GrafanaComSkipOrgRoleSync, - AzureADSkipOrgRoleSync: hs.Cfg.AzureADSkipOrgRoleSync, - GithubSkipOrgRoleSync: hs.Cfg.GithubSkipOrgRoleSync, - GitLabSkipOrgRoleSync: hs.Cfg.GitLabSkipOrgRoleSync, - OktaSkipOrgRoleSync: hs.Cfg.OktaSkipOrgRoleSync, - DisableSyncLock: hs.Cfg.DisableSyncLock, + OAuthSkipOrgRoleUpdateSync: hs.Cfg.OAuthSkipOrgRoleUpdateSync, + SAMLSkipOrgRoleSync: hs.Cfg.SectionWithEnvOverrides("auth.saml").Key("skip_org_role_sync").MustBool(false), + LDAPSkipOrgRoleSync: hs.Cfg.LDAPSkipOrgRoleSync, + GoogleSkipOrgRoleSync: hs.Cfg.GoogleSkipOrgRoleSync, + JWTAuthSkipOrgRoleSync: hs.Cfg.JWTAuthSkipOrgRoleSync, + GrafanaComSkipOrgRoleSync: hs.Cfg.GrafanaComSkipOrgRoleSync, + GenericOAuthSkipOrgRoleSync: hs.Cfg.GenericOAuthSkipOrgRoleSync, + AzureADSkipOrgRoleSync: hs.Cfg.AzureADSkipOrgRoleSync, + GithubSkipOrgRoleSync: hs.Cfg.GithubSkipOrgRoleSync, + GitLabSkipOrgRoleSync: hs.Cfg.GitLabSkipOrgRoleSync, + OktaSkipOrgRoleSync: hs.Cfg.OktaSkipOrgRoleSync, + DisableSyncLock: hs.Cfg.DisableSyncLock, }, BuildInfo: dtos.FrontendSettingsBuildInfoDTO{ diff --git a/pkg/login/social/generic_oauth.go b/pkg/login/social/generic_oauth.go index d0cd1bc716d..7a0425ce3a2 100644 --- a/pkg/login/social/generic_oauth.go +++ b/pkg/login/social/generic_oauth.go @@ -143,15 +143,20 @@ func (s *SocialGenericOAuth) UserInfo(client *http.Client, token *oauth2.Token) } if userInfo.Role == "" { - role, grafanaAdmin := s.extractRoleAndAdmin(data.rawJSON, []string{}, true) - if role != "" { - s.log.Debug("Setting user info role from extracted role") + if !s.skipOrgRoleSync { + role, grafanaAdmin := s.extractRoleAndAdmin(data.rawJSON, []string{}, true) + if role != "" { + s.log.Debug("Setting user info role from extracted role") - userInfo.Role = role - if s.allowAssignGrafanaAdmin { - userInfo.IsGrafanaAdmin = &grafanaAdmin + userInfo.Role = role + if s.allowAssignGrafanaAdmin { + userInfo.IsGrafanaAdmin = &grafanaAdmin + } } } + if s.allowAssignGrafanaAdmin && s.skipOrgRoleSync { + s.log.Warn("allowAssignGrafanaAdmin and skipOrgRoleSync are both set, Grafana Admin role will not be synced, consider setting one or the other") + } } if len(userInfo.Groups) == 0 { diff --git a/pkg/login/social/generic_oauth_test.go b/pkg/login/social/generic_oauth_test.go index 40f19e3a3d9..a9e3a3537bc 100644 --- a/pkg/login/social/generic_oauth_test.go +++ b/pkg/login/social/generic_oauth_test.go @@ -247,6 +247,7 @@ func TestUserInfoSearchesForEmailAndRole(t *testing.T) { tests := []struct { Name string + SkipOrgRoleSync bool AllowAssignGrafanaAdmin bool ResponseBody interface{} OAuth2Extra interface{} @@ -447,11 +448,29 @@ func TestUserInfoSearchesForEmailAndRole(t *testing.T) { ExpectedEmail: "john.doe@example.com", ExpectedRole: "Editor", }, + { + Name: "Given skip org role sync set to true, with a valid id_token, a valid advanced JMESPath role path, a valid API response, no org role should be set", + SkipOrgRoleSync: true, + OAuth2Extra: map[string]interface{}{ + // { "email": "john.doe@example.com", + // "info": { "roles": [ "dev", "engineering" ] }} + "id_token": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJlbWFpbCI6ImpvaG4uZG9lQGV4YW1wbGUuY29tIiwiaW5mbyI6eyJyb2xlcyI6WyJkZXYiLCJlbmdpbmVlcmluZyJdfX0.RmmQfv25eXb4p3wMrJsvXfGQ6EXhGtwRXo6SlCFHRNg", + }, + ResponseBody: map[string]interface{}{ + "info": map[string]interface{}{ + "roles": []string{"engineering", "SRE"}, + }, + }, + RoleAttributePath: "contains(info.roles[*], 'SRE') && 'Admin' || contains(info.roles[*], 'dev') && 'Editor' || 'Viewer'", + ExpectedEmail: "john.doe@example.com", + ExpectedRole: "", + }, } for _, test := range tests { provider.roleAttributePath = test.RoleAttributePath provider.allowAssignGrafanaAdmin = test.AllowAssignGrafanaAdmin + provider.skipOrgRoleSync = test.SkipOrgRoleSync t.Run(test.Name, func(t *testing.T) { body, err := json.Marshal(test.ResponseBody) diff --git a/pkg/services/login/authinfo.go b/pkg/services/login/authinfo.go index ee0146cd5d4..d44f088ed8a 100644 --- a/pkg/services/login/authinfo.go +++ b/pkg/services/login/authinfo.go @@ -46,7 +46,9 @@ func GetAuthProviderLabel(authModule string) string { return "JWT" case AuthProxyAuthModule: return "Auth Proxy" + case "oauth_generic_oauth": + return "Generic OAuth" default: - return "OAuth" // FIXME: replace with "Unknown" and handle generic oauth as a case + return "Unknown" } } diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index 4734f5c12c1..fa484bb6934 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -22,11 +22,12 @@ import ( "time" "github.com/gobwas/glob" + "github.com/prometheus/common/model" + "gopkg.in/ini.v1" + "github.com/grafana/grafana-aws-sdk/pkg/awsds" "github.com/grafana/grafana-azure-sdk-go/azsettings" "github.com/grafana/grafana-plugin-sdk-go/backend/gtime" - "github.com/prometheus/common/model" - "gopkg.in/ini.v1" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/util" @@ -434,6 +435,9 @@ type Cfg struct { // Gitlab GitLabSkipOrgRoleSync bool + // Generic OAuth + GenericOAuthSkipOrgRoleSync bool + // LDAP LDAPEnabled bool LDAPSkipOrgRoleSync bool @@ -1397,6 +1401,11 @@ func readAuthGitlabSettings(iniFile *ini.File, cfg *Cfg) { cfg.GitLabSkipOrgRoleSync = sec.Key("skip_org_role_sync").MustBool(false) } +func readGenericOAuthSettings(iniFile *ini.File, cfg *Cfg) { + sec := iniFile.Section("auth.generic_oauth") + cfg.GenericOAuthSkipOrgRoleSync = sec.Key("skip_org_role_sync").MustBool(false) +} + func readAuthOktaSettings(iniFile *ini.File, cfg *Cfg) { sec := iniFile.Section("auth.okta") cfg.OktaSkipOrgRoleSync = sec.Key("skip_org_role_sync").MustBool(false) @@ -1462,6 +1471,9 @@ func readAuthSettings(iniFile *ini.File, cfg *Cfg) (err error) { // GitLab Auth readAuthGitlabSettings(iniFile, cfg) + // Generic OAuth + readGenericOAuthSettings(iniFile, cfg) + // Okta Auth readAuthOktaSettings(iniFile, cfg) diff --git a/public/app/features/admin/UserAdminPage.tsx b/public/app/features/admin/UserAdminPage.tsx index dcf0ba5dee7..da70959ee2e 100644 --- a/public/app/features/admin/UserAdminPage.tsx +++ b/public/app/features/admin/UserAdminPage.tsx @@ -39,8 +39,6 @@ interface OwnProps extends GrafanaRouteComponentProps<{ id: string }> { error?: UserAdminError; } -const SyncedOAuthLabels: string[] = ['OAuth']; - export class UserAdminPage extends PureComponent { async componentDidMount() { const { match, loadAdminUserPage } = this.props; @@ -109,8 +107,6 @@ export class UserAdminPage extends PureComponent { const isJWTUser = user?.authLabels?.includes('JWT'); const canReadSessions = contextSrv.hasPermission(AccessControlAction.UsersAuthTokenList); const canReadLDAPStatus = contextSrv.hasPermission(AccessControlAction.LDAPStatusRead); - const isOAuthUserWithSkippableSync = - 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'); @@ -119,6 +115,7 @@ export class UserAdminPage extends PureComponent { const isAzureADUser = user?.isExternal && user?.authLabels?.includes('AzureAD'); const isOktaUser = user?.isExternal && user?.authLabels?.includes('Okta'); const isGrafanaComUser = user?.isExternal && user?.authLabels?.includes('grafana.com'); + const isGenericOAuthUser = user?.isExternal && user?.authLabels?.includes('Generic OAuth'); const isUserSynced = !config.auth.DisableSyncLock && ((user?.isExternal && @@ -126,7 +123,7 @@ export class UserAdminPage extends PureComponent { isAuthProxyUser || isGoogleUser || isGitLabUser || - isOAuthUserWithSkippableSync || + isGenericOAuthUser || isSAMLUser || isOktaUser || isLDAPUser || @@ -135,7 +132,6 @@ export class UserAdminPage extends PureComponent { isJWTUser || isGrafanaComUser )) || - (!config.auth.OAuthSkipOrgRoleUpdateSync && isOAuthUserWithSkippableSync) || (!config.auth.SAMLSkipOrgRoleSync && isSAMLUser) || (!config.auth.LDAPSkipOrgRoleSync && isLDAPUser) || (!config.auth.JWTAuthSkipOrgRoleSync && isJWTUser) || @@ -145,6 +141,7 @@ export class UserAdminPage extends PureComponent { (!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.GenericOAuthSkipOrgRoleSync && isGenericOAuthUser) || (!config.auth.OAuthSkipOrgRoleUpdateSync && !config.auth.GoogleSkipOrgRoleSync && isGoogleUser)); const pageNav: NavModelItem = { diff --git a/public/app/features/admin/UserListAdminPage.tsx b/public/app/features/admin/UserListAdminPage.tsx index a1119b509d9..fde3b25c827 100644 --- a/public/app/features/admin/UserListAdminPage.tsx +++ b/public/app/features/admin/UserListAdminPage.tsx @@ -150,7 +150,7 @@ const UserListAdminPageUnConnected = ({ - + Synced from