On 19.01.2019 21:12, Branko Čibej wrote: > On 19.01.2019 17:17, Branko Čibej wrote: >> Hi Doug! >> >> On 18.01.2019 23:07, Doug Robinson wrote: >>> Honored committers (and the rest of us): >>> >>> It's come to my attention that if a group is defined in an AuthZ >>> file without an associated account that SVN is, as of 1.10, generating >>> an error and failing to allow the use of that AuthZ file. >>> >>> Example: >>> >>> [groups] >>> goodGroup = acct1 >>> goodGroup2 = acct1, acct2 >>> badGroup = >>> >>> [repoName:/someplace] >>> @badGroup = rw >>> >>> svnauthz: E220003: Error while parsing authz file: ... >>> svnauthz: E220003: Access entry refers to undefined group ... >> >> >> Thanks for bringing this up. It's most definitely a bug ... the group >> is defined, it's just empty. If the parser accepts an empty group >> definition, it should also allow its use in rules. That ACE should >> just be ignored, since it has no effect. >> >> Emitting a warning from svnauthz would be a bonus, but I'm not sure >> offhand how to do that, as I don't think we have a warning callback >> in the parser; it might involve some API changes, but nothing drastic. >> >> Can you please write this up as a Jira issue and assign it to me? >> >>> My thoughts: >>> >>> 1. From a compatibility standpoint it really should be a Warning, >>> not an Error. If there's no accounts then certainly it can have >>> no impact on the security of the repository/ies. >> >> And further, if one generates the 'groups' file from LDAP, for >> example, some unintentional intermediate state could break Subversion >> authz even though there's nothing specifically wrong with it. >> >>> 2. From a usability standpoint it really should simply be supported. >>> The AuthZ file is a representation of a team structure. There are >>> times when teams will get reduced headcount down to zero and then >>> back up again. To deal with that use case with SVN 1.10 means >>> either: >>> >>> a) stripping out all references to the team and losing all of the >>> places where that team requires access >>> >>> b) configuring a dummy account for the team and hoping that the >>> account will never be created >>> >>> c) leaving the team around and fixing SVN to allow an empty team >>> >>> My preference would be first 2c and, if not, then 1. But that's >>> me. >>> >>> Not sure about the history of why this change was made? I'd like >>> to better understand. >> >> It's an unintentional consequence of the parser rewrite. It appears >> that we don't have a test for this case, and code inspection didn't >> catch the change in semantics ... and so it landed in released code. >> >> This is about to change. :) > > Fixed in r1851687; I'll nominate this for backport to 1.10.x and 1.11.x. > > Emitting a warning from svnauthz requires a public API change that > can't be released before 1.12, so please do write that Jira issue.
r1851823
