Author: stsp
Date: Thu Oct 1 19:36:35 2020
New Revision: 1882186
URL: http://svn.apache.org/viewvc?rev=1882186&view=rev
Log:
Fix issue #4762 "authz doesn't combine global and repository rules"
The new authz implementation of SVN 1.10 introduced an incompatible change
in the interpretation of authz rules: Global rules access were not
considered if per-repository access rules were also supplied.
This change seems entirely unnecessary and is still causing problems today
for deployments which upgrade from earlier versions, such as from SVN 1.8.
Existing authz files no longer produce expected results and adjusting
large existing authz rule files to avoid this problem is a lot of work.
* subversion/libsvn_repos/authz.c
(authz_check_access): New helper function, extracted from ...
(svn_repos_authz_check_access): ... here. Always consider the global
rule set in addition to the repository-specific one. The results
from both rulesets are now merged as was the case before SVN 1.10.
* subversion/tests/cmdline/svnauthz_tests.py
(svnauthz_accessof_file_test,
svnauthz_accessof_repo_test,
svnauthz_accessof_groups_file_test,
svnauthz_accessof_groups_repo_test,
svnauthz_accessof_is_file_test,
svnauthz_accessof_is_repo_test): Adjust test expectations accordingly.
* subversion/tests/libsvn_repos/authz-test.c
(reposful_reposless_stanzas_inherit): Remove XFAIL marker.
* subversion/tests/libsvn_repos/repos-test.c
(test_authz_prefixes): Adjust test expectations. This test shows that
the behaviour change was likely deliberate. This test assumed that
global rules would only apply if listed after per-repository rules.
Change test expectations such that global rules are always taken
into account.
Modified:
subversion/trunk/subversion/libsvn_repos/authz.c
subversion/trunk/subversion/tests/cmdline/svnauthz_tests.py
subversion/trunk/subversion/tests/libsvn_repos/authz-test.c
subversion/trunk/subversion/tests/libsvn_repos/repos-test.c
Modified: subversion/trunk/subversion/libsvn_repos/authz.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/authz.c?rev=1882186&r1=1882185&r2=1882186&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_repos/authz.c (original)
+++ subversion/trunk/subversion/libsvn_repos/authz.c Thu Oct 1 19:36:35 2020
@@ -1674,23 +1674,13 @@ svn_repos_authz_parse2(svn_authz_t **aut
return SVN_NO_ERROR;
}
-svn_error_t *
-svn_repos_authz_check_access(svn_authz_t *authz, const char *repos_name,
- const char *path, const char *user,
- svn_repos_authz_access_t required_access,
- svn_boolean_t *access_granted,
- apr_pool_t *pool)
+static svn_error_t *
+authz_check_access(svn_authz_t *authz, const char *path,
+ authz_user_rules_t *rules,
+ svn_repos_authz_access_t required_access,
+ authz_access_t required,
+ svn_boolean_t *access_granted, apr_pool_t *pool)
{
- const authz_access_t required =
- ((required_access & svn_authz_read ? authz_access_read_flag : 0)
- | (required_access & svn_authz_write ? authz_access_write_flag : 0));
-
- /* Pick or create the suitable pre-filtered path rule tree. */
- authz_user_rules_t *rules = get_user_rules(
- authz,
- (repos_name ? repos_name : AUTHZ_ANY_REPOSITORY),
- user);
-
/* In many scenarios, users have uniform access to a repository
* (blanket access or no access at all).
*
@@ -1736,3 +1726,39 @@ svn_repos_authz_check_access(svn_authz_t
return SVN_NO_ERROR;
}
+
+svn_error_t *
+svn_repos_authz_check_access(svn_authz_t *authz, const char *repos_name,
+ const char *path, const char *user,
+ svn_repos_authz_access_t required_access,
+ svn_boolean_t *access_granted,
+ apr_pool_t *pool)
+{
+ svn_error_t *err;
+ const authz_access_t required =
+ ((required_access & svn_authz_read ? authz_access_read_flag : 0)
+ | (required_access & svn_authz_write ? authz_access_write_flag : 0));
+ authz_user_rules_t *rules;
+ svn_boolean_t access_granted_by_any = FALSE;
+ svn_boolean_t access_granted_by_repos = FALSE;
+
+ *access_granted = FALSE;
+
+ rules = get_user_rules(authz, AUTHZ_ANY_REPOSITORY, user);
+ err = authz_check_access(authz, path, rules, required_access, required,
+ &access_granted_by_any, pool);
+ if (err)
+ return err;
+
+ if (repos_name)
+ {
+ rules = get_user_rules(authz, repos_name, user);
+ err = authz_check_access(authz, path, rules, required_access, required,
+ &access_granted_by_repos, pool);
+ if (err)
+ return err;
+ }
+
+ *access_granted = (access_granted_by_any || access_granted_by_repos);
+ return SVN_NO_ERROR;
+}
Modified: subversion/trunk/subversion/tests/cmdline/svnauthz_tests.py
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/svnauthz_tests.py?rev=1882186&r1=1882185&r2=1882186&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/svnauthz_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/svnauthz_tests.py Thu Oct 1
19:36:35 2020
@@ -224,8 +224,8 @@ def svnauthz_accessof_file_test(sbox):
svntest.actions.run_and_verify_svnauthz(["r\n"], None, 0, False, "accessof",
authz_path, "--path", "/jokes")
- # Anonymous access on /jokes on slapstick repo should be no
- svntest.actions.run_and_verify_svnauthz(["no\n"], None, 0, False, "accessof",
+ # Anonymous access on /jokes on slapstick repo should be r via rules on [/]
+ svntest.actions.run_and_verify_svnauthz(["r\n"], None, 0, False, "accessof",
authz_path, "--path", "/jokes",
"--repository", "slapstick")
@@ -289,8 +289,8 @@ def svnauthz_accessof_repo_test(sbox):
svntest.actions.run_and_verify_svnauthz(["r\n"], None, 0, False, "accessof",
authz_url, "--path", "/jokes")
- # Anonymous access on /jokes on slapstick repo should be no
- svntest.actions.run_and_verify_svnauthz(["no\n"], None, 0, False, "accessof",
+ # Anonymous access on /jokes on slapstick repo should be r via rules on [/]
+ svntest.actions.run_and_verify_svnauthz(["r\n"], None, 0, False, "accessof",
authz_url, "--path", "/jokes",
"--repository", "slapstick")
@@ -356,8 +356,8 @@ def svnauthz_accessof_groups_file_test(s
"--repository", "comedy")
# User stafford (@musicians) specified on /jokes with the repo comedy
- # will be no.
- svntest.actions.run_and_verify_svnauthz(["no\n"], None,
+ # will be rw.
+ svntest.actions.run_and_verify_svnauthz(["rw\n"], None,
0, False, "accessof", authz_path,
"--groups-file", groups_path,
"--path", "jokes",
@@ -440,8 +440,8 @@ def svnauthz_accessof_groups_repo_test(s
"--repository", "comedy")
# User stafford (@musicians) specified on /jokes with the repo comedy
- # will be no.
- svntest.actions.run_and_verify_svnauthz(["no\n"], None,
+ # will be rw via rules on [/].
+ svntest.actions.run_and_verify_svnauthz(["rw\n"], None,
0, False, "accessof", authz_url,
"--groups-file", groups_url,
"--path", "jokes",
@@ -508,19 +508,19 @@ def svnauthz_accessof_is_file_test(sbox)
authz_path, "--path", "/jokes",
"--is", "no")
- # Anonymous access on /jokes on slapstick repo should be no
- # Test --is no returns 0.
+ # Anonymous access on /jokes on slapstick repo should be r via rules on [/]
+ # Test --is r returns 0.
svntest.actions.run_and_verify_svnauthz(None, None, 0, False, "accessof",
authz_path, "--path", "/jokes",
"--repository", "slapstick",
- "--is", "no")
+ "--is", "r")
# Test --is rw returns 3.
svntest.actions.run_and_verify_svnauthz(None, None, 3, False, "accessof",
authz_path, "--path", "/jokes",
"--repository", "slapstick",
"--is", "rw")
- # Test --is r returns 3.
- svntest.actions.run_and_verify_svnauthz(None, None, 3, False, "accessof",
+ # Test --is r returns 0.
+ svntest.actions.run_and_verify_svnauthz(None, None, 0, False, "accessof",
authz_path, "--path", "/jokes",
"--repository", "slapstick",
"--is", "r")
@@ -666,19 +666,19 @@ def svnauthz_accessof_is_repo_test(sbox)
authz_url, "--path", "/jokes",
"--is", "no")
- # Anonymous access on /jokes on slapstick repo should be no
- # Test --is no returns 0.
+ # Anonymous access on /jokes on slapstick repo should be r via rules on [/]
+ # Test --is r returns 0.
svntest.actions.run_and_verify_svnauthz(None, None, 0, False, "accessof",
authz_url, "--path", "/jokes",
"--repository", "slapstick",
- "--is", "no")
+ "--is", "r")
# Test --is rw returns 3.
svntest.actions.run_and_verify_svnauthz(None, None, 3, False, "accessof",
authz_url, "--path", "/jokes",
"--repository", "slapstick",
"--is", "rw")
- # Test --is r returns 3.
- svntest.actions.run_and_verify_svnauthz(None, None, 3, False, "accessof",
+ # Test --is r returns 0.
+ svntest.actions.run_and_verify_svnauthz(None, None, 0, False, "accessof",
authz_url, "--path", "/jokes",
"--repository", "slapstick",
"--is", "r")
Modified: subversion/trunk/subversion/tests/libsvn_repos/authz-test.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_repos/authz-test.c?rev=1882186&r1=1882185&r2=1882186&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_repos/authz-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_repos/authz-test.c Thu Oct 1
19:36:35 2020
@@ -522,7 +522,7 @@ static struct svn_test_descriptor_t test
"test svn_authz__get_global_rights"),
SVN_TEST_PASS2(issue_4741_groups,
"issue 4741 groups"),
- SVN_TEST_XFAIL2(reposful_reposless_stanzas_inherit,
+ SVN_TEST_PASS2(reposful_reposless_stanzas_inherit,
"[foo:/] inherits [/]"),
SVN_TEST_NULL
};
Modified: subversion/trunk/subversion/tests/libsvn_repos/repos-test.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_repos/repos-test.c?rev=1882186&r1=1882185&r2=1882186&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_repos/repos-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_repos/repos-test.c Thu Oct 1
19:36:35 2020
@@ -1601,20 +1601,7 @@ test_authz_prefixes(apr_pool_t *pool)
enum { PATH_COUNT = 2 };
const char *test_paths[PATH_COUNT] = { "/", "/A" };
- /* Definition of the paths to test and expected replies for each. */
- struct check_access_tests test_set1[] = {
- /* Test that read rules are correctly used. */
- { "", "greek", NULL, svn_authz_read, FALSE },
- /* Test that write rules are correctly used. */
- { "", "greek", "plato", svn_authz_read, TRUE },
- { "", "greek", "plato", svn_authz_write, FALSE },
- /* Sentinel */
- { NULL, NULL, NULL, svn_authz_none, FALSE }
- };
-
- /* To be used when global rules are specified after per-repos rules.
- * In that case, the global rules still win. */
- struct check_access_tests test_set2[] = {
+ struct check_access_tests test_set[] = {
/* Test that read rules are correctly used. */
{ "", "greek", NULL, svn_authz_read, TRUE },
{ "", "greek", NULL, svn_authz_write, FALSE },
@@ -1634,7 +1621,6 @@ test_authz_prefixes(apr_pool_t *pool)
const char *repo1 = (combi & 4) ? "greek:" : "";
const char *repo2 = (combi & 4) ? "" : "greek:";
const char *test_path = test_paths[combi / 8];
- struct check_access_tests *test_set = (combi & 4) ? test_set2 :
test_set1;
/* Create and parse the authz rules. */
svn_pool_clear(iterpool);
