launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02172
[Merge] lp:~gary/launchpad/revert12041 into lp:launchpad
Gary Poster has proposed merging lp:~gary/launchpad/revert12041 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
This reverts r12041, which is qa-bad and blocking deployment per the deployable revision report (https://devpad.canonical.com/~lpqateam/qa_reports/deployment-stable.html). No-one from the bugs team is around to confirm that what I am doing is correct, but it looks right to me from what I know of the situation.
Practically, this branch is merely the result of ``bzr merge -r12041..12040``. I did nothing else other than look at the output.
--
https://code.launchpad.net/~gary/launchpad/revert12041/+merge/43811
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~gary/launchpad/revert12041 into lp:launchpad.
=== modified file 'lib/canonical/launchpad/browser/librarian.py'
--- lib/canonical/launchpad/browser/librarian.py 2010-12-07 11:00:09 +0000
+++ lib/canonical/launchpad/browser/librarian.py 2010-12-15 19:14:18 +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,17 +257,18 @@
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:
- - the webapp has to be contacted to get access to a file (required for
- restricted files).
+ 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)
- 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 known.
+ is know.
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-11-18 18:42:05 +0000
+++ lib/canonical/launchpad/rest/bytestorage.py 2010-12-15 19:14:18 +0000
@@ -6,6 +6,7 @@
__metaclass__ = type
__all__ = [
'LibraryBackedByteStorage',
+ 'RestrictedLibraryBackedByteStorage',
]
@@ -15,6 +16,7 @@
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
@@ -73,3 +75,29 @@
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-18 18:42:05 +0000
+++ lib/canonical/launchpad/zcml/webservice.zcml 2010-12-15 19:14:18 +0000
@@ -43,6 +43,19 @@
<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-18 18:42:05 +0000
+++ lib/lp/bugs/interfaces/bugattachment.py 2010-12-15 19:14:18 +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 Title
+from lp.services.fields import RestrictedBytes, Title
class BugAttachmentType(DBEnumeratedType):
@@ -115,7 +115,7 @@
libraryfile = Bytes(title=_("The attachment content."),
required=True)
data = exported(
- Bytes(title=_("The attachment content."),
+ RestrictedBytes(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-12-07 11:00:09 +0000
+++ lib/lp/bugs/stories/webservice/xx-bug.txt 2010-12-15 19:14:18 +0000
@@ -1360,6 +1360,46 @@
---
+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
------------------
@@ -2093,7 +2133,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: 23
+ total_size: 26
...
>>> bug_nine_activity = webservice.get(
=== modified file 'lib/lp/services/fields/__init__.py'
--- lib/lp/services/fields/__init__.py 2010-12-07 11:00:09 +0000
+++ lib/lp/services/fields/__init__.py 2010-12-15 19:14:18 +0000
@@ -22,6 +22,7 @@
'ILocationField',
'INoneableTextLine',
'IPasswordField',
+ 'IRestrictedBytes',
'IStrippedTextLine',
'ISummary',
'ITag',
@@ -45,6 +46,7 @@
'ProductBugTracker',
'ProductNameField',
'PublicPersonChoice',
+ 'RestrictedBytes',
'SearchTag',
'StrippedTextLine',
'Summary',
@@ -227,6 +229,10 @@
"""
+class IRestrictedBytes(IBytes):
+ """A marker interface used for restricted LibraryFileAlias fields."""
+
+
class StrippedTextLine(TextLine):
implements(IStrippedTextLine)
@@ -836,3 +842,8 @@
else:
# The vocabulary prevents the revealing of private team names.
raise PrivateTeamNotAllowed(value)
+
+
+class RestrictedBytes(Bytes):
+ """A field for restricted LibraryFileAlias records."""
+ implements(IRestrictedBytes)
