Auth: Lock organization roles for users who are managed through an external auth provider (#72204)

remove onlyExternalOrgRoleSync feature flag
This commit is contained in:
Ieva 2023-07-25 10:51:47 +01:00 committed by GitHub
parent b503434bd3
commit e9ba6922c0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 16 additions and 34 deletions

View File

@ -94,7 +94,6 @@ Experimental features might be changed or removed without prior notice.
| `editPanelCSVDragAndDrop` | Enables drag and drop for CSV and Excel files | | `editPanelCSVDragAndDrop` | Enables drag and drop for CSV and Excel files |
| `lokiQuerySplittingConfig` | Give users the option to configure split durations for Loki queries | | `lokiQuerySplittingConfig` | Give users the option to configure split durations for Loki queries |
| `individualCookiePreferences` | Support overriding cookie preferences per user | | `individualCookiePreferences` | Support overriding cookie preferences per user |
| `onlyExternalOrgRoleSync` | Prohibits a user from changing organization roles synced with external auth providers |
| `timeSeriesTable` | Enable time series table transformer & sparkline cell type | | `timeSeriesTable` | Enable time series table transformer & sparkline cell type |
| `prometheusResourceBrowserCache` | Displays browser caching options in Prometheus data source configuration | | `prometheusResourceBrowserCache` | Displays browser caching options in Prometheus data source configuration |
| `influxdbBackendMigration` | Query InfluxDB InfluxQL without the proxy | | `influxdbBackendMigration` | Query InfluxDB InfluxQL without the proxy |

View File

@ -64,7 +64,6 @@ export interface FeatureToggles {
lokiQuerySplitting?: boolean; lokiQuerySplitting?: boolean;
lokiQuerySplittingConfig?: boolean; lokiQuerySplittingConfig?: boolean;
individualCookiePreferences?: boolean; individualCookiePreferences?: boolean;
onlyExternalOrgRoleSync?: boolean;
prometheusMetricEncyclopedia?: boolean; prometheusMetricEncyclopedia?: boolean;
timeSeriesTable?: boolean; timeSeriesTable?: boolean;
prometheusResourceBrowserCache?: boolean; prometheusResourceBrowserCache?: boolean;

View File

@ -11,7 +11,6 @@ import (
"github.com/grafana/grafana/pkg/api/response" "github.com/grafana/grafana/pkg/api/response"
"github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol"
contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" 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/login"
"github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/services/user"
@ -389,7 +388,7 @@ func (hs *HTTPServer) updateOrgUserHelper(c *contextmodel.ReqContext, cmd org.Up
if !c.OrgRole.Includes(cmd.Role) && !c.IsGrafanaAdmin { if !c.OrgRole.Includes(cmd.Role) && !c.IsGrafanaAdmin {
return response.Error(http.StatusForbidden, "Cannot assign a role higher than user's role", nil) return response.Error(http.StatusForbidden, "Cannot assign a role higher than user's role", nil)
} }
if hs.Features.IsEnabled(featuremgmt.FlagOnlyExternalOrgRoleSync) {
// we do not allow to change role for external synced users // we do not allow to change role for external synced users
qAuth := login.GetAuthInfoQuery{UserId: cmd.UserID} qAuth := login.GetAuthInfoQuery{UserId: cmd.UserID}
authInfo, err := hs.authInfoService.GetAuthInfo(c.Req.Context(), &qAuth) authInfo, err := hs.authInfoService.GetAuthInfo(c.Req.Context(), &qAuth)
@ -404,7 +403,7 @@ func (hs *HTTPServer) updateOrgUserHelper(c *contextmodel.ReqContext, cmd org.Up
if authInfo != nil && authInfo.AuthModule != "" && login.IsExternallySynced(hs.Cfg, authInfo.AuthModule) { if authInfo != nil && authInfo.AuthModule != "" && login.IsExternallySynced(hs.Cfg, authInfo.AuthModule) {
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 { if err := hs.orgService.UpdateOrgUser(c.Req.Context(), &cmd); err != nil {
if errors.Is(err, org.ErrLastOrgAdmin) { if errors.Is(err, org.ErrLastOrgAdmin) {
return response.Error(http.StatusBadRequest, "Cannot change role so that there is no organization admin left", nil) return response.Error(http.StatusBadRequest, "Cannot change role so that there is no organization admin left", nil)

View File

@ -275,7 +275,6 @@ func TestOrgUsersAPIEndpoint_updateOrgRole(t *testing.T) {
hs.authInfoService = &logintest.AuthInfoServiceFake{ hs.authInfoService = &logintest.AuthInfoServiceFake{
ExpectedUserAuth: &login.UserAuth{AuthModule: tt.AuthModule}, ExpectedUserAuth: &login.UserAuth{AuthModule: tt.AuthModule},
} }
hs.Features = featuremgmt.WithFeatures(featuremgmt.FlagOnlyExternalOrgRoleSync, true)
hs.userService = &usertest.FakeUserService{ExpectedSignedInUser: userWithPermissions} hs.userService = &usertest.FakeUserService{ExpectedSignedInUser: userWithPermissions}
hs.orgService = &orgtest.FakeOrgService{} hs.orgService = &orgtest.FakeOrgService{}
hs.accesscontrolService = &actest.FakeService{ hs.accesscontrolService = &actest.FakeService{

View File

@ -316,12 +316,6 @@ var (
Stage: FeatureStageExperimental, Stage: FeatureStageExperimental,
Owner: grafanaBackendPlatformSquad, Owner: grafanaBackendPlatformSquad,
}, },
{
Name: "onlyExternalOrgRoleSync",
Description: "Prohibits a user from changing organization roles synced with external auth providers",
Stage: FeatureStageExperimental,
Owner: grafanaAuthnzSquad,
},
{ {
Name: "prometheusMetricEncyclopedia", Name: "prometheusMetricEncyclopedia",
Description: "Adds the metrics explorer component to the Prometheus query builder as an option in metric select", 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 lokiQuerySplitting,GA,@grafana/observability-logs,false,false,false,true
lokiQuerySplittingConfig,experimental,@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 individualCookiePreferences,experimental,@grafana/backend-platform,false,false,false,false
onlyExternalOrgRoleSync,experimental,@grafana/grafana-authnz-team,false,false,false,false
prometheusMetricEncyclopedia,GA,@grafana/observability-metrics,false,false,false,true prometheusMetricEncyclopedia,GA,@grafana/observability-metrics,false,false,false,true
timeSeriesTable,experimental,@grafana/app-o11y,false,false,false,true timeSeriesTable,experimental,@grafana/app-o11y,false,false,false,true
prometheusResourceBrowserCache,experimental,@grafana/observability-metrics,false,false,false,true prometheusResourceBrowserCache,experimental,@grafana/observability-metrics,false,false,false,true

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
onlyExternalOrgRoleSync experimental @grafana/grafana-authnz-team false false false false
48 prometheusMetricEncyclopedia GA @grafana/observability-metrics false false false true
49 timeSeriesTable experimental @grafana/app-o11y false false false true
50 prometheusResourceBrowserCache experimental @grafana/observability-metrics false false false true

View File

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

View File

@ -5,7 +5,6 @@ import { Button, ConfirmModal } from '@grafana/ui';
import { UserRolePicker } from 'app/core/components/RolePicker/UserRolePicker'; import { UserRolePicker } from 'app/core/components/RolePicker/UserRolePicker';
import { fetchRoleOptions } from 'app/core/components/RolePicker/api'; import { fetchRoleOptions } from 'app/core/components/RolePicker/api';
import { TagBadge } from 'app/core/components/TagFilter/TagBadge'; import { TagBadge } from 'app/core/components/TagFilter/TagBadge';
import config from 'app/core/config';
import { contextSrv } from 'app/core/core'; import { contextSrv } from 'app/core/core';
import { AccessControlAction, OrgUser, Role } from 'app/types'; import { AccessControlAction, OrgUser, Role } from 'app/types';
@ -57,10 +56,8 @@ export const UsersTable = ({ users, orgId, onRoleChange, onRemoveUser }: Props)
<tbody> <tbody>
{users.map((user, index) => { {users.map((user, index) => {
let basicRoleDisabled = !contextSrv.hasPermissionInMetadata(AccessControlAction.OrgUsersWrite, user); let basicRoleDisabled = !contextSrv.hasPermissionInMetadata(AccessControlAction.OrgUsersWrite, user);
if (config.featureToggles.onlyExternalOrgRoleSync) {
const isUserSynced = user?.isExternallySynced; const isUserSynced = user?.isExternallySynced;
basicRoleDisabled = isUserSynced || basicRoleDisabled; basicRoleDisabled = isUserSynced || basicRoleDisabled;
}
return ( return (
<tr key={`${user.userId}-${index}`}> <tr key={`${user.userId}-${index}`}>
<td className="width-2 text-center"> <td className="width-2 text-center">