subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From br...@apache.org
Subject svn commit: r1847922 - in /subversion/trunk/subversion: libsvn_repos/authz.h libsvn_repos/authz_info.c libsvn_repos/authz_parse.c tests/cmdline/authz_tests.py tests/cmdline/svnauthz_tests.py
Date Sat, 01 Dec 2018 21:55:11 GMT
Author: brane
Date: Sat Dec  1 21:55:11 2018
New Revision: 1847922

URL: http://svn.apache.org/viewvc?rev=1847922&view=rev
Log:
Propagate knowledge about inverted access rule selectors (e.g., ~user,
~@group, ~&alias) to the global level of the parsed authz file structure
and take that information into account during access resolution.

* subversion/libsvn_repos/authz.h
  (authz_full_t): Add members has_neg_rights and neg_rights.
  (authz_acl_t): Add members has_neg_access and neg_access.

* subversion/libsvn_repos/authz_parse.c
  (neg_access_token): "User name" for the global inverted access rights.
  (insert_default_acl): Initialize neg_access and has_neg_access.
  (create_ctor_baton): Initialize the inverted global rights.
  (rules_open_section): Initialize the ALC's inverted access.
  (add_access_entry): Record the inverted rights.
  (expand_acl_callback): Propagate the inverted rights to global scope.

* subversion/libsvn_repos/authz_info.c
  (svn_authz__get_global_rights): When the user is authenticated but
   does not have an explicit entry in the authz file, use the available global
   inverted rights to resolve the user's access.

* subversion/tests/cmdline/authz_tests.py
  (inverted_group_membership): Remove XFail decorator.
   Delete the reference to the mail archives, it's in the issue tracker. 

* subversion/tests/cmdline/svnauthz_tests.py
  (svnauthz_inverted_selector_test): New test case.

Fixes: SVN-4793
Suggested by: Pavel Goran <inbox-17{_AT_}pvgoran.name>

Modified:
    subversion/trunk/subversion/libsvn_repos/authz.h
    subversion/trunk/subversion/libsvn_repos/authz_info.c
    subversion/trunk/subversion/libsvn_repos/authz_parse.c
    subversion/trunk/subversion/tests/cmdline/authz_tests.py
    subversion/trunk/subversion/tests/cmdline/svnauthz_tests.py

Modified: subversion/trunk/subversion/libsvn_repos/authz.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/authz.h?rev=1847922&r1=1847921&r2=1847922&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_repos/authz.h (original)
+++ subversion/trunk/subversion/libsvn_repos/authz.h Sat Dec  1 21:55:11 2018
@@ -139,6 +139,10 @@ typedef struct authz_full_t
   svn_boolean_t has_authn_rights;
   authz_global_rights_t authn_rights;
 
+  /* Globally accumulated rights from inverted groups or aliases. */
+  svn_boolean_t has_neg_rights;
+  authz_global_rights_t neg_rights;
+
   /* Globally accumulated rights, for all concrete users mentioned
      in the authz file. The key is the user name, the value is
      an authz_global_rights_t*. */
@@ -257,14 +261,18 @@ typedef struct authz_acl_t
   /* The parsed rule. */
   authz_rule_t rule;
 
-  /* Access rights for anonymous users */
+  /* Access rights for anonymous users. */
   svn_boolean_t has_anon_access;
   authz_access_t anon_access;
 
-  /* Access rights for authenticated users */
+  /* Access rights for authenticated users. */
   svn_boolean_t has_authn_access;
   authz_access_t authn_access;
 
+  /* Access rights from inverted groups or aliases. */
+  svn_boolean_t has_neg_access;
+  authz_access_t neg_access;
+
   /* All other user- or group-specific access rights.
      Aliases are replaced with their definitions, rules for the same
      user or group are merged. */

