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()
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
