Auth: Remove oauth skip org role sync (grafana#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
Nortech-ai · Mar 22, 2024 · bb792ff · bb792ff
1 parent f2c7023
commit bb792ff
Show file tree

Hide file tree

Showing 11 changed files with 8 additions and 40 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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)