Merge lp:~gary/launchpad/bug538236 into lp:launchpad

[Gary Poster]

add an exception and an exception view for the case of a browser not including a REFERER header.

This fixes bug 538236 by adding an exception and an exception view for the case of a browser not including a REFERER header.

Tests passed on EC2.

This needs a code and a ui review, and is also a release-critical candidate.

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/canonical/launchpad/pagetests/standalone/xx-offsite-form-post.txt
  lib/canonical/launchpad/templates/launchpad-noreferrer.pt
  lib/canonical/launchpad/webapp/error.py
  lib/canonical/launchpad/webapp/interfaces.py
  lib/canonical/launchpad/webapp/publication.py
  lib/canonical/launchpad/zcml/launchpad.zcml

== Pylint notices ==

lib/canonical/launchpad/webapp/interfaces.py
    16: [F0401] Unable to import 'lazr.batchnavigator.interfaces' (No module named batchnavigator)
    17: [F0401] Unable to import 'lazr.enum' (No module named enum)

lib/canonical/launchpad/webapp/publication.py
    63: [F0401] Unable to import 'lazr.uri' (No module named uri)

[Gary Poster (gary)]

This is a screenshot of the new error page, for UI review.

http://launchpadlibrarian.net/42379021/Screen%20shot%202010-03-29%20at%209.26.18%20AM.png

[Guilherme Salgado (salgado)]

Hi Gary,

This looks good. I have just one suggestion below.

 review approve

On Sat, 2010-03-27 at 03:39 +0000, Gary Poster wrote:
> === modified file 'lib/canonical/launchpad/pagetests/standalone/xx-offsite-form-post.txt'
> --- lib/canonical/launchpad/pagetests/standalone/xx-offsite-form-post.txt 2010-03-19 20:02:25 +0000
> +++ lib/canonical/launchpad/pagetests/standalone/xx-offsite-form-post.txt 2010-03-27 03:39:20 +0000
> @@ -66,10 +66,43 @@
> >>> browser.getControl('Create').click()
> Traceback (most recent call last):
> ...
> - OffsiteFormPostError: No value for REFERER header
> -
> -Though, to support apport, we let POST requests without a REFERER header
> -go through to +storeblob.
> + NoReferrerError: No value for REFERER header
> +
> +When a POST request is rejected because the REFERER header is missing, it
> +may be because the user is trying to enforce anonymity. Therefore, we
> +present a hopefully helpful error message.
> +
> + >>> browser.handleErrors = True
> + >>> browser.open('http://launchpad.dev/people/+newteam')
> + MONKEYPATCH ACTIVATED
> + >>> browser.getControl('Name', index=0).value = 'team3'
> + >>> browser.getControl('Display Name').value = 'Team 3'
> + >>> browser.getControl('Create').click()
> + Traceback (most recent call last):
> + ...
> + HTTPError...
> + >>> print browser.headers['status']
> + 403 Forbidden
> + >>> print extract_text(find_main_content(browser.contents))
> + No REFERER Header
> + Launchpad requires a REFERER header to perform
> + this action. There is no REFERER header present.
> + Perhaps you have blocked REFERER headers or perhaps
> + you navigated to this URL in an unexpected way.
> + Unblock REFERER headers for launchpad.net and try
> + again, or see this
> + FAQ for more information.
> + You can also join the
> + #launchpad IRC support channel on irc.freenode.net for further
> + assistance.

I'd elide most of the text above, to make the test less likely to
(unnecessarily) fail when somebody changes the page's content.

> + >>> browser.getLink('this FAQ').url
> + 'https://answers.edge.launchpad.net/launchpad/+faq/1024'
> + >>> browser.handleErrors = False
> +
> +We have a few exceptional cases in which we allow POST requests without a
> +REFERER header.
> +
> +To support apport, we allow it for +storeblob.
>
> >>> browser.post('http://launchpad.dev/+storeblob', 'x=1')
> MONKEYPATCH ACTIVATED
> @@ -95,7 +128,7 @@
> We also let the request go through if the referrer is from a site
> managed by launchpad:
>
> - # Go behing the curtains and change the hostname of one of our sites so that
> + # Go behind the curtains and change the hostname of one of our sites so that
> # we can test this.
> >>> from canonical.launchpad.webapp.vhosts import allvhosts
> >>> allvhosts._hostnames.add('bzr.dev')

--
Guilherme Salgado <email address hidden>

review: Approve

[Michael Nelson (michael.nelson)]

> This is a screenshot of the new error page, for UI review.
>
> http://launchpadlibrarian.net/42379021/Screen%20shot%202010-03-29%20at%209.26.
> 18%20AM.png

Hi Gary,

The language is nice and clear. I have two suggestions:

1. I think it would be better to use a descriptive link text instead of "this FAQ", perhaps "You can find out more information in the <link>FAQ: the relevant faq title</link>."

2. Instead of "Perhaps you have....", I think this should be "This can be caused by..." (but mrevell is the wordsmith around here if you want a second opinion :)). See what you think.

Thanks!

review: Approve (ui)

[Deryck Hodge (deryck)]

review: Approve (release-critical)

[Gary Poster (gary)]

review: Abstain

=== modified file 'lib/canonical/launchpad/pagetests/standalone/xx-offsite-form-post.txt'
--- lib/canonical/launchpad/pagetests/standalone/xx-offsite-form-post.txt	2010-03-19 20:02:25 +0000
+++ lib/canonical/launchpad/pagetests/standalone/xx-offsite-form-post.txt	2010-03-29 16:14:26 +0000
@@ -66,10 +66,34 @@
   >>> browser.getControl('Create').click()
   Traceback (most recent call last):
     ...
-  OffsiteFormPostError: No value for REFERER header
-
-Though, to support apport, we let POST requests without a REFERER header
-go through to +storeblob.
+  NoReferrerError: No value for REFERER header
+
+When a POST request is rejected because the REFERER header is missing, it
+may be because the user is trying to enforce anonymity.  Therefore, we
+present a hopefully helpful error message.
+
+  >>> browser.handleErrors = True
+  >>> browser.open('http://launchpad.dev/people/+newteam')
+  MONKEYPATCH ACTIVATED
+  >>> browser.getControl('Name', index=0).value = 'team3'
+  >>> browser.getControl('Display Name').value = 'Team 3'
+  >>> browser.getControl('Create').click()
+  Traceback (most recent call last):
+    ...
+  HTTPError...
+  >>> print browser.headers['status']
+  403 Forbidden
+  >>> print extract_text(find_main_content(browser.contents))
+  No REFERER Header
+  ...
+  >>> browser.getLink('the FAQ').url
+  'https://answers.edge.launchpad.net/launchpad/+faq/1024'
+  >>> browser.handleErrors = False
+
+We have a few exceptional cases in which we allow POST requests without a
+REFERER header.
+
+To support apport, we allow it for +storeblob.
 
   >>> browser.post('http://launchpad.dev/+storeblob', 'x=1')
   MONKEYPATCH ACTIVATED
@@ -95,7 +119,7 @@
 We also let the request go through if the referrer is from a site
 managed by launchpad:
 
-  # Go behing the curtains and change the hostname of one of our sites so that
+  # Go behind the curtains and change the hostname of one of our sites so that
   # we can test this.
   >>> from canonical.launchpad.webapp.vhosts import allvhosts
   >>> allvhosts._hostnames.add('bzr.dev')
@@ -108,7 +132,7 @@
   >>> print browser.url
   http://launchpad.dev/~team4
 
-  # Now retore our site's hostname.
+  # Now restore our site's hostname.
   >>> allvhosts._hostnames.remove('bzr.dev')
 
 Cheaters never prosper
@@ -131,7 +155,7 @@
   ...     'http://launchpad.dev/api/devel/people', 'ws.op=foo&x=1')
   Traceback (most recent call last):
   ...
-  OffsiteFormPostError: No value for REFERER header
+  NoReferrerError: No value for REFERER header
 
 You can't cheat by making your referrerless POST request seem as
 though it were signed with OAuth.
@@ -146,7 +170,7 @@
   ...     'http://launchpad.dev/', 'oauth_consumer_key=foo&oauth_token=bar')
   Traceback (most recent call last):
   ...
-  OffsiteFormPostError: No value for REFERER header
+  NoReferrerError: No value for REFERER header
 
 You might think you can actually sign a request with an anonymous
 OAuth credential. You don't need any knowledge of the user account to

=== added file 'lib/canonical/launchpad/templates/launchpad-noreferrer.pt'
--- lib/canonical/launchpad/templates/launchpad-noreferrer.pt	1970-01-01 00:00:00 +0000
+++ lib/canonical/launchpad/templates/launchpad-noreferrer.pt	2010-03-29 16:14:26 +0000
@@ -0,0 +1,26 @@
+<html
+  xmlns="http://www.w3.org/1999/xhtml"
+  xmlns:tal="http://xml.zope.org/namespaces/tal"
+  xmlns:metal="http://xml.zope.org/namespaces/metal"
+  xmlns:i18n="http://xml.zope.org/namespaces/i18n"
+  metal:use-macro="view/macro:page/locationless"
+  i18n:domain="launchpad"
+>
+  <body>
+    <div class="top-portlet"  metal:fill-slot="main">
+      <h1 class="exception">No <code>REFERER</code> Header</h1>
+      <p>Launchpad requires a <code>REFERER</code> header to perform
+      this action. There is no <code>REFERER</code> header present.
+      This can be caused by configuring your browser to block
+      <code>REFERER</code> headers.</p>
+      <p>Unblock <code>REFERER</code> headers for launchpad.net and try
+      again, or see <a
+      href="https://answers.edge.launchpad.net/launchpad/+faq/1024">the
+      FAQ <em>Why does Launchpad require a REFERER header?</em></a> for
+      more information.</p>
+      <p>You can also join <a href="irc://irc.freenode.net/launchpad">the
+      #launchpad IRC support channel on irc.freenode.net</a> for further
+      assistance.</p>
+    </div>
+  </body>
+</html>

