Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
96cf65c to
d34bcd0
Compare
app/best-practices/page.mdx
Outdated
| } | ||
| ``` | ||
|
|
||
| This will fail at `WriteSchema` time because the type annotation provided is incomplete; it should be `permission edit: user | serviceaccount = viewer & admin`. |
There was a problem hiding this comment.
This is weird. That silences the error but doesn't fix the semantic.
app/best-practices/page.mdx
Outdated
| } | ||
| ``` | ||
|
|
||
| This schema compiles successfully, but a call to `CheckPermission` for `document#edit` will _always_ return false at runtime because the intersection of `user` and `serviceaccount` is empty. This is a bug that can go unnoticed for a long period of time, because the API call doesn't return an error. |
There was a problem hiding this comment.
"because no relationship can simultaneously be a user relationship and a serviceaccount relationship"
| It can be tempting to define the authorization logic for an endpoint as being the `AND` or `OR` of the checks of other permissions, especially when the alternative is writing a new schema. | ||
| However, this increases the likelihood of drift across your system, hides the authorization logic for a system in that system's codebase, and increases the load on SpiceDB. | ||
|
|
||
| ### Use Typechecking |
There was a problem hiding this comment.
This should be in the best practice list, but I think it should be a shorter description that references the documentation
There was a problem hiding this comment.
This should be in the best practice list
WDYM? it is in the list already (see the name of the file)
There was a problem hiding this comment.
Right - for most other things that are documented elsewhere, the "you should" is a part of this doc, but the "what" and "how" are left to the linked documentation
There was a problem hiding this comment.
Ok, moved there!
app/spicedb/concepts/schema/page.mdx
Outdated
|
|
||
| ### Typechecking | ||
|
|
||
| The `use typechecking` feature allows you to explicitly declare and validate the types that permissions can resolve to, providing compile-time type safety for your authorization schemas. |
There was a problem hiding this comment.
"compile-time" is technically true, but most folks aren't compiling their schemas, and I think this would lead folks to think that you needed to use zed schema compile to make use of this feature.
| permission view: user = viewer // Validated | ||
| permission edit = viewer // Not validated (no annotation) |
There was a problem hiding this comment.
Is that true? Are we not checking every permission?
There was a problem hiding this comment.
This also seems like it contradicts the text in the best practices part of the documentation
There was a problem hiding this comment.
Are we not checking every permission?
I just tried this in the playground and no, it doesn't enforce that every permission has the annotation, it only validates the annotations that exist.
There was a problem hiding this comment.
This also seems like it contradicts the text in the best practices part of the documentation
no, in that example, the annotation exists but it is incorrect
app/spicedb/concepts/schema/page.mdx
Outdated
| relation viewer: user | team | ||
| permission view: user | team = viewer |
There was a problem hiding this comment.
Better to have two separate relations, I think - otherwise this kinda reads as trivial.
d34bcd0 to
e6c2cb6
Compare
e6c2cb6 to
fd32dbc
Compare
No description provided.