yuruguo commented on a change in pull request #13773:
URL: https://github.com/apache/pulsar/pull/13773#discussion_r789300611
##########
File path:
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/PulsarAuthorizationProvider.java
##########
@@ -228,6 +228,40 @@ public void initialize(ServiceConfiguration conf,
PulsarResources pulsarResource
return allowTheSpecifiedActionOpsAsync(namespaceName, role,
authenticationData, AuthAction.sinks);
}
+ private CompletableFuture<Boolean>
allowConsumeOrProduceOpsAsync(NamespaceName namespaceName,
+ String
role,
+
AuthenticationDataSource authenticationData) {
+ CompletableFuture<Boolean> finalResult = new CompletableFuture<>();
+ allowTheSpecifiedActionOpsAsync(namespaceName, role,
authenticationData, AuthAction.produce)
+ .whenComplete((produceAuthorized, e) -> {
+ if (e == null) {
+ if (produceAuthorized) {
+ finalResult.complete(produceAuthorized);
+ return;
+ }
+ } else {
+ if (log.isDebugEnabled()) {
Review comment:
> we cannot let pass every exception
If there is an exception or error when checking the first action and we end
the subsequent checks, so even if the user has the second permission and it
will not take effect. Does this not meet expectations?
> we should fall back to checking the second action only in case of missing
authentication
I traced the implementation of `allowTheSpecifiedActionOpsAsync` and not
found exceptions related to `authentication`, so I can't tell whether to
perform the second check.
In addition, the implementation logic of this PR is similar
`canLookupAsync`, as follows:
https://github.com/apache/pulsar/blob/d972990d251a2c8467e68a38768b9a2d516e6be2/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/PulsarAuthorizationProvider.java#L179-L211
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
