-
Notifications
You must be signed in to change notification settings - Fork 108
Allow principal resource slots in condition #1722
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Allow principal resource slots in condition #1722
Conversation
Signed-off-by: Alan Wang <[email protected]>
Signed-off-by: Alan Wang <[email protected]>
Coverage ReportHead Commit: Base Commit: Download the full coverage report. Coverage of Added or Modified Lines of Rust CodeRequired coverage: 80.00% Actual coverage: 83.70% Status: PASSED ✅ Details
Coverage of All Lines of Rust CodeRequired coverage: 80.00% Actual coverage: 81.57% Status: PASSED ✅ Details
|
Signed-off-by: Alan Wang <[email protected]>
Signed-off-by: Alan Wang <[email protected]>
Coverage ReportHead Commit: Base Commit: Download the full coverage report. Coverage of Added or Modified Lines of Rust CodeRequired coverage: 80.00% Actual coverage: 83.70% Status: PASSED ✅ Details
Coverage of All Lines of Rust CodeRequired coverage: 80.00% Actual coverage: 81.57% Status: PASSED ✅ Details
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
noticed this which probably needs to change
cedar/cedar-policy-core/src/est.rs
Lines 159 to 162 in b0e6683
| pub fn has_slot(&self) -> bool { | |
| // currently, slots are not allowed in clauses | |
| false | |
| } |
Signed-off-by: Alan Wang <[email protected]>
Thanks for pointing that out, just updated it. |
Coverage ReportHead Commit: Base Commit: Download the full coverage report. Coverage of Added or Modified Lines of Rust CodeRequired coverage: 80.00% Actual coverage: 63.91% Status: FAILED ❌ Details
Coverage of All Lines of Rust CodeRequired coverage: 80.00% Actual coverage: 81.49% Status: PASSED ✅ Details
|
Signed-off-by: Alan Wang <[email protected]>
Coverage ReportHead Commit: Base Commit: Download the full coverage report. Coverage of Added or Modified Lines of Rust CodeRequired coverage: 80.00% Actual coverage: 64.33% Status: FAILED ❌ Details
Coverage of All Lines of Rust CodeRequired coverage: 80.00% Actual coverage: 81.49% Status: PASSED ✅ Details
|
Signed-off-by: Alan Wang <[email protected]>
Coverage ReportHead Commit: Base Commit: Download the full coverage report. Coverage of Added or Modified Lines of Rust CodeRequired coverage: 80.00% Actual coverage: 44.71% Status: FAILED ❌ Details
Coverage of All Lines of Rust CodeRequired coverage: 80.00% Actual coverage: 81.28% Status: PASSED ✅ Details
|
Signed-off-by: Alan Wang <[email protected]>
Coverage ReportHead Commit: Base Commit: Download the full coverage report. Coverage of Added or Modified Lines of Rust CodeRequired coverage: 80.00% Actual coverage: 77.47% Status: FAILED ❌ Details
Coverage of All Lines of Rust CodeRequired coverage: 80.00% Actual coverage: 81.53% Status: PASSED ✅ Details
|
Description of changes
Removes the check that prevents
?principaland?resourceslots from appearing in the scope of the policy. Instead now these slots can be used in the condition of the template as long as they also appear in the scope. Since we are not longer going with having user defined slots in the scope of the template, the only way to allow using the slots in the scope in the condition of the template will be to use?principaland?resource.Issue #, if available
cedar-policy/rfcs#98
Checklist for requesting a review
The change in this PR is (choose one, and delete the other options):
cedar-policy(e.g., changes to the signature of an existing API).cedar-policy(e.g., addition of a new API).cedar-policy.cedar-policy-core,cedar-validator, etc.)I confirm that this PR (choose one, and delete the other options):
I confirm that
cedar-spec(choose one, and delete the other options):cedar-spec, and how you have tested that your updates are correct.)cedar-spec. (Post your PR anyways, and we'll discuss in the comments.)I confirm that
docs.cedarpolicy.com(choose one, and delete the other options):cedar-docs. PRs should be targeted at astaging-X.Ybranch, notmain.)