[release-11.3.6] Auth: Fix SAML user IsExternallySynced not being set correctly (#103101)

Auth: Fix SAML user IsExternallySynced not being set correctly (#98487)

(cherry picked from commit 345757c3ae)

Co-authored-by: xavi <114113189+volcanonoodle@users.noreply.github.com>

pkg/api/admin_users.go

@@ -158,8 +158,7 @@ func (hs *HTTPServer) AdminUpdateUserPermissions(c *contextmodel.ReqContext) res
 	}
 
 	if authInfo, err := hs.authInfoService.GetAuthInfo(c.Req.Context(), &login.GetAuthInfoQuery{UserId: userID}); err == nil && authInfo != nil {
-		oauthInfo := hs.SocialService.GetOAuthInfoProvider(authInfo.AuthModule)
+		if hs.isGrafanaAdminExternallySynced(hs.Cfg, authInfo.AuthModule) {
-		if login.IsGrafanaAdminExternallySynced(hs.Cfg, oauthInfo, authInfo.AuthModule) {
 			return response.Error(http.StatusForbidden, "Cannot change Grafana Admin role for externally synced user", nil)
 		}
 	}

pkg/api/admin_users_test.go

@@ -14,10 +14,11 @@ import (
 	"github.com/grafana/grafana/pkg/components/simplejson"
 	"github.com/grafana/grafana/pkg/infra/db"
 	"github.com/grafana/grafana/pkg/infra/db/dbtest"
-	"github.com/grafana/grafana/pkg/login/social"
 	"github.com/grafana/grafana/pkg/login/social/socialtest"
 	"github.com/grafana/grafana/pkg/services/auth"
 	"github.com/grafana/grafana/pkg/services/auth/authtest"
+	"github.com/grafana/grafana/pkg/services/authn"
+	"github.com/grafana/grafana/pkg/services/authn/authntest"
 	contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
 	"github.com/grafana/grafana/pkg/services/login"
 	"github.com/grafana/grafana/pkg/services/login/authinfotest"
@@ -243,39 +244,72 @@ func Test_AdminUpdateUserPermissions(t *testing.T) {
 		authEnabled             bool
 		skipOrgRoleSync         bool
 		expectedRespCode        int
+		enabledAuthnClients     []string
+		authnClientConfig       authn.SSOClientConfig
 	}{
+		// oauth
 		{
-			name:                    "Should allow updating an externally synced OAuth user if Grafana Admin role is not synced",
+			name:                "Should allow updating an externally synced OAuth user if Grafana Admin role is not synced",
-			authModule:              login.GenericOAuthModule,
+			authModule:          login.GenericOAuthModule,
-			authEnabled:             true,
+			enabledAuthnClients: []string{authn.ClientWithPrefix("generic_oauth")},
-			allowAssignGrafanaAdmin: false,
+			authnClientConfig: &authntest.FakeSSOClientConfig{
-			skipOrgRoleSync:         false,
+				ExpectedIsSkipOrgRoleSyncEnabled:         false,
-			expectedRespCode:        http.StatusOK,
+				ExpectedIsAllowAssignGrafanaAdminEnabled: false,
+			},
+			expectedRespCode: http.StatusOK,
 		},
 		{
-			name:                    "Should allow updating an externally synced OAuth user if OAuth provider is not enabled",
+			name:                "Should allow updating an externally synced OAuth user if OAuth provider is not enabled",
-			authModule:              login.GenericOAuthModule,
+			authModule:          login.GenericOAuthModule,
-			authEnabled:             false,
+			expectedRespCode:    http.StatusOK,
-			allowAssignGrafanaAdmin: true,
+			enabledAuthnClients: []string{},
-			skipOrgRoleSync:         false,
+			authnClientConfig: &authntest.FakeSSOClientConfig{
-			expectedRespCode:        http.StatusOK,
+				ExpectedIsSkipOrgRoleSyncEnabled:         false,
+				ExpectedIsAllowAssignGrafanaAdminEnabled: true,
+			},
 		},
 		{
-			name:                    "Should allow updating an externally synced OAuth user if org roles are not being synced",
+			name:                "Should allow updating an externally synced OAuth user if org roles are not being synced",
-			authModule:              login.GenericOAuthModule,
+			authModule:          login.GenericOAuthModule,
-			authEnabled:             true,
+			expectedRespCode:    http.StatusOK,
-			allowAssignGrafanaAdmin: true,
+			enabledAuthnClients: []string{authn.ClientWithPrefix("generic_oauth")},
-			skipOrgRoleSync:         true,
+			authnClientConfig: &authntest.FakeSSOClientConfig{
-			expectedRespCode:        http.StatusOK,
+				ExpectedIsSkipOrgRoleSyncEnabled:         true,
+				ExpectedIsAllowAssignGrafanaAdminEnabled: true,
+			},
 		},
 		{
-			name:                    "Should not allow updating an externally synced OAuth user",
+			name:                "Should not allow updating an externally synced OAuth user",
-			authModule:              login.GenericOAuthModule,
+			authModule:          login.GenericOAuthModule,
-			authEnabled:             true,
+			expectedRespCode:    http.StatusForbidden,
-			allowAssignGrafanaAdmin: true,
+			enabledAuthnClients: []string{authn.ClientWithPrefix("generic_oauth")},
-			skipOrgRoleSync:         false,
+			authnClientConfig: &authntest.FakeSSOClientConfig{
-			expectedRespCode:        http.StatusForbidden,
+				ExpectedIsSkipOrgRoleSyncEnabled:         false,
+				ExpectedIsAllowAssignGrafanaAdminEnabled: true,
+			},
+		},
+		// saml
+		{
+			name:                "Should allow updating an externally synced SAML user if org roles are not being synced",
+			authModule:          login.SAMLAuthModule,
+			expectedRespCode:    http.StatusOK,
+			enabledAuthnClients: []string{authn.ClientSAML},
+			authnClientConfig: &authntest.FakeSSOClientConfig{
+				ExpectedIsSkipOrgRoleSyncEnabled:         true,
+				ExpectedIsAllowAssignGrafanaAdminEnabled: true,
+			},
+		},
+		{
+			name:                "Should not allow updating an externally synced SAML user",
+			authModule:          login.SAMLAuthModule,
+			expectedRespCode:    http.StatusForbidden,
+			enabledAuthnClients: []string{authn.ClientSAML},
+			authnClientConfig: &authntest.FakeSSOClientConfig{
+				ExpectedIsSkipOrgRoleSyncEnabled:         false,
+				ExpectedIsAllowAssignGrafanaAdminEnabled: true,
+			},
 		},
+		// jwt
 		{
 			name:                    "Should allow updating an externally synced JWT user if Grafana Admin role is not synced",
 			authModule:              login.JWTModule,
@@ -316,10 +350,7 @@ func Test_AdminUpdateUserPermissions(t *testing.T) {
 			socialService := &socialtest.FakeSocialService{}
 			cfg := setting.NewCfg()
 
-			switch tc.authModule {
+			if tc.authModule == login.JWTModule {
-			case login.GenericOAuthModule:
-				socialService.ExpectedAuthInfoProvider = &social.OAuthInfo{AllowAssignGrafanaAdmin: tc.allowAssignGrafanaAdmin, Enabled: tc.authEnabled, SkipOrgRoleSync: tc.skipOrgRoleSync}
-			case login.JWTModule:
 				cfg.JWTAuth.Enabled = tc.authEnabled
 				cfg.JWTAuth.SkipOrgRoleSync = tc.skipOrgRoleSync
 				cfg.JWTAuth.AllowAssignGrafanaAdmin = tc.allowAssignGrafanaAdmin
@@ -330,6 +361,10 @@ func Test_AdminUpdateUserPermissions(t *testing.T) {
 				authInfoService: authInfoService,
 				SocialService:   socialService,
 				userService:     usertest.NewUserServiceFake(),
+				authnService: &authntest.FakeService{
+					ExpectedClientConfig: tc.authnClientConfig,
+					EnabledClients:       tc.enabledAuthnClients,
+				},
 			}
 
 			sc := setupScenarioContext(t, "/api/admin/users/1/permissions")

pkg/api/login.go

@@ -24,6 +24,7 @@ import (
 	pref "github.com/grafana/grafana/pkg/services/preference"
 	"github.com/grafana/grafana/pkg/services/secrets"
 	"github.com/grafana/grafana/pkg/services/user"
+	"github.com/grafana/grafana/pkg/setting"
 	"github.com/grafana/grafana/pkg/util"
 )
 
@@ -382,6 +383,22 @@ func (hs *HTTPServer) samlAutoLoginEnabled() bool {
 	return hs.samlEnabled() && config.IsAutoLoginEnabled()
 }
 
+func (hs *HTTPServer) samlSkipOrgRoleSyncEnabled() bool {
+	config, ok := hs.authnService.GetClientConfig(authn.ClientSAML)
+	if !ok {
+		return false
+	}
+	return hs.samlEnabled() && config.IsSkipOrgRoleSyncEnabled()
+}
+
+func (hs *HTTPServer) samlAllowAssignGrafanaAdminEnabled() bool {
+	config, ok := hs.authnService.GetClientConfig(authn.ClientSAML)
+	if !ok {
+		return false
+	}
+	return hs.samlEnabled() && config.IsAllowAssignGrafanaAdminEnabled()
+}
+
 func getLoginExternalError(err error) string {
 	var createTokenErr *auth.CreateTokenErr
 	if errors.As(err, &createTokenErr) {
@@ -411,3 +428,76 @@ func getFirstPublicErrorMessage(err *errutil.Error) string {
 
 	return errPublic.Message
 }
+
+// isExternalySynced is used to tell if the user roles are externally synced
+// true means that the org role sync is handled by Grafana
+// Note: currently the users authinfo is overridden each time the user logs in
+// https://github.com/grafana/grafana/blob/4181acec72f76df7ad02badce13769bae4a1f840/pkg/services/login/authinfoservice/database/database.go#L61
+// this means that if the user has multiple auth providers and one of them is set to sync org roles
+// then isExternallySynced will be true for this one provider and false for the others
+func (hs *HTTPServer) isExternallySynced(cfg *setting.Cfg, authModule string) bool {
+	// provider enabled in config
+	if !hs.isProviderEnabled(cfg, authModule) {
+		return false
+	}
+	// first check SAML, LDAP and JWT
+	switch authModule {
+	case loginservice.SAMLAuthModule:
+		return !hs.samlSkipOrgRoleSyncEnabled()
+	case loginservice.LDAPAuthModule:
+		return !cfg.LDAPSkipOrgRoleSync
+	case loginservice.JWTModule:
+		return !cfg.JWTAuth.SkipOrgRoleSync
+	}
+	switch authModule {
+	case loginservice.GoogleAuthModule, loginservice.OktaAuthModule, loginservice.AzureADAuthModule, loginservice.GitLabAuthModule, loginservice.GithubAuthModule, loginservice.GrafanaComAuthModule, loginservice.GenericOAuthModule:
+		config, ok := hs.authnService.GetClientConfig(oauthModuleToAuthnClient(authModule))
+		if !ok {
+			return false
+		}
+		return !config.IsSkipOrgRoleSyncEnabled()
+	}
+	return true
+}
+
+// isGrafanaAdminExternallySynced returns true if Grafana server admin role is being managed by an external auth provider, and false otherwise.
+// Grafana admin role sync is available for JWT, OAuth providers and LDAP.
+// For JWT and OAuth providers there is an additional config option `allow_assign_grafana_admin` that has to be enabled for Grafana Admin role to be synced.
+func (hs *HTTPServer) isGrafanaAdminExternallySynced(cfg *setting.Cfg, authModule string) bool {
+	if !hs.isExternallySynced(cfg, authModule) {
+		return false
+	}
+
+	switch authModule {
+	case loginservice.JWTModule:
+		return cfg.JWTAuth.AllowAssignGrafanaAdmin
+	case loginservice.SAMLAuthModule:
+		return hs.samlAllowAssignGrafanaAdminEnabled()
+	case loginservice.LDAPAuthModule:
+		return true
+	default:
+		config, ok := hs.authnService.GetClientConfig(oauthModuleToAuthnClient(authModule))
+		if !ok {
+			return false
+		}
+		return config.IsAllowAssignGrafanaAdminEnabled()
+	}
+}
+
+func (hs *HTTPServer) isProviderEnabled(cfg *setting.Cfg, authModule string) bool {
+	switch authModule {
+	case loginservice.SAMLAuthModule:
+		return hs.authnService.IsClientEnabled(authn.ClientSAML)
+	case loginservice.LDAPAuthModule:
+		return cfg.LDAPAuthEnabled
+	case loginservice.JWTModule:
+		return cfg.JWTAuth.Enabled
+	case loginservice.GoogleAuthModule, loginservice.OktaAuthModule, loginservice.AzureADAuthModule, loginservice.GitLabAuthModule, loginservice.GithubAuthModule, loginservice.GrafanaComAuthModule, loginservice.GenericOAuthModule:
+		return hs.authnService.IsClientEnabled(oauthModuleToAuthnClient(authModule))
+	}
+	return false
+}
+
+func oauthModuleToAuthnClient(authModule string) string {
+	return authn.ClientWithPrefix(strings.TrimPrefix(authModule, "oauth_"))
+}

pkg/api/login_test.go

@@ -688,6 +688,172 @@ func TestLogoutSaml(t *testing.T) {
 	require.Equal(t, 302, sc.resp.Code)
 }
 
+func TestIsExternallySynced(t *testing.T) {
+	testcases := []struct {
+		name                string
+		cfg                 *setting.Cfg
+		provider            string
+		enabledAuthnClients []string
+		authnClientConfig   authn.SSOClientConfig
+		expected            bool
+	}{
+		// Same for all of the OAuth providers
+		{
+			name:                "AzureAD external user should return that it is externally synced",
+			cfg:                 &setting.Cfg{},
+			provider:            loginservice.AzureADAuthModule,
+			enabledAuthnClients: []string{authn.ClientWithPrefix("azuread")},
+			authnClientConfig: &authntest.FakeSSOClientConfig{
+				ExpectedIsSkipOrgRoleSyncEnabled: false,
+			},
+			expected: true,
+		},
+		{
+			name:                "AzureAD external user should return that it is not externally synced when org role sync is set",
+			cfg:                 &setting.Cfg{},
+			provider:            loginservice.AzureADAuthModule,
+			enabledAuthnClients: []string{authn.ClientWithPrefix(strings.TrimPrefix(loginservice.AzureADAuthModule, "oauth_"))},
+			authnClientConfig: &authntest.FakeSSOClientConfig{
+				ExpectedIsSkipOrgRoleSyncEnabled: true,
+			},
+			expected: false,
+		},
+		{
+			name:                "AzureAD external user should return that it is not externally synced when the provider is not enabled",
+			cfg:                 &setting.Cfg{},
+			enabledAuthnClients: []string{},
+			authnClientConfig: &authntest.FakeSSOClientConfig{
+				ExpectedIsSkipOrgRoleSyncEnabled: false,
+			},
+			provider: loginservice.AzureADAuthModule,
+			expected: false,
+		},
+		// saml
+		{
+			name:                "SAML synced user should return that it is not externally synced when the provider is disabled",
+			cfg:                 &setting.Cfg{},
+			provider:            loginservice.SAMLAuthModule,
+			enabledAuthnClients: []string{},
+			authnClientConfig: &authntest.FakeSSOClientConfig{
+				ExpectedIsSkipOrgRoleSyncEnabled: false,
+			},
+			expected: false,
+		},
+		{
+			name:                "SAML synced user should return that it is externally synced",
+			cfg:                 &setting.Cfg{},
+			provider:            loginservice.SAMLAuthModule,
+			enabledAuthnClients: []string{authn.ClientSAML},
+			authnClientConfig: &authntest.FakeSSOClientConfig{
+				ExpectedIsSkipOrgRoleSyncEnabled: false,
+			},
+			expected: true,
+		},
+		{
+			name:                "SAML synced user should return that it is not externally synced when org role sync is set",
+			cfg:                 &setting.Cfg{},
+			provider:            loginservice.SAMLAuthModule,
+			enabledAuthnClients: []string{authn.ClientSAML},
+			authnClientConfig: &authntest.FakeSSOClientConfig{
+				ExpectedIsSkipOrgRoleSyncEnabled: true,
+			},
+			expected: false,
+		},
+		// ldap
+		{
+			name:     "LDAP synced user should return that it is externally synced",
+			cfg:      &setting.Cfg{LDAPAuthEnabled: true, LDAPSkipOrgRoleSync: false},
+			provider: loginservice.LDAPAuthModule,
+			expected: true,
+		},
+		{
+			name:     "LDAP synced user should return that it is not externally synced when org role sync is set",
+			cfg:      &setting.Cfg{LDAPAuthEnabled: true, LDAPSkipOrgRoleSync: true},
+			provider: loginservice.LDAPAuthModule,
+			expected: false,
+		},
+		// jwt
+		{
+			name:     "JWT synced user should return that it is externally synced",
+			cfg:      &setting.Cfg{JWTAuth: setting.AuthJWTSettings{Enabled: true, SkipOrgRoleSync: false}},
+			provider: loginservice.JWTModule,
+			expected: true,
+		},
+		{
+			name:     "JWT synced user should return that it is not externally synced when org role sync is set",
+			cfg:      &setting.Cfg{JWTAuth: setting.AuthJWTSettings{Enabled: true, SkipOrgRoleSync: true}},
+			provider: loginservice.JWTModule,
+			expected: false,
+		},
+		// IsProvider test
+		{
+			name:     "If no provider enabled should return false",
+			cfg:      &setting.Cfg{JWTAuth: setting.AuthJWTSettings{Enabled: false, SkipOrgRoleSync: true}},
+			provider: loginservice.JWTModule,
+			expected: false,
+		},
+	}
+
+	for _, tc := range testcases {
+		hs := &HTTPServer{
+			authnService: &authntest.FakeService{
+				ExpectedClientConfig: tc.authnClientConfig,
+				EnabledClients:       tc.enabledAuthnClients,
+			},
+		}
+
+		t.Run(tc.name, func(t *testing.T) {
+			assert.Equal(t, tc.expected, hs.isExternallySynced(tc.cfg, tc.provider))
+		})
+	}
+}
+
+func TestIsProviderEnabled(t *testing.T) {
+	testcases := []struct {
+		name                string
+		provider            string
+		enabledAuthnClients []string
+		expected            bool
+	}{
+		{
+			name:                "Github should return true if enabled",
+			provider:            loginservice.GithubAuthModule,
+			enabledAuthnClients: []string{authn.ClientWithPrefix(strings.TrimPrefix(loginservice.GithubAuthModule, "oauth_"))},
+			expected:            true,
+		},
+		{
+			name:                "Github should return false if not enabled",
+			provider:            loginservice.GithubAuthModule,
+			enabledAuthnClients: []string{},
+			expected:            false,
+		},
+		// saml
+		{
+			name:                "SAML should return true if enabled",
+			provider:            loginservice.SAMLAuthModule,
+			enabledAuthnClients: []string{authn.ClientSAML},
+			expected:            true,
+		},
+		{
+			name:                "SAML should return false if not enabled",
+			provider:            loginservice.SAMLAuthModule,
+			enabledAuthnClients: []string{},
+			expected:            false,
+		},
+	}
+
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			hs := &HTTPServer{
+				authnService: &authntest.FakeService{
+					EnabledClients: tc.enabledAuthnClients,
+				},
+			}
+			assert.Equal(t, tc.expected, hs.isProviderEnabled(setting.NewCfg(), tc.provider))
+		})
+	}
+}
+
 type mockSocialService struct {
 	oAuthInfo       *social.OAuthInfo
 	oAuthInfos      map[string]*social.OAuthInfo

pkg/api/org_users.go

@@ -338,9 +338,8 @@ func (hs *HTTPServer) searchOrgUsersHelper(c *contextmodel.ReqContext, query *or
 	for i := range filteredUsers {
 		filteredUsers[i].AccessControl = accessControlMetadata[fmt.Sprint(filteredUsers[i].UserID)]
 		if module, ok := modules[filteredUsers[i].UserID]; ok {
-			oauthInfo := hs.SocialService.GetOAuthInfoProvider(module)
 			filteredUsers[i].AuthLabels = []string{login.GetAuthProviderLabel(module)}
-			filteredUsers[i].IsExternallySynced = login.IsExternallySynced(hs.Cfg, module, oauthInfo)
+			filteredUsers[i].IsExternallySynced = hs.isExternallySynced(hs.Cfg, module)
 		}
 	}
 
@@ -427,8 +426,7 @@ func (hs *HTTPServer) updateOrgUserHelper(c *contextmodel.ReqContext, cmd org.Up
 		}
 	}
 	if authInfo != nil && authInfo.AuthModule != "" {
-		oauthInfo := hs.SocialService.GetOAuthInfoProvider(authInfo.AuthModule)
+		if hs.isExternallySynced(hs.Cfg, authInfo.AuthModule) {
-		if login.IsExternallySynced(hs.Cfg, authInfo.AuthModule, oauthInfo) {
 			return response.Err(org.ErrCannotChangeRoleForExternallySyncedUser.Errorf("Cannot change role for externally synced user"))
 		}
 	}

pkg/api/org_users_test.go

@@ -9,6 +9,8 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/grafana/grafana/pkg/services/authn"
+	"github.com/grafana/grafana/pkg/services/authn/authntest"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
@@ -258,9 +260,17 @@ func TestOrgUsersAPIEndpoint_updateOrgRole(t *testing.T) {
 			server := SetupAPITestServer(t, func(hs *HTTPServer) {
 				hs.Cfg = setting.NewCfg()
 				hs.Cfg.LDAPAuthEnabled = tt.AuthEnabled
-				if tt.AuthModule == login.LDAPAuthModule {
+				switch tt.AuthModule {
+				case login.LDAPAuthModule:
 					hs.Cfg.LDAPAuthEnabled = tt.AuthEnabled
 					hs.Cfg.LDAPSkipOrgRoleSync = tt.SkipOrgRoleSync
+				case login.GrafanaComAuthModule:
+					hs.authnService = &authntest.FakeService{
+						EnabledClients: []string{authn.ClientWithPrefix("grafana_com")},
+						ExpectedClientConfig: &authntest.FakeSSOClientConfig{
+							ExpectedIsSkipOrgRoleSyncEnabled: tt.SkipOrgRoleSync,
+						},
+					}
 				}
 				// AuthModule empty means basic auth
 

pkg/api/password.go

@@ -39,8 +39,7 @@ func (hs *HTTPServer) SendResetPasswordEmail(c *contextmodel.ReqContext) respons
 
 	getAuthQuery := login.GetAuthInfoQuery{UserId: usr.ID}
 	if authInfo, err := hs.authInfoService.GetAuthInfo(c.Req.Context(), &getAuthQuery); err == nil {
-		oauthInfo := hs.SocialService.GetOAuthInfoProvider(authInfo.AuthModule)
+		if hs.isProviderEnabled(hs.Cfg, authInfo.AuthModule) {
-		if login.IsProviderEnabled(hs.Cfg, authInfo.AuthModule, oauthInfo) {
 			c.Logger.Info("Requested password reset for external user", nil)
 			return response.Error(http.StatusOK, "Email sent", nil)
 		}

pkg/api/user.go

@@ -86,9 +86,8 @@ func (hs *HTTPServer) getUserUserProfile(c *contextmodel.ReqContext, userID int6
 		userProfile.AuthLabels = append(userProfile.AuthLabels, authLabel)
 		userProfile.IsExternal = true
 
-		oauthInfo := hs.SocialService.GetOAuthInfoProvider(authInfo.AuthModule)
+		userProfile.IsExternallySynced = hs.isExternallySynced(hs.Cfg, authInfo.AuthModule)
-		userProfile.IsExternallySynced = login.IsExternallySynced(hs.Cfg, authInfo.AuthModule, oauthInfo)
+		userProfile.IsGrafanaAdminExternallySynced = hs.isGrafanaAdminExternallySynced(hs.Cfg, authInfo.AuthModule)
-		userProfile.IsGrafanaAdminExternallySynced = login.IsGrafanaAdminExternallySynced(hs.Cfg, oauthInfo, authInfo.AuthModule)
 	}
 
 	userProfile.AccessControl = getAccessControlMetadata(c, "global.users:id:", strconv.FormatInt(userID, 10))

pkg/api/user_test.go

@@ -10,6 +10,8 @@ import (
 	"testing"
 	"time"
 
+	"github.com/grafana/grafana/pkg/services/authn"
+	"github.com/grafana/grafana/pkg/services/authn/authntest"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"golang.org/x/oauth2"
@@ -243,6 +245,7 @@ func Test_GetUserByID(t *testing.T) {
 		authEnabled                  bool
 		skipOrgRoleSync              bool
 		expectedIsGrafanaAdminSynced bool
+		expectedIsExternallySynced   bool
 	}{
 		{
 			name:                         "Should return IsGrafanaAdminExternallySynced = false for an externally synced OAuth user if Grafana Admin role is not synced",
@@ -251,6 +254,7 @@ func Test_GetUserByID(t *testing.T) {
 			allowAssignGrafanaAdmin:      false,
 			skipOrgRoleSync:              false,
 			expectedIsGrafanaAdminSynced: false,
+			expectedIsExternallySynced:   true,
 		},
 		{
 			name:                         "Should return IsGrafanaAdminExternallySynced = false for an externally synced OAuth user if OAuth provider is not enabled",
@@ -259,6 +263,7 @@ func Test_GetUserByID(t *testing.T) {
 			allowAssignGrafanaAdmin:      true,
 			skipOrgRoleSync:              false,
 			expectedIsGrafanaAdminSynced: false,
+			expectedIsExternallySynced:   false,
 		},
 		{
 			name:                         "Should return IsGrafanaAdminExternallySynced = false for an externally synced OAuth user if org roles are not being synced",
@@ -267,6 +272,7 @@ func Test_GetUserByID(t *testing.T) {
 			allowAssignGrafanaAdmin:      true,
 			skipOrgRoleSync:              true,
 			expectedIsGrafanaAdminSynced: false,
+			expectedIsExternallySynced:   false,
 		},
 		{
 			name:                         "Should return IsGrafanaAdminExternallySynced = true for an externally synced OAuth user",
@@ -275,6 +281,7 @@ func Test_GetUserByID(t *testing.T) {
 			allowAssignGrafanaAdmin:      true,
 			skipOrgRoleSync:              false,
 			expectedIsGrafanaAdminSynced: true,
+			expectedIsExternallySynced:   true,
 		},
 		{
 			name:                         "Should return IsGrafanaAdminExternallySynced = false for an externally synced JWT user if Grafana Admin role is not synced",
@@ -283,6 +290,7 @@ func Test_GetUserByID(t *testing.T) {
 			allowAssignGrafanaAdmin:      false,
 			skipOrgRoleSync:              false,
 			expectedIsGrafanaAdminSynced: false,
+			expectedIsExternallySynced:   true,
 		},
 		{
 			name:                         "Should return IsGrafanaAdminExternallySynced = false for an externally synced JWT user if JWT provider is not enabled",
@@ -291,6 +299,7 @@ func Test_GetUserByID(t *testing.T) {
 			allowAssignGrafanaAdmin:      true,
 			skipOrgRoleSync:              false,
 			expectedIsGrafanaAdminSynced: false,
+			expectedIsExternallySynced:   false,
 		},
 		{
 			name:                         "Should return IsGrafanaAdminExternallySynced = false for an externally synced JWT user if org roles are not being synced",
@@ -299,6 +308,7 @@ func Test_GetUserByID(t *testing.T) {
 			allowAssignGrafanaAdmin:      true,
 			skipOrgRoleSync:              true,
 			expectedIsGrafanaAdminSynced: false,
+			expectedIsExternallySynced:   false,
 		},
 		{
 			name:                         "Should return IsGrafanaAdminExternallySynced = true for an externally synced JWT user",
@@ -307,6 +317,31 @@ func Test_GetUserByID(t *testing.T) {
 			allowAssignGrafanaAdmin:      true,
 			skipOrgRoleSync:              false,
 			expectedIsGrafanaAdminSynced: true,
+			expectedIsExternallySynced:   true,
+		},
+		{
+			name:                         "Should return IsExternallySynced = true for an externally synced SAML user",
+			authModule:                   login.SAMLAuthModule,
+			authEnabled:                  true,
+			skipOrgRoleSync:              false,
+			expectedIsGrafanaAdminSynced: false,
+			expectedIsExternallySynced:   true,
+		},
+		{
+			name:                         "Should return IsExternallySynced = false for an externally synced SAML user if SAML provider is not enabled",
+			authModule:                   login.SAMLAuthModule,
+			authEnabled:                  false,
+			skipOrgRoleSync:              false,
+			expectedIsGrafanaAdminSynced: false,
+			expectedIsExternallySynced:   false,
+		},
+		{
+			name:                         "Should return IsExternallySynced = false for an externally synced SAML user if  if org roles are not being synced",
+			authModule:                   login.SAMLAuthModule,
+			authEnabled:                  true,
+			skipOrgRoleSync:              true,
+			expectedIsGrafanaAdminSynced: false,
+			expectedIsExternallySynced:   false,
 		},
 	}
 	for _, tc := range testcases {
@@ -315,15 +350,28 @@ func Test_GetUserByID(t *testing.T) {
 			authInfoService := &authinfotest.FakeService{ExpectedUserAuth: userAuth}
 			socialService := &socialtest.FakeSocialService{}
 			userService := &usertest.FakeUserService{ExpectedUserProfileDTO: &user.UserProfileDTO{}}
+			authnService := &authntest.FakeService{
+				ExpectedClientConfig: &authntest.FakeSSOClientConfig{
+					ExpectedIsSkipOrgRoleSyncEnabled:         tc.skipOrgRoleSync,
+					ExpectedIsAllowAssignGrafanaAdminEnabled: tc.allowAssignGrafanaAdmin,
+				},
+				EnabledClients: []string{},
+			}
 			cfg := setting.NewCfg()
 
 			switch tc.authModule {
 			case login.GenericOAuthModule:
-				socialService.ExpectedAuthInfoProvider = &social.OAuthInfo{AllowAssignGrafanaAdmin: tc.allowAssignGrafanaAdmin, Enabled: tc.authEnabled, SkipOrgRoleSync: tc.skipOrgRoleSync}
+				if tc.authEnabled {
+					authnService.EnabledClients = []string{authn.ClientWithPrefix("generic_oauth")}
+				}
 			case login.JWTModule:
 				cfg.JWTAuth.Enabled = tc.authEnabled
 				cfg.JWTAuth.SkipOrgRoleSync = tc.skipOrgRoleSync
 				cfg.JWTAuth.AllowAssignGrafanaAdmin = tc.allowAssignGrafanaAdmin
+			case login.SAMLAuthModule:
+				if tc.authEnabled {
+					authnService.EnabledClients = []string{authn.ClientSAML}
+				}
 			}
 
 			hs := &HTTPServer{
@@ -331,6 +379,7 @@ func Test_GetUserByID(t *testing.T) {
 				authInfoService: authInfoService,
 				SocialService:   socialService,
 				userService:     userService,
+				authnService:    authnService,
 			}
 
 			sc := setupScenarioContext(t, "/api/users/1")
@@ -348,13 +397,13 @@ func Test_GetUserByID(t *testing.T) {
 			require.NoError(t, err)
 
 			assert.Equal(t, tc.expectedIsGrafanaAdminSynced, resp.IsGrafanaAdminExternallySynced)
+			assert.Equal(t, tc.expectedIsExternallySynced, resp.IsExternallySynced)
 		})
 	}
 }
 
 func TestHTTPServer_UpdateUser(t *testing.T) {
 	settings := setting.NewCfg()
-	settings.SAMLAuthEnabled = true
 	sqlStore := db.InitTestDB(t)
 
 	hs := &HTTPServer{
@@ -362,6 +411,9 @@ func TestHTTPServer_UpdateUser(t *testing.T) {
 		SQLStore:      sqlStore,
 		AccessControl: acmock.New(),
 		SocialService: &socialtest.FakeSocialService{ExpectedAuthInfoProvider: &social.OAuthInfo{Enabled: true}},
+		authnService: &authntest.FakeService{
+			EnabledClients: []string{authn.ClientSAML},
+		},
 	}
 
 	updateUserCommand := user.UpdateUserCommand{
@@ -1102,13 +1154,15 @@ func updateUserScenario(t *testing.T, ctx updateUserContext, hs *HTTPServer) {
 func TestHTTPServer_UpdateSignedInUser(t *testing.T) {
 	settings := setting.NewCfg()
 	sqlStore := db.InitTestDB(t)
-	settings.SAMLAuthEnabled = true
 
 	hs := &HTTPServer{
 		Cfg:           settings,
 		SQLStore:      sqlStore,
 		AccessControl: acmock.New(),
 		SocialService: &socialtest.FakeSocialService{},
+		authnService: &authntest.FakeService{
+			EnabledClients: []string{authn.ClientSAML},
+		},
 	}
 
 	updateUserCommand := user.UpdateUserCommand{

pkg/api/utils.go

@@ -49,7 +49,7 @@ func (hs *HTTPServer) isExternalUser(ctx context.Context, userID int64) (bool, e
 		return true, err
 	}
 
-	return login.IsProviderEnabled(hs.Cfg, info.AuthModule, hs.SocialService.GetOAuthInfoProvider(info.AuthModule)), nil
+	return hs.isProviderEnabled(hs.Cfg, info.AuthModule), nil
 }
 
 func ValidateAndNormalizeEmail(email string) (string, error) {

pkg/login/social/social.go

@@ -112,6 +112,14 @@ func (o *OAuthInfo) IsAutoLoginEnabled() bool {
 	return o.AutoLogin
 }
 
+func (o *OAuthInfo) IsSkipOrgRoleSyncEnabled() bool {
+	return o.SkipOrgRoleSync
+}
+
+func (o *OAuthInfo) IsAllowAssignGrafanaAdminEnabled() bool {
+	return o.AllowAssignGrafanaAdmin
+}
+
 type BasicUserInfo struct {
 	Id             string
 	Name           string

pkg/services/authn/authn.go

@@ -93,6 +93,10 @@ type SSOClientConfig interface {
 	IsAutoLoginEnabled() bool
 	// IsSingleLogoutEnabled returns true if the client has single logout enabled
 	IsSingleLogoutEnabled() bool
+	// IsSkipOrgRoleSyncEnabled returns true if the client has enabled skipping org role sync
+	IsSkipOrgRoleSyncEnabled() bool
+	// IsAllowAssignGrafanaAdminEnabled returns true if the client has enabled assigning grafana admin
+	IsAllowAssignGrafanaAdminEnabled() bool
 }
 
 type Service interface {

pkg/services/authn/authntest/fake.go

@@ -11,9 +11,11 @@ import (
 var _ authn.SSOClientConfig = new(FakeSSOClientConfig)
 
 type FakeSSOClientConfig struct {
-	ExpectedName                  string
+	ExpectedName                             string
-	ExpectedIsAutoLoginEnabled    bool
+	ExpectedIsAutoLoginEnabled               bool
-	ExpectedIsSingleLogoutEnabled bool
+	ExpectedIsSingleLogoutEnabled            bool
+	ExpectedIsSkipOrgRoleSyncEnabled         bool
+	ExpectedIsAllowAssignGrafanaAdminEnabled bool
 }
 
 func (f *FakeSSOClientConfig) GetDisplayName() string {
@@ -28,6 +30,14 @@ func (f *FakeSSOClientConfig) IsSingleLogoutEnabled() bool {
 	return f.ExpectedIsSingleLogoutEnabled
 }
 
+func (f *FakeSSOClientConfig) IsSkipOrgRoleSyncEnabled() bool {
+	return f.ExpectedIsSkipOrgRoleSyncEnabled
+}
+
+func (f *FakeSSOClientConfig) IsAllowAssignGrafanaAdminEnabled() bool {
+	return f.ExpectedIsAllowAssignGrafanaAdminEnabled
+}
+
 var (
 	_ authn.Service              = new(FakeService)
 	_ authn.IdentitySynchronizer = new(FakeService)
@@ -41,6 +51,7 @@ type FakeService struct {
 	ExpectedErrs         []error
 	ExpectedIdentities   []*authn.Identity
 	CurrentIndex         int
+	EnabledClients       []string
 }
 
 func (f *FakeService) Authenticate(ctx context.Context, r *authn.Request) (*authn.Identity, error) {
@@ -64,7 +75,17 @@ func (f *FakeService) Authenticate(ctx context.Context, r *authn.Request) (*auth
 }
 
 func (f *FakeService) IsClientEnabled(name string) bool {
-	return true
+	// Consider all clients as enabled if EnabledClients is not explicitly set
+	if f.EnabledClients == nil {
+		return true
+	}
+	// Check if client is in the list of enabled clients
+	for _, s := range f.EnabledClients {
+		if s == name {
+			return true
+		}
+	}
+	return false
 }
 
 func (f *FakeService) GetClientConfig(name string) (authn.SSOClientConfig, bool) {

pkg/services/login/authinfo.go

@@ -2,9 +2,6 @@ package login
 
 import (
 	"context"
-
-	"github.com/grafana/grafana/pkg/login/social"
-	"github.com/grafana/grafana/pkg/setting"
 )
 
 type AuthInfoService interface {
@@ -58,73 +55,6 @@ const (
 	OktaLabel         = "Okta"
 )
 
-// IsExternnalySynced is used to tell if the user roles are externally synced
-// true means that the org role sync is handled by Grafana
-// Note: currently the users authinfo is overridden each time the user logs in
-// https://github.com/grafana/grafana/blob/4181acec72f76df7ad02badce13769bae4a1f840/pkg/services/login/authinfoservice/database/database.go#L61
-// this means that if the user has multiple auth providers and one of them is set to sync org roles
-// then IsExternallySynced will be true for this one provider and false for the others
-func IsExternallySynced(cfg *setting.Cfg, authModule string, oauthInfo *social.OAuthInfo) bool {
-	// provider enabled in config
-	if !IsProviderEnabled(cfg, authModule, oauthInfo) {
-		return false
-	}
-	// first check SAML, LDAP and JWT
-	switch authModule {
-	case SAMLAuthModule:
-		return !cfg.SAMLSkipOrgRoleSync
-	case LDAPAuthModule:
-		return !cfg.LDAPSkipOrgRoleSync
-	case JWTModule:
-		return !cfg.JWTAuth.SkipOrgRoleSync
-	}
-	switch authModule {
-	case GoogleAuthModule, OktaAuthModule, AzureADAuthModule, GitLabAuthModule, GithubAuthModule, GrafanaComAuthModule, GenericOAuthModule:
-		if oauthInfo == nil {
-			return false
-		}
-		return !oauthInfo.SkipOrgRoleSync
-	}
-	return true
-}
-
-// IsGrafanaAdminExternallySynced returns true if Grafana server admin role is being managed by an external auth provider, and false otherwise.
-// Grafana admin role sync is available for JWT, OAuth providers and LDAP.
-// For JWT and OAuth providers there is an additional config option `allow_assign_grafana_admin` that has to be enabled for Grafana Admin role to be synced.
-func IsGrafanaAdminExternallySynced(cfg *setting.Cfg, oauthInfo *social.OAuthInfo, authModule string) bool {
-	if !IsExternallySynced(cfg, authModule, oauthInfo) {
-		return false
-	}
-
-	switch authModule {
-	case JWTModule:
-		return cfg.JWTAuth.AllowAssignGrafanaAdmin
-	case SAMLAuthModule:
-		return cfg.SAMLRoleValuesGrafanaAdmin != ""
-	case LDAPAuthModule:
-		return true
-	default:
-		return oauthInfo != nil && oauthInfo.AllowAssignGrafanaAdmin
-	}
-}
-
-func IsProviderEnabled(cfg *setting.Cfg, authModule string, oauthInfo *social.OAuthInfo) bool {
-	switch authModule {
-	case SAMLAuthModule:
-		return cfg.SAMLAuthEnabled
-	case LDAPAuthModule:
-		return cfg.LDAPAuthEnabled
-	case JWTModule:
-		return cfg.JWTAuth.Enabled
-	case GoogleAuthModule, OktaAuthModule, AzureADAuthModule, GitLabAuthModule, GithubAuthModule, GrafanaComAuthModule, GenericOAuthModule:
-		if oauthInfo == nil {
-			return false
-		}
-		return oauthInfo.Enabled
-	}
-	return false
-}
-
 // used for frontend to display a more user friendly label
 func GetAuthProviderLabel(authModule string) string {
 	switch authModule {

pkg/services/login/authinfo_test.go

@@ -1,131 +0,0 @@
-package login
-
-import (
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-
-	"github.com/grafana/grafana/pkg/login/social"
-	"github.com/grafana/grafana/pkg/setting"
-)
-
-func TestIsExternallySynced(t *testing.T) {
-	testcases := []struct {
-		name      string
-		cfg       *setting.Cfg
-		oauthInfo *social.OAuthInfo
-		provider  string
-		expected  bool
-	}{
-		// Same for all of the OAuth providers
-		{
-			name:      "AzureAD external user should return that it is externally synced",
-			cfg:       &setting.Cfg{},
-			oauthInfo: &social.OAuthInfo{Enabled: true, SkipOrgRoleSync: false},
-			provider:  AzureADAuthModule,
-			expected:  true,
-		},
-		{
-			name:      "AzureAD external user should return that it is not externally synced when org role sync is set",
-			cfg:       &setting.Cfg{},
-			oauthInfo: &social.OAuthInfo{Enabled: true, SkipOrgRoleSync: true},
-			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{},
-			oauthInfo: &social.OAuthInfo{Enabled: false, SkipOrgRoleSync: false},
-			provider:  AzureADAuthModule,
-			expected:  false,
-		},
-		{
-			name:      "AzureAD synced user should return that it is not externally synced when the provider is not enabled and nil",
-			cfg:       &setting.Cfg{},
-			oauthInfo: nil,
-			provider:  AzureADAuthModule,
-			expected:  false,
-		},
-		// saml
-		{
-			name:     "SAML synced user should return that it is externally synced",
-			cfg:      &setting.Cfg{SAMLAuthEnabled: true, SAMLSkipOrgRoleSync: false},
-			provider: SAMLAuthModule,
-			expected: true,
-		},
-		{
-			name:     "SAML synced user should return that it is not externally synced when org role sync is set",
-			cfg:      &setting.Cfg{SAMLAuthEnabled: true, SAMLSkipOrgRoleSync: true},
-			provider: SAMLAuthModule,
-			expected: false,
-		},
-		// ldap
-		{
-			name:     "LDAP synced user should return that it is externally synced",
-			cfg:      &setting.Cfg{LDAPAuthEnabled: true, LDAPSkipOrgRoleSync: false},
-			provider: LDAPAuthModule,
-			expected: true,
-		},
-		{
-			name:     "LDAP synced user should return that it is not externally synced when org role sync is set",
-			cfg:      &setting.Cfg{LDAPAuthEnabled: true, LDAPSkipOrgRoleSync: true},
-			provider: LDAPAuthModule,
-			expected: false,
-		},
-		// jwt
-		{
-			name:     "JWT synced user should return that it is externally synced",
-			cfg:      &setting.Cfg{JWTAuth: setting.AuthJWTSettings{Enabled: true, SkipOrgRoleSync: false}},
-			provider: JWTModule,
-			expected: true,
-		},
-		{
-			name:     "JWT synced user should return that it is not externally synced when org role sync is set",
-			cfg:      &setting.Cfg{JWTAuth: setting.AuthJWTSettings{Enabled: true, SkipOrgRoleSync: true}},
-			provider: JWTModule,
-			expected: false,
-		},
-		// IsProvider test
-		{
-			name:     "If no provider enabled should return false",
-			cfg:      &setting.Cfg{JWTAuth: setting.AuthJWTSettings{Enabled: false, SkipOrgRoleSync: true}},
-			provider: JWTModule,
-			expected: false,
-		},
-	}
-
-	for _, tc := range testcases {
-		t.Run(tc.name, func(t *testing.T) {
-			assert.Equal(t, tc.expected, IsExternallySynced(tc.cfg, tc.provider, tc.oauthInfo))
-		})
-	}
-}
-
-func TestIsProviderEnabled(t *testing.T) {
-	testcases := []struct {
-		name      string
-		oauthInfo *social.OAuthInfo
-		provider  string
-		expected  bool
-	}{
-		// github
-		{
-			name:      "Github should return true if enabled",
-			oauthInfo: &social.OAuthInfo{Enabled: true},
-			provider:  GithubAuthModule,
-			expected:  true,
-		},
-		{
-			name:      "Github should return false if not enabled",
-			oauthInfo: &social.OAuthInfo{Enabled: false},
-			provider:  GithubAuthModule,
-			expected:  false,
-		},
-	}
-
-	for _, tc := range testcases {
-		t.Run(tc.name, func(t *testing.T) {
-			assert.Equal(t, tc.expected, IsProviderEnabled(setting.NewCfg(), tc.provider, tc.oauthInfo))
-		})
-	}
-}