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

ACL: add ACL role functionality to ACL tokens #14110

Merged
merged 3 commits into from
Aug 17, 2022

Conversation

jrasell
Copy link
Member

@jrasell jrasell commented Aug 15, 2022

ACL tokens can now utilize ACL roles in order to provide API
authorization. Each ACL token can be created and linked to an
array of policies as well as an array of ACL role links. The link
can be provided via the role name or ID, but internally, is always
resolved to the ID as this is immutable whereas the name can be
changed by operators.

When resolving an ACL token, the policies linked from an ACL role
are unpacked and combined with the policy array to form the
complete auth set for the token.

The ACL token creation endpoint handles deduplicating ACL role
links as well as ensuring they exist within state.

When reading a token, Nomad will also ensure the ACL role link is
current. This handles ACL roles being deleted from under a token
from a UX standpoint.

The PR also includes adding the ability to create ACL tokens via
the CLI which link to ACL roles.

related: #13120
targets: feature branch

Copy link
Member

@shoenig shoenig left a comment

Choose a reason for hiding this comment

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

LGTM! Just some code golf suggestions, in case you are overwhelmed with free time

command/acl_token_create.go Outdated Show resolved Hide resolved
nomad/acl_endpoint.go Show resolved Hide resolved
nomad/state/state_store_acl.go Outdated Show resolved Hide resolved
Comment on lines +254 to +257
// Track whether we have made an initial copy to ensure we are not
// operating on the token directly from state.
copied := false
Copy link
Member

Choose a reason for hiding this comment

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

Just an idea - this would probably be easier to read if we always created the duplicate, then compare hashes at then end to decide if we should return the duplicate or the original

ACL tokens can now utilize ACL roles in order to provide API
authorization. Each ACL token can be created and linked to an
array of policies as well as an array of ACL role links. The link
can be provided via the role name or ID, but internally, is always
resolved to the ID as this is immutable whereas the name can be
changed by operators.

When resolving an ACL token, the policies linked from an ACL role
are unpacked and combined with the policy array to form the
complete auth set for the token.

The ACL token creation endpoint handles deduplicating ACL role
links as well as ensuring they exist within state.

When reading a token, Nomad will also ensure the ACL role link is
current. This handles ACL roles being deleted from under a token
from a UX standpoint.
@jrasell jrasell force-pushed the f-gh-13120-acl-role-token-support branch from a8a965d to 4b9e2fc Compare August 17, 2022 13:46
@jrasell jrasell force-pushed the f-gh-13120-acl-role-token-support branch from 4b9e2fc to 9953159 Compare August 17, 2022 13:50
@jrasell jrasell merged commit 37a4c32 into f-gh-13120-sso-umbrella Aug 17, 2022
@jrasell jrasell deleted the f-gh-13120-acl-role-token-support branch August 17, 2022 15:14
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants