Code review comment for lp:~jml/bzr/allow-writes-change-84659

Revision history for this message
Robert Collins (lifeless) wrote :

Something that was missed I think, is that this breaks 'bzr push bzr
+ssh://' between versions. So

 review -1

We should _not_ break that fairly fundamental interface without a very
compelling reason.

> > I understand the desire behind being clear about it for "bzr serve" to
> > bring up the http/bzr:// access. I just worry about some of the fallout.
> >
> > (I'm not blocking, just bringing up the discussion.)
>
> So where to from here?

I think --allow-writes is clear and precise. I don't think we should
change it unless the change fixes a problem greater than changing it
will cause.

AIUI the problem is one of expectation? People expecting that allowing
writes implies some sort of authentication, and we *anticipate* that
users are putting live, anonymous write permitting servers on the
internet?

Seems to me that updating the docs is all that can really be done, in
the absence of deeper changes.

Perhaps:
 - add a parameter:
   --auth=(external|anonymous)
 - make --allow-writes error unless --auth= is supplied or --inet was
   supplied.

Then, for ssh:
 --inet is supplied

For http:
 --auth is required

For inetd:
 If the user starts it via the superserver, --inet is passed and its up
 to them. Otherwise --auth is required.

-Rob

review: Disapprove

« Back to merge proposal