improvement: simplified the 'act' helper logic

Author: nandan-bhat

src/auth0_api_python/act.py

@@ -2,13 +2,12 @@
 Helpers for working with the `act` claim on verified access token claims.
 """
 
-from __future__ import annotations
-
 from collections.abc import Mapping
 from typing import Any
 
 from .errors import VerifyAccessTokenError
-from .types import ActClaim
+
+INVALID_ACT_CLAIM_MESSAGE = "Invalid act claim"
 
 
 def get_current_actor(claims: Mapping[str, Any]) -> str | None:
@@ -18,12 +17,21 @@ def get_current_actor(claims: Mapping[str, Any]) -> str | None:
     Only the outermost `act.sub` should be used for authorization decisions.
     Nested `act` values represent prior actors and are informational.
     """
+    if not isinstance(claims, Mapping):
+        raise VerifyAccessTokenError(INVALID_ACT_CLAIM_MESSAGE)
 
-    act_claim = _get_validated_act_claim(claims)
+    act_claim = claims.get("act")
     if act_claim is None:
         return None
 
-    return act_claim["sub"]
+    if not isinstance(act_claim, Mapping):
+        raise VerifyAccessTokenError(INVALID_ACT_CLAIM_MESSAGE)
+
+    sub = act_claim.get("sub")
+    if not isinstance(sub, str) or not sub.strip():
+        raise VerifyAccessTokenError(INVALID_ACT_CLAIM_MESSAGE)
+
+    return sub
 
 
 def get_delegation_chain(claims: Mapping[str, Any]) -> list[str]:
@@ -34,54 +42,23 @@ def get_delegation_chain(claims: Mapping[str, Any]) -> list[str]:
     prior actors from nested `act` values and are typically most useful for audit
     and attribution.
     """
+    if not isinstance(claims, Mapping):
+        raise VerifyAccessTokenError(INVALID_ACT_CLAIM_MESSAGE)
 
-    act_claim = _get_validated_act_claim(claims)
-    if act_claim is None:
+    current = claims.get("act")
+    if current is None:
         return []
 
     chain: list[str] = []
-    current: ActClaim | None = act_claim
     while current is not None:
-        chain.append(current["sub"])
-        current = current.get("act")
-
-    return chain
-
+        if not isinstance(current, Mapping):
+            raise VerifyAccessTokenError(INVALID_ACT_CLAIM_MESSAGE)
 
-def _get_validated_act_claim(claims: Mapping[str, Any]) -> ActClaim | None:
-    if not isinstance(claims, Mapping):
-        raise VerifyAccessTokenError("Verified access token claims must be an object")
-
-    act_claim = claims.get("act")
-    if act_claim is None:
-        return None
-
-    return _parse_act_claim(act_claim, path="act", seen=set())
-
-
-def _parse_act_claim(value: Any, *, path: str, seen: set[int]) -> ActClaim:
-    if not isinstance(value, Mapping):
-        raise VerifyAccessTokenError(f"{path} must be an object")
+        sub = current.get("sub")
+        if not isinstance(sub, str) or not sub.strip():
+            raise VerifyAccessTokenError(INVALID_ACT_CLAIM_MESSAGE)
 
-    object_id = id(value)
-    if object_id in seen:
-        raise VerifyAccessTokenError(f"{path} contains a circular reference")
+        chain.append(sub)
+        current = current.get("act")
 
-    seen.add(object_id)
-    try:
-        sub = value.get("sub")
-        if not isinstance(sub, str) or not sub.strip():
-            raise VerifyAccessTokenError(f"{path}.sub must be a non-empty string")
-
-        parsed: ActClaim = {"sub": sub}
-        nested_act = value.get("act")
-        if nested_act is not None:
-            parsed["act"] = _parse_act_claim(
-                nested_act,
-                path=f"{path}.act",
-                seen=seen,
-            )
-
-        return parsed
-    finally:
-        seen.remove(object_id)
+    return chain

tests/test_act.py

@@ -3,6 +3,8 @@
 from auth0_api_python import get_current_actor, get_delegation_chain
 from auth0_api_python.errors import VerifyAccessTokenError
 
+INVALID_ACT_CLAIM_MESSAGE = "Invalid act claim"
+
 
 def test_get_current_actor_returns_none_when_act_is_missing():
     claims = {"sub": "auth0|user123"}
@@ -36,15 +38,15 @@ def test_get_current_actor_and_delegation_chain_from_nested_act():
 def test_get_current_actor_rejects_blank_actor_subject():
     with pytest.raises(
         VerifyAccessTokenError,
-        match=r"act\.sub must be a non-empty string",
+        match=INVALID_ACT_CLAIM_MESSAGE,
     ):
         get_current_actor({"act": {"sub": "   "}})
 
 
 def test_get_delegation_chain_rejects_invalid_nested_act():
     with pytest.raises(
         VerifyAccessTokenError,
-        match=r"act\.act must be an object",
+        match=INVALID_ACT_CLAIM_MESSAGE,
     ):
         get_delegation_chain(
             {
@@ -54,12 +56,3 @@ def test_get_delegation_chain_rejects_invalid_nested_act():
                 },
             }
         )
-
-
-def test_get_delegation_chain_rejects_circular_act():
-    act = {"sub": "mcp_server_client_id"}
-    act["act"] = act
-
-    with pytest.raises(VerifyAccessTokenError, match="circular reference"):
-        get_delegation_chain({"act": act})
-