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.

Sign up for GitHub

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

Jump to bottom

Auth: Add GET /1.0/auth/identities/current. #13045

Merged
merged 21 commits into from Mar 6, 2024
Merged

Auth: Add GET /1.0/auth/identities/current. #13045

merged 21 commits into from Mar 6, 2024

Conversation

markylaing
Copy link
Contributor

@markylaing markylaing commented Mar 5, 2024 •
edited by tomponline

This PR:

  • Changes the Identities field of the Group API type to a map[string][]string (authentication method to list of identifiers).
  • Changes the Identity type to include IdentityPut (the list of group names).
  • Removes recursion=2 from GET /1.0/identities and always returns the groups for each identity.
  • Changes IdentityInfo to include Identity and two new fields: EffectiveGroups and EffectivePermissions.
  • Implements a new endpoint GET /1.0/auth/identities/current that resolves all of the requestors groups and permissions in the context of any group mappings that may have been set by the IdP.
  • Updates the client for the above API changes.
  • Adds a new command lxc auth identity info [remote:] to show permissions for the current user.

See #12976 (comment) for more details.
#13042 should be merged first.

@markylaing markylaing added Feature New feature, not a bug Documentation Documentation needs updating API Changes to the REST API labels Mar 5, 2024
@markylaing markylaing self-assigned this Mar 5, 2024
@markylaing markylaing marked this pull request as draft March 5, 2024 18:26
@markylaing markylaing mentioned this pull request Mar 5, 2024
Merged
3 tasks
@tomponline
Copy link
Member

tomponline commented Mar 5, 2024 •
edited

  • lxc auth identity show-current [remote:]

What about "lxc auth identity info" ?

@markylaing
Copy link
Contributor Author

  • lxc auth identity show-current [remote:]

What about "lxc auth identity info" ?

Yeah that's better I'll change it tomorrow.

@markylaing markylaing marked this pull request as ready for review March 6, 2024 09:51
@markylaing
Copy link
Contributor Author

  • lxc auth identity show-current [remote:]

What about "lxc auth identity info" ?

Yeah that's better I'll change it tomorrow.

Changed to lxc auth identity info now.

tomponline
tomponline reviewed Mar 6, 2024
View reviewed changes
lxd/identities.go Outdated Show resolved Hide resolved
tomponline
tomponline reviewed Mar 6, 2024
View reviewed changes
lxd/identities.go Outdated Show resolved Hide resolved
tomponline
tomponline requested changes Mar 6, 2024
View reviewed changes
Copy link
Member

@tomponline tomponline left a comment

Choose a reason for hiding this comment

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

Thanks!

Lets move the queries out of the API handler functions.
And can you add a test for lxc auth identity info?

Also I updated the PR description to reflect the new command.

@markylaing
Copy link
Contributor Author

Thanks!

Lets move the queries out of the API handler functions. And can you add a test for lxc auth identity info?

Also I updated the PR description to reflect the new command.

I can add a test that calls it fairly easily, checking that the effective permissions and groups are correct is a bit more involved as we'll have to update mini-oidc to return a custom claim.

@tomponline tomponline mentioned this pull request Mar 6, 2024
Merged
@tomponline
Copy link
Member

I can add a test that calls it fairly easily, checking that the effective permissions and groups are correct is a bit more involved as we'll have to update mini-oidc to return a custom claim.

Lets just check the endpoint works for now.

@markylaing
shared/api: Update identities field of AuthGroup.
ba87566 
Changes the field to a map[string][]string to make the
API responses less circular.

Signed-off-by: Mark Laing <mark.laing@canonical.com>
@markylaing
shared/api: Include groups in the identity API type.
8ce0797 
Signed-off-by: Mark Laing <mark.laing@canonical.com>
@markylaing
shared/api: Add effective groups and permissions to IdentityInfo.
6dab04f 
Signed-off-by: Mark Laing <mark.laing@canonical.com>
@markylaing
lxd/db/cluster: Update AuthGroup.ToAPI to return identity map.
e4d2f3d 
Signed-off-by: Mark Laing <mark.laing@canonical.com>
@markylaing
lxd/db/cluster: Update Identity.ToAPIInfo method.
f90b4f2 
Signed-off-by: Mark Laing <mark.laing@canonical.com>
@markylaing
lxd: Return identity map when listing groups.
c5b2e80 
Signed-off-by: Mark Laing <mark.laing@canonical.com>
@markylaing
lxd: Remove recursion=2 from GET /1.0/identities.
a5150f9 
Signed-off-by: Mark Laing <mark.laing@canonical.com>
@markylaing
lxd: Update calls to Identity.ToAPI.
e210d08 
Signed-off-by: Mark Laing <mark.laing@canonical.com>
@markylaing
lxd/db/cluster: Don't return dangling permissions (log a warning).
d318ad0 
Signed-off-by: Mark Laing <mark.laing@canonical.com>
@markylaing
lxd: Update call to GetPermissionEntityURLs.
ea82ac9 
Signed-off-by: Mark Laing <mark.laing@canonical.com>
@markylaing
lxd/db/cluster: Add function to get all LXD groups from IDP groups.
32964a3 
Signed-off-by: Mark Laing <mark.laing@canonical.com>
@markylaing
lxd/db/cluster: Add function to get distinct permissions from list of…
6e684fa 
… group names.

Signed-off-by: Mark Laing <mark.laing@canonical.com>
@markylaing
lxd: Adds handler for getting the current identity.
81367d7 
Signed-off-by: Mark Laing <mark.laing@canonical.com>
@markylaing
lxd: Call current identity handler on GET /1.0/auth/identities/current.
596a8af 
Signed-off-by: Mark Laing <mark.laing@canonical.com>
@markylaing
client: Update client for modified identities endpoints.
23265f1 
Signed-off-by: Mark Laing <mark.laing@canonical.com>
@markylaing
client: Update client for modified identities endpoints.
d0c1c83 
Signed-off-by: Mark Laing <mark.laing@canonical.com>
@markylaing
lxc: Update client call when listing identities.
edd855d 
Signed-off-by: Mark Laing <mark.laing@canonical.com>
@markylaing
lxc: Add 'info' command to lxc auth identity.
857bc15 
Signed-off-by: Mark Laing <mark.laing@canonical.com>
@markylaing
test/suites: Test the 'lxc auth identity info' command.
e8cc7df 
Signed-off-by: Mark Laing <mark.laing@canonical.com>
@markylaing
i18n: Update translations.
45a6bfb 
Signed-off-by: Mark Laing <mark.laing@canonical.com>
@markylaing
doc: Runs make update-api.
6a622a5 
Signed-off-by: Mark Laing <mark.laing@canonical.com>
@markylaing
Copy link
Contributor Author

I've addressed the feedback when you're ready @tomponline.

1 similar comment
@markylaing
Copy link
Contributor Author

I've addressed the feedback when you're ready @tomponline.

@tomponline tomponline merged commit f48a0c7 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
Reviewers

@tomponline tomponline tomponline approved these changes

@roosterfish roosterfish Awaiting requested review from roosterfish

Assignees

@markylaing markylaing

Labels
API Changes to the REST API Documentation Documentation needs updating Feature New feature, not a bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@markylaing @tomponline