launchpad-reviewers team mailing list archive

[Colin Watson <mp+434170@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:private-team-many-archive-subscriptions into launchpad:master.

Commit message:
Fix private team visibility queries for some users

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/434170

`PublicOrPrivateTeamsExistence.checkAuthenticated` was slow for users with archive subscriptions via many different teams, because it performed security checks on all those teams when computing the set of archive IDs to which the user has access via subscriptions in order to see whether any of them match one of the target team's private PPAs.  We can safely skip those security checks, because we know that the user can view all their own subscriptions.

Fixes https://oops.canonical.com/?oopsid=OOPS-7b1698553c348c7b467e93b8cc067f03.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:private-team-many-archive-subscriptions into launchpad:master.

diff --git a/lib/lp/registry/security.py b/lib/lp/registry/security.py
index 14866a0..0487001 100644
--- a/lib/lp/registry/security.py
+++ b/lib/lp/registry/security.py
@@ -10,6 +10,7 @@ __all__ = [
 
 from storm.expr import Select, Union
 from zope.component import getUtility
+from zope.security.proxy import removeSecurityProxy
 
 from lp.app.security import (
     AnonymousAuthorization,
@@ -678,9 +679,10 @@ class PublicOrPrivateTeamsExistence(AuthorizationBase):
             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
+            # team's private PPA.  We can safely skip security checks here: the
+            # user can view all their own subscriptions.
+            subscriptions = removeSecurityProxy(
+                getUtility(IArchiveSubscriberSet).getBySubscriber(user.person)
             )
             subscriber_archive_ids = {sub.archive_id for sub in subscriptions}
             team_ppa_ids = {ppa.id for ppa in self.obj.ppas if ppa.private}
diff --git a/lib/lp/registry/tests/test_security.py b/lib/lp/registry/tests/test_security.py
index ed8d447..f31563c 100644
--- a/lib/lp/registry/tests/test_security.py
+++ b/lib/lp/registry/tests/test_security.py
@@ -125,7 +125,9 @@ class TestPublicOrPrivateTeamsExistence(TestCaseWithFactory):
                 archive = self.factory.makeArchive(
                     owner=private_team, private=True
                 )
-                archive.newSubscription(person, team_owner)
+                archive.newSubscription(
+                    self.factory.makeTeam(owner=person), team_owner
+                )
 
         def check_team_limited_view():
             person.clearInTeamCache()