Edgewall Software
Modify

Opened 10 years ago

Closed 8 years ago

Last modified 5 years ago

#8289 closed enhancement (fixed)

Make SVN Authz compatible with SVN 1.5

Reported by: alvaro.iradier@… Owned by: Remy Blank
Priority: normal Milestone: 0.12.1
Component: version control Version: 0.11.4
Severity: major Keywords: svnauthz, authzsourcepolicy
Cc:
Release Notes:
API Changes:

Description

SVN 1.5 adds $authenticated and $anonymous tokens to the svn auth file. This should be reflected on the RealSubversionAuthorizer class in svn_authz.py.

I'm attaching a suggested patch.

I think this should be applied in trac 0.11.5, no need to wait for 0.12.

Attachments (2)

svn_authz_svn15.patch (938 bytes ) - added by alvaro.iradier@… 10 years ago.
Patch to svn_auth.py to understand new svn 1.5 tokens $anonymous and $authenticated
svn_authz_svn13.patch (692 bytes ) - added by alvaro.iradier@… 10 years ago.
Patch to svn_auth.py to understand new svn 1.3 tokens $anonymous and $authenticated (same as previous, in uniffied diff)

Download all attachments as: .zip

Change History (23)

Changed 10 years ago by alvaro.iradier@…

Attachment: svn_authz_svn15.patch added

Patch to svn_auth.py to understand new svn 1.5 tokens $anonymous and $authenticated

comment:1 Changed 10 years ago by Christian Boos

Please use unified diff format (diff -u), see TracDev/SubmittingPatches.

comment:2 Changed 10 years ago by ebray

Note also that there's been a ticket #4997 for a while now to just use SVN's authz bindings. The argument against it at the time was that the relevant functions were not available prior to SVN 1.3. Is that really still an issue? Maybe for newer SVN versions Trac could use the available bindings—that's what I've been doing for years, rather than reimplement every new authz feature added to Subversion.

comment:3 Changed 10 years ago by Remy Blank

Then there's also comment:19:ticket:5640, where we were discussing if now would be the right time to drop the authz system altogether in favor of fine-grained permissions.

comment:4 in reply to:  3 ; Changed 10 years ago by ebray

Replying to rblank:

Then there's also comment:19:ticket:5640, where we were discussing if now would be the right time to drop the authz system altogether in favor of fine-grained permissions.

I don't understand. You still need an authz file for the SVN repository. I'd rather Trac just take repository permissions from that.

comment:5 in reply to:  4 ; Changed 10 years ago by Christian Boos

Replying to ebray:

Replying to rblank:

Then there's also comment:19:ticket:5640, where we were discussing if now would be the right time to drop the authz system altogether in favor of fine-grained permissions.

I don't understand. You still need an authz file for the SVN repository. I'd rather Trac just take repository permissions from that.

To clarify: the authz file from Subversion would still be used to perform the same checks (minus the bugs if possible ;-) ), but for doing so we would use a new permission policy (e.g. SvnAuthzPolicy) which would apply the checks at a higher level rather than directly within the svn_fs layer.

The advantages of this are clear:

  • only one system for TracFineGrainedPermissions
  • possibility to re-use the SvnAuthzPolicy for other vcs than just svn
  • possibility to write other dedicated policies
  • it should also be possible to use the authz_policy.py plugin or any other generic permission policy for enforcing permissions in the source browser

comment:6 in reply to:  5 Changed 10 years ago by Remy Blank

Replying to cboos:

  • possibility to re-use the SvnAuthzPolicy for other vcs than just svn

This may be a reason not to use the authz parser from Subversion, to avoid a dependency on the SVN bindings when SVN is not used. OTOH, if the same effect can be achieved with authz_policy.py, I'd be in favor of #4997.

comment:7 Changed 10 years ago by Christian Boos

#4997 could be optional - full support and compatibility with Subversion, at the cost of the dependency on the bindings (this also solves the backward compat issue, in case this is still a problem). It might be interesting to do some benchmarking as well and see which from the python implementation and the bindings one is most effective.

comment:8 in reply to:  5 Changed 10 years ago by ebray

Replying to cboos:

To clarify: the authz file from Subversion would still be used to perform the same checks (minus the bugs if possible ;-) ), but for doing so we would use a new permission policy (e.g. SvnAuthzPolicy) which would apply the checks at a higher level rather than directly within the svn_fs layer.

I see now. In that case I'm all for it, since we long since did something similar: Added SVN_READ and SVN_WRITE permissions to our permission system, and are just kept in sync with the SVN authz file.

