doc: updates about the check API #378
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe pull request introduces enhancements to the documentation for ZenStack's Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (8)
docs/reference/prisma-client-ext.md (4)
Line range hint
13-15: Consider adding context about edge runtime limitations.The warning about edge runtime support is clear, but it would be more helpful to briefly explain why it's not supported (e.g., if it requires Node.js-specific features).
:::warning -The API is not supported on edge runtime (e.g., Cloudflare Workers or Vercel Edge). You'll get a runtime error when calling it. +The API is not supported on edge runtime (e.g., Cloudflare Workers or Vercel Edge) due to its dependency on Node.js-specific features for policy evaluation. You'll get a runtime error when calling it. :::
Line range hint
23-31: Clarify the "over-permissive" behavior.The danger notice warns about over-permissive behavior but doesn't explain what this means in practice. Consider adding a brief example to illustrate when the check might return
trueeven though actual database access would be denied.:::danger -Permission checking is an approximation and can be over-permissive. You MUST NOT trust it and circumvent the real access control mechanism (e.g., calling raw Prisma CRUD operations without further authorization checks). +Permission checking is an approximation and can be over-permissive (e.g., it might return true for reading a post even if the referenced author record is inaccessible). You MUST NOT trust it and circumvent the real access control mechanism (e.g., calling raw Prisma CRUD operations without further authorization checks). :::
Line range hint
35-46: Improve theCheckArgstype definition.The
wheretype is shown with...which might be confusing. Consider showing that it accepts the same filters as the model's regularwhereclause.type CheckArgs = { /** * The operation to check for */ operation: 'create' | 'read' | 'update' | 'delete'; /** * The optional additional constraints to impose on the model fields - */ - where?: { ... }; + * Accepts the same filters as the model's regular `where` clause + */ + where?: Prisma.PostWhereInput; }
Line range hint
50-57: Enhance the example with more realistic scenarios.While the current example is clear, consider adding more practical scenarios that demonstrate checking multiple operations and combining conditions.
const db = enhance(prisma, { user: getCurrentUser() }); -// check if the current user can read published posts -await canRead = await db.post.check({ +// Check various permissions +const canReadPublished = await db.post.check({ operation: 'read', where: { published: true } }); + +// Check update permission with multiple conditions +const canUpdateOwn = await db.post.check({ + operation: 'update', + where: { + authorId: currentUser.id, + published: false + } +});docs/guides/check-permission.md (4)
Line range hint
8-14: Strengthen the preview status warning.Consider adding a more prominent warning banner at the beginning of the document to emphasize:
- The preview/beta status of this feature
- Potential API changes in future versions
- The need for careful testing in non-production environments first
Here's a suggested addition right after the title:
# Checking Permissions Without Hitting the Database (Preview) +:::warning PREVIEW FEATURE +This feature is in preview and: +- The API may change in future versions +- Should be thoroughly tested before production use +- May have limitations not yet discovered +::: ## Introduction
Line range hint
134-141: Enhance type information and error handling examples.The API signature would be more helpful with:
- Complete TypeScript types for better IDE support
- Error handling examples
- Return type documentation
Consider expanding the example:
type CheckArgs = { /** * The operation to check for */ operation: 'create' | 'read' | 'update' | 'delete'; /** * The optional additional constraints to impose on the model fields */ where?: { id?: number; title?: string; published?: boolean; authorId?: number }; -} +}; + +/** + * Example with error handling: + */ +try { + const canRead = await db.post.check({ + operation: 'read', + where: { published: false } + }); + // Handle the result +} catch (error) { + // Handle potential errors: + // - Invalid operation + // - Unsupported field types + // - SAT solver errors +}
Line range hint
175-196: Expand the limitations section with examples.The limitations section would be more helpful with concrete examples of:
- Unsupported scenarios and their workarounds
- Edge runtime limitations and alternative approaches
- How ignored features affect the results
Consider adding examples:
## Limitations + +### Unsupported Features Example + +```ts +// ❌ Unsupported: String operations +@@allow('read', title.startsWith('Draft-')) + +// ❌ Unsupported: Array operations +@@allow('read', tags.includes('public')) + +// ❌ Unsupported: Relation traversal +@@allow('read', author.organization.id == auth().orgId) +``` + +### Edge Runtime Alternative + +When deploying to edge runtimes, consider: +1. Moving permission checks to the main API routes +2. Using simplified boolean checks for UI rendering +3. Implementing a fallback strategy
Line range hint
197-203: Add security considerations for anonymous context.While the evaluation rules are clear, consider adding:
- Security implications of anonymous access
- Best practices for handling anonymous contexts
- Common pitfalls to avoid
Consider adding a security note:
### Security Considerations When handling anonymous contexts: - Always validate permissions again on the server-side - Consider implementing rate limiting for anonymous checks - Log and monitor anonymous permission checks for abuse
Summary by CodeRabbit
New Features
checkAPI for permission checking without database access, enhancing UI performance.Documentation
checkAPI, clarifying its usage and limitations.