michaeljmarshall commented on code in PR #19455:
URL: https://github.com/apache/pulsar/pull/19455#discussion_r1104784653
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/web/PulsarWebResource.java:
##########
@@ -150,19 +150,11 @@ public static boolean isClientAuthenticated(String appId)
{
return appId != null;
}
- private static void validateOriginalPrincipal(Set<String> proxyRoles,
String authenticatedPrincipal,
- String originalPrincipal) {
- if (proxyRoles.contains(authenticatedPrincipal)) {
- // Request has come from a proxy
- if (StringUtils.isBlank(originalPrincipal)) {
- log.warn("Original principal empty in request authenticated as
{}", authenticatedPrincipal);
- throw new RestException(Status.UNAUTHORIZED,
- "Original principal cannot be empty if the request is
via proxy.");
- }
- if (proxyRoles.contains(originalPrincipal)) {
- log.warn("Original principal {} cannot be a proxy role ({})",
originalPrincipal, proxyRoles);
- throw new RestException(Status.UNAUTHORIZED, "Original
principal cannot be a proxy role");
- }
+ private void validateOriginalPrincipal(String authenticatedPrincipal,
String originalPrincipal) {
+ if (!pulsar.getBrokerService().getAuthorizationService()
+ .isValidOriginalPrincipal(authenticatedPrincipal,
originalPrincipal, clientAuthData())) {
+ throw new RestException(Status.UNAUTHORIZED,
Review Comment:
The current code in `AuthorizationService` throws `Status.UNAUTHORIZED`.
Because I don't want to throw a rest exception when `ServerCnx` calls that
method, I moved the exception here. I think we should maintain the behavior for
which exception is thrown.
--
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]
