Skip to content

Commit c2c3bab

Browse files
authored
fix: improve LR consistency and support multiple resourcetypes (#2875)
1 parent fa1d7f4 commit c2c3bab

26 files changed

+185
-94
lines changed

pkg/query/alias.go

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -180,9 +180,32 @@ func (a *Alias) IterResourcesImpl(ctx *Context, subject ObjectAndRelation, filte
180180
return nil, err
181181
}
182182

183-
// If the relation on the alias iterator is the same as the subject relation,
184-
// we have a self-edge and should yield it
185-
shouldAddSelfEdge := a.relation == subject.Relation
183+
// Check if we should add a self-edge (identity check: permission always grants access to itself)
184+
// This matches LookupResources3's logic where subject type+relation must match resource type+relation
185+
shouldAddSelfEdge := false
186+
if a.relation == subject.Relation {
187+
// Get the resource types from the iterator
188+
resourceTypes, err := a.ResourceType()
189+
if err != nil {
190+
return nil, err
191+
}
192+
193+
// Add self-edge if:
194+
// - No resource types defined (empty/unconstrained iterator), OR
195+
// - Subject type matches one of the possible resource types
196+
// This allows self-edges for empty iterators while preventing them in nested contexts
197+
// where the types don't match
198+
if len(resourceTypes) == 0 {
199+
shouldAddSelfEdge = true
200+
} else {
201+
for _, rt := range resourceTypes {
202+
if rt.Type == subject.ObjectType {
203+
shouldAddSelfEdge = true
204+
break
205+
}
206+
}
207+
}
208+
}
186209

187210
return a.maybePrependSelfEdge(GetObject(subject), subSeq, shouldAddSelfEdge), nil
188211
}
@@ -215,7 +238,7 @@ func (a *Alias) ID() string {
215238
return a.id
216239
}
217240

218-
func (a *Alias) ResourceType() (ObjectType, error) {
241+
func (a *Alias) ResourceType() ([]ObjectType, error) {
219242
return a.subIt.ResourceType()
220243
}
221244

pkg/query/alias_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -760,7 +760,8 @@ func TestAlias_Types(t *testing.T) {
760760

761761
resourceType, err := alias.ResourceType()
762762
require.NoError(err)
763-
require.Equal("document", resourceType.Type)
763+
require.Len(resourceType, 1)
764+
require.Equal("document", resourceType[0].Type)
764765
})
765766

766767
t.Run("SubjectTypes", func(t *testing.T) {

pkg/query/arrow.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,11 @@ func (a *Arrow) IterResourcesImpl(ctx *Context, subject ObjectAndRelation, filte
282282
yield(Path{}, err)
283283
return
284284
}
285+
286+
// Note: We used to filter self-edges here, but self-edges from Alias represent valid identity checks
287+
// (e.g., team:first#member accessing team:first via member). Removing the filter allows these
288+
// identity relationships to propagate through arrows correctly.
289+
285290
rightPathCount++
286291

287292
// For each right resource, get resources from left side
@@ -359,7 +364,7 @@ func (a *Arrow) ID() string {
359364
return a.id
360365
}
361366

362-
func (a *Arrow) ResourceType() (ObjectType, error) {
367+
func (a *Arrow) ResourceType() ([]ObjectType, error) {
363368
// Arrow's resources come from the left side
364369
return a.left.ResourceType()
365370
}

pkg/query/arrow_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -713,7 +713,8 @@ func TestArrow_Types(t *testing.T) {
713713

714714
resourceType, err := arrow.ResourceType()
715715
require.NoError(err)
716-
require.Equal("document", resourceType.Type) // From left iterator
716+
require.Len(resourceType, 1)
717+
require.Equal("document", resourceType[0].Type) // From left iterator
717718
})
718719

719720
t.Run("SubjectTypes", func(t *testing.T) {

pkg/query/caveat.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ func (c *CaveatIterator) ID() string {
208208
return c.id
209209
}
210210

211-
func (c *CaveatIterator) ResourceType() (ObjectType, error) {
211+
func (c *CaveatIterator) ResourceType() ([]ObjectType, error) {
212212
// Delegate to the wrapped iterator
213213
return c.subiterator.ResourceType()
214214
}

pkg/query/caveat_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -550,7 +550,8 @@ func TestCaveatIterator_Types(t *testing.T) {
550550

551551
resourceType, err := caveatIter.ResourceType()
552552
require.NoError(err)
553-
require.Equal("document", resourceType.Type) // From subiterator
553+
require.Len(resourceType, 1)
554+
require.Equal("document", resourceType[0].Type) // From subiterator
554555
})
555556

556557
t.Run("SubjectTypes", func(t *testing.T) {

pkg/query/datastore.go

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -270,9 +270,18 @@ func (r *RelationIterator) IterResourcesImpl(ctx *Context, subject ObjectAndRela
270270
return EmptyPathSeq(), nil
271271
}
272272

273+
// Handle wildcards first - they don't have subrelations and match any query relation
273274
if r.base.Wildcard() {
274275
return r.iterResourcesWildcardImpl(ctx, subject)
275276
}
277+
278+
// Check if subject relation matches what this iterator expects.
279+
// Both the schema's expected subrelation and the query's subject relation must match exactly.
280+
// Ellipsis is a specific relation value, not a wildcard.
281+
if r.base.Subrelation() != subject.Relation {
282+
return EmptyPathSeq(), nil
283+
}
284+
276285
filter := datastore.RelationshipsFilter{
277286
OptionalResourceType: r.base.DefinitionName(),
278287
OptionalResourceRelation: r.base.RelationName(),
@@ -354,11 +363,11 @@ func (r *RelationIterator) ID() string {
354363
return r.id
355364
}
356365

357-
func (r *RelationIterator) ResourceType() (ObjectType, error) {
358-
return ObjectType{
366+
func (r *RelationIterator) ResourceType() ([]ObjectType, error) {
367+
return []ObjectType{{
359368
Type: r.base.DefinitionName(),
360-
Subrelation: r.base.RelationName(),
361-
}, nil
369+
Subrelation: tuple.Ellipsis,
370+
}}, nil
362371
}
363372

364373
func (r *RelationIterator) SubjectTypes() ([]ObjectType, error) {

pkg/query/datastore_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -358,8 +358,9 @@ func TestRelationIterator_Types(t *testing.T) {
358358

359359
resourceType, err := relationIter.ResourceType()
360360
require.NoError(err)
361-
require.Equal("document", resourceType.Type)
362-
require.Equal("viewer", resourceType.Subrelation)
361+
require.Len(resourceType, 1)
362+
require.Equal("document", resourceType[0].Type)
363+
require.Equal(tuple.Ellipsis, resourceType[0].Subrelation)
363364
})
364365

365366
t.Run("SubjectTypes_NoSubrelation", func(t *testing.T) {

pkg/query/exclusion.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,7 @@ func (e *Exclusion) ID() string {
302302
return e.id
303303
}
304304

305-
func (e *Exclusion) ResourceType() (ObjectType, error) {
305+
func (e *Exclusion) ResourceType() ([]ObjectType, error) {
306306
// Exclusion's resources come from the main set
307307
return e.mainSet.ResourceType()
308308
}

pkg/query/exclusion_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -873,7 +873,8 @@ func TestExclusion_Types(t *testing.T) {
873873

874874
resourceType, err := exclusion.ResourceType()
875875
require.NoError(err)
876-
require.Equal("document", resourceType.Type) // From mainSet
876+
require.Len(resourceType, 1)
877+
require.Equal("document", resourceType[0].Type) // From mainSet
877878
})
878879

879880
t.Run("SubjectTypes", func(t *testing.T) {

0 commit comments

Comments
 (0)