Modified: subversion/trunk/subversion/libsvn_repos/authz_info.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/authz_info.c?rev=1847922&r1=1847921&r2=1847922&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_repos/authz_info.c (original)
+++ subversion/trunk/subversion/libsvn_repos/authz_info.c Sat Dec  1 21:55:11 2018
@@ -148,37 +148,50 @@ svn_authz__get_global_rights(authz_right
     {
       /* Check if we have explicit rights for anonymous access. */
       if (authz->has_anon_rights)
-        return resolve_global_rights(rights_p, &authz->anon_rights, repos);
+        {
+          return resolve_global_rights(rights_p, &authz->anon_rights, repos);
+        }
+      else
+        {
+          /* Return the implicit rights, i.e., none. */
+          rights_p->min_access = authz_access_none;
+          rights_p->max_access = authz_access_none;
+          return FALSE;
+        }
     }
   else
     {
+      svn_boolean_t combine_user_rights = FALSE;
+      svn_boolean_t access = FALSE;
+
       /* Check if we have explicit rights for this user. */
       const authz_global_rights_t *const user_rights =
         svn_hash_gets(authz->user_rights, user);
 
       if (user_rights)
         {
-          svn_boolean_t explicit
-            = resolve_global_rights(rights_p, user_rights, repos);
-
-          /* Rights given to _any_ authenticated user may apply, too. */
-          if (authz->has_authn_rights)
-            {
-              authz_rights_t authn;
-              explicit |= resolve_global_rights(&authn, &authz->authn_rights,
-                                                repos);
-              combine_rights(rights_p, rights_p, &authn);
-            }
-          return explicit;
+          access = resolve_global_rights(rights_p, user_rights, repos);
+          combine_user_rights = TRUE;
+        }
+      else if (authz->has_neg_rights)
+        {
+          /* Check if inverted-rule rights apply */
+          access = resolve_global_rights(rights_p, &authz->neg_rights, repos);
+          combine_user_rights = TRUE;
         }
 
-      /* Check if we have explicit rights for authenticated access. */
+      /* Rights given to _any_ authenticated user may apply, too. */
       if (authz->has_authn_rights)
-        return resolve_global_rights(rights_p, &authz->authn_rights, repos);
-    }
+        {
+          authz_rights_t authn;
+          access |= resolve_global_rights(&authn, &authz->authn_rights, repos);
 
-  /* Fall-through: return the implicit rights, i.e., none. */
-  rights_p->min_access = authz_access_none;
-  rights_p->max_access = authz_access_none;
-  return FALSE;
+          if (combine_user_rights)
+            combine_rights(rights_p, rights_p, &authn);
+          else
+            *rights_p = authn;
+        }
+
+      return access;
+    }
 }

Modified: subversion/trunk/subversion/libsvn_repos/authz_parse.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/authz_parse.c?rev=1847922&r1=1847921&r2=1847922&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_repos/authz_parse.c (original)
+++ subversion/trunk/subversion/libsvn_repos/authz_parse.c Sat Dec  1 21:55:11 2018
@@ -154,6 +154,8 @@ static const char anon_access_token[] =
 /* The authenticated access token. */
 static const char authn_access_token[] = "$authenticated";
 
+/* Fake token for inverted rights. */
+static const char neg_access_token[] = "~~$inverted";
 
 /* Initialize a rights structure.
    The minimum rights start with all available access and are later
@@ -191,6 +193,8 @@ insert_default_acl(ctor_baton_t *cb)
   acl->acl.has_anon_access = TRUE;
   acl->acl.authn_access = authz_access_none;
   acl->acl.has_authn_access = TRUE;
+  acl->acl.neg_access = authz_access_none;
+  acl->acl.has_neg_access = TRUE;
   acl->acl.user_access = NULL;
   acl->aces = svn_hash__make(cb->parser_pool);
   acl->alias_aces = svn_hash__make(cb->parser_pool);
@@ -208,6 +212,7 @@ create_ctor_baton(apr_pool_t *result_poo
   authz_full_t *const authz = apr_pcalloc(result_pool, sizeof(*authz));
   init_global_rights(&authz->anon_rights, anon_access_token, result_pool);
   init_global_rights(&authz->authn_rights, authn_access_token, result_pool);
+  init_global_rights(&authz->neg_rights, neg_access_token, result_pool);
   authz->user_rights = svn_hash__make(result_pool);
   authz->pool = result_pool;
 
@@ -758,6 +763,8 @@ rules_open_section(void *baton, svn_stri
   acl.acl.has_anon_access = FALSE;
   acl.acl.authn_access = authz_access_none;
   acl.acl.has_authn_access = FALSE;
+  acl.acl.neg_access = authz_access_none;
+  acl.acl.has_neg_access = FALSE;
   acl.acl.user_access = NULL;
 
   acl.aces = svn_hash__make(cb->parser_pool);
@@ -958,6 +965,14 @@ add_access_entry(ctor_baton_t *cb, svn_s
           if (!aliased && *ace->name != '@')
             prepare_global_rights(cb, ace->name);
         }
+
+      /* Propagate rights for inverted selectors to the global rights, otherwise
+         an access check can bail out early. See: SVN-4793 */
+      if (inverted)
+        {
+          acl->acl.has_neg_access = TRUE;
+          acl->acl.neg_access |= access;
+        }
     }
 
   return SVN_NO_ERROR;
