-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Fixed the invalid XACML Policy in documentation (Comparing to Other Systems) to be XACML 3.0 compliant #6438
Conversation
✅ Deploy Preview for openpolicyagent ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
Thanks! Yeah, it must have been many years since anyone touched this :) Just left a suggestion for some more modern Rego, but LGTM.
Should it not be added to the OPA policy? Or what makes it useless? |
Currently in the XACML Policy there is a check that the action should be |
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.
Thanks @cdanger! 😃
Signed-off-by: Cyril Dangerville <[email protected]>
…olicy to two spaces Signed-off-by: Cyril Dangerville <[email protected]>
…n as comparison to the XACML policy Signed-off-by: Cyril Dangerville <[email protected]>
a0ed1f9
to
21e194b
Compare
Why the changes in this PR are needed?
The XACML Policy given as example in the documentation page Comparing to other systems is not valid XML/XACML.
What are the changes in this PR?
This fixes several issues with the Policy:
xmlns=...
is missing), therefore XACML version undefined. Fixed to use the latest XACML 3.0 standard.Version
attribute on PolicyCategory
andMustBePresent
attributes on theAttributeDesignators
in theRule
Notes to assist PR review:
N/A
Further comments:
N/A