subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ben Reser <...@reser.org>
Subject Re: serf error handling for locks without authn
Date Mon, 03 Jun 2013 22:48:06 GMT
On Mon, Jun 3, 2013 at 3:31 PM, Greg Stein <gstein@gmail.com> wrote:
> On Mon, Jun 3, 2013 at 6:09 PM, Philip Martin
> <philip.martin@wandisco.com> wrote:
>> Greg Stein <gstein@gmail.com> writes:
>>
>>> On Mon, Jun 3, 2013 at 5:14 PM, Philip Martin
>>> <philip.martin@wandisco.com> wrote:
>>>> http://subversion.tigris.org/issues/show_bug.cgi?id=4368
>>>>
>>>> Locking with anonymous http fails because there is no username.  At

That's my fault, I fixed the anonymous locking segfault and chose a
401 to return but I forgot about the WWW-Authenticate header
requirement.

>> What is mod_dav_svn going to do, invent a username?
>
> Ah. Well... I would say "yes, invent a username". "_unknown_" or somesuch.
>
> But that's just me :-)

That's problematic, some people want to enforce that only the lock
owner can remove the locks and so on.  I also think supporting
entirely anonymous locks is an almost entirely pointless exercise
given our current limitations.

> I'm not sure if we can determine whether authn/authz is configured for
> the particular path.
>
> Oh. Wait. The config should prevent the LOCK even *before* it gets to
> our code. IOW, if our code is reached *without* a username, then
> authn/authz is not configured.

This is indeed correct.  When authentication is configured correctly
(even on anonymous read allowed setups) a LOCK will be rejected before
it even gets to us.  The anonymous lock issue only happens when you
have a repo setup to allow entirely anonymous commits, which is
effectively nobody.  At least until we have something like obliterate
where cleaning up the crap people would add to your repo is possible.

> 412 (Precondition Failed) is not appropriate, as there are no
> preconditions defined in the headers (eg. an "If" header). The proper
> response is 409 (Conflict). The client *may* be able to proactively
> provide authentication information, but we can't use 401 to automate
> the authn provision (for the lack of WWW-Authenticate, as you noted).

Agreed.

>>>> present mod_dav_svn returns "401 Unathorized" however
>>>
>>> Do you know where/why this is happening?
>>
>> mod_dav_svn/locks.c:append_locks.
>
> Yeah. HTTP_CONFLICT should be correct, there.

Well technically 401 is right but we have now way of filling in the
proper WWW-Authenticate header.  HTTP_CONFLICT doesn't sound
particularly great either because that's supposed to be describing an
issue with the state of the resource.

I'd argue that we should return a 500 range error since the problem
here is that the server is not properly configured.  There is really
nothing a client can do to resolve the issue other than to
authenticate, which our client is only going to do if the server is
setup properly.  So I'd vote for returning HTTP_INTERNAL_SERVER_ERROR.

Mime
View raw message