Skip to content
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

Move ImpossiblePolicy from error to warning #539

Closed
2 tasks
khieta opened this issue Dec 21, 2023 · 1 comment
Closed
2 tasks

Move ImpossiblePolicy from error to warning #539

khieta opened this issue Dec 21, 2023 · 1 comment
Assignees
Labels
4.0 feature-request Request for a new feature papercut

Comments

@khieta
Copy link
Contributor

khieta commented Dec 21, 2023

Category

Cedar validation features

Describe the feature you'd like to request

The validate function currently (v3.x) returns a ValidationResult, which consists of validation_errors and validation_warnings. One current validation error is the TypeError ImpossiblePolicy, which indicates that a policy will always evaluate to false (i.e., it will never fire). We had always intended for this to be more of a "warning" than an "error," but we didn’t add official support for warnings until recently (see #225). We would now like to move this error to be a warning instead.

Benefits

  • Since the ImpossiblePolicy error is returned on a best-effort basis (i.e., we may miss some "impossible" policies), it is sensitive to changes in our typing precision. This means that seemingly small changes to our typing algorithm could easily become breaking changes. For example: say that we decide to encode in the validator that an expression like 1 == 2 is always false (this is currently not checked). Then policies that previously validated will no longer be valid.
  • Although having a policy that never applies is likely an error, it is not an issue that would lead to an authorization-time error, which is what the validator is intended to protect against. So this "feels" more like a warning than an error.
  • There could be legitimate reasons for generating policies that never apply. For example, a user might be generating policies programmatically, expecting that "impossible" policies are ignored.
  • This would set the precedent for adding more warnings to the validator. Easy example: check whether a policy always evaluates to true (i.e., will always fire).

Impact

Some policies that previously would not validate will now validate, but with a warning. All previously-validated policies will remain valid.

Describe alternatives you've considered

We could leave ImpossiblePolicy as an error, which is the status quo.

Additional context

No response

Is this something that you'd be interested in working on?

  • 👋 I may be able to implement this feature request
  • ⚠️ This feature might incur a breaking change
@khieta khieta added backlog We hope to work on this in the future feature-request Request for a new feature labels Dec 21, 2023
@sarahcec sarahcec added 3.1 Features for 3.1 papercut labels Jan 2, 2024
@mstalzer mstalzer added work-in-progress Implementation work in progress and removed backlog We hope to work on this in the future 3.1 Features for 3.1 labels Feb 2, 2024
@khieta khieta self-assigned this Feb 2, 2024
@sarahcec sarahcec added the 3.2 Features for 3.2 label Feb 20, 2024
@shaobo-he-aws
Copy link
Contributor

Implemented via #716

@shaobo-he-aws shaobo-he-aws added 4.0 and removed work-in-progress Implementation work in progress 3.2 Features for 3.2 labels Mar 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4.0 feature-request Request for a new feature papercut
Projects
Status: Done
Development

No branches or pull requests

4 participants