Skip to content

[#13750] Allow policies to control sub-fields of requests #13769

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

maneac
Copy link

@maneac maneac commented Jan 24, 2022

Summary
Introduce a dot-delimited ('.') syntax to policy parameters to allow the application of parameter restrictions to the contents of request maps. These can be applied to allowed, denied, and required parameters.

Description
The unit tests for the ACL define this behaviour in more detail, but for an overview:

  • Arbitrary nesting depths are supported
    • e.g. map.top.middle.bottom
  • Globs for the final section are supported
    • e.g. map.top.* will match map.top.foo and map.top.bar.baz.
  • Denies override allows
    • e.g. a denied parameter of map.* will overrule an allow for map.foo.
  • Behaviour of the values for nested parameters match that of "flat" parameters
    • e.g. map.foo = ["bar", "baz*"] will allow bar, baz, bazza, ...

The policy parser contains validation for the new nested parameter format.

Approach
Nested parameter values was considered as an approach. However, using a flat, delimited key format over nested values provides more flexibility when defining constraints. Their behaviour also proved more intuitive, in particular when combining denied_parameters with allowed_parameters.

Drawbacks
Any existing, "flat" parameter in a policy which contains a '.' will no longer be valid. As far as I can tell, these do not exist.

Resolves #13750.

Introduce the dot ('.') syntax for policy parameters to
restrict the contents of maps.
@hashicorp-cla
Copy link

hashicorp-cla commented Jan 24, 2022

CLA assistant check
All committers have signed the CLA.

@vercel vercel bot temporarily deployed to Preview – vault-storybook January 24, 2022 20:29 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook January 24, 2022 20:35 Inactive
Copy link
Contributor

@HridoyRoy HridoyRoy left a comment

Choose a reason for hiding this comment

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

Hi @maneac , thanks for the contribution! The code changes in the PR looks good to me, but I'd like to get another engineer's review of it as well ( cc @hghaf099 ).

I especially would like to think through some cases to make sure these changes are backward compatible.

Thanks!

@@ -0,0 +1,3 @@
```release-note:feature
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we change this to improvement?

@HridoyRoy HridoyRoy self-assigned this Jan 31, 2022
@vercel vercel bot temporarily deployed to Preview – vault-storybook February 2, 2022 18:01 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow policies to control sub-fields of requests
4 participants