From eca45f64924c70a54a8e508ecc32cc9444c921dd Mon Sep 17 00:00:00 2001 From: Ieva Date: Mon, 13 Nov 2023 09:56:02 +0000 Subject: [PATCH] Chore: remove `gcomOnlyExternalOrgRoleSync` feature toggle (#78001) remove gcomOnlyExternalOrgRoleSync feature toggle --- .../feature-toggles/index.md | 1 - .../src/types/featureToggles.gen.ts | 1 - pkg/api/org_users.go | 7 +---- pkg/api/org_users_test.go | 31 ++++++------------- pkg/services/featuremgmt/registry.go | 7 ----- pkg/services/featuremgmt/toggles_gen.csv | 1 - pkg/services/featuremgmt/toggles_gen.go | 4 --- .../features/admin/Users/OrgUsersTable.tsx | 13 ++------ 8 files changed, 13 insertions(+), 52 deletions(-) diff --git a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md index fa1800d42fe..e242030c117 100644 --- a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md +++ b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md @@ -36,7 +36,6 @@ Some features are enabled by default. You can disable these feature by setting t | `disablePrometheusExemplarSampling` | Disable Prometheus exemplar sampling | | | `logsContextDatasourceUi` | Allow datasource to provide custom UI for context view | Yes | | `lokiQuerySplitting` | Split large interval queries into subqueries with smaller time intervals | Yes | -| `gcomOnlyExternalOrgRoleSync` | Prohibits a user from changing organization roles synced with Grafana Cloud auth provider | | | `prometheusMetricEncyclopedia` | Adds the metrics explorer component to the Prometheus query builder as an option in metric select | Yes | | `influxdbBackendMigration` | Query InfluxDB InfluxQL without the proxy | Yes | | `prometheusDataplane` | Changes responses to from Prometheus to be compliant with the dataplane specification. In particular, when this feature toggle is active, the numeric `Field.Name` is set from 'Value' to the value of the `__name__` label. | Yes | diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index fa480288564..f42edfe3955 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -64,7 +64,6 @@ export interface FeatureToggles { lokiQuerySplitting?: boolean; lokiQuerySplittingConfig?: boolean; individualCookiePreferences?: boolean; - gcomOnlyExternalOrgRoleSync?: boolean; prometheusMetricEncyclopedia?: boolean; influxdbBackendMigration?: boolean; clientTokenRotation?: boolean; diff --git a/pkg/api/org_users.go b/pkg/api/org_users.go index 27960b02031..91d70049fe1 100644 --- a/pkg/api/org_users.go +++ b/pkg/api/org_users.go @@ -11,7 +11,6 @@ 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/searchusers/sortopts" @@ -423,11 +422,7 @@ func (hs *HTTPServer) updateOrgUserHelper(c *contextmodel.ReqContext, cmd org.Up } } if authInfo != nil && authInfo.AuthModule != "" && login.IsExternallySynced(hs.Cfg, authInfo.AuthModule) { - // 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")) - } + return response.Err(org.ErrCannotChangeRoleForExternallySyncedUser.Errorf("Cannot change role for externally synced user")) } if err := hs.orgService.UpdateOrgUser(c.Req.Context(), &cmd); err != nil { diff --git a/pkg/api/org_users_test.go b/pkg/api/org_users_test.go index b18e5ecb8f6..18c09b71347 100644 --- a/pkg/api/org_users_test.go +++ b/pkg/api/org_users_test.go @@ -195,12 +195,11 @@ func TestOrgUsersAPIEndpoint_userLoggedIn(t *testing.T) { func TestOrgUsersAPIEndpoint_updateOrgRole(t *testing.T) { type testCase struct { - desc string - SkipOrgRoleSync bool - GcomOnlyExternalOrgRoleSync bool - AuthEnabled bool - AuthModule string - expectedCode int + desc string + SkipOrgRoleSync bool + AuthEnabled bool + AuthModule string + expectedCode int } permissions := []accesscontrol.Permission{ {Action: accesscontrol.ActionOrgUsersRead, Scope: "users:*"}, @@ -231,20 +230,11 @@ func TestOrgUsersAPIEndpoint_updateOrgRole(t *testing.T) { 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 not be able to change basicRole for a user synced through GCom", + SkipOrgRoleSync: false, + AuthEnabled: true, + AuthModule: login.GrafanaComAuthModule, + expectedCode: http.StatusForbidden, }, { desc: "should be able to change basicRole with a basic Auth", @@ -288,7 +278,6 @@ 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{ diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index 6824726731b..1deeb83e596 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -337,13 +337,6 @@ var ( Stage: FeatureStageExperimental, Owner: grafanaBackendPlatformSquad, }, - { - Name: "gcomOnlyExternalOrgRoleSync", - Description: "Prohibits a user from changing organization roles synced with Grafana Cloud auth provider", - Stage: FeatureStageGeneralAvailability, - Owner: identityAccessTeam, - AllowSelfServe: falsePtr, - }, { Name: "prometheusMetricEncyclopedia", Description: "Adds the metrics explorer component to the Prometheus query builder as an option in metric select", diff --git a/pkg/services/featuremgmt/toggles_gen.csv b/pkg/services/featuremgmt/toggles_gen.csv index e444b8606e6..39dca9b64a4 100644 --- a/pkg/services/featuremgmt/toggles_gen.csv +++ b/pkg/services/featuremgmt/toggles_gen.csv @@ -45,7 +45,6 @@ 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/identity-access-team,false,false,false,false prometheusMetricEncyclopedia,GA,@grafana/observability-metrics,false,false,false,true influxdbBackendMigration,GA,@grafana/observability-metrics,false,false,false,true clientTokenRotation,experimental,@grafana/identity-access-team,false,false,false,false diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index f39cb232761..0260ed854d5 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -191,10 +191,6 @@ 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" diff --git a/public/app/features/admin/Users/OrgUsersTable.tsx b/public/app/features/admin/Users/OrgUsersTable.tsx index 174a881c457..e24078bf57f 100644 --- a/public/app/features/admin/Users/OrgUsersTable.tsx +++ b/public/app/features/admin/Users/OrgUsersTable.tsx @@ -20,7 +20,6 @@ import { import { UserRolePicker } from 'app/core/components/RolePicker/UserRolePicker'; import { fetchRoleOptions } from 'app/core/components/RolePicker/api'; import { TagBadge } from 'app/core/components/TagFilter/TagBadge'; -import config from 'app/core/config'; import { contextSrv } from 'app/core/core'; import { AccessControlAction, OrgUser, Role } from 'app/types'; @@ -32,16 +31,8 @@ const disabledRoleMessage = `This user's role is not editable because it is sync Refer to the Grafana authentication docs for details.`; const getBasicRoleDisabled = (user: OrgUser) => { - let basicRoleDisabled = !contextSrv.hasPermissionInMetadata(AccessControlAction.OrgUsersWrite, user); - let authLabel = Array.isArray(user.authLabels) && user.authLabels.length > 0 ? user.authLabels[0] : ''; - // 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 (authLabel !== 'grafana.com' || config.featureToggles.gcomOnlyExternalOrgRoleSync) { - const isUserSynced = user?.isExternallySynced; - basicRoleDisabled = isUserSynced || basicRoleDisabled; - } - - return basicRoleDisabled; + const isUserSynced = user?.isExternallySynced; + return !contextSrv.hasPermissionInMetadata(AccessControlAction.OrgUsersWrite, user) || isUserSynced; }; const selectors = e2eSelectors.pages.UserListPage.UsersListPage;