feat: add support for application default credentials #348

prescottprue · 2021-12-01T02:19:24Z

Adds firebaseAdminDefaultCredential config option which accepts a boolean - when set to true the application default credentials. This is useful for deploying into Google Cloud Platform environments such as Cloud Functions, Cloud Run, and Compute Engine which have default credentials defined within the environment. This way the keys don't have to be managed/rotated by the user, that is all handled by GCP. Also includes testing and section in README explaining usage.

Instead of just invoking initializeApp without any settings as called out in #75, I opted for using applicationDefault to combine with other configurations such as databaseURL

For firebase-admin v10 the imports would be slightly different (directly from firebase/app), so I'm open to making a PR for that version as well.

NOTE: With current implementation this will take precedence over firebaseAdminInitConfig.credential if it is passed - this makes sense, but wanted to call it out still

Checks

Non-breaking API change
Tests

Open Questions

What are thoughts on the config parameter name? Wanted to keep the firebaseAdmin prefix, but open to ideas for a more clear name
Should default credentials be applied by default if firebaseAdminInitConfig is not passed? I wanted to prevent breaking changes to the interface (and currently there is an error thrown if this is missing) - makes logical sense but could come later

Issues

closes Provide better support for Google Cloud native environments #75

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

* chore: update README with firebaseAdminDefaultCredential Co-Authored-By: Jesse Anderson <jeryanders@gmail.com>

vercel · 2021-12-01T02:19:46Z

@prescottprue is attempting to deploy a commit to the Gladly Team on Vercel.

A member of the Team first needs to authorize it.

kmjennison

Thanks for this PR, the documentation, and test coverage! Requested a few changes.

For your open questions:

I suggested a config property name of useFirebaseAdminDefaultCredential, which seems clearer that the value is a boolean.
Yes, this config is a little redundant. However, it should be useful to retain the credential error. New users of this library commonly get stuck on forgetting or misconfiguring credentials.

README.md

kmjennison · 2021-12-10T16:07:40Z

src/initFirebaseAdminSDK.js

-    if (!firebaseAdminInitConfig) {
+    const { firebaseAdminInitConfig, firebaseAdminDefaultCredential } =
+      getConfig()
+    if (!firebaseAdminInitConfig && !firebaseAdminDefaultCredential) {
      throw new Error(
        'If not initializing the Firebase admin SDK elsewhere, you must provide "firebaseAdminInitConfig" to next-firebase-auth.'


Update to: "Missing Firebase admin credentials in next-firebase-auth. Set "firebaseAdminInitConfig", use default credentials, or initialize Firebase admin yourself."

vercel · 2021-12-10T16:18:24Z

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/gladly-team/nfa-example/4eptidGCFTBDdyV4oxdcwS5Zg2FP
✅ Preview: https://nfa-example-git-fork-reside-eng-application-0ba63b-gladly-team.vercel.app

src/__tests__/initFirebaseAdminSDK.test.js

kmjennison · 2021-12-10T16:34:00Z

@prescottprue I might have the time today to wrap up changes here and merge. If you'd like, feel free to give edit access to maintainers on this PR, though no hurry if you want to handle changes.

prescottprue · 2021-12-10T18:30:42Z

@kmjennison Great to hear, and thanks for the quick response. I updated based on comments - I'll be around today as well, so let me know if there is anything else to change before getting it in. After it is in I can make a similar PR to the v1.x branch as well.

Great work on the next-firebase-auth by the way! It has already saved our team a good bit of time and we are excited to continue to add to it

kmjennison · 2021-12-10T19:51:52Z

@prescottprue Appreciate it! Glad this library's been helpful.

kmjennison

Please fix the broken test.

codecov · 2021-12-10T19:59:35Z

Codecov Report

Merging #348 (ace08af) into main (fddfb88) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #348   +/-   ##
=======================================
  Coverage   99.59%   99.59%           
=======================================
  Files          25       25           
  Lines         495      497    +2     
  Branches      176      178    +2     
=======================================
+ Hits          493      495    +2     
  Misses          2        2

Impacted Files	Coverage Δ
src/initFirebaseAdminSDK.js	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fddfb88...ace08af. Read the comment docs.

kmjennison

Looks great, thanks! I'll aim to have this in a new release soon and will get it onto the 1.x branch too.

* 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>

prescottprue and others added 2 commits November 30, 2021 18:38

feat: fallback to applicationDefault credentials

474ed2f

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

chore: add test for firebaseAdminDefaultCredential

d7f1303

* chore: update README with firebaseAdminDefaultCredential Co-Authored-By: Jesse Anderson <jeryanders@gmail.com>

chore: cleanup wording in README comments

538c4ef

kmjennison requested changes Dec 10, 2021

View reviewed changes

vercel bot deployed to Preview December 10, 2021 16:18 View deployment

kmjennison reviewed Dec 10, 2021

View reviewed changes

src/__tests__/initFirebaseAdminSDK.test.js Outdated Show resolved Hide resolved

prescottprue requested a review from kmjennison December 10, 2021 18:24

fix: updates based on comments

70df466

vercel bot deployed to Preview December 10, 2021 19:48 View deployment

kmjennison requested changes Dec 10, 2021

View reviewed changes

chore: update error message in test

ace08af

vercel bot deployed to Preview December 10, 2021 19:56 View deployment

kmjennison approved these changes Dec 10, 2021

View reviewed changes

kmjennison merged commit d7ef716 into gladly-team:main Dec 10, 2021

prescottprue deleted the application-default-credentials branch December 10, 2021 21:03

kmjennison mentioned this pull request Dec 10, 2021

v1.x: Add support for Firebase application default credentials #359

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add support for application default credentials #348

feat: add support for application default credentials #348

prescottprue commented Dec 1, 2021 •

edited

Loading

vercel bot commented Dec 1, 2021

kmjennison left a comment

kmjennison Dec 10, 2021

prescottprue Dec 10, 2021

vercel bot commented Dec 10, 2021 •

edited

Loading

kmjennison commented Dec 10, 2021

prescottprue commented Dec 10, 2021

kmjennison commented Dec 10, 2021

kmjennison left a comment

codecov bot commented Dec 10, 2021

kmjennison left a comment

feat: add support for application default credentials #348

feat: add support for application default credentials #348

Conversation

prescottprue commented Dec 1, 2021 • edited Loading

Checks

Open Questions

Issues

vercel bot commented Dec 1, 2021

kmjennison left a comment

Choose a reason for hiding this comment

kmjennison Dec 10, 2021

Choose a reason for hiding this comment

prescottprue Dec 10, 2021

Choose a reason for hiding this comment

vercel bot commented Dec 10, 2021 • edited Loading

kmjennison commented Dec 10, 2021

prescottprue commented Dec 10, 2021

kmjennison commented Dec 10, 2021

kmjennison left a comment

Choose a reason for hiding this comment

codecov bot commented Dec 10, 2021

Codecov Report

kmjennison left a comment

Choose a reason for hiding this comment

prescottprue commented Dec 1, 2021 •

edited

Loading

vercel bot commented Dec 10, 2021 •

edited

Loading