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 2016/12/18 15:42:51 UTC

svn commit: r1774899 - in /subversion/branches/authzperf: BRANCH-README subversion/libsvn_repos/authz_info.c subversion/tests/libsvn_repos/authz-test.c

Author: stefan2
Date: Sun Dec 18 15:42:50 2016
New Revision: 1774899

URL: http://svn.apache.org/viewvc?rev=1774899&view=rev
Log:
On the authzperf branch:
Make svn_authz__get_global_rights() more useful by resolving all rules that
may apply to the respective repo and not just the ones explicitly declared
for it.  This is necessary because both sets of rules will be overlaid when
resolving access rights.

IOW, merge the global access with the per-repo ones.  Add a test for it.

* subversion/libsvn_repos/authz_info.c
  (combine_rights): New utility function.
  (resolve_global_rights): Always combine the per-repo rights with the rights
                           for "any" repo.

* subversion/tests/libsvn_repos/authz-test.c
  (NL): Useful define.
  (test_global_rights): New test.
  (test_funcs): Register the new test.

* BRANCH-README
  (TODO, DONE): One down, only one left to go before we are ready for /trunk.

Modified:
    subversion/branches/authzperf/BRANCH-README
    subversion/branches/authzperf/subversion/libsvn_repos/authz_info.c
    subversion/branches/authzperf/subversion/tests/libsvn_repos/authz-test.c

Modified: subversion/branches/authzperf/BRANCH-README
URL: http://svn.apache.org/viewvc/subversion/branches/authzperf/BRANCH-README?rev=1774899&r1=1774898&r2=1774899&view=diff
==============================================================================
--- subversion/branches/authzperf/BRANCH-README (original)
+++ subversion/branches/authzperf/BRANCH-README Sun Dec 18 15:42:50 2016
@@ -13,7 +13,6 @@ TODO:
 
 * remove no-op escape sequences from wildcard rule segments
 * implement lookup access rights
-* correct the global <-> per-repo rule precedence to match 1.9 behavior
 * use a user's accumulated "global" access rights where sufficient
 
 DONE:
@@ -44,3 +43,4 @@ DONE:
 * implement global authz and filtered tree caches
 * add fast lookup path for in-repository authz files
 * support in-registry authz
+* correct the global <-> per-repo rule precedence to match 1.9 behavior

Modified: subversion/branches/authzperf/subversion/libsvn_repos/authz_info.c
URL: http://svn.apache.org/viewvc/subversion/branches/authzperf/subversion/libsvn_repos/authz_info.c?rev=1774899&r1=1774898&r2=1774899&view=diff
==============================================================================
--- subversion/branches/authzperf/subversion/libsvn_repos/authz_info.c (original)
+++ subversion/branches/authzperf/subversion/libsvn_repos/authz_info.c Sun Dec 18 15:42:50 2016
@@ -91,6 +91,18 @@ svn_authz__get_acl_access(authz_access_t
   return has_access;
 }
 
