feat(authz): DSPX-894 auth svc registered resource GetEntitlement support

[ryanulit]

Proposed Changes

• add GetEntitlementsRegisteredResources method to pdp access v2 logic
• add test coverage

Checklist

• I have added or updated unit tests
• I have added or updated integration tests (if appropriate)
• I have added or updated documentation

Testing Instructions

[gemini-code-assist]

Hello @ryanulit, I'm Gemini Code Assist¹! 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!

Summary of Changes

Hello! Gemini here, providing a summary of this pull request to help everyone quickly understand the changes.

This PR introduces support for fetching entitlements based on Registered Resource Values within the authorization service's Policy Decision Point (PDP). Previously, entitlement checks primarily focused on entities resolved via the Entity Resolution Service (ERS). This change adds a new path to handle entitlement requests directly for registered resource values, which do not have corresponding entity representations in ERS.

The core changes involve modifying the PDP to load and cache registered resources and their values, adding a new method to the PDP to calculate entitlements specifically for registered resource values, and updating the Just-In-Time PDP to utilize this new functionality when a registered resource value FQN is provided as the entity identifier.

Highlights

• Registered Resource Entitlements: Adds the capability for the authorization service to calculate entitlements for Registered Resource Values, expanding the types of resources that can be checked for access.
• PDP Enhancement: The Policy Decision Point (PDP) is updated to load and store registered resources and their values in memory upon initialization, alongside attributes and subject mappings.
• New Entitlement Path: A dedicated logic path is added to the GetEntitlements flow in the Just-In-Time PDP to handle requests where the entity identifier is a Registered Resource Value FQN, bypassing the need for ERS resolution.
• Validation and Error Handling: New validation logic and error types are introduced to ensure that registered resources are valid when loaded into the PDP.

Changelog

Click here to see the changelog

• service/internal/access/v2/helpers.go
  • Added ErrInvalidRegisteredResource and ErrInvalidRegisteredResourceValue error variables (lines 19-20).
• service/internal/access/v2/just_in_time_pdp.go
  • Imported strings (line 8) and registeredresources protocol (line 17).
  • Modified NewJustInTimePDP to fetch all registered resources using fetchAllRegisteredResources and pass them to NewPolicyDecisionPoint (lines 68-72).
  • Updated the GetEntitlements method to handle the RegisteredResourceValueFqn case by calling the new pdp.GetEntitlementsRegisteredResource method (lines 159-161).
  • Added the fetchAllRegisteredResources method to retrieve all registered resources from the SDK with pagination (lines 280-306).
• service/internal/access/v2/pdp.go
  • Imported slices (line 8) and identifier library (line 12).
  • Added allRegisteredResourceValuesByFQN map field to PolicyDecisionPoint struct (line 52).
  • Modified NewPolicyDecisionPoint constructor to accept allRegisteredResources (line 72).
  • Added logic in NewPolicyDecisionPoint to populate allRegisteredResourceValuesByFQN map from the provided registered resources, including validation (lines 130-143).
  • Added the GetEntitlementsRegisteredResource method to calculate entitlements for a given registered resource value FQN (lines 321-376).
• service/internal/access/v2/validators.go
  • Added the validateRegisteredResource function to validate registered resource objects (lines 100-125).
• service/internal/access/v2/validators_test.go
  • Added TestValidateRegisteredResource function with various test cases for the new validation logic (lines 286-360).

Using Gemini Code Assist

The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	/gemini help	Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and 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 to provide feedback.

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 new path is laid,
For resources registered,
Access now is clear.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request effectively adds support for GetEntitlement for registered resources. The new functions for fetching and validating registered resources are clear, and the integration into the Policy Decision Point (PDP) is logical. The added unit tests for the validator function are comprehensive.

I've identified a critical issue regarding a potential nil pointer dereference that needs to be addressed. Additionally, there's a TODO comment regarding FQN validation that would be good to resolve for enhanced robustness.

While not part of the code changes themselves, please ensure that unit tests covering the new logic in PolicyDecisionPoint.GetEntitlementsRegisteredResource and the new case in JustInTimePDP.GetEntitlements are added, if not already planned for a subsequent PR, to maintain good test coverage.

Summary of Findings

• Potential Nil Pointer Dereference: In GetEntitlementsRegisteredResource, if the provided registeredResourceValueFQN is not found in the map, registeredResourceValue will be nil, leading to a panic when its methods are called. A nil check is required.
• FQN Validation: A TODO comment in GetEntitlementsRegisteredResource suggests validating the registeredResourceValueFQN. While a nil check after map lookup is crucial, validating the FQN format beforehand would improve robustness.

Merge Readiness

The pull request makes good progress in adding support for registered resource entitlements. However, there is a critical issue regarding a potential nil pointer dereference that must be addressed before this PR can be considered ready for merging. Additionally, there's a suggestion for enhancing FQN validation.

I am unable to approve pull requests directly. Please address the critical issue, consider the medium-severity feedback, and then have other reviewers approve this code before merging.

[github-actions]

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric	Value
Approved Decision Requests	5000
Denied Decision Requests	0
Total Time	484.09279ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric	Value
Error	internal: failed to create new policy decision point: invalid registered resource: registered resource values are empty: access: invalid registered resource
Total Time	34.627065ms

Bulk Benchmark Results

Metric	Value
Total Decrypts	100
Successful Decrypts	0
Failed Decrypts	100
Total Time	176.724722ms
Throughput	0.00 requests/second

Error Summary

