From 3958fb9e0a836ea277f23bb1ef7d69a871023862 Mon Sep 17 00:00:00 2001 From: Matheus Macabu Date: Thu, 9 Jan 2025 05:03:42 +0100 Subject: [PATCH] CloudMigrations: Introduce RBAC role for migration assistant (#98588) * CloudMigrations: delete unused code * CloudMigrations: add access control and protect API + navtree with action * CloudMigrations: register access control roles * CloudMigrations: gate frontend based with access control * CloudMigrations: fix api tests * CloudMigrations: add docs on new actions and roles * CloudMigrations: dont interpolate vars to make it more greppable * CloudMigrations: run prettier --- .../custom-role-actions-scopes/index.md | 1 + .../index.md | 3 +- pkg/api/api.go | 3 +- pkg/services/cloudmigration/accesscontrol.go | 31 +++ pkg/services/cloudmigration/api/api.go | 11 +- pkg/services/cloudmigration/api/api_test.go | 239 ++++++++++-------- .../cloudmigrationimpl/cloudmigration.go | 7 +- .../cloudmigrationimpl/cloudmigration_test.go | 1 + .../cloudmigration/slicesext/slicesext.go | 11 - .../slicesext/slicesext_test.go | 36 --- pkg/services/navtree/navtreeimpl/admin.go | 4 +- public/app/routes/routes.tsx | 2 +- public/app/types/accessControl.ts | 3 + 13 files changed, 188 insertions(+), 164 deletions(-) create mode 100644 pkg/services/cloudmigration/accesscontrol.go delete mode 100644 pkg/services/cloudmigration/slicesext/slicesext.go delete mode 100644 pkg/services/cloudmigration/slicesext/slicesext_test.go diff --git a/docs/sources/administration/roles-and-permissions/access-control/custom-role-actions-scopes/index.md b/docs/sources/administration/roles-and-permissions/access-control/custom-role-actions-scopes/index.md index ff097506b2e..baab890cbef 100644 --- a/docs/sources/administration/roles-and-permissions/access-control/custom-role-actions-scopes/index.md +++ b/docs/sources/administration/roles-and-permissions/access-control/custom-role-actions-scopes/index.md @@ -114,6 +114,7 @@ The following list contains role-based access control actions. | `licensing:delete` | None | Delete the license token. | | `licensing:read` | None | Read licensing information. | | `licensing:write` | None | Update the license token. | +| `migrationassistant:migrate` | None | Execute on-prem to cloud migrations through the Migration Assistant. | | `org.users:write` | | Update the organization role (`None`, `Viewer`, `Editor`, or `Admin`) of a user. | | `org.users:add` | | Add a user to an organization or invite a new user to an organization. | | `org.users:read` | | Get user profiles within an organization. | diff --git a/docs/sources/administration/roles-and-permissions/access-control/rbac-fixed-basic-role-definitions/index.md b/docs/sources/administration/roles-and-permissions/access-control/rbac-fixed-basic-role-definitions/index.md index 963926589df..f651becaa4e 100644 --- a/docs/sources/administration/roles-and-permissions/access-control/rbac-fixed-basic-role-definitions/index.md +++ b/docs/sources/administration/roles-and-permissions/access-control/rbac-fixed-basic-role-definitions/index.md @@ -56,7 +56,7 @@ The following tables list permissions associated with basic and fixed roles. | Basic role | UID | Associated fixed roles | Description | | ------------- | --------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------- | -| Grafana Admin | `basic_grafana_admin` | `fixed:roles:reader`
`fixed:roles:writer`
`fixed:users:reader`
`fixed:users:writer`
`fixed:org.users:reader`
`fixed:org.users:writer`
`fixed:ldap:reader`
`fixed:ldap:writer`
`fixed:stats:reader`
`fixed:settings:reader`
`fixed:settings:writer`
`fixed:provisioning:writer`
`fixed:organization:reader`
`fixed:organization:maintainer`
`fixed:licensing:reader`
`fixed:licensing:writer`
`fixed:datasources.caching:reader`
`fixed:datasources.caching:writer`
`fixed:dashboards.insights:reader`
`fixed:datasources.insights:reader`
`fixed:plugins:maintainer`
`fixed:authentication.config:writer`
`fixed:library.panels:creator`
`fixed:library.panels:reader`
`fixed:library.panels:general.reader`
`fixed:library.panels:writer`
`fixed:library.panels:general.writer`
`fixed:groupsync:writer` | Default [Grafana server administrator](/docs/grafana//administration/roles-and-permissions/#grafana-server-administrators) assignments. | +| Grafana Admin | `basic_grafana_admin` | `fixed:roles:reader`
`fixed:roles:writer`
`fixed:users:reader`
`fixed:users:writer`
`fixed:org.users:reader`
`fixed:org.users:writer`
`fixed:ldap:reader`
`fixed:ldap:writer`
`fixed:stats:reader`
`fixed:settings:reader`
`fixed:settings:writer`
`fixed:provisioning:writer`
`fixed:organization:reader`
`fixed:organization:maintainer`
`fixed:licensing:reader`
`fixed:licensing:writer`
`fixed:datasources.caching:reader`
`fixed:datasources.caching:writer`
`fixed:dashboards.insights:reader`
`fixed:datasources.insights:reader`
`fixed:plugins:maintainer`
`fixed:authentication.config:writer`
`fixed:library.panels:creator`
`fixed:library.panels:reader`
`fixed:library.panels:general.reader`
`fixed:library.panels:writer`
`fixed:library.panels:general.writer`
`fixed:groupsync:writer`
`fixed:migrationassistant:migrator` | Default [Grafana server administrator](/docs/grafana//administration/roles-and-permissions/#grafana-server-administrators) assignments. | | Admin | `basic_admin` | `fixed:reports:reader`
`fixed:reports:writer`
`fixed:datasources:reader`
`fixed:datasources:writer`
`fixed:organization:writer`
`fixed:datasources.permissions:reader`
`fixed:datasources.permissions:writer`
`fixed:teams:writer`
`fixed:dashboards:reader`
`fixed:dashboards:writer`
`fixed:dashboards.permissions:reader`
`fixed:dashboards.permissions:writer`
`fixed:dashboards.public:writer`
`fixed:folders:reader`
`fixed:folders:writer`
`fixed:folders.permissions:reader`
`fixed:folders.permissions:writer`
`fixed:alerting:writer`
`fixed:apikeys:reader`
`fixed:apikeys:writer`
`fixed:alerting.provisioning.secrets:reader`
`fixed:alerting.provisioning:writer`
`fixed:datasources.caching:reader`
`fixed:datasources.caching:writer`
`fixed:dashboards.insights:reader`
`fixed:datasources.insights:reader`
`fixed:plugins:writer`
`fixed:library.panels:creator`
`fixed:library.panels:reader`
`fixed:library.panels:general.reader`
`fixed:library.panels:writer`
`fixed:library.panels:general.writer`
`fixed:alerting.provisioning.status:writer`
`fixed:groupsync:writer` | Default [Grafana organization administrator](ref:rbac-basic-roles) assignments. | | Editor | `basic_editor` | `fixed:datasources:explorer`
`fixed:dashboards:creator`
`fixed:folders:creator`
`fixed:annotations:writer`
`fixed:teams:creator` if the `editors_can_admin` configuration flag is enabled
`fixed:alerting:writer`
`fixed:dashboards.insights:reader`
`fixed:datasources.insights:reader`
`fixed:library.panels:creator`
`fixed:library.panels:general.reader`
`fixed:library.panels:general.writer`
`fixed:alerting.provisioning.status:writer` | Default [Editor](ref:rbac-basic-roles) assignments. | | Viewer | `basic_viewer` | `fixed:datasources.id:reader`
`fixed:organization:reader`
`fixed:annotations:reader`
`fixed:annotations.dashboard:writer`
`fixed:alerting:reader`
`fixed:plugins.app:reader`
`fixed:dashboards.insights:reader`
`fixed:datasources.insights:reader`
`fixed:library.panels:general.reader`
`fixed:datasources:explorer` if the `viewers_can_edit` configuration flag is enabled | Default [Viewer](ref:rbac-basic-roles) assignments. | @@ -125,6 +125,7 @@ To learn how to use the roles API to determine the role UUIDs, refer to [Manage | `fixed:library.panels:writer` | `fixed_JTljAr21LWLTXCkgfBC4H0lhBC8` | All permissions from `fixed:library.panels:reader` plus
`library.panels:create`
`library.panels:delete`
`library.panels:write` | Create, read, write or delete all library panels and their permissions. | | `fixed:licensing:reader` | `fixed_OADpuXvNEylO2Kelu3GIuBXEAYE` | `licensing:read`
`licensing.reports:read` | Read licensing information and licensing reports. | | `fixed:licensing:writer` | `fixed_gzbz3rJpQMdaKHt-E4q0PVaKMoE` | All permissions from `fixed:licensing:viewer` and
`licensing:write`
`licensing:delete` | Read licensing information and licensing reports, update and delete the license token. | +| `fixed:migrationassistant:migrator` | `fixed_LLk2p7TRuBztOAksTQb1Klc8YTk` | `migrationassistant:migrate` | Execute on-prem to cloud migrations through the Migration Assistant. | | `fixed:org.users:reader` | `fixed_oCqNwlVHLOpw7-jAlwp4HzYqwGY` | `org.users:read` | Read users within a single organization. | | `fixed:org.users:writer` | `fixed_VERj5nayasjgf_Yh0sWqqCkxWlw` | All permissions from `fixed:org.users:reader` and
`org.users:add`
`org.users:remove`
`org.users:write` | Within a single organization, add a user, invite a new user, read information about a user and their role, remove a user from that organization, or change the role of a user. | | `fixed:organization:maintainer` | `fixed_CMm-uuBaPUBf4r8XG3jIvxo55bg` | All permissions from `fixed:organization:reader` and
`orgs:write`
`orgs:create`
`orgs:delete`
`orgs.quotas:write` | Create, read, write, or delete an organization. Read or write its quotas. This role needs to be assigned globally. | diff --git a/pkg/api/api.go b/pkg/api/api.go index 72926817804..a8f51aaf717 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -42,6 +42,7 @@ import ( "github.com/grafana/grafana/pkg/services/accesscontrol/ssoutils" "github.com/grafana/grafana/pkg/services/apikey" "github.com/grafana/grafana/pkg/services/auth" + "github.com/grafana/grafana/pkg/services/cloudmigration" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" "github.com/grafana/grafana/pkg/services/correlations" "github.com/grafana/grafana/pkg/services/dashboards" @@ -117,7 +118,7 @@ func (hs *HTTPServer) registerRoutes() { r.Get("/admin/stats", authorize(ac.EvalPermission(ac.ActionServerStatsRead)), hs.Index) if hs.Features.IsEnabledGlobally(featuremgmt.FlagOnPremToCloudMigrations) { - r.Get("/admin/migrate-to-cloud", reqOrgAdmin, hs.Index) + r.Get("/admin/migrate-to-cloud", authorize(cloudmigration.MigrationAssistantAccess), hs.Index) } // feature toggle admin page diff --git a/pkg/services/cloudmigration/accesscontrol.go b/pkg/services/cloudmigration/accesscontrol.go new file mode 100644 index 00000000000..ffb78ea92a2 --- /dev/null +++ b/pkg/services/cloudmigration/accesscontrol.go @@ -0,0 +1,31 @@ +package cloudmigration + +import "github.com/grafana/grafana/pkg/services/accesscontrol" + +const ( + ActionMigrate = "migrationassistant:migrate" +) + +var ( + // MigrationAssistantAccess is used to protect the "Migrate to Grafana Cloud" page. + MigrationAssistantAccess = accesscontrol.EvalPermission(ActionMigrate) +) + +func RegisterAccessControlRoles(service accesscontrol.Service) error { + migrator := accesscontrol.RoleRegistration{ + Role: accesscontrol.RoleDTO{ + Name: "fixed:migrationassistant:migrator", + DisplayName: "Organization resource migrator", + Description: "Migrate organization resources.", + Group: "Migration Assistant", + Permissions: []accesscontrol.Permission{ + { + Action: ActionMigrate, + }, + }, + }, + Grants: []string{string(accesscontrol.RoleGrafanaAdmin)}, + } + + return service.DeclareFixedRoles(migrator) +} diff --git a/pkg/services/cloudmigration/api/api.go b/pkg/services/cloudmigration/api/api.go index 61457166380..739058398c2 100644 --- a/pkg/services/cloudmigration/api/api.go +++ b/pkg/services/cloudmigration/api/api.go @@ -8,7 +8,7 @@ import ( "github.com/grafana/grafana/pkg/api/routing" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/tracing" - "github.com/grafana/grafana/pkg/middleware" + "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/cloudmigration" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" "github.com/grafana/grafana/pkg/util" @@ -28,6 +28,7 @@ func RegisterApi( rr routing.RouteRegister, cms cloudmigration.Service, tracer tracing.Tracer, + acHandler accesscontrol.AccessControl, ) *CloudMigrationAPI { api := &CloudMigrationAPI{ log: log.New("cloudmigrations.api"), @@ -35,12 +36,14 @@ func RegisterApi( cloudMigrationService: cms, tracer: tracer, } - api.registerEndpoints() + api.registerEndpoints(acHandler) return api } // registerEndpoints Registers Endpoints on Grafana Router -func (cma *CloudMigrationAPI) registerEndpoints() { +func (cma *CloudMigrationAPI) registerEndpoints(acHandler accesscontrol.AccessControl) { + authorize := accesscontrol.Middleware(acHandler) + cma.routeRegister.Group("/api/cloudmigration", func(cloudMigrationRoute routing.RouteRegister) { // destination instance endpoints for token management cloudMigrationRoute.Get("/token", routing.Wrap(cma.GetToken)) @@ -59,7 +62,7 @@ func (cma *CloudMigrationAPI) registerEndpoints() { cloudMigrationRoute.Get("/migration/:uid/snapshots", routing.Wrap(cma.GetSnapshotList)) cloudMigrationRoute.Post("/migration/:uid/snapshot/:snapshotUid/upload", routing.Wrap(cma.UploadSnapshot)) cloudMigrationRoute.Post("/migration/:uid/snapshot/:snapshotUid/cancel", routing.Wrap(cma.CancelSnapshot)) - }, middleware.ReqOrgAdmin) + }, authorize(cloudmigration.MigrationAssistantAccess)) } // swagger:route GET /cloudmigration/token migrations getCloudMigrationToken diff --git a/pkg/services/cloudmigration/api/api_test.go b/pkg/services/cloudmigration/api/api_test.go index 5ede5b8b46d..f9496c27f89 100644 --- a/pkg/services/cloudmigration/api/api_test.go +++ b/pkg/services/cloudmigration/api/api_test.go @@ -8,6 +8,8 @@ import ( "github.com/grafana/grafana/pkg/api/routing" "github.com/grafana/grafana/pkg/infra/tracing" + "github.com/grafana/grafana/pkg/services/accesscontrol/acimpl" + "github.com/grafana/grafana/pkg/services/cloudmigration" "github.com/grafana/grafana/pkg/services/cloudmigration/cloudmigrationimpl/fake" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/user" @@ -20,36 +22,55 @@ type TestCase struct { requestHttpMethod string requestUrl string requestBody string - basicRole org.RoleType + user *user.SignedInUser // if the CloudMigrationService should return an error serviceReturnError bool expectedHttpResult int expectedBody string } +var ( + orgID int64 = 1 + + userWithPermissions = &user.SignedInUser{ + OrgID: orgID, + OrgRole: org.RoleEditor, + Permissions: map[int64]map[string][]string{ + orgID: {cloudmigration.ActionMigrate: nil}, + }, + } + + userWithoutPermissions = &user.SignedInUser{ + OrgID: orgID, + OrgRole: org.RoleAdmin, + IsGrafanaAdmin: true, + Permissions: map[int64]map[string][]string{}, + } +) + func TestCloudMigrationAPI_GetToken(t *testing.T) { tests := []TestCase{ { - desc: "should return 200 if everything is ok", + desc: "returns 200 if the user has the right permissions", requestHttpMethod: http.MethodGet, requestUrl: "/api/cloudmigration/token", - basicRole: org.RoleAdmin, + user: userWithPermissions, expectedHttpResult: http.StatusOK, expectedBody: `{"id":"mock_id","displayName":"mock_name","expiresAt":"","firstUsedAt":"","lastUsedAt":"","createdAt":""}`, }, { - desc: "should return 403 if no used is not admin", + desc: "returns 403 if the user does not have the right permissions", requestHttpMethod: http.MethodGet, requestUrl: "/api/cloudmigration/token", - basicRole: org.RoleEditor, + user: userWithoutPermissions, expectedHttpResult: http.StatusForbidden, expectedBody: "", }, { - desc: "should return 500 if service returns an error", + desc: "returns 500 if service returns an error", requestHttpMethod: http.MethodGet, requestUrl: "/api/cloudmigration/token", - basicRole: org.RoleAdmin, + user: userWithPermissions, serviceReturnError: true, expectedHttpResult: http.StatusInternalServerError, expectedBody: "", @@ -64,26 +85,26 @@ func TestCloudMigrationAPI_GetToken(t *testing.T) { func TestCloudMigrationAPI_CreateToken(t *testing.T) { tests := []TestCase{ { - desc: "should return 200 if everything is ok", + desc: "returns 200 if the user has the right permissions", requestHttpMethod: http.MethodPost, requestUrl: "/api/cloudmigration/token", - basicRole: org.RoleAdmin, + user: userWithPermissions, expectedHttpResult: http.StatusOK, expectedBody: `{"token":"mock_token"}`, }, { - desc: "should return 403 if no used is not admin", + desc: "returns 403 if the user does not have the right permissions", requestHttpMethod: http.MethodPost, requestUrl: "/api/cloudmigration/token", - basicRole: org.RoleEditor, + user: userWithoutPermissions, expectedHttpResult: http.StatusForbidden, expectedBody: "", }, { - desc: "should return 500 if service returns an error", + desc: "returns 500 if service returns an error", requestHttpMethod: http.MethodPost, requestUrl: "/api/cloudmigration/token", - basicRole: org.RoleAdmin, + user: userWithPermissions, serviceReturnError: true, expectedHttpResult: http.StatusInternalServerError, expectedBody: "", @@ -98,32 +119,32 @@ func TestCloudMigrationAPI_CreateToken(t *testing.T) { func TestCloudMigrationAPI_DeleteToken(t *testing.T) { tests := []TestCase{ { - desc: "should return 200 if everything is ok", + desc: "returns 200 if the user has the right permissions", requestHttpMethod: http.MethodDelete, requestUrl: "/api/cloudmigration/token/1234", - basicRole: org.RoleAdmin, + user: userWithPermissions, expectedHttpResult: http.StatusNoContent, }, { - desc: "should return 403 if no used is not admin", + desc: "returns 403 if the user does not have the right permissions", requestHttpMethod: http.MethodDelete, requestUrl: "/api/cloudmigration/token/1234", - basicRole: org.RoleEditor, + user: userWithoutPermissions, expectedHttpResult: http.StatusForbidden, }, { - desc: "should return 500 if service returns an error", + desc: "returns 500 if service returns an error", requestHttpMethod: http.MethodDelete, requestUrl: "/api/cloudmigration/token/1234", - basicRole: org.RoleAdmin, + user: userWithPermissions, serviceReturnError: true, expectedHttpResult: http.StatusInternalServerError, }, { - desc: "should return 400 if uid is invalid", + desc: "returns 400 if uid is invalid", requestHttpMethod: http.MethodDelete, requestUrl: "/api/cloudmigration/token/***", - basicRole: org.RoleAdmin, + user: userWithPermissions, serviceReturnError: true, expectedHttpResult: http.StatusBadRequest, }, @@ -137,32 +158,32 @@ func TestCloudMigrationAPI_DeleteToken(t *testing.T) { func TestCloudMigrationAPI_GetMigration(t *testing.T) { tests := []TestCase{ { - desc: "should return 200 if everything is ok", + desc: "returns 200 if the user has the right permissions", requestHttpMethod: http.MethodGet, requestUrl: "/api/cloudmigration/migration/1234", - basicRole: org.RoleAdmin, + user: userWithPermissions, expectedHttpResult: http.StatusOK, }, { - desc: "should return 403 if no used is not admin", + desc: "returns 403 if the user does not have the right permissions", requestHttpMethod: http.MethodGet, requestUrl: "/api/cloudmigration/migration/1234", - basicRole: org.RoleEditor, + user: userWithoutPermissions, expectedHttpResult: http.StatusForbidden, }, { - desc: "should return 500 if service returns an error", + desc: "returns 500 if service returns an error", requestHttpMethod: http.MethodGet, requestUrl: "/api/cloudmigration/migration/1234", - basicRole: org.RoleAdmin, + user: userWithPermissions, serviceReturnError: true, expectedHttpResult: http.StatusNotFound, }, { - desc: "should return 400 if uid is invalid", + desc: "returns 400 if uid is invalid", requestHttpMethod: http.MethodGet, requestUrl: "/api/cloudmigration/migration/****", - basicRole: org.RoleAdmin, + user: userWithPermissions, serviceReturnError: true, expectedHttpResult: http.StatusBadRequest, }, @@ -176,26 +197,26 @@ func TestCloudMigrationAPI_GetMigration(t *testing.T) { func TestCloudMigrationAPI_GetMigrationList(t *testing.T) { tests := []TestCase{ { - desc: "should return 200 if everything is ok", + desc: "returns 200 if the user has the right permissions", requestHttpMethod: http.MethodGet, requestUrl: "/api/cloudmigration/migration", - basicRole: org.RoleAdmin, + user: userWithPermissions, expectedHttpResult: http.StatusOK, expectedBody: `{"sessions":[{"uid":"mock_uid_1","slug":"mock_stack_1","created":"2024-06-05T17:30:40Z","updated":"2024-06-05T17:30:40Z"},{"uid":"mock_uid_2","slug":"mock_stack_2","created":"2024-06-05T17:30:40Z","updated":"2024-06-05T17:30:40Z"}]}`, }, { - desc: "should return 403 if no used is not admin", + desc: "returns 403 if the user does not have the right permissions", requestHttpMethod: http.MethodGet, requestUrl: "/api/cloudmigration/migration", - basicRole: org.RoleEditor, + user: userWithoutPermissions, expectedHttpResult: http.StatusForbidden, expectedBody: "", }, { - desc: "should return 500 if service returns an error", + desc: "returns 500 if service returns an error", requestHttpMethod: http.MethodGet, requestUrl: "/api/cloudmigration/migration", - basicRole: org.RoleAdmin, + user: userWithPermissions, serviceReturnError: true, expectedHttpResult: http.StatusInternalServerError, expectedBody: "", @@ -210,38 +231,38 @@ func TestCloudMigrationAPI_GetMigrationList(t *testing.T) { func TestCloudMigrationAPI_CreateMigration(t *testing.T) { tests := []TestCase{ { - desc: "should return 200 if everything is ok", + desc: "returns 200 if the user has the right permissions", requestHttpMethod: http.MethodPost, requestUrl: "/api/cloudmigration/migration", requestBody: `{"auth_token":"asdf"}`, - basicRole: org.RoleAdmin, + user: userWithPermissions, expectedHttpResult: http.StatusOK, expectedBody: `{"uid":"fake_uid","slug":"fake_stack","created":"2024-06-05T17:30:40Z","updated":"2024-06-05T17:30:40Z"}`, }, { - desc: "should return 403 if no used is not admin", + desc: "returns 403 if the user does not have the right permissions", requestHttpMethod: http.MethodPost, requestUrl: "/api/cloudmigration/migration", - basicRole: org.RoleEditor, + user: userWithoutPermissions, expectedHttpResult: http.StatusForbidden, expectedBody: "", }, { - desc: "should return 500 if service returns an error", + desc: "returns 500 if service returns an error", requestHttpMethod: http.MethodPost, requestUrl: "/api/cloudmigration/migration", requestBody: `{"authToken":"asdf"}`, - basicRole: org.RoleAdmin, + user: userWithPermissions, serviceReturnError: true, expectedHttpResult: http.StatusInternalServerError, expectedBody: "", }, { - desc: "should return 400 if body is not a valid json", + desc: "returns 400 if body is not a valid json", requestHttpMethod: http.MethodPost, requestUrl: "/api/cloudmigration/migration", requestBody: "asdf", - basicRole: org.RoleAdmin, + user: userWithPermissions, serviceReturnError: false, expectedHttpResult: http.StatusBadRequest, expectedBody: "", @@ -256,35 +277,35 @@ func TestCloudMigrationAPI_CreateMigration(t *testing.T) { func TestCloudMigrationAPI_DeleteMigration(t *testing.T) { tests := []TestCase{ { - desc: "should return 200 if everything is ok", + desc: "returns 200 if the user has the right permissions", requestHttpMethod: http.MethodDelete, requestUrl: "/api/cloudmigration/migration/1234", - basicRole: org.RoleAdmin, + user: userWithPermissions, expectedHttpResult: http.StatusOK, expectedBody: "", }, { - desc: "should return 403 if no used is not admin", + desc: "returns 403 if the user does not have the right permissions", requestHttpMethod: http.MethodDelete, requestUrl: "/api/cloudmigration/migration/1234", - basicRole: org.RoleEditor, + user: userWithoutPermissions, expectedHttpResult: http.StatusForbidden, expectedBody: "", }, { - desc: "should return 500 if service returns an error", + desc: "returns 500 if service returns an error", requestHttpMethod: http.MethodDelete, requestUrl: "/api/cloudmigration/migration/1234", - basicRole: org.RoleAdmin, + user: userWithPermissions, serviceReturnError: true, expectedHttpResult: http.StatusInternalServerError, expectedBody: "", }, { - desc: "should return 400 if uid is invalid", + desc: "returns 400 if uid is invalid", requestHttpMethod: http.MethodDelete, requestUrl: "/api/cloudmigration/migration/****", - basicRole: org.RoleAdmin, + user: userWithPermissions, serviceReturnError: true, expectedHttpResult: http.StatusBadRequest, }, @@ -298,35 +319,35 @@ func TestCloudMigrationAPI_DeleteMigration(t *testing.T) { func TestCloudMigrationAPI_CreateSnapshot(t *testing.T) { tests := []TestCase{ { - desc: "should return 200 if everything is ok", + desc: "returns 200 if the user has the right permissions", requestHttpMethod: http.MethodPost, requestUrl: "/api/cloudmigration/migration/1234/snapshot", - basicRole: org.RoleAdmin, + user: userWithPermissions, expectedHttpResult: http.StatusOK, expectedBody: `{"uid":"fake_uid"}`, }, { - desc: "should return 403 if no used is not admin", + desc: "returns 403 if the user does not have the right permissions", requestHttpMethod: http.MethodPost, requestUrl: "/api/cloudmigration/migration/1234/snapshot", - basicRole: org.RoleEditor, + user: userWithoutPermissions, expectedHttpResult: http.StatusForbidden, expectedBody: "", }, { - desc: "should return 500 if service returns an error", + desc: "returns 500 if service returns an error", requestHttpMethod: http.MethodPost, requestUrl: "/api/cloudmigration/migration/1234/snapshot", - basicRole: org.RoleAdmin, + user: userWithPermissions, serviceReturnError: true, expectedHttpResult: http.StatusInternalServerError, expectedBody: "", }, { - desc: "should return 400 if uid is invalid", + desc: "returns 400 if uid is invalid", requestHttpMethod: http.MethodPost, requestUrl: "/api/cloudmigration/migration/***/snapshot", - basicRole: org.RoleAdmin, + user: userWithPermissions, serviceReturnError: true, expectedHttpResult: http.StatusBadRequest, }, @@ -340,43 +361,43 @@ func TestCloudMigrationAPI_CreateSnapshot(t *testing.T) { func TestCloudMigrationAPI_GetSnapshot(t *testing.T) { tests := []TestCase{ { - desc: "should return 200 if everything is ok", + desc: "returns 200 if the user has the right permissions", requestHttpMethod: http.MethodGet, requestUrl: "/api/cloudmigration/migration/1234/snapshot/1", - basicRole: org.RoleAdmin, + user: userWithPermissions, expectedHttpResult: http.StatusOK, expectedBody: `{"uid":"fake_uid","status":"CREATING","sessionUid":"1234","created":"0001-01-01T00:00:00Z","finished":"0001-01-01T00:00:00Z","results":[{"name":"dashboard name","parentName":"dashboard parent name","type":"DASHBOARD","refId":"123","status":"PENDING"},{"name":"datasource name","parentName":"dashboard parent name","type":"DATASOURCE","refId":"456","status":"OK"}],"stats":{"types":{},"statuses":{},"total":0}}`, }, { - desc: "should return 403 if no used is not admin", + desc: "returns 403 if the user does not have the right permissions", requestHttpMethod: http.MethodGet, requestUrl: "/api/cloudmigration/migration/1234/snapshot/1", - basicRole: org.RoleEditor, + user: userWithoutPermissions, expectedHttpResult: http.StatusForbidden, expectedBody: "", }, { - desc: "should return 500 if service returns an error", + desc: "returns 500 if service returns an error", requestHttpMethod: http.MethodGet, requestUrl: "/api/cloudmigration/migration/1234/snapshot/1", - basicRole: org.RoleAdmin, + user: userWithPermissions, serviceReturnError: true, expectedHttpResult: http.StatusInternalServerError, expectedBody: "", }, { - desc: "should return 400 if uid is invalid", + desc: "returns 400 if uid is invalid", requestHttpMethod: http.MethodGet, requestUrl: "/api/cloudmigration/migration/***/snapshot/1", - basicRole: org.RoleAdmin, + user: userWithPermissions, serviceReturnError: true, expectedHttpResult: http.StatusBadRequest, }, { - desc: "should return 400 if snapshot_uid is invalid", + desc: "returns 400 if snapshot_uid is invalid", requestHttpMethod: http.MethodGet, requestUrl: "/api/cloudmigration/migration/1234/snapshot/***", - basicRole: org.RoleAdmin, + user: userWithPermissions, serviceReturnError: true, expectedHttpResult: http.StatusBadRequest, }, @@ -390,51 +411,51 @@ func TestCloudMigrationAPI_GetSnapshot(t *testing.T) { func TestCloudMigrationAPI_GetSnapshotList(t *testing.T) { tests := []TestCase{ { - desc: "should return 200 if everything is ok", + desc: "returns 200 if the user has the right permissions", requestHttpMethod: http.MethodGet, requestUrl: "/api/cloudmigration/migration/1234/snapshots", - basicRole: org.RoleAdmin, + user: userWithPermissions, expectedHttpResult: http.StatusOK, expectedBody: `{"snapshots":[{"uid":"fake_uid","status":"CREATING","sessionUid":"1234","created":"2024-06-05T17:30:40Z","finished":"0001-01-01T00:00:00Z"},{"uid":"fake_uid","status":"CREATING","sessionUid":"1234","created":"2024-06-05T18:30:40Z","finished":"0001-01-01T00:00:00Z"}]}`, }, { - desc: "with limit query param should return 200 if everything is ok", + desc: "with limit query param returns 200 if everything is ok", requestHttpMethod: http.MethodGet, requestUrl: "/api/cloudmigration/migration/1234/snapshots?limit=1", - basicRole: org.RoleAdmin, + user: userWithPermissions, expectedHttpResult: http.StatusOK, expectedBody: `{"snapshots":[{"uid":"fake_uid","status":"CREATING","sessionUid":"1234","created":"2024-06-05T17:30:40Z","finished":"0001-01-01T00:00:00Z"}]}`, }, { - desc: "with sort query param should return 200 if everything is ok", + desc: "with sort query param returns 200 if everything is ok", requestHttpMethod: http.MethodGet, requestUrl: "/api/cloudmigration/migration/1234/snapshots?sort=latest", - basicRole: org.RoleAdmin, + user: userWithPermissions, expectedHttpResult: http.StatusOK, expectedBody: `{"snapshots":[{"uid":"fake_uid","status":"CREATING","sessionUid":"1234","created":"2024-06-05T18:30:40Z","finished":"0001-01-01T00:00:00Z"},{"uid":"fake_uid","status":"CREATING","sessionUid":"1234","created":"2024-06-05T17:30:40Z","finished":"0001-01-01T00:00:00Z"}]}`, }, { - desc: "should return 403 if no used is not admin", + desc: "returns 403 if the user does not have the right permissions", requestHttpMethod: http.MethodGet, requestUrl: "/api/cloudmigration/migration/1234/snapshots", - basicRole: org.RoleEditor, + user: userWithoutPermissions, expectedHttpResult: http.StatusForbidden, expectedBody: "", }, { - desc: "should return 500 if service returns an error", + desc: "returns 500 if service returns an error", requestHttpMethod: http.MethodGet, requestUrl: "/api/cloudmigration/migration/1234/snapshots", - basicRole: org.RoleAdmin, + user: userWithPermissions, serviceReturnError: true, expectedHttpResult: http.StatusInternalServerError, expectedBody: "", }, { - desc: "should return 400 if uid is invalid", + desc: "returns 400 if uid is invalid", requestHttpMethod: http.MethodGet, requestUrl: "/api/cloudmigration/migration/***/snapshots", - basicRole: org.RoleAdmin, + user: userWithPermissions, serviceReturnError: true, expectedHttpResult: http.StatusBadRequest, }, @@ -448,43 +469,43 @@ func TestCloudMigrationAPI_GetSnapshotList(t *testing.T) { func TestCloudMigrationAPI_UploadSnapshot(t *testing.T) { tests := []TestCase{ { - desc: "should return 200 if everything is ok", + desc: "returns 200 if the user has the right permissions", requestHttpMethod: http.MethodPost, requestUrl: "/api/cloudmigration/migration/1234/snapshot/1/upload", - basicRole: org.RoleAdmin, + user: userWithPermissions, expectedHttpResult: http.StatusOK, expectedBody: "", }, { - desc: "should return 403 if no used is not admin", + desc: "returns 403 if the user does not have the right permissions", requestHttpMethod: http.MethodPost, requestUrl: "/api/cloudmigration/migration/1234/snapshot/1/upload", - basicRole: org.RoleEditor, + user: userWithoutPermissions, expectedHttpResult: http.StatusForbidden, expectedBody: "", }, { - desc: "should return 500 if service returns an error", + desc: "returns 500 if service returns an error", requestHttpMethod: http.MethodPost, requestUrl: "/api/cloudmigration/migration/1234/snapshot/1/upload", - basicRole: org.RoleAdmin, + user: userWithPermissions, serviceReturnError: true, expectedHttpResult: http.StatusInternalServerError, expectedBody: "", }, { - desc: "should return 400 if uid is invalid", + desc: "returns 400 if uid is invalid", requestHttpMethod: http.MethodPost, requestUrl: "/api/cloudmigration/migration/***/snapshot/1/upload", - basicRole: org.RoleAdmin, + user: userWithPermissions, serviceReturnError: true, expectedHttpResult: http.StatusBadRequest, }, { - desc: "should return 400 if snapshot_uid is invalid", + desc: "returns 400 if snapshot_uid is invalid", requestHttpMethod: http.MethodPost, requestUrl: "/api/cloudmigration/migration/1234/snapshot/***/upload", - basicRole: org.RoleAdmin, + user: userWithPermissions, serviceReturnError: true, expectedHttpResult: http.StatusBadRequest, }, @@ -498,43 +519,43 @@ func TestCloudMigrationAPI_UploadSnapshot(t *testing.T) { func TestCloudMigrationAPI_CancelSnapshot(t *testing.T) { tests := []TestCase{ { - desc: "should return 200 if everything is ok", + desc: "returns 200 if the user has the right permissions", requestHttpMethod: http.MethodPost, requestUrl: "/api/cloudmigration/migration/1234/snapshot/1/cancel", - basicRole: org.RoleAdmin, + user: userWithPermissions, expectedHttpResult: http.StatusOK, expectedBody: "", }, { - desc: "should return 403 if no used is not admin", + desc: "returns 403 if the user does not have the right permissions", requestHttpMethod: http.MethodPost, requestUrl: "/api/cloudmigration/migration/1234/snapshot/1/cancel", - basicRole: org.RoleEditor, + user: userWithoutPermissions, expectedHttpResult: http.StatusForbidden, expectedBody: "", }, { - desc: "should return 500 if service returns an error", + desc: "returns 500 if service returns an error", requestHttpMethod: http.MethodPost, requestUrl: "/api/cloudmigration/migration/1234/snapshot/1/cancel", - basicRole: org.RoleAdmin, + user: userWithPermissions, serviceReturnError: true, expectedHttpResult: http.StatusInternalServerError, expectedBody: "", }, { - desc: "should return 400 if uid is invalid", + desc: "returns 400 if uid is invalid", requestHttpMethod: http.MethodPost, requestUrl: "/api/cloudmigration/migration/***/snapshot/1/cancel", - basicRole: org.RoleAdmin, + user: userWithPermissions, serviceReturnError: true, expectedHttpResult: http.StatusBadRequest, }, { - desc: "should return 400 if snapshot_uid is invalid", + desc: "returns 400 if snapshot_uid is invalid", requestHttpMethod: http.MethodPost, requestUrl: "/api/cloudmigration/migration/1234/snapshot/***/cancel", - basicRole: org.RoleAdmin, + user: userWithPermissions, serviceReturnError: true, expectedHttpResult: http.StatusBadRequest, }, @@ -548,7 +569,13 @@ func TestCloudMigrationAPI_CancelSnapshot(t *testing.T) { func runSimpleApiTest(tt TestCase) func(t *testing.T) { return func(t *testing.T) { // setup server - api := RegisterApi(routing.NewRouteRegister(), fake.FakeServiceImpl{ReturnError: tt.serviceReturnError}, tracing.InitializeTracerForTest()) + api := RegisterApi( + routing.NewRouteRegister(), + fake.FakeServiceImpl{ReturnError: tt.serviceReturnError}, + tracing.InitializeTracerForTest(), + acimpl.ProvideAccessControlTest(), + ) + server := webtest.NewServer(t, api.routeRegister) var body io.Reader = nil @@ -559,12 +586,10 @@ func runSimpleApiTest(tt TestCase) func(t *testing.T) { req.Header.Set("Content-Type", "application/json") // create test request - webtest.RequestWithSignedInUser(req, &user.SignedInUser{ - OrgID: 1, - OrgRole: tt.basicRole, - }) + webtest.RequestWithSignedInUser(req, tt.user) res, err := server.Send(req) - defer func() { require.NoError(t, res.Body.Close()) }() + t.Cleanup(func() { require.NoError(t, res.Body.Close()) }) + // validations require.NoError(t, err) require.Equal(t, tt.expectedHttpResult, res.StatusCode) diff --git a/pkg/services/cloudmigration/cloudmigrationimpl/cloudmigration.go b/pkg/services/cloudmigration/cloudmigrationimpl/cloudmigration.go index 6ecbca31257..fe74d5c88bb 100644 --- a/pkg/services/cloudmigration/cloudmigrationimpl/cloudmigration.go +++ b/pkg/services/cloudmigration/cloudmigrationimpl/cloudmigration.go @@ -111,6 +111,7 @@ func ProvideService( pluginStore pluginstore.Store, pluginSettingsService pluginsettings.Service, accessControl accesscontrol.AccessControl, + acService accesscontrol.Service, kvStore kvstore.KVStore, libraryElementsService libraryelements.Service, ngAlert *ngalert.AlertNG, @@ -119,6 +120,10 @@ func ProvideService( return &NoopServiceImpl{}, nil } + if err := cloudmigration.RegisterAccessControlRoles(acService); err != nil { + return nil, fmt.Errorf("registering access control roles: %w", err) + } + s := &Service{ store: &sqlStore{db: db, secretsStore: secretsStore, secretsService: secretsService}, log: log.New(LogPrefix), @@ -137,7 +142,7 @@ func ProvideService( libraryElementsService: libraryElementsService, ngAlert: ngAlert, } - s.api = api.RegisterApi(routeRegister, s, tracer) + s.api = api.RegisterApi(routeRegister, s, tracer, accessControl) httpClientS3, err := httpClientProvider.New() if err != nil { diff --git a/pkg/services/cloudmigration/cloudmigrationimpl/cloudmigration_test.go b/pkg/services/cloudmigration/cloudmigrationimpl/cloudmigration_test.go index 64454b54d17..378d2546832 100644 --- a/pkg/services/cloudmigration/cloudmigrationimpl/cloudmigration_test.go +++ b/pkg/services/cloudmigration/cloudmigrationimpl/cloudmigration_test.go @@ -929,6 +929,7 @@ func setUpServiceTest(t *testing.T, withDashboardMock bool, cfgOverrides ...conf &pluginstore.FakePluginStore{}, &pluginsettings.FakePluginSettings{}, actest.FakeAccessControl{ExpectedEvaluate: true}, + fakeAccessControlService, kvstore.ProvideService(sqlStore), &libraryelementsfake.LibraryElementService{}, ng, diff --git a/pkg/services/cloudmigration/slicesext/slicesext.go b/pkg/services/cloudmigration/slicesext/slicesext.go deleted file mode 100644 index 276858d5418..00000000000 --- a/pkg/services/cloudmigration/slicesext/slicesext.go +++ /dev/null @@ -1,11 +0,0 @@ -package slicesext - -func Map[T any, U any](xs []T, f func(T) U) []U { - out := make([]U, 0, len(xs)) - - for _, x := range xs { - out = append(out, f(x)) - } - - return out -} diff --git a/pkg/services/cloudmigration/slicesext/slicesext_test.go b/pkg/services/cloudmigration/slicesext/slicesext_test.go deleted file mode 100644 index 8e7080be9c1..00000000000 --- a/pkg/services/cloudmigration/slicesext/slicesext_test.go +++ /dev/null @@ -1,36 +0,0 @@ -package slicesext_test - -import ( - "strconv" - "testing" - - "github.com/stretchr/testify/require" - - "github.com/grafana/grafana/pkg/services/cloudmigration/slicesext" -) - -func TestMap(t *testing.T) { - t.Parallel() - - t.Run("mapping a nil slice does nothing and returns an empty slice", func(t *testing.T) { - t.Parallel() - - require.Empty(t, slicesext.Map[any, any](nil, nil)) - }) - - t.Run("mapping a non-nil slice with a nil function panics", func(t *testing.T) { - t.Parallel() - - require.Panics(t, func() { slicesext.Map[int, any]([]int{1, 2, 3}, nil) }) - }) - - t.Run("mapping a non-nil slice with a non-nil function returns the mapped slice", func(t *testing.T) { - t.Parallel() - - original := []int{1, 2, 3} - expected := []string{"1", "2", "3"} - fn := func(i int) string { return strconv.Itoa(i) } - - require.ElementsMatch(t, expected, slicesext.Map(original, fn)) - }) -} diff --git a/pkg/services/navtree/navtreeimpl/admin.go b/pkg/services/navtree/navtreeimpl/admin.go index 66f50147643..9bc0b500130 100644 --- a/pkg/services/navtree/navtreeimpl/admin.go +++ b/pkg/services/navtree/navtreeimpl/admin.go @@ -4,11 +4,11 @@ import ( "github.com/grafana/grafana/pkg/login/social" ac "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol/ssoutils" + "github.com/grafana/grafana/pkg/services/cloudmigration" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" "github.com/grafana/grafana/pkg/services/correlations" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/navtree" - "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginaccesscontrol" "github.com/grafana/grafana/pkg/services/serviceaccounts" "github.com/grafana/grafana/pkg/setting" @@ -61,7 +61,7 @@ func (s *ServiceImpl) getAdminNode(c *contextmodel.ReqContext) (*navtree.NavLink Url: s.cfg.AppSubURL + "/admin/storage", }) } - if s.features.IsEnabled(ctx, featuremgmt.FlagOnPremToCloudMigrations) && c.SignedInUser.HasRole(org.RoleAdmin) { + if hasAccess(cloudmigration.MigrationAssistantAccess) && s.features.IsEnabled(ctx, featuremgmt.FlagOnPremToCloudMigrations) { generalNodeLinks = append(generalNodeLinks, &navtree.NavLink{ Text: "Migrate to Grafana Cloud", Id: "migrate-to-cloud", diff --git a/public/app/routes/routes.tsx b/public/app/routes/routes.tsx index d3417e17b98..8a59242f820 100644 --- a/public/app/routes/routes.tsx +++ b/public/app/routes/routes.tsx @@ -377,7 +377,7 @@ export function getAppRoutes(): RouteDescriptor[] { }, config.featureToggles.onPremToCloudMigrations && { path: '/admin/migrate-to-cloud', - roles: () => ['Admin'], + roles: () => contextSrv.evaluatePermission([AccessControlAction.MigrationAssistantMigrate]), component: SafeDynamicImport( () => import(/* webpackChunkName: "MigrateToCloud" */ 'app/features/migrate-to-cloud/MigrateToCloud') ), diff --git a/public/app/types/accessControl.ts b/public/app/types/accessControl.ts index f7c0e3d5c21..87d71e5b4ed 100644 --- a/public/app/types/accessControl.ts +++ b/public/app/types/accessControl.ts @@ -164,6 +164,9 @@ export enum AccessControlAction { // GroupSync GroupSyncMappingsRead = 'groupsync.mappings:read', GroupSyncMappingsWrite = 'groupsync.mappings:write', + + // Migration Assistant + MigrationAssistantMigrate = 'migrationassistant:migrate', } export interface Role {