[PATCH 1 of 2] acl: add support for branch-based access control; more informative messages

[Matt Mackall]

On Fri, 2010-04-30 at 15:58 -0300, elifarley at gmail.com wrote:
> +DENY_BRANCHES = 'acl.deny.branches'
> +ALLOW_BRANCHES = 'acl.allow.branches'
> +DENY_FILES = 'acl.deny'
> +ALLOW_FILES = 'acl.allow'

This is silly. 

> +
>  def _getusers(group):
>      return grp.getgrnam(group).gr_mem
>  
> @@ -111,25 +156,31 @@
>  
>      return False
>  
> -def buildmatch(ui, repo, user, key):
> +def _buildmatch(ui, repo, user, key):
>      '''return tuple of (match function, list enabled).'''
> +
>      if not ui.has_section(key):
> -        ui.debug('acl: %s not enabled\n' % key)
> +        ui.debug('acl: [%s] not defined\n' % key)

Please don't mix these sorts of unrelated changes in with your patches.
If you want to make preparatory clean-ups or reworks, those should be in
separate patches.

>          return None
>  
> -    pats = [pat for pat, users in ui.configitems(key)
> +    items = [item for item, users in ui.configitems(key)
>              if _usermatch(user, users)]
> -    ui.debug('acl: %s enabled, %d entries for user %s\n' %
> -             (key, len(pats), user))
> -    if pats:
> -        return match.match(repo.root, '', pats)
> -    return match.exact(repo.root, '', [])
> +    ui.debug('acl: [%s] has %d entries\n' % (key, len(items)))
>  
> +    if not items:
> +        return lambda b: False
> +
> +    if repo:
> +        return match.match(repo.root, '', items)
> +
> +    return lambda b: '*' in items or b in items

I can't tell if any of the changes above are actually relevant. None of
them have any rationale given.

>  def hook(ui, repo, hooktype, node=None, source=None, **kwargs):
> +

Bzzt.

>      if hooktype not in ['pretxnchangegroup', 'pretxncommit']:
>          raise util.Abort(_('config error - hook type "%s" cannot stop '
>                             'incoming changesets nor commits') % hooktype)
> +

Bzzt.

>      if (hooktype == 'pretxnchangegroup' and
>          source not in ui.config('acl', 'sources', 'serve').split()):
>          ui.debug('acl: changes have source "%s" - skipping\n' % source)
> @@ -144,19 +195,43 @@
>      if user is None:
>          user = getpass.getuser()
>  
> +    ui.debug('acl: checking access for user "%s"\n' % user)
> +
>      cfg = ui.config('acl', 'config')
>      if cfg:
> -        ui.readconfig(cfg, sections = ['acl.allow', 'acl.deny'])
> -    allow = buildmatch(ui, repo, user, 'acl.allow')
> -    deny = buildmatch(ui, repo, user, 'acl.deny')
> +        ui.readconfig(cfg, sections = [ALLOW_BRANCHES, DENY_BRANCHES,
> +                                       ALLOW_FILES, DENY_FILES])
>  
> -    for rev in xrange(repo[node], len(repo)):
> -        ctx = repo[rev]
> -        for f in ctx.files():
> -            if deny and deny(f):
> -                ui.debug('acl: user %s denied on %s\n' % (user, f))
> -                raise util.Abort(_('acl: access denied for changeset %s') % ctx)
> -            if allow and not allow(f):
> -                ui.debug('acl: user %s not allowed on %s\n' % (user, f))
> -                raise util.Abort(_('acl: access denied for changeset %s') % ctx)
> -        ui.debug('acl: allowing changeset %s\n' % ctx)
> +    allow = _buildmatch(ui, None, user, ALLOW_BRANCHES)
> +    deny = _buildmatch(ui, None, user, DENY_BRANCHES)
> +
> +    if deny or allow:
> +        ui.debug('acl: checking branch access\n')
> +        for rev in xrange(repo[node], len(repo)):
> +            branch = repo[rev].branch()
> +            if deny and deny(branch):
> +                raise util.Abort(_('acl: user "%s" denied on branch "%s"'
> +                                   ' (changeset "%s")')
> +                                   % (user, branch, repo[rev]))
> +            if allow and not allow(branch):
> +                raise util.Abort(_('acl: user "%s" not allowed on branch "%s"'
> +                                   ' (changeset "%s")')
> +                                   % (user, branch, repo[rev]))
> +            ui.debug('acl: access granted: "%s" on branch "%s"\n'
> +            % (repo[rev], branch))
> +
> +    allow = _buildmatch(ui, repo, user, ALLOW_FILES)
> +    deny = _buildmatch(ui, repo, user, DENY_FILES)
> +
> +    if deny or allow:
> +        ui.debug('acl: checking path access\n')
> +        for rev in xrange(repo[node], len(repo)):
> +            ctx = repo[rev]
> +            for f in ctx.files():
> +                if deny and deny(f):
> +                    raise util.Abort(_('acl: user "%s" denied on "%s"'
> +                    ' (changeset "%s")') % (user, f, ctx))
> +                if allow and not allow(f):
> +                    raise util.Abort(_('acl: user "%s" not allowed on "%s"'
> +                    ' (changeset "%s")') % (user, f, ctx))
> +            ui.debug('acl: access granted: "%s"\n' % ctx)

Why are two loops needed?

-- 
http://selenic.com : development and support for Mercurial and Linux