From bb792ff540c87141769308339938b8961b9d6b6b Mon Sep 17 00:00:00 2001 From: Eric Leijonmarck Date: Fri, 22 Mar 2024 15:34:05 +0100 Subject: [PATCH] Auth: Remove oauth skip org role sync (#84972) * remove oauth wide skip org role sync * we are warning from config * set it to false * removed from config ini files and updated docs --- conf/defaults.ini | 4 ---- conf/sample.ini | 4 ---- docs/sources/setup-grafana/configure-grafana/_index.md | 4 ++-- pkg/api/frontendsettings.go | 1 - pkg/login/social/connectors/azuread_oauth_test.go | 6 ++---- pkg/login/social/connectors/gitlab_oauth_test.go | 3 +-- pkg/login/social/connectors/okta_oauth_test.go | 3 +-- pkg/services/authn/clients/oauth.go | 3 --- pkg/services/login/authinfo.go | 6 ------ pkg/services/login/authinfo_test.go | 8 -------- pkg/setting/setting.go | 6 ++---- 11 files changed, 8 insertions(+), 40 deletions(-) diff --git a/conf/defaults.ini b/conf/defaults.ini index 304928eed26..f954832df20 100644 --- a/conf/defaults.ini +++ b/conf/defaults.ini @@ -561,10 +561,6 @@ oauth_auto_login = false # OAuth state max age cookie duration in seconds. Defaults to 600 seconds. oauth_state_cookie_max_age = 600 -# Skip forced assignment of OrgID 1 or 'auto_assign_org_id' for social logins -# Deprecated, use skip_org_role_sync option for specific provider instead. -oauth_skip_org_role_update_sync = false - # limit of api_key seconds to live before expiration api_key_max_seconds_to_live = -1 diff --git a/conf/sample.ini b/conf/sample.ini index 36ca32eea82..d8cdd7d72e1 100644 --- a/conf/sample.ini +++ b/conf/sample.ini @@ -538,10 +538,6 @@ # OAuth state max age cookie duration in seconds. Defaults to 600 seconds. ;oauth_state_cookie_max_age = 600 -# Skip forced assignment of OrgID 1 or 'auto_assign_org_id' for social logins -# Deprecated, use skip_org_role_sync option for specific provider instead. -;oauth_skip_org_role_update_sync = false - # limit of api_key seconds to live before expiration ;api_key_max_seconds_to_live = -1 diff --git a/docs/sources/setup-grafana/configure-grafana/_index.md b/docs/sources/setup-grafana/configure-grafana/_index.md index 20e97399c06..00082953439 100644 --- a/docs/sources/setup-grafana/configure-grafana/_index.md +++ b/docs/sources/setup-grafana/configure-grafana/_index.md @@ -938,10 +938,10 @@ Administrators can increase this if they experience OAuth login state mismatch e ### oauth_skip_org_role_update_sync {{% admonition type="note" %}} -This option is deprecated in favor of OAuth provider specific `skip_org_role_sync` settings. The following sections explain settings for each provider. +This option is removed from G11 in favor of OAuth provider specific `skip_org_role_sync` settings. The following sections explain settings for each provider. {{% /admonition %}} -If you want to change the `oauth_skip_org_role_update_sync` setting to `false`, then for each provider you have set up, use the `skip_org_role_sync` setting to specify whether you want to skip the synchronization. +If you want to change the `oauth_skip_org_role_update_sync` setting from `true` to `false`, then each provider you have set up, use the `skip_org_role_sync` setting to specify whether you want to skip the synchronization. {{% admonition type="warning" %}} Currently if no organization role mapping is found for a user, Grafana doesn't update the user's organization role. diff --git a/pkg/api/frontendsettings.go b/pkg/api/frontendsettings.go index 209972efe39..45c8f45e9b1 100644 --- a/pkg/api/frontendsettings.go +++ b/pkg/api/frontendsettings.go @@ -327,7 +327,6 @@ func (hs *HTTPServer) getFrontendSettings(c *contextmodel.ReqContext) (*dtos.Fro oauthProviders := hs.SocialService.GetOAuthInfoProviders() frontendSettings.Auth = dtos.FrontendSettingsAuthDTO{ AuthProxyEnableLoginToken: hs.Cfg.AuthProxy.EnableLoginToken, - OAuthSkipOrgRoleUpdateSync: hs.Cfg.OAuthSkipOrgRoleUpdateSync, SAMLSkipOrgRoleSync: hs.Cfg.SAMLSkipOrgRoleSync, LDAPSkipOrgRoleSync: hs.Cfg.LDAPSkipOrgRoleSync, JWTAuthSkipOrgRoleSync: hs.Cfg.JWTAuth.SkipOrgRoleSync, diff --git a/pkg/login/social/connectors/azuread_oauth_test.go b/pkg/login/social/connectors/azuread_oauth_test.go index 670426086cd..c1738b3baf8 100644 --- a/pkg/login/social/connectors/azuread_oauth_test.go +++ b/pkg/login/social/connectors/azuread_oauth_test.go @@ -800,8 +800,7 @@ func TestSocialAzureAD_SkipOrgRole(t *testing.T) { SkipOrgRoleSync: false, }, cfg: &setting.Cfg{ - AutoAssignOrgRole: "", - OAuthSkipOrgRoleUpdateSync: false, + AutoAssignOrgRole: "", }, }, claims: &azureClaims{ @@ -831,8 +830,7 @@ func TestSocialAzureAD_SkipOrgRole(t *testing.T) { SkipOrgRoleSync: false, }, cfg: &setting.Cfg{ - AutoAssignOrgRole: "", - OAuthSkipOrgRoleUpdateSync: false, + AutoAssignOrgRole: "", }, }, claims: &azureClaims{ diff --git a/pkg/login/social/connectors/gitlab_oauth_test.go b/pkg/login/social/connectors/gitlab_oauth_test.go index a09944d29b3..25640066603 100644 --- a/pkg/login/social/connectors/gitlab_oauth_test.go +++ b/pkg/login/social/connectors/gitlab_oauth_test.go @@ -362,8 +362,7 @@ func TestSocialGitlab_extractFromToken(t *testing.T) { TokenUrl: tc.config.Endpoint.TokenURL, }, &setting.Cfg{ - AutoAssignOrgRole: "", - OAuthSkipOrgRoleUpdateSync: false, + AutoAssignOrgRole: "", }, &ssosettingstests.MockService{}, featuremgmt.WithFeatures()) diff --git a/pkg/login/social/connectors/okta_oauth_test.go b/pkg/login/social/connectors/okta_oauth_test.go index d3c44308342..c60e7b24382 100644 --- a/pkg/login/social/connectors/okta_oauth_test.go +++ b/pkg/login/social/connectors/okta_oauth_test.go @@ -111,8 +111,7 @@ func TestSocialOkta_UserInfo(t *testing.T) { SkipOrgRoleSync: tt.settingSkipOrgRoleSync, }, &setting.Cfg{ - AutoAssignOrgRole: tt.autoAssignOrgRole, - OAuthSkipOrgRoleUpdateSync: false, + AutoAssignOrgRole: tt.autoAssignOrgRole, }, &ssosettingstests.MockService{}, featuremgmt.WithFeatures()) diff --git a/pkg/services/authn/clients/oauth.go b/pkg/services/authn/clients/oauth.go index 7fd04b1fddf..66d65051c4a 100644 --- a/pkg/services/authn/clients/oauth.go +++ b/pkg/services/authn/clients/oauth.go @@ -162,9 +162,6 @@ func (c *OAuth) Authenticate(ctx context.Context, r *authn.Request) (*authn.Iden } orgRoles, isGrafanaAdmin, _ := getRoles(c.cfg, func() (org.RoleType, *bool, error) { - if c.cfg.OAuthSkipOrgRoleUpdateSync { - return "", nil, nil - } return userInfo.Role, userInfo.IsGrafanaAdmin, nil }) diff --git a/pkg/services/login/authinfo.go b/pkg/services/login/authinfo.go index 4f3d95bd931..103372c942d 100644 --- a/pkg/services/login/authinfo.go +++ b/pkg/services/login/authinfo.go @@ -78,12 +78,6 @@ func IsExternallySynced(cfg *setting.Cfg, authModule string, oauthInfo *social.O case JWTModule: return !cfg.JWTAuth.SkipOrgRoleSync } - // then check the rest of the oauth providers - // FIXME: remove this once we remove the setting - // is a deprecated setting that is used to skip org role sync for all external oauth providers - if cfg.OAuthSkipOrgRoleUpdateSync { - return false - } switch authModule { case GoogleAuthModule, OktaAuthModule, AzureADAuthModule, GitLabAuthModule, GithubAuthModule, GrafanaComAuthModule, GenericOAuthModule: if oauthInfo == nil { diff --git a/pkg/services/login/authinfo_test.go b/pkg/services/login/authinfo_test.go index 39746ef8745..53996b6785c 100644 --- a/pkg/services/login/authinfo_test.go +++ b/pkg/services/login/authinfo_test.go @@ -32,14 +32,6 @@ func TestIsExternallySynced(t *testing.T) { provider: AzureADAuthModule, expected: false, }, - // FIXME: remove this test as soon as we remove the deprecated setting for skipping org role sync for all external oauth providers - { - name: "AzureAD external user should return that it is not externally synced when oauth org role sync is set", - cfg: &setting.Cfg{OAuthSkipOrgRoleUpdateSync: true}, - oauthInfo: &social.OAuthInfo{Enabled: true, SkipOrgRoleSync: false}, - provider: AzureADAuthModule, - expected: false, - }, { name: "AzureAD external user should return that it is not externally synced when the provider is not enabled", cfg: &setting.Cfg{}, diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index 440ecd182fc..3d92c0560da 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -1571,11 +1571,9 @@ func readAuthSettings(iniFile *ini.File, cfg *Cfg) (err error) { cfg.OAuthCookieMaxAge = auth.Key("oauth_state_cookie_max_age").MustInt(600) cfg.SignoutRedirectUrl = valueAsString(auth, "signout_redirect_url", "") + // Deprecated - cfg.OAuthSkipOrgRoleUpdateSync = auth.Key("oauth_skip_org_role_update_sync").MustBool(false) - if cfg.OAuthSkipOrgRoleUpdateSync { - cfg.Logger.Warn("[Deprecated] The oauth_skip_org_role_update_sync configuration setting is deprecated. Please use skip_org_role_sync inside the auth provider section instead.") - } + cfg.OAuthSkipOrgRoleUpdateSync = false cfg.DisableLogin = auth.Key("disable_login").MustBool(false)