-
Notifications
You must be signed in to change notification settings - Fork 32
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
Link multiple auth providers #3
Comments
Hello @deg, I have a proof of concept implemenation in my fork branch. You can see a demo at https://proto-app.bzdyl.net/ (please check Outstanding issues below before testing). The flow is quite simple:
Linking accounts works without issues in popup mode as the app stores pending credential in app-db. For redirect flow, the application would have to store it e.g. in session storage, as suggested by Firebase docs:
This hasn't been implemented in my demo app. What do you think of this approach? Outstanding issues:
|
It will probably be a few days before I can look at this closely.
I’ll try to review by early next week.
The Google issue you found seems strange. I don’t know enough about it to comment intelligently now.
Did you tackle just account merging half of this issue? Or also the unified login flow?
The merging part is definitely useful on its own. The unified login part would be wonderful to have, but it is harder because it requires the library to handle GUI issues that normally belong to the app.
I played with Stormpath a bit last year, and they handled this by supplying default html templates that the app could override.
|
I just tested by logging in via Twitter, logging out, then via Google, log out, and again via twitter. The app got stuck with a spin wheel and the word "loading..." The console showed
I think your approach is exactly right, but this needs a bit more work before it is ready. |
My fix supports only handling the linking - there is no GUI. I am not sure it is a good idea to force users of the library to depend on library's author choice of the GUI design and GUI components. The error you have seen is probably because of a bug in my updated app - I have made some changes recently and the issue you've seen is caused in my application code, not in re-frame-firebase. I have already reverted it to the previous version. |
Ok. When you are ready, please submit a PR. Even if there are still problems, I think it is worth having a partial solution. Btw, re your point (1) above: It would be good to have a solution to this in place. externs breakages in advanced builds are definitely annoying to users! It's probably enough to just add |
Thanks! I've committed your PR. I'm leaving this issue open because some points are still open. |
Thank you! |
We support authorization with Google, Facebook, Twitter, or Github, but don't support some key functions that will make this feature more useful for real apps. These include:
Account merging. A user (as identified by his email address) may login via multiple auth providers (e.g. Google and Facebook). Our current behavior when this happens is to report an error "An account already exists with the same email address". Instead, we need to implement account linking, as described here: https://firebase.google.com/docs/auth/web/account-linking.
Unified login flow. (This is trickier, because it involves introducing GUI decisions into this library and/or figuring out the right hook points). Currently, applications using this library need to handle login via each auth provider. We should offer a single call that, e.g., pops up a set of buttons to let the user choose a desired auth provider.
The text was updated successfully, but these errors were encountered: