feat(authz): add SQL store for Casbin #3004
Conversation
- support multi group claims - refactor for efficiency - refactor for future casbin adapter support
Co-authored-by: Jake Van Vorhis <[email protected]>
This reverts commit 6c8ad88.
This reverts commit 12daa44.
github-actions
bot
added
comp:policy
Summary of ChangesHello @alkalescent, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly upgrades the platform's authorization framework by introducing a new, more granular system. The new approach enables resource-level access control by allowing services to define and extract specific dimensions from requests, which are then evaluated against policies. This is complemented by the ability to persist policies in a database, facilitating dynamic updates and improved operational flexibility. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. With code we build, a world unseen, Where access rules, both strict and keen. New policies bloom, a finer grain, To guard the data, through sun and rain. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a significant and well-designed enhancement to the authorization system, adding support for resource-level permissions via a new v2 authorization model. The changes are extensive, touching authentication, authorization, configuration, and service-level logic, but are well-structured and thoroughly documented.
Key improvements include:
- A new
authzpackage with a pluggableAuthorizerinterface, which is a great architectural improvement for future flexibility. - The introduction of an Inversion of Control (IoC) pattern for authorization "resolvers", allowing services to provide their own logic for extracting resource dimensions.
- A new Casbin v2 model that supports fine-grained policies based on RPC and custom dimensions.
- Support for storing Casbin policies in a SQL database, enabling runtime policy updates.
- Comprehensive documentation, including new ADRs and developer guides, which will be invaluable for maintainability and onboarding.
- Thoughtful implementation details, such as caching layers to mitigate performance impact and maintaining backward compatibility with the v1 authorization model.
The code is of high quality and demonstrates a solid understanding of software architecture principles. I have one minor suggestion for improving the clarity of an architectural diagram in the new ADR. Overall, this is an excellent contribution to the platform.
| │ │ │ (IoC / "Hollywood Principle" - framework calls service) │ │ │ | ||
| │ │ └─────────────────────────────────────────────────────────────┘ │ │ | ||
| │ │ ┌─────────────────────────────────────────────────────────────┐ │ │ | ||
| │ │ │ 3. Enforce → Casbin(sub, type, action, serialized_dims) │ │ │ |
There was a problem hiding this comment.
The diagram for the request flow seems to contain a small inconsistency with the v2 model described in the document. The enforcement step shows Casbin(sub, type, action, serialized_dims), which appears to be based on the v1 model.
According to the v2 model definition ((subject, rpc, dimensions)), the enforcement call should be Casbin(sub, rpc, serialized_dims). Updating the diagram to reflect this would improve clarity and consistency with the rest of the document.
| │ │ │ 3. Enforce → Casbin(sub, type, action, serialized_dims) │ │ │ | |
| │ │ │ 3. Enforce → Casbin(sub, rpc, serialized_dims) │ │ │ |
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
Error Summary
TDF3 Benchmark Results:
Error Summary:
NANOTDF Benchmark Results:
Error Summary:
|
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
Error Summary
TDF3 Benchmark Results:
Error Summary:
NANOTDF Benchmark Results:
Error Summary:
|
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
Error Summary
TDF3 Benchmark Results:
Error Summary:
NANOTDF Benchmark Results:
Error Summary:
|
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
NANOTDF Benchmark Results:
|
Proposed Changes
Checklist
Testing Instructions