[PATCH] mod_dav_svn: Prevent multi-author commits, prevent no-author commits when there is an author

Hi,

A few reports came in from people who are using mod_authz_svn in combination
with the following config:

<Location ...>
  AuthzSVNAccessFile ...

  Satisfy any
  Require valid-user

  ...
</Location>

Even though write access is denied for anonymous users, people were seeing
(no author) commits. I've seen this behaviour aswell.

I've filed this is issue 1786:
  http://subversion.tigris.org/issues/show_bug.cgi?id=1786

Part of the problem lies in this todo in mod_authz_svn:

    if (!repos_path) {
        /* XXX: Check if the user has 'required_access' _anywhere_ in the
         * XXX: repository. For now, make this always succeed, until
         * XXX: we come up with a good way of figuring this out.
         */
        return 1;
    }

The solution there is a fairly straight forward brute force search,
or a per user index up front. But even when this is fixed, the problem
will persist if there is anonymous write access allowed anywhere in
the repository.

The other part of the problem is that mod_dav_svn assigns the author
at a single point in the commit, the MKACTIVITY request, and later on
does not verify that all requests are from the same user.

This patch tries to fix that. It also prevents multi-author commits,
which we don't support (yet). We haven't put any thought in how to
display or represent multiple authors. And, the other ra layers don't
support it.

The error messages in the patch could use some work, and I'm not entirely
sure this is the correct place to do this.

Thoughts?

Sander

Log:
Fix issue #1786: mod_dav_svn accepts multiple authors, only records one

* subversion/mod_dav_svn/repos.c

  (dav_svn_prep_working): Set the txn author if not previously set. Protect
   against multi-author commits by verifying authenticated user associated
   with the current request is the same as the txn author.

Index: subversion/mod_dav_svn/repos.c
===================================================================
--- subversion/mod_dav_svn/repos.c (revision 9049)
+++ subversion/mod_dav_svn/repos.c (working copy)
@@ -674,6 +674,45 @@
       return NULL;
     }
 
+ /* Set the txn author if not previously set. Protect against multi-author
+ * commits by verifying authenticated user associated with the current
+ * request is the same as the txn author.
+ */
+ if (comb->priv.repos->username)
+ {
+ svn_string_t *current_author;
+ svn_string_t *request_author;
+
+ serr = svn_fs_txn_prop(&current_author, comb->priv.root.txn,
+ SVN_PROP_REVISION_AUTHOR, pool);
+ if (serr != NULL)
+ {
+ return dav_svn_convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
+ "Failed to retrieve author of the SVN FS transaction "
+ "corresponding to the specified activity.",
+ pool);
+ }
+
+ request_author = svn_string_create(comb->priv.repos->username, pool);
+ if (!current_author)
+ {
+ serr = svn_fs_change_txn_prop(comb->priv.root.txn,
+ SVN_PROP_REVISION_AUTHOR, request_author, pool);
+ if (serr != NULL)
+ {
+ return dav_svn_convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
+ "Failed to set the author of the SVN FS transaction "
+ "corresponding to the specified activity.",
+ pool);
+ }
+ }
+ else if (!svn_string_compare(current_author, request_author))
+ {
+ return dav_new_error(pool, HTTP_INTERNAL_SERVER_ERROR, 0,
+ "Multi-author commits not supported.");
+ }
+ }
+
   /* get the root of the tree */
   serr = svn_fs_txn_root(&comb->priv.root.root, comb->priv.root.txn, pool);
   if (serr != NULL)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Mar 15 12:12:41 2004