=== modified file 'lib/canonical/launchpad/webapp/error.py'
--- lib/canonical/launchpad/webapp/error.py	2010-03-18 18:55:02 +0000
+++ lib/canonical/launchpad/webapp/error.py	2010-03-29 16:14:26 +0000
@@ -283,3 +283,8 @@
     def __call__(self):
         return self.index()
 
+
+class NoReferrerErrorView(SystemErrorView):
+    """View rendered when a POST request does not include a REFERER header."""
+
+    response_code = 403 # Forbidden.

=== modified file 'lib/canonical/launchpad/webapp/interfaces.py'
--- lib/canonical/launchpad/webapp/interfaces.py	2010-02-22 11:09:22 +0000
+++ lib/canonical/launchpad/webapp/interfaces.py	2010-03-29 16:14:26 +0000
@@ -109,6 +109,10 @@
     """An attempt was made to post a form from a remote site."""
 
 
+class NoReferrerError(Exception):
+    """At attempt was made to post a form without a REFERER header."""
+
+
 class UnsafeFormGetSubmissionError(Exception):
     """An attempt was made to submit an unsafe form action with GET."""
 

=== modified file 'lib/canonical/launchpad/webapp/publication.py'
--- lib/canonical/launchpad/webapp/publication.py	2010-03-23 19:07:44 +0000
+++ lib/canonical/launchpad/webapp/publication.py	2010-03-29 16:14:26 +0000
@@ -37,7 +37,7 @@
 from zope.interface import implements, providedBy
 from zope.publisher.interfaces import IPublishTraverse, Retry
 from zope.publisher.interfaces.browser import (
-    IDefaultSkin, IBrowserRequest, IBrowserApplicationRequest)
+    IDefaultSkin, IBrowserRequest)
 from zope.publisher.publish import mapply
 from zope.security.proxy import removeSecurityProxy
 from zope.security.management import newInteraction
@@ -55,13 +55,12 @@
 from canonical.launchpad.webapp.interfaces import (
     IDatabasePolicy, ILaunchpadRoot, INotificationResponse, IOpenLaunchBag,
     IPlacelessAuthUtility, IPrimaryContext, IStoreSelector, MAIN_STORE,
-    MASTER_FLAVOR, OffsiteFormPostError, SLAVE_FLAVOR)
+    MASTER_FLAVOR, OffsiteFormPostError, NoReferrerError, SLAVE_FLAVOR)
 from canonical.launchpad.webapp.dbpolicy import (
     DatabaseBlockedPolicy, LaunchpadDatabasePolicy)
 from canonical.launchpad.webapp.menu import structured
 from canonical.launchpad.webapp.opstats import OpStats
 from lazr.uri import URI, InvalidURIError
-from lazr.restful.interfaces import IWebServiceClientRequest
 from canonical.launchpad.webapp.vhosts import allvhosts
 
 
@@ -353,7 +352,7 @@
             return
         referrer = request.getHeader('referer') # match HTTP spec misspelling
         if not referrer:
-            raise OffsiteFormPostError('No value for REFERER header')
+            raise NoReferrerError('No value for REFERER header')
         # XXX: jamesh 2007-04-26 bug=98437:
         # The Zope testing infrastructure sets a default (incorrect)
         # referrer value of "localhost" or "localhost:9000" if no

=== modified file 'lib/canonical/launchpad/zcml/launchpad.zcml'
--- lib/canonical/launchpad/zcml/launchpad.zcml	2010-02-18 10:52:51 +0000
+++ lib/canonical/launchpad/zcml/launchpad.zcml	2010-03-29 16:14:26 +0000
@@ -82,6 +82,15 @@
       class="canonical.launchpad.webapp.error.SystemErrorView"
       />
 
+  <!-- NoReferrerError -->
+  <browser:page
+      for="canonical.launchpad.webapp.interfaces.NoReferrerError"
+      name="index.html"
+      permission="zope.Public"
+      template="../templates/launchpad-noreferrer.pt"
+      class="canonical.launchpad.webapp.error.NoReferrerErrorView"
+      />
+
   <!-- UnsafeFormGetSubmissionError -->
   <browser:page
       for="canonical.launchpad.webapp.interfaces.UnsafeFormGetSubmissionError"