diff --git a/pkg/registry/apis/iam/resource_permission_hooks.go b/pkg/registry/apis/iam/resource_permission_hooks.go index 1de5bb9f110..e725726b2ec 100644 --- a/pkg/registry/apis/iam/resource_permission_hooks.go +++ b/pkg/registry/apis/iam/resource_permission_hooks.go @@ -3,118 +3,22 @@ package iam import ( "context" "errors" - "fmt" - "strings" "time" - "google.golang.org/protobuf/types/known/structpb" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apiserver/pkg/registry/generic/registry" iamv0 "github.com/grafana/grafana/apps/iam/pkg/apis/iam/v0alpha1" v1 "github.com/grafana/grafana/pkg/services/authz/proto/v1" - "github.com/grafana/grafana/pkg/services/authz/zanzana" ) var ( - errEmptyName = errors.New("name cannot be empty") - errInvalidBasicRole = errors.New("invalid basic role") - errUnknownKind = errors.New("unknown permission kind") + errEmptyName = errors.New("name cannot be empty") defaultWriteTimeout = 15 * time.Second ) -func toZanzanaSubject(kind iamv0.ResourcePermissionSpecPermissionKind, name string) (string, error) { - if name == "" { - return "", errEmptyName - } - switch kind { - case iamv0.ResourcePermissionSpecPermissionKindUser: - return zanzana.NewTupleEntry(zanzana.TypeUser, name, ""), nil - case iamv0.ResourcePermissionSpecPermissionKindServiceAccount: - return zanzana.NewTupleEntry(zanzana.TypeServiceAccount, name, ""), nil - case iamv0.ResourcePermissionSpecPermissionKindTeam: - return zanzana.NewTupleEntry(zanzana.TypeTeam, name, zanzana.RelationTeamMember), nil - case iamv0.ResourcePermissionSpecPermissionKindBasicRole: - basicRole := zanzana.TranslateBasicRole(name) - if basicRole == "" { - return "", fmt.Errorf("%w: %s", errInvalidBasicRole, name) - } - - // e.g role:basic_viewer#assignee - return zanzana.NewTupleEntry(zanzana.TypeRole, basicRole, zanzana.RelationAssignee), nil - } - - // should not happen since we are after create - // validation webhook should have caught invalid kinds - return "", errUnknownKind -} - -func toZanzanaType(apiGroup string) string { - if apiGroup == "folder.grafana.app" { - return zanzana.TypeFolder - } - return zanzana.TypeResource -} - -func NewResourceTuple(object string, resource iamv0.ResourcePermissionspecResource, perm iamv0.ResourcePermissionspecPermission) (*v1.TupleKey, error) { - // Typ is "folder" or "resource" - typ := toZanzanaType(resource.ApiGroup) - - // subject - subject, err := toZanzanaSubject(perm.Kind, perm.Name) - if err != nil { - return nil, err - } - - key := &v1.TupleKey{ - // e.g. "user:{uid}", "serviceaccount:{uid}", "team:{uid}", "basicrole:{viewer|editor|admin}" - User: subject, - // "view", "edit", "admin" - Relation: strings.ToLower(perm.Verb), - // e.g. "folder:{name}" or "resource:{apiGroup}/{resource}/{name}" - Object: object, - } - - // For resources we add a condition to filter by apiGroup/resource - // e.g "group_filter": {"group_resource": "dashboards.grafana.app/dashboards"} - if typ == zanzana.TypeResource { - key.Condition = &v1.RelationshipCondition{ - Name: "group_filter", - Context: &structpb.Struct{ - Fields: map[string]*structpb.Value{ - "group_resource": structpb.NewStringValue( - resource.ApiGroup + "/" + resource.Resource, - ), - }, - }, - } - } - - return key, nil -} - -// tupleToTupleKeyWithoutCondition converts a TupleKey to TupleKeyWithoutCondition -// This is needed for delete operations which don't support conditions -func tupleToTupleKeyWithoutCondition(tuple *v1.TupleKey) *v1.TupleKeyWithoutCondition { - return &v1.TupleKeyWithoutCondition{ - User: tuple.User, - Relation: tuple.Relation, - Object: tuple.Object, - } -} - -// toTupleKeysWithoutCondition converts v1.TupleKey to v1.TupleKeyWithoutCondition -// by stripping the condition field, which is required for delete operations -func toTupleKeysWithoutCondition(tuples []*v1.TupleKey) []*v1.TupleKeyWithoutCondition { - result := make([]*v1.TupleKeyWithoutCondition, len(tuples)) - for i, t := range tuples { - result[i] = tupleToTupleKeyWithoutCondition(t) - } - return result -} - // AfterResourcePermissionCreate is a post-create hook that writes the resource permission to Zanzana (openFGA) func (b *IdentityAccessManagementAPIBuilder) AfterResourcePermissionCreate(obj runtime.Object, _ *metav1.CreateOptions) { if b.zClient == nil { @@ -151,56 +55,54 @@ func (b *IdentityAccessManagementAPIBuilder) AfterResourcePermissionCreate(obj r resource := rp.Spec.Resource permissions := rp.Spec.Permissions - object := zanzana.NewObjectEntry(toZanzanaType(resource.ApiGroup), resource.ApiGroup, resource.Resource, "", resource.Name) - - tuples := make([]*v1.TupleKey, 0, len(permissions)) + operations := make([]*v1.MutateOperation, 0, len(permissions)) for _, p := range permissions { - tuple, err := NewResourceTuple(object, resource, p) - if err != nil { - b.logger.Error("failed to create resource permission tuple", - "namespace", rp.Namespace, - "object", object, - "err", err, - ) - - continue - } - tuples = append(tuples, tuple) + operations = append(operations, &v1.MutateOperation{ + Operation: &v1.MutateOperation_CreatePermission{ + CreatePermission: &v1.CreatePermissionOperation{ + Resource: &v1.Resource{ + Group: resource.ApiGroup, + Resource: resource.Resource, + Name: resource.Name, + }, + Permission: &v1.Permission{ + Kind: string(p.Kind), + Name: p.Name, + Verb: p.Verb, + }, + }, + }, + }) } - // Avoid writing if there are no valid tuples - if len(tuples) == 0 { - b.logger.Warn("no valid tuples to write", "namespace", rp.Namespace, "resource", object) - status = "failure" + if len(operations) == 0 { return } b.logger.Debug("writing resource permission to zanzana", "namespace", rp.Namespace, - "object", object, - "tuplesCnt", len(tuples), + "resource", resource, + "operationsCount", len(operations), ) ctx, cancel := context.WithTimeout(context.Background(), defaultWriteTimeout) defer cancel() - err := b.zClient.Write(ctx, &v1.WriteRequest{ - Namespace: rp.Namespace, - Writes: &v1.WriteRequestWrites{ - TupleKeys: tuples, - }, + err := b.zClient.Mutate(ctx, &v1.MutateRequest{ + Namespace: rp.Namespace, + Operations: operations, }) if err != nil { status = "failure" b.logger.Error("failed to write resource permission to zanzana", "err", err, "namespace", rp.Namespace, - "object", object, - "tuplesCnt", len(tuples), + "resource", resource, + "operationsCount", len(operations), ) } else { // Record successful tuple writes - hooksTuplesCounter.WithLabelValues(resourceType, operation, "write").Add(float64(len(tuples))) + hooksTuplesCounter.WithLabelValues(resourceType, operation, "write").Add(float64(len(operations))) } }(rp.DeepCopy()) // Pass a copy of the object } @@ -223,45 +125,51 @@ func (b *IdentityAccessManagementAPIBuilder) BeginResourcePermissionUpdate(ctx c return nil, nil } - // Convert old permissions to tuples for deletion - var oldTuples []*v1.TupleKey + // Convert old permissions to delete operations + deleteOperations := make([]*v1.MutateOperation, 0, len(oldRP.Spec.Permissions)) if len(oldRP.Spec.Permissions) > 0 { oldResource := oldRP.Spec.Resource - oldObject := zanzana.NewObjectEntry(toZanzanaType(oldResource.ApiGroup), oldResource.ApiGroup, oldResource.Resource, "", oldResource.Name) - - oldTuples = make([]*v1.TupleKey, 0, len(oldRP.Spec.Permissions)) for _, p := range oldRP.Spec.Permissions { - tuple, err := NewResourceTuple(oldObject, oldResource, p) - if err != nil { - b.logger.Error("failed to create old resource permission tuple", - "namespace", oldRP.Namespace, - "object", oldObject, - "err", err, - ) - continue - } - oldTuples = append(oldTuples, tuple) + deleteOperations = append(deleteOperations, &v1.MutateOperation{ + Operation: &v1.MutateOperation_DeletePermission{ + DeletePermission: &v1.DeletePermissionOperation{ + Resource: &v1.Resource{ + Group: oldResource.ApiGroup, + Resource: oldResource.Resource, + Name: oldResource.Name, + }, + Permission: &v1.Permission{ + Kind: string(p.Kind), + Name: p.Name, + Verb: p.Verb, + }, + }, + }, + }) } } - // Convert new permissions to tuples for writing - var newTuples []*v1.TupleKey + // Convert new permissions to create operations + createOperations := make([]*v1.MutateOperation, 0, len(newRP.Spec.Permissions)) if len(newRP.Spec.Permissions) > 0 { newResource := newRP.Spec.Resource - newObject := zanzana.NewObjectEntry(toZanzanaType(newResource.ApiGroup), newResource.ApiGroup, newResource.Resource, "", newResource.Name) - - newTuples = make([]*v1.TupleKey, 0, len(newRP.Spec.Permissions)) for _, p := range newRP.Spec.Permissions { - tuple, err := NewResourceTuple(newObject, newResource, p) - if err != nil { - b.logger.Error("failed to create new resource permission tuple", - "namespace", newRP.Namespace, - "object", newObject, - "err", err, - ) - continue - } - newTuples = append(newTuples, tuple) + createOperations = append(createOperations, &v1.MutateOperation{ + Operation: &v1.MutateOperation_CreatePermission{ + CreatePermission: &v1.CreatePermissionOperation{ + Resource: &v1.Resource{ + Group: newResource.ApiGroup, + Resource: newResource.Resource, + Name: newResource.Name, + }, + Permission: &v1.Permission{ + Kind: string(p.Kind), + Name: p.Name, + Verb: p.Verb, + }, + }, + }, + }) } } @@ -299,36 +207,32 @@ func (b *IdentityAccessManagementAPIBuilder) BeginResourcePermissionUpdate(ctx c defer cancel() // Prepare write request - req := &v1.WriteRequest{ - Namespace: newRP.Namespace, + req := &v1.MutateRequest{ + Namespace: newRP.Namespace, + Operations: append(deleteOperations, createOperations...), } - // Add deletes for old tuples - if len(oldTuples) > 0 { - deleteTuples := toTupleKeysWithoutCondition(oldTuples) - req.Deletes = &v1.WriteRequestDeletes{ - TupleKeys: deleteTuples, - } + if len(req.Operations) == 0 { + return + } + + if len(deleteOperations) > 0 { b.logger.Debug("deleting existing resource permissions from zanzana", "namespace", newRP.Namespace, - "tuplesCnt", len(deleteTuples), + "operationsCount", len(deleteOperations), ) } - // Add writes for new tuples - if len(newTuples) > 0 { - req.Writes = &v1.WriteRequestWrites{ - TupleKeys: newTuples, - } + if len(createOperations) > 0 { b.logger.Debug("writing new resource permissions to zanzana", "namespace", newRP.Namespace, - "tuplesCnt", len(newTuples), + "operationsCount", len(createOperations), ) } // Only make the request if there are deletes or writes - if (req.Deletes != nil && len(req.Deletes.TupleKeys) > 0) || (req.Writes != nil && len(req.Writes.TupleKeys) > 0) { - err := b.zClient.Write(ctx, req) + if len(req.Operations) > 0 { + err := b.zClient.Mutate(ctx, req) if err != nil { status = "failure" b.logger.Error("failed to update resource permission in zanzana", @@ -337,11 +241,11 @@ func (b *IdentityAccessManagementAPIBuilder) BeginResourcePermissionUpdate(ctx c ) } else { // Record successful tuple operations - if len(oldTuples) > 0 { - hooksTuplesCounter.WithLabelValues("resourcepermission", "update", "delete").Add(float64(len(oldTuples))) + if len(deleteOperations) > 0 { + hooksTuplesCounter.WithLabelValues("resourcepermission", "update", "delete").Add(float64(len(deleteOperations))) } - if len(newTuples) > 0 { - hooksTuplesCounter.WithLabelValues("resourcepermission", "update", "write").Add(float64(len(newTuples))) + if len(createOperations) > 0 { + hooksTuplesCounter.WithLabelValues("resourcepermission", "update", "write").Add(float64(len(createOperations))) } } } else { @@ -387,56 +291,56 @@ func (b *IdentityAccessManagementAPIBuilder) AfterResourcePermissionDelete(obj r resource := rp.Spec.Resource permissions := rp.Spec.Permissions - object := zanzana.NewObjectEntry(toZanzanaType(resource.ApiGroup), resource.ApiGroup, resource.Resource, "", resource.Name) - // Generate delete tuples from the permissions - deleteTuples := make([]*v1.TupleKeyWithoutCondition, 0, len(permissions)) + deleteOperations := make([]*v1.MutateOperation, 0, len(permissions)) for _, p := range permissions { - tuple, err := NewResourceTuple(object, resource, p) - if err != nil { - b.logger.Error("failed to create resource permission tuple for deletion", - "namespace", rp.Namespace, - "object", object, - "err", err, - ) - continue - } - deleteTuples = append(deleteTuples, tupleToTupleKeyWithoutCondition(tuple)) + deleteOperations = append(deleteOperations, &v1.MutateOperation{ + Operation: &v1.MutateOperation_DeletePermission{ + DeletePermission: &v1.DeletePermissionOperation{ + Resource: &v1.Resource{ + Group: resource.ApiGroup, + Resource: resource.Resource, + Name: resource.Name, + }, + Permission: &v1.Permission{ + Kind: string(p.Kind), + Name: p.Name, + Verb: p.Verb, + }, + }, + }, + }) } // Avoid writing if there are no valid tuples - if len(deleteTuples) == 0 { - b.logger.Warn("no valid tuples to delete", "namespace", rp.Namespace, "resource", object) - status = "failure" + if len(deleteOperations) == 0 { return } b.logger.Debug("deleting resource permission from zanzana", "namespace", rp.Namespace, - "object", object, - "tuplesCnt", len(deleteTuples), + "resource", resource, + "operationsCount", len(deleteOperations), ) ctx, cancel := context.WithTimeout(context.Background(), defaultWriteTimeout) defer cancel() - err := b.zClient.Write(ctx, &v1.WriteRequest{ - Namespace: rp.Namespace, - Deletes: &v1.WriteRequestDeletes{ - TupleKeys: deleteTuples, - }, + err := b.zClient.Mutate(ctx, &v1.MutateRequest{ + Namespace: rp.Namespace, + Operations: deleteOperations, }) if err != nil { status = "failure" b.logger.Error("failed to delete resource permission from zanzana", "err", err, "namespace", rp.Namespace, - "object", object, - "tuplesCnt", len(deleteTuples), + "resource", resource, + "operationsCount", len(deleteOperations), ) } else { // Record successful tuple deletions - hooksTuplesCounter.WithLabelValues(resourceType, operation, "delete").Add(float64(len(deleteTuples))) + hooksTuplesCounter.WithLabelValues(resourceType, operation, "delete").Add(float64(len(deleteOperations))) } }(rp.DeepCopy()) // Pass a copy of the object } diff --git a/pkg/registry/apis/iam/resource_permission_hooks_test.go b/pkg/registry/apis/iam/resource_permission_hooks_test.go index ea79b11545b..adb29a779a3 100644 --- a/pkg/registry/apis/iam/resource_permission_hooks_test.go +++ b/pkg/registry/apis/iam/resource_permission_hooks_test.go @@ -7,11 +7,12 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/stretchr/testify/require" + iamv0 "github.com/grafana/grafana/apps/iam/pkg/apis/iam/v0alpha1" "github.com/grafana/grafana/pkg/infra/log" v1 "github.com/grafana/grafana/pkg/services/authz/proto/v1" "github.com/grafana/grafana/pkg/services/authz/zanzana" - "github.com/stretchr/testify/require" ) type FakeZanzanaClient struct { @@ -42,42 +43,6 @@ func (f *FakeZanzanaClient) Mutate(ctx context.Context, req *v1.MutateRequest) e return nil } -func requireTuplesMatch(t *testing.T, actual []*v1.TupleKey, expected []*v1.TupleKey, msgAndArgs ...interface{}) { - t.Helper() - for _, exp := range expected { - found := false - for _, act := range actual { - if act.User == exp.User && - act.Relation == exp.Relation && - act.Object == exp.Object { - found = true - break - } - } - if !found { - require.Fail(t, "Expected tuple not found", "Tuple: %+v\n%v", exp, msgAndArgs) - } - } -} - -func requireDeleteTuplesMatch(t *testing.T, actual []*v1.TupleKeyWithoutCondition, expected []*v1.TupleKeyWithoutCondition, msgAndArgs ...interface{}) { - t.Helper() - for _, exp := range expected { - found := false - for _, act := range actual { - if act.User == exp.User && - act.Relation == exp.Relation && - act.Object == exp.Object { - found = true - break - } - } - if !found { - require.Fail(t, "Expected delete tuple not found", "Tuple: %+v\n%v", exp, msgAndArgs) - } - } -} - func TestAfterResourcePermissionCreate(t *testing.T) { var wg sync.WaitGroup b := &IdentityAccessManagementAPIBuilder{ @@ -101,23 +66,48 @@ func TestAfterResourcePermissionCreate(t *testing.T) { }, } - testFolderEntries := func(ctx context.Context, req *v1.WriteRequest) error { + testFolderEntries := func(ctx context.Context, req *v1.MutateRequest) error { defer wg.Done() require.NotNil(t, req) - require.NotNil(t, req.Writes) - require.Len(t, req.Writes.TupleKeys, 2) + require.NotNil(t, req.Operations) + require.Len(t, req.Operations, 2) require.Equal(t, "org-2", req.Namespace) - expectedTuples := []*v1.TupleKey{ - {User: "user:u1", Relation: "view", Object: "folder:fold1"}, - {User: "role:basic_editor#assignee", Relation: "edit", Object: "folder:fold1"}, + expectedOperations := []*v1.MutateOperation{ + { + Operation: &v1.MutateOperation_CreatePermission{ + CreatePermission: &v1.CreatePermissionOperation{ + Resource: &v1.Resource{ + Group: "folder.grafana.app", Resource: "folders", Name: "fold1", + }, + Permission: &v1.Permission{ + Kind: string(iamv0.ResourcePermissionSpecPermissionKindUser), + Name: "u1", + Verb: "View", + }, + }, + }, + }, + { + Operation: &v1.MutateOperation_CreatePermission{ + CreatePermission: &v1.CreatePermissionOperation{ + Resource: &v1.Resource{ + Group: "folder.grafana.app", Resource: "folders", Name: "fold1", + }, + Permission: &v1.Permission{ + Kind: string(iamv0.ResourcePermissionSpecPermissionKindBasicRole), + Name: "Editor", + Verb: "Edit", + }, + }, + }, + }, } - - requireTuplesMatch(t, req.Writes.TupleKeys, expectedTuples) + require.ElementsMatch(t, expectedOperations, req.Operations) return nil } - b.zClient = &FakeZanzanaClient{writeCallback: testFolderEntries} + b.zClient = &FakeZanzanaClient{mutateCallback: testFolderEntries} b.AfterResourcePermissionCreate(&folderPerm, nil) wg.Wait() }) @@ -139,34 +129,54 @@ func TestAfterResourcePermissionCreate(t *testing.T) { }, } - testDashEntries := func(ctx context.Context, req *v1.WriteRequest) error { + testDashEntries := func(ctx context.Context, req *v1.MutateRequest) error { defer wg.Done() - object := "resource:dashboard.grafana.app/dashboards/dash1" + // object := "resource:dashboard.grafana.app/dashboards/dash1" require.NotNil(t, req) - require.NotNil(t, req.Writes) - require.Len(t, req.Writes.TupleKeys, 2) + require.NotNil(t, req.Operations) + require.Len(t, req.Operations, 2) require.Equal(t, "default", req.Namespace) - // Verify all tuples have the group_filter condition - for _, tuple := range req.Writes.TupleKeys { - require.NotNil(t, tuple.Condition, "Condition should not be nil for tuple %+v", tuple) - require.Equal(t, "group_filter", tuple.Condition.Name) + expectedOperations := []*v1.MutateOperation{ + { + Operation: &v1.MutateOperation_CreatePermission{ + CreatePermission: &v1.CreatePermissionOperation{ + Resource: &v1.Resource{ + Group: "dashboard.grafana.app", Resource: "dashboards", Name: "dash1", + }, + Permission: &v1.Permission{ + Kind: string(iamv0.ResourcePermissionSpecPermissionKindServiceAccount), + Name: "sa1", + Verb: "View", + }, + }, + }, + }, + { + Operation: &v1.MutateOperation_CreatePermission{ + CreatePermission: &v1.CreatePermissionOperation{ + Resource: &v1.Resource{ + Group: "dashboard.grafana.app", Resource: "dashboards", Name: "dash1", + }, + Permission: &v1.Permission{ + Kind: string(iamv0.ResourcePermissionSpecPermissionKindTeam), + Name: "team1", + Verb: "Edit", + }, + }, + }, + }, } + require.ElementsMatch(t, expectedOperations, req.Operations) - expectedTuples := []*v1.TupleKey{ - {User: "service-account:sa1", Relation: "view", Object: object}, - {User: "team:team1#member", Relation: "edit", Object: object}, - } - - requireTuplesMatch(t, req.Writes.TupleKeys, expectedTuples) return nil } - b.zClient = &FakeZanzanaClient{writeCallback: testDashEntries} + b.zClient = &FakeZanzanaClient{mutateCallback: testDashEntries} b.AfterResourcePermissionCreate(&dashPerm, nil) + wg.Wait() }) - wg.Wait() } func TestBeginResourcePermissionUpdate(t *testing.T) { @@ -206,33 +216,66 @@ func TestBeginResourcePermissionUpdate(t *testing.T) { }, } - testFolderWrite := func(ctx context.Context, req *v1.WriteRequest) error { + testFolderWrite := func(ctx context.Context, req *v1.MutateRequest) error { defer wg.Done() require.NotNil(t, req) require.Equal(t, "org-2", req.Namespace) // Should delete old permission - require.NotNil(t, req.Deletes) - require.Len(t, req.Deletes.TupleKeys, 1) - require.Equal( - t, - req.Deletes.TupleKeys[0], - &v1.TupleKeyWithoutCondition{User: "user:u1", Relation: "view", Object: "folder:fold1"}, - ) + require.NotNil(t, req.Operations) + require.Len(t, req.Operations, 3) + require.Equal(t, "org-2", req.Namespace) - // Should write new permissions - require.NotNil(t, req.Writes) - require.Len(t, req.Writes.TupleKeys, 2) - - expectedWrites := []*v1.TupleKey{ - {User: "user:u2", Relation: "edit", Object: "folder:fold1"}, - {User: "team:team1#member", Relation: "view", Object: "folder:fold1"}, + expectedOperations := []*v1.MutateOperation{ + { + Operation: &v1.MutateOperation_DeletePermission{ + DeletePermission: &v1.DeletePermissionOperation{ + Resource: &v1.Resource{ + Group: "folder.grafana.app", Resource: "folders", Name: "fold1", + }, + Permission: &v1.Permission{ + Kind: string(iamv0.ResourcePermissionSpecPermissionKindUser), + Name: "u1", + Verb: "View", + }, + }, + }, + }, + { + Operation: &v1.MutateOperation_CreatePermission{ + CreatePermission: &v1.CreatePermissionOperation{ + Resource: &v1.Resource{ + Group: "folder.grafana.app", Resource: "folders", Name: "fold1", + }, + Permission: &v1.Permission{ + Kind: string(iamv0.ResourcePermissionSpecPermissionKindTeam), + Name: "team1", + Verb: "View", + }, + }, + }, + }, + { + Operation: &v1.MutateOperation_CreatePermission{ + CreatePermission: &v1.CreatePermissionOperation{ + Resource: &v1.Resource{ + Group: "folder.grafana.app", Resource: "folders", Name: "fold1", + }, + Permission: &v1.Permission{ + Kind: string(iamv0.ResourcePermissionSpecPermissionKindUser), + Name: "u2", + Verb: "Edit", + }, + }, + }, + }, } - requireTuplesMatch(t, req.Writes.TupleKeys, expectedWrites) + + require.ElementsMatch(t, expectedOperations, req.Operations) return nil } - b.zClient = &FakeZanzanaClient{writeCallback: testFolderWrite} + b.zClient = &FakeZanzanaClient{mutateCallback: testFolderWrite} // Call BeginUpdate which does all the work finishFunc, err := b.BeginResourcePermissionUpdate(context.Background(), &newFolderPerm, &oldFolderPerm, nil) @@ -274,40 +317,52 @@ func TestBeginResourcePermissionUpdate(t *testing.T) { }, } - object := "resource:dashboard.grafana.app/dashboards/dash1" - - testDashWrite := func(ctx context.Context, req *v1.WriteRequest) error { + testDashWrite := func(ctx context.Context, req *v1.MutateRequest) error { defer wg.Done() require.NotNil(t, req) require.Equal(t, "default", req.Namespace) // Should delete old permission - require.NotNil(t, req.Deletes) - require.Len(t, req.Deletes.TupleKeys, 1) - require.Equal( - t, - req.Deletes.TupleKeys[0], - &v1.TupleKeyWithoutCondition{User: "user:u1", Relation: "view", Object: object}, - ) + require.NotNil(t, req.Operations) + require.Len(t, req.Operations, 2) - // Should write new permission - require.NotNil(t, req.Writes) - require.Len(t, req.Writes.TupleKeys, 1) + expectedOperations := []*v1.MutateOperation{ + { + Operation: &v1.MutateOperation_DeletePermission{ + DeletePermission: &v1.DeletePermissionOperation{ + Resource: &v1.Resource{ + Group: "dashboard.grafana.app", Resource: "dashboards", Name: "dash1", + }, + Permission: &v1.Permission{ + Kind: string(iamv0.ResourcePermissionSpecPermissionKindUser), + Name: "u1", + Verb: "View", + }, + }, + }, + }, + { + Operation: &v1.MutateOperation_CreatePermission{ + CreatePermission: &v1.CreatePermissionOperation{ + Resource: &v1.Resource{ + Group: "dashboard.grafana.app", Resource: "dashboards", Name: "dash1", + }, + Permission: &v1.Permission{ + Kind: string(iamv0.ResourcePermissionSpecPermissionKindServiceAccount), + Name: "sa1", + Verb: "Edit", + }, + }, + }, + }, + } - tuple := req.Writes.TupleKeys[0] - require.NotNil(t, tuple.Condition) - require.Equal(t, "group_filter", tuple.Condition.Name) - tuple.Condition = nil - require.Equal( - t, - tuple, - &v1.TupleKey{User: "service-account:sa1", Relation: "edit", Object: object}, - ) + require.ElementsMatch(t, expectedOperations, req.Operations) return nil } - b.zClient = &FakeZanzanaClient{writeCallback: testDashWrite} + b.zClient = &FakeZanzanaClient{mutateCallback: testDashWrite} // Call BeginUpdate which does all the work finishFunc, err := b.BeginResourcePermissionUpdate(context.Background(), &newDashPerm, &oldDashPerm, nil) @@ -343,30 +398,50 @@ func TestAfterResourcePermissionDelete(t *testing.T) { }, } - testFolderDelete := func(ctx context.Context, req *v1.WriteRequest) error { + testFolderDelete := func(ctx context.Context, req *v1.MutateRequest) error { defer wg.Done() require.NotNil(t, req) require.Equal(t, "org-2", req.Namespace) // Should have deletes but no writes - require.NotNil(t, req.Deletes) - require.Len(t, req.Deletes.TupleKeys, 2) - require.Nil(t, req.Writes) + require.NotNil(t, req.Operations) + require.Len(t, req.Operations, 2) - require.Equal( - t, - req.Deletes.TupleKeys[0], - &v1.TupleKeyWithoutCondition{User: "user:u1", Relation: "view", Object: "folder:fold1"}, - ) - require.Equal( - t, - req.Deletes.TupleKeys[1], - &v1.TupleKeyWithoutCondition{User: "role:basic_editor#assignee", Relation: "edit", Object: "folder:fold1"}, - ) + expectedOperations := []*v1.MutateOperation{ + { + Operation: &v1.MutateOperation_DeletePermission{ + DeletePermission: &v1.DeletePermissionOperation{ + Resource: &v1.Resource{ + Group: "folder.grafana.app", Resource: "folders", Name: "fold1", + }, + Permission: &v1.Permission{ + Kind: string(iamv0.ResourcePermissionSpecPermissionKindUser), + Name: "u1", + Verb: "View", + }, + }, + }, + }, + { + Operation: &v1.MutateOperation_DeletePermission{ + DeletePermission: &v1.DeletePermissionOperation{ + Resource: &v1.Resource{ + Group: "folder.grafana.app", Resource: "folders", Name: "fold1", + }, + Permission: &v1.Permission{ + Kind: string(iamv0.ResourcePermissionSpecPermissionKindBasicRole), + Name: "Editor", + Verb: "Edit", + }, + }, + }, + }, + } + require.ElementsMatch(t, expectedOperations, req.Operations) return nil } - b.zClient = &FakeZanzanaClient{writeCallback: testFolderDelete} + b.zClient = &FakeZanzanaClient{mutateCallback: testFolderDelete} b.AfterResourcePermissionDelete(&folderPerm, nil) wg.Wait() }) @@ -388,33 +463,51 @@ func TestAfterResourcePermissionDelete(t *testing.T) { }, } - testDashDelete := func(ctx context.Context, req *v1.WriteRequest) error { + testDashDelete := func(ctx context.Context, req *v1.MutateRequest) error { defer wg.Done() - object := "resource:dashboard.grafana.app/dashboards/dash1" require.NotNil(t, req) require.Equal(t, "default", req.Namespace) // Should have deletes but no writes - require.NotNil(t, req.Deletes) - require.Len(t, req.Deletes.TupleKeys, 2) - require.Nil(t, req.Writes) - - require.Equal( - t, - req.Deletes.TupleKeys[0], - &v1.TupleKeyWithoutCondition{User: "service-account:sa1", Relation: "view", Object: object}, - ) - require.Equal( - t, - req.Deletes.TupleKeys[1], - &v1.TupleKeyWithoutCondition{User: "team:team1#member", Relation: "edit", Object: object}, - ) + require.NotNil(t, req.Operations) + require.Len(t, req.Operations, 2) + expectedOperations := []*v1.MutateOperation{ + { + Operation: &v1.MutateOperation_DeletePermission{ + DeletePermission: &v1.DeletePermissionOperation{ + Resource: &v1.Resource{ + Group: "dashboard.grafana.app", Resource: "dashboards", Name: "dash1", + }, + Permission: &v1.Permission{ + Kind: string(iamv0.ResourcePermissionSpecPermissionKindServiceAccount), + Name: "sa1", + Verb: "View", + }, + }, + }, + }, + { + Operation: &v1.MutateOperation_DeletePermission{ + DeletePermission: &v1.DeletePermissionOperation{ + Resource: &v1.Resource{ + Group: "dashboard.grafana.app", Resource: "dashboards", Name: "dash1", + }, + Permission: &v1.Permission{ + Kind: string(iamv0.ResourcePermissionSpecPermissionKindTeam), + Name: "team1", + Verb: "Edit", + }, + }, + }, + }, + } + require.ElementsMatch(t, expectedOperations, req.Operations) return nil } - b.zClient = &FakeZanzanaClient{writeCallback: testDashDelete} + b.zClient = &FakeZanzanaClient{mutateCallback: testDashDelete} b.AfterResourcePermissionDelete(&dashPerm, nil) wg.Wait() }) diff --git a/pkg/registry/apis/iam/role_hooks.go b/pkg/registry/apis/iam/role_hooks.go index 9c4271c94aa..831a2993fe5 100644 --- a/pkg/registry/apis/iam/role_hooks.go +++ b/pkg/registry/apis/iam/role_hooks.go @@ -411,3 +411,23 @@ func (b *IdentityAccessManagementAPIBuilder) BeginRoleUpdate(ctx context.Context }(oldRole.DeepCopy(), newRole.DeepCopy()) }, nil } + +// tupleToTupleKeyWithoutCondition converts a TupleKey to TupleKeyWithoutCondition +// This is needed for delete operations which don't support conditions +func tupleToTupleKeyWithoutCondition(tuple *v1.TupleKey) *v1.TupleKeyWithoutCondition { + return &v1.TupleKeyWithoutCondition{ + User: tuple.User, + Relation: tuple.Relation, + Object: tuple.Object, + } +} + +// toTupleKeysWithoutCondition converts v1.TupleKey to v1.TupleKeyWithoutCondition +// by stripping the condition field, which is required for delete operations +func toTupleKeysWithoutCondition(tuples []*v1.TupleKey) []*v1.TupleKeyWithoutCondition { + result := make([]*v1.TupleKeyWithoutCondition, len(tuples)) + for i, t := range tuples { + result[i] = tupleToTupleKeyWithoutCondition(t) + } + return result +} diff --git a/pkg/registry/apis/iam/role_hooks_test.go b/pkg/registry/apis/iam/role_hooks_test.go index 282576b6ec7..497150f1c61 100644 --- a/pkg/registry/apis/iam/role_hooks_test.go +++ b/pkg/registry/apis/iam/role_hooks_test.go @@ -7,12 +7,49 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/stretchr/testify/require" + iamv0 "github.com/grafana/grafana/apps/iam/pkg/apis/iam/v0alpha1" "github.com/grafana/grafana/pkg/infra/log" v1 "github.com/grafana/grafana/pkg/services/authz/proto/v1" - "github.com/stretchr/testify/require" ) +func requireTuplesMatch(t *testing.T, actual []*v1.TupleKey, expected []*v1.TupleKey, msgAndArgs ...interface{}) { + t.Helper() + for _, exp := range expected { + found := false + for _, act := range actual { + if act.User == exp.User && + act.Relation == exp.Relation && + act.Object == exp.Object { + found = true + break + } + } + if !found { + require.Fail(t, "Expected tuple not found", "Tuple: %+v\n%v", exp, msgAndArgs) + } + } +} + +func requireDeleteTuplesMatch(t *testing.T, actual []*v1.TupleKeyWithoutCondition, expected []*v1.TupleKeyWithoutCondition, msgAndArgs ...interface{}) { + t.Helper() + for _, exp := range expected { + found := false + for _, act := range actual { + if act.User == exp.User && + act.Relation == exp.Relation && + act.Object == exp.Object { + found = true + break + } + } + if !found { + require.Fail(t, "Expected delete tuple not found", "Tuple: %+v\n%v", exp, msgAndArgs) + } + } +} + func TestAfterCoreRoleCreate(t *testing.T) { var wg sync.WaitGroup