launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02118
[Merge] lp:~adeuring/launchpad/librarian-filealiases into lp:launchpad
Abel Deuring has proposed merging lp:~adeuring/launchpad/librarian-filealiases into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
This branch removes a temporary workaround which we needed to give certain webservice clients access to restricted Librarian files.
The core of the workaround was to point webservice clients to an HTTP (not HHTPS) URL having a non-public hostname. This was necessary because we wanted to avoid serving the data via app servers, as done in loder variants of ProxiedLibraryFileAlias. The proper fix, access to restricted files via time-limited tokens, was not available at that time, but the Apport retracers (which are located in our DC and which can resolve the non-public hostname) needed access to the restricted files.
The workaround was implemented by the class RestrictedLibraryBackedByteStorage, together with a marker interface IRestriedBytes. These classes are now gone, together with some ZCML stuff. I also deleted a test for accessing restricted files. This test lived in longer page test file; removing it (the test, not the file...) changed the number of activities of test bug #1, which in turn required a minor change of at the end of the file.
And there is a bit of lint cleanup.
There are not special tests for this branch; the full test suite passed an EC2 run.
--
https://code.launchpad.net/~adeuring/launchpad/librarian-filealiases/+merge/42942
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~adeuring/launchpad/librarian-filealiases into lp:launchpad.
=== modified file 'lib/canonical/launchpad/browser/librarian.py'
--- lib/canonical/launchpad/browser/librarian.py 2010-11-08 12:52:43 +0000
+++ lib/canonical/launchpad/browser/librarian.py 2010-12-07 13:50:27 +0000
@@ -85,11 +85,11 @@
If the file is public, it will redirect to the files http url.
- Otherwise if the feature flag publicrestrictedlibrarian is set to 'on' this
- will allocate a token and redirect to the aliases private url.
+ Otherwise if the feature flag publicrestrictedlibrarian is set to 'on'
+ 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.
@@ -98,7 +98,7 @@
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
+ 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
@@ -161,7 +161,7 @@
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
@@ -182,7 +182,7 @@
# Public file, just point the client at the right place.
return RedirectionView(self.context.http_url, self.request), ()
if getFeatureFlag(u'publicrestrictedlibrarian') != 'on':
- # Restricted file and we have not enabled the public
+ # Restricted file and we have not enabled the public
# restricted librarian yet :- deliver inline.
self._when_streaming()
return self, ()
@@ -203,12 +203,12 @@
"""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()
+ super(
+ SafeStreamOrRedirectLibraryFileAliasView, self)._when_streaming()
self.request.response.setHeader(
'Content-Disposition', 'attachment')
@@ -257,18 +257,17 @@
class ProxiedLibraryFileAlias:
"""A `LibraryFileAlias` decorator for use in URL generation.
- The URL's output by this decorator will always point at the webapp. This is
- useful when:
- - we are proxying files via the webapp (as we do at the moment)
- - when the webapp has to be contacted to get access to a file (the case
- for restricted files in the future)
+ The URL's output by this decorator will always point at the webapp. This
+ is useful when:
+ - the webapp has to be contacted to get access to a file (required for
+ restricted files).
- files might change from public to private and thus not work even if the
user has access to the once its private, unless they go via the webapp.
This should be used anywhere we are outputting URL's to LibraryFileAliases
other than directly in rendered pages. For rendered pages, using a
LibraryFileAlias directly is OK as at that point the status of the file
- is know.
+ is known.
Overrides `ILibraryFileAlias.http_url` to always point to the webapp URL,
even when called from the webservice domain.
=== modified file 'lib/canonical/launchpad/rest/bytestorage.py'
--- lib/canonical/launchpad/rest/bytestorage.py 2010-09-03 20:28:36 +0000
+++ lib/canonical/launchpad/rest/bytestorage.py 2010-12-07 13:50:27 +0000
@@ -6,7 +6,6 @@
__metaclass__ = type
__all__ = [
'LibraryBackedByteStorage',
- 'RestrictedLibraryBackedByteStorage',
]
@@ -16,7 +15,6 @@
from zope.component import getUtility
from zope.interface import implements
-from canonical.launchpad.browser.librarian import ProxiedLibraryFileAlias
from canonical.launchpad.interfaces.librarian import ILibraryFileAliasSet
from canonical.launchpad.webapp.interfaces import ICanonicalUrlData
@@ -75,29 +73,3 @@
def deleteStored(self):
"""See `IByteStorage`."""
setattr(self.entry, self.field.__name__, None)
-
-
-class RestrictedLibraryBackedByteStorage(LibraryBackedByteStorage):
- """See `IByteStorage`.
-
- This variant of LibraryBackedByteStorage provides an alias_url
- which points to a StreamOrRedirectLibraryFileAliasView for
- restricted Librarian files.
- """
-
- @property
- def alias_url(self):
- """See `IByteStorage`."""
- if self.file_alias.restricted:
- # XXX Abel Deuring 2010-09-03, bug=629804
- # This is a bad hack: We can't give ordinary users API access to
- # restricted files at present. But we can allow access to
- # some machines in the data center (and should do that).
- # We need here an HTTP URL, not an HTTPS URL, so we don't
- # use getUrl(). Since http_url uses not an official domain
- # name, all we do is expose an unaccessible HTTP URL also
- # users outside of the data center, instead of the HTTPS URL
- # returned by getURL().
- return self.file_alias.http_url
- else:
- return self.file_alias.getURL()
=== modified file 'lib/canonical/launchpad/zcml/webservice.zcml'
--- lib/canonical/launchpad/zcml/webservice.zcml 2010-11-09 14:35:44 +0000
+++ lib/canonical/launchpad/zcml/webservice.zcml 2010-12-07 13:50:27 +0000
@@ -43,19 +43,6 @@
<allow interface='lazr.restful.interfaces.IByteStorage' />
</class>
- <!-- Registration for the class that manages an entry's RestrictedBytes
- fields. -->
- <adapter
- for="lazr.restful.interfaces.IEntry
- lp.services.fields.IRestrictedBytes"
- provides="lazr.restful.interfaces.IByteStorage"
- factory="canonical.launchpad.rest.bytestorage.RestrictedLibraryBackedByteStorage"
- />
-
- <class class="canonical.launchpad.rest.bytestorage.RestrictedLibraryBackedByteStorage">
- <allow interface='lazr.restful.interfaces.IByteStorage' />
- </class>
-
<!-- WebService uses the default LaunchpadRootNavigation -->
<view
for="canonical.launchpad.interfaces.launchpad.IWebServiceApplication"
=== modified file 'lib/lp/bugs/interfaces/bugattachment.py'
--- lib/lp/bugs/interfaces/bugattachment.py 2010-11-02 19:54:59 +0000
+++ lib/lp/bugs/interfaces/bugattachment.py 2010-12-07 13:50:27 +0000
@@ -39,7 +39,7 @@
from canonical.launchpad import _
from canonical.launchpad.interfaces.launchpad import IHasBug
from canonical.launchpad.interfaces.message import IMessage
-from lp.services.fields import RestrictedBytes, Title
+from lp.services.fields import Title
class BugAttachmentType(DBEnumeratedType):
@@ -115,7 +115,7 @@
libraryfile = Bytes(title=_("The attachment content."),
required=True)
data = exported(
- RestrictedBytes(title=_("The attachment content."),
+ Bytes(title=_("The attachment content."),
required=True,
readonly=True))
message = exported(
=== modified file 'lib/lp/bugs/stories/webservice/xx-bug.txt'
--- lib/lp/bugs/stories/webservice/xx-bug.txt 2010-10-22 13:58:41 +0000
+++ lib/lp/bugs/stories/webservice/xx-bug.txt 2010-12-07 13:50:27 +0000
@@ -1360,46 +1360,6 @@
---
-Attachments of private bugs
-~~~~~~~~~~~~~~~~~~~~~~~~~~~
-
-Attachments of private bugs are served by the app server itself.
-
- >>> print webservice.patch(
- ... bug_one['self_link'], 'application/json', dumps(dict(private=True)))
- HTTP/1.1 209 Content Returned...
-
- >>> response = webservice.named_post(
- ... bug_one['self_link'], 'addAttachment',
- ... data="67890", filename="more-numbers.txt", content_type='foo/bar',
- ... comment="The numbers you asked for.")
- >>> attachments = webservice.get(
- ... bug_one['attachments_collection_link']).jsonBody()
- >>> attachment = attachments['entries'][0]
- >>> pprint_entry(attachment)
- bug_link:...
- data_link: u'http://api.launchpad.dev/beta/bugs/1/+attachment/.../data'
- ...
-
-XXX Abel Deuring 2010-09-03, bug=629804: Please note that the http URLs in this
-example are part of a temporary hack. Please see the comment in
-canonical.launchpad.rest.bytestorage.RestrictedLibraryBackedByteStorage.
-alias_url for details.
-
- >>> response = webservice.get(attachment['data_link'])
- >>> print response
- HTTP/1.1 303 See Other
- Status: 303 See Other
- ...
- Location: http://localhost:58005/94/more-numbers.txt
- ...
-
- >>> # Let's make the bug public again for later tests.
- >>> print webservice.patch(
- ... bug_one['self_link'], 'application/json', dumps(dict(private=False)))
- HTTP/1.1 209 Content Returned...
-
-
Searching for bugs
------------------
@@ -2133,7 +2093,7 @@
next_collection_link: u'http://.../bugs/1/activity?ws.start=5&ws.size=5'
resource_type_link: u'http://.../#bug_activity-page-resource'
start: 0
- total_size: 26
+ total_size: 23
...
>>> bug_nine_activity = webservice.get(
=== modified file 'lib/lp/services/fields/__init__.py'
--- lib/lp/services/fields/__init__.py 2010-10-23 16:45:43 +0000
+++ lib/lp/services/fields/__init__.py 2010-12-07 13:50:27 +0000
@@ -22,7 +22,6 @@
'ILocationField',
'INoneableTextLine',
'IPasswordField',
- 'IRestrictedBytes',
'IStrippedTextLine',
'ISummary',
'ITag',
@@ -46,7 +45,6 @@
'ProductBugTracker',
'ProductNameField',
'PublicPersonChoice',
- 'RestrictedBytes',
'SearchTag',
'StrippedTextLine',
'Summary',
@@ -229,10 +227,6 @@
"""
-class IRestrictedBytes(IBytes):
- """A marker interface used for restricted LibraryFileAlias fields."""
-
-
class StrippedTextLine(TextLine):
implements(IStrippedTextLine)
@@ -842,8 +836,3 @@
else:
# The vocabulary prevents the revealing of private team names.
raise PrivateTeamNotAllowed(value)
-
-
-class RestrictedBytes(Bytes):
- """A field for restricted LibraryFileAlias records."""
- implements(IRestrictedBytes)
