dragonls opened a new pull request, #25121:
URL: https://github.com/apache/pulsar/pull/25121
### Motivation
When using `MultiRolesTokenAuthorizationProvider` with multiple roles in a
JWT token, if one of the roles fails the subscription prefix check in
`PulsarAuthorizationProvider#canConsumeAsync`, it throws a
`PulsarServerException` with the message "The subscription name needs to be
prefixed by the authentication role".
This exception propagates up and causes the entire authorization to fail,
even if another role in the token has valid permissions. This is problematic in
multi-role scenarios where:
- A JWT token contains multiple roles (e.g., `["user-a", "user-b"]`)
- Only one role needs to have permission for the operation to succeed
- The `FutureUtil.waitForAny` mechanism should return success as soon as any
role is authorized
### Modifications
Modified `MultiRolesTokenAuthorizationProvider#authorize` method to handle
exceptions differently based on the number of roles:
1. **Single role**: Keep the original behavior - exceptions are propagated
as-is. This ensures backward compatibility and proper error reporting when
there's only one role.
2. **Multiple roles**: Swallow all exceptions and convert them to `false`
(authorization failed). This allows `FutureUtil.waitForAny` to work correctly -
if any role succeeds, the overall authorization succeeds; only if all roles
fail (return `false` or throw exceptions), the authorization fails.
```java
if (roles.size() == 1) {
roles.forEach(r -> futures.add(authorizeFunc.apply(r)));
} else {
roles.forEach(r -> futures.add(authorizeFunc.apply(r).exceptionally(ex
-> false)));
}
```
### Verifying this change
- [ ] Make sure that the change passes the CI checks.
This change added new test cases in
`MultiRolesTokenAuthorizationProviderTest`:
- `testMultiRolesAuthzWithSubscriptionPrefixMismatchException`: Tests
multi-role scenarios where:
- One role succeeds, another throws exception -> returns `true`
- All roles throw exceptions -> returns `false`
- `testSingleRoleAuthzWithSubscriptionPrefixMismatchException`: Tests
single-role scenario where:
- Single role throws exception -> propagates the original exception
### Does this pull request potentially affect one of the following parts:
<!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
*If the box was checked, please highlight the changes*
- [ ] Dependencies (add or upgrade a dependency)
- [ ] The public API
- [ ] The schema
- [ ] The default values of configurations
- [ ] The threading model
- [ ] The binary protocol
- [ ] The REST endpoints
- [ ] The admin CLI options
- [ ] The metrics
- [ ] Anything that affects deployment
### Documentation
<!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
- [ ] `doc` <!-- Your PR contains doc changes. -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update
later -->
- [x] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->
### Matching PR in forked repository
PR in forked repository: https://github.com/dragonls/pulsar/pull/12
<!--
After opening this PR, the build in apache/pulsar will fail and instructions
will
be provided for opening a PR in the PR author's forked repository.
apache/pulsar pull requests should be first tested in your own fork since
the
apache/pulsar CI based on GitHub Actions has constrained resources and quota.
GitHub Actions provides separate quota for pull requests that are executed
in
a forked repository.
The tests will be run in the forked repository until all PR review comments
have
been handled, the tests pass and the PR is approved by a reviewer.
-->
--
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]
