feat(core): add fine-grain authorization support#2999
feat(core): add fine-grain authorization support#2999alkalescent wants to merge 24 commits intoDSPX-2185-casbin-stacked-prfrom
Conversation
- support multi group claims - refactor for efficiency - refactor for future casbin adapter support
Co-authored-by: Jake Van Vorhis <83739412+jakedoublev@users.noreply.github.com>
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 capabilities by introducing a new fine-grained system. It shifts the focus from broad path-based access to granular control based on specific RPC methods and dynamic resource attributes. This enhancement provides a more robust and flexible security model, allowing services to define and enforce access policies at a much finer level of detail, while ensuring a smooth transition and continued support for existing authorization rules. 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. A fine-grain net, so strong and new, Resources guarded, clear and true. No path alone, but context deep, Where access rules, the secrets keep. 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 is an excellent pull request that introduces a well-designed and robust fine-grained authorization system. The use of a pluggable authorizer interface, a resolver registry for service-specific logic, and the Inversion of Control pattern are all great architectural choices that will improve the platform's extensibility and maintainability. The code is clean, well-structured, and thoroughly tested. The accompanying documentation and ADRs are very detailed and helpful for understanding the new system. I have one high-severity comment regarding an inconsistency in the ADR document that should be addressed to avoid confusion. Overall, this is a fantastic contribution.
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
NANOTDF Benchmark Results:
|
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
NANOTDF Benchmark Results:
|
|
Tests pass; changing base to stacked pr branch. |
|
Nice! I like the cache. How do we handle memory pressure with the cache fails? We also have a cache manager which might be useful here to make sure we have a central management interface for all cache behavior. I'm good with making that an enhancement as another PR since it's not crucial for determining if this is the right direction. |
|
There's no memory pressure handling since this cache is request-scoped (data is assumed to be small and cache is assumed to be short-lived). I also thought the cache manager would be an elegant approach here after reading |
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
NANOTDF Benchmark Results:
|
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
NANOTDF Benchmark Results:
|
|
I chose a hybrid approach, using the platform cache manager for inter-request caching and keeping the resolver context for intra-request caching. The benefit of the resolver context is that the handlers don't need knowledge of the cache keys. |
2 participants
Proposed Changes
Checklist
Testing Instructions