Chore: remove gcomOnlyExternalOrgRoleSync feature toggle (#78001)

remove gcomOnlyExternalOrgRoleSync feature toggle
This commit is contained in:
Ieva 2023-11-13 09:56:02 +00:00 committed by GitHub
parent fe18c63204
commit eca45f6492
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 13 additions and 52 deletions

View File

@ -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 |

View File

@ -64,7 +64,6 @@ export interface FeatureToggles {
lokiQuerySplitting?: boolean;
lokiQuerySplittingConfig?: boolean;
individualCookiePreferences?: boolean;
gcomOnlyExternalOrgRoleSync?: boolean;
prometheusMetricEncyclopedia?: boolean;
influxdbBackendMigration?: boolean;
clientTokenRotation?: boolean;

View File

@ -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 {

View File

@ -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{

View File

@ -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",

View File

@ -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

1 Name Stage Owner requiresDevMode RequiresLicense RequiresRestart FrontendOnly
45 lokiQuerySplitting GA @grafana/observability-logs false false false true
46 lokiQuerySplittingConfig experimental @grafana/observability-logs false false false true
47 individualCookiePreferences experimental @grafana/backend-platform false false false false
gcomOnlyExternalOrgRoleSync GA @grafana/identity-access-team false false false false
48 prometheusMetricEncyclopedia GA @grafana/observability-metrics false false false true
49 influxdbBackendMigration GA @grafana/observability-metrics false false false true
50 clientTokenRotation experimental @grafana/identity-access-team false false false false

View File

@ -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"

View File

@ -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;