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: demonstrate new authz interface #12169

Closed
wants to merge 1 commit into from
Closed

Conversation

dnephin
Copy link
Contributor

@dnephin dnephin commented Jan 23, 2022

Branched from #12167, demonstrates the authorization interface proposed in #11690.

This PR also demonstrates how we can used a typed error, built in a centralized place, to provide better error messages when permission is denied.

TODO:

  • remove Cause field from PermissionDeniedError
  • move ACLResolveResult and the types used in its method signatures to a new package under acl/ maybe acl/aclauthz or something like that.

@dnephin dnephin added the pr/no-changelog PR does not need a corresponding .changelog entry label Jan 23, 2022
@github-actions github-actions bot added the theme/acls ACL and token generation label Jan 23, 2022
@dnephin dnephin added theme/acls ACL and token generation theme/internal-cleanup Used to identify tech debt, testing improvements, code refactoring, and non-impactful optimization labels Jan 23, 2022
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging January 26, 2022 22:45 Inactive
@vercel vercel bot temporarily deployed to Preview – consul January 26, 2022 22:45 Inactive
@@ -1132,6 +1133,27 @@ func (a ACLResolveResult) AccessorID() string {
return a.ACLIdentity.ID()
}

type ACLKVResourceID interface {
KVResourceID() string
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if we want a separate interface for each resource type, or a small number of interfaces (one for resources with identifiers+entMeta, one for just identifiers, and one for just entMeta).

@dnephin dnephin force-pushed the dnephin/acl-resolve-token-3 branch 2 times, most recently from e4d0b9f to 343b6de Compare January 31, 2022 23:04
Base automatically changed from dnephin/acl-resolve-token-3 to main February 1, 2022 00:21
@vercel vercel bot temporarily deployed to Preview – consul February 4, 2022 17:41 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging February 4, 2022 17:41 Inactive
@markan markan self-requested a review February 5, 2022 01:29
@hashicorp-cla
Copy link

hashicorp-cla commented Mar 12, 2022

CLA assistant check
All committers have signed the CLA.

@github-actions
Copy link

This pull request has been automatically flagged for inactivity because it has not been acted upon in the last 60 days. It will be closed if no new activity occurs in the next 30 days. Please feel free to re-open to resurrect the change if you feel this has happened by mistake. Thank you for your contributions.

@github-actions github-actions bot added the meta/stale Automatically flagged for inactivity by stalebot label Jun 16, 2022
@github-actions
Copy link

Closing due to inactivity. If you feel this was a mistake or you wish to re-open at any time in the future, please leave a comment and it will be re-surfaced for the maintainers to review.

@github-actions github-actions bot closed this Aug 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta/stale Automatically flagged for inactivity by stalebot pr/no-changelog PR does not need a corresponding .changelog entry theme/acls ACL and token generation theme/internal-cleanup Used to identify tech debt, testing improvements, code refactoring, and non-impactful optimization
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants