https://authzed.com logo
Join Discord
Powered by
Unhandled case when following relation on subject ...
# spicedb
b
I think I have found a bug or at least an edge case that isn't being handled, let me know if I should create a GitHub issue about it. I've attached the schema, relations and assertions to reproduce it, but basically the problem is that SpiceDB doesn't seem to follow permissions of permissions in scenarios like:
Copy code
definition myObject {
    relation myRel: otherObj#rel1

    permission myPerm = myRel->rel2
}
Where 
rel1
and 
rel2
are both permissions (I haven't tested with relations, but in my use case I need them to be permissions), it doesn't seem like 
myPerm
ever evaluates to anything. Even doing an 
expand
or using 
--explain
on a 
check
seems to show that the relations aren't being followed properly. https://cdn.discordapp.com/attachments/844600078948630559/1228201342204379177/follow-permission-of-permission.yml?ex=662b2ea2&is=6618b9a2&hm=860a8a096fbc343d182116cd416d8b61a0d86b6a2f0ffe9fc6d128bce107dcca&
After a bit of digging in the code, I think the issue lies within the specific scenario of when you define an arrow in the permissions (tuple to userset) when the subject of your tupleset has a subject relation as opposed to being a straight subject ID. In particular, the code fails here (https://github.com/authzed/spicedb/blob/main/internal/graph/check.go#L532) when performing the 
checkComputedUserset
because it tries to typecheck the pair (
targetRR.Namespace, targetRR.Relation
) which in the case of a tuple to userset is 
rr.Namespace
and 
cu.Relation
respectively, which means that the check completely ignores 
rr.Relation
which is populated in this case.
v
It's not a bug, this is by design. I've brought this up to @Joey a couple of times, I think this is a surprising behaviour folks don't anticipate.
b
Is there a reason behind it? It doesn't seem to make much sense from the outside and isn't documented as far as I can see. I feel like it would "just" be a case of adding a check in https://github.com/authzed/spicedb/blob/main/internal/graph/check.go#L500-L507 that checks 
if rr.Relation != Ellipsis
and if so dispatch another check to expand the subject with relation into a list of singular subjects that can then be used in the continuation of the 
checkComputedUserset
?
Ok looking into it a bit more there might not be an easy way by dispatching another check as those are always designed to provide a check-type answer with a subject and a resource if I'm understanding the code correctly. What would be needed is more akin to a 
lookupSubjects
call which might get tricky as the 
ConcurrentChecker
and 
ConcurrentLookupSubjects
are two distinct 
struct
and we therefore can't easily dispatch a 
lookupSubjects
call as part of a 
check
call without some restructuring first (unless we use the fact that the 
localDispatcher
has both a 
checker
field and a 
lookupSubjectsHandler
field that we could use as a way for them to communicate?)
@vroldanbet If this is an issue that has already been brought up a few times and that is not intuitive for users, do you mind if I raise a GitHub Issue for it, this would allow for better visibility of the issue as well as a more technical place to have any potential discussions around how we could get around the limitation
j
It’s because not all subject types are required to have the target permission of the arrow
So it cannot be a type error if some do not
b
I see what you mean, but I feel like the expected behaviour would be to properly resolve the permission instead of "giving up" halfway through the evaluation when encountering a tuple to userset relation where the tupleset is a subject with a subject relation on it
v
I think this should be addressed, specially since failing silently can lead to a security issue if folks don't know that's the behaviour. @Joey ?
j
It does resolve the permission, if present
It skips if it isn’t, because we can’t resolve an unknown
We plan to add a lint rule if there is no permission to follow for all cars
Cases*
But if even one has to be skipped, we can’t warn
v
I may be missing something. In bens case, there is effectively a "double arrow", but the second arrow is not being resolved
I assume that rel exists, the schema example does not illustrate it
j
Does rel2 exist on otherObj?
I cannot see the schema on a phone
rel1 doesn’t matter for the arrow
v
Yeah I think what Ben was hitting is that the relationship stored has the subject relation pointing to a permission, and then is doing an arrow on top of that
j
But the subject relation doesn’t matter for an arrow
Arrows operate on the object
v
but that's the point Ben is bringing up
j
Doesn’t matter: that’s the spec
v
folks gravitate towards doing this
and then hit the problem
j
And because it’s the spec, we can’t warn if there are any valid branches
b
> rel1 doesn’t matter for the arrow It should though IMO
j
it can’t: arrows cannot operate over computed results
it would be a massive performance trap, for one
That’s why you can’t use a permission on the left side of an arrow either
directly, I mean
I’m all for adding a warning if t
The arrow can never be computed
But as I said, if even one branch is valid, then we can’t warn
v
at the very least this should be a compilation error
j
It can’t be
Unless all cases are invalid
Because this does work:
v
the compiler knows that's a permission, not a relation
on the relationship type
j
relation foo: first | second#whatever
v
ah, I see
j
Permission bar = foo->somepermundersecond
v
yeah, i get it
maybe we should reconsider allowing referencing non existing relations/permissions
I know it's ergonomic
but leads to this particular trap, and also folks have been "surprised" that it did not complain when it did not exist
its a footgun
j
We can but note the example above: it doesn’t exist under first but is still valid
And Ben’s example does, presumably, have the permission
b
My realistic scenario looks like this: I have groups that are hierarchical but can also have several parents. I'm trying to target all groups that are under 2 given groups and then using that to give access to documents. Something along the lines of
Copy code
definition user {}

definition group {
    relation direct_member: user
    relation child: group

    permission member = direct_member + child->member
    permission descendants = child + child->descendants
}

definition group_intersection {
    relation first: group
    relation second: group

    permission intersection = (first + first->descendants) & (second + second->descendants)
}

definition group_intersection_member {
    relation base: group_intersection#intersection

    permission member = base->member
}

definition document {
    relation viewer: user | group#member | group_intersection_member#member

    permission view = viewer
}
v
@Joey could this at least throw an error in runtime if applying an arrow over a permission?
j
No, see my example above that is valid
v
yeah but I mean, if it tries to dispatch it
j
it doesn’t matter if it is a relation or permission
The subject relation is not used at all for arrows
v
I know you say "it does not matter", but can we do something about it? we have enough information in runtime to determine this right?
j
Again, only if there aren’t any valid branches
It is valid to arrow over a subject with a relation or permission
changing that is a breaking change nonconformant with the paper
b
Do you mean the Zanzibar paper?
v
yes, Joey means that
There are alternatives to what you are trying to do - basically creating synthethic arrows
b
I don't believe the Zanzibar whitepaper ever mentions that the tupleset of a 
tuple_to_userset
can't be a 
_this
with an indirect ACL (subject with a subject relations in SpiceDB)
Do you have an example of how I could do that?
v
Copy code
definition user {}

definition group {
    relation direct_member: user
    relation child: group

    permission member = direct_member + child->member
    permission descendants = child + child->descendants
}

definition group_intersection {
    relation first: group
    relation second: group

    permission intersection = (first + first->descendants) & (second + second->descendants)
    permission intersection_member = intersection->member
}

definition group_intersection_member {
    relation base: group_intersection

    permission member = base->intersection_member
}

definition document {
    relation viewer: user | group#member | group_intersection_member#member

    permission view = viewer
}
b
@Joey You point out that this works:
Copy code
definition someObject {
   relation foo: first | second#whatever

   permission bar = foo->somepermundersecond
}
I really don't feel like it should work. When using 
second#whatever
for the 
foo
relation, 
second
is almost irrelevant here, and 
second#whatever
should be considered an un-breakable entity. Even if for now we can't use 
->
to follow a permission from a subject relation for performance reasons, I'm struggling to see scenarios where it would make sense to follow the 
->
on the subject type when there is a subject relation present
@vroldanbet
Copy code
permission intersection = (first + first->descendants) & (second + second->descendants)
    permission intersection_member = intersection->member
That part fails when I try to write the schema as I can't use a permission on the left side of the arrow
j
Except the arrow always operates over the object
And if “somepermundersecond” exists on “foo” as well
Then it works for both
I agree it’s non obvious
b
That's the current behaviour as it was implemented, but I don't think it makes sense nor is that behaviour ever mentioned in the whitepaper
j
the paper shows the TTU operating always over the object of the subject
TTU being the arrow
because it, as its name implies, is a tupleset with the arrow relation replacing the base object
anyway, I’m traveling right now so hard to deep dive
If the TTU operated off of the resolved subjects for the the subject relation, then check would have to invoke an equivalent of LookupSubjects on that subject permission in order to compute the expression
Which is very much not what the paper shows
b
I see what you mean. Do you have any suggestion on how to solve the scenario above?
j
@Ben not sure I follow what, exactly, you're trying to accomplish, but perhaps this will solve it? https://github.com/authzed/spicedb/issues/597
b
No, not really. That would avoid me having to define relations 
first
and 
second
and instead only have one relation that I can take the intersection arrow on, but at the end of the day I will still need to follow another permission after that. Let's say I have hierarchical groups that can have several parents. I have 
TX > Austin
, 
TX > Dallas
, 
CA > Sacramento
, 
CA > LA
, 
Capital > Austin
, 
Capital > Sacramento
, 
Regional > Dallas
, 
Regional > LA
. I want to give access to a document to members of groups that are both under 
TX
and 
Capital
(i.e. 
Austin
in this example). It is important to note that this is not the same as the intersection of inherited members of 
TX
and inherited members of 
Capital
, as I could have a user that is both in 
Sacramento
and 
Dallas
and would therefore match that requirement which I do not want.
j
sorry @Ben, I was traveling last week
in your above description, why not just grant the members directly, since that seems to be your intent?
the grant would be 
tx#member
and 
capital#member
, right?
b
The intersection of the members is different from the members of the intersection. I could have users that are both members of 
Sacramento
and 
Dallas
that would match 
tx#member & capital#member
but not 
(tx#child & capital#child)->member
which is what I want
j
but why do you need the child?
you said you're granting to the locations themselves
at least, that's what it seems
(apologies for not understanding)
b
Because that’s the logic we have? Let’s say you have a supermarket chain across several states, that has both Stores and Warehouses. We might have 2 documents, one for forklift regulations in Texas and one for forklift regulations in California. We want to give access to the first doc to all users that are part of a Warehouse in Texas, and the second to all users that are part of a Warehouse in California. There could however be a user that is part of a Warehouse in a different state and part of a Store in Texas, and we don’t want them to have access to the Texas forklift regulations
j
let's consider both a set of users called a 
group
(give me a bit to write this out)
let's define groups with names 
warehouse-tx
and 
warehouse-ca
and 
store-tx
and `store-ca`; then you'd grant access to the document like:
Copy code
document:forklistregs#viewer@group:warehouse-tx#member
now, that's simplified, but it does solve your immediate need
I suspect, however, you want to have the document link somehow to "tx" and "ca" and have it infer based on that?
b
Yes, it is a bit more complex than that: - The actual source of truth is the groups themselves, might be like 
location1
(under 
dallas
and 
warehouse
), 
location2
(under 
houston
and 
warehouse
), 
location3
(under 
sacramento
and 
store
), 
location4
(under 
LA
and 
warehouse
); it would be quite a bit of overhead to have to manually maintain secondary groups such as 
warehouse-tx
- This would be for a SaaS model, so we don't directly control what intersections are going to be used for access, the users will, so we can't even pre-determine that we will need 
warehouse-tx
for example - In some of the use cases, one half of the intersection will be static while the other one will be dynamic (like you said, using some other relation on the document)
j
if the intersections are dynamic
then you might need to create synthesized objects to perform the intersection
b
Yeah that's what I was thinking
v
@Joey with a denormalization approach like Authzed Materialize, I could imagine having a relation with 
subject_type+subject_relation
that is a TTU become denormalized. Wouldn0t that allow us to do chained TTUs over ComputedUsersets?
I'm probably not grasping the nuances of the model, but I'm going to try nevertheless. Why is this something that wouldnt work?
Copy code
definition user {}

definition document {
    relation owner: state#stores | state#warehouses

    permission read = owner->member
}

definition state {
    relation stores: store
    relation warehouses: warehouse
}

definition warehouse {
    relation members: user
}

definition store {
    relation members: user
}
b
cc @corkrean
@vroldanbet The issue is that we don't control the concepts of 
state
, 
warehouse
and 
store
, from our point of view they are all equivalent as a 
location
for example (as a SaaS provider), those concepts are how one customer might be using it, however a different customer might have a different sub-division of their locations, so it's not something we can inherently bake into the schema unfortunately
j
yes, but since Materialize is a cache, it would fail for any full consistency checks
b
Sorry just a quick follow up, clarifying what was meant by “synthesized objects” above, does it mean calculating what the intersection is outside of SpiceDB and writing the result in SpiceDB to be able to use it? Or is there a way to do synthesized objects directly in SpiceDB?
j
it depends on the schema
I still don't fully understand your exact data model requirements, so its hard to comment from a position of knowledge
28 Views
PreviousNext
Powered by