Bugzilla webhook authorization

docs/hackbot/triggers.md

@@ -137,10 +137,14 @@ Guards, each closing a specific failure mode:
 - **Latest flag wins** — BMO orders flags by id, so the last matching one is the newly
   requested one.
 
-Authorization is Bugzilla's own: anyone who can set a needinfo on the bot can ask it for
-something. There is no separate group check like the Phabricator trigger's
-`bmo-editbugs-team`, because a private bug is already excluded and the flag itself is the
-request.
+Only requesters in Bugzilla's `editbugs` group are authorized (all Mozilla Corporation
+members belong to this group) — see
+[bugzilla_authorization.py](../../services/hackbot-api/app/bugzilla_authorization.py).
+Membership is checked per login with BMO's server-side `group_ids` filter on `/rest/user`.
+
+- An unauthorized request is ignored without
+  consuming the dedupe key, leaving the same flag eligible for a later delivery after the
+  requester becomes authorized.
 
 The receiver passes the requester's login and the change timestamp to the agent as context
 for locating the accompanying comment — a needinfo may be filed without one, in which case
@@ -154,6 +158,6 @@ existing one. The needinfo flag is cleared automatically as a recorded
 the reply comment into a single Bugzilla transaction (see [actions.md](actions.md)). A run
 that records nothing leaves the flag standing.
 
-Configuration is three env vars — `BUGZILLA_WEBHOOK_SECRET` (required, no default),
-`BUGZILLA_WEBHOOK_BOT_LOGIN` and `BUGZILLA_WEBHOOK_DEDUPE_TTL_SECONDS`; see
-[deployment.md](deployment.md).
+Configuration is four env vars — `BUGZILLA_WEBHOOK_SECRET` (required, no default),
+`BUGZILLA_WEBHOOK_BOT_LOGIN`, `BUGZILLA_WEBHOOK_URL` and
+`BUGZILLA_WEBHOOK_DEDUPE_TTL_SECONDS`; see [deployment.md](deployment.md).

services/hackbot-api/app/bugzilla_authorization.py

@@ -0,0 +1,52 @@
+"""Authorization checks for Bugzilla webhook actors."""
+
+from __future__ import annotations
+
+import asyncio
+from typing import TYPE_CHECKING
+
+from cachetools import TTLCache
+
+if TYPE_CHECKING:
+    from app.bugzilla_client import BugzillaUserClient
+
+AUTHORIZED_GROUP_ID = 9  # bmo-editbugs-team
+
+
+class BugzillaAuthorizer:
+    """Cache-backed per-user authorization checks against a Bugzilla group."""
+
+    def __init__(
+        self,
+        client: BugzillaUserClient,
+        authorized_group_id: int,
+        *,
+        cache_ttl_seconds: int = 300,
+        cache_maxsize: int = 4096,
+    ) -> None:
+        self._client = client
+        self._authorized_group_id = authorized_group_id
+        self._cache: TTLCache[str, bool] = TTLCache(
+            maxsize=cache_maxsize,
+            ttl=cache_ttl_seconds,
+        )
+        self._lock = asyncio.Lock()
+
+    async def is_authorized(self, login: str) -> bool:
+        """Return whether a Bugzilla login belongs to the authorized group."""
+        login = login.lower()
+
+        cached = self._cache.get(login)
+        if cached is not None:
+            return cached
+
+        async with self._lock:
+            cached = self._cache.get(login)
+            if cached is not None:
+                return cached
+
+            authorized = await self._client.is_user_in_group(
+                login, self._authorized_group_id
+            )
+            self._cache[login] = authorized
+            return authorized

services/hackbot-api/app/bugzilla_client.py

@@ -0,0 +1,43 @@
+"""Minimal async client for Bugzilla's REST API.
+
+TODO: Replace with a shared ``bugzilla-client`` workspace lib (the
+``phabricator-client`` treatment) once one exists.
+"""
+
+from __future__ import annotations
+
+import httpx
+
+_REQUEST_TIMEOUT_SECONDS = 30
+
+
+class BugzillaUserClient:
+    """Minimal async client for Bugzilla's user lookup endpoint."""
+
+    def __init__(self, url: str) -> None:
+        self._rest_url = url.rstrip("/") + "/rest"
+
+    async def is_user_in_group(self, login: str, group_id: int) -> bool:
+        """Return whether a Bugzilla account exists and belongs to a group.
+
+        ``group_ids`` filters server-side: the account appears in ``users``
+        only when it exists and is a member. The filter needs no API key,
+        which keeps this internet-facing service free of privileged Bugzilla
+        credentials (reading another account's ``groups`` directly would
+        require one). BMO rejects an unknown ``group_ids`` value outright, so
+        the filter cannot be silently ignored.
+        """
+        async with httpx.AsyncClient(timeout=_REQUEST_TIMEOUT_SECONDS) as client:
+            response = await client.get(
+                f"{self._rest_url}/user",
+                params={
+                    "names": login,
+                    "group_ids": str(group_id),
+                    "include_fields": "name",
+                    # Report an unknown login in ``faults`` instead of failing
+                    # the request, so it maps to "not authorized", not a 500.
+                    "permissive": "1",
+                },
+            )
+        response.raise_for_status()
+        return bool(response.json().get("users"))

