From d2b9da9dde73ca75de8abd17c36d1363eeb778ef Mon Sep 17 00:00:00 2001 From: Karl Persson Date: Thu, 10 Feb 2022 17:47:48 +0100 Subject: [PATCH] Access control: Support uids for resource permissions (#45226) * add middleware to solve uid -> id for requests --- .../accesscontrol/resourcepermissions/api.go | 9 +- .../resourcepermissions/api_test.go | 97 ++++++++++++++++--- .../resourcepermissions/middleware.go | 28 ++++++ .../resourcepermissions/options.go | 2 + .../components/AccessControl/Permissions.tsx | 19 ++-- .../features/datasources/state/navModel.ts | 4 +- 6 files changed, 131 insertions(+), 28 deletions(-) create mode 100644 pkg/services/accesscontrol/resourcepermissions/middleware.go diff --git a/pkg/services/accesscontrol/resourcepermissions/api.go b/pkg/services/accesscontrol/resourcepermissions/api.go index 987a57948fd..0f90577282d 100644 --- a/pkg/services/accesscontrol/resourcepermissions/api.go +++ b/pkg/services/accesscontrol/resourcepermissions/api.go @@ -32,15 +32,16 @@ func newApi(ac accesscontrol.AccessControl, router routing.RouteRegister, manage func (a *api) registerEndpoints() { auth := middleware.Middleware(a.ac) + uidSolver := solveUID(a.service.options.UidSolver) disable := middleware.Disable(a.ac.IsDisabled()) a.router.Group(fmt.Sprintf("/api/access-control/%s", a.service.options.Resource), func(r routing.RouteRegister) { idScope := accesscontrol.Scope(a.service.options.Resource, "id", accesscontrol.Parameter(":resourceID")) actionWrite, actionRead := fmt.Sprintf("%s.permissions:write", a.service.options.Resource), fmt.Sprintf("%s.permissions:read", a.service.options.Resource) r.Get("/description", auth(disable, accesscontrol.EvalPermission(actionRead)), routing.Wrap(a.getDescription)) - r.Get("/:resourceID", auth(disable, accesscontrol.EvalPermission(actionRead, idScope)), routing.Wrap(a.getPermissions)) - r.Post("/:resourceID/users/:userID", auth(disable, accesscontrol.EvalPermission(actionWrite, idScope)), routing.Wrap(a.setUserPermission)) - r.Post("/:resourceID/teams/:teamID", auth(disable, accesscontrol.EvalPermission(actionWrite, idScope)), routing.Wrap(a.setTeamPermission)) - r.Post("/:resourceID/builtInRoles/:builtInRole", auth(disable, accesscontrol.EvalPermission(actionWrite, idScope)), routing.Wrap(a.setBuiltinRolePermission)) + r.Get("/:resourceID", uidSolver, auth(disable, accesscontrol.EvalPermission(actionRead, idScope)), routing.Wrap(a.getPermissions)) + r.Post("/:resourceID/users/:userID", uidSolver, auth(disable, accesscontrol.EvalPermission(actionWrite, idScope)), routing.Wrap(a.setUserPermission)) + r.Post("/:resourceID/teams/:teamID", uidSolver, auth(disable, accesscontrol.EvalPermission(actionWrite, idScope)), routing.Wrap(a.setTeamPermission)) + r.Post("/:resourceID/builtInRoles/:builtInRole", uidSolver, auth(disable, accesscontrol.EvalPermission(actionWrite, idScope)), routing.Wrap(a.setBuiltinRolePermission)) }) } diff --git a/pkg/services/accesscontrol/resourcepermissions/api_test.go b/pkg/services/accesscontrol/resourcepermissions/api_test.go index bf64c93d0d4..515b37f6e75 100644 --- a/pkg/services/accesscontrol/resourcepermissions/api_test.go +++ b/pkg/services/accesscontrol/resourcepermissions/api_test.go @@ -3,6 +3,7 @@ package resourcepermissions import ( "context" "encoding/json" + "errors" "fmt" "net/http" "net/http/httptest" @@ -17,6 +18,7 @@ import ( "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/web" ) @@ -152,20 +154,7 @@ func TestApi_getPermissions(t *testing.T) { service, sql := setupTestEnvironment(t, tt.permissions, testOptions) server := setupTestServer(t, &models.SignedInUser{OrgId: 1}, service) - // seed team 1 with "Edit" permission on dashboard 1 - team, err := sql.CreateTeam("test", "test@test.com", 1) - require.NoError(t, err) - _, err = service.SetTeamPermission(context.Background(), team.OrgId, team.Id, tt.resourceID, "Edit") - require.NoError(t, err) - // seed user 1 with "View" permission on dashboard 1 - u, err := sql.CreateUser(context.Background(), models.CreateUserCommand{Login: "test", OrgId: 1}) - require.NoError(t, err) - _, err = service.SetUserPermission(context.Background(), u.OrgId, accesscontrol.User{ID: u.Id}, tt.resourceID, "View") - require.NoError(t, err) - - // seed built in role Admin with "Edit" permission on dashboard 1 - _, err = service.SetBuiltInRolePermission(context.Background(), 1, "Admin", tt.resourceID, "Edit") - require.NoError(t, err) + seedPermissions(t, tt.resourceID, sql, service) permissions, recorder := getPermission(t, server, testOptions.Resource, tt.resourceID) assert.Equal(t, tt.expectedStatus, recorder.Code) @@ -418,6 +407,62 @@ func TestApi_setUserPermission(t *testing.T) { } } +type uidSolverTestCase struct { + desc string + uid string + resourceID string + expectedStatus int +} + +func TestApi_UidSolver(t *testing.T) { + tests := []uidSolverTestCase{ + { + desc: "expect uid to be mapped to id", + uid: "resourceUID", + resourceID: "1", + expectedStatus: http.StatusOK, + }, + { + desc: "expect 404 when uid is not mapped to an id", + uid: "notfound", + resourceID: "1", + expectedStatus: http.StatusNotFound, + }, + } + + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + userPermissions := []*accesscontrol.Permission{{Action: "dashboards.permissions:read", Scope: "dashboards:id:1"}} + service, sql := setupTestEnvironment(t, userPermissions, withSolver(testOptions, testSolver)) + server := setupTestServer(t, &models.SignedInUser{OrgId: 1}, service) + seedPermissions(t, tt.resourceID, sql, service) + + permissions, recorder := getPermission(t, server, testOptions.Resource, tt.uid) + assert.Equal(t, tt.expectedStatus, recorder.Code) + + if tt.expectedStatus == http.StatusOK { + assert.Len(t, permissions, 3, "expected three assignments: user, team, builtin") + for _, p := range permissions { + if p.UserID != 0 { + assert.Equal(t, "View", p.Permission) + } else if p.TeamID != 0 { + assert.Equal(t, "Edit", p.Permission) + } else { + assert.Equal(t, "Edit", p.Permission) + } + } + } else { + assert.Equal(t, tt.expectedStatus, recorder.Code) + } + }) + } +} + +func withSolver(options Options, solver uidSolver) Options { + options.UidSolver = solver + return options +} + func setupTestServer(t *testing.T, user *models.SignedInUser, service *Service) *web.Mux { server := web.New() server.UseMiddleware(web.Renderer(path.Join(setting.StaticRootPath, "views"), "[[", "]]")) @@ -457,6 +502,13 @@ var testOptions = Options{ }, } +var testSolver = func(ctx context.Context, orgID int64, uid string) (int64, error) { + if uid == "resourceUID" { + return 1, nil + } + return 0, errors.New("not found") +} + func getPermission(t *testing.T, server *web.Mux, resource, resourceID string) ([]resourcePermissionDTO, *httptest.ResponseRecorder) { req, err := http.NewRequest(http.MethodGet, fmt.Sprintf("/api/access-control/%s/%s", resource, resourceID), nil) require.NoError(t, err) @@ -480,3 +532,20 @@ func setPermission(t *testing.T, server *web.Mux, resource, resourceID, permissi return recorder } + +func seedPermissions(t *testing.T, resourceID string, sql *sqlstore.SQLStore, service *Service) { + t.Helper() + // seed team 1 with "Edit" permission on dashboard 1 + team, err := sql.CreateTeam("test", "test@test.com", 1) + require.NoError(t, err) + _, err = service.SetTeamPermission(context.Background(), team.OrgId, team.Id, resourceID, "Edit") + require.NoError(t, err) + // seed user 1 with "View" permission on dashboard 1 + u, err := sql.CreateUser(context.Background(), models.CreateUserCommand{Login: "test", OrgId: 1}) + require.NoError(t, err) + _, err = service.SetUserPermission(context.Background(), u.OrgId, accesscontrol.User{ID: u.Id}, resourceID, "View") + require.NoError(t, err) + // seed built in role Admin with "Edit" permission on dashboard 1 + _, err = service.SetBuiltInRolePermission(context.Background(), 1, "Admin", resourceID, "Edit") + require.NoError(t, err) +} diff --git a/pkg/services/accesscontrol/resourcepermissions/middleware.go b/pkg/services/accesscontrol/resourcepermissions/middleware.go new file mode 100644 index 00000000000..4412618554b --- /dev/null +++ b/pkg/services/accesscontrol/resourcepermissions/middleware.go @@ -0,0 +1,28 @@ +package resourcepermissions + +import ( + "context" + "net/http" + "strconv" + + "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/util" + "github.com/grafana/grafana/pkg/web" +) + +type uidSolver func(ctx context.Context, orgID int64, uid string) (int64, error) + +func solveUID(solve uidSolver) web.Handler { + return func(c *models.ReqContext) { + if solve != nil && util.IsValidShortUID(web.Params(c.Req)[":resourceID"]) { + params := web.Params(c.Req) + id, err := solve(c.Req.Context(), c.OrgId, params[":resourceID"]) + if err != nil { + c.JsonApiErr(http.StatusNotFound, "Resource not found", err) + return + } + params[":resourceID"] = strconv.FormatInt(id, 10) + web.SetURLParams(c.Req, params) + } + } +} diff --git a/pkg/services/accesscontrol/resourcepermissions/options.go b/pkg/services/accesscontrol/resourcepermissions/options.go index 12fcb5acdb9..2bd2bf66eb8 100644 --- a/pkg/services/accesscontrol/resourcepermissions/options.go +++ b/pkg/services/accesscontrol/resourcepermissions/options.go @@ -34,4 +34,6 @@ type Options struct { OnSetTeam func(session *sqlstore.DBSession, orgID, teamID int64, resourceID, permission string) error // OnSetBuiltInRole if configured will be called each time a permission is set for a built-in role OnSetBuiltInRole func(session *sqlstore.DBSession, orgID int64, builtInRole, resourceID, permission string) error + // UidSolver if configured will be used in a middleware to translate an uid to id for each request + UidSolver uidSolver } diff --git a/public/app/core/components/AccessControl/Permissions.tsx b/public/app/core/components/AccessControl/Permissions.tsx index a75dfa30f3b..b8dd561382d 100644 --- a/public/app/core/components/AccessControl/Permissions.tsx +++ b/public/app/core/components/AccessControl/Permissions.tsx @@ -19,12 +19,15 @@ const INITIAL_DESCRIPTION: Description = { }, }; +type ResourceId = string | number; +type Type = 'users' | 'teams' | 'builtInRoles'; + export type Props = { title?: string; buttonLabel?: string; addPermissionTitle?: string; resource: string; - resourceId: number; + resourceId: ResourceId; canListUsers: boolean; canSetPermissions: boolean; @@ -183,23 +186,23 @@ const getDescription = async (resource: string): Promise => { } }; -const getPermissions = (resource: string, resourceId: number): Promise => +const getPermissions = (resource: string, resourceId: ResourceId): Promise => getBackendSrv().get(`/api/access-control/${resource}/${resourceId}`); -const setUserPermission = (resource: string, resourceId: number, userId: number, permission: string) => +const setUserPermission = (resource: string, resourceId: ResourceId, userId: number, permission: string) => setPermission(resource, resourceId, 'users', userId, permission); -const setTeamPermission = (resource: string, resourceId: number, teamId: number, permission: string) => +const setTeamPermission = (resource: string, resourceId: ResourceId, teamId: number, permission: string) => setPermission(resource, resourceId, 'teams', teamId, permission); -const setBuiltInRolePermission = (resource: string, resourceId: number, builtInRole: string, permission: string) => +const setBuiltInRolePermission = (resource: string, resourceId: ResourceId, builtInRole: string, permission: string) => setPermission(resource, resourceId, 'builtInRoles', builtInRole, permission); const setPermission = ( resource: string, - resourceId: number, - type: 'users' | 'teams' | 'builtInRoles', + resourceId: ResourceId, + type: Type, typeId: number | string, permission: string ): Promise => - getBackendSrv().post(`/api/access-control/${resource}/${resourceId}/${type}/${typeId}/`, { permission }); + getBackendSrv().post(`/api/access-control/${resource}/${resourceId}/${type}/${typeId}`, { permission }); diff --git a/public/app/features/datasources/state/navModel.ts b/public/app/features/datasources/state/navModel.ts index 256ccd9e6c9..8f2dd5250ee 100644 --- a/public/app/features/datasources/state/navModel.ts +++ b/public/app/features/datasources/state/navModel.ts @@ -56,9 +56,9 @@ export function buildNavModel(dataSource: DataSourceSettings, plugin: GenericDat const dsPermissions = { active: false, icon: 'lock', - id: `datasource-permissions-${dataSource.id}`, + id: `datasource-permissions-${dataSource.uid}`, text: 'Permissions', - url: `datasources/edit/${dataSource.id}/permissions`, + url: `datasources/edit/${dataSource.uid}/permissions`, }; if (featureEnabled('dspermissions')) {