Skip to content

Commit e528277

Browse files
dragonlsdruidliu
authored andcommitted
[fix][broker] Fix MultiRolesTokenAuthorizationProvider error when subscription prefix doesn't match. (#25121)
Co-authored-by: druidliu <druidliu@tencent.com> (cherry picked from commit 133fe20)
1 parent f789330 commit e528277

File tree

2 files changed

+112
-1
lines changed

2 files changed

+112
-1
lines changed

pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/MultiRolesTokenAuthorizationProvider.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,11 @@ public CompletableFuture<Boolean> authorize(String role, AuthenticationDataSourc
217217
return CompletableFuture.completedFuture(false);
218218
}
219219
List<CompletableFuture<Boolean>> futures = new ArrayList<>(roles.size());
220-
roles.forEach(r -> futures.add(authorizeFunc.apply(r)));
220+
if (roles.size() == 1) {
221+
roles.forEach(r -> futures.add(authorizeFunc.apply(r)));
222+
} else {
223+
roles.forEach(r -> futures.add(authorizeFunc.apply(r).exceptionally(ex -> false)));
224+
}
221225
return FutureUtil.waitForAny(futures, ret -> (boolean) ret).thenApply(v -> v.isPresent());
222226
});
223227
}

pulsar-broker-common/src/test/java/org/apache/pulsar/broker/authorization/MultiRolesTokenAuthorizationProviderTest.java

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,14 @@
2121
import static org.mockito.Mockito.mock;
2222
import static org.testng.Assert.assertFalse;
2323
import static org.testng.Assert.assertTrue;
24+
import static org.testng.Assert.expectThrows;
2425
import io.jsonwebtoken.Jwts;
2526
import io.jsonwebtoken.SignatureAlgorithm;
2627
import java.util.Properties;
28+
import java.util.concurrent.ExecutionException;
2729
import java.util.function.Function;
2830
import lombok.Cleanup;
31+
import org.apache.pulsar.broker.PulsarServerException;
2932
import org.apache.pulsar.broker.ServiceConfiguration;
3033
import org.apache.pulsar.broker.authentication.AuthenticationDataSource;
3134
import org.apache.pulsar.broker.authentication.AuthenticationDataSubscription;
@@ -302,4 +305,108 @@ public String getHttpHeader(String name) {
302305
assertTrue(provider.authorize("admin1", null, authorizeFunc).get());
303306
assertFalse(provider.authorize("admin2", null, authorizeFunc).get());
304307
}
308+
309+
/**
310+
* Test subscription prefix mismatch exception handling.
311+
* <p>
312+
* Scenario 1: One role authorization succeeds, another role throws subscription prefix mismatch exception
313+
* -> Returns true (exception is swallowed)
314+
* Scenario 2: All roles throw subscription prefix mismatch exception -> Returns false
315+
*/
316+
@Test
317+
public void testMultiRolesAuthzWithSubscriptionPrefixMismatchException() throws Exception {
318+
SecretKey secretKey = AuthTokenUtils.createSecretKey(SignatureAlgorithm.HS256);
319+
String userA = "user-a";
320+
String userB = "user-b";
321+
String token = Jwts.builder()
322+
.claim("sub", new String[]{userA, userB})
323+
.signWith(secretKey).compact();
324+
325+
MultiRolesTokenAuthorizationProvider provider = new MultiRolesTokenAuthorizationProvider();
326+
ServiceConfiguration conf = new ServiceConfiguration();
327+
provider.initialize(conf, mock(PulsarResources.class));
328+
329+
AuthenticationDataSource ads = new AuthenticationDataSource() {
330+
@Override
331+
public boolean hasDataFromHttp() {
332+
return true;
333+
}
334+
335+
@Override
336+
public String getHttpHeader(String name) {
337+
if (name.equals("Authorization")) {
338+
return "Bearer " + token;
339+
} else {
340+
throw new IllegalArgumentException("Wrong HTTP header");
341+
}
342+
}
343+
};
344+
345+
// userA throws subscription prefix mismatch exception, userB returns true -> result should be true
346+
assertTrue(provider.authorize("test", ads, role -> {
347+
if (role.equals(userA)) {
348+
CompletableFuture<Boolean> future = new CompletableFuture<>();
349+
future.completeExceptionally(new PulsarServerException(
350+
"The subscription name needs to be prefixed by the authentication role"));
351+
return future;
352+
}
353+
return CompletableFuture.completedFuture(true);
354+
}).get());
355+
356+
// All roles throw subscription prefix mismatch exception -> result should be false
357+
assertFalse(provider.authorize("test", ads, role -> {
358+
CompletableFuture<Boolean> future = new CompletableFuture<>();
359+
future.completeExceptionally(new PulsarServerException(
360+
"The subscription name needs to be prefixed by the authentication role"));
361+
return future;
362+
}).get());
363+
}
364+
365+
/**
366+
* Test single role with subscription prefix mismatch exception.
367+
* <p>
368+
* Single role throws subscription prefix mismatch exception -> Should throw the original exception
369+
* (Single role keeps original behavior, does not swallow exception)
370+
*/
371+
@Test
372+
public void testSingleRoleAuthzWithSubscriptionPrefixMismatchException() throws Exception {
373+
SecretKey secretKey = AuthTokenUtils.createSecretKey(SignatureAlgorithm.HS256);
374+
String userA = "user-a";
375+
String token = Jwts.builder()
376+
.claim("sub", userA)
377+
.signWith(secretKey).compact();
378+
379+
MultiRolesTokenAuthorizationProvider provider = new MultiRolesTokenAuthorizationProvider();
380+
ServiceConfiguration conf = new ServiceConfiguration();
381+
provider.initialize(conf, mock(PulsarResources.class));
382+
383+
AuthenticationDataSource ads = new AuthenticationDataSource() {
384+
@Override
385+
public boolean hasDataFromHttp() {
386+
return true;
387+
}
388+
389+
@Override
390+
public String getHttpHeader(String name) {
391+
if (name.equals("Authorization")) {
392+
return "Bearer " + token;
393+
} else {
394+
throw new IllegalArgumentException("Wrong HTTP header");
395+
}
396+
}
397+
};
398+
399+
// Single role throws subscription prefix mismatch exception -> should propagate exception
400+
ExecutionException ex = expectThrows(ExecutionException.class, () -> {
401+
provider.authorize("test", ads, role -> {
402+
CompletableFuture<Boolean> future = new CompletableFuture<>();
403+
future.completeExceptionally(new PulsarServerException(
404+
"The subscription name needs to be prefixed by the authentication role"));
405+
return future;
406+
}).get();
407+
});
408+
assertTrue(ex.getCause() instanceof PulsarServerException);
409+
assertTrue(ex.getCause().getMessage().contains(
410+
"The subscription name needs to be prefixed by the authentication role"));
411+
}
305412
}

0 commit comments

Comments
 (0)