From ca13cbc8176d17a2ff67d17797e871cf6fc54a26 Mon Sep 17 00:00:00 2001 From: Bailin He <15058035+bailinhe@users.noreply.github.com> Date: Mon, 22 Apr 2024 16:03:56 -0400 Subject: [PATCH] RBAC V2 - Roles APIs (#245) * add roles query and APIs Signed-off-by: Bailin He * add changes per review Signed-off-by: Bailin He * Apply suggestions from code review Co-authored-by: John Schaeffer Signed-off-by: Bailin He <15058035+bailinhe@users.noreply.github.com> --------- Signed-off-by: Bailin He Signed-off-by: Bailin He <15058035+bailinhe@users.noreply.github.com> Co-authored-by: John Schaeffer --- Makefile | 4 +- cmd/server.go | 6 +- internal/api/assignments.go | 7 +- internal/api/assignments_test.go | 20 +- internal/api/errors.go | 10 + internal/api/response.go | 39 ++ internal/api/roles.go | 32 +- internal/api/roles_test.go | 20 +- internal/api/roles_v2.go | 260 +++++++++++++ internal/api/router.go | 13 + internal/api/router_test.go | 8 +- internal/api/types.go | 9 + internal/iapl/policy.go | 12 +- internal/iapl/rbac.go | 26 +- internal/query/default_test.go | 309 ++++++++++++++++ internal/query/errors.go | 28 +- internal/query/mock/mock.go | 90 ++++- internal/query/relations.go | 47 ++- internal/query/relations_test.go | 24 +- internal/query/roles.go | 15 + internal/query/roles_v2.go | 615 +++++++++++++++++++++++++++++++ internal/query/roles_v2_test.go | 401 ++++++++++++++++++++ internal/query/service.go | 43 +++ internal/query/zedtokens_test.go | 2 +- internal/storage/roles.go | 56 ++- 25 files changed, 1985 insertions(+), 111 deletions(-) create mode 100644 internal/api/errors.go create mode 100644 internal/api/roles_v2.go create mode 100644 internal/query/default_test.go create mode 100644 internal/query/roles_v2.go create mode 100644 internal/query/roles_v2_test.go diff --git a/Makefile b/Makefile index b478f7f1..08b9c270 100644 --- a/Makefile +++ b/Makefile @@ -52,12 +52,12 @@ ci: | golint test coverage ## Setup dev database and run tests. .PHONY: test test: ## Runs unit tests. @echo Running unit tests... - @go test -v -timeout 30s -cover -short -tags testtools ./... + @go test -v -timeout 120s -cover -short -tags testtools ./... .PHONY: coverage coverage: ## Generates a test coverage report. @echo Generating coverage report... - @go test -timeout 30s -tags testtools ./... -coverprofile=coverage.out -covermode=atomic + @go test -timeout 120s -tags testtools ./... -coverprofile=coverage.out -covermode=atomic @go tool cover -func=coverage.out @go tool cover -html=coverage.out diff --git a/cmd/server.go b/cmd/server.go index 250dad6f..e4e67408 100644 --- a/cmd/server.go +++ b/cmd/server.go @@ -21,9 +21,7 @@ import ( "go.infratographer.com/permissions-api/internal/storage" ) -var ( - apiDefaultListen = "0.0.0.0:7602" -) +var apiDefaultListen = "0.0.0.0:7602" var serverCmd = &cobra.Command{ Use: "server", @@ -89,7 +87,7 @@ func serve(_ context.Context, cfg *config.AppConfig) { logger.Fatalw("invalid spicedb policy", "error", err) } - engine, err := query.NewEngine("infratographer", spiceClient, kv, store, query.WithPolicy(policy)) + engine, err := query.NewEngine("infratographer", spiceClient, kv, store, query.WithPolicy(policy), query.WithLogger(logger)) if err != nil { logger.Fatalw("error creating engine", "error", err) } diff --git a/internal/api/assignments.go b/internal/api/assignments.go index aec8fb82..c39d8777 100644 --- a/internal/api/assignments.go +++ b/internal/api/assignments.go @@ -6,6 +6,7 @@ import ( "go.infratographer.com/x/gidx" + "go.infratographer.com/permissions-api/internal/iapl" "go.infratographer.com/permissions-api/internal/query" "go.infratographer.com/permissions-api/internal/types" @@ -62,7 +63,7 @@ func (r *Router) assignmentCreate(c echo.Context) error { return echo.NewHTTPError(http.StatusInternalServerError, "error getting role").SetInternal(err) } - if err := r.checkActionWithResponse(ctx, subjectResource, actionRoleUpdate, resource); err != nil { + if err := r.checkActionWithResponse(ctx, subjectResource, string(iapl.RoleActionUpdate), resource); err != nil { return err } @@ -112,7 +113,7 @@ func (r *Router) assignmentsList(c echo.Context) error { return echo.NewHTTPError(http.StatusInternalServerError, "error getting role").SetInternal(err) } - if err := r.checkActionWithResponse(ctx, subjectResource, actionRoleGet, resource); err != nil { + if err := r.checkActionWithResponse(ctx, subjectResource, string(iapl.RoleActionGet), resource); err != nil { return err } @@ -190,7 +191,7 @@ func (r *Router) assignmentDelete(c echo.Context) error { return echo.NewHTTPError(http.StatusInternalServerError, "error getting role").SetInternal(err) } - if err := r.checkActionWithResponse(ctx, subjectResource, actionRoleUpdate, resource); err != nil { + if err := r.checkActionWithResponse(ctx, subjectResource, string(iapl.RoleActionUpdate), resource); err != nil { return err } diff --git a/internal/api/assignments_test.go b/internal/api/assignments_test.go index 9aec492e..3aec473f 100644 --- a/internal/api/assignments_test.go +++ b/internal/api/assignments_test.go @@ -42,7 +42,7 @@ func TestAssignmentCreate(t *testing.T) { "subject_id": "bad-id", }, }, - SetupFn: func(ctx context.Context, t *testing.T) context.Context { + SetupFn: func(ctx context.Context, _ *testing.T) context.Context { engine := mock.Engine{ Namespace: "test", } @@ -67,7 +67,7 @@ func TestAssignmentCreate(t *testing.T) { "subject_id": "idntusr-abc123", }, }, - SetupFn: func(ctx context.Context, t *testing.T) context.Context { + SetupFn: func(ctx context.Context, _ *testing.T) context.Context { engine := mock.Engine{ Namespace: "test", } @@ -92,7 +92,7 @@ func TestAssignmentCreate(t *testing.T) { "subject_id": "idntusr-def456", }, }, - SetupFn: func(ctx context.Context, t *testing.T) context.Context { + SetupFn: func(ctx context.Context, _ *testing.T) context.Context { engine := mock.Engine{ Namespace: "test", } @@ -119,7 +119,7 @@ func TestAssignmentCreate(t *testing.T) { "subject_id": "idntusr-def456", }, }, - SetupFn: func(ctx context.Context, t *testing.T) context.Context { + SetupFn: func(ctx context.Context, _ *testing.T) context.Context { engine := mock.Engine{ Namespace: "test", } @@ -211,7 +211,7 @@ func TestAssignmentsList(t *testing.T) { { Name: "RoleResourceNotFound", Input: "/api/v1/roles/permrol-abc123/assignments", - SetupFn: func(ctx context.Context, t *testing.T) context.Context { + SetupFn: func(ctx context.Context, _ *testing.T) context.Context { engine := mock.Engine{ Namespace: "test", } @@ -233,7 +233,7 @@ func TestAssignmentsList(t *testing.T) { { Name: "AssignmentsRetrieved", Input: "/api/v1/roles/permrol-abc123/assignments", - SetupFn: func(ctx context.Context, t *testing.T) context.Context { + SetupFn: func(ctx context.Context, _ *testing.T) context.Context { engine := mock.Engine{ Namespace: "test", } @@ -327,7 +327,7 @@ func TestAssignmentDelete(t *testing.T) { "subject_id": "bad-id", }, }, - SetupFn: func(ctx context.Context, t *testing.T) context.Context { + SetupFn: func(ctx context.Context, _ *testing.T) context.Context { engine := mock.Engine{ Namespace: "test", } @@ -352,7 +352,7 @@ func TestAssignmentDelete(t *testing.T) { "subject_id": "idntusr-abc123", }, }, - SetupFn: func(ctx context.Context, t *testing.T) context.Context { + SetupFn: func(ctx context.Context, _ *testing.T) context.Context { engine := mock.Engine{ Namespace: "test", } @@ -377,7 +377,7 @@ func TestAssignmentDelete(t *testing.T) { "subject_id": "idntusr-def456", }, }, - SetupFn: func(ctx context.Context, t *testing.T) context.Context { + SetupFn: func(ctx context.Context, _ *testing.T) context.Context { engine := mock.Engine{ Namespace: "test", } @@ -404,7 +404,7 @@ func TestAssignmentDelete(t *testing.T) { "subject_id": "idntusr-def456", }, }, - SetupFn: func(ctx context.Context, t *testing.T) context.Context { + SetupFn: func(ctx context.Context, _ *testing.T) context.Context { engine := mock.Engine{ Namespace: "test", } diff --git a/internal/api/errors.go b/internal/api/errors.go new file mode 100644 index 00000000..f11ff885 --- /dev/null +++ b/internal/api/errors.go @@ -0,0 +1,10 @@ +package api + +import "errors" + +var ( + // ErrInvalidID is returned when the ID is invalid + ErrInvalidID = errors.New("invalid ID") + // ErrParsingRequestBody is returned when failing to parse the request body + ErrParsingRequestBody = errors.New("error parsing request body") +) diff --git a/internal/api/response.go b/internal/api/response.go index a06b8c24..27353a47 100644 --- a/internal/api/response.go +++ b/internal/api/response.go @@ -2,6 +2,15 @@ package api import ( "errors" + "fmt" + "net/http" + + "github.com/labstack/echo/v4" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + + "go.infratographer.com/permissions-api/internal/query" + "go.infratographer.com/permissions-api/internal/storage" ) // ErrorResponse represents the data that the server will return on any given call @@ -20,3 +29,33 @@ var ( // ErrResourceAlreadyExists is returned when the resource already exists ErrResourceAlreadyExists = errors.New("resource already exists") ) + +func (r *Router) errorResponse(basemsg string, err error) *echo.HTTPError { + msg := fmt.Sprintf("%s: %s", basemsg, err.Error()) + httpstatus := http.StatusInternalServerError + + switch { + case + errors.Is(err, query.ErrInvalidType), + errors.Is(err, query.ErrInvalidArgument), + errors.Is(err, query.ErrInvalidAction), + errors.Is(err, query.ErrInvalidNamespace), + errors.Is(err, ErrInvalidID), + status.Code(err) == codes.InvalidArgument, + status.Code(err) == codes.FailedPrecondition: + httpstatus = http.StatusBadRequest + case + errors.Is(err, storage.ErrNoRoleFound), + errors.Is(err, query.ErrRoleNotFound), + errors.Is(err, query.ErrRoleBindingNotFound): + httpstatus = http.StatusNotFound + case + errors.Is(err, storage.ErrRoleAlreadyExists), + errors.Is(err, storage.ErrRoleNameTaken): + httpstatus = http.StatusConflict + default: + msg = basemsg + } + + return echo.NewHTTPError(httpstatus, msg).SetInternal(err) +} diff --git a/internal/api/roles.go b/internal/api/roles.go index 05b1f8f6..2e9895e3 100644 --- a/internal/api/roles.go +++ b/internal/api/roles.go @@ -3,6 +3,7 @@ package api import ( "errors" "net/http" + "strings" "time" "github.com/labstack/echo/v4" @@ -10,18 +11,11 @@ import ( "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" + "go.infratographer.com/permissions-api/internal/iapl" "go.infratographer.com/permissions-api/internal/query" "go.infratographer.com/permissions-api/internal/storage" ) -const ( - actionRoleCreate = "role_create" - actionRoleGet = "role_get" - actionRoleList = "role_list" - actionRoleUpdate = "role_update" - actionRoleDelete = "role_delete" -) - func (r *Router) roleCreate(c echo.Context) error { resourceIDStr := c.Param("id") @@ -50,11 +44,14 @@ func (r *Router) roleCreate(c echo.Context) error { return echo.NewHTTPError(http.StatusBadRequest, "error creating resource").SetInternal(err) } - if err := r.checkActionWithResponse(ctx, subjectResource, actionRoleCreate, resource); err != nil { + if err := r.checkActionWithResponse(ctx, subjectResource, string(iapl.RoleActionCreate), resource); err != nil { return err } - role, err := r.engine.CreateRole(ctx, subjectResource, resource, reqBody.Name, reqBody.Actions) + role, err := r.engine.CreateRole( + ctx, subjectResource, resource, + strings.TrimSpace(reqBody.Name), reqBody.Actions, + ) switch { case err == nil: @@ -120,11 +117,14 @@ func (r *Router) roleUpdate(c echo.Context) error { return echo.NewHTTPError(http.StatusInternalServerError, "error getting resource").SetInternal(err) } - if err := r.checkActionWithResponse(ctx, subjectResource, actionRoleUpdate, resource); err != nil { + if err := r.checkActionWithResponse(ctx, subjectResource, string(iapl.RoleActionUpdate), resource); err != nil { return err } - role, err := r.engine.UpdateRole(ctx, subjectResource, roleResource, reqBody.Name, reqBody.Actions) + role, err := r.engine.UpdateRole( + ctx, subjectResource, roleResource, + strings.TrimSpace(reqBody.Name), reqBody.Actions, + ) switch { case err == nil: @@ -185,7 +185,7 @@ func (r *Router) roleGet(c echo.Context) error { // TODO: This shows an error for the role's resource, not the role. Determine if that // matters. - if err := r.checkActionWithResponse(ctx, subjectResource, actionRoleGet, resource); err != nil { + if err := r.checkActionWithResponse(ctx, subjectResource, string(iapl.RoleActionGet), resource); err != nil { return err } @@ -234,7 +234,7 @@ func (r *Router) rolesList(c echo.Context) error { return echo.NewHTTPError(http.StatusBadRequest, "error creating resource").SetInternal(err) } - if err := r.checkActionWithResponse(ctx, subjectResource, actionRoleList, resource); err != nil { + if err := r.checkActionWithResponse(ctx, subjectResource, string(iapl.RoleActionList), resource); err != nil { return err } @@ -297,7 +297,7 @@ func (r *Router) roleDelete(c echo.Context) error { return echo.NewHTTPError(http.StatusInternalServerError, "error getting resource").SetInternal(err) } - if err := r.checkActionWithResponse(ctx, subjectResource, actionRoleDelete, resource); err != nil { + if err := r.checkActionWithResponse(ctx, subjectResource, string(iapl.RoleActionDelete), resource); err != nil { return err } @@ -351,7 +351,7 @@ func (r *Router) roleGetResource(c echo.Context) error { return echo.NewHTTPError(http.StatusInternalServerError, "error getting role").SetInternal(err) } - if err := r.checkActionWithResponse(ctx, subjectResource, actionRoleGet, resource); err != nil { + if err := r.checkActionWithResponse(ctx, subjectResource, string(iapl.RoleActionGet), resource); err != nil { return err } diff --git a/internal/api/roles_test.go b/internal/api/roles_test.go index 4c81b7ba..f7a00f43 100644 --- a/internal/api/roles_test.go +++ b/internal/api/roles_test.go @@ -50,7 +50,7 @@ func TestRoleCreate(t *testing.T) { }, }, }, - SetupFn: func(ctx context.Context, t *testing.T) context.Context { + SetupFn: func(ctx context.Context, _ *testing.T) context.Context { engine := mock.Engine{ Namespace: "test", } @@ -82,7 +82,7 @@ func TestRoleCreate(t *testing.T) { }, }, }, - SetupFn: func(ctx context.Context, t *testing.T) context.Context { + SetupFn: func(ctx context.Context, _ *testing.T) context.Context { engine := mock.Engine{ Namespace: "test", } @@ -114,7 +114,7 @@ func TestRoleCreate(t *testing.T) { }, }, }, - SetupFn: func(ctx context.Context, t *testing.T) context.Context { + SetupFn: func(ctx context.Context, _ *testing.T) context.Context { engine := mock.Engine{ Namespace: "test", } @@ -267,7 +267,7 @@ func TestRoleUpdate(t *testing.T) { }, }, }, - SetupFn: func(ctx context.Context, t *testing.T) context.Context { + SetupFn: func(ctx context.Context, _ *testing.T) context.Context { engine := mock.Engine{ Namespace: "test", } @@ -300,7 +300,7 @@ func TestRoleUpdate(t *testing.T) { }, }, }, - SetupFn: func(ctx context.Context, t *testing.T) context.Context { + SetupFn: func(ctx context.Context, _ *testing.T) context.Context { engine := mock.Engine{ Namespace: "test", } @@ -333,7 +333,7 @@ func TestRoleUpdate(t *testing.T) { }, }, }, - SetupFn: func(ctx context.Context, t *testing.T) context.Context { + SetupFn: func(ctx context.Context, _ *testing.T) context.Context { engine := mock.Engine{ Namespace: "test", } @@ -473,7 +473,7 @@ func TestRoleGet(t *testing.T) { { Name: "RoleResourceNotFound", Input: "/api/v1/roles/permrol-abc123", - SetupFn: func(ctx context.Context, t *testing.T) context.Context { + SetupFn: func(ctx context.Context, _ *testing.T) context.Context { engine := mock.Engine{ Namespace: "test", } @@ -593,7 +593,7 @@ func TestRoleDelete(t *testing.T) { { Name: "UnknownError", Input: "/api/v1/roles/permrol-abc123", - SetupFn: func(ctx context.Context, t *testing.T) context.Context { + SetupFn: func(ctx context.Context, _ *testing.T) context.Context { engine := mock.Engine{ Namespace: "test", } @@ -617,7 +617,7 @@ func TestRoleDelete(t *testing.T) { { Name: "RoleResourceNotFound", Input: "/api/v1/roles/permrol-abc123", - SetupFn: func(ctx context.Context, t *testing.T) context.Context { + SetupFn: func(ctx context.Context, _ *testing.T) context.Context { engine := mock.Engine{ Namespace: "test", } @@ -639,7 +639,7 @@ func TestRoleDelete(t *testing.T) { { Name: "RoleDeleted", Input: "/api/v1/roles/permrol-abc123", - SetupFn: func(ctx context.Context, t *testing.T) context.Context { + SetupFn: func(ctx context.Context, _ *testing.T) context.Context { engine := mock.Engine{ Namespace: "test", } diff --git a/internal/api/roles_v2.go b/internal/api/roles_v2.go new file mode 100644 index 00000000..27ac65d7 --- /dev/null +++ b/internal/api/roles_v2.go @@ -0,0 +1,260 @@ +package api + +import ( + "fmt" + "net/http" + "strings" + "time" + + "go.infratographer.com/permissions-api/internal/iapl" + + "github.com/labstack/echo/v4" + "go.infratographer.com/x/gidx" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/trace" +) + +func (r *Router) roleV2Create(c echo.Context) error { + resourceIDStr := c.Param("id") + + ctx, span := tracer.Start(c.Request().Context(), "api.roleV2Create", trace.WithAttributes(attribute.String("id", resourceIDStr))) + defer span.End() + + resourceID, err := gidx.Parse(resourceIDStr) + if err != nil { + return r.errorResponse("error parsing resource ID", fmt.Errorf("%w: %s", ErrInvalidID, err.Error())) + } + + var reqBody createRoleRequest + + err = c.Bind(&reqBody) + if err != nil { + return r.errorResponse(err.Error(), ErrParsingRequestBody) + } + + subjectResource, err := r.currentSubject(c) + if err != nil { + return err + } + + resource, err := r.engine.NewResourceFromID(resourceID) + if err != nil { + return r.errorResponse("error creating resource", err) + } + + if err := r.checkActionWithResponse(ctx, subjectResource, string(iapl.RoleActionCreate), resource); err != nil { + return err + } + + role, err := r.engine.CreateRoleV2( + ctx, subjectResource, resource, + strings.TrimSpace(reqBody.Name), reqBody.Actions, + ) + if err != nil { + return r.errorResponse("error creating resource", err) + } + + resp := roleResponse{ + ID: role.ID, + Name: role.Name, + Actions: role.Actions, + ResourceID: role.ResourceID, + CreatedBy: role.CreatedBy, + UpdatedBy: role.UpdatedBy, + CreatedAt: role.CreatedAt.Format(time.RFC3339), + UpdatedAt: role.UpdatedAt.Format(time.RFC3339), + } + + return c.JSON(http.StatusCreated, resp) +} + +func (r *Router) roleV2Update(c echo.Context) error { + roleIDStr := c.Param("role_id") + + ctx, span := tracer.Start(c.Request().Context(), "api.roleV2Update", trace.WithAttributes(attribute.String("id", roleIDStr))) + defer span.End() + + roleID, err := gidx.Parse(roleIDStr) + if err != nil { + return r.errorResponse("error parsing resource ID", fmt.Errorf("%w: %s", ErrInvalidID, err.Error())) + } + + var reqBody updateRoleRequest + + err = c.Bind(&reqBody) + if err != nil { + return r.errorResponse(err.Error(), ErrParsingRequestBody) + } + + subjectResource, err := r.currentSubject(c) + if err != nil { + return err + } + + // Roles themselves are the resource, permissions check should be performed + // on the roles themselves. + roleResource, err := r.engine.NewResourceFromID(roleID) + if err != nil { + return r.errorResponse("error creating resource", err) + } + + if err := r.checkActionWithResponse(ctx, subjectResource, string(iapl.RoleActionGet), roleResource); err != nil { + return err + } + + role, err := r.engine.UpdateRoleV2( + ctx, subjectResource, roleResource, + strings.TrimSpace(reqBody.Name), reqBody.Actions, + ) + if err != nil { + return r.errorResponse("error updating role", err) + } + + resp := roleResponse{ + ID: role.ID, + Name: role.Name, + Actions: role.Actions, + ResourceID: role.ResourceID, + CreatedBy: role.CreatedBy, + UpdatedBy: role.UpdatedBy, + CreatedAt: role.CreatedAt.Format(time.RFC3339), + UpdatedAt: role.UpdatedAt.Format(time.RFC3339), + } + + return c.JSON(http.StatusOK, resp) +} + +func (r *Router) roleV2Get(c echo.Context) error { + roleIDStr := c.Param("role_id") + + ctx, span := tracer.Start(c.Request().Context(), "api.roleV2Get", trace.WithAttributes(attribute.String("id", roleIDStr))) + defer span.End() + + roleResourceID, err := gidx.Parse(roleIDStr) + if err != nil { + return r.errorResponse("error parsing resource ID", fmt.Errorf("%w: %s", ErrInvalidID, err.Error())) + } + + subjectResource, err := r.currentSubject(c) + if err != nil { + return err + } + + // Roles themselves are the resource, permissions check should be performed + // on the roles themselves. + roleResource, err := r.engine.NewResourceFromID(roleResourceID) + if err != nil { + return r.errorResponse("error creating resource", err) + } + + if err := r.checkActionWithResponse(ctx, subjectResource, string(iapl.RoleActionGet), roleResource); err != nil { + return err + } + + role, err := r.engine.GetRoleV2(ctx, roleResource) + if err != nil { + return r.errorResponse("error getting role", err) + } + + resp := roleResponse{ + ID: role.ID, + Name: role.Name, + Actions: role.Actions, + ResourceID: role.ResourceID, + CreatedBy: role.CreatedBy, + UpdatedBy: role.UpdatedBy, + CreatedAt: role.CreatedAt.Format(time.RFC3339), + UpdatedAt: role.UpdatedAt.Format(time.RFC3339), + } + + return c.JSON(http.StatusOK, resp) +} + +func (r *Router) roleV2sList(c echo.Context) error { + resourceIDStr := c.Param("id") + + ctx, span := tracer.Start(c.Request().Context(), "api.roleV2sList", trace.WithAttributes(attribute.String("id", resourceIDStr))) + defer span.End() + + resourceID, err := gidx.Parse(resourceIDStr) + if err != nil { + return r.errorResponse("error parsing resource ID", fmt.Errorf("%w: %s", ErrInvalidID, err.Error())) + } + + subjectResource, err := r.currentSubject(c) + if err != nil { + return err + } + + resource, err := r.engine.NewResourceFromID(resourceID) + if err != nil { + return r.errorResponse("error creating resource", err) + } + + if err := r.checkActionWithResponse(ctx, subjectResource, string(iapl.RoleActionList), resource); err != nil { + return err + } + + roles, err := r.engine.ListRolesV2(ctx, resource) + if err != nil { + return r.errorResponse("error getting roles", err) + } + + resp := listRolesV2Response{ + Data: []listRolesV2Role{}, + } + + for _, role := range roles { + roleResp := listRolesV2Role{ + ID: role.ID, + Name: role.Name, + } + + resp.Data = append(resp.Data, roleResp) + } + + return c.JSON(http.StatusOK, resp) +} + +func (r *Router) roleV2Delete(c echo.Context) error { + roleIDStr := c.Param("id") + + ctx, span := tracer.Start(c.Request().Context(), "api.roleV2Delete", trace.WithAttributes(attribute.String("id", roleIDStr))) + defer span.End() + + roleResourceID, err := gidx.Parse(roleIDStr) + if err != nil { + return r.errorResponse("error parsing resource ID", fmt.Errorf("%w: %s", ErrInvalidID, err.Error())) + } + + subjectResource, err := r.currentSubject(c) + if err != nil { + return err + } + + // Roles themselves are the resource, permissions check should be performed + // on the roles themselves. + roleResource, err := r.engine.NewResourceFromID(roleResourceID) + if err != nil { + return r.errorResponse("error creating resource", err) + } + + if err := r.checkActionWithResponse(ctx, subjectResource, string(iapl.RoleActionDelete), roleResource); err != nil { + return err + } + + err = r.engine.DeleteRoleV2(ctx, roleResource) + if err != nil { + return r.errorResponse("error deleting role", err) + } + + resp := deleteRoleResponse{ + Success: true, + } + + return c.JSON(http.StatusOK, resp) +} + +func (r *Router) listActions(c echo.Context) error { + return c.JSON(http.StatusOK, r.engine.AllActions()) +} diff --git a/internal/api/router.go b/internal/api/router.go index 4ce585b9..bc389f9c 100644 --- a/internal/api/router.go +++ b/internal/api/router.go @@ -75,6 +75,19 @@ func (r *Router) Routes(rg *echo.Group) { v1.GET("/allow", r.checkAction) v1.POST("/allow", r.checkAllActions) } + + v2 := rg.Group("api/v2") + { + v2.Use(r.authMW) + + v2.POST("/resources/:id/roles", r.roleV2Create) + v2.GET("/resources/:id/roles", r.roleV2sList) + v2.GET("/roles/:role_id", r.roleV2Get) + v2.PATCH("/roles/:role_id", r.roleV2Update) + v2.DELETE("/roles/:id", r.roleV2Delete) + + v2.GET("/actions", r.listActions) + } } func errorMiddleware(next echo.HandlerFunc) echo.HandlerFunc { diff --git a/internal/api/router_test.go b/internal/api/router_test.go index 1d4ea5be..cd8366e2 100644 --- a/internal/api/router_test.go +++ b/internal/api/router_test.go @@ -53,7 +53,7 @@ func TestErrorMiddleware(t *testing.T) { Input: testinput{ path: "/test", }, - CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[*httptest.ResponseRecorder]) { + CheckFn: func(_ context.Context, t *testing.T, res testingx.TestResult[*httptest.ResponseRecorder]) { require.NoError(t, res.Err) require.NotNil(t, res.Success) @@ -65,7 +65,7 @@ func TestErrorMiddleware(t *testing.T) { Input: testinput{ path: "/test?error=echo", }, - CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[*httptest.ResponseRecorder]) { + CheckFn: func(_ context.Context, t *testing.T, res testingx.TestResult[*httptest.ResponseRecorder]) { require.NoError(t, res.Err) require.NotNil(t, res.Success) @@ -77,7 +77,7 @@ func TestErrorMiddleware(t *testing.T) { Input: testinput{ path: "/test?error=other", }, - CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[*httptest.ResponseRecorder]) { + CheckFn: func(_ context.Context, t *testing.T, res testingx.TestResult[*httptest.ResponseRecorder]) { require.NoError(t, res.Err) require.NotNil(t, res.Success) @@ -90,7 +90,7 @@ func TestErrorMiddleware(t *testing.T) { path: "/test", delay: time.Second / 2, }, - CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[*httptest.ResponseRecorder]) { + CheckFn: func(_ context.Context, t *testing.T, res testingx.TestResult[*httptest.ResponseRecorder]) { require.NoError(t, res.Err) require.NotNil(t, res.Success) diff --git a/internal/api/types.go b/internal/api/types.go index e02e96c7..48aa720c 100644 --- a/internal/api/types.go +++ b/internal/api/types.go @@ -71,3 +71,12 @@ type assignmentItem struct { type listAssignmentsResponse struct { Data []assignmentItem `json:"data"` } + +type listRolesV2Response struct { + Data []listRolesV2Role `json:"data"` +} + +type listRolesV2Role struct { + ID gidx.PrefixedID `json:"id"` + Name string `json:"name"` +} diff --git a/internal/iapl/policy.go b/internal/iapl/policy.go index f79146a0..12f4f51d 100644 --- a/internal/iapl/policy.go +++ b/internal/iapl/policy.go @@ -290,7 +290,7 @@ func (v *policy) validateResourceTypes() error { return fmt.Errorf("%s: relationships: %s: %w", resourceType.Name, tt.Name, ErrorUnknownType) } - if tt.SubjectRelation != "" && !v.findRelationship(v.rt[tt.Name].Relationships, tt.SubjectRelation) { + if tt.SubjectRelation != "" && !v.findRelationship(v.rt[tt.Name].Relationships, tt.SubjectRelation) && !v.findActionBinding(tt.SubjectRelation, tt.Name) { return fmt.Errorf("%s: subject-relation: %s: %w", resourceType.Name, tt.SubjectRelation, ErrorUnknownRelation) } } @@ -840,3 +840,13 @@ func (v *policy) findRelationship(rels []Relationship, name string) bool { return false } + +func (v *policy) findActionBinding(actionName string, typeName string) bool { + for _, bn := range v.bn { + if bn.ActionName == actionName && bn.TypeName == typeName { + return true + } + } + + return false +} diff --git a/internal/iapl/rbac.go b/internal/iapl/rbac.go index 81b9d21b..13c0815e 100644 --- a/internal/iapl/rbac.go +++ b/internal/iapl/rbac.go @@ -26,20 +26,36 @@ const ( GrantRelationship = "grant" ) +// RoleAction is the list of actions that can be performed on a role resource +type RoleAction string + +const ( + // RoleActionCreate is the action name to create a role + RoleActionCreate RoleAction = "role_create" + // RoleActionGet is the action name to get a role + RoleActionGet RoleAction = "role_get" + // RoleActionList is the action name to list roles + RoleActionList RoleAction = "role_list" + // RoleActionUpdate is the action name to update a role + RoleActionUpdate RoleAction = "role_update" + // RoleActionDelete is the action name to delete a role + RoleActionDelete RoleAction = "role_delete" +) + // RoleBindingAction is the list of actions that can be performed on a role-binding resource type RoleBindingAction string const ( // RoleBindingActionCreate is the action name to create a role binding - RoleBindingActionCreate RoleBindingAction = "rolebinding_create" + RoleBindingActionCreate RoleBindingAction = "iam_rolebinding_create" // RoleBindingActionUpdate is the action name to update a role binding - RoleBindingActionUpdate RoleBindingAction = "rolebinding_update" + RoleBindingActionUpdate RoleBindingAction = "iam_rolebinding_update" // RoleBindingActionDelete is the action name to delete a role binding - RoleBindingActionDelete RoleBindingAction = "rolebinding_delete" + RoleBindingActionDelete RoleBindingAction = "iam_rolebinding_delete" // RoleBindingActionGet is the action name to get a role binding - RoleBindingActionGet RoleBindingAction = "rolebinding_get" + RoleBindingActionGet RoleBindingAction = "iam_rolebinding_get" // RoleBindingActionList is the action name to list role bindings - RoleBindingActionList RoleBindingAction = "rolebinding_list" + RoleBindingActionList RoleBindingAction = "iam_rolebinding_list" ) // ResourceRoleBindingV2 describes the relationships that will be created diff --git a/internal/query/default_test.go b/internal/query/default_test.go new file mode 100644 index 00000000..ac2b5703 --- /dev/null +++ b/internal/query/default_test.go @@ -0,0 +1,309 @@ +package query + +import ( + "go.infratographer.com/permissions-api/internal/iapl" + "go.infratographer.com/permissions-api/internal/types" +) + +// DefaultPolicyDocumentV2 returns the default policy document that supports +// RBAC V2 +func DefaultPolicyDocumentV2() iapl.PolicyDocument { + rbv2WithInheritFromOwner := iapl.Condition{ + RoleBindingV2: &iapl.ConditionRoleBindingV2{}, + } + + rbv2WithInheritFromParent := iapl.Condition{ + RoleBindingV2: &iapl.ConditionRoleBindingV2{}, + } + + return iapl.PolicyDocument{ + RBAC: &iapl.RBAC{ + RoleResource: iapl.RBACResourceDefinition{ + Name: "rolev2", + IDPrefix: "permrv2", + }, + RoleBindingResource: iapl.RBACResourceDefinition{ + Name: "role_binding", + IDPrefix: "permrbn", + }, + RoleSubjectTypes: []string{"user", "client"}, + RoleOwners: []string{"tenant"}, + RoleBindingSubjects: []types.TargetType{ + {Name: "user"}, + {Name: "client"}, + {Name: "group", SubjectRelation: "member"}, + }, + }, + Unions: []iapl.Union{ + { + Name: "group_member", + ResourceTypes: []types.TargetType{ + {Name: "user"}, + {Name: "client"}, + {Name: "group", SubjectRelation: "member"}, + }, + }, + { + Name: "tenant_member", + ResourceTypes: []types.TargetType{ + {Name: "user"}, + {Name: "client"}, + {Name: "group", SubjectRelation: "member"}, + {Name: "tenant", SubjectRelation: "member"}, + }, + }, + { + Name: "resourceowner", + ResourceTypes: []types.TargetType{ + {Name: "tenant"}, + }, + }, + { + Name: "resourceowner_relationship", + ResourceTypes: []types.TargetType{ + {Name: "tenant"}, {Name: "group", SubjectRelation: "parent"}, + }, + }, + { + Name: "subject", + ResourceTypes: []types.TargetType{ + {Name: "user"}, {Name: "client"}, + }, + }, + { + Name: "group_parent", + ResourceTypes: []types.TargetType{ + {Name: "group"}, + {Name: "group", SubjectRelation: "parent"}, + {Name: "tenant"}, + {Name: "tenant", SubjectRelation: "parent"}, + }, + }, + { + Name: "tenant_parent", + ResourceTypes: []types.TargetType{ + {Name: "tenant"}, {Name: "tenant", SubjectRelation: "parent"}, + }, + }, + }, + ResourceTypes: []iapl.ResourceType{ + {Name: "rolev2", IDPrefix: "permrv2"}, + {Name: "role_binding", IDPrefix: "permrbn"}, + {Name: "user", IDPrefix: "idntusr"}, + {Name: "client", IDPrefix: "idntclt"}, + { + Name: "group", + IDPrefix: "idntgrp", + RoleBindingV2: &iapl.ResourceRoleBindingV2{ + InheritPermissionsFrom: []string{"parent"}, + }, + Relationships: []iapl.Relationship{ + {Relation: "parent", TargetTypes: []types.TargetType{{Name: "group_parent"}}}, + {Relation: "member", TargetTypes: []types.TargetType{{Name: "group_member"}}}, + {Relation: "grant", TargetTypes: []types.TargetType{{Name: "role_binding"}}}, + }, + }, + { + Name: "tenant", + IDPrefix: "tnntten", + RoleBindingV2: &iapl.ResourceRoleBindingV2{ + InheritPermissionsFrom: []string{"parent"}, + }, + Relationships: []iapl.Relationship{ + {Relation: "parent", TargetTypes: []types.TargetType{{Name: "tenant_parent"}}}, + {Relation: "member", TargetTypes: []types.TargetType{{Name: "tenant_member"}}}, + {Relation: "grant", TargetTypes: []types.TargetType{{Name: "role_binding"}}}, + }, + }, + { + Name: "loadbalancer", + IDPrefix: "loadbal", + RoleBindingV2: &iapl.ResourceRoleBindingV2{ + InheritPermissionsFrom: []string{"owner"}, + }, + Relationships: []iapl.Relationship{ + {Relation: "owner", TargetTypes: []types.TargetType{{Name: "resourceowner_relationship"}}}, + {Relation: "grant", TargetTypes: []types.TargetType{{Name: "role_binding"}}}, + }, + }, + }, + Actions: []iapl.Action{ + {Name: "role_create"}, + {Name: "role_get"}, + {Name: "role_list"}, + {Name: "role_update"}, + {Name: "role_delete"}, + {Name: "loadbalancer_create"}, + {Name: "loadbalancer_get"}, + {Name: "loadbalancer_list"}, + {Name: "loadbalancer_update"}, + {Name: "loadbalancer_delete"}, + }, + ActionBindings: []iapl.ActionBinding{ + { + ActionName: "role_get", + TypeName: "rolev2", + Conditions: []iapl.Condition{ + { + RelationshipAction: &iapl.ConditionRelationshipAction{ + Relation: "owner", + ActionName: "role_get", + }, + }, + }, + }, + { + ActionName: "role_update", + TypeName: "rolev2", + Conditions: []iapl.Condition{ + { + RelationshipAction: &iapl.ConditionRelationshipAction{ + Relation: "owner", + ActionName: "role_update", + }, + }, + }, + }, + { + ActionName: "role_delete", + TypeName: "rolev2", + Conditions: []iapl.Condition{ + { + RelationshipAction: &iapl.ConditionRelationshipAction{ + Relation: "owner", + ActionName: "role_delete", + }, + }, + }, + }, + { + ActionName: "role_create", + TypeName: "resourceowner", + Conditions: []iapl.Condition{rbv2WithInheritFromParent}, + }, + { + ActionName: "role_create", + TypeName: "group", + Conditions: []iapl.Condition{rbv2WithInheritFromParent}, + }, + { + ActionName: "role_get", + TypeName: "resourceowner", + Conditions: []iapl.Condition{rbv2WithInheritFromParent}, + }, + { + ActionName: "role_get", + TypeName: "group", + Conditions: []iapl.Condition{rbv2WithInheritFromParent}, + }, + { + ActionName: "role_list", + TypeName: "resourceowner", + Conditions: []iapl.Condition{rbv2WithInheritFromParent}, + }, + { + ActionName: "role_list", + TypeName: "group", + Conditions: []iapl.Condition{rbv2WithInheritFromParent}, + }, + { + ActionName: "role_update", + TypeName: "resourceowner", + Conditions: []iapl.Condition{rbv2WithInheritFromParent}, + }, + { + ActionName: "role_update", + TypeName: "group", + Conditions: []iapl.Condition{rbv2WithInheritFromParent}, + }, + { + ActionName: "role_delete", + TypeName: "resourceowner", + Conditions: []iapl.Condition{rbv2WithInheritFromParent}, + }, + { + ActionName: "role_delete", + TypeName: "group", + Conditions: []iapl.Condition{rbv2WithInheritFromParent}, + }, + { + ActionName: "loadbalancer_get", + TypeName: "loadbalancer", + Conditions: []iapl.Condition{rbv2WithInheritFromOwner}, + }, + { + ActionName: "loadbalancer_update", + TypeName: "loadbalancer", + Conditions: []iapl.Condition{rbv2WithInheritFromOwner}, + }, + { + ActionName: "loadbalancer_delete", + TypeName: "loadbalancer", + Conditions: []iapl.Condition{rbv2WithInheritFromOwner}, + }, + { + ActionName: "loadbalancer_create", + TypeName: "resourceowner", + Conditions: []iapl.Condition{rbv2WithInheritFromParent}, + }, + { + ActionName: "loadbalancer_create", + TypeName: "group", + Conditions: []iapl.Condition{rbv2WithInheritFromParent}, + }, + { + ActionName: "loadbalancer_get", + TypeName: "resourceowner", + Conditions: []iapl.Condition{rbv2WithInheritFromParent}, + }, + { + ActionName: "loadbalancer_get", + TypeName: "group", + Conditions: []iapl.Condition{rbv2WithInheritFromParent}, + }, + { + ActionName: "loadbalancer_list", + TypeName: "resourceowner", + Conditions: []iapl.Condition{rbv2WithInheritFromParent}, + }, + { + ActionName: "loadbalancer_list", + TypeName: "group", + Conditions: []iapl.Condition{rbv2WithInheritFromParent}, + }, + { + ActionName: "loadbalancer_update", + TypeName: "resourceowner", + Conditions: []iapl.Condition{rbv2WithInheritFromParent}, + }, + { + ActionName: "loadbalancer_update", + TypeName: "group", + Conditions: []iapl.Condition{rbv2WithInheritFromParent}, + }, + { + ActionName: "loadbalancer_delete", + TypeName: "resourceowner", + Conditions: []iapl.Condition{rbv2WithInheritFromParent}, + }, + { + ActionName: "loadbalancer_delete", + TypeName: "group", + Conditions: []iapl.Condition{rbv2WithInheritFromParent}, + }, + }, + } +} + +// DefaultPolicyV2 generates the default policy for permissions-api that supports +// RBAC V2 +func DefaultPolicyV2() iapl.Policy { + policyDocument := DefaultPolicyDocumentV2() + + policy := iapl.NewPolicy(policyDocument) + if err := policy.Validate(); err != nil { + panic(err) + } + + return policy +} diff --git a/internal/query/errors.go b/internal/query/errors.go index 57212232..6f21394e 100644 --- a/internal/query/errors.go +++ b/internal/query/errors.go @@ -1,6 +1,9 @@ package query -import "errors" +import ( + "errors" + "fmt" +) var ( // ErrActionNotAssigned represents an error condition where the subject is not able to complete @@ -25,6 +28,29 @@ var ( // ErrRoleNotFound represents an error when no matching role was found on resource ErrRoleNotFound = errors.New("role not found") + // ErrResourceNotFound represents an error when no matching resource was found + ErrResourceNotFound = errors.New("resource not found") + + // ErrRoleBindingNotFound represents an error when no matching role binding was found + ErrRoleBindingNotFound = errors.New("role binding not found") + // ErrRoleHasTooManyResources represents an error which a role has too many resources ErrRoleHasTooManyResources = errors.New("role has too many resources") + + // ErrInvalidArgument represents an error when there is an invalid argument passed to a function + ErrInvalidArgument = errors.New("invalid argument") + + // ErrRoleV2ResourceNotDefined is returned when a role v2 resource is not defined + // in the policy + ErrRoleV2ResourceNotDefined = errors.New("role v2 resource not defined") + + // ErrRoleAlreadyExists represents an error when a role already exists + ErrRoleAlreadyExists = fmt.Errorf("%w: role already exists", ErrInvalidArgument) + + // ErrInvalidRoleBindingSubjectType represents an error when a role binding subject type is invalid + ErrInvalidRoleBindingSubjectType = fmt.Errorf("%w: invalid role binding subject type", ErrInvalidArgument) + + // ErrResourceDoesNotSupportRoleBindingV2 represents an error when a role binding + // request attempts to use a resource that does not support role binding v2 + ErrResourceDoesNotSupportRoleBindingV2 = fmt.Errorf("%w: resource does not support role binding v2", ErrInvalidArgument) ) diff --git a/internal/query/mock/mock.go b/internal/query/mock/mock.go index dc68b901..d64da981 100644 --- a/internal/query/mock/mock.go +++ b/internal/query/mock/mock.go @@ -12,9 +12,7 @@ import ( "go.infratographer.com/x/gidx" ) -var ( - errorInvalidNamespace = errors.New("invalid namespace") -) +var errorInvalidNamespace = errors.New("invalid namespace") var _ query.Engine = &Engine{} @@ -26,28 +24,28 @@ type Engine struct { } // AssignSubjectRole does nothing but satisfies the Engine interface. -func (e *Engine) AssignSubjectRole(ctx context.Context, subject types.Resource, role types.Role) error { +func (e *Engine) AssignSubjectRole(context.Context, types.Resource, types.Role) error { args := e.Called() return args.Error(0) } // UnassignSubjectRole does nothing but satisfies the Engine interface. -func (e *Engine) UnassignSubjectRole(ctx context.Context, subject types.Resource, role types.Role) error { +func (e *Engine) UnassignSubjectRole(context.Context, types.Resource, types.Role) error { args := e.Called() return args.Error(0) } // CreateRelationships does nothing but satisfies the Engine interface. -func (e *Engine) CreateRelationships(ctx context.Context, rels []types.Relationship) error { +func (e *Engine) CreateRelationships(context.Context, []types.Relationship) error { args := e.Called() return args.Error(0) } // CreateRole creates a Role object and does not persist it anywhere. -func (e *Engine) CreateRole(ctx context.Context, actor, res types.Resource, name string, actions []string) (types.Role, error) { +func (e *Engine) CreateRole(context.Context, types.Resource, types.Resource, string, []string) (types.Role, error) { args := e.Called() retRole := args.Get(0).(types.Role) @@ -55,8 +53,19 @@ func (e *Engine) CreateRole(ctx context.Context, actor, res types.Resource, name return retRole, args.Error(1) } +// CreateRoleV2 creates a v2 role object +// TODO: Implement this +func (e *Engine) CreateRoleV2(context.Context, types.Resource, types.Resource, string, []string) (types.Role, error) { + return types.Role{}, nil +} + +// ListRolesV2 list roles +func (e *Engine) ListRolesV2(context.Context, types.Resource) ([]types.Role, error) { + return nil, nil +} + // UpdateRole returns the provided mock results. -func (e *Engine) UpdateRole(ctx context.Context, actor, roleResource types.Resource, newName string, newActions []string) (types.Role, error) { +func (e *Engine) UpdateRole(context.Context, types.Resource, types.Resource, string, []string) (types.Role, error) { args := e.Called() retRole := args.Get(0).(types.Role) @@ -64,8 +73,13 @@ func (e *Engine) UpdateRole(ctx context.Context, actor, roleResource types.Resou return retRole, args.Error(1) } +// UpdateRoleV2 returns nothing but satisfies the Engine interface. +func (e *Engine) UpdateRoleV2(context.Context, types.Resource, types.Resource, string, []string) (types.Role, error) { + return types.Role{}, nil +} + // GetRole returns nothing but satisfies the Engine interface. -func (e *Engine) GetRole(ctx context.Context, roleResource types.Resource) (types.Role, error) { +func (e *Engine) GetRole(context.Context, types.Resource) (types.Role, error) { args := e.Called() retRole := args.Get(0).(types.Role) @@ -73,8 +87,13 @@ func (e *Engine) GetRole(ctx context.Context, roleResource types.Resource) (type return retRole, args.Error(1) } +// GetRoleV2 returns nothing but satisfies the Engine interface. +func (e *Engine) GetRoleV2(context.Context, types.Resource) (types.Role, error) { + return types.Role{}, nil +} + // GetRoleResource returns nothing but satisfies the Engine interface. -func (e *Engine) GetRoleResource(ctx context.Context, roleResource types.Resource) (types.Resource, error) { +func (e *Engine) GetRoleResource(context.Context, types.Resource) (types.Resource, error) { args := e.Called() retResc := args.Get(0).(types.Resource) @@ -83,7 +102,7 @@ func (e *Engine) GetRoleResource(ctx context.Context, roleResource types.Resourc } // ListAssignments returns nothing but satisfies the Engine interface. -func (e *Engine) ListAssignments(ctx context.Context, role types.Role) ([]types.Resource, error) { +func (e *Engine) ListAssignments(context.Context, types.Role) ([]types.Resource, error) { args := e.Called() ret := args.Get(0).([]types.Resource) @@ -92,36 +111,41 @@ func (e *Engine) ListAssignments(ctx context.Context, role types.Role) ([]types. } // ListRelationshipsFrom returns nothing but satisfies the Engine interface. -func (e *Engine) ListRelationshipsFrom(ctx context.Context, resource types.Resource) ([]types.Relationship, error) { +func (e *Engine) ListRelationshipsFrom(context.Context, types.Resource) ([]types.Relationship, error) { return nil, nil } // ListRelationshipsTo returns nothing but satisfies the Engine interface. -func (e *Engine) ListRelationshipsTo(ctx context.Context, resource types.Resource) ([]types.Relationship, error) { +func (e *Engine) ListRelationshipsTo(context.Context, types.Resource) ([]types.Relationship, error) { return nil, nil } // ListRoles returns nothing but satisfies the Engine interface. -func (e *Engine) ListRoles(ctx context.Context, resource types.Resource) ([]types.Role, error) { +func (e *Engine) ListRoles(context.Context, types.Resource) ([]types.Role, error) { return nil, nil } // DeleteRelationships does nothing but satisfies the Engine interface. -func (e *Engine) DeleteRelationships(ctx context.Context, relationships ...types.Relationship) error { +func (e *Engine) DeleteRelationships(context.Context, ...types.Relationship) error { args := e.Called() return args.Error(0) } // DeleteRole does nothing but satisfies the Engine interface. -func (e *Engine) DeleteRole(ctx context.Context, roleResource types.Resource) error { +func (e *Engine) DeleteRole(context.Context, types.Resource) error { args := e.Called() return args.Error(0) } +// DeleteRoleV2 does nothing but satisfies the Engine interface. +func (e *Engine) DeleteRoleV2(context.Context, types.Resource) error { + return nil +} + // DeleteResourceRelationships does nothing but satisfies the Engine interface. -func (e *Engine) DeleteResourceRelationships(ctx context.Context, resource types.Resource) error { +func (e *Engine) DeleteResourceRelationships(context.Context, types.Resource) error { args := e.Called() return args.Error(0) @@ -173,8 +197,38 @@ func (e *Engine) GetResourceType(name string) *types.ResourceType { } // SubjectHasPermission returns nil to satisfy the Engine interface. -func (e *Engine) SubjectHasPermission(ctx context.Context, subject types.Resource, action string, resource types.Resource) error { +func (e *Engine) SubjectHasPermission(context.Context, types.Resource, string, types.Resource) error { e.Called() return nil } + +// CreateRoleBinding returns nothing but satisfies the Engine interface. +func (e *Engine) CreateRoleBinding(context.Context, types.Resource, types.Resource, []types.RoleBindingSubject) (types.RoleBinding, error) { + return types.RoleBinding{}, nil +} + +// ListRoleBindings returns nothing but satisfies the Engine interface. +func (e *Engine) ListRoleBindings(context.Context, types.Resource, *types.Resource) ([]types.RoleBinding, error) { + return nil, nil +} + +// GetRoleBinding returns nothing but satisfies the Engine interface. +func (e *Engine) GetRoleBinding(context.Context, types.Resource) (types.RoleBinding, error) { + return types.RoleBinding{}, nil +} + +// DeleteRoleBinding returns nothing but satisfies the Engine interface. +func (e *Engine) DeleteRoleBinding(context.Context, types.Resource, types.Resource) error { + return nil +} + +// UpdateRoleBinding returns nothing but satisfies the Engine interface. +func (e *Engine) UpdateRoleBinding(context.Context, types.Resource, []types.RoleBindingSubject) (types.RoleBinding, error) { + return types.RoleBinding{}, nil +} + +// AllActions returns nothing but satisfies the Engine interface. +func (e *Engine) AllActions() []string { + return nil +} diff --git a/internal/query/relations.go b/internal/query/relations.go index c4be30a9..ddbfbc7b 100644 --- a/internal/query/relations.go +++ b/internal/query/relations.go @@ -313,8 +313,6 @@ func (e *engine) CreateRole(ctx context.Context, actor, res types.Resource, role return types.Role{}, err } - roleName = strings.TrimSpace(roleName) - role := newRole(roleName, actions) roleRels := e.roleRelationships(role, res) @@ -362,34 +360,34 @@ func (e *engine) CreateRole(ctx context.Context, actor, res types.Resource, role return role, nil } -// actionsDiff determines which actions needs to be added and removed. -// If no new actions are provided it is assumed no changes are requested. -func actionsDiff(oldActions, newActions []string) ([]string, []string) { - if len(newActions) == 0 { +// diff determines which entities needs to be added and removed. +// If no new entity are provided it is assumed no changes are requested. +func diff(current, incoming []string) ([]string, []string) { + if len(incoming) == 0 { return nil, nil } - old := make(map[string]struct{}, len(oldActions)) - new := make(map[string]struct{}, len(newActions)) //nolint:revive // new makes sense in this context + curr := make(map[string]struct{}, len(current)) + in := make(map[string]struct{}, len(incoming)) var add, rem []string - for _, action := range oldActions { - old[action] = struct{}{} + for _, entity := range current { + curr[entity] = struct{}{} } - for _, action := range newActions { - new[action] = struct{}{} + for _, action := range incoming { + in[action] = struct{}{} // If the new action is not in the old actions, then we need to add the action. - if _, ok := old[action]; !ok { + if _, ok := curr[action]; !ok { add = append(add, action) } } - for _, action := range oldActions { + for _, action := range current { // If the old action is not in the new actions, then we need to remove it. - if _, ok := new[action]; !ok { + if _, ok := in[action]; !ok { rem = append(rem, action) } } @@ -443,13 +441,11 @@ func (e *engine) UpdateRole(ctx context.Context, actor, roleResource types.Resou return types.Role{}, err } - newName = strings.TrimSpace(newName) - if newName == "" { newName = role.Name } - addActions, remActions := actionsDiff(role.Actions, newActions) + addActions, remActions := diff(role.Actions, newActions) // If no changes, return existing role with no changes. if newName == role.Name && len(addActions) == 0 && len(remActions) == 0 { @@ -1220,3 +1216,18 @@ func (e *engine) GetResourceType(name string) *types.ResourceType { return &rType } + +// NewResourceFromIDString creates a new resource from a string. +func (e *engine) NewResourceFromIDString(id string) (types.Resource, error) { + subjID, err := gidx.Parse(id) + if err != nil { + return types.Resource{}, err + } + + subject, err := e.NewResourceFromID(subjID) + if err != nil { + return types.Resource{}, err + } + + return subject, nil +} diff --git a/internal/query/relations_test.go b/internal/query/relations_test.go index 75811dfc..8f64a5dd 100644 --- a/internal/query/relations_test.go +++ b/internal/query/relations_test.go @@ -21,7 +21,7 @@ import ( "go.infratographer.com/permissions-api/internal/types" ) -func testEngine(ctx context.Context, t *testing.T, namespace string) *engine { +func testEngine(ctx context.Context, t *testing.T, namespace string, policy iapl.Policy) *engine { config := spicedbx.Config{ Endpoint: "spicedb:50051", Key: "infradev", @@ -33,8 +33,6 @@ func testEngine(ctx context.Context, t *testing.T, namespace string) *engine { store, cleanStore := teststore.NewTestStorage(t) - policy := testPolicy() - schema, err := spicedbx.GenerateSchema(namespace, policy.Schema()) require.NoError(t, err) @@ -107,7 +105,7 @@ func cleanDB(ctx context.Context, t *testing.T, client *authzed.Client, namespac func TestCreateRoles(t *testing.T) { namespace := "testroles" ctx := context.Background() - e := testEngine(ctx, t, namespace) + e := testEngine(ctx, t, namespace, testPolicy()) testCases := []testingx.TestCase[[]string, []types.Role]{ { @@ -167,7 +165,7 @@ func TestCreateRoles(t *testing.T) { func TestGetRoles(t *testing.T) { namespace := "testroles" ctx := context.Background() - e := testEngine(ctx, t, namespace) + e := testEngine(ctx, t, namespace, testPolicy()) tenID, err := gidx.NewID("tnntten") require.NoError(t, err) tenRes, err := e.NewResourceFromID(tenID) @@ -222,7 +220,7 @@ func TestGetRoles(t *testing.T) { func TestRoleUpdate(t *testing.T) { namespace := "testroles" ctx := context.Background() - e := testEngine(ctx, t, namespace) + e := testEngine(ctx, t, namespace, testPolicy()) tenID, err := gidx.NewID("tnntten") require.NoError(t, err) @@ -292,7 +290,7 @@ func TestRoleUpdate(t *testing.T) { func TestListRoles(t *testing.T) { namespace := "testroles" ctx := context.Background() - e := testEngine(ctx, t, namespace) + e := testEngine(ctx, t, namespace, testPolicy()) actorRes, err := e.NewResourceFromID(gidx.MustNewID("idntusr")) require.NoError(t, err) @@ -377,7 +375,7 @@ func TestListRoles(t *testing.T) { func TestRoleDelete(t *testing.T) { namespace := "testroles" ctx := context.Background() - e := testEngine(ctx, t, namespace) + e := testEngine(ctx, t, namespace, testPolicy()) tenID, err := gidx.NewID("tnntten") require.NoError(t, err) @@ -439,7 +437,7 @@ func TestRoleDelete(t *testing.T) { func TestAssignments(t *testing.T) { namespace := "testassignments" ctx := context.Background() - e := testEngine(ctx, t, namespace) + e := testEngine(ctx, t, namespace, testPolicy()) tenID, err := gidx.NewID("tnntten") require.NoError(t, err) @@ -499,7 +497,7 @@ func TestAssignments(t *testing.T) { func TestUnassignments(t *testing.T) { namespace := "testassignments" ctx := context.Background() - e := testEngine(ctx, t, namespace) + e := testEngine(ctx, t, namespace, testPolicy()) tenID, err := gidx.NewID("tnntten") require.NoError(t, err) @@ -581,7 +579,7 @@ func TestUnassignments(t *testing.T) { func TestRelationships(t *testing.T) { namespace := "testrelationships" ctx := context.Background() - e := testEngine(ctx, t, namespace) + e := testEngine(ctx, t, namespace, testPolicy()) parentID, err := gidx.NewID("tnntten") require.NoError(t, err) @@ -672,7 +670,7 @@ func TestRelationships(t *testing.T) { func TestRelationshipDelete(t *testing.T) { namespace := "testrelationships" ctx := context.Background() - e := testEngine(ctx, t, namespace) + e := testEngine(ctx, t, namespace, testPolicy()) parentID, err := gidx.NewID("tnntten") require.NoError(t, err) @@ -744,7 +742,7 @@ func TestRelationshipDelete(t *testing.T) { func TestSubjectActions(t *testing.T) { namespace := "infratestactions" ctx := context.Background() - e := testEngine(ctx, t, namespace) + e := testEngine(ctx, t, namespace, testPolicy()) parentID, err := gidx.NewID("tnntten") require.NoError(t, err) diff --git a/internal/query/roles.go b/internal/query/roles.go index 5cddb45c..1772b34a 100644 --- a/internal/query/roles.go +++ b/internal/query/roles.go @@ -20,3 +20,18 @@ func newRole(name string, actions []string) types.Role { Actions: actions, } } + +func newRoleWithPrefix(prefix string, name string, actions []string) (types.Role, error) { + id, err := gidx.NewID(prefix) + if err != nil { + return types.Role{}, err + } + + r := types.Role{ + ID: id, + Name: name, + Actions: actions, + } + + return r, nil +} diff --git a/internal/query/roles_v2.go b/internal/query/roles_v2.go new file mode 100644 index 00000000..e5e08aa8 --- /dev/null +++ b/internal/query/roles_v2.go @@ -0,0 +1,615 @@ +package query + +import ( + "context" + "errors" + "fmt" + "io" + + pb "github.com/authzed/authzed-go/proto/authzed/api/v1" + "go.infratographer.com/x/gidx" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/codes" + "go.opentelemetry.io/otel/trace" + + "go.infratographer.com/permissions-api/internal/iapl" + "go.infratographer.com/permissions-api/internal/types" +) + +// V2 Role and Role Bindings + +func (e *engine) namespaced(name string) string { + return e.namespace + "/" + name +} + +func (e *engine) CreateRoleV2(ctx context.Context, actor, owner types.Resource, roleName string, actions []string) (types.Role, error) { + ctx, span := e.tracer.Start(ctx, "engine.CreateRoleV2") + + defer span.End() + + role, err := newRoleWithPrefix(e.schemaTypeMap[e.rbac.RoleResource.Name].IDPrefix, roleName, actions) + if err != nil { + return types.Role{}, err + } + + roleRels, err := e.roleV2Relationships(role) + if err != nil { + return types.Role{}, err + } + + ownerRels, err := e.roleV2OwnerRelationship(role, owner) + if err != nil { + return types.Role{}, err + } + + roleRels = append(roleRels, ownerRels...) + + dbCtx, err := e.store.BeginContext(ctx) + if err != nil { + return types.Role{}, nil + } + + dbRole, err := e.store.CreateRole(dbCtx, actor.ID, role.ID, roleName, owner.ID) + if err != nil { + return types.Role{}, err + } + + request := &pb.WriteRelationshipsRequest{Updates: roleRels} + + if _, err := e.client.WriteRelationships(ctx, request); err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) + + return types.Role{}, err + } + + if err = e.store.CommitContext(dbCtx); err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) + + // No rollback of spicedb relations are done here. + // This does result in dangling unused entries in spicedb, + // however there are no assignments to these newly created + // and now discarded roles and so they won't be used. + + return types.Role{}, err + } + + role.CreatedBy = dbRole.CreatedBy + role.UpdatedBy = dbRole.UpdatedBy + role.ResourceID = dbRole.ResourceID + role.CreatedAt = dbRole.CreatedAt + role.UpdatedAt = dbRole.UpdatedAt + + return role, nil +} + +func (e *engine) ListRolesV2(ctx context.Context, owner types.Resource) ([]types.Role, error) { + ctx, span := e.tracer.Start( + ctx, + "engine.ListRolesV2", + trace.WithAttributes( + attribute.Stringer( + "owner", + owner.ID, + ), + ), + ) + defer span.End() + + if _, ok := e.rbac.RoleOwnersSet()[owner.Type]; !ok { + err := fmt.Errorf("%w: %s is not a valid role owner", ErrInvalidType, owner.Type) + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return nil, err + } + + lookupClient, err := e.client.LookupSubjects(ctx, &pb.LookupSubjectsRequest{ + Consistency: &pb.Consistency{ + Requirement: &pb.Consistency_FullyConsistent{ + FullyConsistent: true, + }, + }, + Resource: resourceToSpiceDBRef(e.namespace, owner), + Permission: iapl.AvailableRolesList, + SubjectObjectType: e.namespaced(e.rbac.RoleResource.Name), + }) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return nil, err + } + + roleIDs := []gidx.PrefixedID{} + + for { + lookup, err := lookupClient.Recv() + if err != nil { + if !errors.Is(err, io.EOF) { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + } + + break + } + + id, err := gidx.Parse(lookup.Subject.SubjectObjectId) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + continue + } + + roleIDs = append(roleIDs, id) + } + + storageRoles, err := e.store.BatchGetRoleByID(ctx, roleIDs) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return nil, err + } + + roles := make([]types.Role, len(storageRoles)) + + for i, r := range storageRoles { + roles[i] = types.Role{ + Name: r.Name, + ID: r.ID, + } + } + + return roles, nil +} + +func (e *engine) GetRoleV2(ctx context.Context, role types.Resource) (types.Role, error) { + ctx, span := e.tracer.Start( + ctx, + "engine.GetRoleV2", + trace.WithAttributes(attribute.Stringer("permissions.role_id", role.ID)), + ) + defer span.End() + + // check if the role is a valid v2 role + if role.Type != e.rbac.RoleResource.Name { + err := fmt.Errorf("%w: %s is not a valid v2 Role", ErrInvalidType, role.Type) + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return types.Role{}, err + } + + // 1. Get role actions from spice DB + + actions, err := e.listRoleV2Actions(ctx, types.Role{ID: role.ID}) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return types.Role{}, err + } + + // 2. Get role info (name, created_by, etc.) from permissions API DB + dbrole, err := e.store.GetRoleByID(ctx, role.ID) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return types.Role{}, err + } + + resp := types.Role{ + ID: dbrole.ID, + Name: dbrole.Name, + Actions: actions, + + ResourceID: dbrole.ResourceID, + CreatedBy: dbrole.CreatedBy, + UpdatedBy: dbrole.UpdatedBy, + CreatedAt: dbrole.CreatedAt, + UpdatedAt: dbrole.UpdatedAt, + } + + return resp, nil +} + +func (e *engine) UpdateRoleV2(ctx context.Context, actor, roleResource types.Resource, newName string, newActions []string) (types.Role, error) { + ctx, span := e.tracer.Start(ctx, "engine.UpdateRoleV2") + defer span.End() + + dbCtx, err := e.store.BeginContext(ctx) + if err != nil { + return types.Role{}, err + } + + err = e.store.LockRoleForUpdate(dbCtx, roleResource.ID) + if err != nil { + sErr := fmt.Errorf("failed to lock role: %s: %w", roleResource.ID, err) + + span.RecordError(sErr) + span.SetStatus(codes.Error, sErr.Error()) + + logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) + + return types.Role{}, err + } + + role, err := e.GetRoleV2(dbCtx, roleResource) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) + + return types.Role{}, err + } + + if newName == "" { + newName = role.Name + } + + addActions, rmActions := diff(role.Actions, newActions) + + // If no changes, return existing role + if newName == role.Name && len(addActions) == 0 && len(rmActions) == 0 { + if err = e.store.CommitContext(dbCtx); err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return types.Role{}, err + } + + return role, nil + } + + // 1. update role in permissions-api DB + dbRole, err := e.store.UpdateRole(dbCtx, actor.ID, role.ID, newName) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) + + return types.Role{}, err + } + + // 2. update permissions relationships in SpiceDB + updates := []*pb.RelationshipUpdate{} + roleRef := resourceToSpiceDBRef(e.namespace, roleResource) + + // 2.a remove old actions + for _, action := range rmActions { + updates = append( + updates, + e.createRoleV2RelationshipUpdatesForAction( + action, roleRef, + pb.RelationshipUpdate_OPERATION_DELETE, + )..., + ) + } + + // 2.b add new actions + for _, action := range addActions { + updates = append( + updates, + e.createRoleV2RelationshipUpdatesForAction( + action, roleRef, + pb.RelationshipUpdate_OPERATION_TOUCH, + )..., + ) + } + + // 2.c write updates to SpiceDB + request := &pb.WriteRelationshipsRequest{Updates: updates} + + if _, err := e.client.WriteRelationships(ctx, request); err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) + + return types.Role{}, err + } + + if err = e.store.CommitContext(dbCtx); err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) + + // At this point, SpiceDB changes have already been applied. + // Attempting to rollback could result in failures that could result in the same situation. + // + // TODO: add SpiceDB rollback logic along with rollback failure scenarios. + + return types.Role{}, err + } + + role.Name = dbRole.Name + role.CreatedBy = dbRole.CreatedBy + role.UpdatedBy = dbRole.UpdatedBy + role.ResourceID = dbRole.ResourceID + role.CreatedAt = dbRole.CreatedAt + role.UpdatedAt = dbRole.UpdatedAt + role.Actions = newActions + + return role, nil +} + +func (e *engine) DeleteRoleV2(ctx context.Context, roleResource types.Resource) error { + ctx, span := e.tracer.Start(ctx, "engine.DeleteRoleV2") + defer span.End() + + dbCtx, err := e.store.BeginContext(ctx) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return err + } + + err = e.store.LockRoleForUpdate(dbCtx, roleResource.ID) + if err != nil { + sErr := fmt.Errorf("failed to lock role: %s: %w", roleResource.ID, err) + + span.RecordError(sErr) + span.SetStatus(codes.Error, sErr.Error()) + + logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) + + return err + } + + dbRole, err := e.store.GetRoleByID(dbCtx, roleResource.ID) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return err + } + + roleOwner, err := e.NewResourceFromID(dbRole.ResourceID) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return err + } + + // 1. delete role from permission-api DB + if _, err = e.store.DeleteRole(dbCtx, roleResource.ID); err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) + + return err + } + + // 2. delete role relationships from spice db + errs := []error{} + + // 2.a remove all relationships from this role + + delRoleRelationshipReq := &pb.DeleteRelationshipsRequest{ + RelationshipFilter: &pb.RelationshipFilter{ + ResourceType: e.namespaced(e.rbac.RoleResource.Name), + OptionalResourceId: roleResource.ID.String(), + }, + } + + if _, err := e.client.DeleteRelationships(ctx, delRoleRelationshipReq); err != nil { + errs = append(errs, err) + } + + // 2.b remove all relationships to this role from its owner + + ownerRelReq := &pb.DeleteRelationshipsRequest{ + RelationshipFilter: &pb.RelationshipFilter{ + ResourceType: e.namespaced(roleOwner.Type), + OptionalSubjectFilter: &pb.SubjectFilter{ + SubjectType: e.namespaced(e.rbac.RoleResource.Name), + OptionalSubjectId: roleResource.ID.String(), + }, + }, + } + + if _, err := e.client.DeleteRelationships(ctx, ownerRelReq); err != nil { + errs = append(errs, err) + } + + // 2.c TODO remove all role relationships in role bindings associated with this role + + for _, err := range errs { + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) + + return err + } + } + + if err = e.store.CommitContext(dbCtx); err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) + + // At this point, spicedb changes have already been applied. + // Attempting to rollback could result in failures that could result in the same situation. + // + // In this particular case, in the absence of spiceDB rollback, the expected + // behavior in this case would be that the role only exists in the permissions-api + // DB and not in spiceDB. This would result in the role being "orphaned" + // (lack of ownership in spiceDB will prevent this role from being accessed or bind + // by any user), and would need to be cleaned up manually. + // + // TODO: add spicedb rollback logic along with rollback failure scenarios. + + return err + } + + return nil +} + +// roleV2OwnerRelationship creates a relationships between a V2 role and its owner. +func (e *engine) roleV2OwnerRelationship(role types.Role, owner types.Resource) ([]*pb.RelationshipUpdate, error) { + roleResource, err := e.NewResourceFromID(role.ID) + if err != nil { + return nil, err + } + + roleResourceType := e.GetResourceType(e.rbac.RoleResource.Name) + if roleResourceType == nil { + return nil, nil + } + + roleRef := resourceToSpiceDBRef(e.namespace, roleResource) + ownerRef := resourceToSpiceDBRef(e.namespace, owner) + + // e.g., rolev2:super-admin#owner@tenant:tnntten-root + ownerRel := &pb.RelationshipUpdate{ + Operation: pb.RelationshipUpdate_OPERATION_TOUCH, + Relationship: &pb.Relationship{ + Resource: roleRef, + Relation: iapl.RoleOwnerRelation, + Subject: &pb.SubjectReference{ + Object: ownerRef, + }, + }, + } + + // e.g., tenant:tnntten-root#member_role@rolev:super-admin + memberRel := &pb.RelationshipUpdate{ + Operation: pb.RelationshipUpdate_OPERATION_TOUCH, + Relationship: &pb.Relationship{ + Resource: ownerRef, + Relation: iapl.RoleOwnerMemberRoleRelation, + Subject: &pb.SubjectReference{ + Object: roleRef, + }, + }, + } + + return []*pb.RelationshipUpdate{ownerRel, memberRel}, nil +} + +// createRoleV2RelationshipUpdatesForAction creates permission relationship lines in role +// i.e., role:#_rel@/:* +func (e *engine) createRoleV2RelationshipUpdatesForAction( + action string, + roleRef *pb.ObjectReference, + op pb.RelationshipUpdate_Operation, +) []*pb.RelationshipUpdate { + rels := make([]*pb.RelationshipUpdate, len(e.rbac.RoleSubjectTypes)) + + for i, subjType := range e.rbac.RoleSubjectTypes { + rels[i] = &pb.RelationshipUpdate{ + Operation: op, + Relationship: &pb.Relationship{ + Resource: roleRef, + Relation: actionToRelation(action), + Subject: &pb.SubjectReference{ + Object: &pb.ObjectReference{ + ObjectType: e.namespaced(subjType), + ObjectId: "*", + }, + }, + }, + } + } + + return rels +} + +// roleV2Relationships creates relationships between a V2 role and its permissions. +func (e *engine) roleV2Relationships(role types.Role) ([]*pb.RelationshipUpdate, error) { + var rels []*pb.RelationshipUpdate + + roleResource, err := e.NewResourceFromID(role.ID) + if err != nil { + return nil, err + } + + roleResourceType := e.GetResourceType(e.rbac.RoleResource.Name) + if roleResourceType == nil { + return rels, ErrRoleV2ResourceNotDefined + } + + roleRef := resourceToSpiceDBRef(e.namespace, roleResource) + + for _, action := range role.Actions { + rels = append( + rels, + e.createRoleV2RelationshipUpdatesForAction( + action, roleRef, + pb.RelationshipUpdate_OPERATION_TOUCH, + )..., + ) + } + + return rels, nil +} + +func (e *engine) listRoleV2Actions(ctx context.Context, role types.Role) ([]string, error) { + if len(e.rbac.RoleSubjectTypes) == 0 { + return nil, nil + } + + // there could be multiple subject types for a permission, + // e.g. + // infratographer/rolev2:lb_viewer#loadbalancer_get_rel@infratographer/user:* + // infratographer/rolev2:lb_viewer#loadbalancer_get_rel@infratographer/client:* + // here we only need one of them since the action is the only thing we care + // about + permRelationshipSubjType := e.namespaced(e.rbac.RoleSubjectTypes[0]) + + rid := role.ID.String() + filter := &pb.RelationshipFilter{ + ResourceType: e.namespaced(e.rbac.RoleResource.Name), + OptionalResourceId: rid, + OptionalSubjectFilter: &pb.SubjectFilter{ + SubjectType: permRelationshipSubjType, + OptionalSubjectId: "*", + }, + } + + relationships, err := e.readRelationships(ctx, filter) + if err != nil { + return nil, err + } + + actions := make([]string, len(relationships)) + + for i, rel := range relationships { + actions[i] = relationToAction(rel.Relation) + } + + return actions, nil +} + +// AllActions list all available actions for a role +func (e *engine) AllActions() []string { + rbv2, ok := e.schemaTypeMap[e.rbac.RoleBindingResource.Name] + if !ok { + return nil + } + + actions := make([]string, len(rbv2.Actions)) + + for i, action := range rbv2.Actions { + actions[i] = action.Name + } + + return actions +} diff --git a/internal/query/roles_v2_test.go b/internal/query/roles_v2_test.go new file mode 100644 index 00000000..2d57fa11 --- /dev/null +++ b/internal/query/roles_v2_test.go @@ -0,0 +1,401 @@ +package query + +import ( + "context" + "fmt" + "testing" + + pb "github.com/authzed/authzed-go/proto/authzed/api/v1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + + "go.infratographer.com/permissions-api/internal/iapl" + "go.infratographer.com/permissions-api/internal/storage" + "go.infratographer.com/permissions-api/internal/testingx" + "go.infratographer.com/permissions-api/internal/types" +) + +func rbacv2TestPolicy() iapl.Policy { + p := DefaultPolicyV2() + + if err := p.Validate(); err != nil { + panic(err) + } + + return p +} + +func rbacV2CreateParentRel(parent, child types.Resource, namespace string) []*pb.RelationshipUpdate { + return []*pb.RelationshipUpdate{ + { + Operation: pb.RelationshipUpdate_OPERATION_TOUCH, + Relationship: &pb.Relationship{ + Resource: resourceToSpiceDBRef(namespace, child), + Relation: "parent", + Subject: &pb.SubjectReference{ + Object: resourceToSpiceDBRef(namespace, parent), + }, + }, + }, + { + Operation: pb.RelationshipUpdate_OPERATION_TOUCH, + Relationship: &pb.Relationship{ + Resource: resourceToSpiceDBRef(namespace, child), + Relation: "parent", + Subject: &pb.SubjectReference{ + Object: resourceToSpiceDBRef(namespace, parent), + OptionalRelation: "parent", + }, + }, + }, + { + Operation: pb.RelationshipUpdate_OPERATION_TOUCH, + Relationship: &pb.Relationship{ + Resource: resourceToSpiceDBRef(namespace, parent), + Relation: "member", + Subject: &pb.SubjectReference{ + Object: resourceToSpiceDBRef(namespace, child), + OptionalRelation: "member", + }, + }, + }, + } +} + +func TestCreateRolesV2(t *testing.T) { + namespace := "testroles" + ctx := context.Background() + e := testEngine(ctx, t, namespace, rbacv2TestPolicy()) + + tenant, err := e.NewResourceFromIDString("tnntten-root") + require.NoError(t, err) + actor, err := e.NewResourceFromIDString("idntusr-actor") + require.NoError(t, err) + + // group is not a role owner in this policy + invalidOwner, err := e.NewResourceFromIDString("idntgrp-group") + require.NoError(t, err) + + type input struct { + name string + actions []string + owner types.Resource + } + + tc := []testingx.TestCase[input, types.Role]{ + { + Name: "InvalidActions", + Input: input{ + name: "role1", + actions: []string{"action1", "action2"}, + owner: tenant, + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[types.Role]) { + require.Error(t, res.Err) + }, + }, + { + Name: "InvalidOwner", + Input: input{ + name: "lb_viewer", + owner: invalidOwner, + actions: []string{ + "loadbalancer_list", + "loadbalancer_get", + }, + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[types.Role]) { + require.Error(t, res.Err) + assert.ErrorContains(t, res.Err, "not allowed on relation") + }, + }, + { + Name: "CreateSuccess", + Input: input{ + name: "lb_viewer", + owner: tenant, + actions: []string{ + "loadbalancer_list", + "loadbalancer_get", + }, + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[types.Role]) { + require.NoError(t, res.Err) + + role := res.Success + require.Equal(t, "lb_viewer", role.Name) + require.Len(t, role.Actions, 2) + }, + }, + } + + testFn := func(ctx context.Context, in input) testingx.TestResult[types.Role] { + r, err := e.CreateRoleV2(ctx, actor, in.owner, in.name, in.actions) + if err != nil { + return testingx.TestResult[types.Role]{Err: err} + } + + res, err := e.NewResourceFromID(r.ID) + if err != nil { + return testingx.TestResult[types.Role]{Err: err} + } + + role, err := e.GetRoleV2(ctx, res) + + return testingx.TestResult[types.Role]{Success: role, Err: err} + } + + testingx.RunTests(ctx, t, tc, testFn) +} + +func TestGetRoleV2(t *testing.T) { + namespace := "testroles" + ctx := context.Background() + e := testEngine(ctx, t, namespace, rbacv2TestPolicy()) + + tenant, err := e.NewResourceFromIDString("tnntten-root") + require.NoError(t, err) + actor, err := e.NewResourceFromIDString("idntusr-actor") + require.NoError(t, err) + + role, err := e.CreateRoleV2(ctx, actor, tenant, "lb_viewer", []string{"loadbalancer_list", "loadbalancer_get"}) + require.NoError(t, err) + + roleRes, err := e.NewResourceFromID(role.ID) + require.NoError(t, err) + + missingRes, err := e.NewResourceFromIDString("permrv2-notfound") + require.NoError(t, err) + + invalidInput, err := e.NewResourceFromIDString("idntgrp-group") + require.NoError(t, err) + + tc := []testingx.TestCase[types.Resource, types.Role]{ + { + Name: "GetRoleNotFound", + Input: missingRes, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[types.Role]) { + assert.ErrorIs(t, res.Err, storage.ErrNoRoleFound) + }, + }, + { + Name: "GetRoleInvalidInput", + Input: invalidInput, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[types.Role]) { + assert.ErrorIs(t, res.Err, ErrInvalidType) + }, + }, + { + Name: "GetRoleSuccess", + Input: roleRes, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[types.Role]) { + require.NoError(t, res.Err) + + resp := res.Success + + require.Equal(t, role.Name, resp.Name) + require.Len(t, resp.Actions, len(role.Actions)) + }, + }, + } + + testFn := func(ctx context.Context, in types.Resource) testingx.TestResult[types.Role] { + role, err := e.GetRoleV2(ctx, in) + if err != nil { + return testingx.TestResult[types.Role]{Err: err} + } + + return testingx.TestResult[types.Role]{Success: role, Err: err} + } + + testingx.RunTests(ctx, t, tc, testFn) +} + +func TestListRolesV2(t *testing.T) { + namespace := "testroles" + ctx := context.Background() + e := testEngine(ctx, t, namespace, rbacv2TestPolicy()) + + root, err := e.NewResourceFromIDString("tnntten-root") + require.NoError(t, err) + child, err := e.NewResourceFromIDString("tnntten-child") + require.NoError(t, err) + orphan, err := e.NewResourceFromIDString("tnntten-orphan") + require.NoError(t, err) + + _, err = e.client.WriteRelationships(ctx, &pb.WriteRelationshipsRequest{ + Updates: rbacV2CreateParentRel(root, child, namespace), + }) + require.NoError(t, err) + + actor, err := e.NewResourceFromIDString("idntusr-actor") + require.NoError(t, err) + + _, err = e.CreateRoleV2(ctx, actor, root, "lb_viewer", []string{"loadbalancer_list", "loadbalancer_get"}) + require.NoError(t, err) + + _, err = e.CreateRoleV2(ctx, actor, root, "lb_editor", []string{"loadbalancer_list", "loadbalancer_get", "loadbalancer_update"}) + require.NoError(t, err) + + _, err = e.CreateRoleV2(ctx, actor, child, "custom_role", []string{"loadbalancer_list"}) + require.NoError(t, err) + + invalidOwner, err := e.NewResourceFromIDString("idntgrp-group") + require.NoError(t, err) + + tc := []testingx.TestCase[types.Resource, []types.Role]{ + { + Name: "InvalidOwner", + Input: invalidOwner, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[[]types.Role]) { + assert.ErrorIs(t, res.Err, ErrInvalidType) + }, + }, + { + Name: "ListParentRoles", + Input: root, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[[]types.Role]) { + assert.NoError(t, res.Err) + assert.Len(t, res.Success, 2) + }, + }, + { + Name: "ListInheritedRoles", + Input: child, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[[]types.Role]) { + assert.NoError(t, res.Err) + assert.Len(t, res.Success, 3) + }, + }, + { + Name: "ListNoRoles", + Input: orphan, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[[]types.Role]) { + require.NoError(t, res.Err) + assert.Len(t, res.Success, 0) + }, + }, + } + + testFn := func(ctx context.Context, in types.Resource) testingx.TestResult[[]types.Role] { + roles, err := e.ListRolesV2(ctx, in) + if err != nil { + return testingx.TestResult[[]types.Role]{Err: err} + } + + return testingx.TestResult[[]types.Role]{Success: roles, Err: err} + } + + testingx.RunTests(ctx, t, tc, testFn) +} + +func TestUpdateRolesV2(t *testing.T) { + namespace := "testroles" + ctx := context.Background() + e := testEngine(ctx, t, namespace, rbacv2TestPolicy()) + + tenant, err := e.NewResourceFromIDString("tnntten-root") + require.NoError(t, err) + actor, err := e.NewResourceFromIDString("idntusr-actor") + require.NoError(t, err) + + role, err := e.CreateRoleV2(ctx, actor, tenant, "lb_viewer", []string{"loadbalancer_list", "loadbalancer_get"}) + require.NoError(t, err) + + roleRes, err := e.NewResourceFromID(role.ID) + require.NoError(t, err) + + notfoundRes, err := e.NewResourceFromIDString("permrv2-notfound") + require.NoError(t, err) + + type input struct { + name string + actions []string + role types.Resource + } + + tc := []testingx.TestCase[input, types.Role]{ + { + Name: "UpdateRoleNotFound", + Input: input{ + name: "lb_viewer", + actions: []string{"loadbalancer_list", "loadbalancer_get"}, + role: notfoundRes, + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[types.Role]) { + assert.ErrorIs(t, res.Err, storage.ErrNoRoleFound) + }, + Sync: true, + }, + { + Name: "UpdateRoleInvalidInput", + Input: input{ + name: "lb_viewer", + actions: []string{"loadbalancer_list", "loadbalancer_get"}, + role: actor, + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[types.Role]) { + require.Error(t, res.Err) + }, + Sync: true, + }, + { + Name: "UpdateRoleActionNotFound", + Input: input{ + name: "lb_viewer", + actions: []string{"notfound"}, + role: roleRes, + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[types.Role]) { + fmt.Printf("err: %v\n", res.Err) + assert.Equal(t, status.Code(res.Err), codes.FailedPrecondition) + assert.ErrorContains(t, res.Err, "not found") + }, + Sync: true, + }, + { + Name: "UpdateNoChange", + Input: input{ + actions: []string{"loadbalancer_list", "loadbalancer_get"}, + role: roleRes, + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[types.Role]) { + assert.NoError(t, res.Err) + assert.Equal(t, role.Name, res.Success.Name) + assert.Len(t, res.Success.Actions, len(role.Actions)) + }, + Sync: true, + }, + { + Name: "UpdateSuccess", + Input: input{ + name: "new_name", + actions: []string{"loadbalancer_get", "loadbalancer_update"}, + role: roleRes, + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[types.Role]) { + require.NoError(t, res.Err) + + assert.Equal(t, "new_name", res.Success.Name) + assert.Len(t, res.Success.Actions, 2) + assert.Contains(t, res.Success.Actions, "loadbalancer_update") + assert.Contains(t, res.Success.Actions, "loadbalancer_get") + }, + Sync: true, + }, + } + + testFn := func(ctx context.Context, in input) testingx.TestResult[types.Role] { + if _, err := e.UpdateRoleV2(ctx, actor, in.role, in.name, in.actions); err != nil { + return testingx.TestResult[types.Role]{Err: err} + } + + role, err := e.GetRoleV2(ctx, in.role) + + return testingx.TestResult[types.Role]{Success: role, Err: err} + } + + testingx.RunTests(ctx, t, tc, testFn) +} diff --git a/internal/query/service.go b/internal/query/service.go index 64a3b929..b430bf6b 100644 --- a/internal/query/service.go +++ b/internal/query/service.go @@ -39,6 +39,21 @@ type Engine interface { NewResourceFromID(id gidx.PrefixedID) (types.Resource, error) GetResourceType(name string) *types.ResourceType SubjectHasPermission(ctx context.Context, subject types.Resource, action string, resource types.Resource) error + + // v2 functions, add role bindings support + + // CreateRoleV2 creates a v2 role scoped to the given owner resource with the given actions. + CreateRoleV2(ctx context.Context, actor, owner types.Resource, roleName string, actions []string) (types.Role, error) + // ListRolesV2 returns all V2 roles owned by the given resource. + ListRolesV2(ctx context.Context, owner types.Resource) ([]types.Role, error) + // GetRoleV2 returns a V2 role + GetRoleV2(ctx context.Context, role types.Resource) (types.Role, error) + // UpdateRoleV2 updates a V2 role with the given name and actions. + UpdateRoleV2(ctx context.Context, actor, roleResource types.Resource, newName string, newActions []string) (types.Role, error) + // DeleteRoleV2 deletes a V2 role. + DeleteRoleV2(ctx context.Context, roleResource types.Resource) error + + AllActions() []string } type engine struct { @@ -55,6 +70,12 @@ type engine struct { schemaRoleables []types.ResourceType rbac iapl.RBAC + // rolebindingSubjectsMap maps the name of the role-binding subject to the target type + // and provide quick lookups for the role-binding subjects. + rolebindingSubjectsMap map[string]types.TargetType + // rbacV2ResourceTypes is a list of resource types that had rbac V2 enabled, + // role-binding only works with resource types that are in this list + rbacV2ResourceTypes []types.ResourceType } func (e *engine) cacheSchemaResources() { @@ -62,6 +83,8 @@ func (e *engine) cacheSchemaResources() { e.schemaTypeMap = make(map[string]types.ResourceType, len(e.schema)) e.schemaSubjectRelationMap = make(map[string]map[string][]string) e.schemaRoleables = []types.ResourceType{} + e.rolebindingSubjectsMap = make(map[string]types.TargetType, len(e.rbac.RoleBindingSubjects)) + e.rbacV2ResourceTypes = []types.ResourceType{} for _, res := range e.schema { e.schemaPrefixMap[res.IDPrefix] = res @@ -80,6 +103,14 @@ func (e *engine) cacheSchemaResources() { if resourceHasRoleBindings(res) { e.schemaRoleables = append(e.schemaRoleables, res) } + + if rb := resourceHasRoleBindingV2(res); rb != nil { + e.rbacV2ResourceTypes = append(e.rbacV2ResourceTypes, res) + } + } + + for _, subj := range e.rbac.RoleBindingSubjects { + e.rolebindingSubjectsMap[subj.Name] = subj } } @@ -95,6 +126,18 @@ func resourceHasRoleBindings(resType types.ResourceType) bool { return false } +func resourceHasRoleBindingV2(resType types.ResourceType) *types.ConditionRoleBindingV2 { + for _, action := range resType.Actions { + for _, cond := range action.Conditions { + if cond.RoleBindingV2 != nil { + return cond.RoleBindingV2 + } + } + } + + return nil +} + // NewEngine returns a new client for making permissions queries. func NewEngine(namespace string, client *authzed.Client, kv nats.KeyValue, store storage.Storage, options ...Option) (Engine, error) { tracer := otel.GetTracerProvider().Tracer("go.infratographer.com/permissions-api/internal/query") diff --git a/internal/query/zedtokens_test.go b/internal/query/zedtokens_test.go index 3fd1f23c..aa5c3dbb 100644 --- a/internal/query/zedtokens_test.go +++ b/internal/query/zedtokens_test.go @@ -15,7 +15,7 @@ import ( func TestConsistency(t *testing.T) { namespace := "testconsistency" ctx := context.Background() - e := testEngine(ctx, t, namespace) + e := testEngine(ctx, t, namespace, testPolicy()) tenantID, err := gidx.NewID("tnntten") require.NoError(t, err) diff --git a/internal/storage/roles.go b/internal/storage/roles.go index 870445ae..f65ffe16 100644 --- a/internal/storage/roles.go +++ b/internal/storage/roles.go @@ -19,6 +19,7 @@ type RoleService interface { UpdateRole(ctx context.Context, actorID, roleID gidx.PrefixedID, name string) (Role, error) DeleteRole(ctx context.Context, roleID gidx.PrefixedID) (Role, error) LockRoleForUpdate(ctx context.Context, roleID gidx.PrefixedID) error + BatchGetRoleByID(ctx context.Context, ids []gidx.PrefixedID) ([]Role, error) } // Role represents a role in the database. @@ -63,7 +64,6 @@ func (e *engine) GetRoleByID(ctx context.Context, id gidx.PrefixedID) (Role, err &role.CreatedAt, &role.UpdatedAt, ) - if err != nil { if errors.Is(err, sql.ErrNoRows) { return Role{}, fmt.Errorf("%w: %s", ErrNoRoleFound, id.String()) @@ -135,7 +135,6 @@ func (e *engine) GetResourceRoleByName(ctx context.Context, resourceID gidx.Pref &role.CreatedAt, &role.UpdatedAt, ) - if err != nil { if errors.Is(err, sql.ErrNoRows) { return Role{}, fmt.Errorf("%w: %s", ErrNoRoleFound, name) @@ -170,7 +169,6 @@ func (e *engine) ListResourceRoles(ctx context.Context, resourceID gidx.Prefixed `, resourceID.String(), ) - if err != nil { return nil, err } @@ -219,7 +217,6 @@ func (e *engine) CreateRole(ctx context.Context, actorID, roleID gidx.PrefixedID &role.CreatedAt, &role.UpdatedAt, ) - if err != nil { if pqIsRoleAlreadyExistsError(err) { return Role{}, fmt.Errorf("%w: %s", ErrRoleAlreadyExists, roleID.String()) @@ -261,7 +258,6 @@ func (e *engine) UpdateRole(ctx context.Context, actorID, roleID gidx.PrefixedID &role.CreatedAt, &role.UpdatedAt, ) - if err != nil { if errors.Is(err, sql.ErrNoRows) { return Role{}, fmt.Errorf("%w: %s", ErrNoRoleFound, roleID.String()) @@ -308,3 +304,53 @@ func (e *engine) DeleteRole(ctx context.Context, roleID gidx.PrefixedID) (Role, return role, nil } + +// BatchGetRoleByID retrieves multiple roles from the database by the provided prefixed IDs. +// If no roles are found an empty slice is returned. +func (e *engine) BatchGetRoleByID(ctx context.Context, ids []gidx.PrefixedID) ([]Role, error) { + db, err := getContextDBQuery(ctx, e) + if err != nil { + return nil, err + } + + inClause := "" + args := make([]any, len(ids)) + + for i, id := range ids { + fmtStr := "$%d" + + if i > 0 { + fmtStr = ", $%d" + } + + inClause += fmt.Sprintf(fmtStr, i+1) + args[i] = id.String() + } + + q := fmt.Sprintf(` + SELECT + id, name, resource_id, + created_by, updated_by, created_at, updated_at + FROM roles + WHERE id IN (%s) + `, inClause) + + rows, err := db.QueryContext(ctx, q, args...) + if err != nil { + return nil, err + } + + var roles []Role + + for rows.Next() { + var role Role + + if err := rows.Scan(&role.ID, &role.Name, &role.ResourceID, &role.CreatedBy, &role.UpdatedBy, &role.CreatedAt, &role.UpdatedAt); err != nil { + return nil, err + } + + roles = append(roles, role) + } + + return roles, nil +}