Skip to content

fix: emulator auth_time (#3608)#3611

Merged
yuchenshi merged 6 commits intofirebase:masterfrom
lovelle-cardoso:fix-emulator-auth_time
Jul 28, 2021
Merged

fix: emulator auth_time (#3608)#3611
yuchenshi merged 6 commits intofirebase:masterfrom
lovelle-cardoso:fix-emulator-auth_time

Conversation

@lovelle-cardoso
Copy link
Copy Markdown
Contributor

Description

Fixes #3608 so that emulator sets auth_time the same way production does (auth_time should match lastLoginAt in seconds)

Scenarios Tested

Added unit test to src/test/emulators/auth/misc.spec.ts called "should populate auth_time to match lastLoginAt (in seconds since epoch)"

This unit test...

  1. Registers a user
  2. Exchanges a refresh token for a new token
  3. Checks that the new token's auth_time matches the user's lastLoginAt in seconds.

Made emulator auth_time match how auth_time is populated in production. (auth_time should match user's lastLoginAt in seconds)
Copy link
Copy Markdown
Member

@yuchenshi yuchenshi left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! Looks pretty good to me. A few comments inline for you though. @lisajian will also help with the review

Copy link
Copy Markdown
Contributor

@lisajian lisajian left a comment

Choose a reason for hiding this comment

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

Overall, LGTM pending @yuchenshi's feedback. Thank you for the fix!

@lovelle-cardoso
Copy link
Copy Markdown
Contributor Author

@yuchenshi @lisajian Resolved all 3 comments. I think that's everything! 👍

Copy link
Copy Markdown
Member

@yuchenshi yuchenshi left a comment

Choose a reason for hiding this comment

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

LGTM. I'll update the comments and merge after CI passes

@yuchenshi
Copy link
Copy Markdown
Member

Apparently linter is not happy about the code formatting, but we'll fix this on our side -- just don't delete your fork branch yet!

@yuchenshi yuchenshi merged commit 8e8043b into firebase:master Jul 28, 2021
@yuchenshi
Copy link
Copy Markdown
Member

This is now merged and will be included in the next release. Thanks again for the help!

devpeerapong pushed a commit to devpeerapong/firebase-tools that referenced this pull request Dec 14, 2021
* fix: emulator auth_time (firebase#3608)

Made emulator auth_time match how auth_time is populated in production. (auth_time should match user's lastLoginAt in seconds)

* Check not null just in case lastLoginAt is 0 because of unit test clock mocking

* Advance clock to verify auth_time is not refresh time

* assert user.lastLoginAt is not undefined

* Apply suggestions from code review

* Format code.

Co-authored-by: Yuchen Shi <yuchenshi@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes Manual indication that this has passed CLA.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

auth_time doesn't match user's lastSignInTime (always matches iat instead)

3 participants