Skip to content

Commit 03fe605

Browse files
committed
feat(fdc): Add constructor validation and unit tests for Impersonation
Added an explicit __init__ constructor to Impersonation to validate parameter configurations at object instantiation time. Enforced choosing either unauthenticated=True or auth_claims, with support for both auth_claims (snake_case) and authClaims (camelCase). Updated class docstring to recommend factory methods. Also added unit test suite TestImpersonation in tests/test_data_connect.py.
1 parent 1a1d12e commit 03fe605

2 files changed

Lines changed: 105 additions & 4 deletions

File tree

firebase_admin/dataconnect.py

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,39 @@ def __post_init__(self):
106106

107107

108108
class Impersonation(dict):
109-
"""Represents impersonation configuration for DataConnect requests."""
109+
"""Represents impersonation configuration for DataConnect requests.
110+
111+
It is recommended to construct instances using the static factory methods
112+
:meth:`unauthenticated` or :meth:`authenticated`.
113+
"""
114+
115+
def __init__(
116+
self,
117+
*,
118+
unauthenticated: Optional[bool] = None,
119+
auth_claims: Optional[Dict[str, Any]] = None,
120+
authClaims: Optional[Dict[str, Any]] = None
121+
) -> None:
122+
if auth_claims is not None and authClaims is not None:
123+
raise ValueError("Cannot specify both 'auth_claims' and 'authClaims'.")
124+
125+
claims = auth_claims if auth_claims is not None else authClaims
126+
127+
if unauthenticated is None and claims is None:
128+
raise ValueError(
129+
"Impersonation requires either 'unauthenticated=True' or 'auth_claims'."
130+
)
131+
if unauthenticated is not None and claims is not None:
132+
raise ValueError("Cannot specify both 'unauthenticated' and 'auth_claims'.")
133+
134+
if unauthenticated is not None:
135+
if not isinstance(unauthenticated, bool):
136+
raise ValueError("'unauthenticated' must be a boolean.")
137+
super().__init__(unauthenticated=unauthenticated)
138+
else:
139+
if not isinstance(claims, dict):
140+
raise ValueError("'auth_claims' must be a dictionary.")
141+
super().__init__(authClaims=claims)
110142

111143
@staticmethod
112144
def unauthenticated() -> 'Impersonation':
@@ -119,7 +151,7 @@ def authenticated(auth_claims: Dict[str, Any]) -> 'Impersonation':
119151
120152
# TODO: More strongly type auth_claims later.
121153
"""
122-
return Impersonation(authClaims=auth_claims)
154+
return Impersonation(auth_claims=auth_claims)
123155

124156

125157
@dataclass
@@ -129,6 +161,7 @@ class GraphqlOptions(Generic[_Variables]):
129161
impersonate: Optional[Union[Impersonation, Dict[str, Any]]] = None
130162

131163

164+
# TODO(b/406281627): Add support for partial errors.
132165
@dataclass
133166
class ExecuteGraphqlResponse(Generic[_Data]):
134167
"""Represents the response from a DataConnect GraphQL execution.
@@ -141,6 +174,7 @@ class ExecuteGraphqlResponse(Generic[_Data]):
141174

142175
class DataConnect:
143176

