Skip to content

[FOR PROGRESS, DO NOT MERGE] Rewrite FirebaseUI auth #962

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

Closed
wants to merge 164 commits into from
Closed

[FOR PROGRESS, DO NOT MERGE] Rewrite FirebaseUI auth #962

wants to merge 164 commits into from

Conversation

SUPERCILEX
Copy link
Collaborator

@SUPERCILEX SUPERCILEX commented Oct 10, 2017

#856, #948, #526, future support for #724 and #661, easier #123 implementation. In addition, every config change bug you can think of that people haven't reported.

cc: @samtstern + another human?

I was going to name this PR something more specific like "Fix all config change bugs," but then I saw the LoC changed... sorry @samtstern! 😊

Intro

Basically, if you try rotating your screen on pre-PR FirebaseUI, either the user is half signed-in but the UI doesn't show it, or we crash, or we just break randomly. To be frank, we suck! For example, rotate the screen after clicking the email sign up button, the user will actually be signed-in, but we're now broken. Click the button again, and we'll say the user already has an existing account. Duh, they just signed up! 😄

If you can't already tell, I'm super excited about this one which is why I'm sharing it early. So far, it just works! Plain and simple, it works. On the emulator, I've been mashing the Ctrl + Left/Right buttons to create config change after config change and we nail the sign in every single time. Every. Single. Time!

Anyway, the post-PR world will be a new era of ultimate stability. 😄

TODO:

The Problem

Fundamentally, most architectures assume that data doesn't talk back to UI. If we think of the auth process of signing-in and saving credentials etc. as "data," then we run into a problem: the data now needs to tell the UI to do stuff. This means both the user and the data are going to be fighting for control over the UI and need to somehow stay connected. In pre-PR FirebaseUI, this lead to nightmares where we just kinda gave up and ignored auth results if the user rotated their screen.

The Solution

I've used a mix of an eventbus + Rx model. Basically, all background auth stuff exists independently from the UI. However, since we need to talk to the UI, we use 4 data flow tracks: one for starting an Intent, another for starting a PendingIntent, yet another one for sending IdpResponses back to the UI to decide what to do, and the last one to send activity results. No more forwarding onActivityResult everywhere! 😄

This means setRetainInstance(true) fragments are dead, the memory leak hacks in FacebookProvider and whatnot are dead, the SaveSmartLock fragment is dead, etc.

I was hoping to be able to submit my PR in more bite friendly chunks, but everything was such a mess that it just wasn't possible.

Project structure overview

  • AuthViewModelBase and its descendents have access to FirebaseAuth (and only them)
    • SignInHandler manages all sign-in business login from failures to saving credentials to merging accounts etc.
    • SignInKickstarter manages the intro stuff like figuring out where to send users and the opening credential picker
  • Providers are now UI only and have their companion ProviderHandlers. For example GoogleSignInHandler manages all the logic for the GoogleApiClient and the Facebook and Twitter ones manage retrieving profile info, etc.

The rest is essentially the same, just forwarding everything to those classes above. I did rewrite the GoogleApiClient managers and error handling.

Potential limitations

The only one I can think of is perhaps callback hell. I'm not sure how to fix that and for now, I just want it to work. However, I really want it done right. So @samtstern and anyone else, if there's anything you think we can do better or another better solution you can think of, please tell me! I want auth to be a solved problem once and for all! 😄

@samtstern Since this is a massive PR, I'm expecting review to possibly take up to a month, so just take your time with it. It's got to be done right. I'll update it with a review of the classes and changes once I finish the TODOs listed above, but for now you can familiarize yourself with it. Enjoy! 😄

