Merge lp:~julian-edwards/launchpad/p3a-sub-private-teams-bug-597783 into lp:launchpad

[Julian Edwards]

= Summary =
Subscribers of P3As owned by private teams should be able to see the team

== Proposed fix ==
https://bugs.edge.launchpad.net/soyuz/+bug/597783

Currently a subscriber to a private PPA owned by a private team will get a 403
when they visit their +archivesubscriptions page because the security adapter
still prevents access from seeing that team, but which is required to build a
URL to the repo location in the subscription token.

This branch changes the security adapter to take the subscriptions a user
might have, and allows launchpad.View to the team if it has a PPA and the user
has a subscription.

== Pre-implementation notes ==
Talked to Curtis, we both agreed there's not much else we can do here.

== Implementation details ==
As above.

== Tests ==
bin/test -cvv test_archive_subscriptions

== Demo and Q/A ==
QA Plan:

 * Create a private team
 * Give the team a private PPA
 * Confirm that Joe User can't see the team, but can see his
+archivesubscriptions
 * Subscribe Joe User to the PPA
 * Confirm that Joe User can see the subscription on his +archivesubscriptions
page

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/lp/soyuz/tests/test_archive_subscriptions.py
  lib/canonical/launchpad/security.py

[Julian Edwards (julian-edwards)]

/me grumbles at the stupid person picker

Maris, that's an r-c stamp I need from you please, you already gave me one on Friday for this but I'd like it if you can make it official.

Cheers.

[Curtis Hovey (sinzui)]

Hi Julian. Thanks for working on this. I have one trivial remark: There should be two blank lines before `def test_suite():`. I think we should consider this for an RC.

review: Approve (code)

[Māris Fogels (mars)]

Looks good to me, release-critical=mars

review: Approve

[Henning Eggers (henninge)]

Thanks for the fix and the improvement. Now just remove the unused import for IArchiveSet. ;)

review: Approve (code)

=== modified file 'lib/canonical/launchpad/security.py'
--- lib/canonical/launchpad/security.py	2010-06-24 18:52:56 +0000
+++ lib/canonical/launchpad/security.py	2010-06-28 14:56:38 +0000
@@ -14,7 +14,6 @@
 from canonical.launchpad.interfaces.account import IAccount
 from canonical.launchpad.interfaces.emailaddress import IEmailAddress
 from lp.registry.interfaces.announcement import IAnnouncement
-from lp.soyuz.interfaces.archive import IArchive
 from lp.soyuz.interfaces.archivepermission import (
     IArchivePermissionSet)
 from lp.soyuz.interfaces.archiveauthtoken import IArchiveAuthToken
@@ -735,6 +734,19 @@
             if (invitee.is_team and
                 invitee in user.person.getAdministratedTeams()):
                 return True
+
+        if (self.obj.is_team
+            and self.obj.visibility == PersonVisibility.PRIVATE):
+            # Grant visibility to people with subscriptions on a private
+            # team's private PPA.
+            subscriptions = getUtility(
+                IArchiveSubscriberSet).getBySubscriber(user.person)
+            subscriber_archive_ids = set(
+                sub.archive.id for sub in subscriptions)
+            team_ppa_ids = set(
+                ppa.id for ppa in self.obj.ppas if ppa.private)
+            if len(subscriber_archive_ids.intersection(team_ppa_ids)) > 0:
+                return True
         return False
 
 
 
=== added file 'lib/lp/soyuz/tests/test_archive_subscriptions.py'
--- lib/lp/soyuz/tests/test_archive_subscriptions.py	1970-01-01 00:00:00 +0000
+++ lib/lp/soyuz/tests/test_archive_subscriptions.py	2010-06-28 14:56:38 +0000
@@ -0,0 +1,57 @@
+# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test Archive features."""
+
+import unittest
+
+from zope.security.interfaces import Unauthorized
+
+from canonical.testing import DatabaseFunctionalLayer
+
+from lp.registry.interfaces.person import PersonVisibility
+from lp.testing import login_person, TestCaseWithFactory
+
+
+class TestArchiveSubscriptions(TestCaseWithFactory):
+    """Edge-case tests for private PPA subscribers.
+
+    See also lib/lp/soyuz/stories/ppa/xx-private-ppa-subscription-stories.txt
+    """
+
+    layer = DatabaseFunctionalLayer
+
+    def setUp(self):
+        """Create a test archive."""
+        super(TestArchiveSubscriptions, self).setUp()
+        self.private_team = self.factory.makeTeam(
+            visibility=PersonVisibility.PRIVATE, name="subscribertest")
+        login_person(self.private_team.teamowner)
+        self.archive = self.factory.makeArchive(
+            private=True, owner=self.private_team)
+        self.subscriber = self.factory.makePerson()
+
+    def test_subscriber_can_access_private_team_ppa(self):
+        # As per bug 597783, we need to make sure a subscriber can see
+        # a private team's PPA after they have been given a subscription.
+        # This is essentially allowing access for the subscriber to see
+        # the private team.
+
+        def get_name():
+            return self.archive.owner.name
+
+        # Before a subscription, accessing the team name will raise.
+        login_person(self.subscriber)
+        self.assertRaises(Unauthorized, get_name)
+
+        login_person(self.private_team.teamowner)
+        self.archive.newSubscription(
+            self.subscriber, registrant=self.archive.owner)
+
+        # When a subscription exists, it's fine.
+        login_person(self.subscriber)
+        self.assertEqual(self.archive.owner.name, "subscribertest")
+
+
+def test_suite():
+    return unittest.TestLoader().loadTestsFromName(__name__)