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