Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Handle additional errors from verifyIdToken #174

Closed
Tracked by #265
kmjennison opened this issue May 8, 2021 · 3 comments
Closed
Tracked by #265

Handle additional errors from verifyIdToken #174

kmjennison opened this issue May 8, 2021 · 3 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@kmjennison
Copy link
Contributor

Is your feature request related to a problem? Please describe.

Firebase admin's verifyIdToken may throw some errors that this module should probably handle. See this thread for a background discussion: #125

See a list of Firebase admin auth errors here:
https://firebase.google.com/docs/reference/node/firebase.auth.Error

Currently, we only handle auth/id-token-expired.

Describe the solution you'd like and how you'd implement it

When calling verifyIdToken, this module should probably handle the following errors:

  • auth/invalid-user-token: return an unauthed user
  • auth/user-token-expired: return an unauthed user
  • auth/user-disabled: return an unauthed user
  • auth/argument-error: call to refresh the ID token—however, we need to confirm this will address the kid claim error (see this comment for more info).

Is this a breaking change?
Yes, in the case developers are catching and handling these errors themselves.

Describe alternatives you've considered
Continue to ignore errors and let developers handle them. I can't think of a compelling reason not to handle any of the above errors.

@kmjennison kmjennison added enhancement New feature or request help wanted Extra attention is needed breaking The issue or PR will introduce a breaking change labels May 8, 2021
@kmjennison kmjennison removed the breaking The issue or PR will introduce a breaking change label May 28, 2021
@kmjennison kmjennison mentioned this issue Aug 7, 2021
24 tasks
HofmannZ added a commit to zino-hofmann/next-firebase-auth that referenced this issue Oct 10, 2021
HofmannZ added a commit to zino-hofmann/next-firebase-auth that referenced this issue Oct 10, 2021
HofmannZ added a commit to zino-hofmann/next-firebase-auth that referenced this issue Oct 10, 2021
HofmannZ added a commit to zino-hofmann/next-firebase-auth that referenced this issue Oct 26, 2021
HofmannZ added a commit to zino-hofmann/next-firebase-auth that referenced this issue Oct 26, 2021
HofmannZ added a commit to zino-hofmann/next-firebase-auth that referenced this issue Nov 22, 2021
HofmannZ added a commit to zino-hofmann/next-firebase-auth that referenced this issue Nov 22, 2021
@alex-knyazev
Copy link

alex-knyazev commented Nov 24, 2021

If understand correctly this is related to the described issue:
I have this error when I use emulator. I deleted user from Firebase auth and then refreshed the page. Of course we shouldn't have such error, we should handle it in some way.

image

E.g. for this error I think we should be able to delete cooks and use 'unAuthed; flow

export default withAuthUser({
  whenAuthed: AuthAction.REDIRECT_TO_LOGIN,
  whenError: AuthAction.REDIRECT_TO_LOGIN,
})(IndexPage)

kmjennison added a commit that referenced this issue Dec 10, 2021
* fix: check for 'auth/argument-error' when verifying token

* feat: upgrade firebase and firebase-admin

