cedar-policy / cedar-go

Apache License 2.0
62 stars 8 forks source link

Add Allowed() and Denied() methods to Decision alias type #5

Closed JoshuaWilkes closed 6 months ago

JoshuaWilkes commented 6 months ago

Adds Allowed() and Denied() methods to the Decision type to allow for less verbose handling of authorization results.

result, _ := policyset.IsAuthorized(entities, request)
if result.Allowed() {
    // do thing
}
philhassey commented 6 months ago

You can do this, which is even more terse:

ok, _ := policyset.IsAuthorized(entities, request)
if ok {
    // do thing
}
jmccarthy commented 6 months ago

Hi @JoshuaWilkes, thanks for the PR!

As @philhassey pointed out, the Decision struct is a bool, and should be used directly for boolean tests: https://github.com/cedar-policy/cedar-go/blob/470d1fe984bbea8af48ec8480c16f42711b81ed1/cedar.go#L141

And, as documented in the README: https://github.com/cedar-policy/cedar-go/blob/470d1fe984bbea8af48ec8480c16f42711b81ed1/README.md?plain=1#L97

@philhassey anticipating that others may have this question, I looked at the doc comment for IsAuthorized but couldn't really come up with a succinct way of clarifying the intended usage further, so I'm not proposing a change at this time.

JoshuaWilkes commented 6 months ago

Thanks for the review, I'm happy to use the suggested syntax instead, I'll close this PR