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