On 02.12.2018 08:25, Branko Čibej wrote:
> On 08.09.2018 11:17, Stefan Fuhrmann wrote:
>> These are the guiding principles for the 1.10 authz design:
>>
>> (1) ACLs are only evaluated on a per-user bases; ACLs that
>> don't mention this user (or any of their groups) are ignored.
>> Rationale: We don't want to explicitly repeat inherited access
>> specs that don't change for the respective path / section.
>
> This is not entirely true, as seen in the fix for SVN-4793. If a user is
> "not mentioned" in an inverted selector, those rights do propagate to
> the global level. For example:
>
> [groups]
> readers = foo, bar
>
> [/]
> ~@readers = rw
> @readers = r
>
>
> In this case 'user' has read-write access to '[/]' even though she's not
> mentioned anywhere in the authz file or the specific ACL for '[/]'.
>
>
>>> In 1.9 any repeat acl lines that were the exact same match, such as:
>>>
>>> [/some/path]
>>> user = rw
>>> user = r
>>>
>>> resulted in the last line overriding all the other lines, so user=r in
>>> the example above. In 1.10 the lines combine, so user=rw in the example
>>> above. Is this a bug in 1.10, or a bug in 1.9 that is fixed in 1.10, or
>>> an acceptable behaviour change?
>> Ouch. That is a bad one and an oversight in the design - I think.
>>
>> According to (3), 1.9 behaves correctly. I guess we consider it
>> an unordered collection in 1.10 and then a union is the only thing
>> that approximates intent when a user is a member of different
>> groups with different access rights. Strict ordering becomes
>> very useful when assigning rights to groups:
>>
>> [/some/path]
>> @Users = rw
>> @BadUsers = r
> We already have strict ordering within an ACL (authz_acl_t in
> libsvn_repos/authz.h):
>
> /* All other user- or group-specific access rights.
> Aliases are replaced with their definitions, rules for the same
> user or group are merged. */
> apr_array_header_t *user_access;
>
>
>
> The "merge" semantics was intentional; if we decide it's a bug (and I
> think it is), it's fairly easy to change. I would lean in the direction
> of making repeating the same access entry selection a hard error at
> parsing time. This requires changes to the merge semantics implemented
> in add_access_entry() and merge_alias_ace() in
> libsvn_repos/authz_parse.c. The nice part is that it would catch errors
> like this:
>
> [aliases]
> afoo = foo
> abar = bar
>
> [/]
> &afoo = rw
> foo = r
> ~&abar = rw
> ~bar = r
>
>
> With the current implementation we translate the ACL to:
>
> [/]
> foo = rw
> foo = r
> ~bar = rw
> ~bar = r
>
>
> and even with strict ordering I'd say this is a bug and not intentional.
Note that this should also be an error:
[/]
$anonymous = r
~$authenticated = rw
-- Brane
