lhotari opened a new pull request, #25618:
URL: https://github.com/apache/pulsar/pull/25618
PIP: #460
Follow-up to #25565, which introduced the Scalable Topics (Topics v5) Admin
API without enforcing authentication or authorization on its REST endpoints.
### Motivation
The initial Scalable Topics Admin API merged in #25565 only called
`validateNamespaceName(...)` (a name-format parser) on its REST endpoints. None
of the 9 endpoints under `/admin/v2/scalable/...` performed any authn/authz
check, and the 3 endpoints under `/admin/v2/segments/...` only did
`validateTopicOwnershipAsync(...)` (a routing/ownership check, not an
authorization check). The OpenAPI annotations on the `ScalableTopics` endpoints
already advertise `401 / 403` responses, so the public contract was written but
not enforced.
PIP-460 §Security Considerations states: *"The new admin API endpoints for
segment operations (split, merge, metadata read) will require the same
permissions as equivalent topic-level admin operations."* This PR wires that
up, mirroring the patterns used elsewhere in the Admin API (`PersistentTopics`
/ `PersistentTopicsBase`).
### Modifications
`pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/ScalableTopics.java`
— added an authz call as the first step of the async chain on each of the 9
endpoints. Operation mapping follows the conventions used by the
partitioned-topic admin API:
| Endpoint | Authorization |
|---|---|
| `GET /{tenant}/{ns}` (list) |
`validateNamespaceOperationAsync(GET_TOPICS)` |
| `PUT /{tenant}/{ns}/{topic}` (create) |
`validateNamespaceOperationAsync(CREATE_TOPIC)` |
| `GET /{tenant}/{ns}/{topic}` (metadata) |
`validateTopicOperationAsync(GET_METADATA)` |
| `DELETE /{tenant}/{ns}/{topic}` (delete) |
`validateNamespaceOperationAsync(DELETE_TOPIC)` |
| `GET .../stats` | `validateTopicOperationAsync(GET_STATS)` |
| `PUT .../subscriptions/{sub}` | `validateTopicOperationAsync(SUBSCRIBE,
subscription)` |
| `DELETE .../subscriptions/{sub}` |
`validateTopicOperationAsync(UNSUBSCRIBE, subscription)` |
| `POST .../split/{segmentId}` | `validateSuperUserAccessAsync()` |
| `POST .../merge/{id1}/{id2}` | `validateSuperUserAccessAsync()` |
`splitSegment` and `mergeSegments` are super-user only for now. Auto-split /
auto-merge and any tenant-admin-initiated workflows can revisit this without
breaking compatibility (relaxing access is non-breaking; tightening is
breaking).
`pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/Segments.java` —
all three endpoints (`createSegment`, `terminateSegment`, `deleteSegment`) now
call `validateSuperUserAccessAsync()` before the existing
`validateTopicOwnershipAsync(...)` routing check. These endpoints are
cross-broker coordination primitives invoked by the controller broker during
split/merge — not user-facing. The controller broker uses its broker-internal
credentials, which are super-user, so internal coordination continues to work;
tenant admins (and other principals) cannot bypass the super-user requirement
on `splitSegment`/`mergeSegments` by calling the segment-level endpoints
directly. Class-level Javadoc documents this security boundary, and `401`/`403`
are added to `@ApiResponses` on each endpoint.
`pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/ScalableTopicsAuthZTest.java`
— new TestNG class modeled after `TopicAuthZTest`. 12 tests cover all 12
endpoints (9 in `ScalableTopics`, 3 in `Segments`) against three roles:
anonymous (`NOBODY_TOKEN`), tenant admin, and super-user. The test asserts the
authz contract only — non-`NotAuthorized` failures from the underlying service
are tolerated, since the happy paths are already covered by
`ScalableTopicServiceTest` / `ScalableTopicControllerTest`.
When `authentication` or `authorization` is disabled, the existing
`validateXxxAsync` helpers complete immediately with `null`, so behavior is
unchanged for clusters that do not enable auth.
### Verifying this change
- [x] Make sure that the change passes the CI checks.
This change added tests and can be verified as follows:
- New `ScalableTopicsAuthZTest` (12 tests): all pass. Covers nobody /
tenant-admin / super-user roles against all 12 endpoints.
- Existing scalable-topic suite — `ConsumerSessionTest`,
`DagWatchSessionTest`, `ScalableTopicControllerTest`,
`ScalableTopicServiceTest`, `SegmentLayoutTest`, `SubscriptionCoordinatorTest`
— 92/92 pass, no regressions.
- `./gradlew :pulsar-broker:checkstyleMain :pulsar-broker:checkstyleTest`
clean.
### Does this pull request potentially affect one of the following parts:
- [ ] Dependencies (add or upgrade a dependency)
- [ ] The public API
- [ ] The schema
- [ ] The default values of configurations
- [ ] The threading model
- [ ] The binary protocol
- [x] The REST endpoints
- [ ] The admin CLI options
- [ ] The metrics
- [ ] Anything that affects deployment
The REST endpoints under `/admin/v2/scalable/*` and `/admin/v2/segments/*`
now enforce authentication/authorization. Clusters that previously relied on
these endpoints being unauthenticated will see `401`/`403` for non-super-user
(and, for the segment-level endpoints, non-super-user-only) callers when
authentication and authorization are enabled. This brings the endpoints in line
with the rest of the Admin API and matches the contract already advertised by
the existing `@ApiResponses` annotations.
--
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]