Signed-off-by: Alex Saveau <[email protected]>
Signed-off-by: Alex Saveau <[email protected]>
# Conflicts:
#	auth/src/main/java/com/firebase/ui/auth/provider/TwitterProvider.java
#	auth/src/main/java/com/firebase/ui/auth/ui/phone/PhoneVerificationActivity.java
#	auth/src/main/java/com/firebase/ui/auth/util/signincontainer/SignInDelegate.java
Signed-off-by: Alex Saveau <[email protected]>
Signed-off-by: Alex Saveau <[email protected]>
Signed-off-by: Alex Saveau <[email protected]>
Signed-off-by: Alex Saveau <[email protected]>
Signed-off-by: Alex Saveau <[email protected]>
Signed-off-by: Alex Saveau <[email protected]>
Signed-off-by: Alex Saveau <[email protected]>
Signed-off-by: Alex Saveau <[email protected]>
Signed-off-by: Alex Saveau <[email protected]>
Signed-off-by: Alex Saveau <[email protected]>
Signed-off-by: Alex Saveau <[email protected]>
Signed-off-by: Alex Saveau <[email protected]>
Signed-off-by: Alex Saveau <[email protected]>
Signed-off-by: Alex Saveau <[email protected]>
Signed-off-by: Alex Saveau <[email protected]>
Signed-off-by: Alex Saveau <[email protected]>
Signed-off-by: Alex Saveau <[email protected]>
Signed-off-by: Alex Saveau <[email protected]>
Signed-off-by: Alex Saveau <[email protected]>
Signed-off-by: Alex Saveau <[email protected]>
Signed-off-by: Alex Saveau <[email protected]>
Signed-off-by: Alex Saveau <[email protected]>
Signed-off-by: Alex Saveau <[email protected]>
Signed-off-by: Alex Saveau <[email protected]>
Signed-off-by: Alex Saveau <[email protected]>
# Conflicts:
#	auth/src/main/java/com/firebase/ui/auth/util/signincontainer/SignInDelegate.java
Signed-off-by: Alex Saveau <[email protected]>
# Conflicts:
#	auth/src/main/java/com/firebase/ui/auth/data/remote/SignInKickstarter.java
…hitecture

