[Feature Request] Devise a way to reuse policies from a related model
@ymc9 this feature would be a huge help in simplifying the access controls. Currently we have to use 7 layer deep nested access, and copying it to each child models is a laborious task, as well as maintaining it over time.
I will try working on the feature once I get some time free. Would like to learn the internals of zenstack :)
@ymc9 this feature would be a huge help in simplifying the access controls. Currently we have to use 7 layer deep nested access, and copying it to each child models is a laborious task, as well as maintaining it over time.
I will try working on the feature once I get some time free. Would like to learn the internals of zenstack :)
Contribution would be great @sidharthv96 ! @jiashengguo and I were discussing this feature two days ago 😄.
Just to confirm, the duplication you currently have is caused by the need to duplicate rules in a model and in other models where there's a relation to that model, right? As explained in the main post of this issue.
I think it might be slightly different from the mentioned issue.
In our simplified use case, a user in a team should be able to access all diagrams in that team. So we have some complex rules in diagrams table, based on the user role, team membership, whether the diagram is public or not, etc.
When adding a related object to the diagram, say attachments or comments, now we have to write all these rules, with one extra layer (diagram.
By your suggested approach, we could simply add a check(diagram, 'action') instead to the complex rules. Which would be much cleaner and maintainable.
@ymc9 do you have some pointers on where I should get started? Or some general idea on where all the changes would need to be made.
I think it might be slightly different from the mentioned issue.
In our simplified use case, a user in a team should be able to access all diagrams in that team. So we have some complex rules in diagrams table, based on the user role, team membership, whether the diagram is public or not, etc.
When adding a related object to the diagram, say attachments or comments, now we have to write all these rules, with one extra layer (diagram.), in each of the child models.
By your suggested approach, we could simply add a
check(diagram, 'action')instead to the complex rules. Which would be much cleaner and maintainable.@ymc9 do you have some pointers on where I should get started? Or some general idea on where all the changes would need to be made.
Got it. Yes, I agree a check function could really help here.
I thought about it, and maybe it's not too complex. Here're some ideas:
-
The
checkfunction should be declared instdlib.zmodel. It should be marked to be used only in access policy context. You can use thefuture()function as a reference. -
Access policies are translated by the "@core/access-policy" (
/packages/schema/src/plugins/access-policy) built-in plugin into typescript functions. Each model has separate functions for CRUD permissions. The functions take a context as input, and return a partial Prisma query filter. At runtime, ZenStack calls these functions and then inject into query payload.Here's an example:
model List { id Int todos Todo[] owner User @relation(...) @@allow('read', owner == auth()) } model Todo { id Int list List @relation(...) @@allow('read', list.owner == auth()) }The generated function will look like (simplified):
function List_read(context) { return { owner: { id: context.user.id } }; } return Todo_read(context) { return { list: { owner: { id: context.user.id } } }; }Now with the
checkfunction, the model can be changed to:model List { id Int todos Todo[] owner User @relation(...) @@allow('read', owner == auth()) } model Todo { id Int list List @relation(...) @@allow('read', check('read', list)) }And we can generate a function call to
List_readfromTodo_read:function List_read(context) { return { owner: { id: context.user.id } }; } return Todo_read(context) { return { list: List_read(context) }; }
The code that transforms ZModel AST to the policy functions as shown above resides in packages/schema/src/plugins/access-policy/expression-writer.
One caveat is that there can potentially be cyclic reference, resulting in infinite recursion. I'm not sure what's the best way to handle it ... Maybe we just disallow cycles first during validation?
Thank you.
Maybe we just disallow cycles first during validation?
This seems like the best option.
@ymc9 is this test, what should be the output?
it('auth using parent relation', async () => {
await check(
`
model User {
id String @id
posts Post[]
}
model Post {
id String @id
owner User @relation(fields: [ownerId], references: [id])
ownerId String @unique
comments Test[]
@@allow('all', auth() == owner)
}
model Test {
id String @id
post Post @relation(fields: [postId], references: [id])
postId String @unique
@@allow('all', check(post, 'read'))
}
`,
(model) => model.attributes[0].args[1].value,
`<what should be the case?>`
);
});
const option1 = {
post: {
...(user == null
? { OR: [] }
: {
owner: {
is: {
id: user.id,
},
},
}),
},
};
const option2 = user == null ? { OR: [] } : { post: { owner: { is: { id: user.id } } } };
This is what I'm assuming needs to be done. Please let me know if I'm on the right path.
auth using parent relation
I think it should be the option2 because the object will be directly as as a filter for model Test.
We shouldn't need to change TypeScriptExpressionTransformer because that class is for compiling ZModel expressions into TS expressions (for supporting @@validate etc.). The target should be the ExpressionWriter class which transforms expressions into Prisma query objects.
Btw, I suggest we swap the parameters of check to check('read', post), to make it look more consistent with @@allow.
You're making changes of the dev branch, right?
Btw, I suggest we swap the parameters of check to check('read', post), to make it look more consistent with @@allow.
check(post, 'read') is consistent with all the other function like search, startsWith, etc.
And we can generate a function call to
List_readfromTodo_read:function List_read(context) { return { owner: { id: context.user.id } }; } function Todo_read(context) { return { list: List_read(context) }; }
I think it should be the option2 because the object will be directly as as a filter for model Test.
const option1 = { post: user == null ? { OR: [] } : { owner: { is: { id: user.id } } } };
const option2 = user == null ? { OR: [] } : { post: { owner: { is: { id: user.id } } } }
Option 1 seems more inline with the example you mentioned earlier. There, we're only manipulating the children, in option 2, we'd have to modify the parent based on the conditions that the children might have. Is that feasible?
You're making changes of the dev branch, right?
Yes 👍
From my understanding so far, given a statement like @@allow(<action>, <expression>), ExpressionWriter only handles the <expression> part, and does not bother with <action>.
So, if we have to write @@allow('read', check(post, 'read')), ExpressionWriter would have to somehow use PolicyGenerator to figure out what expressions are allowing/denying post's read.
Is there any way for ExpressionWriter to figure out user == null ? { OR: [] } : { owner: { is: { id: user.id } } } without using PolicyGenerator? If Post had another allow operation, the condition would be different.
From my understanding so far, given a statement like
@@allow(<action>, <expression>),ExpressionWriteronly handles the<expression>part, and does not bother with<action>.So, if we have to write
@@allow('read', check(post, 'read')),ExpressionWriterwould have to somehow usePolicyGeneratorto figure out what expressions are allowing/denyingpost's read.Is there any way for
ExpressionWriterto figure outuser == null ? { OR: [] } : { owner: { is: { id: user.id } } }without usingPolicyGenerator? IfPosthad another allow operation, the condition would be different.
I see what you mean now.
ExpressionWriter's job is to turn a boolean expression into a prisma filter. In a more complex use case one can mix check call with other conditions to form a deep boolean tree, like: @@allow('read', check(post, 'read') && ...). ExpressionWriter deals with traversing such a tree. I think it's easier to let it continue processing the entire policy condition expression.
My thought is that when transforming check(post, 'read'), we don't really need to generate an inlined filter based on "read" policies of "post". Instead, we can generate a TS function call to Post_read (which is generated by PolicyGenerator), so the final composition of the policy rules happens at runtime. Still, it will result in some implicit coupling between ExpressionWriter and PolicyGenerator because the writer needs to know the naming convention of the generated policy functions. But I think it's OK since these two guys are tightly coupled anyway.
Also, I want to make a correction to my previous reply. I think option1 and option2 are equivalent so either of them should work. If we take the function call approach, it'll be just whatever the result is with the call 😄.
I'm not sure if I explained my thoughts clearly. Let me know if anything is confusing or doesn't sound right.
Btw, you can use this method to see he pre-compiled policies in TS form.
plugin policy {
provider = '@core/access-policy'
preserveTsFiles = true
After running zenstack generate, you can find the "policy.ts" file under "node_modules/.zenstack" folder.