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

Auth: Filter out identities, groups and, IdP groups that the requestor cannot view #13047

Merged

Conversation

markylaing
Copy link
Contributor

@markylaing markylaing commented Mar 5, 2024

This is an oversight from #12914.

  • When listing identities, the groups that the identities are a member of should be filtered by what the requestor is able to view.
  • When listing groups, the identities that are members and the identity provider groups that are mapped to the groups should be filtered by what the requestor is able to view.
  • When listing identity provider groups, the groups that are mapped to it should be filtered by what the requestor is able to view.

@markylaing markylaing self-assigned this Mar 5, 2024
@github-actions github-actions bot added Documentation Documentation needs updating API Changes to the REST API labels Mar 5, 2024
@markylaing markylaing force-pushed the filter-identities-groups-and-idp-groups branch from 56414e8 to e184165 Compare March 5, 2024 19:57
@tomponline
Copy link
Member

tomponline commented Mar 5, 2024

@markylaing does this have the fix to allow partial idp group match? Or is it in a different PR?

@markylaing
Copy link
Contributor Author

@markylaing does this have the fix to allow partial idp group match? Or is it in a different PR?

No, that will be in the embedded-openfga branch.

@markylaing markylaing force-pushed the filter-identities-groups-and-idp-groups branch 2 times, most recently from 905873a to 74b98f2 Compare March 6, 2024 15:40
Signed-off-by: Mark Laing <mark.laing@canonical.com>
Signed-off-by: Mark Laing <mark.laing@canonical.com>
Signed-off-by: Mark Laing <mark.laing@canonical.com>
Signed-off-by: Mark Laing <mark.laing@canonical.com>
Signed-off-by: Mark Laing <mark.laing@canonical.com>
Signed-off-by: Mark Laing <mark.laing@canonical.com>
Signed-off-by: Mark Laing <mark.laing@canonical.com>
Signed-off-by: Mark Laing <mark.laing@canonical.com>
@markylaing markylaing force-pushed the filter-identities-groups-and-idp-groups branch from 74b98f2 to f6e0545 Compare March 6, 2024 16:28
@markylaing markylaing marked this pull request as ready for review March 6, 2024 16:28
@github-actions github-actions bot removed Documentation Documentation needs updating API Changes to the REST API labels Mar 6, 2024
@markylaing
Copy link
Contributor Author

@tomponline this is now rebased and ready for review.

@@ -290,15 +292,17 @@ func (i Identity) Subject() (string, error) {
}

// ToAPI converts an Identity to an api.Identity, executing database queries as necessary.
func (i *Identity) ToAPI(ctx context.Context, tx *sql.Tx) (*api.Identity, error) {
func (i *Identity) ToAPI(ctx context.Context, tx *sql.Tx, canViewGroup auth.PermissionChecker) (*api.Identity, error) {
Copy link
Member

Choose a reason for hiding this comment

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

@markylaing the note for the forthcoming openga server, but can auth.PermissionChecker remain returning only a bool when each call to it could fail with an error (e.g. when consulting the DB)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The permission checker will still only return a boolean. Getting the permission checker can fail, but we're not getting one here, just passing one in.

Copy link
Member

Choose a reason for hiding this comment

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

ok cool - so this remains true even with openfga?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah the OpenFGA does a ListObjects to get the entities of a given type that the identity is related to via a given entitlement. When calling the permission checker, we just check if the given URL is in the list. Getting the list can fail but once it's in scope, checking if something is in the list is just a shared.ValueInSlice.

@tomponline tomponline merged commit 6dfcb00 into canonical:main Mar 6, 2024
27 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.

None yet

2 participants