Canonical SSO provider

Merge lp:~matt-goodall/canonical-identity-provider/new-account-head into lp:canonical-identity-provider/release

  1. new-account-head
  2. Merge into trunk
Proposed by Matt Goodall
Status: Merged
Approved by: Natalia Bidart
Approved revision: no longer in the source branch.
Merged at revision: 1313
Proposed branch: lp:~matt-goodall/canonical-identity-provider/new-account-head
Merge into: lp:canonical-identity-provider/release
Diff against target: 36 lines (+8/-0)
2 files modified
src/webui/tests/test_views_registration.py (+6/-0)
src/webui/views/registration.py (+2/-0)
To merge this branch: bzr merge lp:~matt-goodall/canonical-identity-provider/new-account-head
Reviewer Review Type Date Requested Status
Natalia Bidart (community) Approve
Review via email: mp+266388@code.launchpad.net

Commit message

Handle HEAD requests to the new account view.

Description of the change

Handle HEAD requests to the new account view. Fixes https://oops.canonical.com/oops/?oopsid=OOPS-d773f1cd8e9f42d492880f3df2d05110.

To post a comment you must log in.
Revision history for this message
Natalia Bidart (nataliabidart) wrote :

Hi Matt!

Thanks for working on this. I'm a bit surprised by this solution, meaning that we usually don't allow HEAD requests to views that are not meant to HEAD to. IMHO, is also an odd pattern in django views to return the response for a GET in a HEAD.

What we usually do to solve the oops is decorate the view with @require_http_methods(['GET', 'POST']) to avoid this oopses (that will return 405 to caller on a HEAD request).
Do you see any reason not to do that in this branch?

review: Needs Information
Revision history for this message
Natalia Bidart (nataliabidart) wrote :

Looks good!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/webui/tests/test_views_registration.py'
2--- src/webui/tests/test_views_registration.py 2015-06-29 20:38:49 +0000
3+++ src/webui/tests/test_views_registration.py 2015-07-30 13:20:59 +0000
4@@ -106,6 +106,12 @@
5 self.assert_form_displayed(response)
6 self.assert_stat_calls(['requested'])
7
8+ def test_head(self):
9+ request = self.factory.make_request(
10+ self.URL, method='HEAD', user=AnonymousUser())
11+ response = new_account(request)
12+ self.assertEqual(response.status_code, 405)
13+
14 def test_get_with_email(self):
15 response = self.get(email='test@test.com')
16 ctx = response.context_data
17
18=== modified file 'src/webui/views/registration.py'
19--- src/webui/views/registration.py 2015-07-09 18:35:57 +0000
20+++ src/webui/views/registration.py 2015-07-30 13:20:59 +0000
21@@ -17,6 +17,7 @@
22 )
23 from django.utils.decorators import method_decorator
24 from django.utils.translation import ugettext_lazy as _
25+from django.views.decorators.http import require_http_methods
26 from django.views.generic.base import View, TemplateResponseMixin
27
28 from gargoyle import gargoyle
29@@ -85,6 +86,7 @@
30 @redirect_home_if_logged_in
31 @check_readonly
32 @requires_cookies
33+@require_http_methods(['GET', 'POST'])
34 def new_account(request, token=None):
35 captcha_required = gargoyle.is_active('CAPTCHA', request)
36 captcha_error = ''