Skip to content

Commit

Permalink
[fix][broker] Only validate superuser access if authz enabled (#19989)
Browse files Browse the repository at this point in the history
In #19455, I added a requirement that only the proxy role could supply an original principal. That check is only supposed to apply when the broker has authorization enabled. However, in one case, that was not the case. This PR does a check and returns early when authorization is not enabled in the broker.

See #19830 (comment) for additional motivation.

* Update the `PulsarWebResource#validateSuperUserAccessAsync` to only validate when authentication and authorization are enabled in the configuration.

This is a trivial change. It'd be good to add tests, but I didn't include them here because this is a somewhat urgent fix. There was one test that broke because of this change, so there is at least some existing coverage.

- [x] `doc-not-needed`

PR in forked repository: michaeljmarshall#39

(cherry picked from commit 1a6c28d)
(cherry picked from commit 36f0db5)
  • Loading branch information
michaeljmarshall committed Apr 6, 2023
1 parent ca022bd commit 7337470
Showing 1 changed file with 2 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ protected boolean hasSuperUserAccess() {
* if not authorized
*/
public void validateSuperUserAccess() {
if (config().isAuthenticationEnabled()) {
if (config().isAuthenticationEnabled() && config().isAuthorizationEnabled()) {
String appId = clientAppId();
if (log.isDebugEnabled()) {
log.debug("[{}] Check super user access: Authenticated: {} -- Role: {}", uri.getRequestUri(),
Expand Down Expand Up @@ -218,7 +218,7 @@ public void validateSuperUserAccess() {
log.debug("Successfully authorized {} (proxied by {}) as super-user",
originalPrincipal, appId);
} else {
if (config().isAuthorizationEnabled() && !pulsar.getBrokerService()
if (!pulsar.getBrokerService()
.getAuthorizationService()
.isSuperUser(appId, clientAuthData())
.join()) {
Expand Down

0 comments on commit 7337470

Please sign in to comment.