Auth: Align loading the legacy auth.grafananet section to the current behaviour in OAuthStrategy (#83479)

* Align oauth_strategy to the current behaviour

* lint

* Address feedback

pkg/login/social/socialimpl/service_test.go

@@ -91,6 +91,101 @@ func TestSocialService_ProvideService(t *testing.T) {
 	}
 }
 
+func TestSocialService_ProvideService_GrafanaComGrafanaNet(t *testing.T) {
+	testCases := []struct {
+		name                        string
+		rawIniContent               string
+		expectedGrafanaComOAuthInfo *social.OAuthInfo
+	}{
+		{
+			name: "should setup the connector using auth.grafana_com section if it is enabled",
+			rawIniContent: `
+			[auth.grafana_com]
+			enabled = true
+			client_id = grafanaComClientId
+			
+			[auth.grafananet]
+			enabled = false
+			client_id = grafanaNetClientId`,
+			expectedGrafanaComOAuthInfo: &social.OAuthInfo{
+				AuthStyle: "inheader",
+				AuthUrl:   "/oauth2/authorize",
+				TokenUrl:  "/api/oauth2/token",
+				Enabled:   true,
+				ClientId:  "grafanaComClientId",
+			},
+		},
+		{
+			name: "should setup the connector using auth.grafananet section if it is enabled",
+			rawIniContent: `
+			[auth.grafana_com]
+			enabled = false
+			client_id = grafanaComClientId
+			
+			[auth.grafananet]
+			enabled = true
+			client_id = grafanaNetClientId`,
+			expectedGrafanaComOAuthInfo: &social.OAuthInfo{
+				AuthStyle: "inheader",
+				AuthUrl:   "/oauth2/authorize",
+				TokenUrl:  "/api/oauth2/token",
+				Enabled:   true,
+				ClientId:  "grafanaNetClientId",
+			},
+		},
+		{
+			name: "should setup the connector using auth.grafana_com section if both are enabled",
+			rawIniContent: `
+			[auth.grafana_com]
+			enabled = true
+			client_id = grafanaComClientId
+			
+			[auth.grafananet]
+			enabled = true
+			client_id = grafanaNetClientId`,
+			expectedGrafanaComOAuthInfo: &social.OAuthInfo{
+				AuthStyle: "inheader",
+				AuthUrl:   "/oauth2/authorize",
+				TokenUrl:  "/api/oauth2/token",
+				Enabled:   true,
+				ClientId:  "grafanaComClientId",
+			},
+		},
+		{
+			name: "should not setup the connector when both are disabled",
+			rawIniContent: `
+			[auth.grafana_com]
+			enabled = false
+			client_id = grafanaComClientId
+			
+			[auth.grafananet]
+			enabled = false
+			client_id = grafanaNetClientId`,
+			expectedGrafanaComOAuthInfo: nil,
+		},
+	}
+
+	cfg := setting.NewCfg()
+	secrets := secretsfake.NewMockService(t)
+	accessControl := acimpl.ProvideAccessControl(cfg)
+	sqlStore := db.InitTestDB(t)
+
+	ssoSettingsSvc := ssosettingsimpl.ProvideService(cfg, sqlStore, accessControl, routing.NewRouteRegister(), featuremgmt.WithFeatures(), secrets, &usagestats.UsageStatsMock{}, nil)
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			iniFile, err := ini.Load([]byte(tc.rawIniContent))
+			require.NoError(t, err)
+
+			cfg := setting.NewCfg()
+			cfg.Raw = iniFile
+
+			socialService := ProvideService(cfg, featuremgmt.WithFeatures(), &usagestats.UsageStatsMock{}, supportbundlestest.NewFakeBundleService(), remotecache.NewFakeStore(t), ssoSettingsSvc)
+			require.EqualValues(t, tc.expectedGrafanaComOAuthInfo, socialService.GetOAuthInfoProvider("grafana_com"))
+		})
+	}
+}
+
 func TestMapping_IniSectionOAuthInfo(t *testing.T) {
 	iniContent := `
 [test]

pkg/services/ssosettings/strategies/oauth_strategy.go

@@ -51,13 +51,20 @@ func (s *OAuthStrategy) loadAllSettings() {
 	allProviders := append(ssosettings.AllOAuthProviders, social.GrafanaNetProviderName)
 	for _, provider := range allProviders {
 		settings := s.loadSettingsForProvider(provider)
-		if provider == social.GrafanaNetProviderName {
+		// This is required to support the legacy settings for the provider (auth.grafananet section)
+		// It will use the settings (and overwrite the current grafana_com settings) from auth.grafananet if
+		// the auth.grafananet section is enabled and the auth.grafana_com section is disabled.
+		if provider == social.GrafanaNetProviderName && s.shouldUseGrafanaNetSettings() && settings["enabled"] == true {
 			provider = social.GrafanaComProviderName
 		}
 		s.settingsByProvider[provider] = settings
 	}
 }
 
+func (s *OAuthStrategy) shouldUseGrafanaNetSettings() bool {
+	return s.settingsByProvider[social.GrafanaComProviderName]["enabled"] == false
+}
+
 func (s *OAuthStrategy) loadSettingsForProvider(provider string) map[string]any {
 	section := s.cfg.Raw.Section("auth." + provider)
 

pkg/services/ssosettings/strategies/oauth_strategy_test.go

@@ -127,6 +127,7 @@ func TestGetProviderConfig_ExtraFields(t *testing.T) {
 	allowed_organizations = org1, org2
 
 	[auth.grafana_com]
+	enabled = true
 	allowed_organizations = org1, org2
 	`
 
@@ -166,10 +167,100 @@ func TestGetProviderConfig_ExtraFields(t *testing.T) {
 	})
 
 	t.Run("grafana_com", func(t *testing.T) {
-		t.Skip("Skipping to revert an issue.")
 		result, err := strategy.GetProviderConfig(context.Background(), "grafana_com")
 		require.NoError(t, err)
 
 		require.Equal(t, "org1, org2", result["allowed_organizations"])
 	})
 }
