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 PlainPermissionChecker.check confusion #6594

Closed
miles-ton opened this issue Apr 16, 2023 · 3 comments · Fixed by #6613
Closed

ACL PlainPermissionChecker.check confusion #6594

miles-ton opened this issue Apr 16, 2023 · 3 comments · Fixed by #6613
Assignees

Comments

@miles-ton
Copy link
Contributor

miles-ton commented Apr 16, 2023

image

  1. i found a very confused code segment in PlainPermissionChecker.check, dose the firs segment in frame above want to indicate that the admin have any permission and don't need verification?
    if that's the case, why still go through the verification when ownedPermMap isn't null?

  2. what does 'isGroup' do, is it the duplicated code for the second and third red frame?

@francisoliverlee
Copy link
Member

  1. agree with you, if admin, it could stop to do verification
  2. retry topic does not have any acl in files, if resource is a retry topic, it check the default group's acl

@miles-ton
Copy link
Contributor Author

  1. agree with you, if admin, it could stop to do verification
  2. retry topic does not have any acl in files, if resource is a retry topic, it check the default group's acl

thanks for your explanation, i will give out a PR to fix 1

@drpmma
Copy link
Contributor

drpmma commented Apr 18, 2023

Good catch! I will assign this issue to you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants