You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2020/10/08 12:00:29 UTC

svn commit: r1882319 - in /subversion/trunk/subversion: libsvn_repos/authz.c tests/cmdline/svnauthz_tests.py tests/libsvn_repos/authz-test.c tests/libsvn_repos/repos-test.c

Author: stsp
Date: Thu Oct  8 12:00:28 2020
New Revision: 1882319

URL: http://svn.apache.org/viewvc?rev=1882319&view=rev
Log:
Revert r1882186.

The change introduced a regression where behaviour of trunk differs
from both 1.9 and 1.10:
http://mail-archives.apache.org/mod_mbox/subversion-dev/202010.mbox/%3C20201006162606.GA86427%40byrne.stsp.name%3E

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=1882319&r1=1882318&r2=1882319&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_repos/authz.c (original)
+++ subversion/trunk/subversion/libsvn_repos/authz.c Thu Oct  8 12:00:28 2020
@@ -1674,13 +1674,23 @@ svn_repos_authz_parse2(svn_authz_t **aut
   return SVN_NO_ERROR;
 }
 
-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)
+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)
 {
+  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).
    *
@@ -1726,39 +1736,3 @@ authz_check_access(svn_authz_t *authz, c
 
   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=1882319&r1=1882318&r2=1882319&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/svnauthz_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/svnauthz_tests.py Thu Oct  8 12:00:28 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 r via rules on [/]
-  svntest.actions.run_and_verify_svnauthz(["r\n"], None, 0, False, "accessof",
+  # Anonymous access on /jokes on slapstick repo should be no
+  svntest.actions.run_and_verify_svnauthz(["no\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 r via rules on [/]
-  svntest.actions.run_and_verify_svnauthz(["r\n"], None, 0, False, "accessof",
+  # Anonymous access on /jokes on slapstick repo should be no
+  svntest.actions.run_and_verify_svnauthz(["no\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 rw.
-  svntest.actions.run_and_verify_svnauthz(["rw\n"], None,
+  # will be no.
+  svntest.actions.run_and_verify_svnauthz(["no\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 rw via rules on [/].
-  svntest.actions.run_and_verify_svnauthz(["rw\n"], None,
+  # will be no.
+  svntest.actions.run_and_verify_svnauthz(["no\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 r via rules on [/]
-  # Test --is r returns 0.
+  # Anonymous access on /jokes on slapstick repo should be no
+  # Test --is no returns 0.
   svntest.actions.run_and_verify_svnauthz(None, None, 0, False, "accessof",
                                           authz_path, "--path", "/jokes",
                                           "--repository", "slapstick",
-                                          "--is", "r")
+                                          "--is", "no")
   # 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 0.
-  svntest.actions.run_and_verify_svnauthz(None, None, 0, False, "accessof",
+  # Test --is r returns 3.
+  svntest.actions.run_and_verify_svnauthz(None, None, 3, 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 r via rules on [/]
-  # Test --is r returns 0.
+  # Anonymous access on /jokes on slapstick repo should be no
+  # Test --is no returns 0.
   svntest.actions.run_and_verify_svnauthz(None, None, 0, False, "accessof",
                                           authz_url, "--path", "/jokes",
                                           "--repository", "slapstick",
-                                          "--is", "r")
+                                          "--is", "no")
   # 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 0.
-  svntest.actions.run_and_verify_svnauthz(None, None, 0, False, "accessof",
+  # Test --is r returns 3.
+  svntest.actions.run_and_verify_svnauthz(None, None, 3, 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=1882319&r1=1882318&r2=1882319&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_repos/authz-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_repos/authz-test.c Thu Oct  8 12:00:28 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_PASS2(reposful_reposless_stanzas_inherit,
+    SVN_TEST_XFAIL2(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=1882319&r1=1882318&r2=1882319&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_repos/repos-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_repos/repos-test.c Thu Oct  8 12:00:28 2020
@@ -1601,7 +1601,20 @@ test_authz_prefixes(apr_pool_t *pool)
   enum { PATH_COUNT = 2 };
   const char *test_paths[PATH_COUNT] = { "/", "/A" };
 
-  struct check_access_tests test_set[] = {
+  /* 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[] = {
     /* Test that read rules are correctly used. */
     { "", "greek", NULL, svn_authz_read, TRUE },
     { "", "greek", NULL, svn_authz_write, FALSE },
@@ -1621,6 +1634,7 @@ 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);