# Conflicts:
#	auth/src/main/java/com/firebase/ui/auth/AuthUI.java
#	auth/src/main/java/com/firebase/ui/auth/KickoffActivity.java
#	auth/src/main/java/com/firebase/ui/auth/data/model/FlowParameters.java
#	auth/src/main/java/com/firebase/ui/auth/data/remote/FacebookSignInHandler.java
#	auth/src/main/java/com/firebase/ui/auth/data/remote/GoogleSignInHandler.java
#	auth/src/main/java/com/firebase/ui/auth/data/remote/SignInKickstarter.java
#	auth/src/main/java/com/firebase/ui/auth/data/remote/TwitterSignInHandler.java
#	auth/src/main/java/com/firebase/ui/auth/ui/FragmentBase.java
#	auth/src/main/java/com/firebase/ui/auth/ui/HelperActivityBase.java
#	auth/src/main/java/com/firebase/ui/auth/ui/email/CheckEmailFragment.java
#	auth/src/main/java/com/firebase/ui/auth/ui/email/EmailActivity.java
#	auth/src/main/java/com/firebase/ui/auth/ui/email/RegisterEmailFragment.java
#	auth/src/main/java/com/firebase/ui/auth/ui/idp/AuthMethodPickerActivity.java
#	auth/src/main/java/com/firebase/ui/auth/ui/idp/SingleSignInActivity.java
#	auth/src/main/java/com/firebase/ui/auth/ui/idp/WelcomeBackIdpPrompt.java
#	auth/src/main/java/com/firebase/ui/auth/ui/phone/VerifyPhoneNumberFragment.java
#	auth/src/main/java/com/firebase/ui/auth/ui/provider/Provider.java
#	auth/src/main/java/com/firebase/ui/auth/util/AuthHelper.java
#	auth/src/main/java/com/firebase/ui/auth/util/signincontainer/SignInDelegate.java
#	auth/src/main/java/com/firebase/ui/auth/util/ui/FlowUtils.java
#	auth/src/main/java/com/firebase/ui/auth/viewmodel/AuthViewModelBase.java
#	auth/src/main/java/com/firebase/ui/auth/viewmodel/smartlock/SmartLockHandler.java
#	auth/src/test/java/com/firebase/ui/auth/testhelpers/AuthHelperShadow.java
Signed-off-by: Alex Saveau <[email protected]>
Signed-off-by: Alex Saveau <[email protected]>
# Conflicts:
#	auth/src/main/java/com/firebase/ui/auth/KickoffActivity.java
#	auth/src/main/java/com/firebase/ui/auth/data/model/FlowParameters.java
#	auth/src/main/java/com/firebase/ui/auth/ui/HelperActivityBase.java
#	auth/src/main/java/com/firebase/ui/auth/ui/email/CheckEmailFragment.java
#	auth/src/main/java/com/firebase/ui/auth/ui/email/EmailActivity.java
#	auth/src/main/java/com/firebase/ui/auth/ui/email/RecoverPasswordActivity.java
#	auth/src/main/java/com/firebase/ui/auth/ui/email/RegisterEmailFragment.java
#	auth/src/main/java/com/firebase/ui/auth/ui/email/WelcomeBackPasswordPrompt.java
#	auth/src/main/java/com/firebase/ui/auth/ui/idp/AuthMethodPickerActivity.java
#	auth/src/main/java/com/firebase/ui/auth/ui/idp/SingleSignInActivity.java
#	auth/src/main/java/com/firebase/ui/auth/ui/phone/PhoneActivity.java
#	auth/src/main/java/com/firebase/ui/auth/ui/phone/SubmitConfirmationCodeFragment.java
#	auth/src/main/java/com/firebase/ui/auth/ui/phone/VerifyPhoneNumberFragment.java
#	auth/src/test/java/com/firebase/ui/auth/ui/phone/PhoneActivityTest.java
Signed-off-by: Alex Saveau <[email protected]>
Signed-off-by: Alex Saveau <[email protected]>
Signed-off-by: Alex Saveau <[email protected]>
Signed-off-by: Alex Saveau <[email protected]>
@SUPERCILEX SUPERCILEX changed the title Rewrite FirebaseUI auth [FOR PROGRESS, DO NOT MERGE] Rewrite FirebaseUI auth Mar 25, 2018
# Conflicts:
#	auth/src/main/java/com/firebase/ui/auth/ui/FragmentBase.java
#	auth/src/main/java/com/firebase/ui/auth/ui/HelperActivityBase.java
#	auth/src/main/java/com/firebase/ui/auth/ui/email/CheckEmailFragment.java
#	auth/src/main/java/com/firebase/ui/auth/ui/email/RegisterEmailFragment.java
#	auth/src/main/java/com/firebase/ui/auth/ui/email/WelcomeBackPasswordPrompt.java
#	auth/src/main/java/com/firebase/ui/auth/ui/phone/VerifyPhoneNumberFragment.java
#	auth/src/main/java/com/firebase/ui/auth/viewmodel/idp/EmailProviderResponseHandler.java
#	auth/src/test/java/com/firebase/ui/auth/ui/email/WelcomeBackPasswordPromptTest.java
@SUPERCILEX SUPERCILEX changed the base branch from version-3.3.0-dev to master March 28, 2018 21:48
…hitecture

# Conflicts:
#	auth/src/main/java/com/firebase/ui/auth/ui/HelperActivityBase.java
#	auth/src/main/java/com/firebase/ui/auth/viewmodel/smartlock/SmartLockHandler.java
Signed-off-by: Alex Saveau <[email protected]>
Signed-off-by: Alex Saveau <[email protected]>
@SUPERCILEX SUPERCILEX changed the base branch from master to version-3.3.1-dev April 12, 2018 23:26
@SUPERCILEX
Copy link
Collaborator Author

All the changes left here have been moved to #1253.

@SUPERCILEX SUPERCILEX closed this Apr 12, 2018
@SUPERCILEX SUPERCILEX deleted the re-architecture branch April 12, 2018 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants