Re: Call for feedback: First GSoC patch ready to land

2009-11-17 Thread Russell Keith-Magee
On Wed, Nov 18, 2009 at 12:03 AM, Schmilblick wrote: > Sorry for crashing the list, but how does this affect this bug: > http://code.djangoproject.com/ticket/5390 ? > > I'm currently using django with the patch provided in that ticket, and > the latest commits regarding the gsoc-project created a

Re: Call for feedback: First GSoC patch ready to land

2009-11-17 Thread Schmilblick
Sorry for crashing the list, but how does this affect this bug: http://code.djangoproject.com/ticket/5390 ? I'm currently using django with the patch provided in that ticket, and the latest commits regarding the gsoc-project created a diff im not sure how to handle. Let me know if you need any mo

Re: Call for feedback: First GSoC patch ready to land

2009-11-02 Thread Russell Keith-Magee
On Mon, Nov 2, 2009 at 11:40 PM, Jacob Kaplan-Moss wrote: > > Hi Russ -- > > I'm +1 on merging the patch immediately. I have some feedback on the > couple of issues you raised below, but I see no reason they can't be > addressed after merging. > > On Fri, Oct 30, 2009 at 3:33 AM, Russell Keith-Ma

Re: Call for feedback: First GSoC patch ready to land

2009-11-02 Thread Jacob Kaplan-Moss
Hi Russ -- I'm +1 on merging the patch immediately. I have some feedback on the couple of issues you raised below, but I see no reason they can't be addressed after merging. On Fri, Oct 30, 2009 at 3:33 AM, Russell Keith-Magee wrote: > The automatically generated m2m model is the equivalent of

Re: Call for feedback: First GSoC patch ready to land

2009-10-30 Thread Russell Keith-Magee
On Sat, Oct 31, 2009 at 9:53 AM, Russell Keith-Magee wrote: > On Sat, Oct 31, 2009 at 9:36 AM, Johannes Dollinger > wrote: >> >> The m2m-refactor patch breaks ManyToManyFields on abstract models: >> "AssertionError: ForeignKey cannot define a relation with abstract >> class A" > > Thanks for the

Re: Call for feedback: First GSoC patch ready to land

2009-10-30 Thread Russell Keith-Magee
On Sat, Oct 31, 2009 at 9:36 AM, Johannes Dollinger wrote: > > The m2m-refactor patch breaks ManyToManyFields on abstract models: > "AssertionError: ForeignKey cannot define a relation with abstract > class A" Thanks for the report, Johannes. I'll take a look at this and see what I can do about

Re: Call for feedback: First GSoC patch ready to land

2009-10-30 Thread Johannes Dollinger
The m2m-refactor patch breaks ManyToManyFields on abstract models: "AssertionError: ForeignKey cannot define a relation with abstract class A" {{{ class R(models.Model): pass class A(models.Model): r_set = models.ManyToManyField(R, related_name="%(class)s_set") class Meta:

Re: Call for feedback: First GSoC patch ready to land

2009-10-30 Thread Russell Keith-Magee
On Oct 30, 4:33 pm, Russell Keith-Magee wrote: > Hi all, > > I've just finished the polishing the first part of Alex's GSoC work to > add Multi-db, which I'd like to commit. This is a call for feedback. I forgot to mention - the patch _should_ be completely backwards compatible, and require no

Call for feedback: First GSoC patch ready to land

2009-10-30 Thread Russell Keith-Magee
Hi all, I've just finished the polishing the first part of Alex's GSoC work to add Multi-db, which I'd like to commit. This is a call for feedback. This first patch isn't directly related to multi-db, but it is a prerequisite that will allow the multi-db code to function. The goal of this patch

Re: CSRF proposal and patch ready for review

2009-09-23 Thread Luke Plant
On Tuesday 22 September 2009 21:24:48 Luke Plant wrote: > 2) Get the view to be exempted from the normal CSRF checks done > by the middleware. Thankfully, we already have not one but two > ways of doing this - the manual @csrf_exempt decorator on views, > and the internal mechanism that allow

Re: CSRF proposal and patch ready for review

2009-09-23 Thread Simon Willison
On Sep 22, 9:24 pm, Luke Plant wrote: > > 2. I'm not at all keen on the implementation as a middleware > > (especially a view middleware, which doesn't play well with generic > > views and redispatching to other view functions, both patterns I like > > a lot). > > Could you explain a bit more abo

Re: CSRF proposal and patch ready for review

2009-09-22 Thread Luke Plant
On Tuesday 22 September 2009 12:57:16 Simon Willison wrote: > The main reason I really like preserving form data is that it means > CSRF failures are less of a problem, allowing us to be much more > strict about how they work (setting form tokens to expire after a few > hours, tying tokens to ind

Re: CSRF proposal and patch ready for review

2009-09-22 Thread Luke Plant
On Tuesday 22 September 2009 13:12:51 Russell Keith-Magee wrote: > On Tue, Sep 22, 2009 at 10:34 AM, Luke Plant wrote: > > I've left most of the code itself under django/contrib/csrf because: > > > > 1) backwards compatibility with people importing the middleware > >means we have to leave dj

Re: CSRF proposal and patch ready for review

2009-09-22 Thread Jacob Kaplan-Moss
On Tue, Sep 22, 2009 at 7:12 AM, Russell Keith-Magee wrote: > At this point, I'm convinced, mod the minor things I've flagged. > However, I'd like to see Jacob and Malcolm chime in before this is > committed. I've mostly stayed out of the discussion because I haven't had much helpful to say that

Re: CSRF proposal and patch ready for review

2009-09-22 Thread James Bennett
On Tue, Sep 22, 2009 at 6:57 AM, Simon Willison wrote: > Yeah, I'd like a builtin shortcut like that - used like this: >  render(request, 'template_name.html', {'foo':bar }) > The biggest problem, for me, is finding a decent name - since > 'render_to_response' is already taken.  Maybe something a

Re: CSRF proposal and patch ready for review

2009-09-22 Thread Russell Keith-Magee
On Tue, Sep 22, 2009 at 10:34 AM, Luke Plant wrote: > > OK, you convinced me.  I really would rather this wasn't baked in, but given > the migration issues and the fact that it is security related, I guess I can > stomach it. > > I've updated the patch [1] to move things to builtin functionality.

Re: CSRF proposal and patch ready for review

2009-09-22 Thread Simon Willison
On Sep 19, 4:56 pm, Russell Keith-Magee wrote: > End users should be allowed to be as lazy as they like, but > their laziness shouldn't open security holes in an app that Django > ships, since the contrib apps (and admin in particular) are the > obvious first port of call for any systematic CSRF

Re: CSRF proposal and patch ready for review

2009-09-21 Thread Luke Plant
OK, you convinced me. I really would rather this wasn't baked in, but given the migration issues and the fact that it is security related, I guess I can stomach it. I've updated the patch [1] to move things to builtin functionality. I also had to fix some bugs to get the csrf_protect decorat

Re: CSRF proposal and patch ready for review

2009-09-21 Thread Russell Keith-Magee
On Sun, Sep 20, 2009 at 2:57 AM, Luke Plant wrote: > > On Saturday 19 September 2009 16:56:52 Russell Keith-Magee wrote: > >> On Fri, Sep 18, 2009 at 6:09 AM, Luke Plant wrote: >> >  If the target of a is internal: >> >   * add {% load csrf %} to the template and {% csrf_token %} to the form >>

Re: CSRF proposal and patch ready for review

2009-09-21 Thread Luke Plant
On Saturday 19 September 2009 16:56:52 Russell Keith-Magee wrote: > So - let's have both. A middleware enabled by default protects the > rest of the views. However, we can also have a view decorator lets us > protect admin (and other contrib apps) explicitly. If users disable > the middleware, ad

Re: CSRF proposal and patch ready for review

2009-09-19 Thread Luke Plant
On Saturday 19 September 2009 16:56:52 Russell Keith-Magee wrote: > On Fri, Sep 18, 2009 at 6:09 AM, Luke Plant wrote: > > If the target of a is internal: > > * add {% load csrf %} to the template and {% csrf_token %} to the form > > * use RequestContext in the corresponding view. > > > >

Re: CSRF proposal and patch ready for review

2009-09-19 Thread Russell Keith-Magee
On Fri, Sep 18, 2009 at 6:09 AM, Luke Plant wrote: > > Hi Simon, > > OK, here is my response.  I hope this doesn't turn into a personal my-code-vs- > your-code match (especially as most of "my code" is really other people's code > and ideas :-) but I want to make sure we do this right, as it pote

Re: CSRF proposal and patch ready for review

2009-09-19 Thread Luke Plant
Hi Simon, > 1. SafeForm has no dependency on Django's session framework. This is A > Good Thing as I personally avoid sessions in my projects (no need to > access state on every request if you can get away with not doing it). ... > It's a bit more than that - I also want something that works > in

Re: CSRF proposal and patch ready for review

2009-09-18 Thread Simon Willison
On Sep 18, 12:09 am, Luke Plant wrote: > OK, here is my response.  I hope this doesn't turn into a personal my-code-vs- > your-code match (especially as most of "my code" is really other people's code > and ideas :-) but I want to make sure we do this right, as it potentially has > big implicatio

