Launchpad itself

Merge lp:~salgado/launchpad/vostok-no-external-redirect into lp:launchpad

  1. vostok-no-external-redirect
  2. Merge into devel
Proposed by Guilherme Salgado
Status: Merged
Approved by: Curtis Hovey
Approved revision: no longer in the source branch.
Merged at revision: 11359
Proposed branch: lp:~salgado/launchpad/vostok-no-external-redirect
Merge into: lp:launchpad
Prerequisite: lp:~salgado/launchpad/vostok-traverse-package
Diff against target: 80 lines (+44/-2)
2 files modified
lib/lp/vostok/publisher.py (+22/-1)
lib/lp/vostok/tests/test_publisher.py (+22/-1)
To merge this branch: bzr merge lp:~salgado/launchpad/vostok-no-external-redirect
Reviewer Review Type Date Requested Status
Curtis Hovey (community) code Approve
Review via email: mp+32623@code.launchpad.net

Commit message

Add a safety net so that vostok doesn't redirect users to any host other than vostok.dev

Description of the change

Make sure vostok doesn't redirect users to any host other than vostok.dev

To post a comment you must log in.
Revision history for this message
Curtis Hovey (sinzui) wrote :

We discussed some redirect scenarios on IRC. Changelogs and attachments are not an issue right now. If the librarian needs to be supported, we can add that. What is important is that redirects are locked down to host we trust.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/vostok/publisher.py'
--- lib/lp/vostok/publisher.py 2010-08-06 17:39:52 +0000
+++ lib/lp/vostok/publisher.py 2010-08-16 19:16:50 +0000
@@ -20,7 +20,8 @@
20from canonical.launchpad.webapp import canonical_url, Navigation20from canonical.launchpad.webapp import canonical_url, Navigation
21from canonical.launchpad.webapp.publication import LaunchpadBrowserPublication21from canonical.launchpad.webapp.publication import LaunchpadBrowserPublication
22from canonical.launchpad.webapp.servers import (22from canonical.launchpad.webapp.servers import (
23 LaunchpadBrowserRequest, VirtualHostRequestPublicationFactory)23 LaunchpadBrowserRequest, LaunchpadBrowserResponse,
24 VirtualHostRequestPublicationFactory)
24from canonical.launchpad.webapp.vhosts import allvhosts25from canonical.launchpad.webapp.vhosts import allvhosts
2526
26from lp.registry.interfaces.distribution import IDistributionSet27from lp.registry.interfaces.distribution import IDistributionSet
@@ -43,6 +44,26 @@
43class VostokBrowserRequest(VostokRequestMixin, LaunchpadBrowserRequest):44class VostokBrowserRequest(VostokRequestMixin, LaunchpadBrowserRequest):
44 """Request class for Vostok layer."""45 """Request class for Vostok layer."""
4546
47 def _createResponse(self):
48 """As per zope.publisher.browser.BrowserRequest._createResponse"""
49 return VostokBrowserResponse()
50
51
52class VostokBrowserResponse(LaunchpadBrowserResponse):
53
54 def redirect(self, location, status=None, trusted=False,
55 temporary_if_possible=False):
56 """Override the parent method to make redirects untrusted by default.
57
58 This is so that we don't allow redirects to any hosts other than
59 vostok's.
60 """
61 # Need to call LaunchpadBrowserResponse.redirect() directly because
62 # the temporary_if_possible argument only exists there.
63 LaunchpadBrowserResponse.redirect(
64 self, location, status=status, trusted=trusted,
65 temporary_if_possible=temporary_if_possible)
66
4667
47class IVostokRoot(Interface):68class IVostokRoot(Interface):
48 """Marker interface for the root vostok object."""69 """Marker interface for the root vostok object."""
4970
=== modified file 'lib/lp/vostok/tests/test_publisher.py'
--- lib/lp/vostok/tests/test_publisher.py 2010-08-05 01:08:24 +0000
+++ lib/lp/vostok/tests/test_publisher.py 2010-08-16 19:16:50 +0000
@@ -13,7 +13,8 @@
13from lp.testing import TestCase13from lp.testing import TestCase
14from lp.testing.publication import get_request_and_publication14from lp.testing.publication import get_request_and_publication
1515
16from lp.vostok.publisher import VostokLayer, VostokRoot16from lp.vostok.publisher import (
17 VostokBrowserResponse, VostokBrowserRequest, VostokLayer, VostokRoot)
1718
1819
19class TestRegistration(TestCase):20class TestRegistration(TestCase):
@@ -38,5 +39,25 @@
38 self.assertIsInstance(root, VostokRoot)39 self.assertIsInstance(root, VostokRoot)
3940
4041
42class TestVostokBrowserRequest(TestCase):
43
44 def test_createResponse(self):
45 request = VostokBrowserRequest(None, {})
46 self.assertIsInstance(
47 request._createResponse(), VostokBrowserResponse)
48
49
50class TestVostokBrowserResponse(TestCase):
51
52 def test_redirect_to_different_host(self):
53 # Unlike Launchpad's BrowserResponse class, VostokBrowserResponse
54 # doesn't allow redirects to any host other than the current one.
55 request = VostokBrowserRequest(None, {})
56 response = request._createResponse()
57 response._request = request
58 self.assertRaises(
59 ValueError, response.redirect, 'http://launchpad.dev')
60
61
41def test_suite():62def test_suite():
42 return unittest.TestLoader().loadTestsFromName(__name__)63 return unittest.TestLoader().loadTestsFromName(__name__)