I switched to the name "permissionOnSubscriptionRequired" for this feature [0]. It describes the feature while satisfying the requirement to default to false.
I plan to address the PR's remaining feedback and write tests tomorrow (Wednesday). If there isn't any other discussion, I'll start the vote once I get tests passing. Thanks, Michael [0] https://github.com/apache/pulsar/pull/15576 On Fri, May 13, 2022 at 2:53 PM Michael Marshall <mmarsh...@apache.org> wrote: > > Hello Pulsar Community, > > Here is a PIP to add a new namespace policy to configure how the > PulsarAuthorizationProvider handles the default subscription > permission value (null/an empty set). I look forward to your feedback. > > PIP: https://github.com/apache/pulsar/issues/15597 > > Thanks, > Michael > > ## Motivation > > Pulsar supports subscription level authorization. When combined with > topic level authorization, a user can configure Pulsar to limit which > roles can consume from which topic subscriptions. However, when this > feature is left unconfigured for a subscription, a role that has > permission to consume from a topic is, by default, implicitly granted > permission to consume from any subscription on that topic. As a > consequence, a missed security configuration could lead to accidental > privilege escalation. Here is a reference to the code responsible for > the current behavior: > > https://github.com/apache/pulsar/blob/6864b0ae5520e06b9d0fc5dcfa5a0a0a44feee87/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/PulsarAuthorizationProvider.java#L115-L122 > > ## Goal > > I propose we add a namespace policy to configure a Pulsar namespace to > either allow all or reject all roles when there is no configuration > for a specific subscription’s permission. This way, a missed > configuration results in a rejected request due to insufficient > permission. > > This PIP will not change the current behavior and will be backwards > compatible. It will add a new boolean field to the existing > `auth_policies` namespace policy to configure how the > `PulsarAuthorizationProvider` handles an empty set of allowed roles in > the `canConsume` method. > > ## Naming > > I am not settled on the right name for this feature/namespace policy > yet. Hopefully this thread can help identify the right name. > > First, the existing subscription level authorization feature has > several names. The Admin API calls this feature > `PermissionOnSubscription`, the Pulsar Admin CLI tool calls it > `subscription-permission`, the AuthPolicies interface calls it > `SubscriptionAuthentication`, and the value is stored in the metadata > store as `subscription_auth_roles`. > > My preferred names for this feature are `implicit_subscription_auth` > and `implicitPermissionOnSubscription` because they work well with the > “grant” and “revoke” actions, e.g. > `grantImplicitPermissionOnSubscription` would be a PUT/POST call to > the `/implicitPermissionOnSubscription` endpoint to set the policy > value to true. However, that policy name requires the default value to > be true to maintain backwards compatibility. Enrico expressed concern > that defaulting to true is problematic for the upgrade path: > https://github.com/apache/pulsar/pull/15576#discussion_r872045946. > > Alternatively, we could use the names > `PermissionOnSubscriptionRequired` and `subscription_auth_required`. > In that case, I would switch the admin API so that the admin API has a > single setter endpoint that takes the configuration as a part of the > body instead of relying on PUT to mean grant permission and DELETE to > mean revoke permission. > > Please let me know if you have thoughts on what name(s) make sense for > this feature. > > ## API Changes > > The API changes include updating the Admin API to enable getting and > modifying the namespace policy, as well as updating the namespace > AuthPolicy interface to store this new metadata field. > > ## Implementation > > Draft implementation: https://github.com/apache/pulsar/pull/15576 > > The core update is to the > `PulsarAuthorizationProvider#canConsumeAsync` method so that when > `implicit_subscription_auth` is true, a null or empty set of roles for > a subscription’s permission will result in granted permission to > consume from the subscription, and when `implicit_subscription_auth` > is false, a null or empty set of roles for a subscription’s permission > will result in rejected permission to consume from the subscription. > Note that if we negate the meaning of the variable name, the logic > will also be inverted appropriately. > > ## Rejected Alternatives > > First, we have already received a PR proposing to change the default > behavior for all use cases: > https://github.com/apache/pulsar/pull/11113. This PR went stale > because changing the default would break many deployments. By making > the behavior configurable, we satisfy the requirements requested in > that PR without breaking existing deployments. > > Second, it’s possible to implement a new `AuthorizationProvider` to > get this feature. However, I believe this feature will benefit users > and is a natural development on the existing Pulsar features around > subscription authorization, so I think we should include it in the > default `PulsarAuthorizationProvider`.