+
+// TestGetProviderConfig_GrafanaComGrafanaNet tests that the connector is setup using the correct section and it supports
+// the legacy settings for the provider (auth.grafananet section). The test cases are based on the current behavior of the
+// SocialService's ProvideService method (TestSocialService_ProvideService_GrafanaComGrafanaNet).
+func TestGetProviderConfig_GrafanaComGrafanaNet(t *testing.T) {
+	testCases := []struct {
+		name                       string
+		rawIniContent              string
+		expectedGrafanaComSettings map[string]any
+	}{
+		{
+			name: "should setup the connector using auth.grafana_com section if it is enabled",
+			rawIniContent: `
+			[auth.grafana_com]
+			enabled = true
+			client_id = grafanaComClientId
+			
+			[auth.grafananet]
+			enabled = false
+			client_id = grafanaNetClientId`,
+			expectedGrafanaComSettings: map[string]any{
+				"enabled":   true,
+				"client_id": "grafanaComClientId",
+			},
+		},
+		{
+			name: "should setup the connector using auth.grafananet section if it is enabled",
+			rawIniContent: `
+			[auth.grafana_com]
+			enabled = false
+			client_id = grafanaComClientId
+			
+			[auth.grafananet]
+			enabled = true
+			client_id = grafanaNetClientId`,
+			expectedGrafanaComSettings: map[string]any{
+				"enabled":   true,
+				"client_id": "grafanaNetClientId",
+			},
+		},
+		{
+			name: "should setup the connector using auth.grafana_com section if both are enabled",
+			rawIniContent: `
+			[auth.grafana_com]
+			enabled = true
+			client_id = grafanaComClientId
+			
+			[auth.grafananet]
+			enabled = true
+			client_id = grafanaNetClientId`,
+			expectedGrafanaComSettings: map[string]any{
+				"enabled":   true,
+				"client_id": "grafanaComClientId",
+			},
+		},
+		{
+			name: "should not setup the connector when both are disabled",
+			rawIniContent: `
+			[auth.grafana_com]
+			enabled = false
+			client_id = grafanaComClientId
+			
+			[auth.grafananet]
+			enabled = false
+			client_id = grafanaNetClientId`,
+			expectedGrafanaComSettings: map[string]any{
+				"enabled":   false,
+				"client_id": "grafanaComClientId",
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			iniFile, err := ini.Load([]byte(tc.rawIniContent))
+			require.NoError(t, err)
+
+			cfg := setting.NewCfg()
+			cfg.Raw = iniFile
+
+			strategy := NewOAuthStrategy(cfg)
+
+			actualConfig, err := strategy.GetProviderConfig(context.Background(), "grafana_com")
+			require.NoError(t, err)
+
+			for key, value := range tc.expectedGrafanaComSettings {
+				require.Equal(t, value, actualConfig[key], "Difference in key: %s. Expected: %v, got: %v", key, value, actualConfig[key])
+			}
+		})
+	}
+}