Merge lp:~wallyworld/launchpad/no-private-bug-rss into lp:launchpad

[Ian Booth]

[r=allenap][bug=302097] Stop exposing rss feeds for private bugs or branches.

This branch stops exposing rss feeds for private bugs or branches.

== Implementation ==

Add a class method feed_allowed() to FeedLinkBase, default to returning True. However, this is overridden in BranchFeedLink and BugFeedLink to return true only if the branch/bug is not private.

TODO: when a user uses the inline ajax widget to change the privacy status, the rss feed status is not toggled. This will be done in a subsequent branch.

== Tests ==

Extends the xx-links.txt doctest to check that private bugs and branches pages do not render the rss link element.

== Lint ==

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/canonical/launchpad/browser/feeds.py
  lib/canonical/launchpad/pagetests/feeds/xx-links.txt

./lib/canonical/launchpad/pagetests/feeds/xx-links.txt
       1: narrative uses a moin header.
       9: narrative uses a moin header.
      33: narrative uses a moin header.
      58: narrative uses a moin header.
      88: narrative uses a moin header.
     122: narrative uses a moin header.
     149: want exceeds 78 characters.
     152: want exceeds 78 characters.
     155: want exceeds 78 characters.
     158: want exceeds 78 characters.
     160: narrative uses a moin header.
     208: narrative uses a moin header.
     235: narrative uses a moin header.
     248: narrative uses a moin header.
     261: narrative uses a moin header.
     270: want exceeds 78 characters.
     274: narrative uses a moin header.
     290: narrative uses a moin header.
     306: narrative uses a moin header.
     322: narrative uses a moin header.
     332: want exceeds 78 characters.
     333: want exceeds 78 characters.

[Gavin Panella (allenap)]

review: Approve

=== modified file 'lib/canonical/launchpad/browser/feeds.py'
--- lib/canonical/launchpad/browser/feeds.py	2010-11-08 12:52:43 +0000
+++ lib/canonical/launchpad/browser/feeds.py	2011-05-20 08:58:39 +0000
@@ -177,6 +177,14 @@
             "Context %r does not provide interface %r"
             % (context, self.usedfor))
 
+    @classmethod
+    def feed_allowed(cls, context):
+        """Return True if a feed is allowed for the given context.
+
+        Subclasses should override this method as necessary.
+        """
+        return True
+
 
 class BugFeedLink(FeedLinkBase):
     usedfor = IBugTask
@@ -190,6 +198,12 @@
         return urlappend(self.rooturl,
                          'bugs/' + str(self.context.bug.id) + '/bug.atom')
 
+    @classmethod
+    def feed_allowed(cls, context):
+        """See `FeedLinkBase`"""
+        # No feeds for private bugs.
+        return not context.bug.private
+
 
 class BugTargetLatestBugsFeedLink(FeedLinkBase):
     usedfor = IHasBugs
@@ -222,6 +236,7 @@
             return urlappend(canonical_url(self.context, rootsite='feeds'),
                              'announcements.atom')
 
+
 class RootAnnouncementsFeedLink(AnnouncementsFeedLink):
     usedfor = ILaunchpadRoot
 
@@ -296,11 +311,18 @@
     @property
     def title(self):
         return 'Latest Revisions for Branch %s' % self.context.displayname
+
     @property
     def href(self):
         return urlappend(canonical_url(self.context, rootsite="feeds"),
                          'branch.atom')
 
+    @classmethod
+    def feed_allowed(cls, context):
+        """See `FeedLinkBase`"""
+        # No feeds for private branches.
+        return not context.private
+
 
 class PersonRevisionsFeedLink(FeedLinkBase):
     """Feed links for revisions created by a person."""
@@ -346,4 +368,5 @@
     def feed_links(self):
         return [feed_type(self.context)
                 for feed_type in self.feed_types
-                if feed_type.usedfor.providedBy(self.context)]
+                if feed_type.usedfor.providedBy(self.context) and
+                feed_type.feed_allowed(self.context)]

=== modified file 'lib/canonical/launchpad/pagetests/feeds/xx-links.txt'
--- lib/canonical/launchpad/pagetests/feeds/xx-links.txt	2011-05-05 05:46:41 +0000
+++ lib/canonical/launchpad/pagetests/feeds/xx-links.txt	2011-05-20 08:58:39 +0000
@@ -42,6 +42,37 @@
         href="http://feeds.launchpad.dev/bugs/1/bug.atom"
         title="Bug 1 Feed" />]
 
+But if the bug is private, there should be no link.
+
+    >>> from canonical.launchpad.testing.pages import setupBrowserForUser
+    >>> from zope.component import getUtility
+    >>> from lp.registry.interfaces.person import IPersonSet
+    >>> login(ANONYMOUS)
+    >>> user = getUtility(IPersonSet).getByEmail('[e-mail]')
+    >>> logout()
+    >>> auth_browser = setupBrowserForUser(user, 'daf')
+    >>> auth_browser.open('http://bugs.launchpad.dev/jokosher/+bug/14')
+    >>> soup = BeautifulSoup(auth_browser.contents)
+    >>> soup.head.findAll('link', type='application/atom+xml')
+    []
+
+But if they somehow manage to hack the url or use inline ajax editing of the
+bug status and attempt to subscribe, they are redirected to the bug page:
+
+    >>> def print_notifications(browser):
+    ...     tags = find_tags_by_class(
+    ...         browser.contents, 'error message')
+    ...     if len(tags) == 0:
+    ...         print 'No messages.'
+    ...     else:
+    ...         for tag in tags:
+    ...             print extract_text(tag)
+
+    >>> auth_browser.open('http://feeds.launchpad.dev/bugs/14/bug.atom')
+    >>> print auth_browser.url
+    http://bugs.launchpad.dev/jokosher/+bug/14
+    >>> print_notifications(auth_browser)
+    Feeds do not serve private bugs.
 
 == Latest Bugs and Branches for a Person ==
 
@@ -319,3 +350,24 @@
     [<link rel="alternate" type="application/atom+xml"
         href="http://feeds.launchpad.dev/~mark/firefox/release--0.9.1/branch.atom"
 	title="Latest Revisions for Branch lp://dev/~mark/firefox/release--0.9.1" />]
+
+But if the branch is private, there should be no link.
+
+    >>> login(ANONYMOUS)
+    >>> user = getUtility(IPersonSet).getByEmail('[e-mail]')
+    >>> logout()
+    >>> auth_browser = setupBrowserForUser(user)
+    >>> auth_browser.open(
+    ... 'https://code.launchpad.dev/~name12/landscape/feature-x')
+    >>> soup = BeautifulSoup(auth_browser.contents)
+    >>> soup.head.findAll('link', type='application/atom+xml')
+    []
+
+But if they somehow manage to hack the url, they are redirected to a page with
+an error notification:
+
+    >>> browser.open('http://feeds.launchpad.dev/~name12/landscape/feature-x/branch.atom')
+    >>> print browser.url
+    http://code.launchpad.dev/
+    >>> print_notifications(browser)
+    Feeds do not serve private branches.

=== modified file 'lib/lp/bugs/feed/bug.py'
--- lib/lp/bugs/feed/bug.py	2010-08-20 20:31:18 +0000
+++ lib/lp/bugs/feed/bug.py	2011-05-20 08:58:39 +0000
@@ -14,13 +14,14 @@
 
 from z3c.ptcompat import ViewPageTemplateFile
 from zope.component import getUtility
-from zope.security.interfaces import Unauthorized
 
 from canonical.config import config
 from canonical.launchpad.webapp import (
     canonical_url,
     urlparse,
     )
+from canonical.launchpad.webapp.authorization import check_permission
+from canonical.launchpad.webapp.interfaces import ILaunchpadRoot
 from canonical.launchpad.webapp.publisher import LaunchpadView
 from canonical.lazr.feed import (
     FeedBase,
@@ -141,8 +142,8 @@
     def getBugsFromBugTasks(self, tasks):
         """Given a list of BugTasks return the list of associated bugs.
 
-        Since a Bug can have multiple BugTasks, we only select bugs that have not
-        yet been seen.
+        Since a Bug can have multiple BugTasks, we only select bugs that have
+        not yet been seen.
         """
         bug_ids = []
         for task in tasks:
@@ -171,7 +172,15 @@
         # For a `BugFeed` we must ensure that the bug is not private.
         super(BugFeed, self).initialize()
         if self.context.private:
-            raise Unauthorized("Feeds do not serve private bugs")
+            self.request.response.addErrorNotification(
+                "Feeds do not serve private bugs.")
+            if check_permission("launchpad.View", self.context):
+                self.request.response.redirect(canonical_url(self.context))
+            else:
+                # Bug cannot be seen so redirect to the bugs index page.
+                root = getUtility(ILaunchpadRoot)
+                self.request.response.redirect(
+                    canonical_url(root, rootsite='bugs'))
 
     @property
     def title(self):

=== modified file 'lib/lp/code/feed/branch.py'
--- lib/lp/code/feed/branch.py	2011-03-03 01:13:47 +0000
+++ lib/lp/code/feed/branch.py	2011-05-20 08:58:39 +0000
@@ -30,6 +30,7 @@
     LaunchpadView,
     urlparse,
     )
+from canonical.launchpad.webapp.interfaces import ILaunchpadRoot
 from canonical.lazr.feed import (
     FeedBase,
     FeedEntry,
@@ -74,6 +75,7 @@
         super(BranchFeedContentView, self).__init__(context, request)
         self.feed = feed
         self.template_ = template
+
     def render(self):
         """Render the view."""
         return ViewPageTemplateFile(self.template_)(self)
@@ -375,7 +377,7 @@
 
     def __init__(self, person, rootsite):
 
-        no_email =  person.name_without_email
+        no_email = person.name_without_email
         if no_email:
             self.name = no_email
         else:
@@ -400,8 +402,22 @@
         """See `IFeed`."""
         # For a `BranchFeed` we must ensure that the branch is not private.
         super(BranchFeed, self).initialize()
-        if self.context.private:
-            raise Unauthorized("Feeds do not serve private branches")
+        try:
+            feed_allowed = not self.context.private
+            if not feed_allowed:
+                # We are logged in and can see the branch so redirect to the
+                # branch index page.
+                redirect_url = canonical_url(self.context)
+        except Unauthorized:
+            # Branch cannot be seen so redirect to the code index page.
+            feed_allowed = False
+            root = getUtility(ILaunchpadRoot)
+            redirect_url = canonical_url(root, rootsite='code')
+
+        if not feed_allowed:
+            self.request.response.addErrorNotification(
+                "Feeds do not serve private branches.")
+            self.request.response.redirect(redirect_url)
 
     @property
     def title(self):