launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03081
[Merge] lp:~wgrant/launchpad/delete-librarian-streaming into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/delete-librarian-streaming into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #743960 in Launchpad itself: "Remove StreamOrRedirectLibraryFileAliasView"
https://bugs.launchpad.net/launchpad/+bug/743960
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/delete-librarian-streaming/+merge/55039
The public restricted librarian has been enabled on production for several months now, and it has been working fine. This branch cleans up by removing the old streaming views, and merging the token redirect code into LibraryFileAlias.getURL().
The only significant functional change is that FileNavigation no longer returns a custom LibraryFileAlias view which redirects: it redirects to the librarian URL itself. We would ideally just return the LFA, leaving LibraryFileAliasView to do the dirty work. But we don't know that LFAV isn't accessible on some restricted LFAs that it shouldn't be, so allowing LFAV to request tokens for arbitrary files would be Bad.
Some tests needed significant mangling, since restricted file redirects don't work on dev/test environments any more (no wildcard DNS). EC2 will find more, I'm sure.
--
https://code.launchpad.net/~wgrant/launchpad/delete-librarian-streaming/+merge/55039
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/delete-librarian-streaming into lp:launchpad.
=== modified file 'lib/canonical/launchpad/browser/librarian.py'
--- lib/canonical/launchpad/browser/librarian.py 2011-02-15 05:53:58 +0000
+++ lib/canonical/launchpad/browser/librarian.py 2011-03-28 05:33:32 +0000
@@ -11,40 +11,25 @@
'LibraryFileAliasMD5View',
'LibraryFileAliasView',
'ProxiedLibraryFileAlias',
- 'SafeStreamOrRedirectLibraryFileAliasView',
- 'StreamOrRedirectLibraryFileAliasView',
- 'RedirectPerhapsWithTokenLibraryFileAliasView',
]
-import os
-import tempfile
-import urllib2
-
from lazr.delegates import delegates
-from zope.interface import implements
from zope.publisher.interfaces import NotFound
-from zope.publisher.interfaces.browser import IBrowserPublisher
from zope.security.interfaces import Unauthorized
from canonical.launchpad.interfaces.librarian import ILibraryFileAlias
from canonical.launchpad.layers import WebServiceLayer
from canonical.launchpad.webapp.authorization import check_permission
+from canonical.launchpad.webapp.publisher import RedirectionView
from lazr.restful.interfaces import IWebBrowserOriginatingRequest
from canonical.launchpad.webapp.publisher import (
canonical_url,
LaunchpadView,
- RedirectionView,
stepthrough,
)
from canonical.launchpad.webapp.url import urlappend
from canonical.lazr.utils import get_current_browser_request
from canonical.librarian.client import url_path_quote
-from canonical.librarian.interfaces import LibrarianServerError
-from canonical.librarian.utils import (
- filechunks,
- guess_librarian_encoding,
- )
-from lp.services.features import getFeatureFlag
class LibraryFileAliasView(LaunchpadView):
@@ -57,16 +42,24 @@
def initialize(self):
"""Redirect the request to the URL of the file in the Librarian."""
- # Redirect based on the scheme of the request, as set by Apache in the
- # 'X-SCHEME' environment variable, which is mapped to 'HTTP_X_SCHEME.
- # Note that only some requests for librarian files are allowed to come
- # in via http as most are forced to https via Apache redirection.
- request_scheme = self.request.get('HTTP_X_SCHEME')
- if request_scheme == 'http':
- redirect_to = self.context.http_url
- else:
- redirect_to = self.context.getURL()
- self.request.response.redirect(redirect_to)
+ # Refuse to serve restricted files. We're not sure that no
+ # restricted files are being leaked in the traversal hierarchy.
+ assert not self.context.restricted
+ # Perhaps we should give a 404 at this point rather than asserting?
+ # If someone has a page open with an attachment link, then someone
+ # else deletes the attachment, this is a normal situation, not an
+ # error. -- RBC 20100726.
+ assert not self.context.deleted, (
+ "LibraryFileAliasView can not operate on deleted librarian files,"
+ " since their URL is undefined.")
+ # Redirect based on the scheme of the request, as set by
+ # Apache in the 'X-SCHEME' environment variable, which is
+ # mapped to 'HTTP_X_SCHEME. Note that only some requests
+ # for librarian files are allowed to come in via http as
+ # most are forced to https via Apache redirection.
+ self.request.response.redirect(
+ self.context.getURL(
+ secure=self.request.get('HTTP_X_SCHEME') != 'http'))
class LibraryFileAliasMD5View(LaunchpadView):
@@ -78,146 +71,6 @@
return '%s %s' % (self.context.content.md5, self.context.filename)
-class MixedFileAliasView(LaunchpadView):
- """Stream or redirects to `ILibraryFileAlias`.
-
- If the file is public, it will redirect to the files http url.
-
- Otherwise if the feature flag publicrestrictedlibrarian is true this will
- allocate a token and redirect to the aliases private url.
-
- Otherwise it will proxy the file in the appserver.
-
- Once we no longer have any proxy code at all it should be possible to
- consolidate this with LibraryFileAliasView.
-
- Note that streaming restricted files is a security concern - they show up
- in the launchpad.net domain rather than launchpadlibrarian.net and thus
- we have to take special care about their origin.
- SafeStreamOrRedirectLibraryFileAliasView is used when we do not trust the
- content, otherwise StreamOrRedirectLibraryFileAliasView. We are working
- to remove both of these views entirely, but some transition will be
- needed.
-
- The context provides a file-like interface - it can be opened and closed
- and read from.
- """
- implements(IBrowserPublisher)
-
- def getFileContents(self):
- # Reset system proxy setting if it exists. The urllib2 default
- # opener is cached that's why it has to be re-installed after
- # the shell environment changes. Download the library file
- # content into a local temporary file. Finally, restore original
- # proxy-settings and refresh the urllib2 opener.
- # XXX: This is not threadsafe, so two calls at once will collide and
- # can then corrupt the variable. bug=395960
- original_proxy = os.getenv('http_proxy')
- try:
- if original_proxy is not None:
- del os.environ['http_proxy']
- urllib2.install_opener(urllib2.build_opener())
- tmp_file = tempfile.TemporaryFile()
- self.context.open()
- for chunk in filechunks(self.context):
- tmp_file.write(chunk)
- self.context.close()
- finally:
- if original_proxy is not None:
- os.environ['http_proxy'] = original_proxy
- urllib2.install_opener(urllib2.build_opener())
- return tmp_file
-
- def __call__(self):
- """Streams the contents of the context `ILibraryFileAlias`.
-
- The file content is downloaded in chunks directly to a
- `tempfile.TemporaryFile` avoiding using large amount of memory.
-
- The temporary file is returned to the zope publishing machinery as
- documented in lib/zope/publisher/httpresults.txt, after adjusting
- the response 'Content-Type' appropriately.
-
- This method explicit ignores the local 'http_proxy' settings.
- """
- try:
- tmp_file = self.getFileContents()
- except LibrarianServerError:
- self.request.response.setHeader('Content-Type', 'text/plain')
- self.request.response.setStatus(503)
- return (u'There was a problem fetching the contents of this '
- 'file. Please try again in a few minutes.')
-
- # XXX: Brad Crittenden 2007-12-05 bug=174204: When encodings are
- # stored as part of a file's metadata this logic will be replaced.
- encoding, mimetype = guess_librarian_encoding(
- self.context.filename, self.context.mimetype)
-
- self.request.response.setHeader('Content-Encoding', encoding)
- self.request.response.setHeader('Content-Type', mimetype)
- return tmp_file
-
- def browserDefault(self, request):
- """Decides how to deliver the file.
-
- The options are:
- - redirect to the contexts http url
- - redirect to a time limited secure url
- - stream the file content.
-
- Only restricted file contents are streamed, finishing the traversal
- chain with this view. If the context file is public return the
- appropriate `RedirectionView` for its HTTP url.
- """
- # Perhaps we should give a 404 at this point rather than asserting?
- # If someone has a page open with an attachment link, then someone
- # else deletes the attachment, this is a normal situation, not an
- # error. -- RBC 20100726.
- assert not self.context.deleted, (
- "MixedFileAliasView can not operate on deleted librarian files, "
- "since their URL is undefined.")
- if not self.context.restricted:
- # Public file, just point the client at the right place.
- return RedirectionView(self.context.http_url, self.request), ()
- if not getFeatureFlag(u'publicrestrictedlibrarian'):
- # Restricted file and we have not enabled the public
- # restricted librarian yet :- deliver inline.
- self._when_streaming()
- return self, ()
- # Avoids a circular import seen in
- # scripts/ftests/librarianformatter.txt
- from canonical.launchpad.database.librarian import TimeLimitedToken
- # Allocate a token for the file to be accessed for a limited time and
- # redirect the client to the file.
- token = TimeLimitedToken.allocate(self.context.private_url)
- final_url = self.context.private_url + '?token=%s' % token
- return RedirectionView(final_url, self.request), ()
-
- def publishTraverse(self, request, name):
- """See `IBrowserPublisher` - can't traverse below a file."""
- raise NotFound(name, self.context)
-
- def _when_streaming(self):
- """Hook for SafeStreamOrRedirectLibraryFileAliasView."""
-
-
-class SafeStreamOrRedirectLibraryFileAliasView(MixedFileAliasView):
- """A view for Librarian files that sets the content disposition header."""
-
- def _when_streaming(self):
- super(
- SafeStreamOrRedirectLibraryFileAliasView, self)._when_streaming()
- self.request.response.setHeader(
- 'Content-Disposition', 'attachment')
-
-
-# The eventual name of MixedFileAliasView once the proxy code
-# is ripped out.
-RedirectPerhapsWithTokenLibraryFileAliasView = MixedFileAliasView
-# The name for the old behaviour being removed:
-StreamOrRedirectLibraryFileAliasView = MixedFileAliasView
-
-
class DeletedProxiedLibraryFileAlias(NotFound):
"""Raised when a deleted `ProxiedLibraryFileAlias` is accessed."""
@@ -228,15 +81,14 @@
The navigation goes through +files/<filename> where file reference is
provided by context `getFileByName(filename)`.
- The requested file is proxied via `StreamOrRedirectLibraryFileAliasView`,
- making it possible to serve both, public and restricted, files.
+ The requested file is proxied via `LibraryFileAliasView`,
+ making it possible to serve both public and restricted files.
This navigation approach only supports domains with unique filenames,
which is the case of IArchive and IBuild. It will probably have to be
extended in order to allow traversing to multiple files potentially
with the same filename (product files or bug attachments).
"""
- view_class = StreamOrRedirectLibraryFileAliasView
@stepthrough('+files')
def traverse_files(self, filename):
@@ -249,7 +101,9 @@
if library_file.deleted:
raise DeletedProxiedLibraryFileAlias(filename, self.context)
- return self.view_class(library_file, self.request)
+ return RedirectionView(
+ library_file.getURL(include_token=True),
+ self.request)
class ProxiedLibraryFileAlias:
=== removed file 'lib/canonical/launchpad/browser/tests/test_librarian.py'
--- lib/canonical/launchpad/browser/tests/test_librarian.py 2010-09-03 07:53:11 +0000
+++ lib/canonical/launchpad/browser/tests/test_librarian.py 1970-01-01 00:00:00 +0000
@@ -1,31 +0,0 @@
-# Copyright 2010 Canonical Ltd. All rights reserved.
-
-__metaclass__ = type
-
-__all__ = []
-
-from canonical.launchpad.browser.librarian import (
- StreamOrRedirectLibraryFileAliasView,
- )
-from canonical.launchpad.webapp.servers import LaunchpadTestRequest
-from canonical.librarian.interfaces import LibrarianServerError
-from lp.testing import TestCase
-
-
-class FakeRestrictedLibraryFileAlias:
- deleted = False
- restricted = True
-
- def open(self):
- raise LibrarianServerError('Librarian is down')
-
-
-class TestStreamOrRedirectLibraryFileAliasView(TestCase):
-
- def test_restricted_file_when_librarian_is_down(self):
- view = StreamOrRedirectLibraryFileAliasView(
- FakeRestrictedLibraryFileAlias(), LaunchpadTestRequest())
- html = view()
- self.assertEqual(503, view.request.response.getStatus())
- self.assertIn(
- 'There was a problem fetching the contents of this file', html)
=== modified file 'lib/canonical/launchpad/database/librarian.py'
--- lib/canonical/launchpad/database/librarian.py 2011-01-20 19:39:08 +0000
+++ lib/canonical/launchpad/database/librarian.py 2011-03-28 05:33:32 +0000
@@ -140,14 +140,19 @@
"""See ILibraryFileAlias.https_url"""
return self.client.getURLForAlias(self.id, secure=True)
- def getURL(self):
+ def getURL(self, secure=True, include_token=False):
"""See ILibraryFileAlias.getURL"""
- if self.restricted:
- return self.private_url
- if config.librarian.use_https:
- return self.https_url
+ if not self.restricted:
+ if config.librarian.use_https and secure:
+ return self.https_url
+ else:
+ return self.http_url
else:
- return self.http_url
+ url = self.private_url
+ if include_token:
+ token = TimeLimitedToken.allocate(url)
+ url += '?token=%s' % token
+ return url
_datafile = None
=== modified file 'lib/canonical/launchpad/doc/librarian.txt'
--- lib/canonical/launchpad/doc/librarian.txt 2011-03-23 16:28:51 +0000
+++ lib/canonical/launchpad/doc/librarian.txt 2011-03-28 05:33:32 +0000
@@ -314,17 +314,45 @@
>>> file_alias.restricted
True
+ >>> transaction.commit()
+ >>> file_alias.open()
+ >>> print file_alias.read()
+ This is private data.
+ >>> file_alias.close()
+
+Restricted files are accessible with HTTP on a private domain.
+
>>> print file_alias.http_url
http://.../private.txt
>>> file_alias.http_url.startswith(
... config.librarian.restricted_download_url)
True
- >>> transaction.commit()
- >>> file_alias.open()
- >>> print file_alias.read()
- This is private data.
- >>> file_alias.close()
+They can also be accessed externally using a time-limited token appended
+to their private_url. Possession of a token is sufficient to grant access
+to a file, regardless of who is logged in. getURL can be asked to provide
+such a token.
+
+ >>> token_url = file_alias.getURL(include_token=True)
+ >>> print token_url
+ https://i...restricted.../private.txt?token=...
+ >>> token_url.startswith('https://i%d.restricted.' % file_alias.id)
+ True
+ >>> private_path = TimeLimitedToken.url_to_token_path(
+ ... file_alias.private_url)
+ >>> token_url.endswith(session_store().find(TimeLimitedToken,
+ ... path=private_path).any().token)
+ True
+
+LibraryFileAliasView doesn't work on restricted files. This is a temporary
+measure until we're sure no restricted files leak into the traversal
+hierarchy.
+
+ >>> view = getMultiAdapter((file_alias, request), name='+index')
+ >>> view.initialize()
+ Traceback (most recent call last):
+ ...
+ AssertionError
If you try to retrieve this file through the standard ILibrarianClient,
you'll get a DownloadFailed error.
@@ -513,11 +541,9 @@
... 'text2.txt', len(data), StringIO(data), 'text/plain',
... NEVER_EXPIRES)
>>> transaction.commit()
- >>> lfa_view = getMultiAdapter((alias,req), name='+index')
+ >>> lfa_view = getMultiAdapter((alias, req), name='+index')
>>> lfa_view.initialize()
- >>> lfa_url = lfa_view.context.getURL()
- >>> url = alias.getURL()
- >>> lfa_url == url
+ >>> req.response.getHeader("Location") == alias.getURL()
True
@@ -562,226 +588,6 @@
>>> _ = session_store().find(TimeLimitedToken).remove()
-== RedirectPerhapsWithTokenLibraryFileAliasView ==
-
-This is the eventual name of StreamOrRedirectLibraryFileAliasView once we have
-migrated over to the token based approach. For now the code for the two things
-is interleaved: zope's registration system is too static to permit doing it
-cleanly with separate classes.
-
-To enable the behaviour we want to test while its controlled by a feature
-flag, we must enable a feature flag for it:
-
- >>> from lp.services.features.model import FeatureFlag, getFeatureStore
- >>> ignore = getFeatureStore().add(FeatureFlag(
- ... scope=u'default', flag=u'publicrestrictedlibrarian', value=u'on',
- ... priority=1))
- >>> transaction.commit()
-
-This special view allows granting access to restricted `LibraryFileAlias`
-objects on the launchpadlibrarian. All requests get redirected, restricted
-files have an access token allocated.
-
-This view implements `IBrowserPublisher` which allows us to use it in
-traversals.
-
- >>> from canonical.launchpad.browser.librarian import (
- ... RedirectPerhapsWithTokenLibraryFileAliasView)
-
- >>> empty_request = LaunchpadTestRequest()
-
-XXX bug=631884 we have to establish the flags object manually for testing.
-
- >>> from lp.services.features import install_feature_controller
- >>> from lp.services.features.flags import FeatureController
- >>> from lp.services.features.webapp import ScopesFromRequest
- >>> install_feature_controller(FeatureController(
- ... ScopesFromRequest(empty_request).lookup))
-
- >>> restricted_view = RedirectPerhapsWithTokenLibraryFileAliasView(
- ... restricted_file, empty_request)
-
- >>> from zope.publisher.interfaces.browser import IBrowserPublisher
- >>> verifyObject(IBrowserPublisher, restricted_view)
- True
-
-When the context file is a restricted `LibraryFileAlias`, traversal causes an
-access token to be allocated and a redirection to https on a unique domain to
-be issued.
-
- >>> view, extra = restricted_view.browserDefault(empty_request)
- >>> print view
- <...RedirectionView...>
- >>> view.target
- 'https://...restricted.txt?token=...'
- >>> private_path = TimeLimitedToken.url_to_token_path(
- ... restricted_file.private_url)
- >>> view.target.endswith(session_store().find(TimeLimitedToken,
- ... path=private_path).any().token)
- True
-
-The domain for the target starts with the id of the LibraryFileAlias.
-
- >>> view.target.startswith('https://i%d.restricted.' % restricted_file.id)
- True
-
-Otherwise, no token is allocated and a redirection is issued to the http url.
-
- >>> public_view = RedirectPerhapsWithTokenLibraryFileAliasView(
- ... public_file, empty_request)
- >>> verifyObject(IBrowserPublisher, public_view)
- True
- >>> view, extra = public_view.browserDefault(empty_request)
- >>> print view
- <...RedirectionView ...>
- >>> view.target == public_file.http_url
- True
- >>> view.target
- 'http://.../public.txt'
-
-Deleted files cannot be handled by the view and it raises an appropriate
-AssertionError when it gets traversed.
-
- >>> deleted_view = RedirectPerhapsWithTokenLibraryFileAliasView(
- ... deleted_file, empty_request)
-
- >>> view, extra = deleted_view.browserDefault(empty_request)
- Traceback (most recent call last):
- ...
- AssertionError: MixedFileAliasView can not operate on deleted librarian
- files, since their URL is undefined.
-
-
-As RedirectPerhapsWithTokenLibraryFileAliasView is
-StreamOrRedirectLibraryFileAliasView influence by a feature flag, until the
-proxy code is removed, we must remove the feature to test the old behaviour.
-
- >>> ignore = getFeatureStore().find(FeatureFlag,
- ... FeatureFlag.flag==u'publicrestrictedlibrarian').remove()
- >>> transaction.commit()
-
-Because the flags code aggressively caches, we have to do a small dance to
-convince it a new request has started too.
-
- >>> empty_request = LaunchpadTestRequest()
- >>> install_feature_controller(FeatureController(
- ... ScopesFromRequest(empty_request).lookup))
-
-== StreamOrRedirectLibraryFileAliasView ==
-
-This special view allows call sites to stream restricted `LibraryFileAlias`
-contents directly from launchpad instances.
-
-This view implements `IBrowserPublisher` which allows us to use it in
-traversals.
-
- >>> from canonical.launchpad.browser.librarian import (
- ... StreamOrRedirectLibraryFileAliasView)
-
- >>> empty_request = LaunchpadTestRequest()
- >>> restricted_view = StreamOrRedirectLibraryFileAliasView(
- ... restricted_file, empty_request)
-
- >>> from zope.publisher.interfaces.browser import IBrowserPublisher
- >>> verifyObject(IBrowserPublisher, restricted_view)
- True
-
-When the context file is a restricted `LibraryFileAlias`, the
-traversal chain is finished with the view object itself.
-
- >>> view, extra = restricted_view.browserDefault(empty_request)
- >>> if view != restricted_view:
- ... print view
- >>> view == restricted_view
- True
-
-The zope publishing system will then call that view and receive an open
-file containig the content to be streamed, as documented in
-lib/zope/publisher/httpresults.txt. This approach minimises the memory
-usage when streaming huge files.
-
- >>> result = view()
- >>> print result
- <open file '<fdopen>', mode 'w+b' ...>
-
- >>> result.seek(0)
- >>> print result.read()
- RESTRICTED
-
-`StreamOrRedirectLibraryFileAliasView` ignores the system http_proxy
-configuration and is still able to access the restricted file even if
-a invalid proxy address is set.
-
-This approach was adopted because all launchpad machines are
-configured to use the same proxy, originally for external
-access. Openning the restricted librarian for the proxy host would be
-too permissive and that's why we only allow direct access from the
-app-servers.
-
-Let's configure a broken proxy and refresh the default urllib2 opener
-to follow it.
-
- >>> import os
- >>> import urllib2
- >>> os.environ['http_proxy'] = 'http://foo.bar:1234'
- >>> urllib2.install_opener(urllib2.build_opener())
-
-The file can be reached by `StreamOrRedirectLibraryFileAliasView`.
-
- >>> result = view()
- >>> result.seek(0)
- >>> print result.read()
- RESTRICTED
-
-But is unreachable in the test scope, due to the broken proxy
-settings.
-
- >>> print os.environ['http_proxy']
- http://foo.bar:1234
-
- # AaronBentley 2009-02-13 bug=329140
- # This causes a spurious test failure on systems that use wildcard DNS.
- #>>> print restricted_file.read()
- Traceback (most recent call last):
- ...
- URLError: <urlopen error (-2, 'Name or service not known')>
-
-Let's remove the broken proxy settings to avoid compromising other
-tests.
-
- >>> del os.environ['http_proxy']
- >>> urllib2.install_opener(urllib2.build_opener())
-
-When the context given to `StreamOrRedirectLibraryFileAliasView` is a
-public `LibraryFileAlias` the traversal chain is extended with a
-`RedirectionView` pointing to its public librarian url.
-
- >>> public_view = StreamOrRedirectLibraryFileAliasView(
- ... public_file, empty_request)
-
- >>> verifyObject(IBrowserPublisher, public_view)
- True
-
- >>> view, extra = public_view.browserDefault(empty_request)
-
- >>> print view
- <...RedirectionView ...>
-
- >>> view.target == public_file.http_url
- True
-
-Files in this condition cannot be handled by the view and it raises an
-appropriate AssertionError when it gets traversed.
-
- >>> deleted_view = StreamOrRedirectLibraryFileAliasView(
- ... deleted_file, empty_request)
-
- >>> view, extra = deleted_view.browserDefault(empty_request)
- Traceback (most recent call last):
- ...
- AssertionError: MixedFileAliasView can not operate on deleted librarian
- files, since their URL is undefined.
-
== LibraryFileAliasMD5View ==
=== modified file 'lib/canonical/launchpad/interfaces/librarian.py'
--- lib/canonical/launchpad/interfaces/librarian.py 2010-12-20 17:42:47 +0000
+++ lib/canonical/launchpad/interfaces/librarian.py 2011-03-28 05:33:32 +0000
@@ -83,18 +83,18 @@
https_url = Attribute(_("The https URL to this file"))
private_url = Attribute(_("The secure URL to this file (private files)"))
- def getURL():
+ def getURL(secure=True, include_token=False):
"""Return this file's http or https URL.
If the file is a restricted file, the private_url will be returned,
which is on https and uses unique domains per file alias.
- The generated URL will be https if the use_https config variable is
- set, in order to prevent warnings about insecure objects from
- happening in some browsers (this is used for, e.g., branding).
-
- If config.launchpad.virtual_host.use_https is set, then return the
- https URL. Otherwise return the http URL.
+ :param secure: generate HTTPS URLs if the use_https config variable
+ is set, in order to prevent warnings about insecure objects
+ from happening in some browsers (this is used for, e.g.,
+ branding).
+ :param include_token: create a time-limited token and include it in
+ the URL to authorise access to restricted files.
"""
def open(timeout=LIBRARIAN_SERVER_DEFAULT_TIMEOUT):
=== modified file 'lib/canonical/launchpad/zcml/librarian.zcml'
--- lib/canonical/launchpad/zcml/librarian.zcml 2010-12-20 17:42:47 +0000
+++ lib/canonical/launchpad/zcml/librarian.zcml 2011-03-28 05:33:32 +0000
@@ -52,17 +52,6 @@
class="canonical.launchpad.browser.LibraryFileAliasMD5View"
/>
- <!-- StreamOrRedirectLibraryFileAliasView -->
- <class class="canonical.launchpad.browser.librarian.StreamOrRedirectLibraryFileAliasView">
- <allow attributes="__call__" />
- <allow interface="zope.publisher.interfaces.browser.IBrowserPublisher" />
- </class>
-
- <class class="canonical.launchpad.browser.librarian.SafeStreamOrRedirectLibraryFileAliasView">
- <allow attributes="__call__" />
- <allow interface="zope.publisher.interfaces.browser.IBrowserPublisher" />
- </class>
-
<adapter
factory="canonical.launchpad.database.librarian.LibraryFileAliasWithParent"
provides="canonical.launchpad.interfaces.librarian.ILibraryFileAliasWithParent"
=== modified file 'lib/lp/bugs/browser/bugattachment.py'
--- lib/lp/bugs/browser/bugattachment.py 2011-02-02 15:43:31 +0000
+++ lib/lp/bugs/browser/bugattachment.py 2011-03-28 05:33:32 +0000
@@ -22,7 +22,6 @@
from canonical.launchpad.browser.librarian import (
FileNavigationMixin,
ProxiedLibraryFileAlias,
- SafeStreamOrRedirectLibraryFileAliasView,
)
from canonical.launchpad.interfaces.librarian import (
ILibraryFileAliasWithParent,
@@ -237,5 +236,3 @@
"""Traversal to +files/${filename}."""
usedfor = IBugAttachment
-
- view_class = SafeStreamOrRedirectLibraryFileAliasView
=== modified file 'lib/lp/bugs/browser/tests/test_bugattachment_file_access.py'
--- lib/lp/bugs/browser/tests/test_bugattachment_file_access.py 2011-03-23 16:28:51 +0000
+++ lib/lp/bugs/browser/tests/test_bugattachment_file_access.py 2011-03-28 05:33:32 +0000
@@ -19,8 +19,7 @@
from zope.security.management import endInteraction
from canonical.launchpad.browser.librarian import (
- SafeStreamOrRedirectLibraryFileAliasView,
- StreamOrRedirectLibraryFileAliasView,
+ LibraryFileAliasView,
)
from canonical.launchpad.interfaces.librarian import (
ILibraryFileAliasWithParent,
@@ -35,8 +34,6 @@
)
from lazr.restfulclient.errors import NotFound as RestfulNotFound
from lp.bugs.browser.bugattachment import BugAttachmentFileNavigation
-import lp.services.features
-from lp.services.features.flags import NullFeatureController
from lp.testing import (
launchpadlib_for,
login_person,
@@ -61,13 +58,13 @@
def test_traversal_to_lfa_of_bug_attachment(self):
# Traversing to the URL provided by a ProxiedLibraryFileAlias of a
- # bug attachament returns a StreamOrRedirectLibraryFileAliasView.
+ # bug attachament returns a RedirectionView.
request = LaunchpadTestRequest()
request.setTraversalStack(['foo.txt'])
navigation = BugAttachmentFileNavigation(
self.bugattachment, request)
view = navigation.publishTraverse(request, '+files')
- self.assertIsInstance(view, StreamOrRedirectLibraryFileAliasView)
+ self.assertIsInstance(view, RedirectionView)
def test_traversal_to_lfa_of_bug_attachment_wrong_filename(self):
# If the filename provided in the URL does not match the
@@ -85,10 +82,7 @@
navigation = BugAttachmentFileNavigation(
self.bugattachment, request)
view = navigation.publishTraverse(request, '+files')
- next_view, traversal_path = view.browserDefault(request)
- self.assertIsInstance(next_view, RedirectionView)
- mo = re.match(
- '^http://.*/\d+/foo.txt$', next_view.target)
+ mo = re.match('^http://.*/\d+/foo.txt$', view.target)
self.assertIsNot(None, mo)
def test_access_to_restricted_file(self):
@@ -106,16 +100,9 @@
request.setTraversalStack(['foo.txt'])
navigation = BugAttachmentFileNavigation(self.bugattachment, request)
view = navigation.publishTraverse(request, '+files')
- # XXX Ensure the feature will be off - everything is off with
- # NullFeatureController. bug=631884
- lp.services.features.install_feature_controller(
- NullFeatureController())
- self.addCleanup(lp.services.features.install_feature_controller, None)
- next_view, traversal_path = view.browserDefault(request)
- self.assertEqual(view, next_view)
- file_ = next_view()
- file_.seek(0)
- self.assertEqual('file content', file_.read())
+ mo = re.match(
+ '^https://.*.restricted.*/\d+/foo.txt\?token=.*$', view.target)
+ self.assertIsNot(None, mo)
def test_access_to_restricted_file_unauthorized(self):
# If a user cannot access the bug attachment itself, he can neither
@@ -135,33 +122,6 @@
self.assertRaises(
Unauthorized, navigation.publishTraverse, request, '+files')
- def test_content_disposition_of_restricted_file(self):
- # The content of restricted Librarian files for bug attachments
- # is served by instances of SafeStreamOrRedirectLibraryFileAliasView
- # which set the content disposition header of the HTTP response for
- # to "attachment".
- lfa_with_parent = getMultiAdapter(
- (self.bugattachment.libraryfile, self.bugattachment),
- ILibraryFileAliasWithParent)
- lfa_with_parent.restricted = True
- self.bug.setPrivate(True, self.bug_owner)
- transaction.commit()
- request = LaunchpadTestRequest()
- request.setTraversalStack(['foo.txt'])
- navigation = BugAttachmentFileNavigation(self.bugattachment, request)
- view = navigation.publishTraverse(request, '+files')
- # XXX Ensure the feature will be off - everything is off with
- # NullFeatureController. bug=631884
- lp.services.features.install_feature_controller(
- NullFeatureController())
- self.addCleanup(lp.services.features.install_feature_controller, None)
- next_view, traversal_path = view.browserDefault(request)
- self.assertIsInstance(
- next_view, SafeStreamOrRedirectLibraryFileAliasView)
- next_view()
- self.assertEqual(
- 'attachment', request.response.getHeader('Content-Disposition'))
-
class TestWebserviceAccessToBugAttachmentFiles(TestCaseWithFactory):
"""Tests access to bug attachments via the webservice."""
=== modified file 'lib/lp/soyuz/stories/ppa/xx-ppa-files.txt'
--- lib/lp/soyuz/stories/ppa/xx-ppa-files.txt 2011-01-17 21:51:09 +0000
+++ lib/lp/soyuz/stories/ppa/xx-ppa-files.txt 2011-03-28 05:33:32 +0000
@@ -248,83 +248,14 @@
The 'No Privileges' user, the PPA owner, can download the DSC file.
- >>> no_priv_browser.open(file_lp_url)
-
-And its informations match the library file.
-
- >>> print file_content
- I do not care about sources.
- >>> print file_size
- 28
- >>> print file_mimetype
- application/text
-
- >>> print no_priv_browser.contents
- I do not care about sources.
- >>> print no_priv_browser.headers['content-length']
- 28
- >>> print no_priv_browser.headers['content-type']
- application/text
-
-Gzipped package diffs ('.diff.gz') and buildlogs ('.txt.gz') are
-treated differently from the other files. Since we want their contents
-to be rendered inline by the browsers we override their 'content-encoding'
-and 'content-type' to 'gzip' and 'text/plain', respectively.
-
- >>> login('foo.bar@xxxxxxxxxxxxx')
-
- >>> buildlog_content = buildlog.read()
- >>> buildlog_size = str(buildlog.content.filesize)
- >>> buildlog_lp_url = str(
- ... 'http://launchpad.dev/~no-priv/+archive/p3a/+buildjob/%d/+files/%s' %
- ... (build.id, buildlog.filename))
-
- >>> diff_content = package_diff.diff_content.read()
- >>> diff_size = str(package_diff.diff_content.content.filesize)
- >>> diff_lp_url = str(
- ... 'http://launchpad.dev/~no-priv/+archive/p3a/+files/%s' %
- ... package_diff.diff_content.filename)
-
- >>> logout()
-
-Restricted buildlogs are served with the pristine content and size,
-but 'content-type' and 'content-encoding' are modified.
-
- >>> no_priv_browser.open(buildlog_lp_url)
-
- >>> print buildlog_content
- bogus buildlog
- >>> print buildlog_size
- 14
-
- >>> print no_priv_browser.contents
- bogus buildlog
- >>> print no_priv_browser.headers['content-length']
- 14
-
- >>> print no_priv_browser.headers['content-type']
- text/plain
- >>> print no_priv_browser.headers['content-encoding']
- gzip
-
-The same happens for package diffs.
-
- >>> no_priv_browser.open(diff_lp_url)
-
- >>> print diff_content
- bogus diff
- >>> print diff_size
- 10
-
- >>> print no_priv_browser.contents
- bogus diff
- >>> print no_priv_browser.headers['content-length']
- 10
-
- >>> print no_priv_browser.headers['content-type']
- text/plain
- >>> print no_priv_browser.headers['content-encoding']
- gzip
+ >>> print http(r"""
+ ... GET %s HTTP/1.1
+ ... Authorization: Basic no-priv@xxxxxxxxxxxxx:test
+ ... """ % (file_lp_url.replace('http://launchpad.dev', '')))
+ HTTP/1.1 303 See Other
+ ...
+ Location: https://...restricted.../test-pkg_1.0.dsc?token=...
+ ...
If the associated PPA and the `LibraryFileAlias` are public, the +files/
proxy redirects to the public http url. We'll copy the test sources and
@@ -382,18 +313,15 @@
The same redirection happens for +archive/+buildjob/blah urls:
- >>> buildlog_lp_url_without_ppa_name = buildlog_lp_url.replace(
- ... '/p3a', '')
- >>> print buildlog_lp_url_without_ppa_name
- http://.../~no-priv/+archive/+buildjob/31/+files/...
-
+ >>> buildlog_lp_url_without_ppa_name = (
+ ... 'http://launchpad.dev/~no-priv/+archive/+buildjob/1/+files/foo')
>>> print http(r"""
... GET %s HTTP/1.1
... """ % buildlog_lp_url_without_ppa_name.replace(
... 'http://launchpad.dev', ''))
HTTP/1.1 301 Moved Permanently
...
- Location: http://.../~no-priv/+archive/ppa/+buildjob/31/+files/...
+ Location: http://.../~no-priv/+archive/ppa/+buildjob/1/+files/...
...