services/hackbot-api/app/bugzilla_webhook.py

@@ -12,6 +12,7 @@ class BugzillaNeedinfoEvent:
     bug_id: int
     flag_id: int
     comment: str
+    actor_login: str
 
 
 def detect_needinfo_request(
@@ -71,4 +72,6 @@ def detect_needinfo_request(
         "A needinfo may be requested without a comment, so use the surrounding "
         "bug context if none exists."
     )
-    return BugzillaNeedinfoEvent(bug_id=bug_id, flag_id=flag_id, comment=comment)
+    return BugzillaNeedinfoEvent(
+        bug_id=bug_id, flag_id=flag_id, comment=comment, actor_login=actor_login
+    )

services/hackbot-api/app/config.py

@@ -38,6 +38,8 @@ class BugzillaWebhookSettings(BaseModel):
     bot_login: str = "hackbot@mozilla.tld"
     # Best-effort in-memory dedupe of retried bug-modification deliveries.
     dedupe_ttl_seconds: int = 6 * 60 * 60
+    # Bugzilla instance queried to authorize the requesting user.
+    url: str = "https://bugzilla.mozilla.org"
 
 
 class SlackSettings(BaseModel):
@@ -80,8 +82,8 @@ class Settings(BaseSettings):
     webhook: WebhookSettings
 
     # Bugzilla uses a separate shared-secret header and bot identity. These map
-    # from BUGZILLA_WEBHOOK_SECRET, BUGZILLA_WEBHOOK_BOT_LOGIN, and
-    # BUGZILLA_WEBHOOK_DEDUPE_TTL_SECONDS.
+    # from BUGZILLA_WEBHOOK_SECRET, BUGZILLA_WEBHOOK_BOT_LOGIN,
+    # BUGZILLA_WEBHOOK_DEDUPE_TTL_SECONDS, and BUGZILLA_WEBHOOK_URL.
     bugzilla_webhook: BugzillaWebhookSettings
 
     slack: SlackSettings

services/hackbot-api/app/routers/webhooks.py

@@ -11,6 +11,8 @@
     require_bugzilla_webhook_secret,
     require_phabricator_signature,
 )
+from app.bugzilla_authorization import AUTHORIZED_GROUP_ID, BugzillaAuthorizer
+from app.bugzilla_client import BugzillaUserClient
 from app.bugzilla_webhook import detect_needinfo_request
 from app.config import settings
 from app.phabricator_authorization import (
@@ -52,6 +54,16 @@ def get_phabricator_authorizer(
     return authorizer
 
 
+def get_bugzilla_authorizer(request: Request) -> BugzillaAuthorizer:
+    """Dependency: lazily create the app-scoped authorizer and its user cache."""
+    authorizer = getattr(request.app.state, "bugzilla_authorizer", None)
+    if authorizer is None:
+        client = BugzillaUserClient(settings.bugzilla_webhook.url)
+        authorizer = BugzillaAuthorizer(client, AUTHORIZED_GROUP_ID)
+        request.app.state.bugzilla_authorizer = authorizer
+    return authorizer
+
+
 # Best-effort dedupe of retried deliveries, keyed by triggering transaction PHID.
 # Per-instance and reset on restart; a durable dedupe (using the DB) can replace
 # this if needed. Sized well above the number of mentions expected in a window.
@@ -149,6 +161,7 @@ async def phabricator_webhook(
 async def bugzilla_webhook(
     request: Request,
     api_client: HackbotClient = Depends(get_hackbot_client),
+    authorizer: BugzillaAuthorizer = Depends(get_bugzilla_authorizer),
 ) -> dict:
     """Trigger a bug-fix follow-up for a bot-directed ``needinfo?`` change."""
     payload = await request.json()
@@ -170,6 +183,15 @@ async def bugzilla_webhook(
         )
         return {"status": "ignored", "reason": "duplicate delivery"}
 
+    # Ignore requests from users without editbugs
+    if not await authorizer.is_authorized(detected.actor_login):
+        log.info(
+            "Ignored Bugzilla needinfo webhook for bug %s: %s is not authorized",
+            detected.bug_id,
+            detected.actor_login,
+        )
+        return {"status": "ignored", "reason": "unauthorized user"}
+
     run = await api_client.trigger_run(
         "bug-fix",
         {

services/hackbot-api/pyproject.toml

@@ -18,6 +18,7 @@ dependencies = [
     "google-auth>=2.29.0",
     "sentry-sdk>=2.51.0",
     "cachetools>=5.3.0",
+    "httpx>=0.26.0",
     "slack-sdk>=3.27.0",
     "python-multipart>=0.0.9",
     "hackbot-client",

services/hackbot-api/tests/test_bugzilla_authorization.py

@@ -0,0 +1,45 @@
+"""Tests for Bugzilla webhook actor authorization."""
+
+from unittest.mock import AsyncMock
+
+from app.bugzilla_authorization import AUTHORIZED_GROUP_ID, BugzillaAuthorizer
+
+
+class _FakeClient:
+    def __init__(self, member: bool) -> None:
+        self.is_user_in_group = AsyncMock(return_value=member)
+
+
+def _authorizer(member: bool) -> tuple[BugzillaAuthorizer, _FakeClient]:
+    client = _FakeClient(member)
+    return BugzillaAuthorizer(client, AUTHORIZED_GROUP_ID), client
+
+
+async def test_is_authorized_caches_positive_lookup():
+    authorizer, client = _authorizer(member=True)
+
+    assert await authorizer.is_authorized("dev@mozilla.com") is True
+    assert await authorizer.is_authorized("dev@mozilla.com") is True
+    client.is_user_in_group.assert_awaited_once_with(
+        "dev@mozilla.com", AUTHORIZED_GROUP_ID
+    )
+
+
+async def test_is_authorized_caches_negative_lookup():
+    authorizer, client = _authorizer(member=False)
+
+    assert await authorizer.is_authorized("someone@example.com") is False
+    assert await authorizer.is_authorized("someone@example.com") is False
+    client.is_user_in_group.assert_awaited_once_with(
+        "someone@example.com", AUTHORIZED_GROUP_ID
+    )
+
+
+async def test_is_authorized_normalizes_login_case():
+    authorizer, client = _authorizer(member=True)
+
+    assert await authorizer.is_authorized("Dev@Mozilla.com") is True
+    assert await authorizer.is_authorized("dev@mozilla.com") is True
+    client.is_user_in_group.assert_awaited_once_with(
+        "dev@mozilla.com", AUTHORIZED_GROUP_ID
+    )

services/hackbot-api/tests/test_bugzilla_client.py

@@ -0,0 +1,67 @@
+"""Tests for the Bugzilla user client, on BMO's captured payload shapes."""
+
+import httpx
+from app.bugzilla_client import BugzillaUserClient
+
+
+def _user_client(monkeypatch, json_body: dict) -> tuple[BugzillaUserClient, list]:
+    """A client whose HTTP layer replays ``json_body``, capturing requests."""
+    requests: list[httpx.Request] = []
+
+    def handler(request: httpx.Request) -> httpx.Response:
+        requests.append(request)
+        return httpx.Response(200, json=json_body)
+
+    real_async_client = httpx.AsyncClient
+    monkeypatch.setattr(
+        httpx,
+        "AsyncClient",
+        lambda **kwargs: real_async_client(
+            transport=httpx.MockTransport(handler), **kwargs
+        ),
+    )
+    client = BugzillaUserClient("https://bugzilla.example.com")
+    return client, requests
+
+
+async def test_is_user_in_group_true_for_member(monkeypatch):
+    client, requests = _user_client(
+        monkeypatch, {"users": [{"name": "dev@mozilla.com"}], "faults": []}
+    )
+
+    assert await client.is_user_in_group("dev@mozilla.com", 9) is True
+
+    request = requests[0]
+    assert request.url.host == "bugzilla.example.com"
+    assert request.url.path == "/rest/user"
+    assert request.url.params["names"] == "dev@mozilla.com"
+    assert request.url.params["group_ids"] == "9"
+    assert request.url.params["permissive"] == "1"
+    # The membership filter is anonymous: no credential must ever be sent.
+    assert "X-Bugzilla-API-Key" not in request.headers
+
+
+async def test_is_user_in_group_false_for_non_member(monkeypatch):
+    # An existing account outside the group is filtered out server-side
+    # (live BMO shape: empty ``users``, empty ``faults``).
+    client, _ = _user_client(monkeypatch, {"users": [], "faults": []})
+    assert await client.is_user_in_group("outsider@example.com", 9) is False
+
+
+async def test_is_user_in_group_false_for_unknown_user(monkeypatch):
+    # With permissive=1, BMO reports an unknown login as a 200 with the error
+    # in ``faults`` and an empty ``users`` list (live BMO shape).
+    client, _ = _user_client(
+        monkeypatch,
+        {
+            "users": [],
+            "faults": [
+                {
+                    "error": True,
+                    "name": "ghost@example.com",
+                    "message": "There is no user named 'ghost@example.com'.",
+                }
+            ],
+        },
+    )
+    assert await client.is_user_in_group("ghost@example.com", 9) is False