177+
144178
"""Represents a Firebase Data Connect client instance.
145179
146180
This client provides access to the Firebase Data Connect service
@@ -479,11 +513,13 @@ def _parse_graphql_response(
479513
"""Parses a raw GraphQL response payload into ExecuteGraphqlResponse."""
480514
if not isinstance(resp_dict, dict):
481515
raise exceptions.InternalError(
482-
message="Response payload is not a valid JSON dictionary."
516+
message=f"Response payload is not a valid JSON dictionary: {resp_dict}"
483517
)
484518

519+
# TODO(b/406281627): Add support for partial errors.
485520
return ExecuteGraphqlResponse(data=resp_dict.get("data"))
486521

522+
487523
def _execute_graphql_helper(
488524
self,
489525
query: str,

tests/test_data_connect.py

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -883,8 +883,72 @@ def test_parse_graphql_response_non_dict_error(self):
883883
assert str(excinfo.value) == "Response payload is not a valid JSON dictionary."
884884

885885

886+
class TestImpersonation:
887+
"""Unit tests for Impersonation class and constructor validation."""
888+
889+
def test_unauthenticated_factory(self):
890+
"""Tests factory method for unauthenticated impersonation."""
891+
imp = dataconnect.Impersonation.unauthenticated()
892+
assert imp == {"unauthenticated": True}
893+
894+
def test_authenticated_factory(self):
895+
"""Tests factory method for authenticated impersonation."""
896+
claims = {"sub": "user_123"}
897+
imp = dataconnect.Impersonation.authenticated(claims)
898+
assert imp == {"authClaims": {"sub": "user_123"}}
899+
900+
def test_constructor_unauthenticated(self):
901+
"""Tests direct constructor with unauthenticated=True."""
902+
imp = dataconnect.Impersonation(unauthenticated=True)
903+
assert imp == {"unauthenticated": True}
904+
905+
def test_constructor_auth_claims(self):
906+
"""Tests direct constructor with auth_claims dict."""
907+
claims = {"sub": "user_123"}
908+
imp = dataconnect.Impersonation(auth_claims=claims)
909+
assert imp == {"authClaims": {"sub": "user_123"}}
910+
911+
def test_constructor_auth_claims_camel_case(self):
912+
"""Tests direct constructor with authClaims dict."""
913+
claims = {"sub": "user_123"}
914+
imp = dataconnect.Impersonation(authClaims=claims)
915+
assert imp == {"authClaims": {"sub": "user_123"}}
916+
917+
def test_constructor_both_claims_and_camel_case_fails(self):
918+
"""Tests specifying both auth_claims and authClaims raises ValueError."""
919+
with pytest.raises(ValueError, match="Cannot specify both 'auth_claims' and 'authClaims'."):
920+
dataconnect.Impersonation(auth_claims={"sub": "1"}, authClaims={"sub": "2"})
921+
922+
def test_constructor_neither_unauth_nor_claims_fails(self):
923+
"""Tests specifying neither unauthenticated nor claims raises ValueError."""
924+
with pytest.raises(
925+
ValueError,
926+
match="Impersonation requires either 'unauthenticated=True' or 'auth_claims'."
927+
):
928+
dataconnect.Impersonation()
929+
930+
def test_constructor_both_unauth_and_claims_fails(self):
931+
"""Tests specifying both unauthenticated and claims raises ValueError."""
932+
with pytest.raises(
933+
ValueError,
934+
match="Cannot specify both 'unauthenticated' and 'auth_claims'."
935+
):
936+
dataconnect.Impersonation(unauthenticated=True, auth_claims={"sub": "123"})
937+
938+
def test_constructor_invalid_unauthenticated_type(self):
939+
"""Tests non-boolean unauthenticated raises ValueError."""
940+
with pytest.raises(ValueError, match="'unauthenticated' must be a boolean."):
941+
dataconnect.Impersonation(unauthenticated="not-a-bool")
942+
943+
def test_constructor_invalid_auth_claims_type(self):
944+
"""Tests non-dict auth_claims raises ValueError."""
945+
with pytest.raises(ValueError, match="'auth_claims' must be a dictionary."):
946+
dataconnect.Impersonation(auth_claims="not-a-dict")
947+
948+
886949
class TestDataConnectExecuteGraphql:
887950

951+
888952
def setup_method(self):
889953
self.cred = testutils.MockCredential()
890954
self.app = firebase_admin.initialize_app(
@@ -990,9 +1054,10 @@ class User:
9901054
name: str
9911055

9921056
options = dataconnect.GraphqlOptions(variables={"name": "Fred"})
993-
with pytest.raises(ValueError, match="Expected variables of type User"):
1057+
with pytest.raises(ValueError, match="variables must be of type User"):
9941058
self.api_client.execute_graphql("query { foo }", options=options, variables_type=User)
9951059

1060+
9961061
@mock.patch.object(dataconnect._DataConnectApiClient, "_make_gql_request")
9971062

9981063
def test_execute_graphql_success(self, mock_make_gql_request):

0 commit comments

Comments
 (0)