On Tue, Oct 23, 2012 at 4:23 PM, C. Michael Pilato <cmpil...@collab.net> wrote:
> On 10/23/2012 07:24 AM, Ivan Zhakov wrote:
>> I'm working on the patch to list only readable repositories. There is
>> already TODO comment in the code by cmpilato:
>> subversion\mod_dav_svn\repos.c:3461
>> [[[
>> /* ### TODO: We could test for readability of the root
>> directory of each repository and hide those that
>> the user can't see. */
>> ]]]
>
> I, too, started looking into this, Ivan, but I realized that I was probably
> about to run into a whole mess of code refactoring that I wasn't really up
> for dealing with at the time. (Trying to stay as 1.8-focused as I can.)
> I'm happy to review any work you do on this issue, though.
>
Hi Mike,
Please find attached patch to hide unreadable repositories in
"Collection of Repositories":
[[[
mod_dav_svn: Hide repositories from list that are not accessible for user.
* subversion/mod_dav_svn/authz.c
* subversion/mod_dav_svn/dav_svn.h
(dav_svn__allow_list_repos): New.
* subversion/mod_dav_svn/repos.c
(deliver): Check for readability of the root directory of each
repository and hide those that the user can't see.
]]]
Code in deliver() method is not best now, but I was trying to minimize
changes in my patch. I'm going to refactor code later after committing
my patch.
Looking forward for your review. Thanks!
--
Ivan Zhakov
Index: subversion/mod_dav_svn/authz.c
===================================================================
--- subversion/mod_dav_svn/authz.c (revision 1403730)
+++ subversion/mod_dav_svn/authz.c (working copy)
@@ -97,6 +97,55 @@
}
+svn_boolean_t
+dav_svn__allow_list_repos(request_rec *r,
+ const char *repos_name,
+ apr_pool_t *pool)
+{
+ const char *uri;
+ request_rec *subreq;
+ svn_boolean_t allowed = FALSE;
+ authz_svn__subreq_bypass_func_t allow_read_bypass = NULL;
+
+ /* Easy out: if the admin has explicitly set 'SVNPathAuthz Off',
+ then this whole callback does nothing. */
+ if (! dav_svn__get_pathauthz_flag(r))
+ {
+ return TRUE;
+ }
+
+ /* If bypass is specified and authz has exported the provider.
+ Otherwise, we fall through to the full version. This should be
+ safer than allowing or disallowing all accesses if there is a
+ configuration error.
+ XXX: Is this the proper thing to do in this case? */
+ allow_read_bypass = dav_svn__get_pathauthz_bypass(r);
+ if (allow_read_bypass != NULL)
+ {
+ if (allow_read_bypass(r, "/", repos_name) == OK)
+ return TRUE;
+ else
+ return FALSE;
+ }
+
+ /* Build a Public Resource uri representing repository root. */
+ uri = svn_urlpath__join(dav_svn__get_root_dir(r),
+ svn_path_uri_encode(repos_name, pool), pool);
+
+ /* Check if GET would work against this uri. */
+ subreq = ap_sub_req_method_uri("GET", uri, r, r->output_filters);
+
+ if (subreq)
+ {
+ if (subreq->status == HTTP_OK)
+ allowed = TRUE;
+
+ ap_destroy_sub_req(subreq);
+ }
+
+ return allowed;
+}
+
/* This function implements 'svn_repos_authz_func_t', specifically
for read authorization.
Index: subversion/mod_dav_svn/dav_svn.h
===================================================================
--- subversion/mod_dav_svn/dav_svn.h (revision 1403730)
+++ subversion/mod_dav_svn/dav_svn.h (working copy)
@@ -746,6 +746,20 @@
apr_pool_t *pool);
+/* Return TRUE if the current user (as determined by Apache's
+ authentication system) has permission to read repository REPOS_NAME.
+ This will invoke any authz modules loaded into Apache unless this
+ Subversion location has been configured to bypass those in favor of a
+ direct lookup in the Subversion authz subsystem. Use POOL for any
+ temporary allocation.
+ IMPORTANT: R must be request for DAV_SVN_RESTYPE_PARENTPATH_COLLECTION
+ resource.
+*/
+svn_boolean_t
+dav_svn__allow_list_repos(request_rec *r,
+ const char *repos_name,
+ apr_pool_t *pool);
+
/* If authz is enabled in the specified BATON, return a read authorization
function. Otherwise, return NULL. */
svn_repos_authz_func_t
Index: subversion/mod_dav_svn/repos.c
===================================================================
--- subversion/mod_dav_svn/repos.c (revision 1403730)
+++ subversion/mod_dav_svn/repos.c (working copy)
@@ -3469,9 +3469,9 @@
}
else
{
- /* ### TODO: We could test for readability of the root
- directory of each repository and hide those that
- the user can't see. */
+ if (! dav_svn__allow_list_repos(resource->info->r,
+ entry->name, entry_pool))
+ continue;
}
/* append a trailing slash onto the name for directories. we NEED
