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

client: fix role permission issue with duplicate policies. #18419

Merged
merged 2 commits into from
Sep 11, 2023
Merged

Conversation

jrasell
Copy link
Member

@jrasell jrasell commented Sep 7, 2023

This change deduplicates the ACL policy list generated from ACL roles referenced within an ACL token on the client.

Previously the list could contain duplicates, which would cause erronous permission denied errors when calling client related RPC/ HTTP API endpoints. This is because the client calls the ACL get policies endpoint which subsequently ensures the caller has permission to view the ACL policies. This check is performed by comparing the requested list args with the policies referenced by the caller ACL token. When a duplicate is present, this check fails, as the check must ensure the slices match exactly.

The linked issue has details on how to reproduce the problem, if readers want to test this before and after the code changes.

Closes #17201

This change deduplicates the ACL policy list generated from ACL
roles referenced within an ACL token on the client.

Previously the list could contain duplicates, which would cause
erronous permission denied errors when calling client related RPC/
HTTP API endpoints. This is because the client calls the ACL get
policies endpoint which subsequently ensures the caller has
permission to view the ACL policies. This check is performed by
comparing the requested list args with the policies referenced by
the caller ACL token. When a duplicate is present, this check
fails, as the check must ensure the slices match exactly.
@jrasell jrasell added backport/1.4.x backport to 1.4.x release line backport/1.5.x backport to 1.5.x release line backport/1.6.x backport to 1.6.x release line labels Sep 7, 2023
@jrasell jrasell self-assigned this Sep 7, 2023
@jrasell jrasell marked this pull request as ready for review September 7, 2023 09:41
Copy link
Member

@gulducat gulducat left a comment

Choose a reason for hiding this comment

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

LGTM!

client/acl_test.go Show resolved Hide resolved
client/acl_test.go Show resolved Hide resolved
@jrasell jrasell merged commit 668dc5f into main Sep 11, 2023
25 checks passed
@jrasell jrasell deleted the gh-17201 branch September 11, 2023 11:52
jrasell added a commit that referenced this pull request Sep 11, 2023
This change deduplicates the ACL policy list generated from ACL
roles referenced within an ACL token on the client.

Previously the list could contain duplicates, which would cause
erronous permission denied errors when calling client related RPC/
HTTP API endpoints. This is because the client calls the ACL get
policies endpoint which subsequently ensures the caller has
permission to view the ACL policies. This check is performed by
comparing the requested list args with the policies referenced by
the caller ACL token. When a duplicate is present, this check
fails, as the check must ensure the slices match exactly.
jrasell added a commit that referenced this pull request Sep 11, 2023
…18446)

This change deduplicates the ACL policy list generated from ACL
roles referenced within an ACL token on the client.

Previously the list could contain duplicates, which would cause
erronous permission denied errors when calling client related RPC/
HTTP API endpoints. This is because the client calls the ACL get
policies endpoint which subsequently ensures the caller has
permission to view the ACL policies. This check is performed by
comparing the requested list args with the policies referenced by
the caller ACL token. When a duplicate is present, this check
fails, as the check must ensure the slices match exactly.

Co-authored-by: James Rasell <jrasell@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.4.x backport to 1.4.x release line backport/1.5.x backport to 1.5.x release line backport/1.6.x backport to 1.6.x release line
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Nomad ACL roles with duplicated policies
2 participants