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

Moving to structured JSON policy decisions. #1718

Merged
merged 1 commit into from
Apr 10, 2023

Conversation

matajoh
Copy link
Member

@matajoh matajoh commented Apr 5, 2023

This PR makes two major changes:

  1. All policy enforcement points now receive a context objects and use it to log policy errors and denial decisions.
  2. Policy denials are now conveyed as structured JSON objects.

Whereas previously policy denial was surfaced as a text error message, the policy now generates a bracketed base64 encoded string:

policydecision< (base64) >policydecision

When decoded, this will be a JSON object with the following structure:

{
  "input": {/*redacted input object*/},
  "decision": "deny",
  "reason": {/*reason object explaining decision*/},
  "policyError": "optional error field"
}

NB: the "policyError" field above is only present if the denial was triggered by an actual error in the Rego policy.

This commit makes two major changes:

1. All policy enforcement points now receive a context objects and use it
   to log policy errors and denial decisions.
2. Policy denials are now conveyed as structured JSON objects.

Whereas previously policy denial was surfaced as a text error message,
the policy now generates a bracketed base64 encoded string:

policydecision< (base64) >policydecision

When decoded, this will be a JSON object with the following structure:

```json
{
  "input": <redacted input object>,
  "decision": "deny",
  "reason": <reason object explaining decision>,
  "policyError": <optional error field>
}
```

NB: the `"policyError"` field above is only present if the denial was
triggered by an actual error in the Rego policy.

Signed-off-by: Matthew A Johnson <matjoh@microsoft.com>
@matajoh matajoh requested a review from a team as a code owner April 5, 2023 12:45
@anmaxvl anmaxvl merged commit 8f5f651 into microsoft:main Apr 10, 2023
if len(rawResult) == 0 {
return result, nil
return nil, errors.New("emtpy result from Rego query")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any way to get context around why the rego query is empty or what to do about it?


errorBytes, err := base64.RawURLEncoding.DecodeString(matches[1])
if err != nil {
return "", err
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to also return the policy decision error so that action can be taken despite the fact that it's not properly base64 encoded?

}

if versionMissing {
return nil, errors.New(noAPIVersionError)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than using errors.New here, if you make the error types exported and define the global variables with errors.New above, it would be possible in the future to use errors.Is to take different actions based on different issues and decouple from the exact wording in the error message.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants