Version control folder permissions

[chamith.malinda@…]

Hi,

I want to restrict several folders to several trac users. So I used FineGrainedPermissions.

If we say john not allowed to view the trunk/src/some/location then I put below in the /var/www/trac/conf/authzpolicy.conf file.

[repository:test_repo@*/source:trunk/src/some/location/*@*]
john = !BROWSER_VIEW, !FILE_VIEW

But it not getting effected. (I already give the permissions to group which the john also in for BROWSER_VIEW and FILE_VIEW in admin panel permissions) I need restrict the trunk/src/some/location folder only to john.

[Remy Blank]

You should use AuthzSourcePolicy for controlling access to repositories, not AuthzPolicy.

[Christian Boos]

Theoretically AuthzPolicy should be able to handle this as well…

[theYT <dev@…>]

I think it is needed to check BROWSER_VIEW permission with resource when processing request.

Patch (tested with r12742):

trac/versioncontrol/web_ui/browser.py

@@ -326 +326 @@
             return True
 
     def process_request(self, req):
-        req.perm.require('BROWSER_VIEW')
-
         presel = req.args.get('preselected')
         if presel and (presel + '/').startswith(req.href.browser() + '/'):
             req.redirect(presel)
@@ -389 +387 @@
                                                    version=rev_or_latest))
             display_rev = repos.display_rev
 
+        if node:
+            req.perm(node.resource).require('BROWSER_VIEW')
+        else:
+            req.perm.require('BROWSER_VIEW')
+
         # Prepare template data
         path_links = get_path_links(req.href, reponame, path, rev,
                                     order, desc)

[Ryan J Ollos]

Defect confirmed.

It is not entirely clear to me what the allowed realms should be. On the one hand we have two aliases for source TracLinks: TracLinks. However, browser is not registered as a realm with the ResourceManager: tags/trac-1.0.1/trac/versioncontrol/api.py@:366#L364​.

With the following change,

trac/versioncontrol/web_ui/browser.py

@@ -326 +326 @@
             return True
 
     def process_request(self, req):
-        req.perm.require('BROWSER_VIEW')
-
         presel = req.args.get('preselected')
         if presel and (presel + '/').startswith(req.href.browser() + '/'):
             req.redirect(presel)
@@ -341 +339 @@
         desc = 'desc' in req.args
         xhr = req.get_header('X-Requested-With') == 'XMLHttpRequest'
 
+        req.perm('source', path).require('BROWSER_VIEW')
+
         rm = RepositoryManager(self.env)
         all_repositories = rm.get_all_repositories()
         reponame, repos, path = rm.get_repository_by_path(path)

we get a PermissionError with a proper message:

BROWSER_VIEW privileges are required to perform this operation on path /teo/trac. You don't have the required permissions.

If we change the realm of the permission check to the repository realm the message isn't quite as good:

BROWSER_VIEW privileges are required to perform this operation on Repository /teo/trac. You don't have the required permissions.

If browser was returned by the ResourceManager, we could check all 3 realms:

trac/versioncontrol/web_ui/browser.py

@@ -326 +326 @@
             return True
 
     def process_request(self, req):
-        req.perm.require('BROWSER_VIEW')
-
         presel = req.args.get('preselected')
         if presel and (presel + '/').startswith(req.href.browser() + '/'):
             req.redirect(presel)
@@ -342 +340 @@
         xhr = req.get_header('X-Requested-With') == 'XMLHttpRequest'
 
         rm = RepositoryManager(self.env)
+        for realm in rm.get_resource_realms():
+            req.perm(realm, path).require('BROWSER_VIEW')
+
         all_repositories = rm.get_all_repositories()
         reponame, repos, path = rm.get_repository_by_path(path)

However, I tend to think we should just use source as the realm for TracFineGrainedPermissions checks. Please let me know if you have other opinions on how the fix should be implemented.

[Jun Omae]

We could use is_viewable() method of Repository and Node classes to check viewable rather than directly using req.perm and the realms.

See tags/trac-1.0.1/trac/versioncontrol/api.py#L960​ and #L1078​.

trac/versioncontrol/web_ui/browser.py

@@ -24 +24 @@
 from trac.config import ListOption, BoolOption, Option
 from trac.core import *
 from trac.mimeview.api import IHTMLPreviewAnnotator, Mimeview, is_binary
-from trac.perm import IPermissionRequestor
+from trac.perm import IPermissionRequestor, PermissionError
 from trac.resource import Resource, ResourceNotFound
 from trac.util import as_bool, embedded_numbers
 from trac.util.datefmt import http_date, to_datetime, utc
@@ -326 +326 @@
             return True
 
     def process_request(self, req):
-        req.perm.require('BROWSER_VIEW')
-
         presel = req.args.get('preselected')
         if presel and (presel + '/').startswith(req.href.browser() + '/'):
             req.redirect(presel)
@@ -355 +353 @@
         if not repos and reponame:
             raise ResourceNotFound(_("Repository '%(repo)s' not found",
                                      repo=reponame))
+        if not repos.is_viewable(req.perm):
+            raise PermissionError('BROWSER_VIEW', repos.resource, self.env)
 
         if reponame and reponame != repos.reponame: # Redirect alias
             qs = req.query_string
@@ -398 +398 @@
             repo_data = self._render_repository_index(
                                         context, all_repositories, order, desc)
         if node:
+            if not node.is_viewable(req.perm):
+                raise PermissionError('BROWSER_VIEW' if node.isdir else
+                                      'FILE_VIEW', node.resource, self.env)
             if node.isdir:
                 if format in ('zip',): # extension point here...
                     self._render_zip(req, context, repos, node, rev)

[Ryan J Ollos]

Using the is_viewable methods looks like the way to go. Please feel free to take control of the ticket and push the change.

[Jun Omae]

Okay. I'll push the patch with tests.

[Jun Omae]

Proposed changes in jomae.git@t10961_1.0.

BTW, I noticed that authz policy doesn't work for default repository with like this. That's a defect?

[repository:@*]
foobar = !BROWSER_VIEW, !FILE_VIEW

[repository:(default)@*]
foobar = !BROWSER_VIEW, !FILE_VIEW

The following works for default repository but makes all repositories to be not viewable.

[repository:*@*]
foobar = !BROWSER_VIEW, !FILE_VIEW

[Jun Omae]

Committed in [12814] and merged to trunk in [12815].