Re: CSRF proposal and patch ready for review

2009-09-18 Thread Simon Willison
On Sep 17, 3:42 pm, Simon Willison wrote: > All good points - the change in function signature naturally fell out > of the CSRF work (since the form needs access to the request object in > both cases) but you've convinced me that it's a step too far - I'll > see if I can fix that. I just pushed

Re: CSRF proposal and patch ready for review

2009-09-17 Thread Luke Plant
Hi Simon, OK, here is my response. I hope this doesn't turn into a personal my-code-vs- your-code match (especially as most of "my code" is really other people's code and ideas :-) but I want to make sure we do this right, as it potentially has big implications, so the following will be "sleev

Re: CSRF proposal and patch ready for review

2009-09-17 Thread Simon Willison
All good points - the change in function signature naturally fell out of the CSRF work (since the form needs access to the request object in both cases) but you've convinced me that it's a step too far - I'll see if I can fix that. Cheers, Simon On Sep 17, 2:10 pm, Russell Keith-Magee wrote: >

Re: CSRF proposal and patch ready for review

2009-09-17 Thread Russell Keith-Magee
On Thu, Sep 17, 2009 at 3:11 PM, Simon Willison wrote: > > On Sep 15, 2:57 pm, Luke Plant wrote: >> OK, I'll wait and see. > > Here's the code: http://github.com/simonw/django-safeform > >>  * it requires using Django's form system.  I've got plenty of views that >> don't (e.g. anything with a d

Re: CSRF proposal and patch ready for review

2009-09-17 Thread Simon Willison
On Sep 15, 2:57 pm, Luke Plant wrote: > OK, I'll wait and see. Here's the code: http://github.com/simonw/django-safeform >  * it requires using Django's form system.  I've got plenty of views that > don't (e.g. anything with a dynamic number of controls).  People not using > django.Forms are le

Re: CSRF proposal and patch ready for review

2009-09-15 Thread Luke Plant
On Tuesday 15 September 2009 12:28:51 Russell Keith-Magee wrote: > The CSRF tag approach you have implemented didn't win a lot of fans > whenever I described it, and for pretty much the same reasons I have > expressed previously - too many moving parts, and a little too much > manual intervention

Re: CSRF proposal and patch ready for review

2009-09-15 Thread Russell Keith-Magee
On Mon, Sep 14, 2009 at 9:05 PM, Luke Plant wrote: > > On Monday 31 August 2009 15:26:42 Russell Keith-Magee wrote: > >>  3. CSRF is currently a contrib app. Why? CSRF control is the very >> model of a feature that shouldn't be decoupled from the base >> framework. If we're aiming to make CSRF su

Re: CSRF proposal and patch ready for review

2009-09-14 Thread Luke Plant
On Monday 31 August 2009 15:26:42 Russell Keith-Magee wrote: > 3. CSRF is currently a contrib app. Why? CSRF control is the very > model of a feature that shouldn't be decoupled from the base > framework. If we're aiming to make CSRF support like XSS support, > surely it should be baked into the

Re: CSRF proposal and patch ready for review

2009-09-10 Thread Luke Plant
On Monday 31 August 2009 15:26:42 Russell Keith-Magee wrote: > 1. Is there anything we can do to fix the problems with the > ResponseMiddleware? I know there are problems, but at the core there > is something really good - it seems a waste to throw it all away. I just stumbled across a new argu

