Auth: add a feature toggle for locking user roles for users synced through GCom (#72202)

* add a new feature toggle for locking down role sync for users managed by GCom

* protect the frontend and the backend using the new feature toggle

* fix merge

pkg/api/org_users.go

@@ -11,6 +11,7 @@ import (
 	"github.com/grafana/grafana/pkg/api/response"
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
 	contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
+	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"github.com/grafana/grafana/pkg/services/login"
 	"github.com/grafana/grafana/pkg/services/org"
 	"github.com/grafana/grafana/pkg/services/user"
@@ -401,7 +402,11 @@ func (hs *HTTPServer) updateOrgUserHelper(c *contextmodel.ReqContext, cmd org.Up
 		}
 	}
 	if authInfo != nil && authInfo.AuthModule != "" && login.IsExternallySynced(hs.Cfg, authInfo.AuthModule) {
-		return response.Err(org.ErrCannotChangeRoleForExternallySyncedUser.Errorf("Cannot change role for externally synced user"))
+		// A GCom specific feature toggle for role locking has been introduced, as the previous implementation had a bug with locking down external users synced through GCom (https://github.com/grafana/grafana/pull/72044)
+		// Remove this conditional once FlagGcomOnlyExternalOrgRoleSync feature toggle has been removed
+		if authInfo.AuthModule != login.GrafanaComAuthModule || hs.Features.IsEnabled(featuremgmt.FlagGcomOnlyExternalOrgRoleSync) {
+			return response.Err(org.ErrCannotChangeRoleForExternallySyncedUser.Errorf("Cannot change role for externally synced user"))
+		}
 	}
 
 	if err := hs.orgService.UpdateOrgUser(c.Req.Context(), &cmd); err != nil {

pkg/api/org_users_test.go

@@ -202,11 +202,12 @@ func TestOrgUsersAPIEndpoint_userLoggedIn(t *testing.T) {
 
 func TestOrgUsersAPIEndpoint_updateOrgRole(t *testing.T) {
 	type testCase struct {
-		desc            string
-		SkipOrgRoleSync bool
-		AuthEnabled     bool
-		AuthModule      string
-		expectedCode    int
+		desc                        string
+		SkipOrgRoleSync             bool
+		GcomOnlyExternalOrgRoleSync bool
+		AuthEnabled                 bool
+		AuthModule                  string
+		expectedCode                int
 	}
 	permissions := []accesscontrol.Permission{
 		{Action: accesscontrol.ActionOrgUsersRead, Scope: "users:*"},
@@ -236,6 +237,22 @@ func TestOrgUsersAPIEndpoint_updateOrgRole(t *testing.T) {
 			AuthModule:      login.GenericOAuthModule,
 			expectedCode:    http.StatusForbidden,
 		},
+		{
+			desc:                        "should be able to change basicRole for a user synced through GCom if GcomOnlyExternalOrgRoleSync flag is set to false",
+			SkipOrgRoleSync:             false,
+			GcomOnlyExternalOrgRoleSync: false,
+			AuthEnabled:                 true,
+			AuthModule:                  login.GrafanaComAuthModule,
+			expectedCode:                http.StatusOK,
+		},
+		{
+			desc:                        "should not be able to change basicRole for a user synced through GCom if GcomOnlyExternalOrgRoleSync flag is set to true",
+			SkipOrgRoleSync:             false,
+			GcomOnlyExternalOrgRoleSync: true,
+			AuthEnabled:                 true,
+			AuthModule:                  login.GrafanaComAuthModule,
+			expectedCode:                http.StatusForbidden,
+		},
 		{
 			desc:            "should be able to change basicRole with a basic Auth",
 			SkipOrgRoleSync: false,
@@ -266,6 +283,9 @@ func TestOrgUsersAPIEndpoint_updateOrgRole(t *testing.T) {
 				} else if tt.AuthModule == login.GenericOAuthModule {
 					hs.Cfg.GenericOAuthAuthEnabled = tt.AuthEnabled
 					hs.Cfg.GenericOAuthSkipOrgRoleSync = tt.SkipOrgRoleSync
+				} else if tt.AuthModule == login.GrafanaComAuthModule {
+					hs.Cfg.GrafanaNetAuthEnabled = tt.AuthEnabled
+					hs.Cfg.GrafanaComSkipOrgRoleSync = tt.SkipOrgRoleSync
 				} else if tt.AuthModule == "" {
 					// authmodule empty means basic auth
 				} else {
@@ -275,6 +295,7 @@ func TestOrgUsersAPIEndpoint_updateOrgRole(t *testing.T) {
 				hs.authInfoService = &logintest.AuthInfoServiceFake{
 					ExpectedUserAuth: &login.UserAuth{AuthModule: tt.AuthModule},
 				}
+				hs.Features = featuremgmt.WithFeatures(featuremgmt.FlagGcomOnlyExternalOrgRoleSync, tt.GcomOnlyExternalOrgRoleSync)
 				hs.userService = &usertest.FakeUserService{ExpectedSignedInUser: userWithPermissions}
 				hs.orgService = &orgtest.FakeOrgService{}
 				hs.accesscontrolService = &actest.FakeService{

pkg/services/featuremgmt/registry.go

@@ -316,6 +316,12 @@ var (
 			Stage:       FeatureStageExperimental,
 			Owner:       grafanaBackendPlatformSquad,
 		},
+		{
+			Name:        "gcomOnlyExternalOrgRoleSync",
+			Description: "Prohibits a user from changing organization roles synced with Grafana Cloud auth provider",
+			Stage:       FeatureStageGeneralAvailability,
+			Owner:       grafanaAuthnzSquad,
+		},
 		{
 			Name:         "prometheusMetricEncyclopedia",
 			Description:  "Adds the metrics explorer component to the Prometheus query builder as an option in metric select",

pkg/services/featuremgmt/toggles_gen.csv

@@ -45,6 +45,7 @@ logsContextDatasourceUi,GA,@grafana/observability-logs,false,false,false,true
 lokiQuerySplitting,GA,@grafana/observability-logs,false,false,false,true
 lokiQuerySplittingConfig,experimental,@grafana/observability-logs,false,false,false,true
 individualCookiePreferences,experimental,@grafana/backend-platform,false,false,false,false
+gcomOnlyExternalOrgRoleSync,GA,@grafana/grafana-authnz-team,false,false,false,false
 prometheusMetricEncyclopedia,GA,@grafana/observability-metrics,false,false,false,true
 timeSeriesTable,experimental,@grafana/app-o11y,false,false,false,true
 prometheusResourceBrowserCache,experimental,@grafana/observability-metrics,false,false,false,true

pkg/services/featuremgmt/toggles_gen.go

@@ -191,6 +191,10 @@ const (
 	// Support overriding cookie preferences per user
 	FlagIndividualCookiePreferences = "individualCookiePreferences"
 
+	// FlagGcomOnlyExternalOrgRoleSync
+	// Prohibits a user from changing organization roles synced with Grafana Cloud auth provider
+	FlagGcomOnlyExternalOrgRoleSync = "gcomOnlyExternalOrgRoleSync"
+
 	// FlagPrometheusMetricEncyclopedia
 	// Adds the metrics explorer component to the Prometheus query builder as an option in metric select
 	FlagPrometheusMetricEncyclopedia = "prometheusMetricEncyclopedia"