Skip to content

Commit

Permalink
Merge pull request from GHSA-7p8f-8hjm-wm92
Browse files Browse the repository at this point in the history
Fix handling of wildcards in lookup dispatch
  • Loading branch information
josephschorr committed Jan 11, 2022
2 parents d002401 + 635b575 commit 15bba2e
Show file tree
Hide file tree
Showing 28 changed files with 1,761 additions and 657 deletions.
4 changes: 3 additions & 1 deletion internal/graph/check.go
Expand Up @@ -45,7 +45,9 @@ type ValidatedCheckRequest struct {
func (cc *ConcurrentChecker) Check(ctx context.Context, req ValidatedCheckRequest, relation *v0.Relation) (*v1.DispatchCheckResponse, error) {
var directFunc ReduceableCheckFunc

if onrEqual(req.Subject, req.ObjectAndRelation) {
if req.Subject.ObjectId == tuple.PublicWildcard {
directFunc = checkError(NewErrInvalidArgument(errors.New("cannot perform check on wildcard")))
} else if onrEqual(req.Subject, req.ObjectAndRelation) {
// If we have found the goal's ONR, then we know that the ONR is a member.
directFunc = alwaysMember()
} else if relation.UsersetRewrite == nil {
Expand Down
12 changes: 12 additions & 0 deletions internal/graph/errors.go
Expand Up @@ -122,3 +122,15 @@ func NewRelationMissingTypeInfoErr(nsName string, relationName string) error {
relationName: relationName,
}
}

// ErrInvalidArgument occurs when a request sent has an invalid argument.
type ErrInvalidArgument struct {
error
}

// NewErrInvalidArgument constructs a request sent has an invalid argument.
func NewErrInvalidArgument(baseErr error) error {
return ErrInvalidArgument{
error: baseErr,
}
}
49 changes: 46 additions & 3 deletions internal/graph/lookup.go
Expand Up @@ -2,6 +2,7 @@ package graph

import (
"context"
"errors"
"fmt"

v0 "github.com/authzed/authzed-go/proto/authzed/api/v0"
Expand Down Expand Up @@ -46,6 +47,10 @@ const (
// Lookup performs a lookup request with the provided request and context.
func (cl *ConcurrentLookup) Lookup(ctx context.Context, req ValidatedLookupRequest) (*v1.DispatchLookupResponse, error) {
funcToResolve := cl.lookupInternal(ctx, req)
if req.Subject.ObjectId == tuple.PublicWildcard {
funcToResolve = returnResult(lookupResultError(req, NewErrInvalidArgument(errors.New("cannot perform lookup on wildcard")), emptyMetadata))
}

resolved := lookupOne(ctx, req, funcToResolve)

// Remove the resolved relation reference from the excluded direct list to mark that it was completely resolved.
Expand Down Expand Up @@ -115,9 +120,7 @@ func (cl *ConcurrentLookup) lookupInternal(ctx context.Context, req ValidatedLoo
var requests []ReduceableLookupFunc
for _, obj := range toCheck.AsSlice() {
// If we've already found the target ONR, no further resolution is necessary.
if obj.Namespace == req.Subject.Namespace &&
obj.Relation == req.Subject.Relation &&
obj.ObjectId == req.Subject.ObjectId {
if onrEqualOrWildcard(obj, req.Subject) {
continue
}

Expand Down Expand Up @@ -206,6 +209,46 @@ func (cl *ConcurrentLookup) lookupDirect(ctx context.Context, req ValidatedLooku
})
}

// Dispatch a check for the subject wildcard, if allowed.
isWildcardAllowed, err := typeSystem.IsAllowedPublicNamespace(req.ObjectRelation.Relation, req.Subject.Namespace)
if isWildcardAllowed == namespace.PublicSubjectAllowed {
requests = append(requests, func(ctx context.Context, resultChan chan<- LookupResult) {
objects := tuple.NewONRSet()
it, err := cl.ds.ReverseQueryTuples(
ctx,
tuple.UsersetToSubjectFilter(&v0.ObjectAndRelation{
Namespace: req.Subject.Namespace,
ObjectId: tuple.PublicWildcard,
Relation: req.Subject.Relation,
}),
req.Revision,
options.WithResRelation(&options.ResourceRelation{
Namespace: req.ObjectRelation.Namespace,
Relation: req.ObjectRelation.Relation,
}),
)
if err != nil {
resultChan <- lookupResultError(req, err, emptyMetadata)
return
}
defer it.Close()

for tpl := it.Next(); tpl != nil; tpl = it.Next() {
objects.Add(tpl.ObjectAndRelation)
if objects.Length() >= req.Limit {
break
}
}

if it.Err() != nil {
resultChan <- lookupResultError(req, it.Err(), emptyMetadata)
return
}

resultChan <- lookupResult(req, objects.AsSlice(), emptyMetadata)
})
}

// Dispatch to any allowed subject relation types that don't match the target ONR, collect
// the found object IDs, and then search for those.
allowedDirect, err := typeSystem.AllowedSubjectRelations(req.ObjectRelation.Relation)
Expand Down
142 changes: 142 additions & 0 deletions internal/membership/foundsubject.go
@@ -0,0 +1,142 @@
package membership

import (
"fmt"
"sort"
"strings"

v0 "github.com/authzed/authzed-go/proto/authzed/api/v0"

"github.com/authzed/spicedb/pkg/tuple"
)

// NewFoundSubject creates a new FoundSubject for a subject and a set of its resources.
func NewFoundSubject(subject *v0.ObjectAndRelation, resources ...*v0.ObjectAndRelation) FoundSubject {
return FoundSubject{subject, tuple.NewONRSet(), tuple.NewONRSet(resources...)}
}

// FoundSubject contains a single found subject and all the relationships in which that subject
// is a member which were found via the ONRs expansion.
type FoundSubject struct {
// subject is the subject found.
subject *v0.ObjectAndRelation

// excludedSubjects are any subjects excluded. Only should be set if subject is a wildcard.
excludedSubjects *tuple.ONRSet

// relations are the relations under which the subject lives that informed the locating
// of this subject for the root ONR.
relationships *tuple.ONRSet
}

// Subject returns the Subject of the FoundSubject.
func (fs FoundSubject) Subject() *v0.ObjectAndRelation {
return fs.subject
}

// WildcardType returns the object type for the wildcard subject, if this is a wildcard subject.
func (fs FoundSubject) WildcardType() (string, bool) {
if fs.subject.ObjectId == tuple.PublicWildcard {
return fs.subject.Namespace, true
}

return "", false
}

// ExcludedSubjectsFromWildcard returns those subjects excluded from the wildcard subject.
// If not a wildcard subject, returns false.
func (fs FoundSubject) ExcludedSubjectsFromWildcard() ([]*v0.ObjectAndRelation, bool) {
if fs.subject.ObjectId == tuple.PublicWildcard {
return fs.excludedSubjects.AsSlice(), true
}

return []*v0.ObjectAndRelation{}, false
}

// Relationships returns all the relationships in which the subject was found as per the expand.
func (fs FoundSubject) Relationships() []*v0.ObjectAndRelation {
return fs.relationships.AsSlice()
}

// ToValidationString returns the FoundSubject in a format that is consumable by the validationfile
// package.
func (fs FoundSubject) ToValidationString() string {
onrString := tuple.StringONR(fs.Subject())
excluded, isWildcard := fs.ExcludedSubjectsFromWildcard()
if isWildcard && len(excluded) > 0 {
excludedONRStrings := make([]string, 0, len(excluded))
for _, excludedONR := range excluded {
excludedONRStrings = append(excludedONRStrings, tuple.StringONR(excludedONR))
}

sort.Strings(excludedONRStrings)
return fmt.Sprintf("%s - {%s}", onrString, strings.Join(excludedONRStrings, ", "))
}

return onrString
}

// union performs merging of two FoundSubject's with the same subject.
func (fs FoundSubject) union(other FoundSubject) FoundSubject {
if toKey(fs.subject) != toKey(other.subject) {
panic("Got wrong found subject to union")
}

relationships := fs.relationships.Union(other.relationships)
var excludedSubjects *tuple.ONRSet

// If a wildcard, then union together excluded subjects.
_, isWildcard := fs.WildcardType()
if isWildcard {
excludedSubjects = fs.excludedSubjects.Union(other.excludedSubjects)
}

return FoundSubject{
subject: fs.subject,
excludedSubjects: excludedSubjects,
relationships: relationships,
}
}

// intersect performs intersection between two FoundSubject's with the same subject.
func (fs FoundSubject) intersect(other FoundSubject) FoundSubject {
if toKey(fs.subject) != toKey(other.subject) {
panic("Got wrong found subject to intersect")
}

relationships := fs.relationships.Union(other.relationships)
var excludedSubjects *tuple.ONRSet

// If a wildcard, then union together excluded subjects.
_, isWildcard := fs.WildcardType()
if isWildcard {
excludedSubjects = fs.excludedSubjects.Union(other.excludedSubjects)
}

return FoundSubject{
subject: fs.subject,
excludedSubjects: excludedSubjects,
relationships: relationships,
}
}

// FoundSubjects contains the subjects found for a specific ONR.
type FoundSubjects struct {
// subjects is a map from the Subject ONR (as a string) to the FoundSubject information.
subjects map[string]FoundSubject
}

// ListFound returns a slice of all the FoundSubject's.
func (fs FoundSubjects) ListFound() []FoundSubject {
found := []FoundSubject{}
for _, sub := range fs.subjects {
found = append(found, sub)
}
return found
}

// LookupSubject returns the FoundSubject for a matching subject, if any.
func (fs FoundSubjects) LookupSubject(subject *v0.ObjectAndRelation) (FoundSubject, bool) {
found, ok := fs.subjects[toKey(subject)]
return found, ok
}
62 changes: 62 additions & 0 deletions internal/membership/foundsubject_test.go
@@ -0,0 +1,62 @@
package membership

import (
"fmt"
"testing"

"github.com/authzed/spicedb/pkg/validationfile"
"github.com/stretchr/testify/require"
)

func TestToValidationString(t *testing.T) {
testCases := []struct {
name string
fs FoundSubject
expected string
}{
{
"basic",
fs("user", "user1", "..."),
"user:user1",
},
{
"with exclusion",
fs("user", "*", "...", ONR("user", "user1", "...")),
"user:* - {user:user1}",
},
{
"with some exclusion",
fs("user", "*", "...",
ONR("user", "user1", "..."),
ONR("user", "user2", "..."),
ONR("user", "user3", "..."),
ONR("user", "user4", "..."),
ONR("user", "user5", "..."),
),
"user:* - {user:user1, user:user2, user:user3, user:user4, user:user5}",
},
{
"with many exclusion",
fs("user", "*", "...",
ONR("user", "user1", "..."),
ONR("user", "user2", "..."),
ONR("user", "user3", "..."),
ONR("user", "user4", "..."),
ONR("user", "user5", "..."),
ONR("user", "user6", "..."),
),
"user:* - {user:user1, user:user2, user:user3, user:user4, user:user5, user:user6}",
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
require := require.New(t)
require.Equal(tc.expected, tc.fs.ToValidationString())

sub, err := validationfile.ValidationString(fmt.Sprintf("[%s]", tc.expected)).Subject()
require.Nil(err)
require.NotNil(sub)
})
}
}

0 comments on commit 15bba2e

Please sign in to comment.