+/* Set *RIGHTS_P to the combination of LHS and RHS, i.e. intersect the
+ * minimal rights and join the maximum rights.
+ */
+static void
+combine_rights(authz_rights_t *rights_p,
+               const authz_rights_t *lhs,
+               const authz_rights_t *rhs)
+{
+  rights_p->min_access = lhs->min_access & rhs->min_access;
+  rights_p->max_access = lhs->max_access | rhs->max_access;
+}
+
 
 /* Given GLOBAL_RIGHTS and a repository name REPOS, set *RIGHTS_P to
  * to the actual accumulated rights defined for that repository.
@@ -115,7 +127,7 @@ resolve_global_rights(authz_rights_t *ri
 
       if (rights)
         {
-          *rights_p = *rights;
+          combine_rights(rights_p, rights, &global_rights->any_repos_rights);
           return TRUE;
         }
     }

Modified: subversion/branches/authzperf/subversion/tests/libsvn_repos/authz-test.c
URL: http://svn.apache.org/viewvc/subversion/branches/authzperf/subversion/tests/libsvn_repos/authz-test.c?rev=1774899&r1=1774898&r2=1774899&view=diff
==============================================================================
--- subversion/branches/authzperf/subversion/tests/libsvn_repos/authz-test.c (original)
+++ subversion/branches/authzperf/subversion/tests/libsvn_repos/authz-test.c Sun Dec 18 15:42:50 2016
@@ -31,6 +31,8 @@
 
 #include "../svn_test.h"
 
+/* Used to terminate lines in large multi-line string literals. */
+#define NL APR_EOL_STR
 
 static svn_error_t *
 print_group_member(void *baton,
@@ -285,6 +287,83 @@ test_authz_parse(const svn_test_opts_t *
   return SVN_NO_ERROR;
 }
 
+static svn_error_t *
+test_global_rights(apr_pool_t *pool)
+{
+  svn_authz_t *authz;
+  int i;
+
+  /* Some non-trivially overlapping wildcard rules, convering all types
+   * of wildcards: "any", "any-var", "prefix", "postfix" and "complex".
+   */
+  const char *contents =
+    "[/public]"                                                          NL
+    "* = r"                                                              NL
+    ""                                                                   NL
+    "[greek:/A]"                                                         NL
+    "userA = rw"                                                         NL
+    ""                                                                   NL
+    "[repo:/A]"                                                          NL
+    "userA = r"                                                          NL
+    ""                                                                   NL
+    "[repo:/B]"                                                          NL
+    "userA = rw"                                                         NL
+    ""                                                                   NL
+    "[greek:/B]"                                                         NL
+    "userB = rw"                                                         NL;
+
+  const struct
+    {
+      const char *repos;
+      const char *user;
+      authz_rights_t rights;
+      svn_boolean_t found;
+    } test_cases[] =
+    {
+      /* Everyone may get read access b/c there might be a "/public" path. */
+      {      "",      "", { authz_access_none, authz_access_read  },  TRUE },
+      {      "", "userA", { authz_access_none, authz_access_read  },  TRUE },
+      {      "", "userA", { authz_access_none, authz_access_read  },  TRUE },
+      {      "", "userC", { authz_access_none, authz_access_read  },  TRUE },
+
+      /* Two users do even get write access on some paths in "greek".
+       * The root always defaults to n/a due to the default rule. */
+      { "greek",      "", { authz_access_none, authz_access_read  }, FALSE },
+      { "greek", "userA", { authz_access_none, authz_access_write },  TRUE },
+      { "greek", "userB", { authz_access_none, authz_access_write },  TRUE },
+      { "greek", "userC", { authz_access_none, authz_access_read  }, FALSE },
+
+      /* One users has write access to some paths in "repo". */
+      {  "repo",      "", { authz_access_none, authz_access_read  }, FALSE },
+      {  "repo", "userA", { authz_access_none, authz_access_write },  TRUE },
+      {  "repo", "userB", { authz_access_none, authz_access_read  }, FALSE },
+      {  "repo", "userC", { authz_access_none, authz_access_read  }, FALSE },
+
+      /* For unknown repos, we default to the global settings. */
+      {     "X",      "", { authz_access_none, authz_access_read  }, FALSE },
+      {     "X", "userA", { authz_access_none, authz_access_read  }, FALSE },
+      {     "X", "userA", { authz_access_none, authz_access_read  }, FALSE },
+      {     "X", "userC", { authz_access_none, authz_access_read  }, FALSE }
+    };
+
+  svn_stringbuf_t *buffer = svn_stringbuf_create(contents, pool);
+  svn_stream_t *stream = svn_stream_from_stringbuf(buffer, pool);
+  SVN_ERR(svn_repos_authz_parse(&authz, stream, NULL, pool));
+
+  for (i = 0; i < sizeof(test_cases) / sizeof(test_cases[0]); ++i)
+    {
+      authz_rights_t rights = { authz_access_write, authz_access_none };
+      svn_boolean_t found = svn_authz__get_global_rights(&rights, authz->full,
+                                                         test_cases[i].user,
+                                                         test_cases[i].repos);
+
+      SVN_TEST_ASSERT(found == test_cases[i].found);
+      SVN_TEST_ASSERT(rights.min_access == test_cases[i].rights.min_access);
+      SVN_TEST_ASSERT(rights.max_access == test_cases[i].rights.max_access);
+    }
+
+  return SVN_NO_ERROR;
+}
 
 static int max_threads = 4;
 
@@ -293,6 +372,8 @@ static struct svn_test_descriptor_t test
     SVN_TEST_NULL,
     SVN_TEST_OPTS_PASS(test_authz_parse,
                        "test svn_authz__parse"),
+    SVN_TEST_PASS2(test_global_rights,
+                   "test svn_authz__get_global_rights"),
     SVN_TEST_NULL
   };