Re: [DISCUSS] Move superuser/tenantAdmin checks to AuthorizationService from AuthorizationProvider
Hi Jiwei, Sorry, my title seems confusing. I just refactor the PulsarAuthorizationProvider by moving superuser/tenantAdmin checks to AuthorizationService from PulsarAuthorizationProvider. IMO, the superuser and tenant admin are built-in in the Pulsar, not the third party, we also provide the isSuperUser and isTenantAdmin methods used for fine-grained control in the AuthorizationProvider, so I think to move superuser/tenantAdmin checks to AuthorizationService from PulsarAuthorizationProvider. Thanks, Zixuan guo jiwei 于2023年7月10日周一 16:07写道： > > Hi Zixuan, > From the perspective of interface implementation, for example, for > `allowTopicOperationAsync`, > we need to determine whether the role is admin or super user in > PulsarAuthorizationProvider, not in AuthorizationService. > > Regards > Jiwei Guo (Tboy) > > > On Fri, Jul 7, 2023 at 5:20 PM Zixuan Liu wrote: > > > Hello everyone, > > > > When a role wants to use the resource, the role needs to have resource > > permissions. > > > > The process is to first check whether the role is the superuser or > > tenant administrator. If yes, operations are allowed. Otherwise, check > > the policies stored in zk. > > > > Right now, we have the AuthorizationService and AuthorizationProvider, > > the AuthorizationService wraps the AuthorizationProvider call. When > > you check the code, you will find that both classes have the > > superuser/tenantAdmin checks in certain places, this may cause > > confusion when developing the custom AuthorizationProvider, so I > > suggest unifying superuser/tenantAdmin checks in the > > `AuthorizationService`, and then the `AuthorizationProvider` only > > needs to consider their business permissions. > > > > I created a PR a while ago, you can check it out: > > https://github.com/apache/pulsar/pull/20145, > > > > Thanks, > > Zixuan > >
Re: [DISCUSS] Move superuser/tenantAdmin checks to AuthorizationService from AuthorizationProvider
Hi Zixuan, From the perspective of interface implementation, for example, for `allowTopicOperationAsync`, we need to determine whether the role is admin or super user in PulsarAuthorizationProvider, not in AuthorizationService. Regards Jiwei Guo (Tboy) On Fri, Jul 7, 2023 at 5:20 PM Zixuan Liu wrote: > Hello everyone, > > When a role wants to use the resource, the role needs to have resource > permissions. > > The process is to first check whether the role is the superuser or > tenant administrator. If yes, operations are allowed. Otherwise, check > the policies stored in zk. > > Right now, we have the AuthorizationService and AuthorizationProvider, > the AuthorizationService wraps the AuthorizationProvider call. When > you check the code, you will find that both classes have the > superuser/tenantAdmin checks in certain places, this may cause > confusion when developing the custom AuthorizationProvider, so I > suggest unifying superuser/tenantAdmin checks in the > `AuthorizationService`, and then the `AuthorizationProvider` only > needs to consider their business permissions. > > I created a PR a while ago, you can check it out: > https://github.com/apache/pulsar/pull/20145, > > Thanks, > Zixuan >
[DISCUSS] Move superuser/tenantAdmin checks to AuthorizationService from AuthorizationProvider
Hello everyone, When a role wants to use the resource, the role needs to have resource permissions. The process is to first check whether the role is the superuser or tenant administrator. If yes, operations are allowed. Otherwise, check the policies stored in zk. Right now, we have the AuthorizationService and AuthorizationProvider, the AuthorizationService wraps the AuthorizationProvider call. When you check the code, you will find that both classes have the superuser/tenantAdmin checks in certain places, this may cause confusion when developing the custom AuthorizationProvider, so I suggest unifying superuser/tenantAdmin checks in the `AuthorizationService`, and then the `AuthorizationProvider` only needs to consider their business permissions. I created a PR a while ago, you can check it out: https://github.com/apache/pulsar/pull/20145, Thanks, Zixuan
