Skip to content

Commit 133fe20

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

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
@@ -220,7 +220,11 @@ public CompletableFuture<Boolean> authorize(String role, AuthenticationDataSourc
220220
return CompletableFuture.completedFuture(false);
221221
}
222222
List<CompletableFuture<Boolean>> futures = new ArrayList<>(roles.size());
223-
roles.forEach(r -> futures.add(authorizeFunc.apply(r)));
223+
if (roles.size() == 1) {
224+
roles.forEach(r -> futures.add(authorizeFunc.apply(r)));
225+
} else {
226+
roles.forEach(r -> futures.add(authorizeFunc.apply(r).exceptionally(ex -> false)));
227+
}
224228
return FutureUtil.waitForAny(futures, ret -> (boolean) ret).thenApply(v -> v.isPresent());
225229
});
226230
}

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,14 +21,17 @@
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;
2728
import java.util.Set;
2829
import java.util.concurrent.CompletableFuture;
30+
import java.util.concurrent.ExecutionException;
2931
import java.util.function.Function;
3032
import javax.crypto.SecretKey;
3133
import lombok.Cleanup;
34+
import org.apache.pulsar.broker.PulsarServerException;
3235
import org.apache.pulsar.broker.ServiceConfiguration;
3336
import org.apache.pulsar.broker.authentication.AuthenticationDataSource;
3437
import org.apache.pulsar.broker.authentication.AuthenticationDataSubscription;
@@ -304,4 +307,108 @@ public String getHttpHeader(String name) {
304307
assertTrue(provider.authorize("admin1", null, authorizeFunc).get());
305308
assertFalse(provider.authorize("admin2", null, authorizeFunc).get());
306309
}
310+
311+
/**
312+
* Test subscription prefix mismatch exception handling.
313+
* <p>
314+
* Scenario 1: One role authorization succeeds, another role throws subscription prefix mismatch exception
315+
* -> Returns true (exception is swallowed)
316+
* Scenario 2: All roles throw subscription prefix mismatch exception -> Returns false
317+
*/
318+
@Test
319+
public void testMultiRolesAuthzWithSubscriptionPrefixMismatchException() throws Exception {
320+
SecretKey secretKey = AuthTokenUtils.createSecretKey(SignatureAlgorithm.HS256);
321+
String userA = "user-a";
322+
String userB = "user-b";
323+
String token = Jwts.builder()
324+
.claim(MultiRolesTokenAuthorizationProvider.DEFAULT_ROLE_CLAIM, new String[]{userA, userB})
325+
.signWith(secretKey).compact();
326+
327+
MultiRolesTokenAuthorizationProvider provider = new MultiRolesTokenAuthorizationProvider();
328+
ServiceConfiguration conf = new ServiceConfiguration();
329+
provider.initialize(conf, mock(PulsarResources.class));
330+
331+
AuthenticationDataSource ads = new AuthenticationDataSource() {
332+
@Override
333+
public boolean hasDataFromHttp() {
334+
return true;
335+
}
336+
337+
@Override
338+
public String getHttpHeader(String name) {
339+
if (name.equals("Authorization")) {
340+
return "Bearer " + token;
341+
} else {
342+
throw new IllegalArgumentException("Wrong HTTP header");
343+
}
344+
}
345+
};
346+
347+
// userA throws subscription prefix mismatch exception, userB returns true -> result should be true
348+
assertTrue(provider.authorize("test", ads, role -> {
349+
if (role.equals(userA)) {
350+
CompletableFuture<Boolean> future = new CompletableFuture<>();
351+
future.completeExceptionally(new PulsarServerException(
352+
"The subscription name needs to be prefixed by the authentication role"));
353+
return future;
354+
}
355+
return CompletableFuture.completedFuture(true);
356+
}).get());
357+
358+
// All roles throw subscription prefix mismatch exception -> result should be false
359+
assertFalse(provider.authorize("test", ads, role -> {
360+
CompletableFuture<Boolean> future = new CompletableFuture<>();
361+
future.completeExceptionally(new PulsarServerException(
362+
"The subscription name needs to be prefixed by the authentication role"));
363+
return future;
364+
}).get());
365+
}
366+
367+
/**
368+
* Test single role with subscription prefix mismatch exception.
369+
* <p>
370+
* Single role throws subscription prefix mismatch exception -> Should throw the original exception
371+
* (Single role keeps original behavior, does not swallow exception)
372+
*/
373+
@Test
374+
public void testSingleRoleAuthzWithSubscriptionPrefixMismatchException() throws Exception {
375+
SecretKey secretKey = AuthTokenUtils.createSecretKey(SignatureAlgorithm.HS256);
376+
String userA = "user-a";
377+
String token = Jwts.builder()
378+
.claim(MultiRolesTokenAuthorizationProvider.DEFAULT_ROLE_CLAIM, userA)
379+
.signWith(secretKey).compact();
380+
381+
MultiRolesTokenAuthorizationProvider provider = new MultiRolesTokenAuthorizationProvider();
382+
ServiceConfiguration conf = new ServiceConfiguration();
383+
provider.initialize(conf, mock(PulsarResources.class));
384+
385+
AuthenticationDataSource ads = new AuthenticationDataSource() {
386+
@Override
387+
public boolean hasDataFromHttp() {
388+
return true;
389+
}
390+
391+
@Override
392+
public String getHttpHeader(String name) {
393+
if (name.equals("Authorization")) {
394+
return "Bearer " + token;
395+
} else {
396+
throw new IllegalArgumentException("Wrong HTTP header");
397+
}
398+
}
399+
};
400+
401+
// Single role throws subscription prefix mismatch exception -> should propagate exception
402+
ExecutionException ex = expectThrows(ExecutionException.class, () -> {
403+
provider.authorize("test", ads, role -> {
404+
CompletableFuture<Boolean> future = new CompletableFuture<>();
405+
future.completeExceptionally(new PulsarServerException(
406+
"The subscription name needs to be prefixed by the authentication role"));
407+
return future;
408+
}).get();
409+
});
410+
assertTrue(ex.getCause() instanceof PulsarServerException);
411+
assertTrue(ex.getCause().getMessage().contains(
412+
"The subscription name needs to be prefixed by the authentication role"));
413+
}
307414
}

0 commit comments

Comments
 (0)