On 02.12.2018 08:43, Branko Čibej wrote: > 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
I have a patch ready, here are some examples of what it does (currently, all these examples are valid and produce merged access rights): $ cat authz.conf [/] user = rw user = r $ svnauthz validate authz.conf svnauthz: E220003: Error while parsing authz file: 'authz.conf': svnauthz: E220003: Duplicate access entry 'user' in rule [/] $ cat authz.conf [/] $authenticated = rw ~$anonymous = r $ svnauthz validate authz.conf svnauthz: E220003: Error while parsing authz file: 'authz.conf': svnauthz: E220003: Duplicate access entry '~$anonymous' (matches '$authenticated') in rule [/] $ cat authz.conf [aliases] resu = user [/] ~&resu = rw ~user = r $ svnauthz validate authz.conf svnauthz: E220003: Error while parsing authz file: 'authz.conf': svnauthz: E220003: Duplicate access entry '~&resu' (matches '~user') in rule [/] -- Brane
