launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:fix-webhook-visibility-check into launchpad:master.

Commit message:
Handle DelegatedAuthorization in webhook visibility check

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

WebhookSet._checkVisibility needs to do a visibility check for the webhook owner, and it previously did this by looking up the relevant security adapter and calling checkAuthentication on it directly.  However, this doesn't work for DelegatedAuthorization adapters, since they return an iterator rather than a boolean.  Call iter_authorization instead.

This doesn't make any difference right now, but it will matter once there are webhooks for objects that have DelegatedAuthorization-based security adapters.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:fix-webhook-visibility-check into launchpad:master.

diff --git a/lib/lp/services/webhooks/model.py b/lib/lp/services/webhooks/model.py
index 5aa1723..0c38e8e 100644
--- a/lib/lp/services/webhooks/model.py
+++ b/lib/lp/services/webhooks/model.py
@@ -34,10 +34,7 @@ from storm.properties import (
 from storm.references import Reference
 from storm.store import Store
 import transaction
-from zope.component import (
-    getAdapter,
-    getUtility,
-    )
+from zope.component import getUtility
 from zope.interface import (
     implementer,
     provider,
@@ -45,8 +42,6 @@ from zope.interface import (
 from zope.security.proxy import removeSecurityProxy
 
 from lp.app import versioninfo
-from lp.app.interfaces.security import IAuthorization
-from lp.registry.interfaces.role import IPersonRoles
 from lp.registry.model.person import Person
 from lp.services.config import config
 from lp.services.database.bulk import load_related
@@ -65,6 +60,8 @@ from lp.services.job.model.job import (
     )
 from lp.services.job.runner import BaseRunnableJob
 from lp.services.scripts import log
+from lp.services.webapp.authorization import iter_authorization
+from lp.services.webapp.interfaces import IPlacelessAuthUtility
 from lp.services.webhooks.interfaces import (
     IWebhook,
     IWebhookClient,
@@ -234,10 +231,10 @@ class WebhookSet:
         :return: True if the context is visible to the webhook owner,
             otherwise False.
         """
-        roles = IPersonRoles(user)
-        authz = getAdapter(
-            removeSecurityProxy(context), IAuthorization, "launchpad.View")
-        return authz.checkAuthenticated(roles)
+        authutil = getUtility(IPlacelessAuthUtility)
+        return all(iter_authorization(
+            removeSecurityProxy(context), "launchpad.View",
+            authutil.getPrincipal(user.accountID), {}))
 
     def trigger(self, target, event_type, payload, context=None):
         if context is None: