Thanks Han for clarifying this ... ZOOKEEPER-1782 is exactly what I was looking for. I think this JIRA must get in soon.
So the contract remains same for all auth providers i.e. set the scheme as "super" -----Original Message----- From: Michael Han [mailto:[email protected]] Sent: Tuesday, June 13, 2017 12:09 AM To: [email protected] Subject: Re: Regarding Super user check inconsistency Hi Bhupendra, I think it is a known issue that SASL does not support super user. ZOOKEEPER-1782 had a patch to fix this, but the patch never landed. The patch seems reasonable, maybe you can create a PR based on the patch? >> there should be separate super user check based on all the auth >> providers Digest and X509 should already support superuser and the way they did is pretty consistent by having the auth scheme set as "super". SASL superuser can be added in a similar way (which is what ZOOKEEPER-1782 did actually) by materializing SASL super user with same auth scheme "super", so we can still use existing superuser ACL check. On Mon, Jun 12, 2017 at 2:40 AM, bhupendra jain <[email protected]> wrote: > HI > I have some doubts on the contract of super user in Zookeeper .. as > I see the c ode , Different auth provider has different mechanism to > identify the super user. > > When ACL check happens, > > - Based on scheme "Super" it considers the user as super user. > > for (Id authId : ids) { > > if (authId.getScheme().equals("super")) { > > return; > > } > > } > > - If scheme is not super, then first it checks the permission and > then call the AuthProvider to validate the ACL > > for (ACL a : acl) { > > Id id = a.getId(); > > if ((a.getPerms() & perm) != 0) { > > if (id.getScheme().equals("world") > > && id.getId().equals("anyone")) { > > return; > > } > > AuthenticationProvider ap = > ProviderRegistry.getProvider( id > > .getScheme()); > > if (ap != null) { > > for (Id authId : ids) { > > if (authId.getScheme().equals(id.getScheme()) > > && ap.matches(authId.getId(), > id.getId())) { > > return; > > } > > } > > } > > } > > } > > > > In case scheme is SASL and its super user but ACL is not explicitly > configured for that super user, the ACL check will not pass. But for > Digest it will pass even though ACL is not configured. > > I think , there should be separate super user check based on all the > auth providers. Current contract of each auth provider is as below : > > Digest : Based on superDigest configured, Sets the Scheme as "Super" > SASL: checks the ACL based on zookeeper.superUser parameter and if > user name is "super" > X509: Based on zookeeper.X509AuthenticationProvider.superUser > configured , sets the scheme as "Super" > IP: No super user concept > Custom: Custom logic. One way is that custom auth provider sets the > scheme as "super" > > So I think, > > - Either in AuthProvider interface we should have one more method > to let AuthProvider check whether user is super user > > boolean isSuperUser(Id authID); > > - OR each auth provider should set the scheme as "Super" for > super user and handle SASL as a special case. ( So in checkACL we > need to have specific validation for sasl) > > > I think if we have a clear contract, then same can be added at all > places for admin check such as this JIRA https://issues.apache.org/ > jira/browse/ZOOKEEPER-2014 > > Regards > Bhupendra > > > ________________________________ > This e-mail and its attachments contain confidential information from > HUAWEI, which is intended only for the person or entity whose address > is listed above. Any use of the information contained herein in any > way (including, but not limited to, total or partial disclosure, > reproduction, or dissemination) by persons other than the intended > recipient(s) is prohibited. If you receive this e-mail in error, > please notify the sender by phone or email immediately and delete it! > > > -- Cheers Michael.
