Skip to content

Commit

Permalink
Merge pull request #1515 from authzed/revert-1511-self
Browse files Browse the repository at this point in the history
Revert "Add support for `self` keyword in schema for referencing a resource as a subject"
  • Loading branch information
josephschorr authored Aug 30, 2023
2 parents ce3fbd4 + b8d86c4 commit ac2b1e8
Show file tree
Hide file tree
Showing 29 changed files with 233 additions and 1,169 deletions.
27 changes: 0 additions & 27 deletions internal/graph/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -466,38 +466,11 @@ func (cc *ConcurrentChecker) runSetOperation(ctx context.Context, crc currentReq
return cc.checkTupleToUserset(ctx, crc, child.TupleToUserset)
case *core.SetOperation_Child_XNil:
return noMembers()
case *core.SetOperation_Child_XSelf:
return cc.checkSelf(ctx, crc)
default:
return checkResultError(fmt.Errorf("unknown set operation child `%T` in check", child), emptyMetadata)
}
}

func (cc *ConcurrentChecker) checkSelf(ctx context.Context, crc currentRequestContext) CheckResult {
log.Ctx(ctx).Trace().Object("self", crc.parentReq).Send()

// `self` only represents the resource itself, not any child relation.
if crc.parentReq.Subject.Relation != tuple.Ellipsis {
return noMembers()
}

req := crc.parentReq

if req.ResourceRelation.Namespace != req.Subject.Namespace {
return noMembers()
}

for _, resourceID := range req.ResourceIds {
if req.Subject.ObjectId == resourceID {
membershipSet := NewMembershipSet()
membershipSet.AddDirectMember(resourceID, nil)
return checkResultsForMembership(membershipSet, emptyMetadata)
}
}

return noMembers()
}

func (cc *ConcurrentChecker) checkComputedUserset(ctx context.Context, crc currentRequestContext, cu *core.ComputedUserset, rr *core.RelationReference, resourceIds []string) CheckResult {
var startNamespace string
var targetResourceIds []string
Expand Down
28 changes: 1 addition & 27 deletions internal/graph/expand.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
core "github.com/authzed/spicedb/pkg/proto/core/v1"
v1 "github.com/authzed/spicedb/pkg/proto/dispatch/v1"
"github.com/authzed/spicedb/pkg/spiceerrors"
"github.com/authzed/spicedb/pkg/tuple"
)