* feat(#174): handle additional errors from `verifyIdToken`

* test(#174): add tests coverage for new errors in `verifyIdToken`

* feat: upgrade dependencies that have non breaking changes

* feat: implement pr feedback

* chore: upgrade dependencies

* Rebuild lockfile

* Include error if empty refreshToken

* Add TODO

Co-authored-by: Zino Hofmann <zino@hofmann.amsterdam>
@kmjennison
Copy link
Contributor Author

Closed in #361.

kmjennison added a commit that referenced this issue Dec 13, 2021
* fix: check for 'auth/argument-error' when verifying token

* feat: upgrade firebase and firebase-admin

* feat(#174): handle additional errors from `verifyIdToken`

* test(#174): add tests coverage for new errors in `verifyIdToken`

* feat: upgrade dependencies that have non breaking changes

* feat: implement pr feedback

* chore: upgrade dependencies

* Rebuild lockfile

* Include error if empty refreshToken

* Add TODO

Co-authored-by: Zino Hofmann <zino@hofmann.amsterdam>
kmjennison added a commit that referenced this issue Dec 13, 2021
* fix: check for 'auth/argument-error' when verifying token

* feat: upgrade firebase and firebase-admin

* feat(#174): handle additional errors from `verifyIdToken`

* test(#174): add tests coverage for new errors in `verifyIdToken`

* feat: upgrade dependencies that have non breaking changes

* feat: implement pr feedback

* chore: upgrade dependencies

* Rebuild lockfile

* Include error if empty refreshToken

* Add TODO

Co-authored-by: Zino Hofmann <zino@hofmann.amsterdam>

Co-authored-by: Zino Hofmann <zino@hofmann.amsterdam>
@thanhtutzaw

This comment was marked as duplicate.

kmjennison added a commit that referenced this issue Jul 8, 2023
* Debug release action

* 0.14.0-alpha.0

* Revert "0.14.0-alpha.0"

This reverts commit 1e04c95.

* Remove --dry-run from release action

* Display the Firebase version on the demo app

* Update README.md

* Update issue templates

* feat: forceRefresh on getIdToken

* feat: adding description about forceRefresh

* feat: adding argument type to getIdToken

* Update createAuthUser.js

* 0.13.4-alpha.0

* 0.13.4

* Upgrade some dependencies (#325)

* Upgrade some deps

* Upgrade caniuse

* Upgrade Prettier

* Downgrade eslint to satisfy peerdeps

* Minor upgrades for Next and Firebase deps

* Upgrade firebase-admin

* Upgrade firebase-admin in demo

* Minor upgrade demo deps

* Upgrade NFA version in demo

* Upgrade more dependencies

* Upgrade more demo dependencies

* Revert "Upgrade more demo dependencies"

This reverts commit 7da3e58.

* Revert "Minor upgrade demo deps"

This reverts commit 50f928e.

* Minor upgrade Firebase

* Pin typescript version (typing error with 4.4.4) and minor upgrade other types

* Minor bump a few demo deps

* Upgrade more demo deps

* Upgrade some deps

* Remove unused Codecov dependency

* Support Next 12 and Firebase Admin 10 (#328)

* Use Next 12

* Allow latest versions of Next and firebase-admin

* 0.13.5-alpha.0

* Update demo to use Next 12  (#330)

* Use Next 12

* Allow latest versions of Next and firebase-admin

* Upgrade Next to v12

* Update demo

* Update README.md

* Update README.md

* Change example app cookies to use SameSite=lax (#354)

* 0.13.5

* feat: add support for application default credentials (#348)

* feat: fallback to applicationDefault credentials

Co-Authored-By: Jesse Anderson <jeryanders@gmail.com>

* chore: add test for firebaseAdminDefaultCredential

* chore: update README with firebaseAdminDefaultCredential

Co-Authored-By: Jesse Anderson <jeryanders@gmail.com>

* chore: cleanup wording in README comments

* fix: updates based on comments

* chore: update error message in test

Co-authored-by: Jesse Anderson <jeryanders@gmail.com>

* Update README.md

* Upgrade dependencies on v0.x (#356)

* Upgrade most deps

* Upgrade ESLint and Prettier and lint fix

* Remove unneeded jsdom dep

* Upgrade most example app deps

* Upgrade example app lockfile deps

* Upgrade lockfile deps

* Fix peer dependency range syntax for firebase-admin (#358)

* Handle additional token errors in verifyIdToken (#361) (#365)

* fix: check for 'auth/argument-error' when verifying token

* feat: upgrade firebase and firebase-admin

* feat(#174): handle additional errors from `verifyIdToken`

* test(#174): add tests coverage for new errors in `verifyIdToken`

* feat: upgrade dependencies that have non breaking changes

* feat: implement pr feedback

* chore: upgrade dependencies

* Rebuild lockfile

* Include error if empty refreshToken

* Add TODO

Co-authored-by: Zino Hofmann <zino@hofmann.amsterdam>

Co-authored-by: Zino Hofmann <zino@hofmann.amsterdam>

* Remove thrown errors from token refresh & verification logic (#368)

* Add broken tests

* Don't throw on token errors

* Add assertion checks to tests

* Add/modify comments in config validation

* Move tests into describe block

* Add error callbacks to config

* Call error callbacks when we fail to refresh or verify the user's ID token

* Add new config properties to types

* Await error callback functions in case they need to perform something async

* Lint fixes

* Add tests

* Update README.md

* Fix typo in README (#374)

* Cherry pick v1.x #369 (#375)

Co-authored-by: Faris Abusada <abusada@users.noreply.github.com>

* Allow easy error handling for login/logout requests (#376)

* Add error catching to default token changed handler

* Add new config properties

* Update README.md

* Update README.md

* Run Prettier on README (#381)

* 0.14.0-alpha.o

* Update README.md

* 0.14.0-alpha.1

* Update v0.x example (#382)

* Update documentation (#387)

* Update docs on private key formatting and Vercel environment variables (#385)

* Update README.md

* Update README.md

* Update README.md

* Update README.md

* Update README.md

* Add note about not using API routes in getServerSideProps (#386)

* 0.14.0

* Added troubleshooting step to README (#398)

* Update README.md

* feat: add ability to define to redirect to app with different base path (#352)

* feat: add generalized redirect AuthAction

* test: add tests for csr and ssr withAuthUser

* fix: made error message more focused on a given auth state

* fix: removed new AuthAction and address issue 187 with new solution

* test: add main tests for new schema on appPageURL and authPageURL

* refactor: move destination logic into common function

* refactor: add redirects

* refactor: error messaging around redirects

* fix: router and window location

* test: add test coverage for new supported data type

* docs: rework docs

* fix: error message for authPageURL

* fix: bug in ssr component and slightly later url schema

* docs: change schema property

* fix: allow basePath on SSR to be passed based on findings

* chore: make naming consistent between csr / ssr

* fix: rework based on feedback

* refactor: bring config access into redirect module

* style: spacing and formatting

* docs: update readme and types

* refactor: simplify object name

* fix: update typescript

* tests: fix tests causing coverage issues

* fix: rework from feedback

* fix: implement feedback

* test: add additional test

* test: adjust test name

* Update documentation for redirects (#400)

* Add PageURL type to README

* Typo fix

* Link to PageURL type in docs

* Run Prettier on README

* Fix incorrect documentation args

* Link to PageURL type from example

* Tweak README

* Fix typo

* Add info about ctx

* Remove redundant info

* Tweak code comments

* 0.14.1-alpha.0

* Update example app (#402)

* 0.14.1

* added onLogoutRequestError and onLoginRequestError to InitConfig interface (#427)

* feat: add tenant integration

* Update link to documentation

* 0.14.2

* Bugfix: don't error on unset Firebase admin config values (#436)

* Identify bug

* When debug logging, handle unset Firebase config values

* Lint fix

* 0.14.3-alpha.0

* fix: Add useFirebaseAdminDefaultCredential type definition (#451)

* Fix README typos, grammar (#448)

* Add useFirebaseAdminDefaultCredential type definition

* Fix typos, grammar, and clarify Google default credentials usage

* docs: Fix grammar, remove type addition from PR

* 0.14.3-alpha.1

* 0.14.3

* Upgrade NFA version in demo (#455)

* Upgrade NFA version in demo

* Update min version

* v0.x: Add support for React 18 (#472)

* Add support for React 18

* Upgrade some testing libraries

* v0.x: upgrade dependencies (#477)

* Upgrade most deps

* Upgrade additional deps

* 0.14.4-alpha.0

* v0.x: Update example app (React 18, other dependencies) (#471)

* Update example to use React 18

* Add latest NFA

* Upgrade other deps in example app

* Ignore type error

* Use supported version of react-firebaseui

* Upgrade additional example app dependencies (#479)

* fix typo. add missing "b" to README.md (#485)

* Support firebase-admin v11 peer dependency (#504)

* Upgrade dependencies [v0.x] (#505)

* Upgrade some deps

* Upgrade Prettier

* Upgrade dependencies

* Upgrade firebase-admin and copy-webpack-plugin

* Upgrade example deps [v0.x] (#507)

* Upgrade deps

* Upgrade deps

* Use compatible react-firebaseui

* 0.14.4-alpha.1

* Bump NFA in package.json

* Upgrade NFA in example (#508)

* 0.14.4

* Use NFA 0.14.4 in example app (#509)

* Add info about NextAuth.js to README [v1.x]

* Update README.md

* v0.x: Update bug issue template (#542)

* v0.x: allow Next v13 peer dependency (#588)

* 0.15.0

* Update README.md

* docs: adds missing import to withAuthUserTokenSSR example

* fix: make sure Firebase admin is initialized in getUserFromCookies

* docs: tenantId commented by default

* fix: correct attribute name to tenantId in deserializedUser

* fix: extract tenantId from firebaseClientInitConfig

* tests: add tenantId tests for createAuthUser. adds tests for tenantId in initFirebaseClientSDK.

* merge fixes

* merge fixes

* merge fixes

* merge fixes

* fix: firebaseAdmin test. use getAuth

* remove extra changes from docs

* fix doc formatting

* fix minor issues

* empy line

* fix: change auth instance

* fix: remove admin import

* fix: move tenantId outside firebaseClientInitConfig. Fix typos

* Update src/__tests__/firebaseAdmin.test.ts

---------

Co-authored-by: Kevin Jennison <kevin.jennison1@gmail.com>
Co-authored-by: Guilherme <guiilherme.bayer@gmail.com>
Co-authored-by: Scott Prue <prescottprue@users.noreply.github.com>
Co-authored-by: Jesse Anderson <jeryanders@gmail.com>
Co-authored-by: Zino Hofmann <zino@hofmann.amsterdam>
Co-authored-by: Alexander Cai <alexandercai@outlook.com>
Co-authored-by: Faris Abusada <abusada@users.noreply.github.com>
Co-authored-by: Vinny <vpaladino778@gmail.com>
Co-authored-by: Jesse Anderson <jesse.anderson@sideinc.com>
Co-authored-by: camilo-mujica <84539709+camilo-mujica@users.noreply.github.com>
Co-authored-by: Hegar Garcia <hegargarcia@gmail.com>
Co-authored-by: Hung Vu <hunghvu2017@gmail.com>
Co-authored-by: nori-k <norikatsu.kamiya@gmail.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants