From 32056e0b7f88c0df73f7f27b605d61bd51d4a05b Mon Sep 17 00:00:00 2001 From: Sitaram Chamarty Date: Tue, 18 May 2010 14:57:14 +0530 Subject: [PATCH] (big one!) rule sequencing changes! There were 2 problems with rule sequencing. Eli had a use case where everyone is equal, but some are more equal than the others ;-) He wanted a way to say "everyone can create repos under their own names, but only some people should be able to rewind their branches". Something like this would be ideal (follow the rules in sequence for u1/u2/u3/u4, and you will see that the "deny" rule kicks in to prevent u1/u2 from being able to rewind, although they can certainly delete their branches): @private-owners = u1 u2 @experienced-private-owners = u3 u4 repo CREATOR/.* C = @private-owners @experienced-private-owners RWD = CREATOR RW = WRITERS R = READERS - = @private-owners RW+D = CREATOR In normal gitolite this doesn't work because the CREATOR rules (which get translated to "u1" at runtime) end up over-writing the "deny" rule when u1 or u2 are the creators. This over-writing happens directly at the "do compiled.pm" step. With big-config, this does not happen (because @private-owners does not get expanded to u1 and u2), but the problem remains: the order of picking up elements of repo_plus and user_plus is such that, again, the RW+D wins (it appears before the "-" rule). We fix all that by - making CREATOR complete to more than just the creator's name (for "u1", it now becomes "u1 - wild", which is actually illegal to use for real so there's no possibility of a name clash!) - maintaining a rule sequence number that is used to sort the rules eventually applied (this also resulted in the refex+perm hash becoming a list) --- src/gitolite.pm | 11 ++++++----- src/gl-compile-conf | 8 ++++++-- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/gitolite.pm b/src/gitolite.pm index 9501257..fcc122f 100644 --- a/src/gitolite.pm +++ b/src/gitolite.pm @@ -75,14 +75,15 @@ sub check_ref { # codes etc., but for now we're happy to just die. my ($allowed_refs, $repo, $ref, $perm) = @_; - for my $ar (@{$allowed_refs}) { - my $refex = (keys %$ar)[0]; + my @allowed_refs = sort { $a->[0] <=> $b->[0] } @{$allowed_refs}; + for my $ar (@allowed_refs) { + my $refex = $ar->[1]; # refex? sure -- a regex to match a ref against :) next unless $ref =~ /^$refex/; - die "$perm $ref $ENV{GL_USER} DENIED by $refex\n" if $ar->{$refex} eq '-'; + die "$perm $ref $ENV{GL_USER} DENIED by $refex\n" if $ar->[2] eq '-'; # as far as *this* ref is concerned we're ok - return $refex if ($ar->{$refex} =~ /\Q$perm/); + return $refex if ($ar->[2] =~ /\Q$perm/); } die "$perm $ref $repo $ENV{GL_USER} DENIED by fallthru\n"; } @@ -275,7 +276,7 @@ sub parse_acl $repos{$dr}{DELETE_IS_D} = 1 if $repos{$r}{DELETE_IS_D}; $repos{$dr}{NAME_LIMITS} = 1 if $repos{$r}{NAME_LIMITS}; - for my $u ('@all', @user_plus) { + for my $u ('@all', "$gl_user - wild", @user_plus) { my $du = $gl_user; $du = '@all' if $u eq '@all'; $repos{$dr}{C}{$du} = 1 if $repos{$r}{C}{$u}; $repos{$dr}{R}{$du} = 1 if $repos{$r}{R}{$u}; diff --git a/src/gl-compile-conf b/src/gl-compile-conf index d6d6dbf..a6064ee 100755 --- a/src/gl-compile-conf +++ b/src/gl-compile-conf @@ -95,6 +95,9 @@ our %groups = (); # in between :) my %repos = (); +# rule sequence number +my $rule_seq = 0; + # ... having been forced to use a list as described above, we lose some # efficiency due to the possibility of the same {ref, perms} pair showing up # multiple times for the same repo+user. So... @@ -286,8 +289,9 @@ sub parse_conf_file # that do *not* use NAME limits. Setting a flag that # can be checked right away will help us do that $repos{$repo}{NAME_LIMITS} = 1 if $ref =~ /^NAME\//; - push @{ $repos{$repo}{$user} }, { $ref => $perms } - unless $rurp_seen{$repo}{$user}{$ref}{$perms}++; + my $p_user = $user; $p_user =~ s/(creator|readers|writers)$/$1 - wild/; + push @{ $repos{$repo}{$p_user} }, [ $rule_seq++, $ref, $perms ] + unless $rurp_seen{$repo}{$p_user}{$ref}{$perms}++; } } }