// NewConcurrentExpander creates an instance of ConcurrentExpander
Expand Down Expand Up @@ -195,10 +194,8 @@ func (ce *ConcurrentExpander) expandSetOperation(ctx context.Context, req Valida
requests = append(requests, ce.expandTupleToUserset(ctx, req, child.TupleToUserset))
case *core.SetOperation_Child_XNil:
requests = append(requests, emptyExpansion(req.ResourceAndRelation))
case *core.SetOperation_Child_XSelf:
requests = append(requests, selfExpansion(req.ResourceAndRelation))
default:
return expandError(fmt.Errorf("unknown set operation child `%T` in concurrent expand", child))
return expandError(fmt.Errorf("unknown set operation child `%T` in expand", child))
}
}
return func(ctx context.Context, resultChan chan<- ExpandResult) {
Expand Down Expand Up @@ -345,29 +342,6 @@ func expandSetOperation(
return setResult(op, start, children, responseMetadata)
}

// selfExpansion returns a self expansion.
func selfExpansion(start *core.ObjectAndRelation) ReduceableExpandFunc {
return func(ctx context.Context, resultChan chan<- ExpandResult) {
resultChan <- expandResult(&core.RelationTupleTreeNode{
NodeType: &core.RelationTupleTreeNode_LeafNode{
LeafNode: &core.DirectSubjects{
Subjects: []*core.DirectSubject{
{
// `self` expands into the resource itself, without any relation.
Subject: &core.ObjectAndRelation{
Namespace: start.Namespace,
ObjectId: start.ObjectId,
Relation: tuple.Ellipsis,
},
},
},
},
},
Expanded: start,
}, emptyMetadata)
}
}

// emptyExpansion returns an empty expansion.
func emptyExpansion(start *core.ObjectAndRelation) ReduceableExpandFunc {
return func(ctx context.Context, resultChan chan<- ExpandResult) {
Expand Down
24 changes: 1 addition & 23 deletions internal/graph/lookupsubjects.go
Original file line number Diff line number Diff line change
Expand Up @@ -309,30 +309,8 @@ func (cl *ConcurrentLookupSubjects) lookupSetOperation(
// Purposely do nothing.
continue

case *core.SetOperation_Child_XSelf:
// Return the current resource(s) as the found subject(s).
foundResources := map[string]*v1.FoundSubjects{}
for _, resourceID := range req.ResourceIds {
foundResources[resourceID] = &v1.FoundSubjects{
FoundSubjects: []*v1.FoundSubject{
{
SubjectId: resourceID,
},
},
}
}

err := stream.Publish(&v1.DispatchLookupSubjectsResponse{
FoundSubjectsByResourceId: foundResources,
Metadata: emptyMetadata,
})
if err != nil {
return err
}
continue

default:
return fmt.Errorf("unknown set operation child `%T` in lookup subjects", child)
return fmt.Errorf("unknown set operation child `%T` in expand", child)
}
}

Expand Down
21 changes: 0 additions & 21 deletions internal/graph/reachableresources.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,27 +152,6 @@ func (crr *CursoredReachableResources) afterSameType(
case core.ReachabilityEntrypoint_TUPLESET_TO_USERSET_ENTRYPOINT:
return crr.lookupTTUEntrypoint(ctx, ci, entrypoint, rg, reader, req, stream, dispatched)

case core.ReachabilityEntrypoint_SELF_ENTRYPOINT:
selfRelation, err := entrypoint.SelfRelation()
if err != nil {
return err
}

// Redispatch with the relation that used `self`.
rsm := subjectIDsToResourcesMap(selfRelation, req.SubjectIds)
drsm := rsm.asReadOnly()
return crr.redispatchOrReport(
ctx,
ci,
selfRelation,
drsm,
rg,
entrypoint,
stream,
req,
dispatched,
)

default:
return spiceerrors.MustBugf("Unknown kind of entrypoint: %v", entrypoint.EntrypointKind())
}
Expand Down
12 changes: 1 addition & 11 deletions internal/namespace/canonicalization.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,6 @@ const computedKeyPrefix = "%"
// canonical representation of the binary expression. These hashes can then be used for caching,
// representing the same *logical* expressions for a permission, even if the relations have
// different names.
//
// NOTE: These keys are only guaranteed to be stable within a single annotation call and may change
// over time.
func computeCanonicalCacheKeys(typeSystem *ValidatedNamespaceTypeSystem, aliasMap map[string]string) (map[string]string, error) {
varMap, err := buildBddVarMap(typeSystem.nsDef.Relation, aliasMap)
if err != nil {
Expand Down Expand Up @@ -160,9 +157,6 @@ func convertToBdd(relation *core.Relation, bdd *rudd.BDD, so *core.SetOperation,
case *core.SetOperation_Child_XNil:
values = append(values, builder(index, varMap.Nil()))

case *core.SetOperation_Child_XSelf:
values = append(values, builder(index, varMap.Self()))

default:
return nil, spiceerrors.MustBugf("unknown set operation child %T", child)
}
Expand All @@ -188,10 +182,6 @@ func (bvm bddVarMap) Nil() int {
return len(bvm.varMap)
}

func (bvm bddVarMap) Self() int {
return len(bvm.varMap) + 1
}

func (bvm bddVarMap) Get(relName string) (int, error) {
if alias, ok := bvm.aliasMap[relName]; ok {
return bvm.Get(alias)
Expand All @@ -205,7 +195,7 @@ func (bvm bddVarMap) Get(relName string) (int, error) {
}

func (bvm bddVarMap) Len() int {
return len(bvm.varMap) + 2 // +2 for `nil` and `self`
return len(bvm.varMap) + 1 // +1 for `nil`
}

func buildBddVarMap(relations []*core.Relation, aliasMap map[string]string) (bddVarMap, error) {
Expand Down
Loading

0 comments on commit ac2b1e8

Please sign in to comment.