Merge lp:~gary/launchpad/revert12041 into lp:launchpad

[Gary Poster]

[r=flacoste][ui=none][no-qa] revert r12041, which is qa-bad and blocking deployment.

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.

[Francis J. Lacoste (flacoste)]

Looks sane

review: Approve

=== 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)