Error Message	Occurrences
splitKey.unable to reconstruct split key: map[{https://localhost:8080 }:kao unwrap failed for split {https://localhost:8080 }: internal: internal error
rpc error: code = Internal desc = could not perform access]
kao unwrap failed for split {https://localhost:8080 }: internal: internal error
rpc error: code = Internal desc = could not perform access	100 occurrences

TDF3 Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	0
Failed Requests	5000
Concurrent Requests	50
Total Time	23.051718574s
Throughput	0.00 requests/second

Error Summary:

Error Message	Occurrences
read error: reader.WriteTo failed: splitKey.unable to reconstruct split key: map[{https://localhost:8080 }:kao unwrap failed for split {https://localhost:8080 }: internal: internal error rpc error: code = Internal desc = could not perform access] kao unwrap failed for split {https://localhost:8080 }: internal: internal error rpc error: code = Internal desc = could not perform access	5000 occurrences

NANOTDF Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	0
Failed Requests	5000
Concurrent Requests	50
Total Time	18.808733102s
Throughput	0.00 requests/second

Error Summary:

Error Message	Occurrences
ReadNanoTDF error: getNanoRewrapKey: rewrapError: internal: internal error rpc error: code = Internal desc = could not perform access	5000 occurrences

Standard Benchmark Metrics Skipped or Failed

[github-actions]

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric	Value
Approved Decision Requests	5000
Denied Decision Requests	0
Total Time	466.647888ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric	Value
Error	internal: failed to create new policy decision point: invalid registered resource: registered resource values are empty: access: invalid registered resource
Total Time	38.185406ms

Bulk Benchmark Results

Metric	Value
Total Decrypts	100
Successful Decrypts	0
Failed Decrypts	100
Total Time	169.613509ms
Throughput	0.00 requests/second

Error Summary

Error Message	Occurrences
splitKey.unable to reconstruct split key: map[{https://localhost:8080 }:kao unwrap failed for split {https://localhost:8080 }: internal: internal error
rpc error: code = Internal desc = could not perform access]
kao unwrap failed for split {https://localhost:8080 }: internal: internal error
rpc error: code = Internal desc = could not perform access	100 occurrences

TDF3 Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	0
Failed Requests	5000
Concurrent Requests	50
Total Time	23.358189081s
Throughput	0.00 requests/second

Error Summary:

Error Message	Occurrences
read error: reader.WriteTo failed: splitKey.unable to reconstruct split key: map[{https://localhost:8080 }:kao unwrap failed for split {https://localhost:8080 }: internal: internal error rpc error: code = Internal desc = could not perform access] kao unwrap failed for split {https://localhost:8080 }: internal: internal error rpc error: code = Internal desc = could not perform access	5000 occurrences

NANOTDF Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	0
Failed Requests	5000
Concurrent Requests	50
Total Time	18.922077039s
Throughput	0.00 requests/second

Error Summary:

Error Message	Occurrences
ReadNanoTDF error: getNanoRewrapKey: rewrapError: internal: internal error rpc error: code = Internal desc = could not perform access	5000 occurrences

Standard Benchmark Metrics Skipped or Failed

[github-actions]

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric	Value
Approved Decision Requests	5000
Denied Decision Requests	0
Total Time	475.959095ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric	Value
Error	internal: failed to create new policy decision point: invalid registered resource: registered resource values are empty: access: invalid registered resource
Total Time	36.010989ms

Bulk Benchmark Results

Metric	Value
Total Decrypts	100
Successful Decrypts	0
Failed Decrypts	100
Total Time	176.064542ms
Throughput	0.00 requests/second

Error Summary

Error Message	Occurrences
splitKey.unable to reconstruct split key: map[{https://localhost:8080 }:kao unwrap failed for split {https://localhost:8080 }: internal: internal error
rpc error: code = Internal desc = could not perform access]
kao unwrap failed for split {https://localhost:8080 }: internal: internal error
rpc error: code = Internal desc = could not perform access	100 occurrences

TDF3 Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	0
Failed Requests	5000
Concurrent Requests	50
Total Time	23.205803157s
Throughput	0.00 requests/second

Error Summary:

Error Message	Occurrences
read error: reader.WriteTo failed: splitKey.unable to reconstruct split key: map[{https://localhost:8080 }:kao unwrap failed for split {https://localhost:8080 }: internal: internal error rpc error: code = Internal desc = could not perform access] kao unwrap failed for split {https://localhost:8080 }: internal: internal error rpc error: code = Internal desc = could not perform access	5000 occurrences

NANOTDF Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	0
Failed Requests	5000
Concurrent Requests	50
Total Time	18.821219782s
Throughput	0.00 requests/second

Error Summary:

Error Message	Occurrences
ReadNanoTDF error: getNanoRewrapKey: rewrapError: internal: internal error rpc error: code = Internal desc = could not perform access	5000 occurrences

Standard Benchmark Metrics Skipped or Failed

[github-actions]

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric	Value
Approved Decision Requests	5000
Denied Decision Requests	0
Total Time	479.078914ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric	Value
Error	internal: failed to create new policy decision point: invalid registered resource: registered resource values are empty: access: invalid registered resource
Total Time	31.965446ms

Bulk Benchmark Results

Metric	Value
Total Decrypts	100
Successful Decrypts	0
Failed Decrypts	100
Total Time	171.427143ms
Throughput	0.00 requests/second

Error Summary

Error Message	Occurrences
splitKey.unable to reconstruct split key: map[{https://localhost:8080 }:kao unwrap failed for split {https://localhost:8080 }: internal: internal error
rpc error: code = Internal desc = could not perform access]
kao unwrap failed for split {https://localhost:8080 }: internal: internal error
rpc error: code = Internal desc = could not perform access	100 occurrences

TDF3 Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	0
Failed Requests	5000
Concurrent Requests	50
Total Time	23.107730951s
Throughput	0.00 requests/second

Error Summary:

Error Message	Occurrences
read error: reader.WriteTo failed: splitKey.unable to reconstruct split key: map[{https://localhost:8080 }:kao unwrap failed for split {https://localhost:8080 }: internal: internal error rpc error: code = Internal desc = could not perform access] kao unwrap failed for split {https://localhost:8080 }: internal: internal error rpc error: code = Internal desc = could not perform access	5000 occurrences

NANOTDF Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	0
Failed Requests	5000
Concurrent Requests	50
Total Time	18.884674416s
Throughput	0.00 requests/second

Error Summary:

Error Message	Occurrences
ReadNanoTDF error: getNanoRewrapKey: rewrapError: internal: internal error rpc error: code = Internal desc = could not perform access	5000 occurrences

Standard Benchmark Metrics Skipped or Failed

[github-actions]

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric	Value
Approved Decision Requests	5000
Denied Decision Requests	0
Total Time	471.473276ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric	Value
Error	internal: failed to create new policy decision point: invalid registered resource: registered resource values are empty: access: invalid registered resource
Total Time	33.797501ms

Bulk Benchmark Results

Metric	Value
Total Decrypts	100
Successful Decrypts	0
Failed Decrypts	100
Total Time	177.524344ms
Throughput	0.00 requests/second

Error Summary

Error Message	Occurrences
splitKey.unable to reconstruct split key: map[{https://localhost:8080 }:kao unwrap failed for split {https://localhost:8080 }: internal: internal error
rpc error: code = Internal desc = could not perform access]
kao unwrap failed for split {https://localhost:8080 }: internal: internal error
rpc error: code = Internal desc = could not perform access	100 occurrences

TDF3 Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	0
Failed Requests	5000
Concurrent Requests	50
Total Time	24.474094924s
Throughput	0.00 requests/second

Error Summary:

Error Message	Occurrences
read error: reader.WriteTo failed: splitKey.unable to reconstruct split key: map[{https://localhost:8080 }:kao unwrap failed for split {https://localhost:8080 }: internal: internal error rpc error: code = Internal desc = could not perform access] kao unwrap failed for split {https://localhost:8080 }: internal: internal error rpc error: code = Internal desc = could not perform access	5000 occurrences

NANOTDF Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	0
Failed Requests	5000
Concurrent Requests	50
Total Time	19.891297302s
Throughput	0.00 requests/second

Error Summary:

Error Message	Occurrences
ReadNanoTDF error: getNanoRewrapKey: rewrapError: internal: internal error rpc error: code = Internal desc = could not perform access	5000 occurrences

Standard Benchmark Metrics Skipped or Failed

[github-actions]

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric	Value
Approved Decision Requests	5000
Denied Decision Requests	0
Total Time	479.51545ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric	Value
Error	internal: failed to create new policy decision point: invalid registered resource: registered resource values are empty: access: invalid registered resource
Total Time	46.436782ms

Bulk Benchmark Results

Metric	Value
Total Decrypts	100
Successful Decrypts	0
Failed Decrypts	100
Total Time	171.783212ms
Throughput	0.00 requests/second

Error Summary

Error Message	Occurrences
splitKey.unable to reconstruct split key: map[{https://localhost:8080 }:kao unwrap failed for split {https://localhost:8080 }: internal: internal error
rpc error: code = Internal desc = could not perform access]
kao unwrap failed for split {https://localhost:8080 }: internal: internal error
rpc error: code = Internal desc = could not perform access	100 occurrences

TDF3 Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	0
Failed Requests	5000
Concurrent Requests	50
Total Time	23.059860784s
Throughput	0.00 requests/second

Error Summary:

Error Message	Occurrences
read error: reader.WriteTo failed: splitKey.unable to reconstruct split key: map[{https://localhost:8080 }:kao unwrap failed for split {https://localhost:8080 }: internal: internal error rpc error: code = Internal desc = could not perform access] kao unwrap failed for split {https://localhost:8080 }: internal: internal error rpc error: code = Internal desc = could not perform access	5000 occurrences

NANOTDF Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	0
Failed Requests	5000
Concurrent Requests	50
Total Time	18.838790288s
Throughput	0.00 requests/second

Error Summary:

Error Message	Occurrences
ReadNanoTDF error: getNanoRewrapKey: rewrapError: internal: internal error rpc error: code = Internal desc = could not perform access	5000 occurrences

Standard Benchmark Metrics Skipped or Failed

[github-actions]

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric	Value
Approved Decision Requests	5000
Denied Decision Requests	0
Total Time	604.914748ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric	Value
Error	internal: failed to create new policy decision point: invalid registered resource: registered resource values are empty: access: invalid registered resource
Total Time	64.364966ms

Bulk Benchmark Results

Metric	Value
Total Decrypts	100
Successful Decrypts	0
Failed Decrypts	100
Total Time	207.218941ms
Throughput	0.00 requests/second

Error Summary

Error Message	Occurrences
splitKey.unable to reconstruct split key: map[{https://localhost:8080 }:kao unwrap failed for split {https://localhost:8080 }: internal: internal error
rpc error: code = Internal desc = could not perform access]
kao unwrap failed for split {https://localhost:8080 }: internal: internal error
rpc error: code = Internal desc = could not perform access	100 occurrences

TDF3 Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	0
Failed Requests	5000
Concurrent Requests	50
Total Time	26.716406984s
Throughput	0.00 requests/second

Error Summary:

Error Message	Occurrences
read error: reader.WriteTo failed: splitKey.unable to reconstruct split key: map[{https://localhost:8080 }:kao unwrap failed for split {https://localhost:8080 }: internal: internal error rpc error: code = Internal desc = could not perform access] kao unwrap failed for split {https://localhost:8080 }: internal: internal error rpc error: code = Internal desc = could not perform access	5000 occurrences

NANOTDF Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	0
Failed Requests	5000
Concurrent Requests	50
Total Time	23.17795764s
Throughput	0.00 requests/second

Error Summary:

Error Message	Occurrences
ReadNanoTDF error: getNanoRewrapKey: rewrapError: internal: internal error rpc error: code = Internal desc = could not perform access	5000 occurrences

Standard Benchmark Metrics Skipped or Failed

[github-actions]

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric	Value
Approved Decision Requests	5000
Denied Decision Requests	0
Total Time	493.397016ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric	Value
Error	internal: failed to create new policy decision point: invalid registered resource: registered resource values are empty: access: invalid registered resource
Total Time	44.950255ms

Bulk Benchmark Results

Metric	Value
Total Decrypts	100
Successful Decrypts	0
Failed Decrypts	100
Total Time	181.710667ms
Throughput	0.00 requests/second

Error Summary

Error Message	Occurrences
splitKey.unable to reconstruct split key: map[{https://localhost:8080 }:kao unwrap failed for split {https://localhost:8080 }: internal: internal error
rpc error: code = Internal desc = could not perform access]
kao unwrap failed for split {https://localhost:8080 }: internal: internal error
rpc error: code = Internal desc = could not perform access	100 occurrences

TDF3 Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	0
Failed Requests	5000
Concurrent Requests	50
Total Time	23.27656853s
Throughput	0.00 requests/second

Error Summary:

Error Message	Occurrences
read error: reader.WriteTo failed: splitKey.unable to reconstruct split key: map[{https://localhost:8080 }:kao unwrap failed for split {https://localhost:8080 }: internal: internal error rpc error: code = Internal desc = could not perform access] kao unwrap failed for split {https://localhost:8080 }: internal: internal error rpc error: code = Internal desc = could not perform access	5000 occurrences

NANOTDF Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	0
Failed Requests	5000
Concurrent Requests	50
Total Time	19.030489513s
Throughput	0.00 requests/second

Error Summary:

Error Message	Occurrences
ReadNanoTDF error: getNanoRewrapKey: rewrapError: internal: internal error rpc error: code = Internal desc = could not perform access	5000 occurrences

Standard Benchmark Metrics Skipped or Failed

[github-actions]

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric	Value
Approved Decision Requests	5000
Denied Decision Requests	0
Total Time	480.383606ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric	Value
Error	internal: failed to create new policy decision point: invalid registered resource: registered resource values are empty: access: invalid registered resource
Total Time	47.344243ms

Bulk Benchmark Results

Metric	Value
Total Decrypts	100
Successful Decrypts	0
Failed Decrypts	100
Total Time	175.351206ms
Throughput	0.00 requests/second

Error Summary

Error Message	Occurrences
splitKey.unable to reconstruct split key: map[{https://localhost:8080 }:kao unwrap failed for split {https://localhost:8080 }: internal: internal error
rpc error: code = Internal desc = could not perform access]
kao unwrap failed for split {https://localhost:8080 }: internal: internal error
rpc error: code = Internal desc = could not perform access	100 occurrences

TDF3 Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	0
Failed Requests	5000
Concurrent Requests	50
Total Time	22.844122437s
Throughput	0.00 requests/second

Error Summary:

Error Message	Occurrences
read error: reader.WriteTo failed: splitKey.unable to reconstruct split key: map[{https://localhost:8080 }:kao unwrap failed for split {https://localhost:8080 }: internal: internal error rpc error: code = Internal desc = could not perform access] kao unwrap failed for split {https://localhost:8080 }: internal: internal error rpc error: code = Internal desc = could not perform access	5000 occurrences

NANOTDF Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	0
Failed Requests	5000
Concurrent Requests	50
Total Time	18.560027362s
Throughput	0.00 requests/second

Error Summary:

Error Message	Occurrences
ReadNanoTDF error: getNanoRewrapKey: rewrapError: internal: internal error rpc error: code = Internal desc = could not perform access	5000 occurrences

Standard Benchmark Metrics Skipped or Failed

[jakedoublev]

/gemini review

[gemini-code-assist]

Code Review

This pull request effectively introduces support for GetEntitlement for registered resources. The changes are well-structured, with clear separation of concerns between the JustInTimePDP and the core PolicyDecisionPoint. The addition of new helper functions, modifications to existing ones, and the introduction of new test suites demonstrate a thorough approach to implementing this feature.

The new fetchAllRegisteredResources function correctly handles pagination, and the GetEntitlementsRegisteredResource method in the PDP provides the core logic for determining entitlements based on registered resource FQNs, including support for comprehensive hierarchy.

Unit tests are comprehensive, covering various scenarios such as invalid inputs, different combinations of action-attribute values, and hierarchy considerations. The addition of validation for registered resources (validateRegisteredResource and its tests) further enhances the robustness of the system.

Overall, the code quality is high, and the changes appear to be correct and well-implemented.

Summary of Findings

• Code Quality and Testing: The overall code quality is excellent. The new functionality is well-integrated, and the unit tests are comprehensive, covering a wide range of scenarios and edge cases. This significantly increases confidence in the correctness of the implementation.
• Clarity and Maintainability: The code is clear and maintainable. New functions and methods are well-named, and their purpose is evident. The reuse of existing logic, such as populateLowerValuesIfHierarchy, is good practice.
• Log Message Consistency (Minor): In service/internal/access/v2/just_in_time_pdp.go, line 158, the debug log message "getting decision - resolving registered resource value FQN" is within the GetEntitlements method. For consistency with other log messages related to entitlements (e.g., in pdp.go), it could be slightly more precise, like "getting entitlements - resolving registered resource value FQN". This is a minor point and was not formally commented on due to review settings prioritizing medium severity issues and above.

Merge Readiness

The pull request is in excellent shape and implements the new feature thoroughly and correctly. The code is clear, well-tested, and adheres to good practices. I have not identified any critical or high-severity issues. Based on this review, the PR appears ready for merging, pending any other review processes or integration testing requirements. As an AI, I am not authorized to approve pull requests, so please ensure it is reviewed and approved by the appropriate team members.

[github-actions]

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric	Value
Approved Decision Requests	5000
Denied Decision Requests	0
Total Time	474.859466ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric	Value
Error	internal: failed to create new policy decision point: invalid registered resource: registered resource values are empty: access: invalid registered resource
Total Time	50.584456ms

Bulk Benchmark Results

Metric	Value
Total Decrypts	100
Successful Decrypts	0
Failed Decrypts	100
Total Time	182.123816ms
Throughput	0.00 requests/second

Error Summary

Error Message	Occurrences
splitKey.unable to reconstruct split key: map[{https://localhost:8080 }:kao unwrap failed for split {https://localhost:8080 }: internal: internal error
rpc error: code = Internal desc = could not perform access]
kao unwrap failed for split {https://localhost:8080 }: internal: internal error
rpc error: code = Internal desc = could not perform access	100 occurrences

TDF3 Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	0
Failed Requests	5000
Concurrent Requests	50
Total Time	23.571283474s
Throughput	0.00 requests/second

Error Summary:

Error Message	Occurrences
read error: reader.WriteTo failed: splitKey.unable to reconstruct split key: map[{https://localhost:8080 }:kao unwrap failed for split {https://localhost:8080 }: internal: internal error rpc error: code = Internal desc = could not perform access] kao unwrap failed for split {https://localhost:8080 }: internal: internal error rpc error: code = Internal desc = could not perform access	5000 occurrences

NANOTDF Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	0
Failed Requests	5000
Concurrent Requests	50
Total Time	19.962398567s
Throughput	0.00 requests/second

Error Summary:

Error Message	Occurrences
ReadNanoTDF error: getNanoRewrapKey: rewrapError: internal: internal error rpc error: code = Internal desc = could not perform access	5000 occurrences

Standard Benchmark Metrics Skipped or Failed

[github-actions]

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric	Value
Approved Decision Requests	5000
Denied Decision Requests	0
Total Time	439.318712ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric	Value
Approved Decision Requests	5000
Denied Decision Requests	0
Total Time	294.85151ms

Bulk Benchmark Results

Metric	Value
Total Decrypts	100
Successful Decrypts	100
Failed Decrypts	0
Total Time	352.416278ms
Throughput	283.76 requests/second

TDF3 Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	5000
Failed Requests	0
Concurrent Requests	50
Total Time	53.492494981s
Average Latency	532.353925ms
Throughput	93.47 requests/second

NANOTDF Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	4933
Failed Requests	67
Concurrent Requests	50
Total Time	44.938573916s
Average Latency	445.702555ms
Throughput	109.77 requests/second

Error Summary:

Error Message	Occurrences
ReadNanoTDF error: getNanoRewrapKey: rewrapError: internal: internal error rpc error: code = Internal desc = could not perform access	67 occurrences

Standard Benchmark Metrics Skipped or Failed

[ryanulit]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces support for GetEntitlement for registered resources within the authorization service. The changes are quite comprehensive, touching upon the PDP (Policy Decision Point) core logic, JIT PDP, helpers, validators, and extensive test suites.

Overall, the implementation appears robust and well-thought-out. The new functionality is clearly delineated, and the integration into existing components seems logical. The test coverage for the new GetEntitlementsRegisteredResource method and associated validators is particularly commendable, covering a wide range of scenarios including invalid inputs, various data configurations, and hierarchy considerations.

I have one suggestion for a minor refinement in error handling for increased clarity, detailed in a specific comment.

Regarding the PR description:

• The checklist item "I have added or updated unit tests" should be checked, as significant unit tests have indeed been added and updated.
• The status of integration tests and documentation updates is unclear from the description. It would be beneficial to update these as well.

Summary of Findings

• Error Handling Clarity for Non-Existent Registered Resource FQN: In GetEntitlementsRegisteredResource, when a registeredResourceValueFQN is not found in the internal map, the current logic relies on validateRegisteredResourceValue(nil) to signal an error. While correct, a more explicit check for the FQN's existence could provide a clearer error message, distinguishing 'not found' from 'found but invalid'.

Merge Readiness

The pull request is largely in good shape and introduces valuable functionality with good test coverage. There is one medium-severity suggestion regarding error handling clarity in service/internal/access/v2/pdp.go that I recommend considering for improved maintainability. Additionally, updating the PR description checklist would be helpful.

I am not authorized to approve pull requests, so please ensure further review and approval from authorized maintainers before merging. Addressing the suggested refinement would be beneficial.

all comments resolved