As for #4997, making it optional would definitely make sense in this context.

Changed 10 years ago by alvaro.iradier@…

Attachment: svn_authz_svn13.patch added

Patch to svn_auth.py to understand new svn 1.3 tokens $anonymous and $authenticated (same as previous, in uniffied diff)

comment:9 Changed 10 years ago by alvaro.iradier@…

I've attached same patch in uniffied, sorry.

Also check attached patch in 4997 as alternative.

comment:10 Changed 10 years ago by alvaro.iradier@…

I found another problem with current authz file parsing that is fixed when using the svn bindings:

Windows is not case-sensitive, so permissions are applied to, for example, [/secure] even if the directory is named /Secure (uppercase S).

However, this won't work on trac, so if there exist case differences between the authz file specifications and the filesystem, permissions won't be applied. This is correct for linux/unix, but not for windows.

Using SWIG bindings like the attached patch fixes the problem.

comment:11 in reply to:  10 Changed 10 years ago by Christian Boos

Replying to alvaro.iradier@…:

However, this won't work on trac, so if there exist case differences between the authz file specifications and the filesystem, permissions won't be applied. This is correct for linux/unix, but not for windows.

Using SWIG bindings like the attached patch fixes the problem.

I'd rather see that as a bug in the bindings, as the case sensitiveness of the filesystem of the OS where Trac runs is irrelevant to the way the repository is presented.

To convince yourself of that, just try the following (on Windows):

C:\Temp>svnadmin create testrep
C:\Temp>mkdir test
C:\Temp>cd test
C:\Temp\test>mkdir A
C:\Temp\test>svn import file:///C:/Temp/testrep -m "Add A"
Adding         A

Committed revision 1.

C:\Temp\test>rmdir A
C:\Temp\test>mkdir a
C:\Temp\test>svn import file:///C:/Temp/testrep -m "Add a"
Adding         a

Committed revision 2.

C:\Temp\test>svn ls file:///C:/Temp/testrep
A/
a/

So even on Windows, you can create and manipulate repositories containing mixed cases entries. The only thing you can't do obviously is to checkout the directory containing those conflicting entries, but nothing prevents you from checking them out separately.

Back to the topic, I don't think it's legitimate to consider that the [/secure] entry also applies to [/Secure], even if the repository happens to be hosted on a Windows machine.

comment:12 Changed 9 years ago by Christian Boos

Milestone: 0.11.60.13

comment:13 Changed 9 years ago by Christian Boos

Keywords: svnauthz added; svn authz removed

comment:14 Changed 8 years ago by Remy Blank

Milestone: next-major-0.1X0.13
Owner: set to Remy Blank

I'll do this.

comment:15 Changed 8 years ago by Remy Blank

#4997 was closed as a duplicate.

comment:16 Changed 8 years ago by Remy Blank

#9542 has a patch that adds the $anonymous and $authenticated tokens.

comment:17 Changed 8 years ago by Remy Blank

It looks like SVN also supports "negation rules", by prefixing a key with ~.

Does anybody know where this is documented? I couldn't find anything except the changeset that adds the feature.

comment:18 in reply to:  17 Changed 8 years ago by Remy Blank

Replying to rblank:

Does anybody know where this is documented?

Oh, great, the only place this seems to be documented is in the sample conf/authz file of a newly-created repository:

### As shown below each section defines authorizations for the path and
### (optional) repository specified by the section name.
### The authorizations follow. An authorization line can refer to:
###  - a single user,
###  - a group of users defined in a special [groups] section,
###  - an alias defined in a special [aliases] section,
###  - all authenticated users, using the '$authenticated' token,
###  - only anonymous users, using the '$anonymous' token,
###  - anyone, using the '*' wildcard.
###
### A match can be inverted by prefixing the rule with '~'. Rules can
### grant read ('r') access, read-write ('rw') access, or no access
### ('').

comment:19 Changed 8 years ago by Remy Blank

Milestone: 0.130.12.1
Resolution: fixed
Status: newclosed

$anonymous and $authenticated tokens implemented in [10006].

comment:20 Changed 8 years ago by Remy Blank

FWIW, the negation rules seem tricky to implement, so I'm waiting for a concrete request for the functionality before implementing it. Yes, I'm lazy.

comment:21 Changed 5 years ago by Ryan J Ollos

Keywords: authzsourcepolicy added

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Remy Blank.
The resolution will be deleted.
to The owner will be changed from Remy Blank to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.