Skip to content

Add in_group and has_attribute policy functions #2

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

Merged
merged 4 commits into from
Nov 21, 2023

Conversation

Koshroy-Abbey
Copy link
Contributor

This PR adds two policy functions.

  1. in_group checks whether an Abbey user's identity is within a group

  2. has_attribute checks whether an Abbey user has a custom attribute with a given value

This PR adds two policy functions.

1. in_group checks whether an Abbey user's identity is within a group

2. has_attribute checks whether an Abbey user has a custom attribute with
a given value
Copy link
Contributor

@jeffchao jeffchao left a comment

Choose a reason for hiding this comment

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

Requesting 3 changes. 2 are in review comments. Third is to add tests. See examples from prior PR: #1.

import future.keywords.in

in_group(group_name) := true if {
some group in data.system.abbey.group_memberships
Copy link
Contributor

Choose a reason for hiding this comment

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

@Koshroy-Abbey why the extra variable instead of group_name in data.system.abbey.group_memberships?


import future.keywords.if

has_attribute(name, value) := true if {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add code comment to document what this function is supposed to be doing. Same for other functions.

Copy link
Contributor

@jeffchao jeffchao left a comment

Choose a reason for hiding this comment

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

LGTM.

Some nits, but looks good. Also, as a separate PR, would be good to add in GH Actions for CI check on the tests. https://www.openpolicyagent.org/docs/latest/policy-testing/


import future.keywords.if

# Function which checks whether a user has a given attribute
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Add a period to show this is a complete thought and the new sentence (on the next line) begins.

Copy link
Contributor

Choose a reason for hiding this comment

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

Same nits for other code comments.

@Koshroy-Abbey Koshroy-Abbey merged commit b5c7625 into main Nov 21, 2023
@songe songe deleted the rbac-abac-policies branch November 21, 2023 04:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants