You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Sander Striker <st...@apache.org> on 2004/03/15 11:12:19 UTC
[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(¤t_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
Re: [PATCH] mod_dav_svn: Prevent multi-author commits, prevent
no-author commits when there is an author
Posted by Sander Striker <st...@apache.org>.
On Tue, 2004-03-16 at 00:30, Greg Stein wrote:
> On Mon, Mar 15, 2004 at 12:12:19PM +0100, Sander Striker wrote:
[...]
> > + /* 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)
>
> Note that this allows for an author to be set at some point, and then a
> later operation to *not* have a username set. I think this is acceptable.
I think I'd go as far as call it nessecary ;). Remember, some parts of
the repos may have anonymous write, while other parts may not. I'll put
this in the comment.
> The problem really comes up only when you have a change in author.
Agreed.
> >...
> > + request_author = svn_string_create(comb->priv.repos->username, pool);
>
> There isn't a reason to copy the string onto the heap. Just do:
>
> request_author.data = comb->priv.repos->username;
> request_author.len = strlen(request_author.data);
>
> And then use &request_author where necessary.
*nod*, very sensible ;).
Sander
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] mod_dav_svn: Prevent multi-author commits, prevent no-author commits when there is an author
Posted by Greg Stein <gs...@lyra.org>.
On Mon, Mar 15, 2004 at 12:12:19PM +0100, Sander Striker wrote:
>...
> 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.
+1
>...
> +++ 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)
Note that this allows for an author to be set at some point, and then a
later operation to *not* have a username set. I think this is acceptable.
The problem really comes up only when you have a change in author.
>...
> + request_author = svn_string_create(comb->priv.repos->username, pool);
There isn't a reason to copy the string onto the heap. Just do:
request_author.data = comb->priv.repos->username;
request_author.len = strlen(request_author.data);
And then use &request_author where necessary.
>...
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] mod_dav_svn: Prevent multi-author commits, prevent
no-author commits when there is an author
Posted by Ben Collins-Sussman <su...@collab.net>.
On Mon, 2004-03-15 at 05:12, Sander Striker wrote:
> 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.
Hmmm, I'm reviewing this patch. Watch this space for more.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org