Re: CSRF proposal and patch ready for review

2009-08-31 Thread Luke Plant
Hi Russell, > The difference here is that XSS is mentioned in the template docs, > not the tutorial. The tutorial is happily XSS safe, and the new > user is oblivious to this fact. You only really need to hunt down > documentation about XSS when you start dealing with content that > needs to brea

Re: CSRF proposal and patch ready for review

2009-08-31 Thread Russell Keith-Magee
On Mon, Aug 31, 2009 at 8:45 PM, Luke Plant wrote: > > Thanks for your response Russell: > >> I've had a quick look at the patch, and found a few minor cosmetic >> things. I've also done a lot of reading of the archives to >> understand why the patch is the way it is. A comprehensive teardown >> o

Re: CSRF proposal and patch ready for review

2009-08-31 Thread Luke Plant
I wrote: > In fact, I've just discovered that there is a view in > current Django that, if you have the current CSRF protection > enabled, will leak the CSRF token to an external site -- the > technical 500 debug view in django/views/debug.py has a POST form > to dpaste.com. (I'll try to fix that

Re: CSRF proposal and patch ready for review

2009-08-31 Thread Luke Plant
Thanks for your response Russell: > I've had a quick look at the patch, and found a few minor cosmetic > things. I've also done a lot of reading of the archives to > understand why the patch is the way it is. A comprehensive teardown > of the patch will take a bit longer, but before I do that tea

Re: CSRF proposal and patch ready for review

2009-08-30 Thread Russell Keith-Magee
On Mon, Aug 3, 2009 at 11:25 PM, Luke Plant wrote: > > Hi all, > > Some big changes to the CSRF protection nearly got in to Django 1.1, > but didn't.  Since then, more work has been done, overhauling the > whole thing really.  There has been a huge amount of discussion on > mailing lists and ticke

Re: CSRF proposal and patch ready for review

2009-08-29 Thread Russell Keith-Magee
On Sun, Aug 30, 2009 at 3:10 AM, Luke Plant wrote: > > Can I have some feedback on this please? ... > As far as I'm concerned, this is ready for checkin, except that I > haven't had *any* recent feedback or thumbs up etc. from the list or > other core devs. Apologies, Luke. I had this one flagged

Re: CSRF proposal and patch ready for review

2009-08-29 Thread Luke Plant
Can I have some feedback on this please? I've now addressed everything outstanding (tested under HTTPS and updated the tutorials), and I've included a friendly summary at the top of http://code.djangoproject.com/wiki/CsrfProtection For those wanting to see the patch, for once Trac hasn't barfe

CSRF proposal and patch ready for review

2009-08-03 Thread Luke Plant
Hi all, Some big changes to the CSRF protection nearly got in to Django 1.1, but didn't. Since then, more work has been done, overhauling the whole thing really. There has been a huge amount of discussion on mailing lists and tickets, so I've put together what I consider to be the conclusio

Re: Patch ready

2007-10-09 Thread Marc Garcia
I figured it out, but I just wanted to be sure. Thanks! On Oct 9, 3:41 pm, "Jacob Kaplan-Moss" <[EMAIL PROTECTED]> wrote: > On 10/9/07, Marc Garcia <[EMAIL PROTECTED]> wrote: > > > I'm not sure if it's just that I'm being impatient, but in a ticket > > (#4036), after some discussion, some candida

Re: Patch ready

2007-10-09 Thread Jacob Kaplan-Moss
On 10/9/07, Marc Garcia <[EMAIL PROTECTED]> wrote: > I'm not sure if it's just that I'm being impatient, but in a ticket > (#4036), after some discussion, some candidate patches I've upload a > patch that can be commited to subversion, and I've changed its triage > stage to "Ready for checkin". Is

Patch ready

2007-10-09 Thread Marc Garcia
Hi, I'm not sure if it's just that I'm being impatient, but in a ticket (#4036), after some discussion, some candidate patches I've upload a patch that can be commited to subversion, and I've changed its triage stage to "Ready for checkin". Is it enough, or do I have to do anything else for notic