Repository navigation
change(identity): emit timezone-aware UTC datetimes for Auth blocking events - #324
Conversation
…vents BREAKING CHANGE: Auth blocking functions (before_user_created and before_user_signed_in) now emit timezone-aware UTC datetimes (tzinfo=timezone.utc) instead of offset-naive datetimes for all timestamp fields (AuthBlockingEvent.timestamp, AuthUserMetadata.creation_time, AuthUserMetadata.last_sign_in_time, AuthUserRecord.tokens_valid_after_time, AuthMultiFactorInfo.enrollment_time, and Credential.expiration_time), matching 2nd-Gen Auth triggers and the rest of the SDK.
There was a problem hiding this comment.
Code Review
This pull request refactors datetime conversions in _identity_fn.py to use timezone-aware UTC timestamps instead of deprecated UTC methods, and adds a unit test to verify timezone awareness. The review feedback identifies critical edge cases where None or falsy values (such as 0) for timestamps or expiration times could cause runtime errors or incorrect behavior, and suggests robust is not None checks to resolve these issues.
ajperel
left a comment
There was a problem hiding this comment.
AI also thinks:
PR Title Will Misclassify Breaking Change Under ### Bug Fixes on Release (generate_changelog.sh)
Repo settings enforce squash-only merges with squash_merge_commit_title: "PR_TITLE" and squash_merge_commit_message: "BLANK" (stripping commit 5f7fc5e's BREAKING CHANGE: footer).
generate_changelog.sh only maps ^change((.*))?: to ### Breaking Changes; fix(identity): ... lands in ### Bug Fixes, while fix(identity)!: ... misses all regexes and is silently omitted in MISC.
Action: Rename PR #324's title to change(identity): emit timezone-aware UTC datetimes for Auth blocking events.
-- Could you look into all these findings and let me know if they're valid (and if so fix them)
…, and bound oauth_expires_in
ajperel
left a comment
There was a problem hiding this comment.
LGTM, but maybe worth having Invertase also look.
Summary
Unifies all timestamp fields on
AuthBlockingEvent(before_user_createdandbefore_user_signed_in) to emit timezone-aware UTCdatetimeobjects (tzinfo=timezone.utc), matching 2nd-Gen Auth triggers (on_user_created,on_user_deleted) and the rest of thefirebase-functions-pythonSDK.Also resolves two latent timestamp bugs in Auth blocking functions and eliminates Python 3.12+
datetime.utcfromtimestamp()deprecation warnings.Changes
datetime.utcfromtimestamp()withdatetime.fromtimestamp(..., tz=timezone.utc)(src/firebase_functions/private/_identity_fn.py):AuthUserMetadata.creation_timeAuthUserMetadata.last_sign_in_timeAuthUserRecord.tokens_valid_after_timeCredential.expiration_timeAuthBlockingEvent.timestamplocal-timezone drift:_dt.datetime.fromtimestamp(token_data["iat"])to_dt.datetime.fromtimestamp(token_data["iat"], tz=_dt.timezone.utc)soevent.timestampis always UTC rather than the host machine's local timezone.'Z'timestamp parsing onAuthMultiFactorInfo.enrollment_time:_dt.datetime.fromisoformat(enrollment_time)with_util.timestamp_conversion(enrollment_time), which supports trailing'Z'UTC designators and nanosecond precision across Python 3.10+.tests/test_identity_fn.py):test_blocking_event_timestamps_are_timezone_aware_utcverifying all 6 timestamp fields onAuthBlockingEventhavetzinfo=timezone.utc.before_user_createdandbefore_user_signed_in) now emit timezone-aware UTCdatetimeobjects (tzinfo=timezone.utc) instead of offset-naivedatetimeobjects for all timestamp fields (AuthBlockingEvent.timestamp,AuthUserMetadata.creation_time,AuthUserMetadata.last_sign_in_time,AuthUserRecord.tokens_valid_after_time,AuthMultiFactorInfo.enrollment_time, andCredential.expiration_time). Arithmetic and comparisons must use timezone-aware datetimes (e.g.,datetime.now(timezone.utc)).