Skip to content

Fix MFA enrolledAt being sent as a malformed timestamp - #792

Open
rootkiller6788 wants to merge 2 commits into
firebase:mainfrom
rootkiller6788:fix-mfa-enrollment-timestamp
Open

rootkiller6788 wants to merge 2 commits into
firebase:mainfrom
rootkiller6788:fix-mfa-enrollment-timestamp

Conversation

@rootkiller6788

Copy link
Copy Markdown

Fixes #764.

UpdateUser() with a non-zero EnrollmentTimestamp sent a broken enrolledAt and the Auth backend threw out the whole request with "Invalid time format: Illegal trailing data in input string". Two things were wrong in convertMultiFactorInfoToServerFormat:

  • the layout string ended with a literal Z after the Z07:00 token, so the value came out as 53140-03-29T09:33:20+08:00Z
  • the field is milliseconds (GetUser fills it from t.Unix()*1000, and the integration test passes it straight back into UpdateUser) but it was handed to time.Unix as seconds

time.UnixMilli(...).UTC().Format(time.RFC3339) fixes both, and GetUser -> UpdateUser round-trips cleanly now.

I also pinned the MFA case in updateUserCases to a fixed timestamp instead of time.Now() on both sides — it never actually asserted the serialized value and only passed when the machine timezone happened to line up.

go test ./auth/ ./messaging/ passes. Reverting just user_mgt.go against the new test makes TestUpdateUser fail with the old malformed string, so the case really does cover this path.

UpdateUser() with a non-zero EnrollmentTimestamp serialized
"53140-03-29T09:33:20+08:00Z" and the Auth backend rejected the whole
request with "Invalid time format: Illegal trailing data in input string".

The layout had a literal Z after the Z07:00 token, and the field is in
milliseconds but was handed to time.Unix as seconds.
The case built both sides with time.Now(), so it never actually pinned
the serialized enrolledAt and only matched when the machine timezone
happened to line up. Use a fixed millisecond value and expect the exact
RFC3339 string the backend now receives.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the multi-factor authentication enrollment timestamp conversion to correctly handle milliseconds since epoch using time.UnixMilli and formats the output in UTC using time.RFC3339. Additionally, the corresponding test cases in auth/user_mgt_test.go have been updated to use static timestamps instead of dynamic current times, ensuring deterministic test runs. There are no review comments to address.

@rootkiller6788
rootkiller6788 marked this pull request as ready for review October 5, 2026 06:09

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MFA UpdateUser fails with "Illegal trailing data" when EnrollmentTimestamp is set

1 participant