@@ -1271,6 +1286,12 @@ expand_acl_callback(void *baton,
       update_global_rights(&cb->authz->authn_rights,
                            acl->rule.repos, acl->authn_access);
     }
+  if (acl->has_neg_access)
+    {
+      cb->authz->has_neg_rights = TRUE;
+      update_global_rights(&cb->authz->neg_rights,
+                           acl->rule.repos, acl->neg_access);
+    }
   SVN_ERR(svn_iter_apr_hash(NULL, cb->authz->user_rights,
                             update_user_rights, acl, scratch_pool));
   return SVN_NO_ERROR;

Modified: subversion/trunk/subversion/tests/cmdline/authz_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/authz_tests.py?rev=1847922&r1=1847921&r2=1847922&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/authz_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/authz_tests.py Sat Dec  1 21:55:11 2018
@@ -1663,14 +1663,11 @@ def remove_access_after_commit(sbox):
                                         expected_status,
                                         [], True)
 
-@XFail()
 @Issue(4793)
 @Skip(svntest.main.is_ra_type_file)
 def inverted_group_membership(sbox):
   "access rights for user in inverted group"
 
-  # Bug reported here: https://lists.apache.org/thread.html/6cc7b22b211827ff946373407a516a3ab4d866fe03cdc85d22ff276b@%3Cdev.subversion.apache.org%3E
-
   sbox.build(create_wc = False)
 
   svntest.actions.enable_revprop_changes(sbox.repo_dir)

Modified: subversion/trunk/subversion/tests/cmdline/svnauthz_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/svnauthz_tests.py?rev=1847922&r1=1847921&r2=1847922&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/svnauthz_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/svnauthz_tests.py Sat Dec  1 21:55:11 2018
@@ -896,6 +896,75 @@ def svnauthz_compat_mode_repo_test(sbox)
       repo_url + "/zilch"
   )
 
+
+@Issue(4793)
+def svnauthz_inverted_selector_test(sbox):
+  "test inverted selector rights"
+
+  # build an authz file
+  authz_content = ("[groups]\n"
+                   "grouped = grouper\n"
+
+                   "[aliases]\n"
+                   "aliased = aliaser\n"
+
+                   "[A:/]\n"
+                   "@grouped = r\n"
+                   "~@grouped = rw\n"
+
+                   "[B:/]\n"
+                   "&aliased = r\n"
+                   "~&aliased = rw\n"
+
+                   "[C:/]\n"
+                   "user = r\n"
+                   "~user = rw\n"
+
+                   "[D:/]\n"
+                   "~@grouped = rw\n"
+                   "not-grouper = r\n"
+
+                   "[E:/]\n"
+                   "~&aliased = rw\n"
+                   "not-aliaser = r\n"
+
+                   "[F:/]\n"
+                   "~user = rw\n"
+                   "not-user = r\n")
+
+  (authz_fd, authz_path) = tempfile.mkstemp()
+  svntest.main.file_write(authz_path, authz_content)
+
+  def accessof(repos, user, expected_stdout):
+    return svntest.actions.run_and_verify_svnauthz(
+      expected_stdout, None, 0, False, 'accessof',
+      '--repository', repos, '--username', user, authz_path)
+
+  accessof('A', 'grouper', 'r')
+  accessof('A', 'not-grouper', 'rw')
+
+  accessof('B', 'aliaser', 'r')
+  accessof('B', 'not-aliaser', 'rw')
+
+  accessof('C', 'user', 'r')
+  accessof('C', 'not-user', 'rw')
+
+  accessof('D', 'grouper', 'no')
+  accessof('D', 'not-grouper', 'r')
+  accessof('D', 'other', 'rw')
+
+  accessof('E', 'aliaser', 'no')
+  accessof('E', 'not-aliaser', 'r')
+  accessof('E', 'other', 'rw')
+
+  accessof('F', 'user', 'no')
+  accessof('F', 'not-user', 'r')
+  accessof('F', 'other', 'rw')
+
+  os.close(authz_fd)
+  os.remove(authz_path)
+
+
 ########################################################################
 # Run the tests
 
@@ -914,6 +983,7 @@ test_list = [ None,
               svnauthz_accessof_txn_test,
               svnauthz_compat_mode_file_test,
               svnauthz_compat_mode_repo_test,
+              svnauthz_inverted_selector_test,
              ]
 
 if __name__ == '__main__':



Mime
View raw message