Opened 14 years ago
Closed 14 years ago
#9566 closed defect (fixed)
Improper handling of scoped repositories in svn AuthzSourcePolicy
Reported by: | Itamar Oren | Owned by: | Remy Blank |
---|---|---|---|
Priority: | high | Milestone: | 0.12.1 |
Component: | version control | Version: | 0.12 |
Severity: | major | Keywords: | svn authz |
Cc: | Branch: | ||
Release Notes: | |||
API Changes: | |||
Internal Changes: |
Description
The AuthzSourcePolicy component uses the resource.id for matching a repository path in the authz file (trunk/trac/versioncontrol/svn_authz.py?rev=10005#L154), which is only the relative path from the scope in case of scoped repositories.
As a result, it is not possible to reuse the authz file created for serving the SVN repository, which uses full paths.
This can be fixed, at least for SVN-scoped-repositories, by replacing the mentioned line with:
for p in parent_iter(self.env.get_repository(resource.parent.id).scope + resource.id.strip('/')):
Attachments (3)
Change History (12)
comment:1 by , 14 years ago
Milestone: | → 0.12.1 |
---|---|
Owner: | set to |
Priority: | normal → high |
by , 14 years ago
Attachment: | 9566-scoped-repos-r10006.patch added |
---|
follow-up: 3 comment:2 by , 14 years ago
Could you please test 9566-scoped-repos-r10006.patch? It should add support for scoped repositories. It also refactors the permission checking to remove duplicate code.
As CachedRepository
already checks for a .scope
attribute, I wonder if we shouldn't "standardize" this practice by defining a scope = '/'
class attribute in Repository
, so that the scope is always present and "empty" by default?
follow-up: 4 comment:3 by , 14 years ago
Replying to rblank:
Could you please test 9566-scoped-repos-r10006.patch? It should add support for scoped repositories. It also refactors the permission checking to remove duplicate code.
Any chance you could generate the patch using svn instead of git? (or alternatively explain how I apply the patch to a svn working copy)
As
CachedRepository
already checks for a.scope
attribute, I wonder if we shouldn't "standardize" this practice by defining ascope = '/'
class attribute inRepository
, so that the scope is always present and "empty" by default?
+1
comment:4 by , 14 years ago
Replying to itamaro:
Any chance you could generate the patch using svn instead of git? (or alternatively explain how I apply the patch to a svn working copy)
You can apply it normally with patch
:
cd /path/to/working/copy patch -p1 <9566-scoped-repos-r10006.patch
You may have to adjust the end-of-lines if you are on Windows (and find a suitable patch.exe
).
comment:5 by , 14 years ago
OK, it wasn't a svn-git issue with applying the patch, it was a branch-issue. I have trunk, and it appears that the patch is against branches/0.12-stable …
Anyway, I tried taking svn_authz.py from the branch (only that module, without touching any other one) and then applying the patch, which caused an exception on every request, similar to:
Traceback (most recent call last): ... File "c:\var\trac\trac-trunk\trac\versioncontrol\svn_authz.py", line 148, in check_permission if (resource.realm, action) in self._handled_perms: AttributeError: 'NoneType' object has no attribute 'realm'
follow-up: 7 comment:6 by , 14 years ago
Right, I forgot to handle coarse permissions. Please try 9566-scoped-repos-2-r10006.patch.
comment:7 by , 14 years ago
Replying to rblank:
Right, I forgot to handle coarse permissions. Please try 9566-scoped-repos-2-r10006.patch.
That seems to do the trick. This works for me now.
Thanks!
comment:8 by , 14 years ago
Thanks for testing. Patch applied in [10007].
That leaves the question of formalizing the scope, something like 9566-formal-scope-r10007.patch. All tests pass.
comment:9 by , 14 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
9566-formal-scope-r10007.patch applied in [10056].
Support for scoped repositories.