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
1=== modified file 'lib/lp/vostok/publisher.py'
2--- lib/lp/vostok/publisher.py 2010-08-06 17:39:52 +0000
3+++ lib/lp/vostok/publisher.py 2010-08-16 19:16:50 +0000
4@@ -20,7 +20,8 @@
5 from canonical.launchpad.webapp import canonical_url, Navigation
6 from canonical.launchpad.webapp.publication import LaunchpadBrowserPublication
7 from canonical.launchpad.webapp.servers import (
8- LaunchpadBrowserRequest, VirtualHostRequestPublicationFactory)
9+ LaunchpadBrowserRequest, LaunchpadBrowserResponse,
10+ VirtualHostRequestPublicationFactory)
11 from canonical.launchpad.webapp.vhosts import allvhosts
12
13 from lp.registry.interfaces.distribution import IDistributionSet
14@@ -43,6 +44,26 @@
15 class VostokBrowserRequest(VostokRequestMixin, LaunchpadBrowserRequest):
16 """Request class for Vostok layer."""
17
18+ def _createResponse(self):
19+ """As per zope.publisher.browser.BrowserRequest._createResponse"""
20+ return VostokBrowserResponse()
21+
22+
23+class VostokBrowserResponse(LaunchpadBrowserResponse):
24+
25+ def redirect(self, location, status=None, trusted=False,
26+ temporary_if_possible=False):
27+ """Override the parent method to make redirects untrusted by default.
28+
29+ This is so that we don't allow redirects to any hosts other than
30+ vostok's.
31+ """
32+ # Need to call LaunchpadBrowserResponse.redirect() directly because
33+ # the temporary_if_possible argument only exists there.
34+ LaunchpadBrowserResponse.redirect(
35+ self, location, status=status, trusted=trusted,
36+ temporary_if_possible=temporary_if_possible)
37+
38
39 class IVostokRoot(Interface):
40 """Marker interface for the root vostok object."""
41
42=== modified file 'lib/lp/vostok/tests/test_publisher.py'
43--- lib/lp/vostok/tests/test_publisher.py 2010-08-05 01:08:24 +0000
44+++ lib/lp/vostok/tests/test_publisher.py 2010-08-16 19:16:50 +0000
45@@ -13,7 +13,8 @@
46 from lp.testing import TestCase
47 from lp.testing.publication import get_request_and_publication
48
49-from lp.vostok.publisher import VostokLayer, VostokRoot
50+from lp.vostok.publisher import (
51+ VostokBrowserResponse, VostokBrowserRequest, VostokLayer, VostokRoot)
52
53
54 class TestRegistration(TestCase):
55@@ -38,5 +39,25 @@
56 self.assertIsInstance(root, VostokRoot)
57
58
59+class TestVostokBrowserRequest(TestCase):
60+
61+ def test_createResponse(self):
62+ request = VostokBrowserRequest(None, {})
63+ self.assertIsInstance(
64+ request._createResponse(), VostokBrowserResponse)
65+
66+
67+class TestVostokBrowserResponse(TestCase):
68+
69+ def test_redirect_to_different_host(self):
70+ # Unlike Launchpad's BrowserResponse class, VostokBrowserResponse
71+ # doesn't allow redirects to any host other than the current one.
72+ request = VostokBrowserRequest(None, {})
73+ response = request._createResponse()
74+ response._request = request
75+ self.assertRaises(
76+ ValueError, response.redirect, 'http://launchpad.dev')
77+
78+
79 def test_suite():
80 return unittest.TestLoader().loadTestsFromName(__name__)