-
Notifications
You must be signed in to change notification settings - Fork 975
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support email link authentication with Firebase #3347
Conversation
…o v9.0.1 (mudlar sdk, now out of beta and officially released, update firebase types and add FirebaseUser to SupportedUserMetadata type in redwood
…oginWithEmailLink function
Nice work! I use firebase myself and do some custom stuff with magic login via custom tokens. I am curious what is the user flow like for email authentication? and how is it different generating a custom token and sending via email? |
@viperfx I think it's pretty similar and it would be very similar execution path for supporting custom token in the auth client, say by checking if providerId = 'customToken' and passing the token via an optional customToken property on the options object for the login/signup methods. To synchronize login state across browser tabs, my app has a component that subscribes to onAuthStateChanged events and trigger's the AuthProvider's reauthenticate() whenever the firebase auth state changes. like so: import React from 'react'
import { AuthContext } from '@redwoodjs/auth/dist/AuthProvider'
import { getAuth } from '@firebase/auth'
export class FirebaseAuthSubscriber extends React.Component {
private unsubscribe?: () => void
constructor(props) {
super(props)
this.unsubscribe = null
}
async componentDidMount() {
let initial = true
this.unsubscribe = getAuth().onAuthStateChanged(() => {
if (!initial) {
// initial state change on page load, already handled in restoreAuthState()
this.context.reauthenticate()
}
initial = false
})
}
componentWillUnmount() {
if (this.unsubscribe) {
this.unsubscribe()
this.unsubscribe = null
}
}
render() {
return this.props.children
}
}
FirebaseAuthSubscriber.contextType = AuthContext |
Thanks for the detailed reply! Looks interesting. We built our own magic sign in method on top of firebase, but this might be worth exploring. One question I had is - does firebase handle the email sending? I wonder if it's possible to use your own backend to create the link and send email using our own provider. |
@viperfx It's your choice as the whether firebase sends the email for you or you send it. If you app calls sendSignInLinkToEmail() then firebase sends an email for you. If you call generateSignInWithEmailLink() you can put the generated link in your own custom email and send it through your own mail service or SMTP server. |
…: ' 1:1 error There should be no empty line within import group import/order'
…dwood into firebase-email-link-auth
Note: this includes an upgrade to v9 and will fix an existing bug. |
Thanks @doesnotexist for this! Can you confirm that the token is still refreshed once it expires? |
@dac09 > Thanks @doesnotexist for this! Can you confirm that the token is still refreshed once it expires? It doesn't seem that the redwood AuthProvider/AuthClient needs to be aware of refreshing beyond what is done in getToken() since firebase's sdk handles refresh when getIdToken() is called it will refresh the token if it's expiring or close to expiring. |
email as string, | ||
emailLink as string | ||
) | ||
return undefined |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a hunch that "return undefined" is probably not idiomatic typescript/javascript. I only put this here to get rid of warning in the IDE about some execution paths having no return.
@@ -50,82 +60,88 @@ export const firebase = (client: Firebase): AuthClient => { | |||
} | |||
return provider | |||
} | |||
// Firebase auth functions return a goog.Promise which as of 2021-05-12 does |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I can tell this is no longer the case, but I'm not sure, what would be a good test to confirm that removing repackagePromise has not changed this w.r.t. try {await} catch blocks / exception handling?
packages/cli/src/commands/setup/auth/__tests__/__snapshots__/addAuthConfigToApp.test.js.snap
Show resolved
Hide resolved
packages/cli/src/commands/setup/auth/__tests__/__snapshots__/addAuthConfigToApp.test.js.snap
Show resolved
Hide resolved
|
||
// @TODO: Firebase doesn't export a list of providerIds they use | ||
// But I found them here: https://github.com/firebase/firebase-js-sdk/blob/a5768b0aa7d7ce732279931aa436e988c9f36487/packages/rules-unit-testing/src/api/index.ts | ||
// NOTE: 2021-09-15 firebase now exports a const/enum ProviderId which could perhabps be used in place of this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I attempted it but it opened up a bunch of typescript hurdles that I wasn't prepared to address.
Actually, this is not going to work because it breaks "yarn rw build" for projects that don't use the firebase auth client and do not have firebase as a dependency in their web/package.json.
The other way is a bit uglier and introduces that breaking change since the authclient is initialized differently but it doesn't break projects that aren't using firebase auth or require them to introduce a dependency they do not need. So I'm reverting this change back to the other way. |
…forwarding method directly
…s can only be used in TypeScript files'
firebaseAuth, | ||
firebaseApp, | ||
}: FirebaseClient): AuthClient => { | ||
const auth = firebaseAuth.getAuth(firebaseApp) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @doesnotexist - could you help me understand something?
Why do we need to do this, and whats the difference between the object returned from getAuth, and the imported import * as firebaseAuth from '@firebase/auth'
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure @dac09
The reason is to avoid firebase dependency for normal redwood projects that are not using the firebase auth provider. Instead, users who are using firebase auth, are required to pass the module to this factory function so it can call the associated functions. One export is the getAuth() method which returns the auth object (or creates and initializes an auth object) for the firebase app or the default firebase app if no app is given. In VS Code, it might help to look at the autocomplete suggests after typing firebaseAuth vs the auth object returned from getAuth().
The reason this looks so much different than before has to do with the firebase v8 to v9 transition which reorganized and split the firebase sdk into submodules with the goal of supporting tree-shaking. Previously there was one monolithic firebase import and firebase app instances had a method attached that produced the associated firebase auth instance v8: firebaseApp.auth()
v9: equivalent is getAuth(app?) which is now a module export in the @firebase/auth module
One thing I think is confusing about what I've written is, though similarly named there is a distinction between the two properties in the FirebaseClient type, firebaseApp is an instance (usually created/initialized in App.jsx by calling @firebase/app's initializeApp() method) whereas firebaseAuth is the module exports object from @firebase/auth and not a firebase auth instance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I should mention that it isn't sufficient to just pass the auth instance into this factory function, because there are multiple other functions needed from the module which no longer live as methods on an auth instance but are just exports from the module. So even if we passed an auth instance we'd still need to pass in the module.exports from @firebase/auth in order to call those other functions without putting an import ... @firebase/auth in this file and causing non firebase auth projects to have an unneeded firebase dependency in their web/packages.json
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried something else see commit here: a47974e
And we pass it to the auth provider like so:
const firebaseApp = ((config) => {
const apps = getApps()
if (!apps.length) {
initializeApp(config)
}
return getApp()
})(firebaseConfig)
const App = () => (
//...
<RedwoodProvider titleTemplate="%PageTitle | %AppTitle">
<AuthProvider client={firebaseApp} type="firebase">
What do you think? This seems to work and simplifies the setup code in App.tsx
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also I worked out the difference - getAuth gives you an instance of the auth client. Where as the imports are just utility functions, so maybe there's no need to pass it through from the app level right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also prefer a47974e and I tried similar with 70c59e6 but had to revert it back to passing the module exports because when I did create-redwood-app and then 'yarn rw dev' the default app had a compile error.
Described here: #3347 (comment)
Have you tried create-redwood-app with your changes? I think you might run into the same issue
Also, early on I was passing an object with only those exported utility functions before, which might be more ideal from tree-shaking but required a new typescript type to describe the subset of functions required from @firebase/auth's modules.export ... But perhaps thats ok, I was a less familiar with typescript back when I was doing that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks so much for all the effort into this @doesnotexist
Approving, but definitely one for us to revisit when we think about changing how the auth packages are managed, so that we don't have to "inject" firebaseAuth imports!
…dwood into firebase-email-link-auth * 'firebase-email-link-auth' of github.com:doesnotexist/redwood: Bump tmpl from 1.0.4 to 1.0.5 (#3406)
Phenomenal effort on this one @doesnotexist I'll make sure we include your "Upgrade Guide" (from OP) in the Release Notes for v0.37. And I'll also be updating the All Contributors and Readme this week! |
Upgrade Guide
For existing redwood projects using firebase auth, make the following changes to your project:
NOTE If you use v8 firebase sdk elsewhere in your app's code then in those places you would import compat versions from v9 firebase sdk to maintain code compatibility, see docs:
https://firebase.google.com/docs/web/modular-upgrade
firebase auth: support email link authentication
Closes #3346
Closes #3344
Closes #3323
Closes #2260
It is the app's responsibility to send the email link to the user either using firebase's sendSignInLinkToEmail() where firebase sends the email for you or by using generateSignInWithEmailLink() to send it in a more customized email with some other email sending service or SMTP server.
The email link sent the user will ultimately redirect to your app and firebase expects you to handle the users login when they arrive at this URL which you specify with
acitonCodeSettings argument to the method generating/sending the email link.
The handler at this url can then call the AuthProvider's logIn() method with
Here's an example page component for handling email link authentication