From 1f1f2e298f93806ea604e43013cf444ca2311151 Mon Sep 17 00:00:00 2001 From: Bailin He Date: Mon, 4 Mar 2024 21:43:44 +0000 Subject: [PATCH 01/10] update query pkg to support role-binding V2, add v2 APIs Signed-off-by: Bailin He --- cmd/createrole.go | 58 +- internal/api/rolebindings.go | 366 +++++++++ internal/api/router.go | 6 + internal/api/types.go | 38 + internal/query/rolebindings.go | 631 +++++++++++++++ internal/query/rolebindings_test.go | 734 +++++++++++++++++ internal/query/roles_v2.go | 5 +- internal/query/roles_v2_test.go | 107 ++- internal/query/service.go | 19 + openapi-v2.yaml | 1159 +++++++++++++++++++++++++++ 10 files changed, 3103 insertions(+), 20 deletions(-) create mode 100644 internal/api/rolebindings.go create mode 100644 internal/query/rolebindings.go create mode 100644 internal/query/rolebindings_test.go create mode 100644 openapi-v2.yaml diff --git a/cmd/createrole.go b/cmd/createrole.go index 30635741..f6a63a55 100644 --- a/cmd/createrole.go +++ b/cmd/createrole.go @@ -15,6 +15,7 @@ import ( "go.infratographer.com/permissions-api/internal/query" "go.infratographer.com/permissions-api/internal/spicedbx" "go.infratographer.com/permissions-api/internal/storage" + "go.infratographer.com/permissions-api/internal/types" ) const ( @@ -22,17 +23,16 @@ const ( createRoleFlagResource = "resource" createRoleFlagActions = "actions" createRoleFlagName = "name" + createRoleFlagIsV2 = "v2" ) -var ( - createRoleCmd = &cobra.Command{ - Use: "create-role", - Short: "create role in SpiceDB directly", - Run: func(cmd *cobra.Command, _ []string) { - createRole(cmd.Context(), globalCfg) - }, - } -) +var createRoleCmd = &cobra.Command{ + Use: "create-role", + Short: "create role in SpiceDB directly", + Run: func(cmd *cobra.Command, _ []string) { + createRole(cmd.Context(), globalCfg) + }, +} func init() { rootCmd.AddCommand(createRoleCmd) @@ -42,6 +42,7 @@ func init() { flags.StringSlice(createRoleFlagActions, []string{}, "actions to assign to created role") flags.String(createRoleFlagResource, "", "resource to bind to created role") flags.String(createRoleFlagName, "", "name of role to create") + flags.Bool(createRoleFlagIsV2, false, "create a v2 role") v := viper.GetViper() @@ -49,6 +50,7 @@ func init() { viperx.MustBindFlag(v, createRoleFlagActions, flags.Lookup(createRoleFlagActions)) viperx.MustBindFlag(v, createRoleFlagResource, flags.Lookup(createRoleFlagResource)) viperx.MustBindFlag(v, createRoleFlagName, flags.Lookup(createRoleFlagName)) + viperx.MustBindFlag(v, createRoleFlagIsV2, flags.Lookup(createRoleFlagIsV2)) } func createRole(ctx context.Context, cfg *config.AppConfig) { @@ -56,6 +58,7 @@ func createRole(ctx context.Context, cfg *config.AppConfig) { actions := viper.GetStringSlice(createRoleFlagActions) resourceIDStr := viper.GetString(createRoleFlagResource) name := viper.GetString(createRoleFlagName) + v2 := viper.GetBool(createRoleFlagIsV2) if subjectIDStr == "" || len(actions) == 0 || resourceIDStr == "" || name == "" { logger.Fatal("invalid config") @@ -125,14 +128,35 @@ func createRole(ctx context.Context, cfg *config.AppConfig) { logger.Fatalw("error creating subject resource", "error", err) } - role, err := engine.CreateRole(ctx, subjectResource, resource, name, actions) - if err != nil { - logger.Fatalw("error creating role", "error", err) - } + if v2 { + role, err := engine.CreateRoleV2(ctx, subjectResource, resource, name, actions) + if err != nil { + logger.Fatalw("error creating role", "error", err) + } - if err = engine.AssignSubjectRole(ctx, subjectResource, role); err != nil { - logger.Fatalw("error creating role", "error", err) - } + rbsubj := []types.RoleBindingSubject{{SubjectResource: subjectResource}} + + roleres, err := engine.NewResourceFromID(role.ID) + if err != nil { + logger.Fatalw("error creating role resource", "error", err) + } - logger.Infow("role successfully created", "role_id", role.ID) + rb, err := engine.CreateRoleBinding(ctx, resource, roleres, rbsubj) + if err != nil { + logger.Fatalw("error creating role binding", "error", err) + } + + logger.Infof("created role %s[%s] and role-binding %s", role.Name, role.ID, rb.ID) + } else { + role, err := engine.CreateRole(ctx, subjectResource, resource, name, actions) + if err != nil { + logger.Fatalw("error creating role", "error", err) + } + + if err = engine.AssignSubjectRole(ctx, subjectResource, role); err != nil { + logger.Fatalw("error creating role", "error", err) + } + + logger.Infow("role successfully created", "role_id", role.ID) + } } diff --git a/internal/api/rolebindings.go b/internal/api/rolebindings.go new file mode 100644 index 00000000..911990a3 --- /dev/null +++ b/internal/api/rolebindings.go @@ -0,0 +1,366 @@ +package api + +import ( + "fmt" + "net/http" + + "github.com/labstack/echo/v4" + "go.infratographer.com/x/gidx" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/trace" + + "go.infratographer.com/permissions-api/internal/iapl" + "go.infratographer.com/permissions-api/internal/types" +) + +func resourceToSubject(subjects []types.RoleBindingSubject) []roleBindingSubject { + resp := make([]roleBindingSubject, len(subjects)) + for i, subj := range subjects { + resp[i] = roleBindingSubject{ + ID: subj.SubjectResource.ID, + Type: subj.SubjectResource.Type, + Condition: nil, + } + } + + return resp +} + +func (r *Router) roleBindingCreate(c echo.Context) error { + resourceIDStr := c.Param("id") + + ctx, span := tracer.Start( + c.Request().Context(), "api.roleBindingCreate", + 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 body roleBindingRequest + + err = c.Bind(&body) + if err != nil { + return r.errorResponse(err.Error(), ErrParsingRequestBody) + } + + resource, err := r.engine.NewResourceFromID(resourceID) + if err != nil { + return r.errorResponse("error creating resource", err) + } + + subjectResource, err := r.currentSubject(c) + if err != nil { + return err + } + + // permissions on role binding actions, similar to roles v1, are granted on the resources + if err := r.checkActionWithResponse(ctx, subjectResource, string(iapl.RoleBindingActionCreate), resource); err != nil { + return err + } + + roleID, err := gidx.Parse(body.RoleID) + if err != nil { + return r.errorResponse("error parsing role ID", fmt.Errorf("%w: %s", ErrInvalidID, err.Error())) + } + + roleResource, err := r.engine.NewResourceFromID(roleID) + if err != nil { + return r.errorResponse("error creating role resource", err) + } + + subjects := make([]types.RoleBindingSubject, len(body.Subjects)) + + for i, s := range body.Subjects { + subj, err := r.engine.NewResourceFromID(s.ID) + if err != nil { + return r.errorResponse("error creating subject resource", err) + } + + subjects[i] = types.RoleBindingSubject{ + SubjectResource: subj, + Condition: nil, + } + } + + rb, err := r.engine.CreateRoleBinding(ctx, resource, roleResource, subjects) + if err != nil { + return r.errorResponse("error creating role-binding", err) + } + + return c.JSON( + http.StatusOK, + roleBindingResponse{ + ID: rb.ID, + Subjects: resourceToSubject(rb.Subjects), + Role: roleBindingResponseRole{ + ID: rb.Role.ID, + Name: rb.Role.Name, + }, + }, + ) +} + +func (r *Router) roleBindingsList(c echo.Context) error { + resourceIDStr := c.Param("id") + roleIDStr := c.QueryParam("role_id") + + ctx, span := tracer.Start( + c.Request().Context(), "api.roleBindingList", + 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())) + } + + resource, err := r.engine.NewResourceFromID(resourceID) + if err != nil { + return r.errorResponse("error creating resource", err) + } + + subjectResource, err := r.currentSubject(c) + if err != nil { + return err + } + + if err := r.checkActionWithResponse(ctx, subjectResource, string(iapl.RoleBindingActionList), resource); err != nil { + return err + } + + roleFilter := (*types.Resource)(nil) + + if roleIDStr != "" { + roleID, err := gidx.Parse(roleIDStr) + if err != nil { + return r.errorResponse("error parsing role ID", fmt.Errorf("%w: %s", ErrInvalidID, err.Error())) + } + + roleResource, err := r.engine.NewResourceFromID(roleID) + if err != nil { + return r.errorResponse("error creating role resource", err) + } + + roleFilter = &roleResource + } + + rbs, err := r.engine.ListRoleBindings(ctx, resource, roleFilter) + if err != nil { + return r.errorResponse("error listing role-binding", err) + } + + resp := listRoleBindingsResponse{ + Data: make([]roleBindingResponse, len(rbs)), + } + + for i, rb := range rbs { + resp.Data[i] = roleBindingResponse{ + ID: rb.ID, + Subjects: resourceToSubject(rb.Subjects), + Role: roleBindingResponseRole{ + ID: rb.Role.ID, + Name: rb.Role.Name, + }, + } + } + + return c.JSON(http.StatusOK, resp) +} + +func (r *Router) roleBindingsDelete(c echo.Context) error { + resID := c.Param("id") + rbID := c.Param("rb_id") + + ctx, span := tracer.Start( + c.Request().Context(), "api.roleBindingDelete", + trace.WithAttributes(attribute.String("id", rbID)), + ) + defer span.End() + + // resource + resourceID, err := gidx.Parse(resID) + if err != nil { + return r.errorResponse("error parsing resource ID", fmt.Errorf("%w: %s", ErrInvalidID, err.Error())) + } + + resource, err := r.engine.NewResourceFromID(resourceID) + if err != nil { + return r.errorResponse("error creating resource", err) + } + + // role-binding + rolebindingID, err := gidx.Parse(rbID) + if err != nil { + return r.errorResponse("error parsing resource ID", fmt.Errorf("%w: %s", ErrInvalidID, err.Error())) + } + + rbRes, err := r.engine.NewResourceFromID(rolebindingID) + if err != nil { + return r.errorResponse("error creating resource", err) + } + + actor, err := r.currentSubject(c) + if err != nil { + return err + } + + // permissions on role binding actions, similar to roles v1, are granted on the resources + if err := r.checkActionWithResponse(ctx, actor, string(iapl.RoleBindingActionDelete), resource); err != nil { + return err + } + + if err := r.engine.DeleteRoleBinding(ctx, rbRes, resource); err != nil { + return r.errorResponse("error updating role-binding", err) + } + + resp := deleteRoleBindingResponse{Success: true} + + return c.JSON(http.StatusOK, resp) +} + +func (r *Router) roleBindingGet(c echo.Context) error { + resID := c.Param("id") + rbID := c.Param("rb_id") + + ctx, span := tracer.Start( + c.Request().Context(), "api.roleBindingGet", + trace.WithAttributes(attribute.String("id", rbID)), + ) + defer span.End() + + // resource + resourceID, err := gidx.Parse(resID) + if err != nil { + return r.errorResponse("error parsing resource ID", fmt.Errorf("%w: %s", ErrInvalidID, err.Error())) + } + + resource, err := r.engine.NewResourceFromID(resourceID) + if err != nil { + return r.errorResponse("error creating resource", err) + } + + // role-binding + rolebindingID, err := gidx.Parse(rbID) + if err != nil { + return r.errorResponse("error parsing resource ID", fmt.Errorf("%w: %s", ErrInvalidID, err.Error())) + } + + rbRes, err := r.engine.NewResourceFromID(rolebindingID) + if err != nil { + return r.errorResponse("error creating resource", err) + } + + actor, err := r.currentSubject(c) + if err != nil { + return err + } + + // permissions on role binding actions, similar to roles v1, are granted on the resources + if err := r.checkActionWithResponse(ctx, actor, string(iapl.RoleBindingActionGet), resource); err != nil { + return err + } + + rb, err := r.engine.GetRoleBinding(ctx, rbRes) + if err != nil { + return r.errorResponse("error getting role-binding", err) + } + + return c.JSON( + http.StatusOK, + roleBindingResponse{ + ID: rb.ID, + Subjects: resourceToSubject(rb.Subjects), + Role: roleBindingResponseRole{ + ID: rb.Role.ID, + Name: rb.Role.Name, + }, + }, + ) +} + +func (r *Router) roleBindingUpdate(c echo.Context) error { + resID := c.Param("id") + rbID := c.Param("rb_id") + + ctx, span := tracer.Start( + c.Request().Context(), "api.roleBindingUpdate", + trace.WithAttributes(attribute.String("id", resID)), + trace.WithAttributes(attribute.String("rolebinding_id", rbID)), + ) + defer span.End() + + // resource + resourceID, err := gidx.Parse(resID) + if err != nil { + return r.errorResponse("error parsing resource ID", fmt.Errorf("%w: %s", ErrInvalidID, err.Error())) + } + + resource, err := r.engine.NewResourceFromID(resourceID) + if err != nil { + return r.errorResponse("error creating resource", err) + } + + // role-binding + rolebindingID, err := gidx.Parse(rbID) + if err != nil { + return r.errorResponse("error parsing resource ID", fmt.Errorf("%w: %s", ErrInvalidID, err.Error())) + } + + rbRes, err := r.engine.NewResourceFromID(rolebindingID) + if err != nil { + return r.errorResponse("error creating resource", err) + } + + actor, err := r.currentSubject(c) + if err != nil { + return err + } + + // permissions on role binding actions, similar to roles v1, are granted on the resources + if err := r.checkActionWithResponse(ctx, actor, string(iapl.RoleBindingActionUpdate), resource); err != nil { + return err + } + + body := &rolebindingUpdateRequest{} + + err = c.Bind(body) + if err != nil { + return r.errorResponse(err.Error(), ErrParsingRequestBody) + } + + subjects := make([]types.RoleBindingSubject, len(body.Subjects)) + + for i, s := range body.Subjects { + subj, err := r.engine.NewResourceFromID(s.ID) + if err != nil { + return r.errorResponse("error creating subject resource", err) + } + + subjects[i] = types.RoleBindingSubject{ + SubjectResource: subj, + Condition: nil, + } + } + + rb, err := r.engine.UpdateRoleBinding(ctx, rbRes, subjects) + if err != nil { + return r.errorResponse("error updating role-binding", err) + } + + return c.JSON( + http.StatusOK, + roleBindingResponse{ + ID: rb.ID, + Subjects: resourceToSubject(rb.Subjects), + Role: roleBindingResponseRole{ + ID: rb.Role.ID, + Name: rb.Role.Name, + }, + }, + ) +} diff --git a/internal/api/router.go b/internal/api/router.go index bc389f9c..39160255 100644 --- a/internal/api/router.go +++ b/internal/api/router.go @@ -86,6 +86,12 @@ func (r *Router) Routes(rg *echo.Group) { v2.PATCH("/roles/:role_id", r.roleV2Update) v2.DELETE("/roles/:id", r.roleV2Delete) + v2.GET("/resources/:id/role-bindings", r.roleBindingsList) + v2.GET("/resources/:id/role-bindings/:rb_id", r.roleBindingGet) + v2.POST("/resources/:id/role-bindings", r.roleBindingCreate) + v2.DELETE("/resources/:id/role-bindings/:rb_id", r.roleBindingsDelete) + v2.PATCH("/resources/:id/role-bindings/:rb_id", r.roleBindingUpdate) + v2.GET("/actions", r.listActions) } } diff --git a/internal/api/types.go b/internal/api/types.go index 48aa720c..380ffa7a 100644 --- a/internal/api/types.go +++ b/internal/api/types.go @@ -80,3 +80,41 @@ type listRolesV2Role struct { ID gidx.PrefixedID `json:"id"` Name string `json:"name"` } + +// RoleBindings + +type roleBindingResponseRole struct { + ID gidx.PrefixedID `json:"id"` + Name string `json:"name"` +} + +type roleBindingSubjectCondition struct{} + +type roleBindingSubject struct { + ID gidx.PrefixedID `json:"id" binding:"required"` + Type string `json:"type,omitempty"` + Condition *roleBindingSubjectCondition `json:"condition,omitempty"` +} + +type roleBindingRequest struct { + RoleID string `json:"role_id" binding:"required"` + Subjects []roleBindingSubject `json:"subjects" binding:"required"` +} + +type rolebindingUpdateRequest struct { + Subjects []roleBindingSubject `json:"subjects" binding:"required"` +} + +type roleBindingResponse struct { + ID gidx.PrefixedID `json:"id"` + Role roleBindingResponseRole `json:"role"` + Subjects []roleBindingSubject `json:"subjects"` +} + +type listRoleBindingsResponse struct { + Data []roleBindingResponse `json:"data"` +} + +type deleteRoleBindingResponse struct { + Success bool `json:"success"` +} diff --git a/internal/query/rolebindings.go b/internal/query/rolebindings.go new file mode 100644 index 00000000..debcc75e --- /dev/null +++ b/internal/query/rolebindings.go @@ -0,0 +1,631 @@ +package query + +import ( + "context" + "errors" + "fmt" + "sync" + + 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" +) + +func (e *engine) CreateRoleBinding(ctx context.Context, resource, roleResource types.Resource, subjects []types.RoleBindingSubject) (types.RoleBinding, error) { + ctx, span := e.tracer.Start( + ctx, "engine.CreateRoleBinding", + trace.WithAttributes( + attribute.Stringer("role_id", roleResource.ID), + attribute.Stringer("resource_id", resource.ID), + ), + ) + defer span.End() + + if err := e.isRoleBindable(ctx, roleResource, resource); err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return types.RoleBinding{}, err + } + + dbrole, err := e.store.GetRoleByID(ctx, roleResource.ID) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return types.RoleBinding{}, err + } + + role := types.Role{ + ID: roleResource.ID, + Name: dbrole.Name, + } + + rbResourceType, ok := e.schemaTypeMap[e.rbac.RoleBindingResource.Name] + if !ok { + return types.RoleBinding{}, fmt.Errorf( + "%w: invalid role-binding resource type: %s", + ErrInvalidType, e.rbac.RoleBindingResource.Name, + ) + } + + rb := newRoleBindingWithPrefix(rbResourceType.IDPrefix, role) + roleRel := e.rolebindingRoleRelationship(role.ID.String(), rb.ID.String()) + + grantRel, err := e.rolebindingGrantResourceRelationship(resource, rb.ID.String()) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return types.RoleBinding{}, err + } + + updates := []*pb.RelationshipUpdate{ + { + Operation: pb.RelationshipUpdate_OPERATION_TOUCH, + Relationship: roleRel, + }, + { + Operation: pb.RelationshipUpdate_OPERATION_TOUCH, + Relationship: grantRel, + }, + } + + subjUpdates := make([]*pb.RelationshipUpdate, len(subjects)) + rb.Subjects = make([]types.RoleBindingSubject, len(subjects)) + + for i, subj := range subjects { + rel, err := e.rolebindingSubjectRelationship(subj.SubjectResource, rb.ID.String()) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return types.RoleBinding{}, err + } + + rb.Subjects[i] = subj + subjUpdates[i] = &pb.RelationshipUpdate{ + Operation: pb.RelationshipUpdate_OPERATION_TOUCH, + Relationship: rel, + } + } + + if _, err := e.client.WriteRelationships(ctx, &pb.WriteRelationshipsRequest{ + Updates: append(updates, subjUpdates...), + }); err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return types.RoleBinding{}, err + } + + return rb, nil +} + +func (e *engine) DeleteRoleBinding(ctx context.Context, rb, res types.Resource) error { + ctx, span := e.tracer.Start( + ctx, "engine.DeleteRoleBinding", + trace.WithAttributes( + attribute.Stringer("role_binding_id", rb.ID), + ), + ) + defer span.End() + + // delete relationships from the role-binding + if _, err := e.client.DeleteRelationships(ctx, &pb.DeleteRelationshipsRequest{ + RelationshipFilter: &pb.RelationshipFilter{ + ResourceType: e.namespaced(e.rbac.RoleBindingResource.Name), + OptionalResourceId: rb.ID.String(), + }, + }); err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return err + } + + // delete relationships to the role-binding + if _, err := e.client.DeleteRelationships(ctx, &pb.DeleteRelationshipsRequest{ + RelationshipFilter: &pb.RelationshipFilter{ + ResourceType: e.namespaced(res.Type), + OptionalRelation: iapl.GrantRelationship, + OptionalSubjectFilter: &pb.SubjectFilter{ + SubjectType: e.namespaced(e.rbac.RoleBindingResource.Name), + OptionalSubjectId: rb.ID.String(), + }, + }, + }); err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return err + } + + return nil +} + +func (e *engine) ListRoleBindings(ctx context.Context, resource types.Resource, optionalRole *types.Resource) ([]types.RoleBinding, error) { + ctx, span := e.tracer.Start( + ctx, "engine.ListRoleBinding", + trace.WithAttributes( + attribute.Stringer("resource_id", resource.ID), + ), + ) + defer span.End() + + e.logger.Debugf("listing role-bindings for resource: %s, optionalRole: %v", resource.ID, optionalRole) + + // 1. list all grants on the resource + listRbFilter := &pb.RelationshipFilter{ + ResourceType: e.namespaced(resource.Type), + OptionalResourceId: resource.ID.String(), + OptionalRelation: iapl.GrantRelationship, + OptionalSubjectFilter: &pb.SubjectFilter{ + SubjectType: e.namespaced(e.rbac.RoleBindingResource.Name), + }, + } + + grantRel, err := e.readRelationships(ctx, listRbFilter) + if err != nil { + return nil, err + } + + // 2. fetch role-binding details for each grant + bindings := make(chan types.RoleBinding, len(grantRel)) + errs := make(chan error, len(grantRel)) + wg := &sync.WaitGroup{} + + for _, rel := range grantRel { + wg.Add(1) + + go func(grant *pb.Relationship) { + defer wg.Done() + + rbRes, err := e.NewResourceFromIDString(grant.Subject.Object.ObjectId) + if err != nil { + errs <- err + return + } + + rb, err := e.fetchRoleBinding(ctx, rbRes) + if err != nil { + if errors.Is(err, ErrRoleBindingNotFound) { + // print and record a warning message when there's a grant points + // to a role-binding that not longer exists. + err := fmt.Errorf("%w: dangling grant relationship: %s", err, grant.String()) + + span.RecordError(err) + e.logger.Warnf(err.Error()) + + return + } + errs <- err + + return + } + + if optionalRole != nil && rb.Role.ID.String() != optionalRole.ID.String() { + return + } + + if len(rb.Subjects) == 0 { + return + } + + bindings <- rb + }(rel) + } + + wg.Wait() + close(errs) + close(bindings) + + for err := range errs { + if err != nil { + return nil, err + } + } + + resp := make([]types.RoleBinding, 0, len(bindings)) + + for rb := range bindings { + resp = append(resp, rb) + } + + return resp, nil +} + +func (e *engine) UpdateRoleBinding(ctx context.Context, rb types.Resource, subjects []types.RoleBindingSubject) (types.RoleBinding, error) { + ctx, span := e.tracer.Start( + ctx, "engine.UpdateRoleBindings", + trace.WithAttributes( + attribute.Stringer("rolebinding_id", rb.ID), + ), + ) + defer span.End() + + rolebinding, err := e.fetchRoleBinding(ctx, rb) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return types.RoleBinding{}, err + } + + // 1. find the subjects to add or remove + current := make([]string, len(rolebinding.Subjects)) + incoming := make([]string, len(subjects)) + + for i, subj := range rolebinding.Subjects { + current[i] = subj.SubjectResource.ID.String() + } + + for i, subj := range subjects { + incoming[i] = subj.SubjectResource.ID.String() + } + + add, remove := diff(current, incoming) + + // return if there are no changes + if (len(add) + len(remove)) == 0 { + return rolebinding, nil + } + + // 2. create relationship updates + updates := make([]*pb.RelationshipUpdate, len(add)+len(remove)) + i := 0 + + mkupdate := func(id string, op pb.RelationshipUpdate_Operation) (*pb.RelationshipUpdate, error) { + subjRes, err := e.NewResourceFromIDString(id) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return nil, err + } + + rel, err := e.rolebindingSubjectRelationship(subjRes, rb.ID.String()) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return nil, err + } + + return &pb.RelationshipUpdate{Operation: op, Relationship: rel}, nil + } + + for _, id := range add { + update, err := mkupdate(id, pb.RelationshipUpdate_OPERATION_TOUCH) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return types.RoleBinding{}, err + } + + updates[i] = update + i++ + } + + for _, id := range remove { + update, err := mkupdate(id, pb.RelationshipUpdate_OPERATION_DELETE) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return types.RoleBinding{}, err + } + + updates[i] = update + i++ + } + + _, err = e.client.WriteRelationships(ctx, &pb.WriteRelationshipsRequest{Updates: updates}) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return types.RoleBinding{}, err + } + + rolebinding.Subjects = subjects + + return rolebinding, nil +} + +func (e *engine) GetRoleBinding(ctx context.Context, rolebinding types.Resource) (types.RoleBinding, error) { + ctx, span := e.tracer.Start( + ctx, "engine.GetRoleBinding", + trace.WithAttributes(attribute.Stringer("role_binding_id", rolebinding.ID)), + ) + defer span.End() + + return e.fetchRoleBinding(ctx, rolebinding) +} + +// isRoleBindable checks if a role is available for a resource. a role is not +// be available to a resource if it is owner is not associated with the resource +// in any way. +func (e *engine) isRoleBindable(ctx context.Context, role, res types.Resource) error { + req := &pb.CheckPermissionRequest{ + Resource: &pb.ObjectReference{ + ObjectType: e.namespaced(res.Type), + ObjectId: res.ID.String(), + }, + Subject: &pb.SubjectReference{ + Object: &pb.ObjectReference{ + ObjectType: e.namespaced(e.rbac.RoleResource.Name), + ObjectId: role.ID.String(), + }, + }, + Permission: iapl.AvailableRolesList, + Consistency: &pb.Consistency{ + Requirement: &pb.Consistency_FullyConsistent{FullyConsistent: true}, + }, + } + + err := e.checkPermission(ctx, req) + + switch { + case err == nil: + return nil + case errors.Is(err, ErrActionNotAssigned): + return fmt.Errorf("%w: role: %s is not available for resource: %s", ErrRoleNotFound, role.ID, res.ID) + default: + return err + } +} + +// deleteRoleBindingsForRole deletes all role-binding relationships with a given role. +func (e *engine) deleteRoleBindingsForRole(ctx context.Context, roleResource types.Resource) error { + ctx, span := e.tracer.Start( + ctx, "engine.deleteRoleBindingsForRole", + trace.WithAttributes( + attribute.Stringer("role_id", roleResource.ID), + ), + ) + defer span.End() + + requests := []*pb.DeleteRelationshipsRequest{} + + // 1. find all the bindings for the role + findBindingsFilter := &pb.RelationshipFilter{ + ResourceType: e.namespaced(e.rbac.RoleBindingResource.Name), + OptionalRelation: iapl.RolebindingRoleRelation, + OptionalSubjectFilter: &pb.SubjectFilter{ + SubjectType: e.namespaced(e.rbac.RoleResource.Name), + OptionalSubjectId: roleResource.ID.String(), + }, + } + + bindings, err := e.readRelationships(ctx, findBindingsFilter) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return err + } + + // 2. build a list of delete request for the subject and role relationship + // at the same time build a list of delete requests for all the grant + // relationships for all bindable resources + for _, rb := range bindings { + delSubjReq := &pb.DeleteRelationshipsRequest{ + RelationshipFilter: &pb.RelationshipFilter{ + ResourceType: rb.Resource.ObjectType, + OptionalResourceId: rb.Resource.ObjectId, + }, + } + + requests = append(requests, delSubjReq) + + for _, res := range e.rbacV2ResourceTypes { + delGrantReq := &pb.DeleteRelationshipsRequest{ + RelationshipFilter: &pb.RelationshipFilter{ + ResourceType: e.namespaced(res.Name), + OptionalRelation: iapl.GrantRelationship, + OptionalSubjectFilter: &pb.SubjectFilter{ + SubjectType: rb.Resource.ObjectType, + OptionalSubjectId: rb.Resource.ObjectId, + }, + }, + } + + requests = append(requests, delGrantReq) + } + } + + e.logger.Debugf("%d delete requests created", len(requests)) + + // 3. delete all the relationships + wg := &sync.WaitGroup{} + errs := make(chan error, len(requests)) + + for _, req := range requests { + wg.Add(1) + + go func(req *pb.DeleteRelationshipsRequest) { + defer wg.Done() + + if _, err := e.client.DeleteRelationships(ctx, req); err != nil { + errs <- err + } + }(req) + } + + wg.Wait() + close(errs) + + for err := range errs { + if err != nil { + return err + } + } + + return nil +} + +func (e *engine) fetchRoleBinding(ctx context.Context, roleBinding types.Resource) (types.RoleBinding, error) { + ctx, span := e.tracer.Start( + ctx, "engine.fetchRoleBinding", + trace.WithAttributes(attribute.Stringer("role_binding_id", roleBinding.ID)), + ) + defer span.End() + + // gather all relationships from this role-binding + rbRelFilter := &pb.RelationshipFilter{ + ResourceType: e.namespaced(e.rbac.RoleBindingResource.Name), + OptionalResourceId: roleBinding.ID.String(), + } + + rbRel, err := e.readRelationships(ctx, rbRelFilter) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return types.RoleBinding{}, err + } + + if len(rbRel) < 1 { + err := fmt.Errorf("%w: role binding: %s", ErrRoleBindingNotFound, roleBinding.ID.String()) + + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return types.RoleBinding{}, err + } + + rb := types.RoleBinding{ + ID: roleBinding.ID, + Subjects: make([]types.RoleBindingSubject, 0, len(rbRel)), + } + + for _, rel := range rbRel { + // process subject relationships + if rel.Relation == iapl.RolebindingSubjectRelation { + subjectRes, err := e.NewResourceFromIDString(rel.Subject.Object.ObjectId) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return types.RoleBinding{}, err + } + + rb.Subjects = append(rb.Subjects, types.RoleBindingSubject{SubjectResource: subjectRes}) + + continue + } + + // process role relationships + roleID, err := gidx.Parse(rel.Subject.Object.ObjectId) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return types.RoleBinding{}, err + } + + dbRole, err := e.store.GetRoleByID(ctx, roleID) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return types.RoleBinding{}, err + } + + rb.Role = types.Role{ + ID: roleID, + Name: dbRole.Name, + } + } + + return rb, nil +} + +// rolebindingSubjectRelationship is a helper function that creates a +// relationship between a role-binding and a subject. +func (e *engine) rolebindingSubjectRelationship(subj types.Resource, rbID string) (*pb.Relationship, error) { + subjConf, ok := e.rolebindingSubjectsMap[subj.Type] + if !ok { + return nil, fmt.Errorf( + "%w: subject: %s, subject type: %s", ErrInvalidRoleBindingSubjectType, + subj.ID, subj.Type, + ) + } + + relationshipSubject := &pb.SubjectReference{ + Object: &pb.ObjectReference{ + ObjectType: e.namespaced(subjConf.Name), + ObjectId: subj.ID.String(), + }, + } + + // for grants like "group#member" + if subjConf.SubjectRelation != "" { + relationshipSubject.OptionalRelation = subjConf.SubjectRelation + } + + relationship := &pb.Relationship{ + Resource: &pb.ObjectReference{ + ObjectType: e.namespaced(e.rbac.RoleBindingResource.Name), + ObjectId: rbID, + }, + Relation: iapl.RolebindingSubjectRelation, + Subject: relationshipSubject, + } + + return relationship, nil +} + +// rolebindingRoleRelationship is a helper function that creates a relationship +// between a role-binding and a role. +func (e *engine) rolebindingRoleRelationship(roleID, rbID string) *pb.Relationship { + return &pb.Relationship{ + Resource: &pb.ObjectReference{ + ObjectType: e.namespaced(e.rbac.RoleBindingResource.Name), + ObjectId: rbID, + }, + Relation: iapl.RolebindingRoleRelation, + Subject: &pb.SubjectReference{ + Object: &pb.ObjectReference{ + ObjectType: e.namespaced(e.rbac.RoleResource.Name), + ObjectId: roleID, + }, + }, + } +} + +// rolebindingGrantResourceRelationship is a helper function that creates the +// `grant` relationship from a resource to a role-binding. +func (e *engine) rolebindingGrantResourceRelationship(resource types.Resource, rbID string) (*pb.Relationship, error) { + rel := &pb.Relationship{ + Resource: &pb.ObjectReference{ + ObjectType: e.namespaced(resource.Type), + ObjectId: resource.ID.String(), + }, + Relation: iapl.GrantRelationship, + Subject: &pb.SubjectReference{ + Object: &pb.ObjectReference{ + ObjectType: e.namespaced(e.rbac.RoleBindingResource.Name), + ObjectId: rbID, + }, + }, + } + + return rel, nil +} + +func newRoleBindingWithPrefix(prefix string, role types.Role) types.RoleBinding { + rb := types.RoleBinding{ + ID: gidx.MustNewID(prefix), + Role: role, + } + + return rb +} diff --git a/internal/query/rolebindings_test.go b/internal/query/rolebindings_test.go new file mode 100644 index 00000000..1422ef0e --- /dev/null +++ b/internal/query/rolebindings_test.go @@ -0,0 +1,734 @@ +package query + +import ( + "context" + "testing" + + pb "github.com/authzed/authzed-go/proto/authzed/api/v1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "go.infratographer.com/permissions-api/internal/iapl" + "go.infratographer.com/permissions-api/internal/testingx" + "go.infratographer.com/permissions-api/internal/types" +) + +func TestCreateRoleBinding(t *testing.T) { + namespace := "testroles" + ctx := context.Background() + + doc := DefaultPolicyDocumentV2() + doc.ResourceTypes = append(doc.ResourceTypes, iapl.ResourceType{ + Name: "role", + IDPrefix: "permrol", + Relationships: []iapl.Relationship{ + { + Relation: "subject", + TargetTypes: []types.TargetType{{Name: "subject"}}, + }, + }, + }) + + policy := iapl.NewPolicy(doc) + err := policy.Validate() + require.NoError(t, err) + + e := testEngine(ctx, t, namespace, policy) + + 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) + actor, err := e.NewResourceFromIDString("idntusr-actor") + require.NoError(t, err) + + role, err := e.CreateRoleV2(ctx, actor, root, "lb_viewer", []string{"loadbalancer_list", "loadbalancer_get"}) + require.NoError(t, err) + + roleRes, err := e.NewResourceFromID(role.ID) + require.NoError(t, err) + + notfoundRole, err := e.NewResourceFromIDString("permrv2-notfound") + require.NoError(t, err) + + v1role, err := e.NewResourceFromIDString("permrol-v1role") + require.NoError(t, err) + + _, err = e.client.WriteRelationships(ctx, &pb.WriteRelationshipsRequest{ + Updates: rbacV2CreateParentRel(root, child, namespace), + }) + require.NoError(t, err) + + type input struct { + resource types.Resource + role types.Resource + subjects []types.RoleBindingSubject + } + + tc := []testingx.TestCase[input, types.RoleBinding]{ + { + Name: "CreateRoleBindingRoleNotFound", + Input: input{ + resource: root, + role: notfoundRole, + subjects: []types.RoleBindingSubject{{SubjectResource: actor}}, + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[types.RoleBinding]) { + assert.ErrorContains(t, res.Err, ErrRoleNotFound.Error()) + }, + }, + { + Name: "CreateRoleBindingV1Role", + Input: input{ + resource: root, + role: v1role, + subjects: []types.RoleBindingSubject{{SubjectResource: actor}}, + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[types.RoleBinding]) { + assert.ErrorContains(t, res.Err, ErrRoleNotFound.Error()) + }, + }, + { + Name: "CreateRoleBindingChild", + Input: input{ + resource: child, + role: roleRes, + subjects: []types.RoleBindingSubject{{SubjectResource: actor}}, + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[types.RoleBinding]) { + assert.NoError(t, res.Err) + assert.Equal(t, role.ID, res.Success.Role.ID) + assert.Len(t, res.Success.Subjects, 1) + + rb, err := e.ListRoleBindings(ctx, child, nil) + assert.NoError(t, err) + assert.Len(t, rb, 1) + }, + }, + { + Name: "CreateRoleBindingOrphan", + Input: input{ + resource: orphan, + role: roleRes, + subjects: []types.RoleBindingSubject{{SubjectResource: actor}}, + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[types.RoleBinding]) { + assert.ErrorContains(t, res.Err, ErrRoleNotFound.Error()) + }, + }, + { + Name: "CreateRoleBindingSuccess", + Input: input{ + resource: root, + role: roleRes, + subjects: []types.RoleBindingSubject{{SubjectResource: actor}}, + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[types.RoleBinding]) { + assert.NoError(t, res.Err) + + assert.Equal(t, role.ID, res.Success.Role.ID) + assert.Len(t, res.Success.Subjects, 1) + + rb, err := e.ListRoleBindings(ctx, root, nil) + assert.NoError(t, err) + assert.Len(t, rb, 1) + }, + }, + } + + testFn := func(ctx context.Context, in input) testingx.TestResult[types.RoleBinding] { + rb, err := e.CreateRoleBinding(ctx, in.resource, in.role, in.subjects) + return testingx.TestResult[types.RoleBinding]{Success: rb, Err: err} + } + + testingx.RunTests(ctx, t, tc, testFn) +} + +func TestListRoleBindings(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) + actor, err := e.NewResourceFromIDString("idntusr-actor") + require.NoError(t, err) + + viewer, err := e.CreateRoleV2(ctx, actor, root, "lb_viewer", []string{"loadbalancer_list", "loadbalancer_get"}) + require.NoError(t, err) + + editor, err := e.CreateRoleV2(ctx, actor, root, "lb_editor", []string{"loadbalancer_list", "loadbalancer_get", "loadbalancer_create", "loadbalancer_update"}) + require.NoError(t, err) + + viewerRes, err := e.NewResourceFromID(viewer.ID) + require.NoError(t, err) + + editorRes, err := e.NewResourceFromID(editor.ID) + require.NoError(t, err) + + notfoundRole, err := e.NewResourceFromIDString("permrv2-notfound") + require.NoError(t, err) + + _, err = e.CreateRoleBinding(ctx, root, viewerRes, []types.RoleBindingSubject{{SubjectResource: actor}}) + require.NoError(t, err) + + _, err = e.CreateRoleBinding(ctx, root, editorRes, []types.RoleBindingSubject{{SubjectResource: actor}}) + require.NoError(t, err) + + _, err = e.client.WriteRelationships(ctx, &pb.WriteRelationshipsRequest{ + Updates: rbacV2CreateParentRel(root, child, namespace), + }) + require.NoError(t, err) + + type input struct { + resource types.Resource + role *types.Resource + } + + tc := []testingx.TestCase[input, []types.RoleBinding]{ + { + Name: "ListAll", + Input: input{ + resource: root, + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[[]types.RoleBinding]) { + assert.Len(t, res.Success, 2) + }, + }, + { + Name: "ListWithViewerRole", + Input: input{ + resource: root, + role: &viewerRes, + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[[]types.RoleBinding]) { + assert.Len(t, res.Success, 1) + assert.Equal(t, viewer.ID, res.Success[0].Role.ID) + }, + }, + { + Name: "ListWithEditorRole", + Input: input{ + resource: root, + role: &editorRes, + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[[]types.RoleBinding]) { + assert.Len(t, res.Success, 1) + assert.Equal(t, editor.ID, res.Success[0].Role.ID) + }, + }, + { + Name: "ListChildTenant", + Input: input{ + resource: child, + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[[]types.RoleBinding]) { + assert.Len(t, res.Success, 0) + }, + }, + { + Name: "ListWithNonExistentRole", + Input: input{ + resource: root, + role: ¬foundRole, + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[[]types.RoleBinding]) { + assert.Len(t, res.Success, 0) + }, + }, + } + + testFn := func(ctx context.Context, in input) testingx.TestResult[[]types.RoleBinding] { + rb, err := e.ListRoleBindings(ctx, in.resource, in.role) + return testingx.TestResult[[]types.RoleBinding]{Success: rb, Err: err} + } + + testingx.RunTests(ctx, t, tc, testFn) +} + +func TestGetRoleBinding(t *testing.T) { + namespace := "testroles" + ctx := context.Background() + e := testEngine(ctx, t, namespace, rbacv2TestPolicy()) + + root, err := e.NewResourceFromIDString("tnntten-root") + require.NoError(t, err) + actor, err := e.NewResourceFromIDString("idntusr-actor") + require.NoError(t, err) + + viewer, err := e.CreateRoleV2(ctx, actor, root, "lb_viewer", []string{"loadbalancer_list", "loadbalancer_get"}) + require.NoError(t, err) + + viewerRes, err := e.NewResourceFromID(viewer.ID) + require.NoError(t, err) + + notfoundRB, err := e.NewResourceFromIDString("permrbn-notfound") + require.NoError(t, err) + + rb, err := e.CreateRoleBinding(ctx, root, viewerRes, []types.RoleBindingSubject{{SubjectResource: actor}}) + require.NoError(t, err) + + rbRes, err := e.NewResourceFromID(rb.ID) + require.NoError(t, err) + + tc := []testingx.TestCase[types.Resource, types.RoleBinding]{ + { + Name: "GetRoleBindingSuccess", + Input: rbRes, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[types.RoleBinding]) { + assert.NoError(t, res.Err) + assert.Equal(t, viewer.ID, res.Success.Role.ID) + assert.Len(t, res.Success.Subjects, 1) + assert.Equal(t, actor.ID, res.Success.Subjects[0].SubjectResource.ID) + }, + }, + { + Name: "GetRoleBindingNotFound", + Input: notfoundRB, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[types.RoleBinding]) { + assert.ErrorContains(t, res.Err, ErrRoleBindingNotFound.Error()) + }, + }, + } + + testFn := func(ctx context.Context, in types.Resource) testingx.TestResult[types.RoleBinding] { + rb, err := e.GetRoleBinding(ctx, in) + return testingx.TestResult[types.RoleBinding]{Success: rb, Err: err} + } + + testingx.RunTests(ctx, t, tc, testFn) +} + +func TestUpdateRoleBinding(t *testing.T) { + namespace := "testroles" + ctx := context.Background() + e := testEngine(ctx, t, namespace, rbacv2TestPolicy()) + + root, err := e.NewResourceFromIDString("tnntten-root") + require.NoError(t, err) + actor, err := e.NewResourceFromIDString("idntusr-actor") + require.NoError(t, err) + + viewer, err := e.CreateRoleV2(ctx, actor, root, "lb_viewer", []string{"loadbalancer_list", "loadbalancer_get"}) + require.NoError(t, err) + viewerRes, err := e.NewResourceFromID(viewer.ID) + require.NoError(t, err) + + rb, err := e.CreateRoleBinding(ctx, root, viewerRes, []types.RoleBindingSubject{{SubjectResource: actor}}) + require.NoError(t, err) + rbRes, err := e.NewResourceFromID(rb.ID) + require.NoError(t, err) + notfoundRB, err := e.NewResourceFromIDString("permrbn-notfound") + require.NoError(t, err) + + user1, err := e.NewResourceFromIDString("idntusr-user1") + require.NoError(t, err) + group1, err := e.NewResourceFromIDString("idntgrp-group1") + require.NoError(t, err) + invalidsubj, err := e.NewResourceFromIDString("loadbal-lb") + require.NoError(t, err) + + type input struct { + rb types.Resource + subj []types.RoleBindingSubject + } + + tc := []testingx.TestCase[input, types.RoleBinding]{ + { + Name: "UpdateRoleBindingNotFound", + Input: input{ + rb: notfoundRB, + subj: []types.RoleBindingSubject{{SubjectResource: actor}}, + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[types.RoleBinding]) { + assert.ErrorContains(t, res.Err, ErrRoleBindingNotFound.Error()) + }, + }, + { + Name: "UpdateRoleBindingInvalidSubject", + Input: input{ + rb: rbRes, + subj: []types.RoleBindingSubject{{SubjectResource: invalidsubj}}, + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[types.RoleBinding]) { + assert.ErrorContains(t, res.Err, ErrInvalidArgument.Error()) + }, + }, + { + Name: "UpdateRoleBindingSuccess", + Input: input{ + rb: rbRes, + subj: []types.RoleBindingSubject{{SubjectResource: user1}, {SubjectResource: group1}}, + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[types.RoleBinding]) { + assert.NoError(t, res.Err) + + assert.Len(t, res.Success.Subjects, 2) + assert.Contains(t, res.Success.Subjects, types.RoleBindingSubject{SubjectResource: user1}) + assert.Contains(t, res.Success.Subjects, types.RoleBindingSubject{SubjectResource: group1}) + assert.NotContains(t, res.Success.Subjects, types.RoleBindingSubject{SubjectResource: actor}) + }, + }, + } + + testFn := func(ctx context.Context, in input) testingx.TestResult[types.RoleBinding] { + rb, err := e.UpdateRoleBinding(ctx, in.rb, in.subj) + return testingx.TestResult[types.RoleBinding]{Success: rb, Err: err} + } + + testingx.RunTests(ctx, t, tc, testFn) +} + +func TestDeleteRoleBinding(t *testing.T) { + namespace := "testroles" + ctx := context.Background() + e := testEngine(ctx, t, namespace, rbacv2TestPolicy()) + + root, err := e.NewResourceFromIDString("tnntten-root") + require.NoError(t, err) + actor, err := e.NewResourceFromIDString("idntusr-actor") + require.NoError(t, err) + + viewer, err := e.CreateRoleV2(ctx, actor, root, "lb_viewer", []string{"loadbalancer_list", "loadbalancer_get"}) + require.NoError(t, err) + viewerRes, err := e.NewResourceFromID(viewer.ID) + require.NoError(t, err) + + rb, err := e.CreateRoleBinding(ctx, root, viewerRes, []types.RoleBindingSubject{{SubjectResource: actor}}) + require.NoError(t, err) + rbRes, err := e.NewResourceFromID(rb.ID) + require.NoError(t, err) + + notfoundRB, err := e.NewResourceFromIDString("permrbn-notfound") + require.NoError(t, err) + + tc := []testingx.TestCase[types.Resource, types.RoleBinding]{ + { + Name: "DeleteRoleBindingNotFound", + Input: notfoundRB, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[types.RoleBinding]) { + assert.NoError(t, res.Err) + + rb, err := e.ListRoleBindings(ctx, root, nil) + assert.NoError(t, err) + assert.Len(t, rb, 1) + }, + Sync: true, + }, + { + Name: "DeleteRoleBindingSuccess", + Input: rbRes, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[types.RoleBinding]) { + assert.NoError(t, res.Err) + + rb, err := e.ListRoleBindings(ctx, root, nil) + assert.NoError(t, err) + assert.Len(t, rb, 0) + }, + Sync: true, + }, + } + + testFn := func(ctx context.Context, in types.Resource) testingx.TestResult[types.RoleBinding] { + err := e.DeleteRoleBinding(ctx, in, root) + return testingx.TestResult[types.RoleBinding]{Err: err} + } + + testingx.RunTests(ctx, t, tc, testFn) +} + +func TestPermissions(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) + actor, err := e.NewResourceFromIDString("idntusr-actor") + require.NoError(t, err) + + // create child tenant relationships + _, err = e.client.WriteRelationships(ctx, &pb.WriteRelationshipsRequest{ + Updates: rbacV2CreateParentRel(root, child, namespace), + }) + require.NoError(t, err) + + // role + viewer, err := e.CreateRoleV2(ctx, actor, root, "lb_viewer", []string{"loadbalancer_list", "loadbalancer_get"}) + require.NoError(t, err) + viewerRes, err := e.NewResourceFromID(viewer.ID) + require.NoError(t, err) + + // subjects + user1, err := e.NewResourceFromIDString("idntusr-user1") + require.NoError(t, err) + user2, err := e.NewResourceFromIDString("idntusr-user2") + require.NoError(t, err) + group1, err := e.NewResourceFromIDString("idntgrp-group1") + require.NoError(t, err) + + err = e.CreateRelationships(ctx, []types.Relationship{{ + Resource: group1, + Relation: "member", + Subject: user2, + }}) + require.NoError(t, err) + + _, err = e.client.WriteRelationships(ctx, &pb.WriteRelationshipsRequest{ + Updates: rbacV2CreateParentRel(root, group1, namespace), + }) + require.NoError(t, err) + + // resources + lb1, err := e.NewResourceFromIDString("loadbal-lb1") + require.NoError(t, err) + + err = e.CreateRelationships(ctx, []types.Relationship{{ + Resource: lb1, + Relation: "owner", + Subject: child, + }}) + require.NoError(t, err) + + fullconsistency := &pb.Consistency{Requirement: &pb.Consistency_FullyConsistent{FullyConsistent: true}} + + tc := []testingx.TestCase[any, any]{ + { + Name: "PermissionsOnResource", + SetupFn: func(ctx context.Context, t *testing.T) context.Context { + err := e.checkPermission(ctx, &pb.CheckPermissionRequest{ + Consistency: fullconsistency, + Resource: resourceToSpiceDBRef(namespace, lb1), + Permission: "loadbalancer_get", + Subject: &pb.SubjectReference{Object: resourceToSpiceDBRef(namespace, user1)}, + }) + require.Error(t, err) + + _, err = e.CreateRoleBinding(ctx, lb1, viewerRes, []types.RoleBindingSubject{{SubjectResource: user1}}) + require.NoError(t, err) + + return ctx + }, + CheckFn: func(ctx context.Context, t *testing.T, _ testingx.TestResult[any]) { + err := e.checkPermission(ctx, &pb.CheckPermissionRequest{ + Consistency: fullconsistency, + Resource: resourceToSpiceDBRef(namespace, lb1), + Permission: "loadbalancer_get", + Subject: &pb.SubjectReference{Object: resourceToSpiceDBRef(namespace, user1)}, + }) + assert.NoError(t, err) + }, + CleanupFn: func(ctx context.Context) { + rbs, _ := e.ListRoleBindings(ctx, lb1, nil) + for _, rb := range rbs { + rbRes, _ := e.NewResourceFromID(rb.ID) + _ = e.DeleteRoleBinding(ctx, rbRes, lb1) + } + }, + Sync: true, + }, + { + Name: "PermissionsOnOwner", + SetupFn: func(ctx context.Context, t *testing.T) context.Context { + err := e.checkPermission(ctx, &pb.CheckPermissionRequest{ + Consistency: fullconsistency, + Resource: resourceToSpiceDBRef(namespace, lb1), + Permission: "loadbalancer_get", + Subject: &pb.SubjectReference{Object: resourceToSpiceDBRef(namespace, user1)}, + }) + require.Error(t, err) + + _, err = e.CreateRoleBinding(ctx, child, viewerRes, []types.RoleBindingSubject{{SubjectResource: user1}}) + require.NoError(t, err) + + return ctx + }, + CheckFn: func(ctx context.Context, t *testing.T, _ testingx.TestResult[any]) { + err := e.checkPermission(ctx, &pb.CheckPermissionRequest{ + Consistency: fullconsistency, + Resource: resourceToSpiceDBRef(namespace, lb1), + Permission: "loadbalancer_get", + Subject: &pb.SubjectReference{Object: resourceToSpiceDBRef(namespace, user1)}, + }) + assert.NoError(t, err) + }, + CleanupFn: func(ctx context.Context) { + rbs, _ := e.ListRoleBindings(ctx, child, nil) + for _, rb := range rbs { + rbRes, _ := e.NewResourceFromID(rb.ID) + _ = e.DeleteRoleBinding(ctx, rbRes, child) + } + }, + Sync: true, + }, + { + Name: "PermissionsOnOwnerParent", + SetupFn: func(ctx context.Context, t *testing.T) context.Context { + err := e.checkPermission(ctx, &pb.CheckPermissionRequest{ + Consistency: fullconsistency, + Resource: resourceToSpiceDBRef(namespace, lb1), + Permission: "loadbalancer_get", + Subject: &pb.SubjectReference{Object: resourceToSpiceDBRef(namespace, user1)}, + }) + require.Error(t, err) + + _, err = e.CreateRoleBinding(ctx, root, viewerRes, []types.RoleBindingSubject{{SubjectResource: user1}}) + require.NoError(t, err) + + return ctx + }, + CheckFn: func(ctx context.Context, t *testing.T, _ testingx.TestResult[any]) { + err := e.checkPermission(ctx, &pb.CheckPermissionRequest{ + Consistency: fullconsistency, + Resource: resourceToSpiceDBRef(namespace, lb1), + Permission: "loadbalancer_get", + Subject: &pb.SubjectReference{Object: resourceToSpiceDBRef(namespace, user1)}, + }) + assert.NoError(t, err) + }, + CleanupFn: func(ctx context.Context) { + rbs, _ := e.ListRoleBindings(ctx, root, nil) + for _, rb := range rbs { + rbRes, _ := e.NewResourceFromID(rb.ID) + _ = e.DeleteRoleBinding(ctx, rbRes, root) + } + }, + Sync: true, + }, + { + Name: "PermissionsOnGroups", + SetupFn: func(ctx context.Context, t *testing.T) context.Context { + err := e.checkPermission(ctx, &pb.CheckPermissionRequest{ + Consistency: fullconsistency, + Resource: resourceToSpiceDBRef(namespace, lb1), + Permission: "loadbalancer_get", + Subject: &pb.SubjectReference{Object: resourceToSpiceDBRef(namespace, user2)}, + }) + require.Error(t, err) + + _, err = e.CreateRoleBinding(ctx, root, viewerRes, []types.RoleBindingSubject{{SubjectResource: group1}}) + require.NoError(t, err) + + return ctx + }, + CheckFn: func(ctx context.Context, t *testing.T, _ testingx.TestResult[any]) { + err := e.checkPermission(ctx, &pb.CheckPermissionRequest{ + Consistency: fullconsistency, + Resource: resourceToSpiceDBRef(namespace, lb1), + Permission: "loadbalancer_get", + Subject: &pb.SubjectReference{Object: resourceToSpiceDBRef(namespace, user2)}, + }) + assert.NoError(t, err) + }, + // No cleanup + Sync: true, + }, + { + Name: "GroupMembershipRemoval", + SetupFn: func(ctx context.Context, t *testing.T) context.Context { + err := e.checkPermission(ctx, &pb.CheckPermissionRequest{ + Consistency: fullconsistency, + Resource: resourceToSpiceDBRef(namespace, lb1), + Permission: "loadbalancer_get", + Subject: &pb.SubjectReference{Object: resourceToSpiceDBRef(namespace, user2)}, + }) + require.NoError(t, err) + + err = e.DeleteRelationships(ctx, types.Relationship{ + Resource: group1, + Relation: "member", + Subject: user2, + }) + require.NoError(t, err) + + return ctx + }, + CheckFn: func(ctx context.Context, t *testing.T, _ testingx.TestResult[any]) { + err := e.checkPermission(ctx, &pb.CheckPermissionRequest{ + Consistency: fullconsistency, + Resource: resourceToSpiceDBRef(namespace, lb1), + Permission: "loadbalancer_get", + Subject: &pb.SubjectReference{Object: resourceToSpiceDBRef(namespace, user2)}, + }) + assert.Error(t, err) + }, + CleanupFn: func(ctx context.Context) { + _ = e.CreateRelationships(ctx, []types.Relationship{{ + Resource: group1, + Relation: "member", + Subject: user2, + }}) + }, + Sync: true, + }, + { + Name: "RoleActionRemoval", + SetupFn: func(ctx context.Context, t *testing.T) context.Context { + err := e.checkPermission(ctx, &pb.CheckPermissionRequest{ + Consistency: fullconsistency, + Resource: resourceToSpiceDBRef(namespace, lb1), + Permission: "loadbalancer_get", + Subject: &pb.SubjectReference{Object: resourceToSpiceDBRef(namespace, user2)}, + }) + require.NoError(t, err) + + _, err = e.UpdateRoleV2(ctx, root, viewerRes, "lb_viewer", []string{"loadbalancer_list"}) + require.NoError(t, err) + + return ctx + }, + CheckFn: func(ctx context.Context, t *testing.T, _ testingx.TestResult[any]) { + err := e.checkPermission(ctx, &pb.CheckPermissionRequest{ + Consistency: fullconsistency, + Resource: resourceToSpiceDBRef(namespace, lb1), + Permission: "loadbalancer_get", + Subject: &pb.SubjectReference{Object: resourceToSpiceDBRef(namespace, user2)}, + }) + assert.Error(t, err) + }, + CleanupFn: func(ctx context.Context) { + _, _ = e.UpdateRoleV2(ctx, root, viewerRes, "lb_viewer", []string{"loadbalancer_list", "loadbalancer_get"}) + }, + Sync: true, + }, + { + Name: "RoleRemoval", + SetupFn: func(ctx context.Context, t *testing.T) context.Context { + err := e.checkPermission(ctx, &pb.CheckPermissionRequest{ + Consistency: fullconsistency, + Resource: resourceToSpiceDBRef(namespace, lb1), + Permission: "loadbalancer_get", + Subject: &pb.SubjectReference{Object: resourceToSpiceDBRef(namespace, user2)}, + }) + require.NoError(t, err) + + err = e.DeleteRoleV2(ctx, viewerRes) + require.NoError(t, err) + + return ctx + }, + CheckFn: func(ctx context.Context, t *testing.T, _ testingx.TestResult[any]) { + err := e.checkPermission(ctx, &pb.CheckPermissionRequest{ + Consistency: fullconsistency, + Resource: resourceToSpiceDBRef(namespace, lb1), + Permission: "loadbalancer_get", + Subject: &pb.SubjectReference{Object: resourceToSpiceDBRef(namespace, user2)}, + }) + assert.Error(t, err) + }, + Sync: true, + }, + } + + testFn := func(ctx context.Context, in any) testingx.TestResult[any] { + return testingx.TestResult[any]{} + } + + testingx.RunTests(ctx, t, tc, testFn) +} diff --git a/internal/query/roles_v2.go b/internal/query/roles_v2.go index e5e08aa8..f22adf98 100644 --- a/internal/query/roles_v2.go +++ b/internal/query/roles_v2.go @@ -426,7 +426,10 @@ func (e *engine) DeleteRoleV2(ctx context.Context, roleResource types.Resource) errs = append(errs, err) } - // 2.c TODO remove all role relationships in role bindings associated with this role + // 2.c remove all role relationships in role bindings associated with this role + if err := e.deleteRoleBindingsForRole(ctx, roleResource); err != nil { + errs = append(errs, err) + } for _, err := range errs { if err != nil { diff --git a/internal/query/roles_v2_test.go b/internal/query/roles_v2_test.go index 2d57fa11..0726e0fc 100644 --- a/internal/query/roles_v2_test.go +++ b/internal/query/roles_v2_test.go @@ -2,7 +2,6 @@ package query import ( "context" - "fmt" "testing" pb "github.com/authzed/authzed-go/proto/authzed/api/v1" @@ -349,7 +348,6 @@ func TestUpdateRolesV2(t *testing.T) { 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") }, @@ -399,3 +397,108 @@ func TestUpdateRolesV2(t *testing.T) { testingx.RunTests(ctx, t, tc, testFn) } + +func TestDeleteRolesV2(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) + theotherchild, err := e.NewResourceFromIDString("tnntten-theotherchild") + require.NoError(t, err) + actor, err := e.NewResourceFromIDString("idntusr-actor") + require.NoError(t, err) + + role, err := e.CreateRoleV2(ctx, actor, root, "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) + + _, err = e.client.WriteRelationships(ctx, &pb.WriteRelationshipsRequest{ + Updates: rbacV2CreateParentRel(root, child, namespace), + }) + require.NoError(t, err) + + _, err = e.client.WriteRelationships(ctx, &pb.WriteRelationshipsRequest{ + Updates: rbacV2CreateParentRel(root, theotherchild, namespace), + }) + require.NoError(t, err) + + // these bindings are expected to be deleted after the role is deleted + _, err = e.CreateRoleBinding(ctx, root, roleRes, []types.RoleBindingSubject{{SubjectResource: actor}}) + require.NoError(t, err) + + _, err = e.CreateRoleBinding(ctx, child, roleRes, []types.RoleBindingSubject{{SubjectResource: actor}}) + require.NoError(t, err) + + _, err = e.CreateRoleBinding(ctx, theotherchild, roleRes, []types.RoleBindingSubject{{SubjectResource: actor}}) + require.NoError(t, err) + + rb, err := e.ListRoleBindings(ctx, root, &roleRes) + require.NoError(t, err) + require.Len(t, rb, 1) + + rb, err = e.ListRoleBindings(ctx, child, &roleRes) + require.NoError(t, err) + require.Len(t, rb, 1) + + rb, err = e.ListRoleBindings(ctx, theotherchild, &roleRes) + require.NoError(t, err) + require.Len(t, rb, 1) + + tc := []testingx.TestCase[types.Resource, types.Role]{ + { + Name: "DeleteRoleNotFound", + Input: notfoundRes, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[types.Role]) { + assert.ErrorContains(t, res.Err, ErrRoleNotFound.Error()) + }, + Sync: true, + }, + { + Name: "DeleteRoleInvalidInput", + Input: actor, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[types.Role]) { + assert.Error(t, res.Err) + }, + }, + { + Name: "DeleteRoleSuccess", + Input: roleRes, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[types.Role]) { + assert.NoError(t, res.Err) + + _, err := e.GetRoleV2(ctx, roleRes) + assert.ErrorContains(t, err, ErrRoleNotFound.Error()) + + // make sure the role bindings are also deleted + rb, err := e.ListRoleBindings(ctx, root, &roleRes) + assert.NoError(t, err) + assert.Len(t, rb, 0) + + rb, err = e.ListRoleBindings(ctx, child, &roleRes) + assert.NoError(t, err) + assert.Len(t, rb, 0) + + rb, err = e.ListRoleBindings(ctx, theotherchild, &roleRes) + assert.NoError(t, err) + assert.Len(t, rb, 0) + }, + Sync: true, + }, + } + + testFn := func(ctx context.Context, in types.Resource) testingx.TestResult[types.Role] { + err := e.DeleteRoleV2(ctx, in) + return testingx.TestResult[types.Role]{Err: err} + } + + testingx.RunTests(ctx, t, tc, testFn) +} diff --git a/internal/query/service.go b/internal/query/service.go index b430bf6b..80c68f84 100644 --- a/internal/query/service.go +++ b/internal/query/service.go @@ -18,6 +18,11 @@ import ( const ( outcomeAllowed = "allowed" outcomeDenied = "denied" + + // DefaultRoleResourceName is the default name for a role resource + DefaultRoleResourceName = "role" + // DefaultRoleBindingResourceName is the default name for a role binding resource + DefaultRoleBindingResourceName = "role_binding" ) // Engine represents a client for making permissions queries. @@ -53,6 +58,20 @@ type Engine interface { // DeleteRoleV2 deletes a V2 role. DeleteRoleV2(ctx context.Context, roleResource types.Resource) error + // CreateRoleBinding creates all the necessary relationships for a role binding. + // role binding here establishes a three-way relationship between a role, + // a resource, and the subjects. + CreateRoleBinding(ctx context.Context, resource, role types.Resource, subjects []types.RoleBindingSubject) (types.RoleBinding, error) + // ListRoleBindings lists all role-bindings for a resource, an optional Role + // can be provided to filter the role-bindings. + ListRoleBindings(ctx context.Context, resource types.Resource, optionalRole *types.Resource) ([]types.RoleBinding, error) + // GetRoleBinding fetches a role-binding by its ID. + GetRoleBinding(ctx context.Context, rolebinding types.Resource) (types.RoleBinding, error) + // UpdateRoleBinding updates the subjects of a role-binding. + UpdateRoleBinding(ctx context.Context, rolebinding types.Resource, subjects []types.RoleBindingSubject) (types.RoleBinding, error) + // DeleteRoleBinding removes subjects from a role-binding. + DeleteRoleBinding(ctx context.Context, rolebinding, resource types.Resource) error + AllActions() []string } diff --git a/openapi-v2.yaml b/openapi-v2.yaml new file mode 100644 index 00000000..1d45caa6 --- /dev/null +++ b/openapi-v2.yaml @@ -0,0 +1,1159 @@ +openapi: 3.0.3 +info: + version: 0.0.1 + title: Permissions API V2 + contact: + name: Infratographer Authors + url: http://github.com/infratographer + license: + name: Apache 2.0 + url: https://www.apache.org/licenses/LICENSE-2.0.html +servers: + - url: http://localhost:7603/api/v2 +paths: + /resources/{id}/roles: + get: + tags: + - roles + summary: list-roles + description: | + list all available roles for a resource, including roles that are + inherited from parent resources + operationId: listRoles + responses: + "200": + description: tnntten-root + content: + application/json: + schema: + type: object + properties: + data: + type: array + items: + type: object + properties: + actions: + type: array + items: + type: string + example: avail_role + example: + - avail_role + - doc_create + - doc_delete + - doc_get + - doc_list + - doc_update + - role_create + - role_delete + - role_get + - role_list + - role_update + - rolebinding_create + - rolebinding_delete + - rolebinding_get + - rolebinding_list + - rolebinding_update + - secret_create + - secret_delete + - secret_get + - secret_list + - secret_update + created_at: + type: string + example: "2024-04-10T20:09:06Z" + created_by: + type: string + example: idntusr-bailin + id: + type: string + example: permrv2-_vKIY7KTwIABD0V9Qpnef + name: + type: string + example: super_user + updated_at: + type: string + example: "2024-04-10T20:18:10Z" + updated_by: + type: string + example: idntusr-bailin + example: + - actions: + - avail_role + - doc_create + - doc_delete + - doc_get + - doc_list + - doc_update + - role_create + - role_delete + - role_get + - role_list + - role_update + - rolebinding_create + - rolebinding_delete + - rolebinding_get + - rolebinding_list + - rolebinding_update + - secret_create + - secret_delete + - secret_get + - secret_list + - secret_update + created_at: "2024-04-10T20:09:06Z" + created_by: idntusr-bailin + id: permrv2-_vKIY7KTwIABD0V9Qpnef + name: super_user + updated_at: "2024-04-10T20:18:10Z" + updated_by: idntusr-bailin + examples: + tnntten-root: + value: + data: + - actions: + - avail_role + - doc_create + - doc_delete + - doc_get + - doc_list + - doc_update + - role_create + - role_delete + - role_get + - role_list + - role_update + - rolebinding_create + - rolebinding_delete + - rolebinding_get + - rolebinding_list + - rolebinding_update + - secret_create + - secret_delete + - secret_get + - secret_list + - secret_update + created_at: "2024-04-10T20:09:06Z" + created_by: idntusr-bailin + id: permrv2-_vKIY7KTwIABD0V9Qpnef + name: super_user + updated_at: "2024-04-10T20:18:10Z" + updated_by: idntusr-bailin + post: + tags: + - roles + summary: create-role + description: | + create a role for a resource. The role will be available for use in + role-bindings for the resource + operationId: createRole + requestBody: + content: + application/json: + schema: + type: object + properties: + actions: + type: array + items: + type: string + example: loadbalancer_list + example: + - loadbalancer_list + - loadbalancer_get + - loadbalancer_update + - loadbalancer_create + name: + type: string + example: lb_editor + examples: + create-role: + value: + actions: + - loadbalancer_list + - loadbalancer_get + - loadbalancer_update + - loadbalancer_create + name: lb_editor + responses: + "201": + description: "role object" + content: + application/json: + schema: + type: object + properties: + actions: + type: array + items: + type: string + example: role_list + example: + - role_list + - rolebinding_create + - loadbalancer_list + - loadbalancer_update + - loadbalancer_delete + - role_create + - role_delete + - loadbalancer_create + - loadbalancer_get + - rolebinding_list + - rolebinding_delete + - role_get + - role_update + created_at: + type: string + example: "2024-02-28T17:22:04Z" + created_by: + type: string + example: idntusr-bailin + id: + type: string + example: permrv2-ecBlNMsPrvVFgUUAUfmeY + name: + type: string + example: super_admin + resource_id: + type: string + example: tnntten-root + updated_at: + type: string + example: "2024-02-28T17:22:04Z" + updated_by: + type: string + example: idntusr-bailin + examples: + create-role: + value: + actions: + - loadbalancer_list + - loadbalancer_get + - loadbalancer_update + - loadbalancer_create + created_at: "2024-02-29T18:18:18Z" + created_by: idntusr-bailin + id: permrv2-IG7RfsYhyga0EwEbY4BKs + name: lb_editor + resource_id: tnntten-a + updated_at: "2024-02-29T18:18:18Z" + updated_by: idntusr-bailin + lb-viwer: + value: + actions: + - role_list + - rolebinding_create + - loadbalancer_list + - loadbalancer_update + - loadbalancer_delete + - role_create + - role_delete + - loadbalancer_create + - loadbalancer_get + - rolebinding_list + - rolebinding_delete + - role_get + - role_update + created_at: "2024-02-28T17:22:04Z" + created_by: idntusr-bailin + id: permrv2-ecBlNMsPrvVFgUUAUfmeY + name: super_admin + resource_id: tnntten-root + updated_at: "2024-02-28T17:22:04Z" + updated_by: idntusr-bailin + super-admin: + value: + actions: + - role_list + - rolebinding_create + - loadbalancer_list + - loadbalancer_update + - loadbalancer_delete + - role_create + - role_delete + - loadbalancer_create + - loadbalancer_get + - rolebinding_list + - rolebinding_delete + - role_get + - role_update + created_at: "2024-02-28T17:22:04Z" + created_by: idntusr-bailin + id: permrv2-ecBlNMsPrvVFgUUAUfmeY + name: super_admin + resource_id: tnntten-root + updated_at: "2024-02-28T17:22:04Z" + updated_by: idntusr-bailin + parameters: + - name: id + in: path + required: true + schema: + type: string + example: tnntten-root + /roles/{role_id}: + get: + tags: + - roles + summary: get-role + description: get role by ID + operationId: getRole + responses: + "200": + description: get-super-user + content: + application/json: + schema: + type: object + properties: + actions: + type: array + items: + type: string + example: avail_role + example: + - avail_role + - doc_create + - doc_delete + - doc_get + - doc_list + - doc_update + - equinixwatch_streamconfig_create + - equinixwatch_streamconfig_delete + - equinixwatch_streamconfig_get + - equinixwatch_streamconfig_update + - iam_issuer_create + - iam_issuer_delete + - iam_issuer_get + - iam_issuer_update + - iam_oauthclient_create + - iam_oauthclient_delete + - iam_oauthclient_get + - iam_user_get + - iam_workload_federation_token_create + - ipamblock_create + - ipamblock_delete + - ipamblock_get + - ipamblock_update + - ipamblocktype_create + - ipamblocktype_delete + - ipamblocktype_get + - ipamblocktype_update + - loadbalancer_create + - loadbalancer_delete + - loadbalancer_get_history + - loadbalancer_get + - loadbalancer_update + - loadbalancerpool_create + - loadbalancerpool_delete + - loadbalancerpool_get_history + - loadbalancerpool_get + - loadbalancerpool_update + - loadbalancerprovider_create + - loadbalancerprovider_delete + - loadbalancerprovider_get_history + - loadbalancerprovider_get + - loadbalancerprovider_update + - location_create + - location_delete + - location_get + - location_update + - member + - metadata_annotationnamespace_update + - metadata_statusnamespace_update + - metal_organization_get + - metal_project_get + - owner + - parent_perm + - resource_provider_create + - resource_provider_delete + - resource_provider_get + - resource_provider_update + - role_create + - role_delete + - role_get + - role_list + - role_update + - rolebinding_create + - rolebinding_delete + - rolebinding_get + - rolebinding_list + - rolebinding_update + - tenant_create + - tenant_delete + - tenant_get + - tenant_list + - tenant_update + created_at: + type: string + example: "2024-04-11T19:16:38Z" + created_by: + type: string + example: idntusr-bailin + id: + type: string + example: permrv2-nDw3bVXYwHysvZDFyxh2C + name: + type: string + example: super_user + resource_id: + type: string + example: tnntten-root + updated_at: + type: string + example: "2024-04-11T19:18:22Z" + updated_by: + type: string + example: idntusr-bailin + examples: + get-super-user: + value: + actions: + - avail_role + - doc_create + - doc_delete + - doc_get + - doc_list + - doc_update + - equinixwatch_streamconfig_create + - equinixwatch_streamconfig_delete + - equinixwatch_streamconfig_get + - equinixwatch_streamconfig_update + - iam_issuer_create + - iam_issuer_delete + - iam_issuer_get + - iam_issuer_update + - iam_oauthclient_create + - iam_oauthclient_delete + - iam_oauthclient_get + - iam_user_get + - iam_workload_federation_token_create + - ipamblock_create + - ipamblock_delete + - ipamblock_get + - ipamblock_update + - ipamblocktype_create + - ipamblocktype_delete + - ipamblocktype_get + - ipamblocktype_update + - loadbalancer_create + - loadbalancer_delete + - loadbalancer_get_history + - loadbalancer_get + - loadbalancer_update + - loadbalancerpool_create + - loadbalancerpool_delete + - loadbalancerpool_get_history + - loadbalancerpool_get + - loadbalancerpool_update + - loadbalancerprovider_create + - loadbalancerprovider_delete + - loadbalancerprovider_get_history + - loadbalancerprovider_get + - loadbalancerprovider_update + - location_create + - location_delete + - location_get + - location_update + - member + - metadata_annotationnamespace_update + - metadata_statusnamespace_update + - metal_organization_get + - metal_project_get + - owner + - parent_perm + - resource_provider_create + - resource_provider_delete + - resource_provider_get + - resource_provider_update + - role_create + - role_delete + - role_get + - role_list + - role_update + - rolebinding_create + - rolebinding_delete + - rolebinding_get + - rolebinding_list + - rolebinding_update + - tenant_create + - tenant_delete + - tenant_get + - tenant_list + - tenant_update + created_at: "2024-04-11T19:16:38Z" + created_by: idntusr-bailin + id: permrv2-nDw3bVXYwHysvZDFyxh2C + name: super_user + resource_id: tnntten-root + updated_at: "2024-04-11T19:18:22Z" + updated_by: idntusr-bailin + delete: + tags: + - roles + summary: delete-role + description: | + delete role by ID, this will also remove any role-bindings that use + this role + operationId: deleteRole + responses: + "200": + description: "" + patch: + tags: + - roles + summary: update-role + description: | + update role by ID, both name and actions can be modified + operationId: updateRole + requestBody: + content: + application/json: + schema: + type: object + properties: + actions: + type: array + items: + type: string + example: doc_list + example: + - doc_list + - doc_update + - doc_delete + - secret_list + - rolebinding_update + - rolebinding_list + - doc_create + - role_update + - doc_get + - secret_get + - secret_update + - secret_delete + - rolebinding_get + - role_create + - role_delete + - role_list + - secret_create + - rolebinding_create + - rolebinding_delete + - avail_role + - role_get + name: + type: string + example: super_user + examples: + update-role: + value: + actions: + - doc_list + - doc_update + - doc_delete + - secret_list + - rolebinding_update + - rolebinding_list + - doc_create + - role_update + - doc_get + - secret_get + - secret_update + - secret_delete + - rolebinding_get + - role_create + - role_delete + - role_list + - secret_create + - rolebinding_create + - rolebinding_delete + - avail_role + - role_get + name: super_user + responses: + "200": + description: super-user + content: + application/json: + schema: + type: object + properties: + actions: + type: array + items: + type: string + example: doc_list + example: + - doc_list + - doc_update + - doc_delete + - secret_list + - rolebinding_update + - rolebinding_list + - doc_create + - role_update + - doc_get + - secret_get + - secret_update + - secret_delete + - rolebinding_get + - role_create + - role_delete + - role_list + - secret_create + - rolebinding_create + - rolebinding_delete + - avail_role + - role_get + created_at: + type: string + example: "2024-04-10T20:09:06Z" + created_by: + type: string + example: idntusr-bailin + id: + type: string + example: permrv2-_vKIY7KTwIABD0V9Qpnef + name: + type: string + example: super_user + resource_id: + type: string + example: tnntten-root + updated_at: + type: string + example: "2024-04-10T20:18:10Z" + updated_by: + type: string + example: idntusr-bailin + examples: + super-user: + value: + actions: + - doc_list + - doc_update + - doc_delete + - secret_list + - rolebinding_update + - rolebinding_list + - doc_create + - role_update + - doc_get + - secret_get + - secret_update + - secret_delete + - rolebinding_get + - role_create + - role_delete + - role_list + - secret_create + - rolebinding_create + - rolebinding_delete + - avail_role + - role_get + created_at: "2024-04-10T20:09:06Z" + created_by: idntusr-bailin + id: permrv2-_vKIY7KTwIABD0V9Qpnef + name: super_user + resource_id: tnntten-root + updated_at: "2024-04-10T20:18:10Z" + updated_by: idntusr-bailin + parameters: + - name: role_id + in: path + required: true + schema: + type: string + example: permrv2-zNYOpdL1RGyiSp0hgaIYB + /resources/{id}/role-bindings: + get: + tags: + - role-bindings + summary: list-role-bindings + description: | + list role-bindings for a resource. + an optional query parameter `role_id` can be used to filter the results. + operationId: listRoleBindings + parameters: + - name: role_id + in: query + schema: + type: string + example: permrv2-FQbFZMF74D0-WLNO-8MMb + responses: + "200": + description: list-role-bindings + content: + application/json: + schema: + type: object + properties: + data: + type: array + items: + type: object + properties: + id: + type: string + example: permrbn-r7WU4juT0p-76JW-sJwzQ + role: + type: object + properties: + id: + type: string + example: permrv2-nDw3bVXYwHysvZDFyxh2C + name: + type: string + example: super_user + subjects: + type: array + items: + type: object + properties: + id: + type: string + example: idntusr-bailin + type: + type: string + example: user + example: + - id: idntusr-bailin + type: user + example: + - id: permrbn-r7WU4juT0p-76JW-sJwzQ + role: + id: permrv2-nDw3bVXYwHysvZDFyxh2C + name: super_user + subjects: + - id: idntusr-bailin + type: user + - id: permrbn-wNpgDB-ajBTyR6nB-WBcY + role: + id: permrv2-nDw3bVXYwHysvZDFyxh2C + name: super_user + subjects: + - id: idntusr-bailin + type: user + - id: idntusr-bailin-1 + type: user + - id: idntusr-bailin-2 + type: user + - id: idntusr-bailin-3 + type: user + - id: idntusr-bailin-4 + type: user + - id: idntusr-bailin-5 + type: user + examples: + list-role-bindings: + value: + data: + - id: permrbn-r7WU4juT0p-76JW-sJwzQ + role: + id: permrv2-nDw3bVXYwHysvZDFyxh2C + name: super_user + subjects: + - id: idntusr-bailin + type: user + - id: permrbn-wNpgDB-ajBTyR6nB-WBcY + role: + id: permrv2-nDw3bVXYwHysvZDFyxh2C + name: super_user + subjects: + - id: idntusr-bailin + type: user + - id: idntusr-bailin-1 + type: user + - id: idntusr-bailin-2 + type: user + - id: idntusr-bailin-3 + type: user + - id: idntusr-bailin-4 + type: user + - id: idntusr-bailin-5 + type: user + post: + tags: + - role-bindings + summary: create-role-binding + description: | + create a role-binding for a resource. The role-binding will grant the + specified role to the specified subjects. + operationId: createRoleBinding + requestBody: + content: + application/json: + schema: + type: object + properties: + role_id: + type: string + example: permrv2-hoN9DD27g1tfKzjF8kj42 + subjects: + type: array + items: + type: object + properties: + conditions: + type: object + properties: {} + id: + type: string + example: idntgrp-my-subgroup + example: + - conditions: {} + id: idntgrp-my-subgroup + examples: + create-role-binding: + value: + role_id: permrv2-hoN9DD27g1tfKzjF8kj42 + subjects: + - conditions: {} + id: idntgrp-my-subgroup + responses: + "200": + description: root-super-user / create-role-binding-group + content: + application/json: + schema: + type: object + properties: + id: + type: string + example: permrbn-wNpgDB-ajBTyR6nB-WBcY + role: + type: object + properties: + id: + type: string + example: permrv2-nDw3bVXYwHysvZDFyxh2C + name: + type: string + example: super_user + subjects: + type: array + items: + type: object + properties: + id: + type: string + example: idntusr-bailin + type: + type: string + example: user + example: + - id: idntusr-bailin + type: user + - id: idntusr-bailin-1 + type: user + - id: idntusr-bailin-2 + type: user + - id: idntusr-bailin-3 + type: user + - id: idntusr-bailin-4 + type: user + - id: idntusr-bailin-5 + type: user + examples: + create-role-binding-group: + value: + id: permrbn-96Cy2kTlt3jtZwjEF9gaO + role: + id: permrv2-hoN9DD27g1tfKzjF8kj42 + name: my_proj_doc_viewer + subjects: + - id: idntgrp-my-subgroup + type: group + root-super-user: + value: + id: permrbn-wNpgDB-ajBTyR6nB-WBcY + role: + id: permrv2-nDw3bVXYwHysvZDFyxh2C + name: super_user + subjects: + - id: idntusr-bailin + type: user + - id: idntusr-bailin-1 + type: user + - id: idntusr-bailin-2 + type: user + - id: idntusr-bailin-3 + type: user + - id: idntusr-bailin-4 + type: user + - id: idntusr-bailin-5 + type: user + parameters: + - name: id + in: path + required: true + schema: + type: string + example: tnntten-root + /resources/{id}/role-bindings/{rb-id}: + get: + tags: + - role-bindings + summary: get-role-binding + description: get-role-binding + operationId: getRoleBinding + responses: + "200": + description: get-role-binding + content: + application/json: + schema: + type: object + properties: + id: + type: string + example: permrbn-pQByAAcMGUfS-ZvXPlrxp + role: + type: object + properties: + id: + type: string + example: permrv2-pUuZKw25TbCjoPnOYkA4w + name: + type: string + example: super-admin + subjects: + type: array + items: + type: object + properties: + id: + type: string + example: idntclt-xT33dia6DUHdcDXsGRUes + type: + type: string + example: client + example: + - id: idntclt-xT33dia6DUHdcDXsGRUes + type: client + - id: idntgrp-root-admins + type: group + - id: idntusr-DUrM80Cg2VzLVEhpmQb7ovmW1EG1isBnmIFpLTA5N2k + type: user + - id: idntusr-FYfXJww0qhW1XrF-WKupOQCQ9Q84d-ieEamjxz1Hzhs + type: user + - id: idntusr-bailin + type: user + examples: + get-role-binding: + value: + id: permrbn-pQByAAcMGUfS-ZvXPlrxp + role: + id: permrv2-pUuZKw25TbCjoPnOYkA4w + name: super-admin + subjects: + - id: idntclt-xT33dia6DUHdcDXsGRUes + type: client + - id: idntgrp-root-admins + type: group + - id: idntusr-DUrM80Cg2VzLVEhpmQb7ovmW1EG1isBnmIFpLTA5N2k + type: user + - id: idntusr-FYfXJww0qhW1XrF-WKupOQCQ9Q84d-ieEamjxz1Hzhs + type: user + - id: idntusr-bailin + type: user + delete: + tags: + - role-bindings + summary: delete-role-binding + description: delete-role-binding + operationId: deleteRoleBinding + responses: + "200": + description: "" + patch: + tags: + - role-bindings + summary: update-role-binding + description: | + update a role-binding, this will replace the subjects with the new + subjects. note that role_id is immutable + operationId: updateRoleBinding + parameters: + - name: id + in: query + schema: + type: string + example: loadbal-test-1 + requestBody: + content: + application/json: + schema: + type: object + properties: + subjects: + type: array + items: + type: object + properties: + conditions: + type: object + properties: {} + id: + type: string + example: idntclt-xT33dia6DUHdcDXsGRUes + example: + - id: idntclt-xT33dia6DUHdcDXsGRUes + - id: idntusr-DUrM80Cg2VzLVEhpmQb7ovmW1EG1isBnmIFpLTA5N2k + - id: idntusr-FYfXJww0qhW1XrF-WKupOQCQ9Q84d-ieEamjxz1Hzhs + - id: idntgrp-root-admins + examples: + update-role-binding: + value: + subjects: + - id: idntclt-xT33dia6DUHdcDXsGRUes + - id: idntusr-DUrM80Cg2VzLVEhpmQb7ovmW1EG1isBnmIFpLTA5N2k + - id: idntusr-FYfXJww0qhW1XrF-WKupOQCQ9Q84d-ieEamjxz1Hzhs + - id: idntgrp-root-admins + responses: + "200": + description: update-role-binding + headers: + Content-Length: + schema: + type: string + example: "368" + Date: + schema: + type: string + example: Mon, 22 Apr 2024 21:52:22 GMT + X-Request-Id: + schema: + type: string + example: NzmFDigPmfVANuPeLMlMFtpykGKhGwvj + content: + application/json: + schema: + type: object + properties: + id: + type: string + example: permrbn-pQByAAcMGUfS-ZvXPlrxp + role: + type: object + properties: + id: + type: string + example: permrv2-pUuZKw25TbCjoPnOYkA4w + name: + type: string + example: super-admin + subjects: + type: array + items: + type: object + properties: + id: + type: string + example: idntclt-xT33dia6DUHdcDXsGRUes + type: + type: string + example: client + example: + - id: idntclt-xT33dia6DUHdcDXsGRUes + type: client + - id: idntusr-DUrM80Cg2VzLVEhpmQb7ovmW1EG1isBnmIFpLTA5N2k + type: user + - id: idntusr-FYfXJww0qhW1XrF-WKupOQCQ9Q84d-ieEamjxz1Hzhs + type: user + - id: idntgrp-root-admins + type: group + examples: + update-role-binding: + value: + id: permrbn-pQByAAcMGUfS-ZvXPlrxp + role: + id: permrv2-pUuZKw25TbCjoPnOYkA4w + name: super-admin + subjects: + - id: idntclt-xT33dia6DUHdcDXsGRUes + type: client + - id: idntusr-DUrM80Cg2VzLVEhpmQb7ovmW1EG1isBnmIFpLTA5N2k + type: user + - id: idntusr-FYfXJww0qhW1XrF-WKupOQCQ9Q84d-ieEamjxz1Hzhs + type: user + - id: idntgrp-root-admins + type: group + parameters: + - name: id + in: path + required: true + schema: + type: string + example: loadbal-lb1 + - name: rb-id + in: path + required: true + schema: + type: string + example: permrbn-uRNlQEo6yh9DinKhSxL2z + /actions: + get: + summary: list-actions + description: list all available actions in the permission system + operationId: listActions + responses: + "200": + description: list-actions + content: + application/json: + schema: + type: array + items: + type: string + example: role_update + example: + - role_update + - role_delete + - loadbalancer_delete + - role_list + - role_get + - loadbalancer_create + - loadbalancer_update + - avail_role + - iam_rolebinding_get + - iam_rolebinding_list + - role_create + - loadbalancer_get + - loadbalancer_list + - iam_rolebinding_create + - iam_rolebinding_update + - iam_rolebinding_delete + examples: + list-actions: + value: + - role_update + - role_delete + - loadbalancer_delete + - role_list + - role_get + - loadbalancer_create + - loadbalancer_update + - avail_role + - iam_rolebinding_get + - iam_rolebinding_list + - role_create + - loadbalancer_get + - loadbalancer_list + - iam_rolebinding_create + - iam_rolebinding_update + - iam_rolebinding_delete +components: + securitySchemes: + oauth2: + type: oauth2 + flows: + clientCredentials: + tokenUrl: http://localhost:8081/default/token + scopes: + openid: openid + permissions-api: permissions-api +security: + - oauth2: + - 'openid permissions-api' +tags: + - name: roles + - name: role-bindings From 41421b28043d6b9301f36f8ea92c06551cc1f50a Mon Sep 17 00:00:00 2001 From: Bailin He Date: Mon, 22 Apr 2024 21:28:23 +0000 Subject: [PATCH 02/10] use only one write request for all the update operations Signed-off-by: Bailin He --- internal/query/rolebindings.go | 411 ++++++++++++++++----------------- 1 file changed, 202 insertions(+), 209 deletions(-) diff --git a/internal/query/rolebindings.go b/internal/query/rolebindings.go index debcc75e..07795e60 100644 --- a/internal/query/rolebindings.go +++ b/internal/query/rolebindings.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - "sync" pb "github.com/authzed/authzed-go/proto/authzed/api/v1" "go.infratographer.com/x/gidx" @@ -16,6 +15,83 @@ import ( "go.infratographer.com/permissions-api/internal/types" ) +func (e *engine) GetRoleBinding(ctx context.Context, roleBinding types.Resource) (types.RoleBinding, error) { + ctx, span := e.tracer.Start( + ctx, "engine.GetRoleBinding", + trace.WithAttributes(attribute.Stringer("role_binding_id", roleBinding.ID)), + ) + defer span.End() + + // gather all relationships from this role-binding + rbRelFilter := &pb.RelationshipFilter{ + ResourceType: e.namespaced(e.rbac.RoleBindingResource.Name), + OptionalResourceId: roleBinding.ID.String(), + } + + rbRel, err := e.readRelationships(ctx, rbRelFilter) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return types.RoleBinding{}, err + } + + if len(rbRel) < 1 { + err := fmt.Errorf("%w: role binding: %s", ErrRoleBindingNotFound, roleBinding.ID.String()) + + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return types.RoleBinding{}, err + } + + rb := types.RoleBinding{ + ID: roleBinding.ID, + Subjects: make([]types.RoleBindingSubject, 0, len(rbRel)), + } + + for _, rel := range rbRel { + // process subject relationships + if rel.Relation == iapl.RolebindingSubjectRelation { + subjectRes, err := e.NewResourceFromIDString(rel.Subject.Object.ObjectId) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return types.RoleBinding{}, err + } + + rb.Subjects = append(rb.Subjects, types.RoleBindingSubject{SubjectResource: subjectRes}) + + continue + } + + // process role relationships + roleID, err := gidx.Parse(rel.Subject.Object.ObjectId) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return types.RoleBinding{}, err + } + + dbRole, err := e.store.GetRoleByID(ctx, roleID) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return types.RoleBinding{}, err + } + + rb.Role = types.Role{ + ID: roleID, + Name: dbRole.Name, + } + } + + return rb, nil +} + func (e *engine) CreateRoleBinding(ctx context.Context, resource, roleResource types.Resource, subjects []types.RoleBindingSubject) (types.RoleBinding, error) { ctx, span := e.tracer.Start( ctx, "engine.CreateRoleBinding", @@ -116,30 +192,46 @@ func (e *engine) DeleteRoleBinding(ctx context.Context, rb, res types.Resource) ) defer span.End() - // delete relationships from the role-binding - if _, err := e.client.DeleteRelationships(ctx, &pb.DeleteRelationshipsRequest{ - RelationshipFilter: &pb.RelationshipFilter{ - ResourceType: e.namespaced(e.rbac.RoleBindingResource.Name), - OptionalResourceId: rb.ID.String(), - }, - }); err != nil { + // gather all relationships from the role-binding resource + fromRels, err := e.readRelationships(ctx, &pb.RelationshipFilter{ + ResourceType: e.namespaced(e.rbac.RoleBindingResource.Name), + OptionalResourceId: rb.ID.String(), + }) + if err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) return err } - // delete relationships to the role-binding - if _, err := e.client.DeleteRelationships(ctx, &pb.DeleteRelationshipsRequest{ - RelationshipFilter: &pb.RelationshipFilter{ - ResourceType: e.namespaced(res.Type), - OptionalRelation: iapl.GrantRelationship, - OptionalSubjectFilter: &pb.SubjectFilter{ - SubjectType: e.namespaced(e.rbac.RoleBindingResource.Name), - OptionalSubjectId: rb.ID.String(), - }, + // gather relationships to the role-binding + toRels, err := e.readRelationships(ctx, &pb.RelationshipFilter{ + ResourceType: e.namespaced(res.Type), + OptionalRelation: iapl.GrantRelationship, + OptionalSubjectFilter: &pb.SubjectFilter{ + SubjectType: e.namespaced(e.rbac.RoleBindingResource.Name), + OptionalSubjectId: rb.ID.String(), }, - }); err != nil { + }) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return err + } + + // create a list of delete updates for these relationships + updates := make([]*pb.RelationshipUpdate, len(fromRels)+len(toRels)) + + for i, rel := range append(fromRels, toRels...) { + updates[i] = &pb.RelationshipUpdate{ + Operation: pb.RelationshipUpdate_OPERATION_DELETE, + Relationship: rel, + } + } + + // apply changes + if _, err := e.client.WriteRelationships(ctx, &pb.WriteRelationshipsRequest{Updates: updates}); err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) @@ -176,68 +268,54 @@ func (e *engine) ListRoleBindings(ctx context.Context, resource types.Resource, } // 2. fetch role-binding details for each grant - bindings := make(chan types.RoleBinding, len(grantRel)) - errs := make(chan error, len(grantRel)) - wg := &sync.WaitGroup{} + bindings := make([]types.RoleBinding, 0, len(grantRel)) + errs := make([]error, 0, len(grantRel)) for _, rel := range grantRel { - wg.Add(1) + rbRes, err := e.NewResourceFromIDString(rel.Subject.Object.ObjectId) + if err != nil { + errs = append(errs, err) + continue + } - go func(grant *pb.Relationship) { - defer wg.Done() + rb, err := e.GetRoleBinding(ctx, rbRes) + if err != nil { + if errors.Is(err, ErrRoleBindingNotFound) { + // print and record a warning message when there's a grant points + // to a role-binding that not longer exists. + // + // this should not happen in normal circumstances, but it's possible + // if some role-binding relationships are deleted directly through + // spiceDB + err := fmt.Errorf("%w: dangling grant relationship: %s", err, rel.String()) - rbRes, err := e.NewResourceFromIDString(grant.Subject.Object.ObjectId) - if err != nil { - errs <- err - return + span.RecordError(err) + e.logger.Warnf(err.Error()) } - rb, err := e.fetchRoleBinding(ctx, rbRes) - if err != nil { - if errors.Is(err, ErrRoleBindingNotFound) { - // print and record a warning message when there's a grant points - // to a role-binding that not longer exists. - err := fmt.Errorf("%w: dangling grant relationship: %s", err, grant.String()) - - span.RecordError(err) - e.logger.Warnf(err.Error()) - - return - } - errs <- err + errs = append(errs, err) - return - } + continue + } - if optionalRole != nil && rb.Role.ID.String() != optionalRole.ID.String() { - return - } + if optionalRole != nil && rb.Role.ID.String() != optionalRole.ID.String() { + continue + } - if len(rb.Subjects) == 0 { - return - } + if len(rb.Subjects) == 0 { + continue + } - bindings <- rb - }(rel) + bindings = append(bindings, rb) } - wg.Wait() - close(errs) - close(bindings) - - for err := range errs { + for _, err := range errs { if err != nil { return nil, err } } - resp := make([]types.RoleBinding, 0, len(bindings)) - - for rb := range bindings { - resp = append(resp, rb) - } - - return resp, nil + return bindings, nil } func (e *engine) UpdateRoleBinding(ctx context.Context, rb types.Resource, subjects []types.RoleBindingSubject) (types.RoleBinding, error) { @@ -249,7 +327,7 @@ func (e *engine) UpdateRoleBinding(ctx context.Context, rb types.Resource, subje ) defer span.End() - rolebinding, err := e.fetchRoleBinding(ctx, rb) + rolebinding, err := e.GetRoleBinding(ctx, rb) if err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) @@ -280,28 +358,8 @@ func (e *engine) UpdateRoleBinding(ctx context.Context, rb types.Resource, subje updates := make([]*pb.RelationshipUpdate, len(add)+len(remove)) i := 0 - mkupdate := func(id string, op pb.RelationshipUpdate_Operation) (*pb.RelationshipUpdate, error) { - subjRes, err := e.NewResourceFromIDString(id) - if err != nil { - span.RecordError(err) - span.SetStatus(codes.Error, err.Error()) - - return nil, err - } - - rel, err := e.rolebindingSubjectRelationship(subjRes, rb.ID.String()) - if err != nil { - span.RecordError(err) - span.SetStatus(codes.Error, err.Error()) - - return nil, err - } - - return &pb.RelationshipUpdate{Operation: op, Relationship: rel}, nil - } - for _, id := range add { - update, err := mkupdate(id, pb.RelationshipUpdate_OPERATION_TOUCH) + update, err := e.rolebindingRelationshipUpdateForSubject(id, rb.ID.String(), pb.RelationshipUpdate_OPERATION_TOUCH) if err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) @@ -314,7 +372,7 @@ func (e *engine) UpdateRoleBinding(ctx context.Context, rb types.Resource, subje } for _, id := range remove { - update, err := mkupdate(id, pb.RelationshipUpdate_OPERATION_DELETE) + update, err := e.rolebindingRelationshipUpdateForSubject(id, rb.ID.String(), pb.RelationshipUpdate_OPERATION_DELETE) if err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) @@ -339,16 +397,6 @@ func (e *engine) UpdateRoleBinding(ctx context.Context, rb types.Resource, subje return rolebinding, nil } -func (e *engine) GetRoleBinding(ctx context.Context, rolebinding types.Resource) (types.RoleBinding, error) { - ctx, span := e.tracer.Start( - ctx, "engine.GetRoleBinding", - trace.WithAttributes(attribute.Stringer("role_binding_id", rolebinding.ID)), - ) - defer span.End() - - return e.fetchRoleBinding(ctx, rolebinding) -} - // isRoleBindable checks if a role is available for a resource. a role is not // be available to a resource if it is owner is not associated with the resource // in any way. @@ -392,8 +440,6 @@ func (e *engine) deleteRoleBindingsForRole(ctx context.Context, roleResource typ ) defer span.End() - requests := []*pb.DeleteRelationshipsRequest{} - // 1. find all the bindings for the role findBindingsFilter := &pb.RelationshipFilter{ ResourceType: e.namespaced(e.rbac.RoleBindingResource.Name), @@ -412,140 +458,68 @@ func (e *engine) deleteRoleBindingsForRole(ctx context.Context, roleResource typ return err } - // 2. build a list of delete request for the subject and role relationship - // at the same time build a list of delete requests for all the grant - // relationships for all bindable resources - for _, rb := range bindings { - delSubjReq := &pb.DeleteRelationshipsRequest{ - RelationshipFilter: &pb.RelationshipFilter{ - ResourceType: rb.Resource.ObjectType, - OptionalResourceId: rb.Resource.ObjectId, - }, - } + // 2. Gather all the relationships to be deleted - requests = append(requests, delSubjReq) + // 2.1 build a list of requests to get all the subject, role and grant + // relationships for all bindable resources + relFilters := []*pb.RelationshipFilter{} + + for _, rb := range bindings { + relFilters = append(relFilters, &pb.RelationshipFilter{ + ResourceType: rb.Resource.ObjectType, + OptionalResourceId: rb.Resource.ObjectId, + }, + ) for _, res := range e.rbacV2ResourceTypes { - delGrantReq := &pb.DeleteRelationshipsRequest{ - RelationshipFilter: &pb.RelationshipFilter{ - ResourceType: e.namespaced(res.Name), - OptionalRelation: iapl.GrantRelationship, - OptionalSubjectFilter: &pb.SubjectFilter{ - SubjectType: rb.Resource.ObjectType, - OptionalSubjectId: rb.Resource.ObjectId, - }, + relFilters = append(relFilters, &pb.RelationshipFilter{ + ResourceType: e.namespaced(res.Name), + OptionalRelation: iapl.GrantRelationship, + OptionalSubjectFilter: &pb.SubjectFilter{ + SubjectType: rb.Resource.ObjectType, + OptionalSubjectId: rb.Resource.ObjectId, }, - } - - requests = append(requests, delGrantReq) + }, + ) } } - e.logger.Debugf("%d delete requests created", len(requests)) - - // 3. delete all the relationships - wg := &sync.WaitGroup{} - errs := make(chan error, len(requests)) + // 2.2 read all the relationships + rels := []*pb.Relationship{} - for _, req := range requests { - wg.Add(1) - - go func(req *pb.DeleteRelationshipsRequest) { - defer wg.Done() - - if _, err := e.client.DeleteRelationships(ctx, req); err != nil { - errs <- err - } - }(req) - } - - wg.Wait() - close(errs) - - for err := range errs { + for _, filter := range relFilters { + r, err := e.readRelationships(ctx, filter) if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + return err } - } - - return nil -} - -func (e *engine) fetchRoleBinding(ctx context.Context, roleBinding types.Resource) (types.RoleBinding, error) { - ctx, span := e.tracer.Start( - ctx, "engine.fetchRoleBinding", - trace.WithAttributes(attribute.Stringer("role_binding_id", roleBinding.ID)), - ) - defer span.End() - // gather all relationships from this role-binding - rbRelFilter := &pb.RelationshipFilter{ - ResourceType: e.namespaced(e.rbac.RoleBindingResource.Name), - OptionalResourceId: roleBinding.ID.String(), + rels = append(rels, r...) } - rbRel, err := e.readRelationships(ctx, rbRelFilter) - if err != nil { - span.RecordError(err) - span.SetStatus(codes.Error, err.Error()) + // 2.3 create delete requests + updates := make([]*pb.RelationshipUpdate, len(rels)) - return types.RoleBinding{}, err + for i, rel := range rels { + updates[i] = &pb.RelationshipUpdate{ + Operation: pb.RelationshipUpdate_OPERATION_DELETE, + Relationship: rel, + } } - if len(rbRel) < 1 { - err := fmt.Errorf("%w: role binding: %s", ErrRoleBindingNotFound, roleBinding.ID.String()) + e.logger.Debugf("%d relationships will be deleted", len(updates)) + // 3. delete all the relationships + if _, err := e.client.WriteRelationships(ctx, &pb.WriteRelationshipsRequest{Updates: updates}); err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) - return types.RoleBinding{}, err - } - - rb := types.RoleBinding{ - ID: roleBinding.ID, - Subjects: make([]types.RoleBindingSubject, 0, len(rbRel)), - } - - for _, rel := range rbRel { - // process subject relationships - if rel.Relation == iapl.RolebindingSubjectRelation { - subjectRes, err := e.NewResourceFromIDString(rel.Subject.Object.ObjectId) - if err != nil { - span.RecordError(err) - span.SetStatus(codes.Error, err.Error()) - - return types.RoleBinding{}, err - } - - rb.Subjects = append(rb.Subjects, types.RoleBindingSubject{SubjectResource: subjectRes}) - - continue - } - - // process role relationships - roleID, err := gidx.Parse(rel.Subject.Object.ObjectId) - if err != nil { - span.RecordError(err) - span.SetStatus(codes.Error, err.Error()) - - return types.RoleBinding{}, err - } - - dbRole, err := e.store.GetRoleByID(ctx, roleID) - if err != nil { - span.RecordError(err) - span.SetStatus(codes.Error, err.Error()) - - return types.RoleBinding{}, err - } - - rb.Role = types.Role{ - ID: roleID, - Name: dbRole.Name, - } + return err } - return rb, nil + return nil } // rolebindingSubjectRelationship is a helper function that creates a @@ -621,6 +595,25 @@ func (e *engine) rolebindingGrantResourceRelationship(resource types.Resource, r return rel, nil } +// rolebindingRelationshipUpdateForSubject is a helper function that creates a +// relationship update that adds the given subject to a role-binding update +// request +func (e *engine) rolebindingRelationshipUpdateForSubject( + subjID, rolebindingID string, op pb.RelationshipUpdate_Operation, +) (*pb.RelationshipUpdate, error) { + subjRes, err := e.NewResourceFromIDString(subjID) + if err != nil { + return nil, err + } + + rel, err := e.rolebindingSubjectRelationship(subjRes, rolebindingID) + if err != nil { + return nil, err + } + + return &pb.RelationshipUpdate{Operation: op, Relationship: rel}, nil +} + func newRoleBindingWithPrefix(prefix string, role types.Role) types.RoleBinding { rb := types.RoleBinding{ ID: gidx.MustNewID(prefix), From c7b2205e093e9da99fcf0a917154a1cf584fab1c Mon Sep 17 00:00:00 2001 From: Bailin He Date: Tue, 23 Apr 2024 16:11:45 +0000 Subject: [PATCH 03/10] fix typo Signed-off-by: Bailin He --- internal/api/roles_v2.go | 2 +- internal/query/rolebindings.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/api/roles_v2.go b/internal/api/roles_v2.go index 27ac65d7..05151892 100644 --- a/internal/api/roles_v2.go +++ b/internal/api/roles_v2.go @@ -98,7 +98,7 @@ func (r *Router) roleV2Update(c echo.Context) error { return r.errorResponse("error creating resource", err) } - if err := r.checkActionWithResponse(ctx, subjectResource, string(iapl.RoleActionGet), roleResource); err != nil { + if err := r.checkActionWithResponse(ctx, subjectResource, string(iapl.RoleActionUpdate), roleResource); err != nil { return err } diff --git a/internal/query/rolebindings.go b/internal/query/rolebindings.go index 07795e60..39cf1108 100644 --- a/internal/query/rolebindings.go +++ b/internal/query/rolebindings.go @@ -398,7 +398,7 @@ func (e *engine) UpdateRoleBinding(ctx context.Context, rb types.Resource, subje } // isRoleBindable checks if a role is available for a resource. a role is not -// be available to a resource if it is owner is not associated with the resource +// available to a resource if its owner is not associated with the resource // in any way. func (e *engine) isRoleBindable(ctx context.Context, role, res types.Resource) error { req := &pb.CheckPermissionRequest{ From 91f09563611d66925f0f31c09b902cddddd08781 Mon Sep 17 00:00:00 2001 From: Bailin He Date: Thu, 25 Apr 2024 21:41:13 +0000 Subject: [PATCH 04/10] add rolebinding storage Signed-off-by: Bailin He --- cmd/createrole.go | 2 +- internal/api/rolebindings.go | 116 ++++--- internal/api/types.go | 12 +- internal/query/mock/mock.go | 11 +- internal/query/rolebindings.go | 279 +++++++++++++++- internal/query/rolebindings_test.go | 91 +++-- internal/query/roles_v2.go | 4 + internal/query/roles_v2_test.go | 12 +- internal/query/service.go | 9 +- internal/storage/errors.go | 3 + .../20240425000000_role_bindings.sql | 33 ++ internal/storage/rolebinding.go | 297 ++++++++++++++++ internal/storage/rolebinding_test.go | 316 ++++++++++++++++++ internal/storage/roles.go | 15 +- internal/storage/storage.go | 1 + internal/types/types.go | 12 +- 16 files changed, 1078 insertions(+), 135 deletions(-) create mode 100644 internal/storage/migrations/20240425000000_role_bindings.sql create mode 100644 internal/storage/rolebinding.go create mode 100644 internal/storage/rolebinding_test.go diff --git a/cmd/createrole.go b/cmd/createrole.go index f6a63a55..dff4dc6f 100644 --- a/cmd/createrole.go +++ b/cmd/createrole.go @@ -141,7 +141,7 @@ func createRole(ctx context.Context, cfg *config.AppConfig) { logger.Fatalw("error creating role resource", "error", err) } - rb, err := engine.CreateRoleBinding(ctx, resource, roleres, rbsubj) + rb, err := engine.CreateRoleBinding(ctx, subjectResource, resource, roleres, rbsubj) if err != nil { logger.Fatalw("error creating role binding", "error", err) } diff --git a/internal/api/rolebindings.go b/internal/api/rolebindings.go index 911990a3..db4566d2 100644 --- a/internal/api/rolebindings.go +++ b/internal/api/rolebindings.go @@ -3,6 +3,7 @@ package api import ( "fmt" "net/http" + "time" "github.com/labstack/echo/v4" "go.infratographer.com/x/gidx" @@ -52,13 +53,13 @@ func (r *Router) roleBindingCreate(c echo.Context) error { return r.errorResponse("error creating resource", err) } - subjectResource, err := r.currentSubject(c) + actor, err := r.currentSubject(c) if err != nil { return err } // permissions on role binding actions, similar to roles v1, are granted on the resources - if err := r.checkActionWithResponse(ctx, subjectResource, string(iapl.RoleBindingActionCreate), resource); err != nil { + if err := r.checkActionWithResponse(ctx, actor, string(iapl.RoleBindingActionCreate), resource); err != nil { return err } @@ -86,7 +87,7 @@ func (r *Router) roleBindingCreate(c echo.Context) error { } } - rb, err := r.engine.CreateRoleBinding(ctx, resource, roleResource, subjects) + rb, err := r.engine.CreateRoleBinding(ctx, actor, resource, roleResource, subjects) if err != nil { return r.errorResponse("error creating role-binding", err) } @@ -94,12 +95,18 @@ func (r *Router) roleBindingCreate(c echo.Context) error { return c.JSON( http.StatusOK, roleBindingResponse{ - ID: rb.ID, - Subjects: resourceToSubject(rb.Subjects), + ID: rb.ID, + ResourceID: rb.ResourceID, + Subjects: resourceToSubject(rb.Subjects), Role: roleBindingResponseRole{ ID: rb.Role.ID, Name: rb.Role.Name, }, + + CreatedBy: rb.CreatedBy, + UpdatedBy: rb.UpdatedBy, + CreatedAt: rb.CreatedAt.Format(time.RFC3339), + UpdatedAt: rb.UpdatedAt.Format(time.RFC3339), }, ) } @@ -160,12 +167,18 @@ func (r *Router) roleBindingsList(c echo.Context) error { for i, rb := range rbs { resp.Data[i] = roleBindingResponse{ - ID: rb.ID, - Subjects: resourceToSubject(rb.Subjects), + ID: rb.ID, + ResourceID: rb.ResourceID, + Subjects: resourceToSubject(rb.Subjects), Role: roleBindingResponseRole{ ID: rb.Role.ID, Name: rb.Role.Name, }, + + CreatedBy: rb.CreatedBy, + UpdatedBy: rb.UpdatedBy, + CreatedAt: rb.CreatedAt.Format(time.RFC3339), + UpdatedAt: rb.UpdatedAt.Format(time.RFC3339), } } @@ -173,7 +186,6 @@ func (r *Router) roleBindingsList(c echo.Context) error { } func (r *Router) roleBindingsDelete(c echo.Context) error { - resID := c.Param("id") rbID := c.Param("rb_id") ctx, span := tracer.Start( @@ -182,17 +194,6 @@ func (r *Router) roleBindingsDelete(c echo.Context) error { ) defer span.End() - // resource - resourceID, err := gidx.Parse(resID) - if err != nil { - return r.errorResponse("error parsing resource ID", fmt.Errorf("%w: %s", ErrInvalidID, err.Error())) - } - - resource, err := r.engine.NewResourceFromID(resourceID) - if err != nil { - return r.errorResponse("error creating resource", err) - } - // role-binding rolebindingID, err := gidx.Parse(rbID) if err != nil { @@ -209,12 +210,18 @@ func (r *Router) roleBindingsDelete(c echo.Context) error { return err } + // resource + resource, err := r.engine.GetRoleBindingResource(ctx, rbRes) + if err != nil { + return r.errorResponse("error getting role-binding owner resource", err) + } + // permissions on role binding actions, similar to roles v1, are granted on the resources if err := r.checkActionWithResponse(ctx, actor, string(iapl.RoleBindingActionDelete), resource); err != nil { return err } - if err := r.engine.DeleteRoleBinding(ctx, rbRes, resource); err != nil { + if err := r.engine.DeleteRoleBinding(ctx, rbRes); err != nil { return r.errorResponse("error updating role-binding", err) } @@ -224,7 +231,6 @@ func (r *Router) roleBindingsDelete(c echo.Context) error { } func (r *Router) roleBindingGet(c echo.Context) error { - resID := c.Param("id") rbID := c.Param("rb_id") ctx, span := tracer.Start( @@ -233,17 +239,6 @@ func (r *Router) roleBindingGet(c echo.Context) error { ) defer span.End() - // resource - resourceID, err := gidx.Parse(resID) - if err != nil { - return r.errorResponse("error parsing resource ID", fmt.Errorf("%w: %s", ErrInvalidID, err.Error())) - } - - resource, err := r.engine.NewResourceFromID(resourceID) - if err != nil { - return r.errorResponse("error creating resource", err) - } - // role-binding rolebindingID, err := gidx.Parse(rbID) if err != nil { @@ -260,50 +255,52 @@ func (r *Router) roleBindingGet(c echo.Context) error { return err } - // permissions on role binding actions, similar to roles v1, are granted on the resources - if err := r.checkActionWithResponse(ctx, actor, string(iapl.RoleBindingActionGet), resource); err != nil { - return err - } - rb, err := r.engine.GetRoleBinding(ctx, rbRes) if err != nil { return r.errorResponse("error getting role-binding", err) } + // permissions on role binding actions, similar to roles v1, are granted on the resources + // since the rolebinding is returning the resource ID that it belongs to, we + // will use this resource ID to check the permissions + resource, err := r.engine.NewResourceFromID(rb.ResourceID) + if err != nil { + return r.errorResponse("error creating resource", err) + } + + if err := r.checkActionWithResponse(ctx, actor, string(iapl.RoleBindingActionGet), resource); err != nil { + return err + } + return c.JSON( http.StatusOK, roleBindingResponse{ - ID: rb.ID, - Subjects: resourceToSubject(rb.Subjects), + ID: rb.ID, + ResourceID: rb.ResourceID, + Subjects: resourceToSubject(rb.Subjects), Role: roleBindingResponseRole{ ID: rb.Role.ID, Name: rb.Role.Name, }, + + CreatedBy: rb.CreatedBy, + UpdatedBy: rb.UpdatedBy, + CreatedAt: rb.CreatedAt.Format(time.RFC3339), + UpdatedAt: rb.UpdatedAt.Format(time.RFC3339), }, ) } func (r *Router) roleBindingUpdate(c echo.Context) error { - resID := c.Param("id") rbID := c.Param("rb_id") ctx, span := tracer.Start( c.Request().Context(), "api.roleBindingUpdate", - trace.WithAttributes(attribute.String("id", resID)), trace.WithAttributes(attribute.String("rolebinding_id", rbID)), ) defer span.End() // resource - resourceID, err := gidx.Parse(resID) - if err != nil { - return r.errorResponse("error parsing resource ID", fmt.Errorf("%w: %s", ErrInvalidID, err.Error())) - } - - resource, err := r.engine.NewResourceFromID(resourceID) - if err != nil { - return r.errorResponse("error creating resource", err) - } // role-binding rolebindingID, err := gidx.Parse(rbID) @@ -321,6 +318,12 @@ func (r *Router) roleBindingUpdate(c echo.Context) error { return err } + // resource + resource, err := r.engine.GetRoleBindingResource(ctx, rbRes) + if err != nil { + return r.errorResponse("error getting role-binding owner resource", err) + } + // permissions on role binding actions, similar to roles v1, are granted on the resources if err := r.checkActionWithResponse(ctx, actor, string(iapl.RoleBindingActionUpdate), resource); err != nil { return err @@ -347,7 +350,7 @@ func (r *Router) roleBindingUpdate(c echo.Context) error { } } - rb, err := r.engine.UpdateRoleBinding(ctx, rbRes, subjects) + rb, err := r.engine.UpdateRoleBinding(ctx, actor, rbRes, subjects) if err != nil { return r.errorResponse("error updating role-binding", err) } @@ -355,12 +358,19 @@ func (r *Router) roleBindingUpdate(c echo.Context) error { return c.JSON( http.StatusOK, roleBindingResponse{ - ID: rb.ID, - Subjects: resourceToSubject(rb.Subjects), + ID: rb.ID, + ResourceID: rb.ResourceID, + Subjects: resourceToSubject(rb.Subjects), + Role: roleBindingResponseRole{ ID: rb.Role.ID, Name: rb.Role.Name, }, + + CreatedBy: rb.CreatedBy, + UpdatedBy: rb.UpdatedBy, + CreatedAt: rb.CreatedAt.Format(time.RFC3339), + UpdatedAt: rb.UpdatedAt.Format(time.RFC3339), }, ) } diff --git a/internal/api/types.go b/internal/api/types.go index 380ffa7a..33e75627 100644 --- a/internal/api/types.go +++ b/internal/api/types.go @@ -106,9 +106,15 @@ type rolebindingUpdateRequest struct { } type roleBindingResponse struct { - ID gidx.PrefixedID `json:"id"` - Role roleBindingResponseRole `json:"role"` - Subjects []roleBindingSubject `json:"subjects"` + ID gidx.PrefixedID `json:"id"` + ResourceID gidx.PrefixedID `json:"resource_id"` + Role roleBindingResponseRole `json:"role"` + Subjects []roleBindingSubject `json:"subjects"` + + CreatedBy gidx.PrefixedID `json:"created_by"` + UpdatedBy gidx.PrefixedID `json:"updated_by"` + CreatedAt string `json:"created_at"` + UpdatedAt string `json:"updated_at"` } type listRoleBindingsResponse struct { diff --git a/internal/query/mock/mock.go b/internal/query/mock/mock.go index d64da981..71d42c40 100644 --- a/internal/query/mock/mock.go +++ b/internal/query/mock/mock.go @@ -204,7 +204,7 @@ func (e *Engine) SubjectHasPermission(context.Context, types.Resource, string, t } // CreateRoleBinding returns nothing but satisfies the Engine interface. -func (e *Engine) CreateRoleBinding(context.Context, types.Resource, types.Resource, []types.RoleBindingSubject) (types.RoleBinding, error) { +func (e *Engine) CreateRoleBinding(context.Context, types.Resource, types.Resource, types.Resource, []types.RoleBindingSubject) (types.RoleBinding, error) { return types.RoleBinding{}, nil } @@ -219,15 +219,20 @@ func (e *Engine) GetRoleBinding(context.Context, types.Resource) (types.RoleBind } // DeleteRoleBinding returns nothing but satisfies the Engine interface. -func (e *Engine) DeleteRoleBinding(context.Context, types.Resource, types.Resource) error { +func (e *Engine) DeleteRoleBinding(context.Context, 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) { +func (e *Engine) UpdateRoleBinding(context.Context, types.Resource, types.Resource, []types.RoleBindingSubject) (types.RoleBinding, error) { return types.RoleBinding{}, nil } +// GetRoleBindingResource returns nothing but satisfies the Engine interface. +func (e *Engine) GetRoleBindingResource(context.Context, types.Resource) (types.Resource, error) { + return types.Resource{}, nil +} + // AllActions returns nothing but satisfies the Engine interface. func (e *Engine) AllActions() []string { return nil diff --git a/internal/query/rolebindings.go b/internal/query/rolebindings.go index 39cf1108..a1c9f43e 100644 --- a/internal/query/rolebindings.go +++ b/internal/query/rolebindings.go @@ -12,6 +12,7 @@ import ( "go.opentelemetry.io/otel/trace" "go.infratographer.com/permissions-api/internal/iapl" + "go.infratographer.com/permissions-api/internal/storage" "go.infratographer.com/permissions-api/internal/types" ) @@ -22,6 +23,18 @@ func (e *engine) GetRoleBinding(ctx context.Context, roleBinding types.Resource) ) defer span.End() + rb, err := e.store.GetRoleBindingByID(ctx, roleBinding.ID) + if err != nil { + if errors.Is(err, storage.ErrRoleBindingNotFound) { + err = fmt.Errorf("%w: role-binding: %s", ErrRoleBindingNotFound, err) + } + + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return types.RoleBinding{}, err + } + // gather all relationships from this role-binding rbRelFilter := &pb.RelationshipFilter{ ResourceType: e.namespaced(e.rbac.RoleBindingResource.Name), @@ -45,10 +58,7 @@ func (e *engine) GetRoleBinding(ctx context.Context, roleBinding types.Resource) return types.RoleBinding{}, err } - rb := types.RoleBinding{ - ID: roleBinding.ID, - Subjects: make([]types.RoleBindingSubject, 0, len(rbRel)), - } + rb.Subjects = make([]types.RoleBindingSubject, 0, len(rbRel)) for _, rel := range rbRel { // process subject relationships @@ -92,7 +102,11 @@ func (e *engine) GetRoleBinding(ctx context.Context, roleBinding types.Resource) return rb, nil } -func (e *engine) CreateRoleBinding(ctx context.Context, resource, roleResource types.Resource, subjects []types.RoleBindingSubject) (types.RoleBinding, error) { +func (e *engine) CreateRoleBinding( + ctx context.Context, + actor, resource, roleResource types.Resource, + subjects []types.RoleBindingSubject, +) (types.RoleBinding, error) { ctx, span := e.tracer.Start( ctx, "engine.CreateRoleBinding", trace.WithAttributes( @@ -111,6 +125,10 @@ func (e *engine) CreateRoleBinding(ctx context.Context, resource, roleResource t dbrole, err := e.store.GetRoleByID(ctx, roleResource.ID) if err != nil { + if errors.Is(err, storage.ErrNoRoleFound) { + err = fmt.Errorf("%w: role %s", ErrRoleNotFound, roleResource.ID) + } + span.RecordError(err) span.SetStatus(codes.Error, err.Error()) @@ -122,6 +140,11 @@ func (e *engine) CreateRoleBinding(ctx context.Context, resource, roleResource t Name: dbrole.Name, } + dbCtx, err := e.store.BeginContext(ctx) + if err != nil { + return types.RoleBinding{}, nil + } + rbResourceType, ok := e.schemaTypeMap[e.rbac.RoleBindingResource.Name] if !ok { return types.RoleBinding{}, fmt.Errorf( @@ -130,13 +153,33 @@ func (e *engine) CreateRoleBinding(ctx context.Context, resource, roleResource t ) } - rb := newRoleBindingWithPrefix(rbResourceType.IDPrefix, role) + rbid, err := gidx.NewID(rbResourceType.IDPrefix) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) + + return types.RoleBinding{}, err + } + + rb, err := e.store.CreateRoleBinding(dbCtx, actor.ID, rbid, resource.ID) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) + + return types.RoleBinding{}, err + } + + rb.Role = role + roleRel := e.rolebindingRoleRelationship(role.ID.String(), rb.ID.String()) grantRel, err := e.rolebindingGrantResourceRelationship(resource, rb.ID.String()) if err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) + logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) return types.RoleBinding{}, err } @@ -160,6 +203,7 @@ func (e *engine) CreateRoleBinding(ctx context.Context, resource, roleResource t if err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) + logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) return types.RoleBinding{}, err } @@ -176,6 +220,16 @@ func (e *engine) CreateRoleBinding(ctx context.Context, resource, roleResource t }); err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) + logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) + + return types.RoleBinding{}, 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)) + logRollbackErr(e.logger, e.rollbackRoleBindingUpdates(ctx, updates)) return types.RoleBinding{}, err } @@ -183,7 +237,7 @@ func (e *engine) CreateRoleBinding(ctx context.Context, resource, roleResource t return rb, nil } -func (e *engine) DeleteRoleBinding(ctx context.Context, rb, res types.Resource) error { +func (e *engine) DeleteRoleBinding(ctx context.Context, rb types.Resource) error { ctx, span := e.tracer.Start( ctx, "engine.DeleteRoleBinding", trace.WithAttributes( @@ -192,6 +246,38 @@ func (e *engine) DeleteRoleBinding(ctx context.Context, rb, res types.Resource) ) defer span.End() + dbCtx, err := e.store.BeginContext(ctx) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return err + } + + if err := e.store.LockRoleBindingForUpdate(dbCtx, rb.ID); err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) + + return err + } + + rbFromDB, err := e.store.GetRoleBindingByID(dbCtx, rb.ID) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return err + } + + res, err := e.NewResourceFromID(rbFromDB.ResourceID) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return err + } + // gather all relationships from the role-binding resource fromRels, err := e.readRelationships(ctx, &pb.RelationshipFilter{ ResourceType: e.namespaced(e.rbac.RoleBindingResource.Name), @@ -200,6 +286,7 @@ func (e *engine) DeleteRoleBinding(ctx context.Context, rb, res types.Resource) if err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) + logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) return err } @@ -216,6 +303,7 @@ func (e *engine) DeleteRoleBinding(ctx context.Context, rb, res types.Resource) if err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) + logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) return err } @@ -234,6 +322,25 @@ func (e *engine) DeleteRoleBinding(ctx context.Context, rb, res types.Resource) if _, err := e.client.WriteRelationships(ctx, &pb.WriteRelationshipsRequest{Updates: updates}); err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) + logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) + + return err + } + + if err := e.store.DeleteRoleBinding(dbCtx, rb.ID); err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) + logRollbackErr(e.logger, e.rollbackRoleBindingUpdates(ctx, updates)) + + 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)) + logRollbackErr(e.logger, e.rollbackRoleBindingUpdates(ctx, updates)) return err } @@ -264,6 +371,9 @@ func (e *engine) ListRoleBindings(ctx context.Context, resource types.Resource, grantRel, err := e.readRelationships(ctx, listRbFilter) if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + return nil, err } @@ -311,6 +421,9 @@ func (e *engine) ListRoleBindings(ctx context.Context, resource types.Resource, for _, err := range errs { if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + return nil, err } } @@ -318,7 +431,7 @@ func (e *engine) ListRoleBindings(ctx context.Context, resource types.Resource, return bindings, nil } -func (e *engine) UpdateRoleBinding(ctx context.Context, rb types.Resource, subjects []types.RoleBindingSubject) (types.RoleBinding, error) { +func (e *engine) UpdateRoleBinding(ctx context.Context, actor, rb types.Resource, subjects []types.RoleBindingSubject) (types.RoleBinding, error) { ctx, span := e.tracer.Start( ctx, "engine.UpdateRoleBindings", trace.WithAttributes( @@ -327,10 +440,27 @@ func (e *engine) UpdateRoleBinding(ctx context.Context, rb types.Resource, subje ) defer span.End() - rolebinding, err := e.GetRoleBinding(ctx, rb) + dbCtx, err := e.store.BeginContext(ctx) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return types.RoleBinding{}, err + } + + if err := e.store.LockRoleBindingForUpdate(dbCtx, rb.ID); err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) + + return types.RoleBinding{}, err + } + + rolebinding, err := e.GetRoleBinding(dbCtx, rb) if err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) + logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) return types.RoleBinding{}, err } @@ -363,6 +493,7 @@ func (e *engine) UpdateRoleBinding(ctx context.Context, rb types.Resource, subje if err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) + logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) return types.RoleBinding{}, err } @@ -376,6 +507,7 @@ func (e *engine) UpdateRoleBinding(ctx context.Context, rb types.Resource, subje if err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) + logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) return types.RoleBinding{}, err } @@ -388,15 +520,49 @@ func (e *engine) UpdateRoleBinding(ctx context.Context, rb types.Resource, subje if err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) + logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) + + return types.RoleBinding{}, err + } + + // 3. update the role-binding in the database to record latest `updatedBy` and `updatedAt` + rbFromDB, err := e.store.UpdateRoleBinding(dbCtx, actor.ID, rb.ID) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) + logRollbackErr(e.logger, e.rollbackRoleBindingUpdates(ctx, updates)) + } + + if err := e.store.CommitContext(dbCtx); err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) + logRollbackErr(e.logger, e.rollbackRoleBindingUpdates(ctx, updates)) return types.RoleBinding{}, err } rolebinding.Subjects = subjects + rolebinding.UpdatedAt = rbFromDB.UpdatedAt + rolebinding.UpdatedBy = rbFromDB.UpdatedBy return rolebinding, nil } +func (e *engine) GetRoleBindingResource(ctx context.Context, rb types.Resource) (types.Resource, error) { + rbFromDB, err := e.store.GetRoleBindingByID(ctx, rb.ID) + if err != nil { + if errors.Is(err, storage.ErrRoleBindingNotFound) { + err = fmt.Errorf("%w: %s", ErrRoleBindingNotFound, err) + } + + return types.Resource{}, err + } + + return e.NewResourceFromID(rbFromDB.ResourceID) +} + // isRoleBindable checks if a role is available for a resource. a role is not // available to a resource if its owner is not associated with the resource // in any way. @@ -458,6 +624,40 @@ func (e *engine) deleteRoleBindingsForRole(ctx context.Context, roleResource typ return err } + if len(bindings) == 0 { + return nil + } + + rbIDs := make([]gidx.PrefixedID, len(bindings)) + + for i, rel := range bindings { + id, err := gidx.Parse(rel.Resource.ObjectId) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return err + } + + rbIDs[i] = id + } + + dbCtx, err := e.store.BeginContext(ctx) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return err + } + + if err := e.store.BatchLockRoleBindingForUpdate(dbCtx, rbIDs); err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) + + return err + } + // 2. Gather all the relationships to be deleted // 2.1 build a list of requests to get all the subject, role and grant @@ -492,6 +692,7 @@ func (e *engine) deleteRoleBindingsForRole(ctx context.Context, roleResource typ if err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) + logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) return err } @@ -511,10 +712,29 @@ func (e *engine) deleteRoleBindingsForRole(ctx context.Context, roleResource typ e.logger.Debugf("%d relationships will be deleted", len(updates)) - // 3. delete all the relationships + // 3.1 delete all records in permissions-api DB + if err := e.store.BatchDeleteRoleBindings(dbCtx, rbIDs); err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) + + return err + } + + // 3.2 delete all the relationships if _, err := e.client.WriteRelationships(ctx, &pb.WriteRelationshipsRequest{Updates: updates}); 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)) + logRollbackErr(e.logger, e.rollbackRoleBindingUpdates(ctx, updates)) return err } @@ -614,11 +834,40 @@ func (e *engine) rolebindingRelationshipUpdateForSubject( return &pb.RelationshipUpdate{Operation: op, Relationship: rel}, nil } -func newRoleBindingWithPrefix(prefix string, role types.Role) types.RoleBinding { - rb := types.RoleBinding{ - ID: gidx.MustNewID(prefix), - Role: role, +// rollbackRoleBindingUpdates is a helper function that rolls back a list of +// relationship updates on spiceDB. +func (e *engine) rollbackRoleBindingUpdates(ctx context.Context, updates []*pb.RelationshipUpdate) error { + updatesLen := len(updates) + rollbacks := make([]*pb.RelationshipUpdate, 0, updatesLen) + + for i := range updates { + // reversed order + u := updates[updatesLen-i-1] + + if u == nil { + continue + } + + var op pb.RelationshipUpdate_Operation + + switch u.Operation { + case pb.RelationshipUpdate_OPERATION_CREATE: + fallthrough + case pb.RelationshipUpdate_OPERATION_TOUCH: + op = pb.RelationshipUpdate_OPERATION_DELETE + case pb.RelationshipUpdate_OPERATION_DELETE: + op = pb.RelationshipUpdate_OPERATION_TOUCH + default: + continue + } + + rollbacks = append(rollbacks, &pb.RelationshipUpdate{ + Operation: op, + Relationship: u.Relationship, + }) } - return rb + _, err := e.client.WriteRelationships(ctx, &pb.WriteRelationshipsRequest{Updates: rollbacks}) + + return err } diff --git a/internal/query/rolebindings_test.go b/internal/query/rolebindings_test.go index 1422ef0e..33137c6a 100644 --- a/internal/query/rolebindings_test.go +++ b/internal/query/rolebindings_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/require" "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" ) @@ -41,10 +42,12 @@ func TestCreateRoleBinding(t *testing.T) { require.NoError(t, err) orphan, err := e.NewResourceFromIDString("tnntten-orphan") require.NoError(t, err) + subj, err := e.NewResourceFromIDString("idntusr-subj") + require.NoError(t, err) actor, err := e.NewResourceFromIDString("idntusr-actor") require.NoError(t, err) - role, err := e.CreateRoleV2(ctx, actor, root, "lb_viewer", []string{"loadbalancer_list", "loadbalancer_get"}) + role, err := e.CreateRoleV2(ctx, subj, root, "lb_viewer", []string{"loadbalancer_list", "loadbalancer_get"}) require.NoError(t, err) roleRes, err := e.NewResourceFromID(role.ID) @@ -73,10 +76,10 @@ func TestCreateRoleBinding(t *testing.T) { Input: input{ resource: root, role: notfoundRole, - subjects: []types.RoleBindingSubject{{SubjectResource: actor}}, + subjects: []types.RoleBindingSubject{{SubjectResource: subj}}, }, CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[types.RoleBinding]) { - assert.ErrorContains(t, res.Err, ErrRoleNotFound.Error()) + assert.ErrorIs(t, res.Err, ErrRoleNotFound) }, }, { @@ -84,10 +87,10 @@ func TestCreateRoleBinding(t *testing.T) { Input: input{ resource: root, role: v1role, - subjects: []types.RoleBindingSubject{{SubjectResource: actor}}, + subjects: []types.RoleBindingSubject{{SubjectResource: subj}}, }, CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[types.RoleBinding]) { - assert.ErrorContains(t, res.Err, ErrRoleNotFound.Error()) + assert.ErrorIs(t, res.Err, ErrRoleNotFound) }, }, { @@ -95,7 +98,7 @@ func TestCreateRoleBinding(t *testing.T) { Input: input{ resource: child, role: roleRes, - subjects: []types.RoleBindingSubject{{SubjectResource: actor}}, + subjects: []types.RoleBindingSubject{{SubjectResource: subj}}, }, CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[types.RoleBinding]) { assert.NoError(t, res.Err) @@ -112,10 +115,10 @@ func TestCreateRoleBinding(t *testing.T) { Input: input{ resource: orphan, role: roleRes, - subjects: []types.RoleBindingSubject{{SubjectResource: actor}}, + subjects: []types.RoleBindingSubject{{SubjectResource: subj}}, }, CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[types.RoleBinding]) { - assert.ErrorContains(t, res.Err, ErrRoleNotFound.Error()) + assert.ErrorIs(t, res.Err, ErrRoleNotFound) }, }, { @@ -123,23 +126,26 @@ func TestCreateRoleBinding(t *testing.T) { Input: input{ resource: root, role: roleRes, - subjects: []types.RoleBindingSubject{{SubjectResource: actor}}, + subjects: []types.RoleBindingSubject{{SubjectResource: subj}}, }, CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[types.RoleBinding]) { assert.NoError(t, res.Err) - assert.Equal(t, role.ID, res.Success.Role.ID) assert.Len(t, res.Success.Subjects, 1) + assert.Equal(t, role.ID, res.Success.Role.ID) + assert.Equal(t, root.ID, res.Success.ResourceID) + assert.Equal(t, subj.ID, res.Success.Subjects[0].SubjectResource.ID) + assert.Equal(t, actor.ID, res.Success.CreatedBy) - rb, err := e.ListRoleBindings(ctx, root, nil) + rbs, err := e.ListRoleBindings(ctx, root, nil) assert.NoError(t, err) - assert.Len(t, rb, 1) + assert.Len(t, rbs, 1) }, }, } testFn := func(ctx context.Context, in input) testingx.TestResult[types.RoleBinding] { - rb, err := e.CreateRoleBinding(ctx, in.resource, in.role, in.subjects) + rb, err := e.CreateRoleBinding(ctx, actor, in.resource, in.role, in.subjects) return testingx.TestResult[types.RoleBinding]{Success: rb, Err: err} } @@ -155,13 +161,15 @@ func TestListRoleBindings(t *testing.T) { require.NoError(t, err) child, err := e.NewResourceFromIDString("tnntten-child") require.NoError(t, err) + subj, err := e.NewResourceFromIDString("idntusr-subj") + require.NoError(t, err) actor, err := e.NewResourceFromIDString("idntusr-actor") require.NoError(t, err) - viewer, err := e.CreateRoleV2(ctx, actor, root, "lb_viewer", []string{"loadbalancer_list", "loadbalancer_get"}) + viewer, err := e.CreateRoleV2(ctx, subj, root, "lb_viewer", []string{"loadbalancer_list", "loadbalancer_get"}) require.NoError(t, err) - editor, err := e.CreateRoleV2(ctx, actor, root, "lb_editor", []string{"loadbalancer_list", "loadbalancer_get", "loadbalancer_create", "loadbalancer_update"}) + editor, err := e.CreateRoleV2(ctx, subj, root, "lb_editor", []string{"loadbalancer_list", "loadbalancer_get", "loadbalancer_create", "loadbalancer_update"}) require.NoError(t, err) viewerRes, err := e.NewResourceFromID(viewer.ID) @@ -173,10 +181,10 @@ func TestListRoleBindings(t *testing.T) { notfoundRole, err := e.NewResourceFromIDString("permrv2-notfound") require.NoError(t, err) - _, err = e.CreateRoleBinding(ctx, root, viewerRes, []types.RoleBindingSubject{{SubjectResource: actor}}) + _, err = e.CreateRoleBinding(ctx, actor, root, viewerRes, []types.RoleBindingSubject{{SubjectResource: subj}}) require.NoError(t, err) - _, err = e.CreateRoleBinding(ctx, root, editorRes, []types.RoleBindingSubject{{SubjectResource: actor}}) + _, err = e.CreateRoleBinding(ctx, actor, root, editorRes, []types.RoleBindingSubject{{SubjectResource: subj}}) require.NoError(t, err) _, err = e.client.WriteRelationships(ctx, &pb.WriteRelationshipsRequest{ @@ -257,10 +265,12 @@ func TestGetRoleBinding(t *testing.T) { root, err := e.NewResourceFromIDString("tnntten-root") require.NoError(t, err) + subj, err := e.NewResourceFromIDString("idntusr-subj") + require.NoError(t, err) actor, err := e.NewResourceFromIDString("idntusr-actor") require.NoError(t, err) - viewer, err := e.CreateRoleV2(ctx, actor, root, "lb_viewer", []string{"loadbalancer_list", "loadbalancer_get"}) + viewer, err := e.CreateRoleV2(ctx, subj, root, "lb_viewer", []string{"loadbalancer_list", "loadbalancer_get"}) require.NoError(t, err) viewerRes, err := e.NewResourceFromID(viewer.ID) @@ -269,7 +279,7 @@ func TestGetRoleBinding(t *testing.T) { notfoundRB, err := e.NewResourceFromIDString("permrbn-notfound") require.NoError(t, err) - rb, err := e.CreateRoleBinding(ctx, root, viewerRes, []types.RoleBindingSubject{{SubjectResource: actor}}) + rb, err := e.CreateRoleBinding(ctx, actor, root, viewerRes, []types.RoleBindingSubject{{SubjectResource: subj}}) require.NoError(t, err) rbRes, err := e.NewResourceFromID(rb.ID) @@ -283,7 +293,9 @@ func TestGetRoleBinding(t *testing.T) { assert.NoError(t, res.Err) assert.Equal(t, viewer.ID, res.Success.Role.ID) assert.Len(t, res.Success.Subjects, 1) - assert.Equal(t, actor.ID, res.Success.Subjects[0].SubjectResource.ID) + assert.Equal(t, subj.ID, res.Success.Subjects[0].SubjectResource.ID) + assert.Equal(t, actor.ID, res.Success.CreatedBy) + assert.Equal(t, root.ID, res.Success.ResourceID) }, }, { @@ -310,15 +322,17 @@ func TestUpdateRoleBinding(t *testing.T) { root, err := e.NewResourceFromIDString("tnntten-root") require.NoError(t, err) + subj, err := e.NewResourceFromIDString("idntusr-subj") + require.NoError(t, err) actor, err := e.NewResourceFromIDString("idntusr-actor") require.NoError(t, err) - viewer, err := e.CreateRoleV2(ctx, actor, root, "lb_viewer", []string{"loadbalancer_list", "loadbalancer_get"}) + viewer, err := e.CreateRoleV2(ctx, subj, root, "lb_viewer", []string{"loadbalancer_list", "loadbalancer_get"}) require.NoError(t, err) viewerRes, err := e.NewResourceFromID(viewer.ID) require.NoError(t, err) - rb, err := e.CreateRoleBinding(ctx, root, viewerRes, []types.RoleBindingSubject{{SubjectResource: actor}}) + rb, err := e.CreateRoleBinding(ctx, subj, root, viewerRes, []types.RoleBindingSubject{{SubjectResource: subj}}) require.NoError(t, err) rbRes, err := e.NewResourceFromID(rb.ID) require.NoError(t, err) @@ -342,11 +356,12 @@ func TestUpdateRoleBinding(t *testing.T) { Name: "UpdateRoleBindingNotFound", Input: input{ rb: notfoundRB, - subj: []types.RoleBindingSubject{{SubjectResource: actor}}, + subj: []types.RoleBindingSubject{{SubjectResource: subj}}, }, CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[types.RoleBinding]) { assert.ErrorContains(t, res.Err, ErrRoleBindingNotFound.Error()) }, + Sync: true, }, { Name: "UpdateRoleBindingInvalidSubject", @@ -357,6 +372,7 @@ func TestUpdateRoleBinding(t *testing.T) { CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[types.RoleBinding]) { assert.ErrorContains(t, res.Err, ErrInvalidArgument.Error()) }, + Sync: true, }, { Name: "UpdateRoleBindingSuccess", @@ -370,13 +386,18 @@ func TestUpdateRoleBinding(t *testing.T) { assert.Len(t, res.Success.Subjects, 2) assert.Contains(t, res.Success.Subjects, types.RoleBindingSubject{SubjectResource: user1}) assert.Contains(t, res.Success.Subjects, types.RoleBindingSubject{SubjectResource: group1}) - assert.NotContains(t, res.Success.Subjects, types.RoleBindingSubject{SubjectResource: actor}) + assert.NotContains(t, res.Success.Subjects, types.RoleBindingSubject{SubjectResource: subj}) + + assert.Equal(t, actor.ID, res.Success.UpdatedBy) + assert.Equal(t, root.ID, res.Success.ResourceID) + assert.Equal(t, subj.ID, res.Success.CreatedBy) }, + Sync: true, }, } testFn := func(ctx context.Context, in input) testingx.TestResult[types.RoleBinding] { - rb, err := e.UpdateRoleBinding(ctx, in.rb, in.subj) + rb, err := e.UpdateRoleBinding(ctx, actor, in.rb, in.subj) return testingx.TestResult[types.RoleBinding]{Success: rb, Err: err} } @@ -398,7 +419,7 @@ func TestDeleteRoleBinding(t *testing.T) { viewerRes, err := e.NewResourceFromID(viewer.ID) require.NoError(t, err) - rb, err := e.CreateRoleBinding(ctx, root, viewerRes, []types.RoleBindingSubject{{SubjectResource: actor}}) + rb, err := e.CreateRoleBinding(ctx, actor, root, viewerRes, []types.RoleBindingSubject{{SubjectResource: actor}}) require.NoError(t, err) rbRes, err := e.NewResourceFromID(rb.ID) require.NoError(t, err) @@ -411,7 +432,7 @@ func TestDeleteRoleBinding(t *testing.T) { Name: "DeleteRoleBindingNotFound", Input: notfoundRB, CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[types.RoleBinding]) { - assert.NoError(t, res.Err) + assert.ErrorIs(t, res.Err, storage.ErrRoleBindingNotFound) rb, err := e.ListRoleBindings(ctx, root, nil) assert.NoError(t, err) @@ -434,7 +455,7 @@ func TestDeleteRoleBinding(t *testing.T) { } testFn := func(ctx context.Context, in types.Resource) testingx.TestResult[types.RoleBinding] { - err := e.DeleteRoleBinding(ctx, in, root) + err := e.DeleteRoleBinding(ctx, in) return testingx.TestResult[types.RoleBinding]{Err: err} } @@ -510,7 +531,7 @@ func TestPermissions(t *testing.T) { }) require.Error(t, err) - _, err = e.CreateRoleBinding(ctx, lb1, viewerRes, []types.RoleBindingSubject{{SubjectResource: user1}}) + _, err = e.CreateRoleBinding(ctx, user1, lb1, viewerRes, []types.RoleBindingSubject{{SubjectResource: user1}}) require.NoError(t, err) return ctx @@ -528,7 +549,7 @@ func TestPermissions(t *testing.T) { rbs, _ := e.ListRoleBindings(ctx, lb1, nil) for _, rb := range rbs { rbRes, _ := e.NewResourceFromID(rb.ID) - _ = e.DeleteRoleBinding(ctx, rbRes, lb1) + _ = e.DeleteRoleBinding(ctx, rbRes) } }, Sync: true, @@ -544,7 +565,7 @@ func TestPermissions(t *testing.T) { }) require.Error(t, err) - _, err = e.CreateRoleBinding(ctx, child, viewerRes, []types.RoleBindingSubject{{SubjectResource: user1}}) + _, err = e.CreateRoleBinding(ctx, user1, child, viewerRes, []types.RoleBindingSubject{{SubjectResource: user1}}) require.NoError(t, err) return ctx @@ -562,7 +583,7 @@ func TestPermissions(t *testing.T) { rbs, _ := e.ListRoleBindings(ctx, child, nil) for _, rb := range rbs { rbRes, _ := e.NewResourceFromID(rb.ID) - _ = e.DeleteRoleBinding(ctx, rbRes, child) + _ = e.DeleteRoleBinding(ctx, rbRes) } }, Sync: true, @@ -578,7 +599,7 @@ func TestPermissions(t *testing.T) { }) require.Error(t, err) - _, err = e.CreateRoleBinding(ctx, root, viewerRes, []types.RoleBindingSubject{{SubjectResource: user1}}) + _, err = e.CreateRoleBinding(ctx, user1, root, viewerRes, []types.RoleBindingSubject{{SubjectResource: user1}}) require.NoError(t, err) return ctx @@ -596,7 +617,7 @@ func TestPermissions(t *testing.T) { rbs, _ := e.ListRoleBindings(ctx, root, nil) for _, rb := range rbs { rbRes, _ := e.NewResourceFromID(rb.ID) - _ = e.DeleteRoleBinding(ctx, rbRes, root) + _ = e.DeleteRoleBinding(ctx, rbRes) } }, Sync: true, @@ -612,7 +633,7 @@ func TestPermissions(t *testing.T) { }) require.Error(t, err) - _, err = e.CreateRoleBinding(ctx, root, viewerRes, []types.RoleBindingSubject{{SubjectResource: group1}}) + _, err = e.CreateRoleBinding(ctx, user1, root, viewerRes, []types.RoleBindingSubject{{SubjectResource: group1}}) require.NoError(t, err) return ctx diff --git a/internal/query/roles_v2.go b/internal/query/roles_v2.go index f22adf98..83211db8 100644 --- a/internal/query/roles_v2.go +++ b/internal/query/roles_v2.go @@ -51,6 +51,10 @@ func (e *engine) CreateRoleV2(ctx context.Context, actor, owner types.Resource, dbRole, err := e.store.CreateRole(dbCtx, actor.ID, role.ID, roleName, owner.ID) if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) + return types.Role{}, err } diff --git a/internal/query/roles_v2_test.go b/internal/query/roles_v2_test.go index 0726e0fc..f09eb022 100644 --- a/internal/query/roles_v2_test.go +++ b/internal/query/roles_v2_test.go @@ -409,10 +409,12 @@ func TestDeleteRolesV2(t *testing.T) { require.NoError(t, err) theotherchild, err := e.NewResourceFromIDString("tnntten-theotherchild") require.NoError(t, err) + subj, err := e.NewResourceFromIDString("idntusr-subj") + require.NoError(t, err) actor, err := e.NewResourceFromIDString("idntusr-actor") require.NoError(t, err) - role, err := e.CreateRoleV2(ctx, actor, root, "lb_viewer", []string{"loadbalancer_list", "loadbalancer_get"}) + role, err := e.CreateRoleV2(ctx, subj, root, "lb_viewer", []string{"loadbalancer_list", "loadbalancer_get"}) require.NoError(t, err) roleRes, err := e.NewResourceFromID(role.ID) @@ -432,13 +434,13 @@ func TestDeleteRolesV2(t *testing.T) { require.NoError(t, err) // these bindings are expected to be deleted after the role is deleted - _, err = e.CreateRoleBinding(ctx, root, roleRes, []types.RoleBindingSubject{{SubjectResource: actor}}) + _, err = e.CreateRoleBinding(ctx, actor, root, roleRes, []types.RoleBindingSubject{{SubjectResource: subj}}) require.NoError(t, err) - _, err = e.CreateRoleBinding(ctx, child, roleRes, []types.RoleBindingSubject{{SubjectResource: actor}}) + _, err = e.CreateRoleBinding(ctx, actor, child, roleRes, []types.RoleBindingSubject{{SubjectResource: subj}}) require.NoError(t, err) - _, err = e.CreateRoleBinding(ctx, theotherchild, roleRes, []types.RoleBindingSubject{{SubjectResource: actor}}) + _, err = e.CreateRoleBinding(ctx, actor, theotherchild, roleRes, []types.RoleBindingSubject{{SubjectResource: subj}}) require.NoError(t, err) rb, err := e.ListRoleBindings(ctx, root, &roleRes) @@ -464,7 +466,7 @@ func TestDeleteRolesV2(t *testing.T) { }, { Name: "DeleteRoleInvalidInput", - Input: actor, + Input: subj, CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[types.Role]) { assert.Error(t, res.Err) }, diff --git a/internal/query/service.go b/internal/query/service.go index 80c68f84..dadb860d 100644 --- a/internal/query/service.go +++ b/internal/query/service.go @@ -61,16 +61,19 @@ type Engine interface { // CreateRoleBinding creates all the necessary relationships for a role binding. // role binding here establishes a three-way relationship between a role, // a resource, and the subjects. - CreateRoleBinding(ctx context.Context, resource, role types.Resource, subjects []types.RoleBindingSubject) (types.RoleBinding, error) + CreateRoleBinding(ctx context.Context, actor, resource, role types.Resource, subjects []types.RoleBindingSubject) (types.RoleBinding, error) // ListRoleBindings lists all role-bindings for a resource, an optional Role // can be provided to filter the role-bindings. ListRoleBindings(ctx context.Context, resource types.Resource, optionalRole *types.Resource) ([]types.RoleBinding, error) // GetRoleBinding fetches a role-binding by its ID. GetRoleBinding(ctx context.Context, rolebinding types.Resource) (types.RoleBinding, error) // UpdateRoleBinding updates the subjects of a role-binding. - UpdateRoleBinding(ctx context.Context, rolebinding types.Resource, subjects []types.RoleBindingSubject) (types.RoleBinding, error) + UpdateRoleBinding(ctx context.Context, actor, rolebinding types.Resource, subjects []types.RoleBindingSubject) (types.RoleBinding, error) // DeleteRoleBinding removes subjects from a role-binding. - DeleteRoleBinding(ctx context.Context, rolebinding, resource types.Resource) error + DeleteRoleBinding(ctx context.Context, rolebinding types.Resource) error + // GetRoleBindingResource fetches the resource to which a role-binding + // belongs + GetRoleBindingResource(ctx context.Context, rb types.Resource) (types.Resource, error) AllActions() []string } diff --git a/internal/storage/errors.go b/internal/storage/errors.go index 111134fb..61c00464 100644 --- a/internal/storage/errors.go +++ b/internal/storage/errors.go @@ -25,6 +25,9 @@ var ( // ErrorInvalidContextTx represents an error where the given context transaction is of the wrong type. ErrorInvalidContextTx = errors.New("invalid type for transaction context") + + // ErrRoleBindingNotFound is returned when no role binding is found when retrieving or deleting a role binding. + ErrRoleBindingNotFound = errors.New("role binding not found") ) const ( diff --git a/internal/storage/migrations/20240425000000_role_bindings.sql b/internal/storage/migrations/20240425000000_role_bindings.sql new file mode 100644 index 00000000..9a9ce9cf --- /dev/null +++ b/internal/storage/migrations/20240425000000_role_bindings.sql @@ -0,0 +1,33 @@ +-- +goose Up + +-- create "rolebindings" table +CREATE TABLE "rolebindings" ( + "id" character varying NOT NULL, + "resource_id" character varying NOT NULL, + "created_by" character varying NOT NULL, + "updated_by" character varying NOT NULL, + "created_at" timestamptz NOT NULL, + "updated_at" timestamptz NOT NULL, + PRIMARY KEY ("id") +); + +-- create index "rolebindings_created_by" to table: "rolebindings" +CREATE INDEX "rolebindings_created_by" ON "rolebindings" ("created_by"); +-- create index "rolebindings_created_by" to table: "rolebindings" +CREATE INDEX "rolebindings_updated_by" ON "rolebindings" ("updated_by"); +-- create index "rolebindings_created_at" to table: "rolebindings" +CREATE INDEX "rolebindings_created_at" ON "rolebindings" ("created_at"); +-- create index "rolebindings_updated_at" to table: "rolebindings" +CREATE INDEX "rolebindings_updated_at" ON "rolebindings" ("updated_at"); + +-- +goose Down +-- reverse: create index "rolebindings_updated_at" to table: "rolebindings" +DROP INDEX "rolebindings_updated_at"; +-- reverse: create index "rolebindings_created_at" to table: "rolebindings" +DROP INDEX "rolebindings_created_at"; +-- reverse: create index "rolebindings_updated_by" to table: "rolebindings" +DROP INDEX "rolebindings_updated_by"; +-- reverse: create index "rolebindings_created_by" to table: "rolebindings" +DROP INDEX "rolebindings_created_by"; +-- reverse: create "rolebindings" table +DROP TABLE "rolebindings"; diff --git a/internal/storage/rolebinding.go b/internal/storage/rolebinding.go new file mode 100644 index 00000000..906464ce --- /dev/null +++ b/internal/storage/rolebinding.go @@ -0,0 +1,297 @@ +package storage + +import ( + "context" + "database/sql" + "errors" + "fmt" + + "go.infratographer.com/permissions-api/internal/types" + + "go.infratographer.com/x/gidx" +) + +// RoleBindingService represents a service for managing role bindings in the +// permissions API storage +type RoleBindingService interface { + // ListResourceRoleBindings returns all role bindings for a given resource + // an empty slice is returned if no role bindings are found + ListResourceRoleBindings(ctx context.Context, resourceID gidx.PrefixedID) ([]types.RoleBinding, error) + + // GetRoleBindingByID returns a role binding by its prefixed ID + // an ErrRoleBindingNotFound error is returned if no role binding is found + GetRoleBindingByID(ctx context.Context, id gidx.PrefixedID) (types.RoleBinding, error) + + // CreateRoleBinding creates a new role binding in the database + // This method must be called with a context returned from BeginContext. + // CommitContext or RollbackContext must be called afterwards if this method returns no error. + CreateRoleBinding(ctx context.Context, actorID, rbID, resourceID gidx.PrefixedID) (types.RoleBinding, error) + + // UpdateRoleBinding updates a role binding in the database + // Note that this method only updates the updated_at and updated_by fields + // and do not provide a way to update the resource_id field. + // + // This method must be called with a context returned from BeginContext. + // CommitContext or RollbackContext must be called afterwards if this method returns no error. + UpdateRoleBinding(ctx context.Context, actorID, rbID gidx.PrefixedID) (types.RoleBinding, error) + + // DeleteRoleBinding deletes a role binding from the database + // This method must be called with a context returned from BeginContext. + // CommitContext or RollbackContext must be called afterwards if this method returns no error. + DeleteRoleBinding(ctx context.Context, id gidx.PrefixedID) error + + // BatchDeleteRoleBinding deletes multiple role bindings from the database + // This method must be called with a context returned from BeginContext. + // CommitContext or RollbackContext must be called afterwards if this method returns no error. + BatchDeleteRoleBindings(ctx context.Context, ids []gidx.PrefixedID) error + + // LockRoleBindingForUpdate locks a role binding record to be updated to ensure consistency. + // If the role binding is not found, an ErrRoleBindingNotFound error is returned. + LockRoleBindingForUpdate(ctx context.Context, id gidx.PrefixedID) error + + // BatchLockRoleBindingForUpdate locks multiple role binding records to be updated to ensure consistency. + BatchLockRoleBindingForUpdate(ctx context.Context, ids []gidx.PrefixedID) error +} + +func (e *engine) GetRoleBindingByID(ctx context.Context, id gidx.PrefixedID) (types.RoleBinding, error) { + db, err := getContextDBQuery(ctx, e) + if err != nil { + return types.RoleBinding{}, err + } + + var roleBinding types.RoleBinding + + err = db.QueryRowContext(ctx, ` + SELECT id, resource_id, created_by, updated_by, created_at, updated_at + FROM rolebindings WHERE id = $1 + `, id.String(), + ).Scan( + &roleBinding.ID, + &roleBinding.ResourceID, + &roleBinding.CreatedBy, + &roleBinding.UpdatedBy, + &roleBinding.CreatedAt, + &roleBinding.UpdatedAt, + ) + if err != nil { + if errors.Is(err, sql.ErrNoRows) { + return types.RoleBinding{}, fmt.Errorf("%w: %s", ErrRoleBindingNotFound, id.String()) + } + + return types.RoleBinding{}, fmt.Errorf("%w: %s", err, id.String()) + } + + return roleBinding, nil +} + +func (e *engine) ListResourceRoleBindings(ctx context.Context, resourceID gidx.PrefixedID) ([]types.RoleBinding, error) { + db, err := getContextDBQuery(ctx, e) + if err != nil { + return nil, err + } + + rows, err := db.QueryContext(ctx, ` + SELECT id, resource_id, created_by, updated_by, created_at, updated_at + FROM rolebindings WHERE resource_id = $1 + `, resourceID.String(), + ) + if err != nil { + return nil, fmt.Errorf("%w: %s", err, resourceID.String()) + } + defer rows.Close() + + var roleBindings []types.RoleBinding + + for rows.Next() { + var roleBinding types.RoleBinding + + err = rows.Scan( + &roleBinding.ID, + &roleBinding.ResourceID, + &roleBinding.CreatedBy, + &roleBinding.UpdatedBy, + &roleBinding.CreatedAt, + &roleBinding.UpdatedAt, + ) + if err != nil { + return nil, fmt.Errorf("%w: %s", err, resourceID.String()) + } + + roleBindings = append(roleBindings, roleBinding) + } + + return roleBindings, nil +} + +func (e *engine) CreateRoleBinding(ctx context.Context, actorID, rbID, resourceID gidx.PrefixedID) (types.RoleBinding, error) { + tx, err := getContextTx(ctx) + if err != nil { + return types.RoleBinding{}, err + } + + var rb types.RoleBinding + + err = tx.QueryRowContext(ctx, ` + INSERT INTO rolebindings (id, resource_id, created_by, updated_by, created_at, updated_at) + VALUES ($1, $2, $3, $3, now(), now()) + RETURNING id, resource_id, created_by, updated_by, created_at, updated_at + `, rbID.String(), resourceID.String(), actorID.String(), + ).Scan( + &rb.ID, + &rb.ResourceID, + &rb.CreatedBy, + &rb.UpdatedBy, + &rb.CreatedAt, + &rb.UpdatedAt, + ) + if err != nil { + return types.RoleBinding{}, fmt.Errorf("%w: %s", err, rbID.String()) + } + + return rb, nil +} + +func (e *engine) UpdateRoleBinding(ctx context.Context, actorID, rbID gidx.PrefixedID) (types.RoleBinding, error) { + tx, err := getContextTx(ctx) + if err != nil { + return types.RoleBinding{}, err + } + + var rb types.RoleBinding + + err = tx.QueryRowContext(ctx, ` + UPDATE rolebindings + SET updated_by = $1, updated_at = now() + WHERE id = $2 + RETURNING id, resource_id, created_by, updated_by, created_at, updated_at + `, + actorID.String(), rbID.String(), + ).Scan( + &rb.ID, + &rb.ResourceID, + &rb.CreatedBy, + &rb.UpdatedBy, + &rb.CreatedAt, + &rb.UpdatedAt, + ) + if err != nil { + if errors.Is(err, sql.ErrNoRows) { + return types.RoleBinding{}, fmt.Errorf("%w: %s", ErrRoleBindingNotFound, rbID.String()) + } + + return types.RoleBinding{}, fmt.Errorf("%w: %s", err, rbID.String()) + } + + return rb, nil +} + +func (e *engine) DeleteRoleBinding(ctx context.Context, id gidx.PrefixedID) error { + tx, err := getContextTx(ctx) + if err != nil { + return err + } + + result, err := tx.ExecContext(ctx, ` + DELETE FROM rolebindings WHERE id = $1 + `, id.String(), + ) + if err != nil { + return fmt.Errorf("%w: %s", err, id.String()) + } + + rowsAffected, err := result.RowsAffected() + if err != nil { + return fmt.Errorf("%w: %s", err, id.String()) + } + + if rowsAffected == 0 { + return fmt.Errorf("%w: %s", ErrRoleBindingNotFound, id.String()) + } + + return nil +} + +func (e *engine) BatchDeleteRoleBindings(ctx context.Context, ids []gidx.PrefixedID) error { + tx, err := getContextTx(ctx) + if err != nil { + return err + } + + inClause, args := e.buildBatchInClauseWithIDs(ids) + q := fmt.Sprintf("DELETE FROM rolebindings WHERE id IN (%s)", inClause) + + _, err = tx.ExecContext(ctx, q, args...) + if err != nil { + return err + } + + return nil +} + +func (e *engine) LockRoleBindingForUpdate(ctx context.Context, id gidx.PrefixedID) error { + db, err := getContextDBQuery(ctx, e) + if err != nil { + return err + } + + result, err := db.ExecContext(ctx, `SELECT 1 FROM rolebindings WHERE id = $1 FOR UPDATE`, id.String()) + if err != nil { + return err + } + + rowsAffected, err := result.RowsAffected() + if err != nil { + return err + } + + if rowsAffected == 0 { + return ErrRoleBindingNotFound + } + + return nil +} + +func (e *engine) BatchLockRoleBindingForUpdate(ctx context.Context, ids []gidx.PrefixedID) error { + db, err := getContextDBQuery(ctx, e) + if err != nil { + return err + } + + inClause, args := e.buildBatchInClauseWithIDs(ids) + q := fmt.Sprintf("SELECT 1 FROM rolebindings WHERE id IN (%s) FOR UPDATE", inClause) + + result, err := db.ExecContext(ctx, q, args...) + if err != nil { + return err + } + + rowsAffected, err := result.RowsAffected() + if err != nil { + return err + } + + if int(rowsAffected) != len(ids) { + return fmt.Errorf("%w: %d of role-bindings not found", ErrRoleBindingNotFound, len(ids)-int(rowsAffected)) + } + + return nil +} + +// buildBatchInClauseWithIDs is a helper function that builds an IN clause for +// a batch query with the provided prefixed IDs. +func (e *engine) buildBatchInClauseWithIDs(ids []gidx.PrefixedID) (clause string, args []any) { + args = make([]any, len(ids)) + + for i, id := range ids { + fmtStr := "$%d" + + if i > 0 { + fmtStr = ", $%d" + } + + clause += fmt.Sprintf(fmtStr, i+1) + args[i] = id.String() + } + + return clause, args +} diff --git a/internal/storage/rolebinding_test.go b/internal/storage/rolebinding_test.go new file mode 100644 index 00000000..2d3e99c6 --- /dev/null +++ b/internal/storage/rolebinding_test.go @@ -0,0 +1,316 @@ +package storage_test + +import ( + "context" + "testing" + + "go.infratographer.com/permissions-api/internal/storage" + "go.infratographer.com/permissions-api/internal/storage/teststore" + "go.infratographer.com/permissions-api/internal/testingx" + "go.infratographer.com/permissions-api/internal/types" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.infratographer.com/x/gidx" +) + +func TestGetRoleBindingByID(t *testing.T) { + store, closeStore := teststore.NewTestStorage(t) + t.Cleanup(closeStore) + + ctx := context.Background() + actorID := gidx.PrefixedID("idntusr-user") + resourceID := gidx.PrefixedID("tentten-tenant") + rbID := gidx.MustNewID("permrbn") + + dbCtx, err := store.BeginContext(ctx) + require.NoError(t, err, "no error expected beginning transaction context") + + rb, err := store.CreateRoleBinding(dbCtx, actorID, rbID, resourceID) + require.NoError(t, err, "no error expected creating role binding") + + err = store.CommitContext(dbCtx) + require.NoError(t, err, "no error expected committing transaction context") + + tc := []testingx.TestCase[gidx.PrefixedID, types.RoleBinding]{ + { + Name: "NotFound", + Input: "permrbn-definitely_not_exists", + CheckFn: func(_ context.Context, t *testing.T, res testingx.TestResult[types.RoleBinding]) { + require.ErrorIs(t, res.Err, storage.ErrRoleBindingNotFound, "expected error to be role binding not found") + assert.ErrorIs(t, res.Err, storage.ErrRoleBindingNotFound) + require.Empty(t, res.Success.ID) + }, + }, + { + Name: "ok", + Input: rbID, + CheckFn: func(_ context.Context, t *testing.T, res testingx.TestResult[types.RoleBinding]) { + require.NoError(t, res.Err, "no error expected") + + assert.Equal(t, rb.ID, res.Success.ID) + assert.Equal(t, rb.CreatedAt, res.Success.CreatedAt) + assert.Equal(t, rb.UpdatedAt, res.Success.UpdatedAt) + assert.Equal(t, rb.CreatedBy, res.Success.CreatedBy) + assert.Equal(t, rb.UpdatedBy, res.Success.UpdatedBy) + }, + }, + } + + testfn := func(ctx context.Context, input gidx.PrefixedID) testingx.TestResult[types.RoleBinding] { + rb, err := store.GetRoleBindingByID(ctx, input) + + return testingx.TestResult[types.RoleBinding]{Success: rb, Err: err} + } + + testingx.RunTests(ctx, t, tc, testfn) +} + +func TestListResourceRoleBindings(t *testing.T) { + store, closeStore := teststore.NewTestStorage(t) + t.Cleanup(closeStore) + + ctx := context.Background() + actorID := gidx.PrefixedID("idntusr-user") + resourceID := gidx.PrefixedID("tentten-tenant") + + rbIDs := []gidx.PrefixedID{ + gidx.MustNewID("permrbn"), + gidx.MustNewID("permrbn"), + } + + rbs := map[gidx.PrefixedID]types.RoleBinding{} + + dbCtx, err := store.BeginContext(ctx) + require.NoError(t, err, "no error expected beginning transaction context") + + for _, rbID := range rbIDs { + rbs[rbID], err = store.CreateRoleBinding(dbCtx, actorID, rbID, resourceID) + require.NoError(t, err, "no error expected creating role binding") + } + + err = store.CommitContext(dbCtx) + require.NoError(t, err, "no error expected committing transaction context") + + tc := []testingx.TestCase[gidx.PrefixedID, []types.RoleBinding]{ + { + Name: "NotFound", + Input: "tentten-definitely_not_exists", + CheckFn: func(_ context.Context, t *testing.T, res testingx.TestResult[[]types.RoleBinding]) { + assert.NoError(t, res.Err, "no error expected") + assert.Len(t, res.Success, 0, "an empty list is expected") + }, + }, + { + Name: "ok", + Input: resourceID, + CheckFn: func(_ context.Context, t *testing.T, res testingx.TestResult[[]types.RoleBinding]) { + assert.NoError(t, res.Err, "no error expected") + assert.Len(t, res.Success, len(rbs), "expected number of role bindings") + + for _, rb := range res.Success { + assert.Equal(t, rb.ID, rbs[rb.ID].ID) + assert.Equal(t, rb.CreatedAt, rbs[rb.ID].CreatedAt) + assert.Equal(t, rb.UpdatedAt, rbs[rb.ID].UpdatedAt) + assert.Equal(t, rb.CreatedBy, rbs[rb.ID].CreatedBy) + assert.Equal(t, rb.UpdatedBy, rbs[rb.ID].UpdatedBy) + } + }, + }, + } + + testfn := func(ctx context.Context, input gidx.PrefixedID) testingx.TestResult[[]types.RoleBinding] { + rb, err := store.ListResourceRoleBindings(ctx, input) + + return testingx.TestResult[[]types.RoleBinding]{Success: rb, Err: err} + } + + testingx.RunTests(ctx, t, tc, testfn) +} + +func TestCreateRoleBinding(t *testing.T) { + store, closeStore := teststore.NewTestStorage(t) + t.Cleanup(closeStore) + + ctx := context.Background() + actorID := gidx.PrefixedID("idntusr-user") + resourceID := gidx.PrefixedID("tentten-tenant") + rbID := gidx.MustNewID("permrbn") + + tc := []testingx.TestCase[gidx.PrefixedID, types.RoleBinding]{ + { + Name: "ok", + Input: rbID, + CheckFn: func(_ context.Context, t *testing.T, res testingx.TestResult[types.RoleBinding]) { + require.NoError(t, res.Err, "no error expected") + + assert.Equal(t, rbID, res.Success.ID) + assert.NotZero(t, res.Success.CreatedAt, "expected created at to be set") + assert.NotZero(t, res.Success.UpdatedAt, "expected updated at to be set") + assert.Equal(t, actorID, res.Success.CreatedBy) + assert.Equal(t, actorID, res.Success.UpdatedBy) + }, + Sync: true, + }, + { + Name: "IDConflict", + Input: rbID, + CheckFn: func(_ context.Context, t *testing.T, res testingx.TestResult[types.RoleBinding]) { + assert.Error(t, res.Err) + require.Empty(t, res.Success.ID) + }, + Sync: true, + }, + } + + testfn := func(ctx context.Context, input gidx.PrefixedID) testingx.TestResult[types.RoleBinding] { + result := testingx.TestResult[types.RoleBinding]{} + + dbCtx, err := store.BeginContext(ctx) + if err != nil { + result.Err = err + + return result + } + + result.Success, result.Err = store.CreateRoleBinding(dbCtx, actorID, input, resourceID) + if result.Err != nil { + store.RollbackContext(dbCtx) //nolint:errcheck // skip check in test + + return result + } + + result.Err = store.CommitContext(dbCtx) + + return result + } + + testingx.RunTests(ctx, t, tc, testfn) +} + +func TestUpdateRoleBinding(t *testing.T) { + store, closeStore := teststore.NewTestStorage(t) + t.Cleanup(closeStore) + + ctx := context.Background() + actorID := gidx.PrefixedID("idntusr-user") + theOtherGuy := gidx.PrefixedID("idntusr-the_other_guy") + resourceID := gidx.PrefixedID("tentten-tenant") + rbID := gidx.MustNewID("permrbn") + + dbCtx, err := store.BeginContext(ctx) + require.NoError(t, err, "no error expected beginning transaction context") + + _, err = store.CreateRoleBinding(dbCtx, actorID, rbID, resourceID) + require.NoError(t, err, "no error expected creating role binding") + + err = store.CommitContext(dbCtx) + require.NoError(t, err, "no error expected committing transaction context") + + tc := []testingx.TestCase[gidx.PrefixedID, types.RoleBinding]{ + { + Name: "ok", + Input: rbID, + CheckFn: func(_ context.Context, t *testing.T, res testingx.TestResult[types.RoleBinding]) { + require.NoError(t, res.Err, "no error expected") + + assert.Equal(t, rbID, res.Success.ID) + assert.NotZero(t, res.Success.CreatedAt, "expected created at to be set") + assert.NotZero(t, res.Success.UpdatedAt, "expected updated at to be set") + assert.Equal(t, actorID, res.Success.CreatedBy) + assert.Equal(t, theOtherGuy, res.Success.UpdatedBy) + }, + }, + { + Name: "NotFound", + Input: "permrbn-definitely_not_exists", + CheckFn: func(_ context.Context, t *testing.T, res testingx.TestResult[types.RoleBinding]) { + assert.ErrorIs(t, res.Err, storage.ErrRoleBindingNotFound) + require.Empty(t, res.Success.ID) + }, + }, + } + + testfn := func(ctx context.Context, input gidx.PrefixedID) testingx.TestResult[types.RoleBinding] { + result := testingx.TestResult[types.RoleBinding]{} + + dbCtx, err := store.BeginContext(ctx) + if err != nil { + result.Err = err + + return result + } + + result.Success, result.Err = store.UpdateRoleBinding(dbCtx, theOtherGuy, input) + if result.Err != nil { + store.RollbackContext(dbCtx) //nolint:errcheck // skip check in + return result + } + + result.Err = store.CommitContext(dbCtx) + + return result + } + + testingx.RunTests(ctx, t, tc, testfn) +} + +func TestDeleteRoleBinding(t *testing.T) { + store, closeStore := teststore.NewTestStorage(t) + t.Cleanup(closeStore) + + ctx := context.Background() + actorID := gidx.PrefixedID("idntusr-user") + resourceID := gidx.PrefixedID("tentten-tenant") + rbID := gidx.MustNewID("permrbn") + + dbCtx, err := store.BeginContext(ctx) + require.NoError(t, err, "no error expected beginning transaction context") + + _, err = store.CreateRoleBinding(dbCtx, actorID, rbID, resourceID) + require.NoError(t, err, "no error expected creating role binding") + + err = store.CommitContext(dbCtx) + require.NoError(t, err, "no error expected committing transaction context") + + tc := []testingx.TestCase[gidx.PrefixedID, error]{ + { + Name: "ok", + Input: rbID, + CheckFn: func(_ context.Context, t *testing.T, res testingx.TestResult[error]) { + assert.NoError(t, res.Err, "no error expected") + }, + }, + { + Name: "NotFound", + Input: "permrbn-definitely_not_exists", + CheckFn: func(_ context.Context, t *testing.T, res testingx.TestResult[error]) { + assert.ErrorIs(t, res.Err, storage.ErrRoleBindingNotFound) + }, + }, + } + + testfn := func(ctx context.Context, input gidx.PrefixedID) testingx.TestResult[error] { + result := testingx.TestResult[error]{} + + dbCtx, err := store.BeginContext(ctx) + if err != nil { + result.Err = err + + return result + } + + result.Err = store.DeleteRoleBinding(dbCtx, input) + if result.Err != nil { + store.RollbackContext(dbCtx) //nolint:errcheck // skip check in test + + return result + } + + result.Err = store.CommitContext(dbCtx) + + return result + } + + testingx.RunTests(ctx, t, tc, testfn) +} diff --git a/internal/storage/roles.go b/internal/storage/roles.go index f65ffe16..767d063f 100644 --- a/internal/storage/roles.go +++ b/internal/storage/roles.go @@ -313,20 +313,7 @@ func (e *engine) BatchGetRoleByID(ctx context.Context, ids []gidx.PrefixedID) ([ 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() - } - + inClause, args := e.buildBatchInClauseWithIDs(ids) q := fmt.Sprintf(` SELECT id, name, resource_id, diff --git a/internal/storage/storage.go b/internal/storage/storage.go index ea1d64dc..95b60dc9 100644 --- a/internal/storage/storage.go +++ b/internal/storage/storage.go @@ -11,6 +11,7 @@ import ( // Storage defines the interface the engine exposes. type Storage interface { RoleService + RoleBindingService TransactionManager HealthCheck(ctx context.Context) error diff --git a/internal/types/types.go b/internal/types/types.go index 52d2f55e..b22ca92e 100644 --- a/internal/types/types.go +++ b/internal/types/types.go @@ -100,7 +100,13 @@ type Relationship struct { // RoleBinding represents a role binding between a role and a resource. type RoleBinding struct { - ID gidx.PrefixedID - Role Role - Subjects []RoleBindingSubject + ID gidx.PrefixedID + ResourceID gidx.PrefixedID + Role Role + Subjects []RoleBindingSubject + + CreatedBy gidx.PrefixedID + UpdatedBy gidx.PrefixedID + CreatedAt time.Time + UpdatedAt time.Time } From 98f22bc7fd38a65d15d42026a61800ca74ded5c0 Mon Sep 17 00:00:00 2001 From: Bailin He <15058035+bailinhe@users.noreply.github.com> Date: Fri, 26 Apr 2024 17:22:17 -0400 Subject: [PATCH 05/10] Apply suggestions from code review Co-authored-by: John Schaeffer Signed-off-by: Bailin He <15058035+bailinhe@users.noreply.github.com> --- cmd/createrole.go | 45 +++++----------- internal/api/rolebindings.go | 24 ++------- internal/api/router.go | 6 +-- internal/api/types.go | 7 +-- internal/query/relations.go | 56 ++++++++++++++++++++ internal/query/rolebindings.go | 97 ++++++++++++---------------------- openapi-v2.yaml | 8 +-- 7 files changed, 113 insertions(+), 130 deletions(-) diff --git a/cmd/createrole.go b/cmd/createrole.go index dff4dc6f..4beb9145 100644 --- a/cmd/createrole.go +++ b/cmd/createrole.go @@ -23,7 +23,6 @@ const ( createRoleFlagResource = "resource" createRoleFlagActions = "actions" createRoleFlagName = "name" - createRoleFlagIsV2 = "v2" ) var createRoleCmd = &cobra.Command{ @@ -42,7 +41,6 @@ func init() { flags.StringSlice(createRoleFlagActions, []string{}, "actions to assign to created role") flags.String(createRoleFlagResource, "", "resource to bind to created role") flags.String(createRoleFlagName, "", "name of role to create") - flags.Bool(createRoleFlagIsV2, false, "create a v2 role") v := viper.GetViper() @@ -50,7 +48,6 @@ func init() { viperx.MustBindFlag(v, createRoleFlagActions, flags.Lookup(createRoleFlagActions)) viperx.MustBindFlag(v, createRoleFlagResource, flags.Lookup(createRoleFlagResource)) viperx.MustBindFlag(v, createRoleFlagName, flags.Lookup(createRoleFlagName)) - viperx.MustBindFlag(v, createRoleFlagIsV2, flags.Lookup(createRoleFlagIsV2)) } func createRole(ctx context.Context, cfg *config.AppConfig) { @@ -58,7 +55,6 @@ func createRole(ctx context.Context, cfg *config.AppConfig) { actions := viper.GetStringSlice(createRoleFlagActions) resourceIDStr := viper.GetString(createRoleFlagResource) name := viper.GetString(createRoleFlagName) - v2 := viper.GetBool(createRoleFlagIsV2) if subjectIDStr == "" || len(actions) == 0 || resourceIDStr == "" || name == "" { logger.Fatal("invalid config") @@ -128,35 +124,22 @@ func createRole(ctx context.Context, cfg *config.AppConfig) { logger.Fatalw("error creating subject resource", "error", err) } - if v2 { - role, err := engine.CreateRoleV2(ctx, subjectResource, resource, name, actions) - if err != nil { - logger.Fatalw("error creating role", "error", err) - } - - rbsubj := []types.RoleBindingSubject{{SubjectResource: subjectResource}} - - roleres, err := engine.NewResourceFromID(role.ID) - if err != nil { - logger.Fatalw("error creating role resource", "error", err) - } - - rb, err := engine.CreateRoleBinding(ctx, subjectResource, resource, roleres, rbsubj) - if err != nil { - logger.Fatalw("error creating role binding", "error", err) - } + role, err := engine.CreateRoleV2(ctx, subjectResource, resource, name, actions) + if err != nil { + logger.Fatalw("error creating role", "error", err) + } - logger.Infof("created role %s[%s] and role-binding %s", role.Name, role.ID, rb.ID) - } else { - role, err := engine.CreateRole(ctx, subjectResource, resource, name, actions) - if err != nil { - logger.Fatalw("error creating role", "error", err) - } + rbsubj := []types.RoleBindingSubject{{SubjectResource: subjectResource}} - if err = engine.AssignSubjectRole(ctx, subjectResource, role); err != nil { - logger.Fatalw("error creating role", "error", err) - } + roleres, err := engine.NewResourceFromID(role.ID) + if err != nil { + logger.Fatalw("error creating role resource", "error", err) + } - logger.Infow("role successfully created", "role_id", role.ID) + rb, err := engine.CreateRoleBinding(ctx, subjectResource, resource, roleres, rbsubj) + if err != nil { + logger.Fatalw("error creating role binding", "error", err) } + + logger.Infof("created role %s[%s] and role-binding %s", role.Name, role.ID, rb.ID) } diff --git a/internal/api/rolebindings.go b/internal/api/rolebindings.go index db4566d2..feb67bb5 100644 --- a/internal/api/rolebindings.go +++ b/internal/api/rolebindings.go @@ -18,9 +18,8 @@ func resourceToSubject(subjects []types.RoleBindingSubject) []roleBindingSubject resp := make([]roleBindingSubject, len(subjects)) for i, subj := range subjects { resp[i] = roleBindingSubject{ - ID: subj.SubjectResource.ID, - Type: subj.SubjectResource.Type, - Condition: nil, + ID: subj.SubjectResource.ID, + Type: subj.SubjectResource.Type, } } @@ -113,7 +112,6 @@ func (r *Router) roleBindingCreate(c echo.Context) error { func (r *Router) roleBindingsList(c echo.Context) error { resourceIDStr := c.Param("id") - roleIDStr := c.QueryParam("role_id") ctx, span := tracer.Start( c.Request().Context(), "api.roleBindingList", @@ -140,23 +138,7 @@ func (r *Router) roleBindingsList(c echo.Context) error { return err } - roleFilter := (*types.Resource)(nil) - - if roleIDStr != "" { - roleID, err := gidx.Parse(roleIDStr) - if err != nil { - return r.errorResponse("error parsing role ID", fmt.Errorf("%w: %s", ErrInvalidID, err.Error())) - } - - roleResource, err := r.engine.NewResourceFromID(roleID) - if err != nil { - return r.errorResponse("error creating role resource", err) - } - - roleFilter = &roleResource - } - - rbs, err := r.engine.ListRoleBindings(ctx, resource, roleFilter) + rbs, err := r.engine.ListRoleBindings(ctx, resource, nil) if err != nil { return r.errorResponse("error listing role-binding", err) } diff --git a/internal/api/router.go b/internal/api/router.go index 39160255..ceaca65c 100644 --- a/internal/api/router.go +++ b/internal/api/router.go @@ -87,10 +87,10 @@ func (r *Router) Routes(rg *echo.Group) { v2.DELETE("/roles/:id", r.roleV2Delete) v2.GET("/resources/:id/role-bindings", r.roleBindingsList) - v2.GET("/resources/:id/role-bindings/:rb_id", r.roleBindingGet) v2.POST("/resources/:id/role-bindings", r.roleBindingCreate) - v2.DELETE("/resources/:id/role-bindings/:rb_id", r.roleBindingsDelete) - v2.PATCH("/resources/:id/role-bindings/:rb_id", r.roleBindingUpdate) + v2.GET("/role-bindings/:rb_id", r.roleBindingGet) + v2.DELETE("/role-bindings/:rb_id", r.roleBindingsDelete) + v2.PATCH("/role-bindings/:rb_id", r.roleBindingUpdate) v2.GET("/actions", r.listActions) } diff --git a/internal/api/types.go b/internal/api/types.go index 33e75627..0bf128eb 100644 --- a/internal/api/types.go +++ b/internal/api/types.go @@ -88,12 +88,9 @@ type roleBindingResponseRole struct { Name string `json:"name"` } -type roleBindingSubjectCondition struct{} - type roleBindingSubject struct { - ID gidx.PrefixedID `json:"id" binding:"required"` - Type string `json:"type,omitempty"` - Condition *roleBindingSubjectCondition `json:"condition,omitempty"` + ID gidx.PrefixedID `json:"id" binding:"required"` + Type string `json:"type,omitempty"` } type roleBindingRequest struct { diff --git a/internal/query/relations.go b/internal/query/relations.go index ddbfbc7b..40a844cd 100644 --- a/internal/query/relations.go +++ b/internal/query/relations.go @@ -1231,3 +1231,59 @@ func (e *engine) NewResourceFromIDString(id string) (types.Resource, error) { return subject, nil } + +// rollbackUpdates is a helper function that rolls back a list of +// relationship updates on spiceDB. +func (e *engine) rollbackUpdates(ctx context.Context, updates []*pb.RelationshipUpdate) error { + updatesLen := len(updates) + rollbacks := make([]*pb.RelationshipUpdate, 0, updatesLen) + + for i := range updates { + // reversed order + u := updates[updatesLen-i-1] + + if u == nil { + continue + } + + var op pb.RelationshipUpdate_Operation + + switch u.Operation { + case pb.RelationshipUpdate_OPERATION_CREATE: + fallthrough + case pb.RelationshipUpdate_OPERATION_TOUCH: + op = pb.RelationshipUpdate_OPERATION_DELETE + case pb.RelationshipUpdate_OPERATION_DELETE: + op = pb.RelationshipUpdate_OPERATION_TOUCH + default: + continue + } + + rollbacks = append(rollbacks, &pb.RelationshipUpdate{ + Operation: op, + Relationship: u.Relationship, + }) + } + + return e.applyUpdates(ctx, rollbacks) +} + +// applyUpdates is a wrapper function around the spiceDB WriteRelationships method +// it applies the given relationship updates and store the zed token for each resource. +func (e *engine) applyUpdates(ctx context.Context, updates []*pb.RelationshipUpdate) error { + resp, err := e.client.WriteRelationships(ctx, &pb.WriteRelationshipsRequest{Updates: updates}) + if err != nil { + return err + } + + t := resp.WrittenAt.Token + + for _, u := range updates { + resID := u.Relationship.Resource.ObjectId + if err := e.upsertZedToken(ctx, resID, t); err != nil { + return err + } + } + + return nil +} diff --git a/internal/query/rolebindings.go b/internal/query/rolebindings.go index a1c9f43e..43f47f4c 100644 --- a/internal/query/rolebindings.go +++ b/internal/query/rolebindings.go @@ -60,6 +60,8 @@ func (e *engine) GetRoleBinding(ctx context.Context, roleBinding types.Resource) rb.Subjects = make([]types.RoleBindingSubject, 0, len(rbRel)) + var roleID gidx.PrefixedID + for _, rel := range rbRel { // process subject relationships if rel.Relation == iapl.RolebindingSubjectRelation { @@ -77,26 +79,26 @@ func (e *engine) GetRoleBinding(ctx context.Context, roleBinding types.Resource) } // process role relationships - roleID, err := gidx.Parse(rel.Subject.Object.ObjectId) + roleID, err = gidx.Parse(rel.Subject.Object.ObjectId) if err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) return types.RoleBinding{}, err } + } - dbRole, err := e.store.GetRoleByID(ctx, roleID) - if err != nil { - span.RecordError(err) - span.SetStatus(codes.Error, err.Error()) + dbRole, err := e.store.GetRoleByID(ctx, roleID) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) - return types.RoleBinding{}, err - } + return types.RoleBinding{}, err + } - rb.Role = types.Role{ - ID: roleID, - Name: dbRole.Name, - } + rb.Role = types.Role{ + ID: roleID, + Name: dbRole.Name, } return rb, nil @@ -142,15 +144,24 @@ func (e *engine) CreateRoleBinding( dbCtx, err := e.store.BeginContext(ctx) if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + return types.RoleBinding{}, nil } rbResourceType, ok := e.schemaTypeMap[e.rbac.RoleBindingResource.Name] if !ok { - return types.RoleBinding{}, fmt.Errorf( + err := fmt.Errorf( "%w: invalid role-binding resource type: %s", ErrInvalidType, e.rbac.RoleBindingResource.Name, ) + + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) + + return types.RoleBinding{}, err } rbid, err := gidx.NewID(rbResourceType.IDPrefix) @@ -215,9 +226,7 @@ func (e *engine) CreateRoleBinding( } } - if _, err := e.client.WriteRelationships(ctx, &pb.WriteRelationshipsRequest{ - Updates: append(updates, subjUpdates...), - }); err != nil { + if err := e.applyUpdates(ctx, append(updates, subjUpdates...)); err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) @@ -229,7 +238,7 @@ func (e *engine) CreateRoleBinding( span.RecordError(err) span.SetStatus(codes.Error, err.Error()) logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) - logRollbackErr(e.logger, e.rollbackRoleBindingUpdates(ctx, updates)) + logRollbackErr(e.logger, e.rollbackUpdates(ctx, updates)) return types.RoleBinding{}, err } @@ -319,7 +328,7 @@ func (e *engine) DeleteRoleBinding(ctx context.Context, rb types.Resource) error } // apply changes - if _, err := e.client.WriteRelationships(ctx, &pb.WriteRelationshipsRequest{Updates: updates}); err != nil { + if err := e.applyUpdates(ctx, updates); err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) @@ -331,7 +340,7 @@ func (e *engine) DeleteRoleBinding(ctx context.Context, rb types.Resource) error span.RecordError(err) span.SetStatus(codes.Error, err.Error()) logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) - logRollbackErr(e.logger, e.rollbackRoleBindingUpdates(ctx, updates)) + logRollbackErr(e.logger, e.rollbackUpdates(ctx, updates)) return err } @@ -340,7 +349,7 @@ func (e *engine) DeleteRoleBinding(ctx context.Context, rb types.Resource) error span.RecordError(err) span.SetStatus(codes.Error, err.Error()) logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) - logRollbackErr(e.logger, e.rollbackRoleBindingUpdates(ctx, updates)) + logRollbackErr(e.logger, e.rollbackUpdates(ctx, updates)) return err } @@ -400,6 +409,7 @@ func (e *engine) ListRoleBindings(ctx context.Context, resource types.Resource, err := fmt.Errorf("%w: dangling grant relationship: %s", err, rel.String()) span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) e.logger.Warnf(err.Error()) } @@ -516,8 +526,7 @@ func (e *engine) UpdateRoleBinding(ctx context.Context, actor, rb types.Resource i++ } - _, err = e.client.WriteRelationships(ctx, &pb.WriteRelationshipsRequest{Updates: updates}) - if err != nil { + if err := e.applyUpdates(ctx, updates); err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) @@ -531,14 +540,14 @@ func (e *engine) UpdateRoleBinding(ctx context.Context, actor, rb types.Resource span.RecordError(err) span.SetStatus(codes.Error, err.Error()) logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) - logRollbackErr(e.logger, e.rollbackRoleBindingUpdates(ctx, updates)) + logRollbackErr(e.logger, e.rollbackUpdates(ctx, updates)) } if err := e.store.CommitContext(dbCtx); err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) - logRollbackErr(e.logger, e.rollbackRoleBindingUpdates(ctx, updates)) + logRollbackErr(e.logger, e.rollbackUpdates(ctx, updates)) return types.RoleBinding{}, err } @@ -722,7 +731,7 @@ func (e *engine) deleteRoleBindingsForRole(ctx context.Context, roleResource typ } // 3.2 delete all the relationships - if _, err := e.client.WriteRelationships(ctx, &pb.WriteRelationshipsRequest{Updates: updates}); err != nil { + if err := e.applyUpdates(ctx, updates); err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) @@ -734,7 +743,7 @@ func (e *engine) deleteRoleBindingsForRole(ctx context.Context, roleResource typ span.RecordError(err) span.SetStatus(codes.Error, err.Error()) logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) - logRollbackErr(e.logger, e.rollbackRoleBindingUpdates(ctx, updates)) + logRollbackErr(e.logger, e.rollbackUpdates(ctx, updates)) return err } @@ -833,41 +842,3 @@ func (e *engine) rolebindingRelationshipUpdateForSubject( return &pb.RelationshipUpdate{Operation: op, Relationship: rel}, nil } - -// rollbackRoleBindingUpdates is a helper function that rolls back a list of -// relationship updates on spiceDB. -func (e *engine) rollbackRoleBindingUpdates(ctx context.Context, updates []*pb.RelationshipUpdate) error { - updatesLen := len(updates) - rollbacks := make([]*pb.RelationshipUpdate, 0, updatesLen) - - for i := range updates { - // reversed order - u := updates[updatesLen-i-1] - - if u == nil { - continue - } - - var op pb.RelationshipUpdate_Operation - - switch u.Operation { - case pb.RelationshipUpdate_OPERATION_CREATE: - fallthrough - case pb.RelationshipUpdate_OPERATION_TOUCH: - op = pb.RelationshipUpdate_OPERATION_DELETE - case pb.RelationshipUpdate_OPERATION_DELETE: - op = pb.RelationshipUpdate_OPERATION_TOUCH - default: - continue - } - - rollbacks = append(rollbacks, &pb.RelationshipUpdate{ - Operation: op, - Relationship: u.Relationship, - }) - } - - _, err := e.client.WriteRelationships(ctx, &pb.WriteRelationshipsRequest{Updates: rollbacks}) - - return err -} diff --git a/openapi-v2.yaml b/openapi-v2.yaml index 1d45caa6..69e22ea8 100644 --- a/openapi-v2.yaml +++ b/openapi-v2.yaml @@ -888,7 +888,7 @@ paths: schema: type: string example: tnntten-root - /resources/{id}/role-bindings/{rb-id}: + /role-bindings/{rb-id}: get: tags: - role-bindings @@ -1078,12 +1078,6 @@ paths: - id: idntgrp-root-admins type: group parameters: - - name: id - in: path - required: true - schema: - type: string - example: loadbal-lb1 - name: rb-id in: path required: true From b72d193137b411a9c7ef139cd1d7184041398f6a Mon Sep 17 00:00:00 2001 From: Bailin He <15058035+bailinhe@users.noreply.github.com> Date: Fri, 3 May 2024 14:40:37 -0400 Subject: [PATCH 06/10] Apply suggestions from code review Co-authored-by: John Schaeffer Signed-off-by: Bailin He <15058035+bailinhe@users.noreply.github.com> --- internal/api/rolebindings.go | 57 +-- internal/api/router.go | 2 +- internal/api/types.go | 24 +- internal/query/errors.go | 11 + internal/query/relations.go | 3 + internal/query/rolebindings.go | 253 +++---------- internal/query/rolebindings_test.go | 50 ++- internal/query/roles_v2.go | 32 +- internal/query/roles_v2_test.go | 55 ++- internal/storage/rolebinding.go | 56 +-- internal/types/types.go | 9 +- openapi-v2.yaml | 553 ++++++++++++++-------------- 12 files changed, 467 insertions(+), 638 deletions(-) diff --git a/internal/api/rolebindings.go b/internal/api/rolebindings.go index feb67bb5..55fc97d1 100644 --- a/internal/api/rolebindings.go +++ b/internal/api/rolebindings.go @@ -14,18 +14,6 @@ import ( "go.infratographer.com/permissions-api/internal/types" ) -func resourceToSubject(subjects []types.RoleBindingSubject) []roleBindingSubject { - resp := make([]roleBindingSubject, len(subjects)) - for i, subj := range subjects { - resp[i] = roleBindingSubject{ - ID: subj.SubjectResource.ID, - Type: subj.SubjectResource.Type, - } - } - - return resp -} - func (r *Router) roleBindingCreate(c echo.Context) error { resourceIDStr := c.Param("id") @@ -72,17 +60,16 @@ func (r *Router) roleBindingCreate(c echo.Context) error { return r.errorResponse("error creating role resource", err) } - subjects := make([]types.RoleBindingSubject, len(body.Subjects)) + subjects := make([]types.RoleBindingSubject, len(body.SubjectIDs)) - for i, s := range body.Subjects { - subj, err := r.engine.NewResourceFromID(s.ID) + for i, sid := range body.SubjectIDs { + subj, err := r.engine.NewResourceFromID(sid) if err != nil { return r.errorResponse("error creating subject resource", err) } subjects[i] = types.RoleBindingSubject{ SubjectResource: subj, - Condition: nil, } } @@ -96,11 +83,8 @@ func (r *Router) roleBindingCreate(c echo.Context) error { roleBindingResponse{ ID: rb.ID, ResourceID: rb.ResourceID, - Subjects: resourceToSubject(rb.Subjects), - Role: roleBindingResponseRole{ - ID: rb.Role.ID, - Name: rb.Role.Name, - }, + SubjectIDs: rb.SubjectIDs, + RoleID: rb.RoleID, CreatedBy: rb.CreatedBy, UpdatedBy: rb.UpdatedBy, @@ -151,11 +135,8 @@ func (r *Router) roleBindingsList(c echo.Context) error { resp.Data[i] = roleBindingResponse{ ID: rb.ID, ResourceID: rb.ResourceID, - Subjects: resourceToSubject(rb.Subjects), - Role: roleBindingResponseRole{ - ID: rb.Role.ID, - Name: rb.Role.Name, - }, + SubjectIDs: rb.SubjectIDs, + RoleID: rb.RoleID, CreatedBy: rb.CreatedBy, UpdatedBy: rb.UpdatedBy, @@ -167,7 +148,7 @@ func (r *Router) roleBindingsList(c echo.Context) error { return c.JSON(http.StatusOK, resp) } -func (r *Router) roleBindingsDelete(c echo.Context) error { +func (r *Router) roleBindingDelete(c echo.Context) error { rbID := c.Param("rb_id") ctx, span := tracer.Start( @@ -259,11 +240,8 @@ func (r *Router) roleBindingGet(c echo.Context) error { roleBindingResponse{ ID: rb.ID, ResourceID: rb.ResourceID, - Subjects: resourceToSubject(rb.Subjects), - Role: roleBindingResponseRole{ - ID: rb.Role.ID, - Name: rb.Role.Name, - }, + SubjectIDs: rb.SubjectIDs, + RoleID: rb.RoleID, CreatedBy: rb.CreatedBy, UpdatedBy: rb.UpdatedBy, @@ -318,17 +296,16 @@ func (r *Router) roleBindingUpdate(c echo.Context) error { return r.errorResponse(err.Error(), ErrParsingRequestBody) } - subjects := make([]types.RoleBindingSubject, len(body.Subjects)) + subjects := make([]types.RoleBindingSubject, len(body.SubjectIDs)) - for i, s := range body.Subjects { - subj, err := r.engine.NewResourceFromID(s.ID) + for i, sid := range body.SubjectIDs { + subj, err := r.engine.NewResourceFromID(sid) if err != nil { return r.errorResponse("error creating subject resource", err) } subjects[i] = types.RoleBindingSubject{ SubjectResource: subj, - Condition: nil, } } @@ -342,12 +319,8 @@ func (r *Router) roleBindingUpdate(c echo.Context) error { roleBindingResponse{ ID: rb.ID, ResourceID: rb.ResourceID, - Subjects: resourceToSubject(rb.Subjects), - - Role: roleBindingResponseRole{ - ID: rb.Role.ID, - Name: rb.Role.Name, - }, + SubjectIDs: rb.SubjectIDs, + RoleID: rb.RoleID, CreatedBy: rb.CreatedBy, UpdatedBy: rb.UpdatedBy, diff --git a/internal/api/router.go b/internal/api/router.go index ceaca65c..63ed681e 100644 --- a/internal/api/router.go +++ b/internal/api/router.go @@ -89,7 +89,7 @@ func (r *Router) Routes(rg *echo.Group) { v2.GET("/resources/:id/role-bindings", r.roleBindingsList) v2.POST("/resources/:id/role-bindings", r.roleBindingCreate) v2.GET("/role-bindings/:rb_id", r.roleBindingGet) - v2.DELETE("/role-bindings/:rb_id", r.roleBindingsDelete) + v2.DELETE("/role-bindings/:rb_id", r.roleBindingDelete) v2.PATCH("/role-bindings/:rb_id", r.roleBindingUpdate) v2.GET("/actions", r.listActions) diff --git a/internal/api/types.go b/internal/api/types.go index 0bf128eb..90330e43 100644 --- a/internal/api/types.go +++ b/internal/api/types.go @@ -83,30 +83,20 @@ type listRolesV2Role struct { // RoleBindings -type roleBindingResponseRole struct { - ID gidx.PrefixedID `json:"id"` - Name string `json:"name"` -} - -type roleBindingSubject struct { - ID gidx.PrefixedID `json:"id" binding:"required"` - Type string `json:"type,omitempty"` -} - type roleBindingRequest struct { - RoleID string `json:"role_id" binding:"required"` - Subjects []roleBindingSubject `json:"subjects" binding:"required"` + RoleID string `json:"role_id" binding:"required"` + SubjectIDs []gidx.PrefixedID `json:"subject_ids" binding:"required"` } type rolebindingUpdateRequest struct { - Subjects []roleBindingSubject `json:"subjects" binding:"required"` + SubjectIDs []gidx.PrefixedID `json:"subject_ids" binding:"required"` } type roleBindingResponse struct { - ID gidx.PrefixedID `json:"id"` - ResourceID gidx.PrefixedID `json:"resource_id"` - Role roleBindingResponseRole `json:"role"` - Subjects []roleBindingSubject `json:"subjects"` + ID gidx.PrefixedID `json:"id"` + ResourceID gidx.PrefixedID `json:"resource_id"` + RoleID gidx.PrefixedID `json:"role_id"` + SubjectIDs []gidx.PrefixedID `json:"subject_ids"` CreatedBy gidx.PrefixedID `json:"created_by"` UpdatedBy gidx.PrefixedID `json:"updated_by"` diff --git a/internal/query/errors.go b/internal/query/errors.go index 6f21394e..07acb3bd 100644 --- a/internal/query/errors.go +++ b/internal/query/errors.go @@ -44,6 +44,9 @@ var ( // in the policy ErrRoleV2ResourceNotDefined = errors.New("role v2 resource not defined") + // ErrDeleteRoleInUse represents an error when a role is in use and cannot be deleted + ErrDeleteRoleInUse = fmt.Errorf("%w: role is in use", ErrInvalidArgument) + // ErrRoleAlreadyExists represents an error when a role already exists ErrRoleAlreadyExists = fmt.Errorf("%w: role already exists", ErrInvalidArgument) @@ -53,4 +56,12 @@ var ( // 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) + + // ErrCreateRoleBindingWithNoSubjects represents an error when a role + // binding is created with no subjects + ErrCreateRoleBindingWithNoSubjects = fmt.Errorf("%w: role binding must have at least one subject", ErrInvalidArgument) + + // ErrRoleBindingHasNoRelationships represents an internal error when a + // role binding has no relationships + ErrRoleBindingHasNoRelationships = errors.New("role binding has no relationships") ) diff --git a/internal/query/relations.go b/internal/query/relations.go index 40a844cd..e969ddeb 100644 --- a/internal/query/relations.go +++ b/internal/query/relations.go @@ -1234,6 +1234,9 @@ func (e *engine) NewResourceFromIDString(id string) (types.Resource, error) { // rollbackUpdates is a helper function that rolls back a list of // relationship updates on spiceDB. +// Note that we can only rollback spiceDB writes that are serialized, +// that is, there should only be one write (`client.WriteRelationships` or +// `e.ApplyUpdates`) over the course of the transaction. func (e *engine) rollbackUpdates(ctx context.Context, updates []*pb.RelationshipUpdate) error { updatesLen := len(updates) rollbacks := make([]*pb.RelationshipUpdate, 0, updatesLen) diff --git a/internal/query/rolebindings.go b/internal/query/rolebindings.go index 43f47f4c..845319ef 100644 --- a/internal/query/rolebindings.go +++ b/internal/query/rolebindings.go @@ -19,7 +19,7 @@ import ( func (e *engine) GetRoleBinding(ctx context.Context, roleBinding types.Resource) (types.RoleBinding, error) { ctx, span := e.tracer.Start( ctx, "engine.GetRoleBinding", - trace.WithAttributes(attribute.Stringer("role_binding_id", roleBinding.ID)), + trace.WithAttributes(attribute.Stringer("rolebinding_id", roleBinding.ID)), ) defer span.End() @@ -50,7 +50,7 @@ func (e *engine) GetRoleBinding(ctx context.Context, roleBinding types.Resource) } if len(rbRel) < 1 { - err := fmt.Errorf("%w: role binding: %s", ErrRoleBindingNotFound, roleBinding.ID.String()) + err := ErrRoleBindingHasNoRelationships span.RecordError(err) span.SetStatus(codes.Error, err.Error()) @@ -58,14 +58,13 @@ func (e *engine) GetRoleBinding(ctx context.Context, roleBinding types.Resource) return types.RoleBinding{}, err } - rb.Subjects = make([]types.RoleBindingSubject, 0, len(rbRel)) - - var roleID gidx.PrefixedID + rb.SubjectIDs = make([]gidx.PrefixedID, 0, len(rbRel)) for _, rel := range rbRel { + switch { // process subject relationships - if rel.Relation == iapl.RolebindingSubjectRelation { - subjectRes, err := e.NewResourceFromIDString(rel.Subject.Object.ObjectId) + case rel.Relation == iapl.RolebindingSubjectRelation: + subjID, err := gidx.Parse(rel.Subject.Object.ObjectId) if err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) @@ -73,34 +72,20 @@ func (e *engine) GetRoleBinding(ctx context.Context, roleBinding types.Resource) return types.RoleBinding{}, err } - rb.Subjects = append(rb.Subjects, types.RoleBindingSubject{SubjectResource: subjectRes}) - - continue - } + rb.SubjectIDs = append(rb.SubjectIDs, subjID) // process role relationships - roleID, err = gidx.Parse(rel.Subject.Object.ObjectId) - if err != nil { - span.RecordError(err) - span.SetStatus(codes.Error, err.Error()) + default: + rb.RoleID, err = gidx.Parse(rel.Subject.Object.ObjectId) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) - return types.RoleBinding{}, err + return types.RoleBinding{}, err + } } } - dbRole, err := e.store.GetRoleByID(ctx, roleID) - if err != nil { - span.RecordError(err) - span.SetStatus(codes.Error, err.Error()) - - return types.RoleBinding{}, err - } - - rb.Role = types.Role{ - ID: roleID, - Name: dbRole.Name, - } - return rb, nil } @@ -118,6 +103,14 @@ func (e *engine) CreateRoleBinding( ) defer span.End() + if len(subjects) == 0 { + err := ErrCreateRoleBindingWithNoSubjects + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return types.RoleBinding{}, err + } + if err := e.isRoleBindable(ctx, roleResource, resource); err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) @@ -137,11 +130,6 @@ func (e *engine) CreateRoleBinding( return types.RoleBinding{}, err } - role := types.Role{ - ID: roleResource.ID, - Name: dbrole.Name, - } - dbCtx, err := e.store.BeginContext(ctx) if err != nil { span.RecordError(err) @@ -150,19 +138,7 @@ func (e *engine) CreateRoleBinding( return types.RoleBinding{}, nil } - rbResourceType, ok := e.schemaTypeMap[e.rbac.RoleBindingResource.Name] - if !ok { - err := fmt.Errorf( - "%w: invalid role-binding resource type: %s", - ErrInvalidType, e.rbac.RoleBindingResource.Name, - ) - - span.RecordError(err) - span.SetStatus(codes.Error, err.Error()) - logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) - - return types.RoleBinding{}, err - } + rbResourceType := e.schemaTypeMap[e.rbac.RoleBindingResource.Name] rbid, err := gidx.NewID(rbResourceType.IDPrefix) if err != nil { @@ -182,9 +158,9 @@ func (e *engine) CreateRoleBinding( return types.RoleBinding{}, err } - rb.Role = role + rb.RoleID = dbrole.ID - roleRel := e.rolebindingRoleRelationship(role.ID.String(), rb.ID.String()) + roleRel := e.rolebindingRoleRelationship(dbrole.ID.String(), rb.ID.String()) grantRel, err := e.rolebindingGrantResourceRelationship(resource, rb.ID.String()) if err != nil { @@ -207,7 +183,7 @@ func (e *engine) CreateRoleBinding( } subjUpdates := make([]*pb.RelationshipUpdate, len(subjects)) - rb.Subjects = make([]types.RoleBindingSubject, len(subjects)) + rb.SubjectIDs = make([]gidx.PrefixedID, len(subjects)) for i, subj := range subjects { rel, err := e.rolebindingSubjectRelationship(subj.SubjectResource, rb.ID.String()) @@ -219,14 +195,16 @@ func (e *engine) CreateRoleBinding( return types.RoleBinding{}, err } - rb.Subjects[i] = subj + rb.SubjectIDs[i] = subj.SubjectResource.ID subjUpdates[i] = &pb.RelationshipUpdate{ Operation: pb.RelationshipUpdate_OPERATION_TOUCH, Relationship: rel, } } - if err := e.applyUpdates(ctx, append(updates, subjUpdates...)); err != nil { + updates = append(updates, subjUpdates...) + + if err := e.applyUpdates(ctx, updates); err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) @@ -250,7 +228,7 @@ func (e *engine) DeleteRoleBinding(ctx context.Context, rb types.Resource) error ctx, span := e.tracer.Start( ctx, "engine.DeleteRoleBinding", trace.WithAttributes( - attribute.Stringer("role_binding_id", rb.ID), + attribute.Stringer("rolebinding_id", rb.ID), ), ) defer span.End() @@ -418,11 +396,11 @@ func (e *engine) ListRoleBindings(ctx context.Context, resource types.Resource, continue } - if optionalRole != nil && rb.Role.ID.String() != optionalRole.ID.String() { + if optionalRole != nil && rb.RoleID != optionalRole.ID { continue } - if len(rb.Subjects) == 0 { + if len(rb.SubjectIDs) == 0 { continue } @@ -476,15 +454,17 @@ func (e *engine) UpdateRoleBinding(ctx context.Context, actor, rb types.Resource } // 1. find the subjects to add or remove - current := make([]string, len(rolebinding.Subjects)) + current := make([]string, len(rolebinding.SubjectIDs)) incoming := make([]string, len(subjects)) + newSubjectIDs := make([]gidx.PrefixedID, len(subjects)) - for i, subj := range rolebinding.Subjects { - current[i] = subj.SubjectResource.ID.String() + for i, subj := range rolebinding.SubjectIDs { + current[i] = subj.String() } for i, subj := range subjects { incoming[i] = subj.SubjectResource.ID.String() + newSubjectIDs[i] = subj.SubjectResource.ID } add, remove := diff(current, incoming) @@ -495,8 +475,7 @@ func (e *engine) UpdateRoleBinding(ctx context.Context, actor, rb types.Resource } // 2. create relationship updates - updates := make([]*pb.RelationshipUpdate, len(add)+len(remove)) - i := 0 + updates := make([]*pb.RelationshipUpdate, 0, len(add)+len(remove)) for _, id := range add { update, err := e.rolebindingRelationshipUpdateForSubject(id, rb.ID.String(), pb.RelationshipUpdate_OPERATION_TOUCH) @@ -508,8 +487,7 @@ func (e *engine) UpdateRoleBinding(ctx context.Context, actor, rb types.Resource return types.RoleBinding{}, err } - updates[i] = update - i++ + updates = append(updates, update) } for _, id := range remove { @@ -522,8 +500,7 @@ func (e *engine) UpdateRoleBinding(ctx context.Context, actor, rb types.Resource return types.RoleBinding{}, err } - updates[i] = update - i++ + updates = append(updates, update) } if err := e.applyUpdates(ctx, updates); err != nil { @@ -552,7 +529,7 @@ func (e *engine) UpdateRoleBinding(ctx context.Context, actor, rb types.Resource return types.RoleBinding{}, err } - rolebinding.Subjects = subjects + rolebinding.SubjectIDs = newSubjectIDs rolebinding.UpdatedAt = rbFromDB.UpdatedAt rolebinding.UpdatedBy = rbFromDB.UpdatedBy @@ -605,152 +582,6 @@ func (e *engine) isRoleBindable(ctx context.Context, role, res types.Resource) e } } -// deleteRoleBindingsForRole deletes all role-binding relationships with a given role. -func (e *engine) deleteRoleBindingsForRole(ctx context.Context, roleResource types.Resource) error { - ctx, span := e.tracer.Start( - ctx, "engine.deleteRoleBindingsForRole", - trace.WithAttributes( - attribute.Stringer("role_id", roleResource.ID), - ), - ) - defer span.End() - - // 1. find all the bindings for the role - findBindingsFilter := &pb.RelationshipFilter{ - ResourceType: e.namespaced(e.rbac.RoleBindingResource.Name), - OptionalRelation: iapl.RolebindingRoleRelation, - OptionalSubjectFilter: &pb.SubjectFilter{ - SubjectType: e.namespaced(e.rbac.RoleResource.Name), - OptionalSubjectId: roleResource.ID.String(), - }, - } - - bindings, err := e.readRelationships(ctx, findBindingsFilter) - if err != nil { - span.RecordError(err) - span.SetStatus(codes.Error, err.Error()) - - return err - } - - if len(bindings) == 0 { - return nil - } - - rbIDs := make([]gidx.PrefixedID, len(bindings)) - - for i, rel := range bindings { - id, err := gidx.Parse(rel.Resource.ObjectId) - if err != nil { - span.RecordError(err) - span.SetStatus(codes.Error, err.Error()) - - return err - } - - rbIDs[i] = id - } - - dbCtx, err := e.store.BeginContext(ctx) - if err != nil { - span.RecordError(err) - span.SetStatus(codes.Error, err.Error()) - - return err - } - - if err := e.store.BatchLockRoleBindingForUpdate(dbCtx, rbIDs); err != nil { - span.RecordError(err) - span.SetStatus(codes.Error, err.Error()) - logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) - - return err - } - - // 2. Gather all the relationships to be deleted - - // 2.1 build a list of requests to get all the subject, role and grant - // relationships for all bindable resources - relFilters := []*pb.RelationshipFilter{} - - for _, rb := range bindings { - relFilters = append(relFilters, &pb.RelationshipFilter{ - ResourceType: rb.Resource.ObjectType, - OptionalResourceId: rb.Resource.ObjectId, - }, - ) - - for _, res := range e.rbacV2ResourceTypes { - relFilters = append(relFilters, &pb.RelationshipFilter{ - ResourceType: e.namespaced(res.Name), - OptionalRelation: iapl.GrantRelationship, - OptionalSubjectFilter: &pb.SubjectFilter{ - SubjectType: rb.Resource.ObjectType, - OptionalSubjectId: rb.Resource.ObjectId, - }, - }, - ) - } - } - - // 2.2 read all the relationships - rels := []*pb.Relationship{} - - for _, filter := range relFilters { - r, err := e.readRelationships(ctx, filter) - if err != nil { - span.RecordError(err) - span.SetStatus(codes.Error, err.Error()) - logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) - - return err - } - - rels = append(rels, r...) - } - - // 2.3 create delete requests - updates := make([]*pb.RelationshipUpdate, len(rels)) - - for i, rel := range rels { - updates[i] = &pb.RelationshipUpdate{ - Operation: pb.RelationshipUpdate_OPERATION_DELETE, - Relationship: rel, - } - } - - e.logger.Debugf("%d relationships will be deleted", len(updates)) - - // 3.1 delete all records in permissions-api DB - if err := e.store.BatchDeleteRoleBindings(dbCtx, rbIDs); err != nil { - span.RecordError(err) - span.SetStatus(codes.Error, err.Error()) - logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) - - return err - } - - // 3.2 delete all the relationships - if err := e.applyUpdates(ctx, updates); 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)) - logRollbackErr(e.logger, e.rollbackUpdates(ctx, updates)) - - return err - } - - return nil -} - // rolebindingSubjectRelationship is a helper function that creates a // relationship between a role-binding and a subject. func (e *engine) rolebindingSubjectRelationship(subj types.Resource, rbID string) (*pb.Relationship, error) { diff --git a/internal/query/rolebindings_test.go b/internal/query/rolebindings_test.go index 33137c6a..d0f294b9 100644 --- a/internal/query/rolebindings_test.go +++ b/internal/query/rolebindings_test.go @@ -102,8 +102,8 @@ func TestCreateRoleBinding(t *testing.T) { }, CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[types.RoleBinding]) { assert.NoError(t, res.Err) - assert.Equal(t, role.ID, res.Success.Role.ID) - assert.Len(t, res.Success.Subjects, 1) + assert.Equal(t, role.ID, res.Success.RoleID) + assert.Len(t, res.Success.SubjectIDs, 1) rb, err := e.ListRoleBindings(ctx, child, nil) assert.NoError(t, err) @@ -121,6 +121,17 @@ func TestCreateRoleBinding(t *testing.T) { assert.ErrorIs(t, res.Err, ErrRoleNotFound) }, }, + { + Name: "CreateRoleBindingWithNoSubjects", + Input: input{ + resource: root, + role: roleRes, + }, + CheckFn: func(ctx context.Context, t *testing.T, tr testingx.TestResult[types.RoleBinding]) { + assert.ErrorIs(t, tr.Err, ErrInvalidArgument) + assert.ErrorIs(t, tr.Err, ErrCreateRoleBindingWithNoSubjects) + }, + }, { Name: "CreateRoleBindingSuccess", Input: input{ @@ -131,10 +142,10 @@ func TestCreateRoleBinding(t *testing.T) { CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[types.RoleBinding]) { assert.NoError(t, res.Err) - assert.Len(t, res.Success.Subjects, 1) - assert.Equal(t, role.ID, res.Success.Role.ID) + assert.Len(t, res.Success.SubjectIDs, 1) + assert.Equal(t, role.ID, res.Success.RoleID) assert.Equal(t, root.ID, res.Success.ResourceID) - assert.Equal(t, subj.ID, res.Success.Subjects[0].SubjectResource.ID) + assert.Equal(t, subj.ID, res.Success.SubjectIDs[0]) assert.Equal(t, actor.ID, res.Success.CreatedBy) rbs, err := e.ListRoleBindings(ctx, root, nil) @@ -215,7 +226,7 @@ func TestListRoleBindings(t *testing.T) { }, CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[[]types.RoleBinding]) { assert.Len(t, res.Success, 1) - assert.Equal(t, viewer.ID, res.Success[0].Role.ID) + assert.Equal(t, viewer.ID, res.Success[0].RoleID) }, }, { @@ -226,7 +237,7 @@ func TestListRoleBindings(t *testing.T) { }, CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[[]types.RoleBinding]) { assert.Len(t, res.Success, 1) - assert.Equal(t, editor.ID, res.Success[0].Role.ID) + assert.Equal(t, editor.ID, res.Success[0].RoleID) }, }, { @@ -291,9 +302,9 @@ func TestGetRoleBinding(t *testing.T) { Input: rbRes, CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[types.RoleBinding]) { assert.NoError(t, res.Err) - assert.Equal(t, viewer.ID, res.Success.Role.ID) - assert.Len(t, res.Success.Subjects, 1) - assert.Equal(t, subj.ID, res.Success.Subjects[0].SubjectResource.ID) + assert.Equal(t, viewer.ID, res.Success.RoleID) + assert.Len(t, res.Success.SubjectIDs, 1) + assert.Equal(t, subj.ID, res.Success.SubjectIDs[0]) assert.Equal(t, actor.ID, res.Success.CreatedBy) assert.Equal(t, root.ID, res.Success.ResourceID) }, @@ -383,10 +394,10 @@ func TestUpdateRoleBinding(t *testing.T) { CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[types.RoleBinding]) { assert.NoError(t, res.Err) - assert.Len(t, res.Success.Subjects, 2) - assert.Contains(t, res.Success.Subjects, types.RoleBindingSubject{SubjectResource: user1}) - assert.Contains(t, res.Success.Subjects, types.RoleBindingSubject{SubjectResource: group1}) - assert.NotContains(t, res.Success.Subjects, types.RoleBindingSubject{SubjectResource: subj}) + assert.Len(t, res.Success.SubjectIDs, 2) + assert.Contains(t, res.Success.SubjectIDs, user1.ID) + assert.Contains(t, res.Success.SubjectIDs, group1.ID) + assert.NotContains(t, res.Success.SubjectIDs, subj.ID) assert.Equal(t, actor.ID, res.Success.UpdatedBy) assert.Equal(t, root.ID, res.Success.ResourceID) @@ -494,6 +505,9 @@ func TestPermissions(t *testing.T) { group1, err := e.NewResourceFromIDString("idntgrp-group1") require.NoError(t, err) + // rolebinding + var rb types.RoleBinding + err = e.CreateRelationships(ctx, []types.Relationship{{ Resource: group1, Relation: "member", @@ -633,7 +647,7 @@ func TestPermissions(t *testing.T) { }) require.Error(t, err) - _, err = e.CreateRoleBinding(ctx, user1, root, viewerRes, []types.RoleBindingSubject{{SubjectResource: group1}}) + rb, err = e.CreateRoleBinding(ctx, user1, root, viewerRes, []types.RoleBindingSubject{{SubjectResource: group1}}) require.NoError(t, err) return ctx @@ -719,7 +733,7 @@ func TestPermissions(t *testing.T) { Sync: true, }, { - Name: "RoleRemoval", + Name: "DeleteRoleBinding", SetupFn: func(ctx context.Context, t *testing.T) context.Context { err := e.checkPermission(ctx, &pb.CheckPermissionRequest{ Consistency: fullconsistency, @@ -729,7 +743,9 @@ func TestPermissions(t *testing.T) { }) require.NoError(t, err) - err = e.DeleteRoleV2(ctx, viewerRes) + rbRes, err := e.NewResourceFromID(rb.ID) + require.NoError(t, err) + err = e.DeleteRoleBinding(ctx, rbRes) require.NoError(t, err) return ctx diff --git a/internal/query/roles_v2.go b/internal/query/roles_v2.go index 83211db8..78a58be8 100644 --- a/internal/query/roles_v2.go +++ b/internal/query/roles_v2.go @@ -352,6 +352,33 @@ func (e *engine) DeleteRoleV2(ctx context.Context, roleResource types.Resource) ctx, span := e.tracer.Start(ctx, "engine.DeleteRoleV2") defer span.End() + // find all the bindings for the role + findBindingsFilter := &pb.RelationshipFilter{ + ResourceType: e.namespaced(e.rbac.RoleBindingResource.Name), + OptionalRelation: iapl.RolebindingRoleRelation, + OptionalSubjectFilter: &pb.SubjectFilter{ + SubjectType: e.namespaced(e.rbac.RoleResource.Name), + OptionalSubjectId: roleResource.ID.String(), + }, + } + + bindings, err := e.readRelationships(ctx, findBindingsFilter) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return err + } + + // reject delete if role is in use + if len(bindings) > 0 { + err := fmt.Errorf("%w: cannot delete role", ErrDeleteRoleInUse) + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + + return err + } + dbCtx, err := e.store.BeginContext(ctx) if err != nil { span.RecordError(err) @@ -430,11 +457,6 @@ func (e *engine) DeleteRoleV2(ctx context.Context, roleResource types.Resource) errs = append(errs, err) } - // 2.c remove all role relationships in role bindings associated with this role - if err := e.deleteRoleBindingsForRole(ctx, roleResource); err != nil { - errs = append(errs, err) - } - for _, err := range errs { if err != nil { span.RecordError(err) diff --git a/internal/query/roles_v2_test.go b/internal/query/roles_v2_test.go index f09eb022..1aee76e6 100644 --- a/internal/query/roles_v2_test.go +++ b/internal/query/roles_v2_test.go @@ -434,13 +434,13 @@ func TestDeleteRolesV2(t *testing.T) { require.NoError(t, err) // these bindings are expected to be deleted after the role is deleted - _, err = e.CreateRoleBinding(ctx, actor, root, roleRes, []types.RoleBindingSubject{{SubjectResource: subj}}) + rbRoot, err := e.CreateRoleBinding(ctx, actor, root, roleRes, []types.RoleBindingSubject{{SubjectResource: subj}}) require.NoError(t, err) - _, err = e.CreateRoleBinding(ctx, actor, child, roleRes, []types.RoleBindingSubject{{SubjectResource: subj}}) + rbChild, err := e.CreateRoleBinding(ctx, actor, child, roleRes, []types.RoleBindingSubject{{SubjectResource: subj}}) require.NoError(t, err) - _, err = e.CreateRoleBinding(ctx, actor, theotherchild, roleRes, []types.RoleBindingSubject{{SubjectResource: subj}}) + rbTheOtherChild, err := e.CreateRoleBinding(ctx, actor, theotherchild, roleRes, []types.RoleBindingSubject{{SubjectResource: subj}}) require.NoError(t, err) rb, err := e.ListRoleBindings(ctx, root, &roleRes) @@ -460,7 +460,7 @@ func TestDeleteRolesV2(t *testing.T) { Name: "DeleteRoleNotFound", Input: notfoundRes, CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[types.Role]) { - assert.ErrorContains(t, res.Err, ErrRoleNotFound.Error()) + assert.ErrorIs(t, res.Err, storage.ErrNoRoleFound) }, Sync: true, }, @@ -472,26 +472,45 @@ func TestDeleteRolesV2(t *testing.T) { }, }, { - Name: "DeleteRoleSuccess", + Name: "DeleteRoleWithExistingBindings", Input: roleRes, CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[types.Role]) { - assert.NoError(t, res.Err) - - _, err := e.GetRoleV2(ctx, roleRes) - assert.ErrorContains(t, err, ErrRoleNotFound.Error()) - - // make sure the role bindings are also deleted - rb, err := e.ListRoleBindings(ctx, root, &roleRes) + assert.ErrorIs(t, res.Err, ErrDeleteRoleInUse) + }, + Sync: true, + }, + { + Name: "DeleteRoleSuccess", + Input: roleRes, + SetupFn: func(ctx context.Context, t *testing.T) context.Context { + var ( + err error + rb types.Resource + ) + + // delete the role bindings first + rb, err = e.NewResourceFromID(rbRoot.ID) + require.NoError(t, err) + err = e.DeleteRoleBinding(ctx, rb) + require.NoError(t, err) + + rb, err = e.NewResourceFromID(rbChild.ID) + require.NoError(t, err) + err = e.DeleteRoleBinding(ctx, rb) assert.NoError(t, err) - assert.Len(t, rb, 0) - rb, err = e.ListRoleBindings(ctx, child, &roleRes) + rb, err = e.NewResourceFromID(rbTheOtherChild.ID) + require.NoError(t, err) + err = e.DeleteRoleBinding(ctx, rb) assert.NoError(t, err) - assert.Len(t, rb, 0) - rb, err = e.ListRoleBindings(ctx, theotherchild, &roleRes) - assert.NoError(t, err) - assert.Len(t, rb, 0) + return ctx + }, + CheckFn: func(ctx context.Context, t *testing.T, res testingx.TestResult[types.Role]) { + assert.NoError(t, res.Err) + + _, err := e.GetRoleV2(ctx, roleRes) + assert.ErrorIs(t, err, storage.ErrNoRoleFound) }, Sync: true, }, diff --git a/internal/storage/rolebinding.go b/internal/storage/rolebinding.go index 906464ce..1fbe5dc6 100644 --- a/internal/storage/rolebinding.go +++ b/internal/storage/rolebinding.go @@ -5,6 +5,7 @@ import ( "database/sql" "errors" "fmt" + "time" "go.infratographer.com/permissions-api/internal/types" @@ -40,17 +41,9 @@ type RoleBindingService interface { // CommitContext or RollbackContext must be called afterwards if this method returns no error. DeleteRoleBinding(ctx context.Context, id gidx.PrefixedID) error - // BatchDeleteRoleBinding deletes multiple role bindings from the database - // This method must be called with a context returned from BeginContext. - // CommitContext or RollbackContext must be called afterwards if this method returns no error. - BatchDeleteRoleBindings(ctx context.Context, ids []gidx.PrefixedID) error - // LockRoleBindingForUpdate locks a role binding record to be updated to ensure consistency. // If the role binding is not found, an ErrRoleBindingNotFound error is returned. LockRoleBindingForUpdate(ctx context.Context, id gidx.PrefixedID) error - - // BatchLockRoleBindingForUpdate locks multiple role binding records to be updated to ensure consistency. - BatchLockRoleBindingForUpdate(ctx context.Context, ids []gidx.PrefixedID) error } func (e *engine) GetRoleBindingByID(ctx context.Context, id gidx.PrefixedID) (types.RoleBinding, error) { @@ -133,9 +126,9 @@ func (e *engine) CreateRoleBinding(ctx context.Context, actorID, rbID, resourceI err = tx.QueryRowContext(ctx, ` INSERT INTO rolebindings (id, resource_id, created_by, updated_by, created_at, updated_at) - VALUES ($1, $2, $3, $3, now(), now()) + VALUES ($1, $2, $3, $3, $4, $4) RETURNING id, resource_id, created_by, updated_by, created_at, updated_at - `, rbID.String(), resourceID.String(), actorID.String(), + `, rbID.String(), resourceID.String(), actorID.String(), time.Now(), ).Scan( &rb.ID, &rb.ResourceID, @@ -211,23 +204,6 @@ func (e *engine) DeleteRoleBinding(ctx context.Context, id gidx.PrefixedID) erro return nil } -func (e *engine) BatchDeleteRoleBindings(ctx context.Context, ids []gidx.PrefixedID) error { - tx, err := getContextTx(ctx) - if err != nil { - return err - } - - inClause, args := e.buildBatchInClauseWithIDs(ids) - q := fmt.Sprintf("DELETE FROM rolebindings WHERE id IN (%s)", inClause) - - _, err = tx.ExecContext(ctx, q, args...) - if err != nil { - return err - } - - return nil -} - func (e *engine) LockRoleBindingForUpdate(ctx context.Context, id gidx.PrefixedID) error { db, err := getContextDBQuery(ctx, e) if err != nil { @@ -251,32 +227,6 @@ func (e *engine) LockRoleBindingForUpdate(ctx context.Context, id gidx.PrefixedI return nil } -func (e *engine) BatchLockRoleBindingForUpdate(ctx context.Context, ids []gidx.PrefixedID) error { - db, err := getContextDBQuery(ctx, e) - if err != nil { - return err - } - - inClause, args := e.buildBatchInClauseWithIDs(ids) - q := fmt.Sprintf("SELECT 1 FROM rolebindings WHERE id IN (%s) FOR UPDATE", inClause) - - result, err := db.ExecContext(ctx, q, args...) - if err != nil { - return err - } - - rowsAffected, err := result.RowsAffected() - if err != nil { - return err - } - - if int(rowsAffected) != len(ids) { - return fmt.Errorf("%w: %d of role-bindings not found", ErrRoleBindingNotFound, len(ids)-int(rowsAffected)) - } - - return nil -} - // buildBatchInClauseWithIDs is a helper function that builds an IN clause for // a batch query with the provided prefixed IDs. func (e *engine) buildBatchInClauseWithIDs(ids []gidx.PrefixedID) (clause string, args []any) { diff --git a/internal/types/types.go b/internal/types/types.go index b22ca92e..ff7d0509 100644 --- a/internal/types/types.go +++ b/internal/types/types.go @@ -81,14 +81,9 @@ type Resource struct { ID gidx.PrefixedID } -// RoleBindingSubjectCondition is the object that represents the condition of a -// role binding subject. -type RoleBindingSubjectCondition struct{} - // RoleBindingSubject is the object that represents the subject of a role binding. type RoleBindingSubject struct { SubjectResource Resource - Condition *RoleBindingSubjectCondition } // Relationship represents a named association between a resource and a subject. @@ -102,8 +97,8 @@ type Relationship struct { type RoleBinding struct { ID gidx.PrefixedID ResourceID gidx.PrefixedID - Role Role - Subjects []RoleBindingSubject + RoleID gidx.PrefixedID + SubjectIDs []gidx.PrefixedID CreatedBy gidx.PrefixedID UpdatedBy gidx.PrefixedID diff --git a/openapi-v2.yaml b/openapi-v2.yaml index 69e22ea8..46027e38 100644 --- a/openapi-v2.yaml +++ b/openapi-v2.yaml @@ -498,7 +498,33 @@ paths: operationId: deleteRole responses: "200": - description: "" + description: delete-role + content: + application/json: + schema: + type: object + properties: + success: + type: boolean + example: true + examples: + delete-role: + value: + success: true + "400": + description: delete-role-with-existing-bindings + content: + application/json: + schema: + type: object + properties: + message: + type: string + example: 'error deleting role: invalid argument: role is in use: cannot delete role' + examples: + delete-role-with-existing-bindings: + value: + message: 'error deleting role: invalid argument: role is in use: cannot delete role' patch: tags: - roles @@ -662,7 +688,7 @@ paths: required: true schema: type: string - example: permrv2-zNYOpdL1RGyiSp0hgaIYB + example: permrv2-l9XgxtA6EUkwNSCTGlmgF /resources/{id}/role-bindings: get: tags: @@ -691,92 +717,108 @@ paths: items: type: object properties: + created_at: + type: string + example: "2024-05-03T19:35:23Z" + created_by: + type: string + example: idntusr-bailin id: type: string - example: permrbn-r7WU4juT0p-76JW-sJwzQ - role: - type: object - properties: - id: - type: string - example: permrv2-nDw3bVXYwHysvZDFyxh2C - name: - type: string - example: super_user - subjects: + example: permrbn-IYH19GIbDGZ9n2xR0yHvW + resource_id: + type: string + example: tnntten-root + role_id: + type: string + example: permrv2-PLjILDwe8kG_t42tMCDiB + subject_ids: type: array items: - type: object - properties: - id: - type: string - example: idntusr-bailin - type: - type: string - example: user + type: string + example: idntusr-bailin example: - - id: idntusr-bailin - type: user + - idntusr-bailin + updated_at: + type: string + example: "2024-05-03T19:35:23Z" + updated_by: + type: string + example: idntusr-bailin example: - - id: permrbn-r7WU4juT0p-76JW-sJwzQ - role: - id: permrv2-nDw3bVXYwHysvZDFyxh2C - name: super_user - subjects: - - id: idntusr-bailin - type: user - - id: permrbn-wNpgDB-ajBTyR6nB-WBcY - role: - id: permrv2-nDw3bVXYwHysvZDFyxh2C - name: super_user - subjects: - - id: idntusr-bailin - type: user - - id: idntusr-bailin-1 - type: user - - id: idntusr-bailin-2 - type: user - - id: idntusr-bailin-3 - type: user - - id: idntusr-bailin-4 - type: user - - id: idntusr-bailin-5 - type: user + - created_at: "2024-05-03T19:35:23Z" + created_by: idntusr-bailin + id: permrbn-IYH19GIbDGZ9n2xR0yHvW + resource_id: tnntten-root + role_id: permrv2-PLjILDwe8kG_t42tMCDiB + subject_ids: + - idntusr-bailin + updated_at: "2024-05-03T19:35:23Z" + updated_by: idntusr-bailin + - created_at: "2024-05-06T16:04:08Z" + created_by: idntusr-bailin + id: permrbn-K652x2XPJO1mGJFUE4hEu + resource_id: tnntten-root + role_id: permrv2-PLjILDwe8kG_t42tMCDiB + subject_ids: + - idntgrp-my-subgroup + updated_at: "2024-05-06T16:04:08Z" + updated_by: idntusr-bailin + - created_at: "2024-05-06T16:00:46Z" + created_by: idntusr-bailin + id: permrbn-lr5s4g6g1shmDL_htEAR_ + resource_id: tnntten-root + role_id: permrv2-PLjILDwe8kG_t42tMCDiB + subject_ids: + - idntusr-bailin + - idntusr-bailin-1 + - idntusr-bailin-2 + - idntusr-bailin-3 + - idntusr-bailin-4 + - idntusr-bailin-5 + updated_at: "2024-05-06T16:00:46Z" + updated_by: idntusr-bailin examples: list-role-bindings: value: data: - - id: permrbn-r7WU4juT0p-76JW-sJwzQ - role: - id: permrv2-nDw3bVXYwHysvZDFyxh2C - name: super_user - subjects: - - id: idntusr-bailin - type: user - - id: permrbn-wNpgDB-ajBTyR6nB-WBcY - role: - id: permrv2-nDw3bVXYwHysvZDFyxh2C - name: super_user - subjects: - - id: idntusr-bailin - type: user - - id: idntusr-bailin-1 - type: user - - id: idntusr-bailin-2 - type: user - - id: idntusr-bailin-3 - type: user - - id: idntusr-bailin-4 - type: user - - id: idntusr-bailin-5 - type: user + - created_at: "2024-05-03T19:35:23Z" + created_by: idntusr-bailin + id: permrbn-IYH19GIbDGZ9n2xR0yHvW + resource_id: tnntten-root + role_id: permrv2-PLjILDwe8kG_t42tMCDiB + subject_ids: + - idntusr-bailin + updated_at: "2024-05-03T19:35:23Z" + updated_by: idntusr-bailin + - created_at: "2024-05-06T16:04:08Z" + created_by: idntusr-bailin + id: permrbn-K652x2XPJO1mGJFUE4hEu + resource_id: tnntten-root + role_id: permrv2-PLjILDwe8kG_t42tMCDiB + subject_ids: + - idntgrp-my-subgroup + updated_at: "2024-05-06T16:04:08Z" + updated_by: idntusr-bailin + - created_at: "2024-05-06T16:00:46Z" + created_by: idntusr-bailin + id: permrbn-lr5s4g6g1shmDL_htEAR_ + resource_id: tnntten-root + role_id: permrv2-PLjILDwe8kG_t42tMCDiB + subject_ids: + - idntusr-bailin + - idntusr-bailin-1 + - idntusr-bailin-2 + - idntusr-bailin-3 + - idntusr-bailin-4 + - idntusr-bailin-5 + updated_at: "2024-05-06T16:00:46Z" + updated_by: idntusr-bailin post: tags: - role-bindings summary: create-role-binding - description: | - create a role-binding for a resource. The role-binding will grant the - specified role to the specified subjects. + description: create-role-binding operationId: createRoleBinding requestBody: content: @@ -786,28 +828,20 @@ paths: properties: role_id: type: string - example: permrv2-hoN9DD27g1tfKzjF8kj42 - subjects: + example: permrv2-PLjILDwe8kG_t42tMCDiB + subject_ids: type: array items: - type: object - properties: - conditions: - type: object - properties: {} - id: - type: string - example: idntgrp-my-subgroup + type: string + example: idntgrp-my-subgroup example: - - conditions: {} - id: idntgrp-my-subgroup + - idntgrp-my-subgroup examples: create-role-binding: value: - role_id: permrv2-hoN9DD27g1tfKzjF8kj42 - subjects: - - conditions: {} - id: idntgrp-my-subgroup + role_id: permrv2-PLjILDwe8kG_t42tMCDiB + subject_ids: + - idntgrp-my-subgroup responses: "200": description: root-super-user / create-role-binding-group @@ -816,71 +850,81 @@ paths: schema: type: object properties: + created_at: + type: string + example: "2024-05-06T16:00:46Z" + created_by: + type: string + example: idntusr-bailin id: type: string - example: permrbn-wNpgDB-ajBTyR6nB-WBcY - role: - type: object - properties: - id: - type: string - example: permrv2-nDw3bVXYwHysvZDFyxh2C - name: - type: string - example: super_user - subjects: + example: permrbn-lr5s4g6g1shmDL_htEAR_ + resource_id: + type: string + example: tnntten-root + role_id: + type: string + example: permrv2-PLjILDwe8kG_t42tMCDiB + subject_ids: type: array items: - type: object - properties: - id: - type: string - example: idntusr-bailin - type: - type: string - example: user + type: string + example: idntusr-bailin example: - - id: idntusr-bailin - type: user - - id: idntusr-bailin-1 - type: user - - id: idntusr-bailin-2 - type: user - - id: idntusr-bailin-3 - type: user - - id: idntusr-bailin-4 - type: user - - id: idntusr-bailin-5 - type: user + - idntusr-bailin + - idntusr-bailin-1 + - idntusr-bailin-2 + - idntusr-bailin-3 + - idntusr-bailin-4 + - idntusr-bailin-5 + updated_at: + type: string + example: "2024-05-06T16:00:46Z" + updated_by: + type: string + example: idntusr-bailin examples: create-role-binding-group: value: - id: permrbn-96Cy2kTlt3jtZwjEF9gaO - role: - id: permrv2-hoN9DD27g1tfKzjF8kj42 - name: my_proj_doc_viewer - subjects: - - id: idntgrp-my-subgroup - type: group + created_at: "2024-05-06T16:04:08Z" + created_by: idntusr-bailin + id: permrbn-K652x2XPJO1mGJFUE4hEu + resource_id: tnntten-root + role_id: permrv2-PLjILDwe8kG_t42tMCDiB + subject_ids: + - idntgrp-my-subgroup + updated_at: "2024-05-06T16:04:08Z" + updated_by: idntusr-bailin root-super-user: value: - id: permrbn-wNpgDB-ajBTyR6nB-WBcY - role: - id: permrv2-nDw3bVXYwHysvZDFyxh2C - name: super_user - subjects: - - id: idntusr-bailin - type: user - - id: idntusr-bailin-1 - type: user - - id: idntusr-bailin-2 - type: user - - id: idntusr-bailin-3 - type: user - - id: idntusr-bailin-4 - type: user - - id: idntusr-bailin-5 - type: user + created_at: "2024-05-06T16:00:46Z" + created_by: idntusr-bailin + id: permrbn-lr5s4g6g1shmDL_htEAR_ + resource_id: tnntten-root + role_id: permrv2-PLjILDwe8kG_t42tMCDiB + subject_ids: + - idntusr-bailin + - idntusr-bailin-1 + - idntusr-bailin-2 + - idntusr-bailin-3 + - idntusr-bailin-4 + - idntusr-bailin-5 + updated_at: "2024-05-06T16:00:46Z" + updated_by: idntusr-bailin + "400": + description: create-role-binding-empty-subject + content: + application/json: + schema: + type: object + properties: + message: + type: string + example: 'error creating role-binding: invalid argument: role binding must have at least one subject' + examples: + create-role-binding-empty-subject: + value: + message: 'error creating role-binding: invalid argument: role binding must have at least one subject' parameters: - name: id in: path @@ -903,58 +947,56 @@ paths: schema: type: object properties: + created_at: + type: string + example: "2024-05-06T16:00:46Z" + created_by: + type: string + example: idntusr-bailin id: type: string - example: permrbn-pQByAAcMGUfS-ZvXPlrxp - role: - type: object - properties: - id: - type: string - example: permrv2-pUuZKw25TbCjoPnOYkA4w - name: - type: string - example: super-admin - subjects: + example: permrbn-lr5s4g6g1shmDL_htEAR_ + resource_id: + type: string + example: tnntten-root + role_id: + type: string + example: permrv2-PLjILDwe8kG_t42tMCDiB + subject_ids: type: array items: - type: object - properties: - id: - type: string - example: idntclt-xT33dia6DUHdcDXsGRUes - type: - type: string - example: client + type: string + example: idntusr-bailin example: - - id: idntclt-xT33dia6DUHdcDXsGRUes - type: client - - id: idntgrp-root-admins - type: group - - id: idntusr-DUrM80Cg2VzLVEhpmQb7ovmW1EG1isBnmIFpLTA5N2k - type: user - - id: idntusr-FYfXJww0qhW1XrF-WKupOQCQ9Q84d-ieEamjxz1Hzhs - type: user - - id: idntusr-bailin - type: user + - idntusr-bailin + - idntusr-bailin-1 + - idntusr-bailin-2 + - idntusr-bailin-3 + - idntusr-bailin-4 + - idntusr-bailin-5 + updated_at: + type: string + example: "2024-05-06T16:00:46Z" + updated_by: + type: string + example: idntusr-bailin examples: get-role-binding: value: - id: permrbn-pQByAAcMGUfS-ZvXPlrxp - role: - id: permrv2-pUuZKw25TbCjoPnOYkA4w - name: super-admin - subjects: - - id: idntclt-xT33dia6DUHdcDXsGRUes - type: client - - id: idntgrp-root-admins - type: group - - id: idntusr-DUrM80Cg2VzLVEhpmQb7ovmW1EG1isBnmIFpLTA5N2k - type: user - - id: idntusr-FYfXJww0qhW1XrF-WKupOQCQ9Q84d-ieEamjxz1Hzhs - type: user - - id: idntusr-bailin - type: user + created_at: "2024-05-06T16:00:46Z" + created_by: idntusr-bailin + id: permrbn-lr5s4g6g1shmDL_htEAR_ + resource_id: tnntten-root + role_id: permrv2-PLjILDwe8kG_t42tMCDiB + subject_ids: + - idntusr-bailin + - idntusr-bailin-1 + - idntusr-bailin-2 + - idntusr-bailin-3 + - idntusr-bailin-4 + - idntusr-bailin-5 + updated_at: "2024-05-06T16:00:46Z" + updated_by: idntusr-bailin delete: tags: - role-bindings @@ -972,118 +1014,95 @@ paths: update a role-binding, this will replace the subjects with the new subjects. note that role_id is immutable operationId: updateRoleBinding - parameters: - - name: id - in: query - schema: - type: string - example: loadbal-test-1 requestBody: content: application/json: schema: type: object properties: - subjects: + subject_ids: type: array items: - type: object - properties: - conditions: - type: object - properties: {} - id: - type: string - example: idntclt-xT33dia6DUHdcDXsGRUes + type: string + example: idntclt-xT33dia6DUHdcDXsGRUes example: - - id: idntclt-xT33dia6DUHdcDXsGRUes - - id: idntusr-DUrM80Cg2VzLVEhpmQb7ovmW1EG1isBnmIFpLTA5N2k - - id: idntusr-FYfXJww0qhW1XrF-WKupOQCQ9Q84d-ieEamjxz1Hzhs - - id: idntgrp-root-admins + - idntclt-xT33dia6DUHdcDXsGRUes + - idntusr-DUrM80Cg2VzLVEhpmQb7ovmW1EG1isBnmIFpLTA5N2k + - idntusr-FYfXJww0qhW1XrF-WKupOQCQ9Q84d-ieEamjxz1Hzhs + - idntgrp-root-admins + - idntusr-bailin examples: update-role-binding: value: - subjects: - - id: idntclt-xT33dia6DUHdcDXsGRUes - - id: idntusr-DUrM80Cg2VzLVEhpmQb7ovmW1EG1isBnmIFpLTA5N2k - - id: idntusr-FYfXJww0qhW1XrF-WKupOQCQ9Q84d-ieEamjxz1Hzhs - - id: idntgrp-root-admins + subject_ids: + - idntclt-xT33dia6DUHdcDXsGRUes + - idntusr-DUrM80Cg2VzLVEhpmQb7ovmW1EG1isBnmIFpLTA5N2k + - idntusr-FYfXJww0qhW1XrF-WKupOQCQ9Q84d-ieEamjxz1Hzhs + - idntgrp-root-admins + - idntusr-bailin responses: "200": description: update-role-binding - headers: - Content-Length: - schema: - type: string - example: "368" - Date: - schema: - type: string - example: Mon, 22 Apr 2024 21:52:22 GMT - X-Request-Id: - schema: - type: string - example: NzmFDigPmfVANuPeLMlMFtpykGKhGwvj content: application/json: schema: type: object properties: + created_at: + type: string + example: "2024-05-06T16:00:46Z" + created_by: + type: string + example: idntusr-bailin id: type: string - example: permrbn-pQByAAcMGUfS-ZvXPlrxp - role: - type: object - properties: - id: - type: string - example: permrv2-pUuZKw25TbCjoPnOYkA4w - name: - type: string - example: super-admin - subjects: + example: permrbn-lr5s4g6g1shmDL_htEAR_ + resource_id: + type: string + example: tnntten-root + role_id: + type: string + example: permrv2-PLjILDwe8kG_t42tMCDiB + subject_ids: type: array items: - type: object - properties: - id: - type: string - example: idntclt-xT33dia6DUHdcDXsGRUes - type: - type: string - example: client + type: string + example: idntclt-xT33dia6DUHdcDXsGRUes example: - - id: idntclt-xT33dia6DUHdcDXsGRUes - type: client - - id: idntusr-DUrM80Cg2VzLVEhpmQb7ovmW1EG1isBnmIFpLTA5N2k - type: user - - id: idntusr-FYfXJww0qhW1XrF-WKupOQCQ9Q84d-ieEamjxz1Hzhs - type: user - - id: idntgrp-root-admins - type: group + - idntclt-xT33dia6DUHdcDXsGRUes + - idntgrp-root-admins + - idntusr-DUrM80Cg2VzLVEhpmQb7ovmW1EG1isBnmIFpLTA5N2k + - idntusr-FYfXJww0qhW1XrF-WKupOQCQ9Q84d-ieEamjxz1Hzhs + - idntusr-bailin + updated_at: + type: string + example: "2024-05-06T16:09:14Z" + updated_by: + type: string + example: idntusr-bailin examples: update-role-binding: value: - id: permrbn-pQByAAcMGUfS-ZvXPlrxp - role: - id: permrv2-pUuZKw25TbCjoPnOYkA4w - name: super-admin - subjects: - - id: idntclt-xT33dia6DUHdcDXsGRUes - type: client - - id: idntusr-DUrM80Cg2VzLVEhpmQb7ovmW1EG1isBnmIFpLTA5N2k - type: user - - id: idntusr-FYfXJww0qhW1XrF-WKupOQCQ9Q84d-ieEamjxz1Hzhs - type: user - - id: idntgrp-root-admins - type: group + created_at: "2024-05-06T16:00:46Z" + created_by: idntusr-bailin + id: permrbn-lr5s4g6g1shmDL_htEAR_ + resource_id: tnntten-root + role_id: permrv2-PLjILDwe8kG_t42tMCDiB + subject_ids: + - idntclt-xT33dia6DUHdcDXsGRUes + - idntgrp-root-admins + - idntusr-DUrM80Cg2VzLVEhpmQb7ovmW1EG1isBnmIFpLTA5N2k + - idntusr-FYfXJww0qhW1XrF-WKupOQCQ9Q84d-ieEamjxz1Hzhs + - idntusr-bailin + updated_at: "2024-05-06T16:09:14Z" + updated_by: idntusr-bailin parameters: - name: rb-id in: path required: true schema: type: string - example: permrbn-uRNlQEo6yh9DinKhSxL2z + example: permrbn-lr5s4g6g1shmDL_htEAR_ /actions: get: summary: list-actions From d85ed3910707c757c3246d27a52acab01b133691 Mon Sep 17 00:00:00 2001 From: Bailin He <15058035+bailinhe@users.noreply.github.com> Date: Mon, 6 May 2024 14:50:11 -0400 Subject: [PATCH 07/10] Apply suggestions from code review Co-authored-by: John Schaeffer Signed-off-by: Bailin He <15058035+bailinhe@users.noreply.github.com> --- internal/query/relations.go | 4 +--- internal/query/roles_v2.go | 42 ++++++++++++++++----------------- internal/storage/rolebinding.go | 2 +- 3 files changed, 23 insertions(+), 25 deletions(-) diff --git a/internal/query/relations.go b/internal/query/relations.go index e969ddeb..2acbd1c9 100644 --- a/internal/query/relations.go +++ b/internal/query/relations.go @@ -1252,9 +1252,7 @@ func (e *engine) rollbackUpdates(ctx context.Context, updates []*pb.Relationship var op pb.RelationshipUpdate_Operation switch u.Operation { - case pb.RelationshipUpdate_OPERATION_CREATE: - fallthrough - case pb.RelationshipUpdate_OPERATION_TOUCH: + case pb.RelationshipUpdate_OPERATION_CREATE, pb.RelationshipUpdate_OPERATION_TOUCH: op = pb.RelationshipUpdate_OPERATION_DELETE case pb.RelationshipUpdate_OPERATION_DELETE: op = pb.RelationshipUpdate_OPERATION_TOUCH diff --git a/internal/query/roles_v2.go b/internal/query/roles_v2.go index 78a58be8..e13f4282 100644 --- a/internal/query/roles_v2.go +++ b/internal/query/roles_v2.go @@ -352,6 +352,26 @@ func (e *engine) DeleteRoleV2(ctx context.Context, roleResource types.Resource) 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 + } + // find all the bindings for the role findBindingsFilter := &pb.RelationshipFilter{ ResourceType: e.namespaced(e.rbac.RoleBindingResource.Name), @@ -362,7 +382,7 @@ func (e *engine) DeleteRoleV2(ctx context.Context, roleResource types.Resource) }, } - bindings, err := e.readRelationships(ctx, findBindingsFilter) + bindings, err := e.readRelationships(dbCtx, findBindingsFilter) if err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) @@ -379,26 +399,6 @@ func (e *engine) DeleteRoleV2(ctx context.Context, roleResource types.Resource) return err } - 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) diff --git a/internal/storage/rolebinding.go b/internal/storage/rolebinding.go index 1fbe5dc6..6b85181a 100644 --- a/internal/storage/rolebinding.go +++ b/internal/storage/rolebinding.go @@ -85,7 +85,7 @@ func (e *engine) ListResourceRoleBindings(ctx context.Context, resourceID gidx.P rows, err := db.QueryContext(ctx, ` SELECT id, resource_id, created_by, updated_by, created_at, updated_at - FROM rolebindings WHERE resource_id = $1 + FROM rolebindings WHERE resource_id = $1 ORDER BY created_at ASC `, resourceID.String(), ) if err != nil { From c4f142751a9a8f7602605ed268832260d4d4454e Mon Sep 17 00:00:00 2001 From: Bailin He Date: Tue, 7 May 2024 20:31:59 +0000 Subject: [PATCH 08/10] simplify example policy and add tests against it Signed-off-by: Bailin He --- internal/query/default_test.go | 10 +- internal/query/example_policy_test.go | 342 ++++++++++++++++++++++++++ internal/query/relations_test.go | 2 +- policies/policy.example.yaml | 153 ++++-------- 4 files changed, 397 insertions(+), 110 deletions(-) create mode 100644 internal/query/example_policy_test.go diff --git a/internal/query/default_test.go b/internal/query/default_test.go index ac2b5703..34f360b9 100644 --- a/internal/query/default_test.go +++ b/internal/query/default_test.go @@ -23,7 +23,7 @@ func DefaultPolicyDocumentV2() iapl.PolicyDocument { IDPrefix: "permrv2", }, RoleBindingResource: iapl.RBACResourceDefinition{ - Name: "role_binding", + Name: "rolebinding", IDPrefix: "permrbn", }, RoleSubjectTypes: []string{"user", "client"}, @@ -88,7 +88,7 @@ func DefaultPolicyDocumentV2() iapl.PolicyDocument { }, ResourceTypes: []iapl.ResourceType{ {Name: "rolev2", IDPrefix: "permrv2"}, - {Name: "role_binding", IDPrefix: "permrbn"}, + {Name: "rolebinding", IDPrefix: "permrbn"}, {Name: "user", IDPrefix: "idntusr"}, {Name: "client", IDPrefix: "idntclt"}, { @@ -100,7 +100,7 @@ func DefaultPolicyDocumentV2() iapl.PolicyDocument { 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"}}}, + {Relation: "grant", TargetTypes: []types.TargetType{{Name: "rolebinding"}}}, }, }, { @@ -112,7 +112,7 @@ func DefaultPolicyDocumentV2() iapl.PolicyDocument { 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"}}}, + {Relation: "grant", TargetTypes: []types.TargetType{{Name: "rolebinding"}}}, }, }, { @@ -123,7 +123,7 @@ func DefaultPolicyDocumentV2() iapl.PolicyDocument { }, Relationships: []iapl.Relationship{ {Relation: "owner", TargetTypes: []types.TargetType{{Name: "resourceowner_relationship"}}}, - {Relation: "grant", TargetTypes: []types.TargetType{{Name: "role_binding"}}}, + {Relation: "grant", TargetTypes: []types.TargetType{{Name: "rolebinding"}}}, }, }, }, diff --git a/internal/query/example_policy_test.go b/internal/query/example_policy_test.go new file mode 100644 index 00000000..8723c354 --- /dev/null +++ b/internal/query/example_policy_test.go @@ -0,0 +1,342 @@ +package query + +import ( + "context" + "fmt" + "testing" + + "go.infratographer.com/permissions-api/internal/iapl" + "go.infratographer.com/permissions-api/internal/testingx" + "go.infratographer.com/permissions-api/internal/types" + + v1 "github.com/authzed/authzed-go/proto/authzed/api/v1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.infratographer.com/x/gidx" +) + +const PolicyDir = "../../policies" + +/** + * create hierarchical tenants and groups + * this hierarchy ensures memberships, ownerships and inheritance are working + * as intended + * + * -------------- + * | tnntten-root | + * -------------- + * | + * | + * ----------- --------------- ------------------------ + * | tnntten-a | ----- | idntgrp-admin | ------ | idntgrp-admin-subgroup | + * ----------- --------------- ------------------------ + * / \ + * / \ + * ------------ ------------ + * | tnntten-b1 | | tnntten-b2 | + * ------------ ------------ + * | + * | + * ----------- + * | tnntten-c | + * ----------- + * | + * | + * ---------------- + * | loadbal-test-a | + * ---------------- + */ +func TestExamplePolicy(t *testing.T) { + namespace := "infratographertests" + ctx := context.Background() + + policy, err := iapl.NewPolicyFromDirectory(PolicyDir) + require.NoError(t, err) + + e := testEngine(ctx, t, namespace, policy) + + // all actions + allactions := []string{ + "iam_rolebinding_create", + "iam_rolebinding_delete", + "iam_rolebinding_get", + "iam_rolebinding_list", + "iam_rolebinding_update", + "loadbalancer_create", + "loadbalancer_delete", + "loadbalancer_get", + "loadbalancer_list", + "loadbalancer_update", + "role_create", + "role_delete", + "role_get", + "role_list", + "role_update", + } + + iamactions := []string{ + "iam_rolebinding_create", + "iam_rolebinding_delete", + "iam_rolebinding_get", + "iam_rolebinding_list", + "iam_rolebinding_update", + "role_create", + "role_delete", + "role_get", + "role_list", + "role_update", + } + + lbactions := []string{ + "loadbalancer_create", + "loadbalancer_delete", + "loadbalancer_get", + "loadbalancer_list", + "loadbalancer_update", + } + + lbactionsOnLB := []string{ + "loadbalancer_delete", + "loadbalancer_get", + "loadbalancer_update", + } + + // users + superuser := types.Resource{Type: "user", ID: "idntusr-superuser"} + haroldadmin := types.Resource{Type: "user", ID: "idntusr-harold-admin"} + theotheradmin := types.Resource{Type: "user", ID: "idntusr-the-other-admin"} + + // tenants + tnnttenroot := types.Resource{Type: "tenant", ID: gidx.PrefixedID("tnntten-root")} + tnnttena := types.Resource{Type: "tenant", ID: gidx.PrefixedID("tnntten-a")} + tnnttenb1 := types.Resource{Type: "tenant", ID: gidx.PrefixedID("tnntten-b1")} + tnnttenb2 := types.Resource{Type: "tenant", ID: gidx.PrefixedID("tnntten-b2")} + tnnttenc := types.Resource{Type: "tenant", ID: gidx.PrefixedID("tnntten-c")} + + // groups + groupadmin := types.Resource{Type: "group", ID: "idntgrp-admin"} + groupadminsubgroup := types.Resource{Type: "group", ID: "idntgrp-admin-subgroup"} + + // resources + lbtesta := types.Resource{Type: "loadbalancer", ID: "loadbal-test-a"} + + // create hierarchical tenants and groups + err = e.CreateRelationships(ctx, []types.Relationship{ + // tenants + { + Resource: tnnttena, + Relation: "parent", + Subject: tnnttenroot, + }, + { + Resource: tnnttenb1, + Relation: "parent", + Subject: tnnttena, + }, + { + Resource: tnnttenb2, + Relation: "parent", + Subject: tnnttena, + }, + { + Resource: tnnttenc, + Relation: "parent", + Subject: tnnttenb1, + }, + // groups + { + Resource: groupadmin, + Relation: "parent", + Subject: tnnttena, + }, + { + Resource: groupadminsubgroup, + Relation: "parent", + Subject: groupadmin, + }, + { + Resource: groupadmin, + Relation: "subgroup", + Subject: groupadminsubgroup, + }, + { + Resource: groupadmin, + Relation: "direct_member", + Subject: theotheradmin, + }, + { + Resource: groupadminsubgroup, + Relation: "direct_member", + Subject: haroldadmin, + }, + // resources + { + Resource: lbtesta, + Relation: "owner", + Subject: tnnttenc, + }, + }) + require.NoError(t, err) + + // create roles + superadmin, err := e.CreateRoleV2(ctx, superuser, tnnttenroot, "superuser", allactions) + require.NoError(t, err) + + iamadmin, err := e.CreateRoleV2(ctx, superuser, tnnttena, "iam_admin", iamactions) + require.NoError(t, err) + + lbadmin, err := e.CreateRoleV2(ctx, superuser, tnnttenroot, "lb_admin", lbactions) + require.NoError(t, err) + + tc := []testingx.TestCase[any, any]{ + { + Name: "superuser can do anything", + SetupFn: func(ctx context.Context, t *testing.T) context.Context { + role := types.Resource{Type: "role", ID: superadmin.ID} + _, err := e.CreateRoleBinding(ctx, superuser, tnnttenroot, role, []types.RoleBindingSubject{{SubjectResource: superuser}}) + require.NoError(t, err) + + return ctx + }, + CheckFn: func(ctx context.Context, t *testing.T, tr testingx.TestResult[any]) { + res := []types.Resource{tnnttenroot, tnnttena, tnnttenb1, tnnttenb2, tnnttenc} + + for _, r := range res { + for _, a := range allactions { + err := e.checkPermission(ctx, &v1.CheckPermissionRequest{ + Consistency: &v1.Consistency{Requirement: &v1.Consistency_FullyConsistent{FullyConsistent: true}}, + Resource: resourceToSpiceDBRef(e.namespace, r), + Subject: &v1.SubjectReference{Object: resourceToSpiceDBRef(e.namespace, superuser)}, + Permission: a, + }) + assert.NoError(t, err, fmt.Sprintf("superuser should have permission %s on %s", a, r.ID)) + } + } + + for _, a := range lbactionsOnLB { + err := e.checkPermission(ctx, &v1.CheckPermissionRequest{ + Consistency: &v1.Consistency{Requirement: &v1.Consistency_FullyConsistent{FullyConsistent: true}}, + Resource: resourceToSpiceDBRef(e.namespace, lbtesta), + Subject: &v1.SubjectReference{Object: resourceToSpiceDBRef(e.namespace, superuser)}, + Permission: a, + }) + assert.NoError(t, err, fmt.Sprintf("superuser should have permission %s on %s", a, lbtesta.ID)) + } + }, + }, + { + Name: "the other admin can manage lbs under tnntten-a", + Sync: true, + SetupFn: func(ctx context.Context, t *testing.T) context.Context { + role := types.Resource{Type: "role", ID: lbadmin.ID} + _, err := e.CreateRoleBinding(ctx, superuser, tnnttena, role, []types.RoleBindingSubject{{SubjectResource: groupadmin}}) + require.NoError(t, err) + + return ctx + }, + CheckFn: func(ctx context.Context, t *testing.T, tr testingx.TestResult[any]) { + res := []types.Resource{tnnttena, tnnttenb1, tnnttenb2, tnnttenc} + + forbidden := iamactions + allowed := lbactions + + // the other admin has no permissions on tnntten-root + for _, r := range res { + for _, a := range allowed { + err := e.checkPermission(ctx, &v1.CheckPermissionRequest{ + Consistency: &v1.Consistency{Requirement: &v1.Consistency_FullyConsistent{FullyConsistent: true}}, + Resource: resourceToSpiceDBRef(e.namespace, r), + Subject: &v1.SubjectReference{Object: resourceToSpiceDBRef(e.namespace, theotheradmin)}, + Permission: a, + }) + assert.NoError(t, err, fmt.Sprintf("the other admin should have permission %s on %s", a, r.ID)) + } + for _, a := range forbidden { + err := e.checkPermission(ctx, &v1.CheckPermissionRequest{ + Consistency: &v1.Consistency{Requirement: &v1.Consistency_FullyConsistent{FullyConsistent: true}}, + Resource: resourceToSpiceDBRef(e.namespace, r), + Subject: &v1.SubjectReference{Object: resourceToSpiceDBRef(e.namespace, theotheradmin)}, + Permission: a, + }) + assert.Error(t, err, fmt.Sprintf("the other admin should not have permission %s on %s", a, r.ID)) + } + } + + for _, a := range lbactionsOnLB { + err := e.checkPermission(ctx, &v1.CheckPermissionRequest{ + Consistency: &v1.Consistency{Requirement: &v1.Consistency_FullyConsistent{FullyConsistent: true}}, + Resource: resourceToSpiceDBRef(e.namespace, lbtesta), + Subject: &v1.SubjectReference{Object: resourceToSpiceDBRef(e.namespace, haroldadmin)}, + Permission: a, + }) + assert.NoError(t, err, fmt.Sprintf(" should have permission %s on %s", a, lbtesta.ID)) + } + }, + }, + { + // lb-admin permissions should be inherited from group-admin to group-admin-subgroup + // iam-admin + lb-admin = superuser + Name: "harold-admin can do anything under tnntten-a", + Sync: true, + SetupFn: func(ctx context.Context, t *testing.T) context.Context { + role := types.Resource{Type: "role", ID: iamadmin.ID} + _, err := e.CreateRoleBinding(ctx, superuser, tnnttena, role, []types.RoleBindingSubject{{SubjectResource: groupadminsubgroup}}) + require.NoError(t, err) + + return ctx + }, + CheckFn: func(ctx context.Context, t *testing.T, tr testingx.TestResult[any]) { + nopermRes := tnnttenroot + res := []types.Resource{tnnttena, tnnttenb1, tnnttenb2, tnnttenc} + + // harold-admin has no permissions on tnntten-root + for _, a := range allactions { + err := e.checkPermission(ctx, &v1.CheckPermissionRequest{ + Consistency: &v1.Consistency{Requirement: &v1.Consistency_FullyConsistent{FullyConsistent: true}}, + Resource: resourceToSpiceDBRef(e.namespace, nopermRes), + Subject: &v1.SubjectReference{Object: resourceToSpiceDBRef(e.namespace, haroldadmin)}, + Permission: a, + }) + assert.Error(t, err, fmt.Sprintf("harold-admin should have no permission %s", nopermRes.ID)) + } + + for _, r := range res { + for _, a := range allactions { + err := e.checkPermission(ctx, &v1.CheckPermissionRequest{ + Consistency: &v1.Consistency{Requirement: &v1.Consistency_FullyConsistent{FullyConsistent: true}}, + Resource: resourceToSpiceDBRef(e.namespace, r), + Subject: &v1.SubjectReference{Object: resourceToSpiceDBRef(e.namespace, haroldadmin)}, + Permission: a, + }) + assert.NoError(t, err, fmt.Sprintf("harold-admin should have permission %s on %s", a, r.ID)) + } + } + + for _, a := range lbactionsOnLB { + err := e.checkPermission(ctx, &v1.CheckPermissionRequest{ + Consistency: &v1.Consistency{Requirement: &v1.Consistency_FullyConsistent{FullyConsistent: true}}, + Resource: resourceToSpiceDBRef(e.namespace, lbtesta), + Subject: &v1.SubjectReference{Object: resourceToSpiceDBRef(e.namespace, haroldadmin)}, + Permission: a, + }) + assert.NoError(t, err, fmt.Sprintf("harold-admin should have permission %s on %s", a, lbtesta.ID)) + } + }, + }, + { + Name: "iam-admin cannot be bind on tnntten-root", + CheckFn: func(ctx context.Context, t *testing.T, tr testingx.TestResult[any]) { + role := types.Resource{Type: "role", ID: iamadmin.ID} + _, err := e.CreateRoleBinding(ctx, superuser, tnnttenroot, role, []types.RoleBindingSubject{{SubjectResource: groupadminsubgroup}}) + assert.Error(t, err) + assert.ErrorIs(t, err, ErrRoleNotFound) + }, + }, + } + + testFn := func(ctx context.Context, in any) testingx.TestResult[any] { + return testingx.TestResult[any]{} + } + + testingx.RunTests(ctx, t, tc, testFn) +} diff --git a/internal/query/relations_test.go b/internal/query/relations_test.go index 8f64a5dd..2c6d146e 100644 --- a/internal/query/relations_test.go +++ b/internal/query/relations_test.go @@ -90,7 +90,7 @@ func testPolicy() iapl.Policy { } func cleanDB(ctx context.Context, t *testing.T, client *authzed.Client, namespace string) { - for _, dbType := range []string{"user", "client", "role", "tenant"} { + for _, dbType := range []string{"user", "client", "role", "tenant", "group", "loadbalancer", "rolev2", "tenant", "rolebinding"} { namespacedType := namespace + "/" + dbType delRequest := &pb.DeleteRelationshipsRequest{ RelationshipFilter: &pb.RelationshipFilter{ diff --git a/policies/policy.example.yaml b/policies/policy.example.yaml index 5c52a1f7..1638d5e1 100644 --- a/policies/policy.example.yaml +++ b/policies/policy.example.yaml @@ -17,47 +17,24 @@ rbac: subjectrelation: member unions: - - name: group_member - resourcetypes: - - name: user - - name: client - - name: group - subjectrelation: member - - name: tenant_member - resourcetypes: - - name: user - - name: client - - name: group - subjectrelation: member - - name: tenant - subjectrelation: member - name: resourceowner resourcetypes: - name: tenant - - name: resourceowner_relationship + - name: resourcemanager resourcetypes: - name: tenant - - name: tenant - subjectrelation: parent + - name: group - name: subject resourcetypes: - name: user - name: client - - name: group_parent - resourcetypes: - - name: group - - name: group - subjectrelation: parent - - name: tenant - - name: tenant - subjectrelation: parent - - name: tenant_parent - resourcetypes: - - name: tenant - - name: tenant - subjectrelation: parent resourcetypes: + - name: user + idprefix: idntusr + - name: client + idprefix: idntclt + - name: role idprefix: permrol relationships: @@ -65,42 +42,38 @@ resourcetypes: targettypes: - name: subject - - name: user - idprefix: idntusr - - name: client - idprefix: idntclt - - - name: group - idprefix: idntgrp + - name: tenant + idprefix: tnntten rolebindingv2: - &rolesFromParent + &permsFromParent inheritpermissionsfrom: - parent relationships: - relation: parent targettypes: - - name: group_parent - - relation: member - targettypes: - - name: group_member - - relation: grant + - name: tenant + - &grantRel + relation: grant targettypes: - name: rolebinding - - name: tenant - idprefix: tnntten + - name: group + idprefix: idntgrp rolebindingv2: - *rolesFromParent + *permsFromParent relationships: + - *grantRel - relation: parent targettypes: - - name: tenant_parent - - relation: member + - name: group + - name: tenant + - relation: direct_member targettypes: - - name: tenant_member - - relation: grant + - name: user + - name: client + - relation: subgroup targettypes: - - name: rolebinding + - name: group - name: loadbalancer idprefix: loadbal @@ -110,7 +83,7 @@ resourcetypes: relationships: - relation: owner targettypes: - - name: resourceowner_relationship + - name: resourceowner - relation: grant targettypes: - name: rolebinding @@ -126,8 +99,19 @@ actions: - name: loadbalancer_list - name: loadbalancer_update - name: loadbalancer_delete + - name: member actionbindings: + # subgroup and group members + - actionname: member + typename: group + conditions: + - relationshipaction: + relation: direct_member + - relationshipaction: + relation: subgroup + actionname: member + # role management - permissions on role - actionname: role_get typename: rolev2 @@ -147,56 +131,37 @@ actionbindings: - relationshipaction: relation: owner actionname: role_delete - # role management - permissions on owner + + # role management - permissions on owners and managers - actionname: role_create - typename: resourceowner + typename: resourcemanager conditions: - rolebindingv2: {} - rolebinding: {} - - actionname: role_create - typename: group - conditions: - - rolebindingv2: {} - actionname: role_get - typename: resourceowner + typename: resourcemanager conditions: - rolebindingv2: {} - rolebinding: {} - - actionname: role_get - typename: group - conditions: - - rolebindingv2: {} - actionname: role_list - typename: resourceowner + typename: resourcemanager conditions: - rolebindingv2: {} - rolebinding: {} - - actionname: role_list - typename: group - conditions: - - rolebindingv2: {} - actionname: role_update - typename: resourceowner + typename: resourcemanager conditions: - rolebindingv2: {} - rolebinding: {} - - actionname: role_update - typename: group - conditions: - - rolebindingv2: {} - actionname: role_delete - typename: resourceowner + typename: resourcemanager conditions: - rolebindingv2: {} - rolebinding: {} - - actionname: role_delete - typename: group - conditions: - - rolebindingv2: {} # loadbalancer management - permissions on loadbalancer - actionname: loadbalancer_get @@ -215,53 +180,33 @@ actionbindings: - rolebinding: {} - rolebindingv2: {} - # loadbalancer management - permissions on owner + # loadbalancer management - permissions on owners and managers - actionname: loadbalancer_create - typename: resourceowner + typename: resourcemanager conditions: - rolebindingv2: {} - rolebinding: {} - - actionname: loadbalancer_create - typename: group - conditions: - - rolebindingv2: {} - actionname: loadbalancer_get - typename: resourceowner + typename: resourcemanager conditions: - rolebindingv2: {} - rolebinding: {} - - actionname: loadbalancer_get - typename: group - conditions: - - rolebindingv2: {} - actionname: loadbalancer_list - typename: resourceowner + typename: resourcemanager conditions: - rolebindingv2: {} - rolebinding: {} - - actionname: loadbalancer_list - typename: group - conditions: - - rolebindingv2: {} - actionname: loadbalancer_update - typename: resourceowner + typename: resourcemanager conditions: - rolebindingv2: {} - rolebinding: {} - - actionname: loadbalancer_update - typename: group - conditions: - - rolebindingv2: {} - actionname: loadbalancer_delete - typename: resourceowner + typename: resourcemanager conditions: - rolebindingv2: {} - rolebinding: {} - - actionname: loadbalancer_delete - typename: group - conditions: - - rolebindingv2: {} From 77784a82529fadbc629e2b68c8eb71ecf8d31f5d Mon Sep 17 00:00:00 2001 From: Bailin He Date: Tue, 7 May 2024 20:44:44 +0000 Subject: [PATCH 09/10] fix typo Signed-off-by: Bailin He --- internal/query/example_policy_test.go | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/internal/query/example_policy_test.go b/internal/query/example_policy_test.go index 8723c354..3446c611 100644 --- a/internal/query/example_policy_test.go +++ b/internal/query/example_policy_test.go @@ -102,9 +102,9 @@ func TestExamplePolicy(t *testing.T) { } // users - superuser := types.Resource{Type: "user", ID: "idntusr-superuser"} - haroldadmin := types.Resource{Type: "user", ID: "idntusr-harold-admin"} - theotheradmin := types.Resource{Type: "user", ID: "idntusr-the-other-admin"} + superuser := types.Resource{Type: "user", ID: gidx.PrefixedID("idntusr-superuser")} + haroldadmin := types.Resource{Type: "user", ID: gidx.PrefixedID("idntusr-harold-admin")} + theotheradmin := types.Resource{Type: "user", ID: gidx.PrefixedID("idntusr-the-other-admin")} // tenants tnnttenroot := types.Resource{Type: "tenant", ID: gidx.PrefixedID("tnntten-root")} @@ -114,11 +114,11 @@ func TestExamplePolicy(t *testing.T) { tnnttenc := types.Resource{Type: "tenant", ID: gidx.PrefixedID("tnntten-c")} // groups - groupadmin := types.Resource{Type: "group", ID: "idntgrp-admin"} - groupadminsubgroup := types.Resource{Type: "group", ID: "idntgrp-admin-subgroup"} + groupadmin := types.Resource{Type: "group", ID: gidx.PrefixedID("idntgrp-admin")} + groupadminsubgroup := types.Resource{Type: "group", ID: gidx.PrefixedID("idntgrp-admin-subgroup")} // resources - lbtesta := types.Resource{Type: "loadbalancer", ID: "loadbal-test-a"} + lbtesta := types.Resource{Type: "loadbalancer", ID: gidx.PrefixedID("loadbal-test-a")} // create hierarchical tenants and groups err = e.CreateRelationships(ctx, []types.Relationship{ @@ -240,7 +240,6 @@ func TestExamplePolicy(t *testing.T) { forbidden := iamactions allowed := lbactions - // the other admin has no permissions on tnntten-root for _, r := range res { for _, a := range allowed { err := e.checkPermission(ctx, &v1.CheckPermissionRequest{ From be498111d061aa024dc5ba87f654c92568a0d3b8 Mon Sep 17 00:00:00 2001 From: Bailin He Date: Tue, 7 May 2024 21:06:59 +0000 Subject: [PATCH 10/10] fix cleanups Signed-off-by: Bailin He --- internal/query/relations_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/internal/query/relations_test.go b/internal/query/relations_test.go index 2c6d146e..b01a6270 100644 --- a/internal/query/relations_test.go +++ b/internal/query/relations_test.go @@ -51,7 +51,7 @@ func testEngine(ctx context.Context, t *testing.T, namespace string, policy iapl require.NoError(t, err) t.Cleanup(func() { - cleanDB(ctx, t, client, namespace) + cleanDB(ctx, t, client, namespace, policy) cleanStore() }) @@ -89,8 +89,9 @@ func testPolicy() iapl.Policy { return policy } -func cleanDB(ctx context.Context, t *testing.T, client *authzed.Client, namespace string) { - for _, dbType := range []string{"user", "client", "role", "tenant", "group", "loadbalancer", "rolev2", "tenant", "rolebinding"} { +func cleanDB(ctx context.Context, t *testing.T, client *authzed.Client, namespace string, p iapl.Policy) { + for _, resourceType := range p.Schema() { + dbType := resourceType.Name namespacedType := namespace + "/" + dbType delRequest := &pb.DeleteRelationshipsRequest{ RelationshipFilter: &pb.RelationshipFilter{