Skip to content

Add support for generic policy iteration when authorizing a request#85

Merged
patjakdev merged 14 commits intocedar-policy:mainfrom
strongdm:add-support-for-generic-authorize
Apr 22, 2025
Merged

Add support for generic policy iteration when authorizing a request#85
patjakdev merged 14 commits intocedar-policy:mainfrom
strongdm:add-support-for-generic-authorize

Conversation

@patjakdev
Copy link
Copy Markdown
Collaborator

@patjakdev patjakdev commented Apr 17, 2025

Issue #, if available:

Description of changes: Adds a new top-level Authorize() function which accepts a generic iterator of policies.

This functionality supersedes the existing PolicySet.IsAuthorized() method (which is now implemented in terms of Authorize()) and allows for more exotic policy injection a la the EntityGetter interface. This means that policy can be lazily loaded from an external source and passed to the Authorize() function via the iterator or policy sets can be combined for authorization without having to Clone() them into a new PolicySet.

Authorize() can only be called from packages built using Go 1.23 or later.

Also, several other types have gotten some iterator love:

  • types.EntityUIDSet gets an All() method that returns an iterator over the entity UIDs
  • types.Set gets an All() method that returns an iterator over the Values in the set
  • types.Record gets three new methods that return iterators: All(), Keys(), and Values() which behave similarly to maps.All(), maps.Keys(), and maps.Values().
  • types.PolicySet gets an All() method that returns an iterator over the PolicyIDs and *Policys in the PolicySet.

@patjakdev patjakdev force-pushed the add-support-for-generic-authorize branch from a03f59f to 1845891 Compare April 17, 2025 23:26
This function is identical to PolicySet.IsAuthorized(), but allows a generic iterator of (PolicyID, *Policy) tuples to be passed. This allows for flexibility at the callsite if the caller wants to use some other data structure to hold on to a map of PolicyIDs to *Policy.

Note: an All() method is also added to PolicySet which returns an iterator over the internal policy map.

Signed-off-by: Patrick Jakubowski <patrick.jakubowski@strongdm.com>
…dule.

Signed-off-by: Patrick Jakubowski <patrick.jakubowski@strongdm.com>
@patjakdev patjakdev force-pushed the add-support-for-generic-authorize branch from 1845891 to 4b1e2a3 Compare April 18, 2025 18:27
Signed-off-by: Patrick Jakubowski <patrick.jakubowski@strongdm.com>
@patjakdev patjakdev force-pushed the add-support-for-generic-authorize branch from b5b1822 to adc25f6 Compare April 18, 2025 18:39
…uthorized()

Signed-off-by: Patrick Jakubowski <patrick.jakubowski@strongdm.com>
Signed-off-by: Patrick Jakubowski <patrick.jakubowski@strongdm.com>
Signed-off-by: Patrick Jakubowski <patrick.jakubowski@strongdm.com>
# For more information see: https://docs.github.com/en/actions/automating-builds-and-tests/building-and-testing-go

name: Go
name: Verify
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I combined build_and_test.yml and golangci-lint.yml (we weren't getting much value out of the parallelism anyway) and switched to using go-version-file so that we only have to update the Go version in one place.

internalast.Permit().When(internalast.Long(42).ContainsAny(internalast.Long(43))),
},
{
"opContainsIsEmpty",
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This is an unrelated change that I made in order to get test coverage back up to 100%

Conditions: []ast.ConditionType{{Condition: ast.ConditionWhen, Body: ast.NodeTypeHasTag{BinaryNode: ast.BinaryNode{Left: ast.NodeValue{Value: types.Long(42)}, Right: ast.NodeValue{Value: types.String("key")}}}}}},
},
{
"opIsEmpty",
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

More test coverage improvements

NodeTypeExtensionCall{}.isNode()
NodeTypeIfThenElse{}.isNode()
NodeTypeLike{}.isNode()
NodeTypeIsEmpty{}.isNode()
Copy link
Copy Markdown
Collaborator Author

@patjakdev patjakdev Apr 18, 2025

Choose a reason for hiding this comment

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

More unrelated test improvements.


p := ast.Permit().PrincipalEq(types.NewEntityUID("Foo::Bar", "Baz"))
expected := `{
t.Run("roundtrip", func(t *testing.T) {
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

More unrelated changes to get test coverage to 100%

@patjakdev patjakdev marked this pull request as ready for review April 18, 2025 20:57
@patjakdev patjakdev changed the title Add support for generic authorize Add support for generic policy iteration when authorizing a request Apr 18, 2025
…rator as well

Signed-off-by: Patrick Jakubowski <patrick.jakubowski@strongdm.com>
Copy link
Copy Markdown
Contributor

@philhassey philhassey left a comment

Choose a reason for hiding this comment

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

Let's chat a bit about my comments and see what changes we want. Overall looks good!

Also, add some dummy statements to otherwise empty functions so that they're counted for code coverage purposes.

Signed-off-by: Patrick Jakubowski <patrick.jakubowski@strongdm.com>
@patjakdev patjakdev force-pushed the add-support-for-generic-authorize branch from 33e4a70 to 2f5728b Compare April 21, 2025 19:52
… method rather than a bare iter.Seq2

Signed-off-by: Patrick Jakubowski <patrick.jakubowski@strongdm.com>
… Authorize()

Signed-off-by: Patrick Jakubowski <patrick.jakubowski@strongdm.com>
Signed-off-by: Patrick Jakubowski <patrick.jakubowski@strongdm.com>
@patjakdev patjakdev force-pushed the add-support-for-generic-authorize branch 2 times, most recently from 371baa0 to 3bd0436 Compare April 21, 2025 23:34
Signed-off-by: Patrick Jakubowski <patrick.jakubowski@strongdm.com>
@patjakdev patjakdev force-pushed the add-support-for-generic-authorize branch from 3bd0436 to 680ff11 Compare April 21, 2025 23:38
…rary

Signed-off-by: Patrick Jakubowski <patrick.jakubowski@strongdm.com>
…n of golang.org/x/exp that introduces constraints

Signed-off-by: Patrick Jakubowski <patrick.jakubowski@strongdm.com>
@patjakdev patjakdev force-pushed the add-support-for-generic-authorize branch from 680ff11 to f06dc9d Compare April 21, 2025 23:39
Copy link
Copy Markdown
Contributor

@philhassey philhassey left a comment

Choose a reason for hiding this comment

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

LGTM, with allowance for some brief bike-shedding

@patjakdev patjakdev merged commit 213bd2f into cedar-policy:main Apr 22, 2025
2 checks passed
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.

2 participants