michaeljmarshall opened a new pull request, #19270: URL: https://github.com/apache/pulsar/pull/19270
### Motivation The current `ServerCnx` validates the proxy role on certain protocol messages. This is unnecessary. Instead, we should verify that the `authRole` and the `originalPrincipal` are a valid combination before going to the `Connected` state. Note that the one "breaking" change is to prevent connections where the `originalPrincipal` is set with an `authRole` that is not a proxy role. This is an invalid state that was allowed to persist before. That state was not a vulnerability because the `isTopicOperationAllowed` validates that both the `originalPrincipal` and the `authRole` have permission to perform an operation. ### Modifications * Replace `invalidOriginalPrincipal` with `validateRoleAndOriginalPrincipal`. * Remove calls to `invalidOriginalPrincipal`. * Add call to `validateRoleAndOriginalPrincipal` when transitioning from `Connecting` to `Connected` state. ### Verifying this change It's likely that we should add tests for this change. I haven't found a good place to do so yet. The `ServerCnxAuthorizationTest` seems like a good candidate, but it is pretty verbose and relies heavily on mocking. It would be very helpful to add authentication tests that operate similarly to `ServerCnxTest`. ### Documentation - [x] `doc` I updated the appropriate Javadocs. ### Matching PR in forked repository PR in forked repository: https://github.com/michaeljmarshall/pulsar/pull/13 -- 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]
