allowTopicOperationAsync should check the original role is super user (#1355) #7788
+598
−347
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
*Modifications* - We should use the original role to verify if it is allowed for a given topic operation - use the original authentication data - Authz provider doesn't have to be aware of proxyRole - Fix authorization test
codelipenghui
approved these changes
Aug 11, 2020
jiazhai
approved these changes
Aug 12, 2020
wolfstudy
pushed a commit
that referenced
this pull request
Aug 13, 2020
…#1355) (#7788) * Fix allowTopicOperationAsync logic (#1355) *Modifications* - We should use the original role to verify if it is allowed for a given topic operation - use the original authentication data - Authz provider doesn't have to be aware of proxyRole - Fix authorization test * Refactor authorize logic to provide a uniform authorization behavior (cherry picked from commit 48f5a2f)
huangdx0726
pushed a commit
to huangdx0726/pulsar
that referenced
this pull request
Aug 24, 2020
…apache#1355) (apache#7788) * Fix allowTopicOperationAsync logic (apache#1355) *Modifications* - We should use the original role to verify if it is allowed for a given topic operation - use the original authentication data - Authz provider doesn't have to be aware of proxyRole - Fix authorization test * Refactor authorize logic to provide a uniform authorization behavior
jerrypeng
pushed a commit
to jerrypeng/incubator-pulsar
that referenced
this pull request
Aug 24, 2020
…apache#1355) (apache#7788) * Fix allowTopicOperationAsync logic (apache#1355) *Modifications* - We should use the original role to verify if it is allowed for a given topic operation - use the original authentication data - Authz provider doesn't have to be aware of proxyRole - Fix authorization test * Refactor authorize logic to provide a uniform authorization behavior
lbenc135
pushed a commit
to lbenc135/pulsar
that referenced
this pull request
Sep 5, 2020
…apache#1355) (apache#7788) * Fix allowTopicOperationAsync logic (apache#1355) *Modifications* - We should use the original role to verify if it is allowed for a given topic operation - use the original authentication data - Authz provider doesn't have to be aware of proxyRole - Fix authorization test * Refactor authorize logic to provide a uniform authorization behavior
lbenc135
pushed a commit
to lbenc135/pulsar
that referenced
this pull request
Sep 5, 2020
…apache#1355) (apache#7788) * Fix allowTopicOperationAsync logic (apache#1355) *Modifications* - We should use the original role to verify if it is allowed for a given topic operation - use the original authentication data - Authz provider doesn't have to be aware of proxyRole - Fix authorization test * Refactor authorize logic to provide a uniform authorization behavior
lbenc135
pushed a commit
to lbenc135/pulsar
that referenced
this pull request
Sep 5, 2020
…apache#1355) (apache#7788) * Fix allowTopicOperationAsync logic (apache#1355) *Modifications* - We should use the original role to verify if it is allowed for a given topic operation - use the original authentication data - Authz provider doesn't have to be aware of proxyRole - Fix authorization test * Refactor authorize logic to provide a uniform authorization behavior
abhilashmandaliya
pushed a commit
to ashishshinde/pulsar
that referenced
this pull request
Nov 19, 2020
…apache#1355) (apache#7788) * Fix allowTopicOperationAsync logic (apache#1355) *Modifications* - We should use the original role to verify if it is allowed for a given topic operation - use the original authentication data - Authz provider doesn't have to be aware of proxyRole - Fix authorization test * Refactor authorize logic to provide a uniform authorization behavior (cherry picked from commit 48f5a2f)
2 tasks
15 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
In 2.6.0, allowTopicOperationAsync checks if topic operations are allowed for the original role. But it checks if the proxy role is a super-user by mistake. It should check if the original role is a super-user.
Modifications
Fix the
allowTopicOperationAsyncto checks if the original role is a super-user role or topic operations are allowed.The current authorization provider interface is also confused. Because some of the interfaces verify both the proxy role and original role and some don't. The authorization provider doesn't have to care about the proxy role and original role. Hence this pull request refactors the authorization provider to authorize a given role and move the logic of verifying proxy and original roles to AuthorizationService.