Merge ~cjwatson/launchpad:fix-webhook-visibility-check into launchpad:master

[Colin Watson]

Handle DelegatedAuthorization in webhook visibility check

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.

66452d1... by Colin Watson on 2020-01-22

Grant request-daily-builds SELECT on account

The new approach in WebhookSet._checkVisibility requires that anything
that might dispatch webhooks should have access to the account table in
order to construct a principal. This is fairly harmless, so grant it to
request-daily-builds.

Reported by Ioana Lasc.

[William Grant (wgrant)]

review: Approve (code)

diff --git a/database/schema/security.cfg b/database/schema/security.cfg
index 06da0d4..b5d6071 100644
--- a/database/schema/security.cfg
+++ b/database/schema/security.cfg
@@ -801,6 +801,7 @@ type=user
 
 [request-daily-builds]
 groups=script
+public.account                                  = SELECT
 public.archive                                  = SELECT
 public.archivepermission                        = SELECT
 public.branch                                   = SELECT
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: