Feedback from validate_lola_access() review PR

docs/lola-authentication.md

@@ -412,19 +412,23 @@ The binding is persisted inside the same `transaction.atomic()` block DOT
 
 ### Enforcement at Request Time
 
-**Location**: `testbed/core/views/decorators.py` - `validate_lola_access`
+**Location**: `testbed/core/views/decorators.py` — the `@lola_scope_required` /
+`@lola_scope_optional` gate decorators (stacked under `@actor_required`), backed
+by `lola_access_error(request, required_scope, url_pk)`.
 
-Every LOLA actor-scoped endpoint calls `validate_lola_access(request, ...)`,
-which runs two layers governed by *different* conditions:
+Every LOLA actor-scoped view is decorated with `@actor_required` above a gate
+decorator. The gate runs `lola_access_error(...)` and short-circuits with the
+error `Response` it returns (or `None` to let the request through). It runs two
+layers governed by *different* conditions:
 
-- **Layer 1 - Scope presence (controlled by `required_scope`).** Strict
-  endpoints (`required_scope=True`) reject requests without
+- **Layer 1 - Scope presence (the choice of gate decorator).** Strict endpoints
+  (`@lola_scope_required`) reject requests without
   `activitypub_account_portability` scope with 403 `insufficient_scope`.
-  Dual-mode endpoints (`required_scope=False`) skip this layer so public
+  Dual-mode endpoints (`@lola_scope_optional`) skip this layer so public
   traffic passes through to their public response.
 - **Layer 2 - Actor binding check (runs whenever a portability token is
-  present, regardless of `required_scope`).** Reads the requested actor pk from
-  `request.resolver_match.kwargs["pk"]` and compares it to
+  present, regardless of the decorator).** Compares the requested actor pk (the
+  view's URL `<pk>`, passed into the gate) to
   `request.auth.actor_binding.actor_id`. Mismatches, missing binding rows,
   a missing URL pk, and a claimed scope with no token object all return 403
   `actor_mismatch` (fail closed).
@@ -439,7 +443,7 @@ Token-to-actor binding is enforced on **every** actor-scoped LOLA endpoint that
 can expose scope-gated data — both the strict collections and the dual-mode
 endpoints.
 
-**Strict endpoints** — `validate_lola_access(request, required_scope=True)`.
+**Strict endpoints** — `@lola_scope_required`.
 No portability scope ⇒ 403 `insufficient_scope`; wrong actor ⇒ 403
 `actor_mismatch`:
 
@@ -448,7 +452,7 @@ No portability scope ⇒ 403 `insufficient_scope`; wrong actor ⇒ 403
 - `GET /api/actors/<pk>/liked/`
 - `GET /api/actors/<pk>/blocked/` (and `…/migration/blocked/`)
 
-**Dual-mode endpoints** — `validate_lola_access(request, required_scope=False)`.
+**Dual-mode endpoints** — `@lola_scope_optional`.
 No token ⇒ public response (200); a portability token bound to a *different*
 actor ⇒ 403 `actor_mismatch`:
 
@@ -506,7 +510,7 @@ Two core ActivityPub endpoints have been enhanced with LOLA authentication suppo
 | ✅ Portability token bound to **this** actor | Enhanced Actor with LOLA discovery fields |
 | ⛔ Portability token bound to a **different** actor | `403 actor_mismatch` (dual-mode binding) |
 
-This is a dual-mode endpoint: it calls `validate_lola_access(request, required_scope=False)`, so anonymous access stays a normal public response, but a portability token must be bound to `<pk>` before the LOLA fields are exposed (see [Endpoints Enforcing Binding](#endpoints-enforcing-binding)).
+This is a dual-mode endpoint (`@lola_scope_optional`), so anonymous access stays a normal public response, but a portability token must be bound to `<pk>` before the LOLA fields are exposed (see [Endpoints Enforcing Binding](#endpoints-enforcing-binding)).
 
 #### LOLA Fields Added (when authenticated with portability scope)
 
@@ -598,7 +602,7 @@ This is a dual-mode endpoint: it calls `validate_lola_access(request, required_s
 | ✅ Portability token bound to **this** actor | ALL activities (public + private) |
 | ⛔ Portability token bound to a **different** actor | `403 actor_mismatch` (dual-mode binding) |
 
-Like actor-detail, this is a dual-mode endpoint (`validate_lola_access(request, required_scope=False)`): a mis-bound portability token is rejected rather than served another actor's private activities. The same applies to the advertised `…/migration/outbox/` route (same view).
+Like actor-detail, this is a dual-mode endpoint (`@lola_scope_optional`): a mis-bound portability token is rejected rather than served another actor's private activities. The same applies to the advertised `…/migration/outbox/` route (same view).
 
 #### Response Structure
 

testbed/core/oauth/authentication.py

@@ -47,8 +47,8 @@ def authenticate(self, request):
 
         Both enabled paths produce the same (user, token) shape and set
         request.auth to the AccessToken instance. The actor binding check in
-        validate_lola_access() operates on request.auth and therefore covers
-        every path uniformly.
+        lola_access_error() (behind the @lola_scope_* gate decorators) operates on
+        request.auth and therefore covers every path uniformly.
 
         If authentication succeeds, checks if the token has the portability scope.
         If authentication fails, allows the request to continue as unauthenticated.

testbed/core/tests/conftest.py

@@ -31,7 +31,8 @@ def create_isolated_actor(username_prefix, role=None):
 
 When `user` is given, the token is issued for that user so token.user matches actor.user;
 otherwise the factory creates a fresh token user. Only the token<->actor binding is what
-validate_lola_access checks, so both shapes satisfy the gate.
+lola_access_error() (behind the @lola_scope_* gate decorators) checks, so both shapes
+satisfy the gate.
 """
 def bind_portability_token(actor, user=None):
     if user is not None:

testbed/core/tests/test_token_actor_binding.py

@@ -16,7 +16,7 @@
 )
 from testbed.core.models import Actor, TokenActorBinding
 from testbed.core.oauth.validators import ActivityPubOAuth2Validator
-from testbed.core.views.decorators import validate_lola_access
+from testbed.core.views.decorators import lola_access_error
 
 
 # Model
@@ -86,26 +86,26 @@ def test_validator_skips_binding_for_non_portability_token():
 
 # Decorator
 
-def _make_lola_request(actor, token):
-    """Build a fake request with portability scope targeting the given actor pk."""
-    rf = RequestFactory()
-    request = rf.get(f"/api/actors/{actor.pk}/followers/")
+def _make_lola_request(token):
+    """
+    Build a fake request that claims portability scope and carries `token` as request.auth.
+    The actor pk the token must be bound to is passed to lola_access_error directly as url_pk,
+    so no request.resolver_match is needed. Pass token=None to model the "scope claimed but no token object" state.
+    """
+    request = RequestFactory().get("/api/actors/1/followers/")
     request.is_oauth_authenticated = True
     request.has_portability_scope = True
     request.auth = token
-    request.resolver_match = MagicMock()
-    request.resolver_match.kwargs = {"pk": actor.pk}
     return request
 
 
 @pytest.mark.django_db
 def test_same_actor_access_succeeds():
     """Token bound to actor A may access actor A."""
     binding = TokenActorBindingFactory()
-    request = _make_lola_request(binding.actor, binding.token)
+    request = _make_lola_request(binding.token)
 
-    result = validate_lola_access(request)
-    assert result["valid"] is True
+    assert lola_access_error(request, required_scope=True, url_pk=binding.actor.pk) is None
 
 
 @pytest.mark.django_db
@@ -118,18 +118,13 @@ def test_cross_actor_access_denied():
         user=user_b, username=f"{user_b.username}_src", role=Actor.ROLE_SOURCE
     )
 
-    rf = RequestFactory()
-    request = rf.get(f"/api/actors/{actor_b.pk}/followers/")
-    request.is_oauth_authenticated = True
-    request.has_portability_scope = True
-    request.auth = binding.token
-    request.resolver_match = MagicMock()
-    request.resolver_match.kwargs = {"pk": actor_b.pk}
+    # Token bound to actor A, but the URL pk is actor B.
+    request = _make_lola_request(binding.token)
 
-    result = validate_lola_access(request)
-    assert result["valid"] is False
-    assert result["error_response"].status_code == 403
-    assert result["error_response"].data["error_code"] == "actor_mismatch"
+    error = lola_access_error(request, required_scope=True, url_pk=actor_b.pk)
+    assert error is not None
+    assert error.status_code == 403
+    assert error.data["error_code"] == "actor_mismatch"
 
 
 @pytest.mark.django_db
@@ -142,31 +137,26 @@ def test_unbound_token_denied():
     token = AccessTokenFactory(user=user, lola_scope=True)
     # Intentionally: no TokenActorBinding created.
 
-    request = _make_lola_request(actor, token)
+    request = _make_lola_request(token)
 
-    result = validate_lola_access(request)
-    assert result["valid"] is False
-    assert result["error_response"].status_code == 403
-    assert result["error_response"].data["error_code"] == "actor_mismatch"
+    error = lola_access_error(request, required_scope=True, url_pk=actor.pk)
+    assert error is not None
+    assert error.status_code == 403
+    assert error.data["error_code"] == "actor_mismatch"
 
 
 @pytest.mark.django_db
 def test_missing_url_pk_fails_closed():
     """A LOLA request without a URL pk is rejected (fail closed, no silent skip)."""
     binding = TokenActorBindingFactory()
 
-    rf = RequestFactory()
-    request = rf.get("/api/unexpected/")
-    request.is_oauth_authenticated = True
-    request.has_portability_scope = True
-    request.auth = binding.token
-    request.resolver_match = MagicMock()
-    request.resolver_match.kwargs = {}  # no "pk" key
+    # url_pk=None models an actor-scoped gate invoked without a pk in the URL
+    request = _make_lola_request(binding.token)
 
-    result = validate_lola_access(request)
-    assert result["valid"] is False
-    assert result["error_response"].status_code == 403
-    assert result["error_response"].data["error_code"] == "actor_mismatch"
+    error = lola_access_error(request, required_scope=True, url_pk=None)
+    assert error is not None
+    assert error.status_code == 403
+    assert error.data["error_code"] == "actor_mismatch"
 
 
 # Integration
@@ -245,12 +235,11 @@ def test_dual_mode_cross_actor_denied(route_name):
 @pytest.mark.django_db
 def test_scope_claimed_without_token_fails_closed():
     binding = TokenActorBindingFactory()
-    request = _make_lola_request(binding.actor, binding.token)
     # Anomalous state: the scope flag is set but no token object is present, so
     # no binding can be verified. The gate must deny, not grant.
-    request.auth = None
+    request = _make_lola_request(token=None)
 
-    result = validate_lola_access(request)
-    assert result["valid"] is False
-    assert result["error_response"].status_code == 403
-    assert result["error_response"].data["error_code"] == "actor_mismatch"
+    error = lola_access_error(request, required_scope=True, url_pk=binding.actor.pk)
+    assert error is not None
+    assert error.status_code == 403
+    assert error.data["error_code"] == "actor_mismatch"

testbed/core/views/__init__.py

@@ -1,4 +1,10 @@
-from .decorators import activitypub_content, build_auth_context, validate_lola_access
+from .decorators import (
+    actor_required,
+    activitypub_content,
+    build_auth_context,
+    lola_scope_optional,
+    lola_scope_required,
+)
 
 from .api import (
     actor_detail,
@@ -21,7 +27,9 @@
 )
 
 __all__ = [
-    "validate_lola_access",
+    "actor_required",
+    "lola_scope_required",
+    "lola_scope_optional",
     "build_auth_context",
     "activitypub_content",
     "actor_detail",