On 02/12/14 16:49, C. Michael Pilato wrote:
On 11/28/2014 12:03 PM, Hannes Reich wrote:
I'd like to suggest an extension to the authz file format to support the
following scenario:
* Some users of the repository should have access to everything, others
should be restricted to a small set of "public" directories.
* All users should be able to check out from the same "root" directory.
I love the idea of the feature, and in fact began at one point in the
past trying to provide similar functionality on the authz-overhaul
branch.
Thanks for the encouragement and background information.
I've attached the patch, which I would describe as a workaround for the
underlying issue the authz-overhaul branch is addressing.
Since the patch lacks authz-overhaul's concept of "list access", the
"ancestor" permission has some side-effects:
- Users can learn of the existence of the siblings of all ancestors of
paths to which they have access (by poking around in .svn/wc.db). This
is suboptimal but acceptable for my use case.
- Users can access the properties of all ancestors of paths to which
they have access. Perhaps this can be construed as a feature since it
enables access to svn:mergeinfo, though I haven't explored how well
merges as a restricted user work in practice.
/Hannes
[[[
Add support for ancestor path permission in authz files
Extends the authz file format with the ability to specify a permission that
should apply to a directory path and all directories above it. This is useful
to permit users to check out from the root of a repository where they have
limited read access.
* subversion/libsvn_repos/authz.c
(authz_lookup_baton): Add a "required_for_ancestor" flag, to distinguish
lookups for permissions that can be applied to ancestors of the specified path.
(authz_parse_line, authz_validate_rule): Implement the new authz syntax:
Permission settings may contain a "a" character, to indicate that the
permission applies to all containing directories.
(authz_parse_section, authz_get_tree_access): Implement a
"required_for_ancestor" code path that searches for lines that grant ancestor
permissions.
(svn_repos_authz_check_access): When checking for non-recursive access
permissions, look for ancestor permission on subpaths.
* subversion/tests/cmdline/authz_tests.py
(authz_recursive_ls): Add a test case featuring ancestor permissions.
* subversion/tests/libsvn_repos/repos-test.c
(authz): Add a test for ancestor permissions.
]]]
Index: subversion/libsvn_repos/authz.c
===================================================================
--- subversion/libsvn_repos/authz.c (revision 1644608)
+++ subversion/libsvn_repos/authz.c (working copy)
@@ -65,6 +65,10 @@ struct authz_lookup_baton {
/* repos_path prefixed by the repository name and a colon. */
const char *qualified_repos_path;
+ /* Whether a recursive lookup is searching for permissions to apply
+ to a parent path. */
+ svn_boolean_t required_for_ancestors;
+
/* Whether, at the end of a recursive lookup, access is granted. */
svn_boolean_t access;
};
@@ -264,6 +268,11 @@ authz_parse_line(const char *name, const char *val
if (!authz_line_applies_to_user(name, b, pool))
return TRUE;
+ /* Stop if we are looking for ancestor permissions, but the rule is
+ not applicable to ancestors. */
+ if (b->required_for_ancestors && !strchr(value, 'a'))
+ return TRUE;
+
/* Set the access grants for the rule. */
if (strchr(value, 'r'))
b->allow |= svn_authz_read;
@@ -303,7 +312,7 @@ static svn_boolean_t
authz_parse_section(const char *section_name, void *baton, apr_pool_t *pool)
{
struct authz_lookup_baton *b = baton;
- svn_boolean_t conclusive;
+ svn_boolean_t conclusive, granted;
/* Does the section apply to us? */
if (!is_applicable_section(b->qualified_repos_path, section_name)
@@ -319,13 +328,22 @@ authz_parse_section(const char *section_name, void
conclusive = authz_access_is_determined(b->allow, b->deny,
b->required_access);
- /* Is access granted OR inconclusive? */
- b->access = authz_access_is_granted(b->allow, b->deny,
- b->required_access)
- || !conclusive;
-
- /* As long as access isn't conclusively denied, carry on. */
- return b->access;
+ /* Is access granted? */
+ granted = authz_access_is_granted(b->allow, b->deny,
+ b->required_access);
+ if (b->required_for_ancestors)
+ {
+ /* Continue as long as we don't find a determined, granted access. */
+ b->access = granted && conclusive;
+ return !b->access;
+ }
+ else
+ {
+ /* Is access granted OR inconclusive? */
+ b->access = granted || !conclusive;
+ /* As long as access isn't conclusively denied, carry on. */
+ return b->access;
+ }
}
@@ -386,6 +404,7 @@ static svn_boolean_t
authz_get_tree_access(svn_config_t *cfg, const char *repos_name,
const char *path, const char *user,
svn_repos_authz_access_t required_access,
+ svn_boolean_t required_for_ancestors,
apr_pool_t *pool)
{
struct authz_lookup_baton baton = { 0 };
@@ -393,11 +412,13 @@ authz_get_tree_access(svn_config_t *cfg, const cha
baton.config = cfg;
baton.user = user;
baton.required_access = required_access;
+ baton.required_for_ancestors = required_for_ancestors;
baton.repos_path = path;
baton.qualified_repos_path = apr_pstrcat(pool, repos_name,
":", path, SVN_VA_NULL);
- /* Default to access granted if no rules say otherwise. */
- baton.access = TRUE;
+ if(!required_for_ancestors)
+ /* Default to access granted if no rules say otherwise. */
+ baton.access = TRUE;
svn_config_enumerate_sections2(cfg, authz_parse_section,
&baton, pool);
@@ -562,7 +583,7 @@ authz_group_walk(svn_config_t *cfg,
* doing it more than once within the one rule, and that it isn't
* "~*", as that would never match.
* - Check that VALUE part of the rule specifies only allowed rule
- * flag characters ('r' and 'w').
+ * flag characters ('r', 'w' and 'a').
*
* Return TRUE if the rule has no errors. Use BATON for context and
* error reporting.
@@ -660,7 +681,8 @@ static svn_boolean_t authz_validate_rule(const cha
while (*val)
{
- if (*val != 'r' && *val != 'w' && ! svn_ctype_isspace(*val))
+ if (*val != 'r' && *val != 'w' && *val != 'a'
+ && ! svn_ctype_isspace(*val))
{
b->err = svn_error_createf(SVN_ERR_AUTHZ_INVALID_CONFIG, NULL,
"The character '%c' in rule '%s' is not "
@@ -1056,7 +1078,7 @@ svn_repos_authz_check_access(svn_authz_t *authz, c
{
/* Deny access by default. */
*access_granted = FALSE;
- return SVN_NO_ERROR;
+ break;
}
/* Work back to the parent path. */
@@ -1068,7 +1090,15 @@ svn_repos_authz_check_access(svn_authz_t *authz, c
to the requested user. */
if (*access_granted && (required_access & svn_authz_recursive))
*access_granted = authz_get_tree_access(authz->cfg, repos_name, path,
- user, required_access, pool);
+ user, required_access, FALSE,
+ pool);
+ /* If the caller did not request recursive access, then a child path
+ could still allow us to grant access. */
+ if (!*access_granted && !(required_access & svn_authz_recursive))
+ *access_granted = authz_get_tree_access(authz->cfg, repos_name, path,
+ user, required_access, TRUE,
+ pool);
+
return SVN_NO_ERROR;
}
Index: subversion/tests/cmdline/authz_tests.py
===================================================================
--- subversion/tests/cmdline/authz_tests.py (revision 1644608)
+++ subversion/tests/cmdline/authz_tests.py (working copy)
@@ -128,7 +128,7 @@ def broken_authz_file(sbox):
sbox.build(create_wc = False)
- # No characters but 'r', 'w', and whitespace are allowed as a value
+ # No characters but 'r', 'w', 'R', and whitespace are allowed as a value
# in an authz rule.
write_authz_file(sbox, {"/": "jrandom = rw # End-line comments disallowed"})
@@ -1143,6 +1143,28 @@ def authz_recursive_ls(sbox):
[], 'ls', '-R',
sbox.repo_url)
+ write_authz_file(sbox, {'/' : '* =',
+ '/A/B/E' : '* =',
+ # These two enable traversal from / to the directory
+ '/A/B' : '* = ra',
+ '/A/D/G/rho' : '* = rwa',
+ # This one does not
+ '/A/D/H/psi' : '* = r',
+ })
+ expected_entries = [
+ 'A/',
+ 'A/B/',
+ 'A/B/F/',
+ 'A/B/lambda',
+ 'A/D/',
+ 'A/D/G/',
+ 'A/D/G/rho',
+ ]
+ svntest.actions.run_and_verify_svn('recursive ls from /',
+ map(lambda x: x + '\n', expected_entries),
+ [], 'ls', '-R',
+ sbox.repo_url)
+
@Issue(3781)
@Skip(svntest.main.is_ra_type_file)
def case_sensitive_authz(sbox):
Index: subversion/tests/libsvn_repos/repos-test.c
===================================================================
--- subversion/tests/libsvn_repos/repos-test.c (revision 1644608)
+++ subversion/tests/libsvn_repos/repos-test.c (working copy)
@@ -1257,6 +1257,10 @@ authz(apr_pool_t *pool)
of subpaths. */
{ "/A/D", "greek", "plato", svn_authz_read | svn_authz_recursive, TRUE },
{ "/A/D", "greek", NULL, svn_authz_read | svn_authz_recursive, FALSE },
+ /* Test that ancestor access grants non-recursive access to
+ containing directories. */
+ { "/H", "greek", NULL, svn_authz_read, TRUE },
+ { "/H", "greek", NULL, svn_authz_read | svn_authz_recursive, FALSE },
/* Test global write access lookups. */
{ NULL, "greek", "plato", svn_authz_read, TRUE },
{ NULL, "greek", NULL, svn_authz_write, FALSE },
@@ -1308,6 +1312,9 @@ authz(apr_pool_t *pool)
"[greek:/A/B/E/beta]" NL
"* =" NL
"" NL
+ "[greek:/H/I/J]" NL
+ "* = ra" NL
+ "" NL
"[/nowhere]" NL
"nobody = r" NL
""
NL;
