Improvement to ACL extension?

Peter Brogren hg at bems.se
Wed Jan 25 01:41:17 CST 2012


Hello Developers,

While trying to set up some basic access rules to only allow certain 
people to push changes into specific branches I studied the operation of 
the ACL extension. In my opinion the basic rules for determining whether 
a push is allowed or not is non-ideal. Essentially the check is first to 
see a user is on the deny list, then the allow list is checked and 
lastly the default is to allow. In my opinion it would make more sense 
to have the order changed so that either check in the order a) check if 
allowed, b) check if not allowed and c) default to allow if no match or 
the complete reverse. I have seen people trying to set up the basic rule 
of allowing a specific set of users access to push into the default 
branch and all others can push to wherever they want EXCEPT the default 
branch. The way the ACL extension works now, this is pretty hard unless 
we list all these other users under the deny section, and of course we 
would prefer to specifically name the ones with access and everyone else 
(default) will not get access. If users can register themselves it gets 
even harder to use the current ACL hook. However in the suggested 
approach it would be a simple rule to name the ones with access in the 
allow section and then put everyone else in the deny section as shown below:

[acl.allow.branches]
default = repomaster
[acl.deny.branches]
default = *

The suggested rule would also mean that any branch other than 'default' 
would fall through to default behavior of allowing access.

Now looking at how to implement this, the checks would be like this:
     if specifically allowed to push into branch
         great news => you can push!
    else if specifically NOT allowed to push   # note that we only check 
deny if we were not specifically allowed above
         bad news => permission denied!
    else     # if no rule specified the default behavior applies
         good news => you can push!


While then looking a bit further into this (I have never programmed in 
Python before!) I started checking if I could actually change the code 
in the ACL extension to give me this exact behaviour. I also found there 
is a section called "acl.allow" which isn't really obvious from the name 
what it is for (as is the case with the acl.allow.branch) so I renamed 
this section to "acl.allow.paths" which just shows better what it is 
for. I then implemented the above behaviour for both branches and paths.

Now, because I have done this anyway, I thought I could share this since 
I have found quite a few others having similar problems as the example 
above with the 'default' branch access. I'm not sure though if it is a 
good idea to include this as a replacement for the existing ACL 
extension since the configuration is not compatible with the existing 
extension. Anyone using a configuration with the existing ACL extension 
would need to rewrite their configuration so it might not be ideal. 
However, I believe this rule would make more sense for controlling 
access to the repository and hopefully someone else can get use out of this.

If there is interest for this extension to be included, I am happy to 
test thoroughly and update the comment section at the top of the file so 
the rules are explained in a way that represents this new algorithm.

Best Regards,
Peter


-------------- next part --------------
A non-text attachment was scrubbed...
Name: acl.pyc
Type: application/octet-stream
Size: 8453 bytes
Desc: not available
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20120124/c41de477/attachment.obj>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: acl.py
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20120124/c41de477/attachment.ksh>


More information about the Mercurial-devel mailing list