[Issue 5720][authorization provider] Add granularity

[KannarFr]

Fixes #5720

Motivation

Provide "real" authz abilities to pulsar resources.

Modifications

Add stuff to AuthorizationProvider interface, and use them on every pulsar resource management auth (tenant, namespace, topics, functions, connectors, ...)

Verifying this change

• Make sure that the change passes the CI checks.

This change added tests and can be verified as follows:

Will add specified test.

Documentation

• Does this pull request introduce a new feature? yes
• If yes, how is the feature documented? not documented (yet, will do)

[joefk]

I see that there is an unload permission being granted to namespace owners? What is the rationale?

A few many unloads, and you ZK and cluster is dead.

There is a reason these operations are restricted. So unless you are willing to introduce and implement adequate defense measures into Pulsar, these are all opening DOS and vulnerabilities into the system.

[KannarFr]

@joefk I'm open to all discussion I don't know correctly this part. Can you explain why unloads are critical for Pulsar? We can imagine that only one unload is authorized at the same time.

The main goal of this issue is to provide real authz provider.

[joefk]

@KannarFr you should get familiar with the PIP-49 and the discussion on the dev list about it. Just because unload operates on a namespace, it does not imply that it should ever be executed by a namespace owner.

FWIW, I don't think it's feasible to implement an adequate restriction on unload in a simple way, because it's a system management operation, driven by system management needs.

At the least, these kind of shoot-yourself features should not be hard-coded into the broker as a possible option. It should be in a config file.

[KannarFr]

@joefk I think every permissions should be granted by authz provider. So we need to provide authz provider stuff to authorize or not, but it's not to the pulsar side to decide if access is grant or not. Maybe we can document critical stuff as mentioned unload but that's it. I'm not working on the PulsarAuthorizationProvider which is the default used, but on the AuthorizationProvider interface and its usages.

Thats being said, I can be wrong, WDYT?

[joefk]

@KannarF

I think every permissions should be granted by authz provider.

Some of them are not grantable. How did your arrive at that conclusion? That is a very simplistic view of the system. After going through PIP-49 and the discussions about that, what is your take?

As, you state in the description of your PR..

And the biggest part of the task: go through every usage of validateSuperUserAccess and validateAdminAccessForTenant and check if they can be replaced with finer grain access
Can you share your findings for that? I did not see any

My concern is this. How can I, as a Pulsar operator (NOT a tenant) decide which of these operations I will delegate to my tenants for finer control ?

Operators should not be put in a position where if they were to deploy this authz provider, they would have to go and turn off/on certain actions for all fine-grained resources every time someone creates a namespace or topic. And the things you are proposing to delegate here in this proposal are an operators nightmare for a large cluster. Hence my ask. Put the list of the things that can be delegated into a config file, so that operators can decide ONCE when they DEPLOY Pulsar, about what can be potentially delegated. Don't hardcode it into the server, and create runtime maintenance work for operators

[KannarFr]

Hi, Here we are defining an authz provider interface not the authz provider. We want to provide the way to authz every cases as multitenancy stuff. In my case all authorized info will be defined in the role String which will be a token containing authorized information. Like this token has produce operation authorized on this namespace or this topic. In the case you are illustrating we just want that the PulsarAuthzProvider (which is the default for now) handles some default cases as you mentioned. And sure we could define default bindings in configuration file. This stuff will come in other PR (I guess), currently I just want to impl the granularity possibilities without breaking changes for current default PulsarAuthzProvider. Le sam. 29 févr. 2020 à 02:44, joefk <notifications@github.com> a écrit :

…

@KannarF I think every permissions should be granted by authz provider. Some of them are not grantable. How did your arrive at that conclusion? That is a very simplistic view of the system. After going through PIP-49 and the discussions about that, what is your take? As, you state in the description of your PR.. And the biggest part of the task: go through every usage of validateSuperUserAccess and validateAdminAccessForTenant and check if they can be replaced with finer grain access Can you share your findings for that? I did not see any My concern is this. How can I, as a Pulsar operator (NOT a tenant) decide which of these operations I will delegate to my tenants for finer control ? Operators should not be put in a position where if they were to deploy this authz provider, they would have to go and turn off/on certain actions for all fine-grained resources every time someone creates a namespace or topic. And the things you are proposing to delegate here in this proposal are an operators nightmare for a large cluster. Hence my ask. Put the list of the things that can be delegated into a config file, so that operators can decide ONCE when they DEPLOY Pulsar, about what can be potentially delegated. Don't hardcode it into the server, and create runtime maintenance work for operators — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6428?email_source=notifications&email_token=ABZLQJMY5MQQXPQAXPIWBH3RFG4YLA5CNFSM4K4ES5QKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOENKYPUA#issuecomment-592807888>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABZLQJKYUH3RYTKDCFSZTUTRFG4YLANCNFSM4K4ES5QA> .

[sijie]

@KannarFr

Thank you for your contribution!

I think we should have a clear interface about resources and verbs. I feel that you are mixing resources with verbs and generating a lot of "unneeded" operations.

For example, each policy rule in a namespace policy should be treated as a resource. The operations to a given policy rule are write and read. So when you introduce a new policy rule, you don't need to introduce a new verb. Also, I don't think enum is a good way to allow extensibility. I think we can use a string for representing different policy rules within a namespace policy.

Kubernetes' API machinery provides a good example of this.

---

@joefk

I think the pull request here is to allow people to define their own authorization implementation since some organizations have the need to integrate Pulsar into its owner authentication/authorization system to control those resources. The purpose of this pull request is different from PIP-49. We attempt to improve the current authorization model. This pull request should NOT change our current authorization model as the concerns have been raised when discussing PIP-49.

[KannarFr]

@sijie operations like internalGrantPermissionOnNamespace in NamespacesBase.java are antipattern with the reworking I'm doing. I don't know how to manage them. I think I should let them as they are. Due to "real" authz plugin we should drop all roles/actions stuff which are in ZK. DefaultAuthzProvider should be updated regarding this.

That being said, we need maintain legacy stuff so I would like to have your opinions about this.

[joefk]

Due to "real" authz plugin we should drop all roles/actions stuff which are in ZK. DefaultAuthzProvider should be updated regarding this.That being said, we need maintain legacy stuff so I would like to have your opinions about this.

I would like to see legacy stuff work as it is.

My concern is that is trying to impose a model which is not supported in Pulsar. Assumptions like "Any resource operation should be managed/authorized by the resource which owns it." and "ClusterOperations should manage Tenants, TenantOperation should manage Namespaces, etc. Hierarchical approach" it only makes me think that is being designed for some other system, which is not Pulsar.

Pulsar has no operational hierarchy beyond tenants. It is not designed for namespace admins or topic admins. The premise of pulsar is that a tenant manages all its namespaces. There are operations that can be logically managed at the namespace and topic level, but that is a convenience for the tenant admin.

[jiazhai]

@tuteng @zymap Would you also please help review this PR?

[sijie]

Merged this pull request to allow some time to bake the change for the 2.6.0 release.