Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#14116 closed (fixed)

TestClient skips Csrf Middleware

Reported by: jon@… Owned by: nobody
Component: Testing framework Version: 1.2
Severity: Keywords: TestClient
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:


The following code is found in TestClient

            request = WSGIRequest(environ)                                                               
            # sneaky little hack so that we can easily get round                                         
            # CsrfViewMiddleware.  This makes life easier, and is probably                               
            # required for backwards compatibility with external tests against                           
            # admin views.                                                                               
            request._dont_enforce_csrf_checks = True
            response = self.get_response(request) 

While this is nice, it makes the test behave in a way that does not really verify the site works.

Some of my views are run when accessed from a desktop program, and the desktop program does not send a CSRF token, resulting in a 403 Forbidden error, but does not happen when a test is run because of the above code.

Can we get an option to turn this off?


Change History (4)

comment:1 Changed 6 years ago by Luke Plant

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Resolution: wontfix
Status: newclosed

An option to turn it off would not be very helpful. Much better would be a subclass of Client that didn't do this that you can use as needed. You can code that yourself without needing to change core Django behaviour (subclass ClientHandler and override get_response, subclass Client to use the new handler, subclass TestCase and override _pre_setup adding self.client = MyClient()).

The fact of the matter is that the test client does not behave like a browser does in many other ways, and in each way it differs it is also failing to verify that your site actually works. If you want that kind of testing, you need to use something like Twill or Windmill.

Also, I'm not sure how your tests are running, but I can't see that changing this behaviour would actually make your tests more robust. If you are doing functional tests of your desktop program, you shouldn't be using the test client at all. If you are using the test client to send requests in your tests, then you are by definition not testing what your desktop app is actually sending.

Anyway, we are not intending to turn the test client into a proper browser simulator. It is intended to do basic checks of views, and for that purpose it is nicer to separate concerns and avoid the CSRF checks altogether. So I'm closing WONTFIX.


comment:2 Changed 6 years ago by Russell Keith-Magee

Resolution: wontfix
Status: closedreopened
Triage Stage: UnreviewedAccepted

Luke - I'm going to reopen (and very soon, close again with a fix). Specifically, I think the change you suggest -- a client that *doesn't* disable CSRF protection -- is something that could be useful. I've just hit this problem writing tests for contrib.flatpages trying to fix #14156. This is a problem that is specifically about checking the way CSRF protection is handled; while it *could* be tested in other ways, a simple client request is easiest to read and understand by quite some margin.

I agree that *most* people won't need this option, but there is an edge case where it can be useful.

comment:3 Changed 6 years ago by Russell Keith-Magee

Resolution: fixed
Status: reopenedclosed

(In [13640]) Fixed #14116 -- Added a flag to enable CSRF checks in the test client. Thanks to jon@… for the suggestion.

comment:4 Changed 6 years ago by Russell Keith-Magee

(In [13642]) [1.2.X] Fixed #14116 -- Added a flag to enable CSRF checks in the test client. Thanks to jon@… for the suggestion.

Backport of r13640 from trunk.

Note: See TracTickets for help on using tickets.
Back to Top