Repository navigation
feat(identity): add v2 Auth triggers (on_user_created and on_user_deleted) - #318
shettyvarun268 wants to merge 8 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces support for Firebase Authentication event triggers (on_user_created and on_user_deleted) by adding the AuthEvent class, AuthOptions configuration, and the necessary Eventarc CloudEvent parsing logic. It also includes comprehensive unit tests to verify the functionality. The feedback suggests avoiding the use of the private _get_attributes() method on CloudEvent in favor of its public dictionary-like interface, and adding safety checks to ensure that raw_factors and raw_provider_data are lists before iterating over them to prevent potential TypeError exceptions on malformed payloads.
23f6a65 to
6c1969b
Compare
6c1969b to
9a13f3c
Compare
IzaakGough
left a comment
There was a problem hiding this comment.
Left some comments inline :)
| _EPOCH_START_UTC = _dt.datetime.fromtimestamp(0, tz=_dt.timezone.utc) | ||
|
|
||
|
|
||
| def _ensure_utc(value: _dt.datetime) -> _dt.datetime: |
There was a problem hiding this comment.
This path gives tz-aware datetimes but the blocking path (utcfromtimestamp at 79, 82, 141) gives naive ones, into the same AuthUserMetadata fields. Comparing a creation_time from each raises TypeError: can't compare offset-naive and offset-aware datetimes. Do we want these to be consistent?
There was a problem hiding this comment.
Great call-out Izaak. Some additional context, Auth blocking events use timezone-naive timestamps while the rest of the SDK uses timezone aware ones. Plus, tz-naive timestamps have been deprecated as of Python 3.12 due to the number of potential bugs it can cause.
So all things considered, I think it would make sense for us to be consistent and bring the Auth Blocking events to tz-aware timestamps to keep things consistent with the rest of the SDK.
But since its a breaking change(and kinda out of scope for this particular PR stack), I am leaning towards landing this as is and then fixing the tz issue for the blocking path in a follow-up PR in time for a breaking change release.
Thanks again for the review.
…ull/unrecognized payloads
| # Default SMS factors that omit an explicit factorId to "phone". | ||
| factor_id = raw_factor_id or ("phone" if phone_number else "") | ||
| return AuthMultiFactorInfo( | ||
| uid=factor_data.get("uid", ""), |
There was a problem hiding this comment.
nit: factor_data.get("uid", "") returns None when "uid": null is explicitly present in the JSON payload, which breaks the non-optional uid: str annotation on AuthMultiFactorInfo.
| uid=factor_data.get("uid", ""), | |
| uid=factor_data.get("uid") or "", |
| raw_time = event_attributes.get("time") | ||
| event_time = _parse_auth_timestamp(raw_time) or _dt.datetime.now(_dt.timezone.utc) |
There was a problem hiding this comment.
Falling back to _dt.datetime.now(_dt.timezone.utc) makes event.time non-deterministic across Eventarc retries (retry=True), since every retry attempt receives a new wall-clock timestamp. Every other v2 CloudEvent handler in this SDK (db_fn.py, firestore_fn.py, dataconnect_fn.py, storage_fn.py, _alerts_fn.py) raises ValueError when the CloudEvent time attribute is missing or malformed.
| raw_time = event_attributes.get("time") | |
| event_time = _parse_auth_timestamp(raw_time) or _dt.datetime.now(_dt.timezone.utc) | |
| raw_time = event_attributes.get("time") | |
| event_time = _parse_auth_timestamp(raw_time) | |
| if event_time is None: | |
| raise ValueError(f"Invalid or missing CloudEvent 'time' attribute: {raw_time!r}") |
| return AuthUserInfo( | ||
| uid=info_data.get("uid", ""), | ||
| provider_id=info_data.get("providerId", ""), |
There was a problem hiding this comment.
Production Eventarc (google.events.firebase.auth.v2.UserInfo) sends "uid" inside providerData entries, but the Firebase Auth Emulator (firebase-tools toSerializedUserInfo) sends "rawId" with no "uid" key. Right now, event.data.provider_data[i].uid silently evaluates to "" when testing against the local Auth Emulator.
Also using or "" on providerId so an explicit {"providerId": null} falls back to "" instead of None (AuthUserInfo.provider_id is typed as non-nullable str).
| return AuthUserInfo( | |
| uid=info_data.get("uid", ""), | |
| provider_id=info_data.get("providerId", ""), | |
| return AuthUserInfo( | |
| # Production Eventarc sends 'uid'; Auth Emulator sends 'rawId'. | |
| uid=info_data.get("uid") or info_data.get("rawId") or "", | |
| provider_id=info_data.get("providerId") or "", |
| # Check the CloudEvent 'tenantid' attribute (Eventarc standard), falling back | ||
| # to the user record's tenant_id (used by the Auth Emulator). | ||
| tenant_id: str | None = event_attributes.get("tenantid") or user_record.tenant_id or None |
There was a problem hiding this comment.
In production Eventarc (google/events/firebase/auth/v2/data.proto), the User message has no tenantId field in the JSON body—Identity Platform only emits tenantid as a top-level CloudEvent attribute. Only the Auth Emulator includes tenantId inside the user JSON object.
Because _auth_user_record_from_ce_data (line 565) only checks user_data.get("tenantId"), event.data.tenant_id will always be None in production Eventarc, even when event.tenant_id is populated here. Since AuthBlockingEvent populates event.data.tenant_id on the same AuthUserRecord class, developers reading event.data.tenant_id in v2 triggers will hit a silent None in production.
Consider passing event_attributes.get("tenantid") into _auth_user_record_from_ce_data as a fallback (or backfilling user_record via _dataclasses.replace(user_record, tenant_id=tenant_id) when user_record.tenant_id is None).
| if isinstance(value, str) and value.isdigit(): | ||
| value = int(value) |
There was a problem hiding this comment.
str.isdigit() returns True for Unicode superscripts and enclosed digits (such as "²" and "①"), which then raise an unhandled ValueError in int(value) outside the try / except block below. Restricting to ASCII decimal digits avoids the crash.
| if isinstance(value, str) and value.isdigit(): | |
| value = int(value) | |
| if isinstance(value, str) and value.isascii() and value.isdecimal(): | |
| value = int(value) |
| custom_claims=_parse_custom_claims(user_data), | ||
| tenant_id=user_data.get("tenantId") or None, | ||
| tokens_valid_after_time=_parse_auth_timestamp(user_data.get("tokensValidAfterTime")), | ||
| multi_factor=_auth_multi_factor_settings_from_ce_data(user_data.get("multiFactor")), |
There was a problem hiding this comment.
Production Eventarc (data.proto) does not include MFA fields, and the Firebase Auth Emulator sends "mfaInfo": [{"mfaEnrollmentId": ..., "displayName": ..., "enrolledAt": ..., "phoneInfo": ...}] at the top level of the user dict rather than "multiFactor": {"enrolledFactors": [...]}.
As written, event.data.multi_factor will be None in both production Eventarc and the Auth Emulator. If MFA parsing here is meant to work with the Auth Emulator (or Identity Toolkit UserInfo payloads), we should also handle user_data.get("mfaInfo").
| class AuthOptions(EventHandlerOptions): | ||
| """ | ||
| Options that can be set on an Auth event trigger. | ||
| Internal use only. |
There was a problem hiding this comment.
nit: AuthOptions is publicly re-exported in identity_fn.py (from firebase_functions.options import AuthOptions) and linked in the public :type **kwargs: docstrings of on_user_created and on_user_deleted. Should the "Internal use only" string be dropped?
| provider_id=info_data.get("providerId", ""), | ||
| display_name=info_data.get("displayName"), | ||
| email=info_data.get("email"), | ||
| # Auth Emulator sends 'photoURL'; production Eventarc sends 'photoUrl'. |
There was a problem hiding this comment.
Minor comment nit (also on line 556): in google/events/firebase/auth/v2/data.proto, photo_url is annotated with [json_name = "photoURL"], so production Eventarc also sends "photoURL". Checking both keys is still good defensive practice and matches the Node SDK, so only the comment wording is affected.
| "tenantId": "my-tenant", | ||
| } | ||
| }, | ||
| ) | ||
|
|
||
| example(raw_ce) # type: ignore[arg-type] | ||
| self.assertIsNotNone(captured_event) | ||
| assert captured_event is not None | ||
| self.assertEqual(captured_event.project, "my-project") | ||
| self.assertEqual(captured_event.tenant_id, "my-tenant") | ||
| self.assertIsNotNone(captured_event.data) | ||
| assert captured_event.data is not None | ||
| self.assertEqual(captured_event.data.uid, "1qEveruhwnbiG1B9taHIBMmOVE83") | ||
| self.assertEqual(captured_event.data.email, "testuser@example.com") | ||
| self.assertTrue(captured_event.data.email_verified) | ||
| self.assertEqual(captured_event.data.display_name, "Test User") | ||
| self.assertEqual(captured_event.data.photo_url, "http://example.com/photo.jpg") | ||
| self.assertEqual(captured_event.data.phone_number, "+15555550100") | ||
| self.assertFalse(captured_event.data.disabled) | ||
| self.assertEqual( | ||
| captured_event.data.metadata.creation_time, | ||
| dt.datetime(2023, 1, 1, 0, 0, 0, tzinfo=dt.timezone.utc), | ||
| ) | ||
| self.assertEqual( | ||
| captured_event.data.metadata.last_sign_in_time, | ||
| dt.datetime(2023, 1, 2, 12, 30, 0, tzinfo=dt.timezone.utc), | ||
| ) | ||
| self.assertEqual(len(captured_event.data.provider_data), 1) | ||
| self.assertEqual(captured_event.data.provider_data[0].provider_id, "google.com") | ||
| self.assertEqual(captured_event.data.custom_claims, {"admin": True}) | ||
| self.assertEqual(captured_event.data.tenant_id, "my-tenant") |
There was a problem hiding this comment.
Two test coverage notes here:
- Setting both
attributes["tenantid"](line 143) anddata["value"]["tenantId"](line 168) in the same fixture masks the production Eventarc payload shape, wheredata["value"]has notenantIdkey. Splitting into a production Eventarc fixture (header-onlytenantid,providerData[].uid) and an Auth Emulator fixture (bodytenantId,providerData[].rawId,metadata.creationTime) would catch both parity issues. - Line 196 checks
provider_data[0].provider_id, but does not assert.uid,.display_name,.email, or.photo_url.
| import firebase_functions.core as _core | ||
| import firebase_functions.options as _options | ||
| import firebase_functions.private.util as _util | ||
| from firebase_functions.options import AuthOptions # noqa: F401 |
There was a problem hiding this comment.
nit: Since this package ships py.typed (PEP 561), strict type checkers (mypy --no-implicit-reexport, pyright) treat from ... import AuthOptions as private unless it uses as AuthOptions (or __all__). Using as AuthOptions also removes the need for # noqa: F401.
| from firebase_functions.options import AuthOptions # noqa: F401 | |
| from firebase_functions.options import AuthOptions as AuthOptions |
Summary
Adds 2nd Gen Firebase Authentication Eventarc triggers (
on_user_createdandon_user_deleted) tofirebase_functions.identity_fn, bringing feature parity with the Node.js SDK (firebase-functions/v2/identity).Fixes #94
Usage
Architectural & Implementation Details
1. Global Eventarc Trigger Region (
manifest.py,options.py)eventTrigger["region"] = "global"is required: Unlike regional event sources (such as Firestore or Cloud Storage), Firebase Authentication / Identity Platform is a global service and only publishes Eventarc events in the"global"Eventarc location—even when the Cloud Run function itself is deployed to a specific region (e.g.,us-central1).region: NotRequired[str]tomanifest.EventTriggerand setendpoint.eventTrigger["region"] = "global"insideAuthOptions._endpoint().Expressioninstances: Unlike some olderOptions._endpoint()methods that wrapsuper()._endpoint()indataclasses.asdict(),AuthOptions._endpoint()mutatesendpoint.eventTriggerdirectly. Callingdataclasses.asdict()recursively converts nested dataclasses—such asStringParam(Expression)insideeventFilters["tenantid"]—into plaindicts, which would preventmanifest._dict_to_spec()from serializing{{ params.TENANT_ID }}expressions properly.2. Public API & Event Model (
identity_fn.py)@on_user_created()and@on_user_deleted(): Decorators configured viaAuthOptions(EventHandlerOptions), registeringgoogle.firebase.auth.user.v2.createdandgoogle.firebase.auth.user.v2.deletedendpoints respectively. Also re-exportsAuthOptionsfor convenience.AuthEvent(CloudEvent[AuthUserRecord]):_core.CloudEvent[AuthUserRecord]soevent.datais a non-optionalAuthUserRecord(matchingAuthBlockingEventin Python andAuthEvent<User>in the Node SDK).project: str | Noneandtenant_id: str | Nonefields extracted from the CloudEvent attributes.3. Runtime CloudEvent Unpacking & Normalization (
private/_identity_fn.py)AuthEventDataEnvelope Unpacking (_unwrap_auth_event_data):data["value"]foron_user_createdevents and insidedata["oldValue"]foron_user_deletedevents.isinstance(raw_data.get(envelope_key), dict)acrossvalueandoldValue(so protobuf-serialized dicts like{"value": None, "oldValue": {...}}do not shadowoldValue), and falls back to the top-level dictionary for synthetic/unit test events.AuthUserRecord,AuthUserMetadata,AuthUserInfo, andAuthMultiFactorSettings, while handling key differences between production Eventarc and the Firebase Auth Emulator (photoUrlvs.photoURLandcreateTimevs.creationTime).AuthUserMetadata.creation_timefor backward compatibility withbefore_user_created/before_user_signed_in, falling back to UTC epoch 0 (1970-01-01T00:00:00Z) when partial v2 deletion payloads omit metadata timestamps.customClaims(_parse_custom_claims) whether delivered as a parseddictor a JSON-encoded string (e.g.,'{"admin": true}')._parse_auth_timestamp&_ensure_utc) from epoch milliseconds (int,float, or numericstr) and ISO-8601 strings into timezone-aware UTCdatetimeobjects.projectfromsource(//identitytoolkit.googleapis.com/projects/{project}) and resolvestenant_idfrom the CloudEventtenantidattribute with fallback touser_record.tenant_id.Verification
pytest(all unit tests passing, includingTestIdentitysuite covering endpoint metadata,functions_as_yamlStringParamrendering,value/oldValueunpacking, MFA, and JSON-stringcustomClaims)mypy&ruff check/ruff format --check./docs/generate.sh --out=./docs/build/ --pypath=src/) with-W(zero warnings)on_user_createdandon_user_deletedexecution for both default-project users and tenant-scoped users.PR Stack: