firebase / FirebaseUI-Android

Optimized UI components for Firebase
https://firebaseopensource.com/projects/firebase/firebaseui-android/
Apache License 2.0
4.63k stars 1.83k forks source link

Anonymous user and account conversion to permanent account #123

Closed danielkummer closed 4 years ago

danielkummer commented 8 years ago

Hi there

Is there a way in which AuthUI supports the conversion of a anonymous account to a permanent one (one of the supplied auth providers) As stated in the documentation and as I understand it, one must use the linkWithCredential method instead of the "normal" signInWith flow.

Can this be done with the current AuthUI version somehow?

brandall76 commented 6 years ago

@SUPERCILEX thanks for your work on the merging accounts, just what I needed.

Forgive my ignorance, but how do I confirm your alterations on this fork?

implementation "com.github.SUPERCILEX.FirebaseUI-Android:firebase-ui-auth:164165ed92"

I'm trying to work out why my application isn't obfuscating and I want to make sure that this commit is applied to it?

Thanks in advance

SUPERCILEX commented 6 years ago

Sorry for the delay, I've been busy with #962. Here is the latest and greatest with FirebaseUI v3.1.1 bundled in:

implementation "com.github.SUPERCILEX.FirebaseUI-Android:firebase-ui-auth:52df7658de"

The current implementation works fine AFAIK, but I'm going to stop supporting it until #962 goes through at which point I'll have to rewrite this PR anyway.

brandall76 commented 6 years ago

Thanks @SUPERCILEX

I hope they're paying you well!

SUPERCILEX commented 6 years ago

@brandall76 Lol, nah... I reap the benefits in what I learn working on FirebaseUI and other cool perks like alpha access to Firebase SDKs. Oh, and it eventually improves my apps too. πŸ˜†

Anyway, glad this PR is helpful to you! One of these days it'll be merged. πŸ‘

brandall76 commented 6 years ago

@SUPERCILEX well I hope they're going to send you a huge collection of test devices as a sign of appreciation! - You seem like a pretty valuable asset to me, looking around the PRs on here...

I could really do with your expertise on a project I've just pushed!

Anyway, thanks again πŸ‘

TylerMcCraw commented 6 years ago

@SUPERCILEX First off, thank you for your dedication to this project. Your willingness to provide intermediate builds while we're all waiting on that pesky iOS work to be finished is outstanding.

I wanted to mention something real quick. From my testing earlier today, if you have an anonymous type FirebaseUser (via firebaseAuth.signInAnonymously()), and you later call linkWithCredential(), then the two user accounts should be automatically merged for you. And no manual data merging should be necessary. From what I remember, it also removed the anonymous account for me after it was merged. At least, this is what I experienced earlier when merging an anonymous user into a Google user account.

So my question is, why did you go the route of providing an abstract Service that requires devs to manually merge their data if the linkWithCredential() function should properly merge the users (and data) for us?

This is what worked for me (no manual data merging):

private fun linkAnonWithGoogleAccount(account: GoogleSignInAccount) {
        val credential = GoogleAuthProvider.getCredential(account.idToken, null)
        firebaseAuth.currentUser?.linkWithCredential(credential)?.addOnSuccessListener {
            Timber.d("User converted from anonymous to real user")
            // Do other UI stuff here to show user that they were signed in
        }?.addOnFailureListener {
            onGoogleSignInFailed()
        }
}

sorry for the Kotlin ^^

SUPERCILEX commented 6 years ago

@TylerMcCraw Ah, I see you've gotten into the meat of account linking. πŸ˜† Since this is a complicated topic, I actually wrote a full section on it in the PR's README.

PS: No problem about the Kotlin, I've got one of my projects to just under 90% Kotlin! πŸ˜„

TylerMcCraw commented 6 years ago

So, I read your updated README and drilled through the code. It seems that the Merge Service is just for when you know the anonymous account has data and an existing non-anon account has data.

I was trying to tackle the issue of when a new anonymous account wants to authenticate with a provider for the first time. I can happily say that this is working for me when using this build on your fork: implementation "com.github.SUPERCILEX.FirebaseUI-Android:firebase-ui-auth:f8634227b9" πŸ‘

It's unfortunate that the main FirebaseUI library doesn't support at least this flow already.

SUPERCILEX commented 6 years ago

Yep, the merging process is only for that rare case where the user already has an account. Glad it's working for you! πŸ‘

TylerMcCraw commented 6 years ago

Sorry, one more thing.

There may be an issue or maybe I'm not understanding something. In the scenario of:

  1. Sign in user anonymously
  2. Link with provider authenticated account (this combines anon and provider accounts and anon account is deleted)
  3. Sign out
  4. Sign in user anonymously (w/ diff anon account this time)
  5. Link with same provider as before

The second anonymous account is not deleted as it was in step 2. Is this intended? I would imagine that the accounts are combined and the anonymous account is deleted again. Maybe this is because the provider authenticated account already had an anonymous account linked to it? Are developers required to unlink accounts in this scenario?

Thanks for the help.

SUPERCILEX commented 6 years ago

Yes, this is expected behavior. When linkWithCredential is used, it really does link the two accountsβ€”that is, the credential is associated with a Firebase controlled user. Thus, the anonymous account isn't deleted at all. Instead the new provider's credential is added to the Firebase user's stable ID and "linked." However, when a collision occurs, Firebase can't link the accounts because there are now two stable IDs which can't be combined. Hence the need for a data transfer service: we have to drop one account and migrate all data to the account we're going to use. For safety's sake, we don't delete the old account when this happens though we could.

TLDR: you have to shift your mindset from a Firebase user being a set of providers to it being a stable ID that can't be changed or merged which is associated with various credentials. When you have two different stable IDs (or users), they cannot be merged but credentials can still be added to them.

I hope this helps!

brandall76 commented 6 years ago

@SUPERCILEX @TylerMcCraw just to throw my two cents in - I think there are a number of use cases here - some common, some edge.

Given that FirebaseUI attempts to provide a 'plug and play' solution for developers of all experience, I believe there needs to be multiple APIs exposed for both simple and complex merging.

I'm fairly sure the most common use case is where a user is 'upgraded' from anonymous to authenticated. There is no existing authenticated account and therefore the anonymous data is simply transferred and the anonymous account subsequently deleted.

For the above scenario, I think a one-liner

setAccountMergingEnabledAndThenDeleteAnonymous(true)

should be exposed and handled for developers. Obviously with a better naming convention!

Things become a little more complex when there is an existing authenticated account. A user with multiple devices is an obvious scenario. Device A originally creates the authenticated account. Device B & C (etc) need a merging strategy.

I think (although I'm not certain) that many developers may simply want device B & C,D,E... to reflect the existing authenticated data and merging specific data would not be an issue

setAccountMergingEnabledPersistAuthenticatedDataIfExistsAndThenDeleteAnonymous(true)

Again, naming convention caveat!

The remaining scenario is the requirement for a merging strategy. Consider the following simple examples:

I wonder if a generic 'merging strategy' should be considered and exposed, so developers can reuse common rules on defined fields, such as overwrite, append, add, subtract etc. If their merging strategy is more complex, then naturally they will have to manipulate their data accordingly.

In summary, I think account merging is essential and an extremely common requirement. My input would be, that many developers will start to sweat when they see that it's not a one liner for their trivial requirements and they need to implement a Service to achieve this.

I understand that a Service may become an integral part of FirebaseUI if #962 goes through, but I'm pretty sure that if it does, it will be embedded in the library rather than exposed in a way where a user will need to extend it.

Two cents complete! The main thing to take from the above @SUPERCILEX is that the work you have done here is amazing and I'm extremely grateful for it.

SUPERCILEX commented 6 years ago

Well dang, those are some long two cents! Maybe more like three or four? πŸ˜‚

Ok, anyway... First off, thanks for all your input!

For the simple, snap your fingers case of linking accounts, I'm hoping to get that into FirebaseUI by the end of the year with a big refactor we're doing (no promises, of course πŸ˜„). As for the other cases, here's my philosophy: devs should be forced to think about this to ensure a good user experience. You suggested a case where the user's data isn't transfered, but this could lead to devs permanently losing anonymous account data. Unless you store the uid somewhere permanent, it will be impossible to identify an anonymous account once the user had been signed out from it. Thus, my reasoning was that for good UX devs need to think about this. (Though you could implement a service that just returns Tasks.forResult(null) and call it a dayβ€”highly unrecommended).

As for why we need to use a service, FirebaseUI has no knowledge of how your database is organised or even what database you're using. The service provides a guided transfer that enables custom code execution during the sign-in. Thus, devs just have to implement a load and transfer method which isn't too complicated. The other reason you can't do it in onActivityResult is because it's too late by then in terms of security rules. If you lock access to data for only the signed in user, security rules will correctly assume you're a different user, hence the need to inject a load and transfer into the sign in process.

I hope this clears things up! πŸ˜„

krisu7 commented 6 years ago

@SUPERCILEX Hi, is there a new version of your anonymous auth fork for FUI 3.1.1?

SUPERCILEX commented 6 years ago

@krisu7 Yep, I've tested ab184fde51 and updated this comment with the latest install instructions.

krisu7 commented 6 years ago

@SUPERCILEX thank you

txedo commented 6 years ago

Hi @SUPERCILEX, thank you for your effort!

FUI 3.1.2 has been just released and it fixes a bug for transitive dependencies and also upgrades to support library 27.0.2 [1]. Do you have any plans to update your fork or will your development get merged to mainstream anytime soon? Thank you in advance.

[1] https://github.com/firebase/FirebaseUI-Android/releases

SUPERCILEX commented 6 years ago

@txedo Dunzos: fa824262ab

kkdevenda commented 6 years ago

Hey @SUPERCILEX

First of all, thank you very much for all your effortsπŸ™Œ.

So, I have been in one of those rare cases where the user already has an account. I am creating the intent with setIsAccountLinkingEnabled(true, MyManualMergeService.class) to start the sign in flow, the problem is, I get an error saying This credential is already associated with a different user account when the user clicks verify phone number. This should happen if one tries to link an account using firebase's linkWithCredential because the credentials are already linked to another user account as mentioned in the docs, but, as per understanding, after reading your README and going through this thread, you have handled that case in your fork.

I am not able to figure out a reason or solution. Please help.

PS: I am having multiple apps under the same firebase project and they share the database and authentication from the project. Now, in my case, the previous user account is from app A that requires registration to use it but there is this another app B that does not require user login as long as user does not want to perform some specific activities, at which point, user has to sign in. I implemented anonymous login for this app B. I want the user to be able to login into app B with the credentials associated with app A and vice versa. Surprisingly, the vice versa case, that is, first logging in app B and then using same credentials in app A is working fine.

SUPERCILEX commented 6 years ago

@kkdevenda Apologies for the delay, and thanks for the detailed investigation! I'll investigate this issue soon. πŸ˜‰

curiousily commented 6 years ago

Hey @SUPERCILEX

Our team is planning to use your fork (thus use setIsAccountLinkingEnabled()). We want to simplify things a bit. When linking an anonymous user to a registered one we would like to:

We are using the Firestore database. Do you foresee any problems that we might encounter during the implementation?

Still can't believe how core developers do not prioritize implementing this feature since it is considered best practice. and recommended by Google.

Thank you for the huge effort!

SUPERCILEX commented 6 years ago

@kkdevenda my B, you're right. This is fixed in 6a56930af6! πŸŽ‰

@curiousily What you described is exactly what occurs πŸ‘, you'll just have to use the manual merge service as described here.

PS: no promises, but the Firebase team is aware of this oversight and is working to fix it. πŸ˜‰

j2emanue commented 6 years ago

Hi Alex @SUPERCILEX , can you help. i have a question on how i can delete the anonymous account after logging in with a real google account for example. The situation is user was logged in anonymously and then logged in WITH AN EXISTING ACCOUNT. the accounts cannot be linked since its an existing account. so merging must be done manually. and given your example on the mergeService i dont see how i can delete the anonymous account from the firebase console ? i would need the credential right ? can you give an example how this would be done in your mergeService ? for me i dont have any data to merge, i'd just like to remove the anonymous user from firebase as they are now logged in with a proper account.

SUPERCILEX commented 6 years ago

@j2emanue You can't and this is acceptable behavior. I agree that it triggers our internal developer's need for cleanliness, but there's no nice way to ensure the account has been fully migrated and also delete the account. (By the time the migration is done, it's too late to delete the account.) Sorry for not being able to provide a better answer! 😊

GuanacoDevs commented 6 years ago

What I'm doing at the moment to sort this out is to use firebase functions to delete the anonymous user, and copy/delete(Anonymous) its current data in case of a new sign in or just delete the anonymous user data in case of concurrent signin. I have a property "anonymous_user_uid" in my user data which triggers a function, after the "Write" function has finished it deletes the "anonymous_user_uid" related to that uid.

SUPERCILEX commented 6 years ago

Thanks for sharing, that's a cool idea! Not something we can do in the library, but good to know as individual developers.

j2emanue commented 6 years ago

@GuanacoDevs thank you for the feedback. can you show code how you are deleting an account based on anonymous_user_uid only ? i see to delete an account i call currentUser.delete(). i dont see how your getting the anonymous user by uid ? your not even logged in anymore as the anonymous user so how can the account be deleted ? although with the admin sdk i see a way: FirebaseAuth.getInstance().deleteUserAsync(uid).get();

GuanacoDevs commented 6 years ago

@j2emanue Hey man, I meant Google Cloud Functions, not within your App, using GCF you can delete any user even if is not signed In. Delete User. Now what I do with those functions is to set different triggers, that will execute them in case: New User Signs In or Returning User already has data. My app always signs them Anonymously. 1- When they choose to sign in I backup their Anonymous data under a node which is Readable/Writable by everyone as long as they are Authenticated, I set a flag under that node that triggers a copyAnonymousUser GCF and I save in sharedPreferences() the anonymousUid before launching FirebaseUIAuth. 2- If for some reason they don't sign in I delete their backup in onActivityResult() 3- If the signs In is completed I check if is a returning user, if so I just delete the Anonymous User data and account(I should update in case that used the app while anonymous, but I'm waiting for the silent sign in @SUPERCILEX ), I use the anonymousUid saved before to trigger a function so that it will know what data and user to delete. 4- If is a new user I use the backed up data to populate its "new" account. After that I clean the "shared" node, for this I trigger another function to execute always from GCF They just released v1.0 Note that I'm not linking accounts, just copy/paste/delete Wish I knew how to do this before, now my database is cleaner. I really suck at Java Script because functions is the only reason I have to use it, so virtually I'm not really sure what I'm doing.

Regards GuanacoDevs

SUPERCILEX commented 6 years ago

@GuanacoDevs Silent sign-in is ready on my side, just waiting on @samtstern's approval. πŸ™ƒ

curiousily commented 6 years ago

Hey @SUPERCILEX,

What (stable?) commit hash from your branch would you recommend that targets Play Services 15?

Thanks for your response. Our team already implemented your suggestions!

Cheers!

SUPERCILEX commented 6 years ago

@curiousily Sweet! I'd go with 38e2ece9bbβ€”it includes 3.4-dev, but that's only db and opt-in auth features (e.g. #1212) + bug fixes so nothing to worry about.

unxavi commented 6 years ago

Hey @SUPERCILEX ,

Did this made it to the v4.0.0? If not what would be the last stable commit hash from your branch? Is it the same from Apr 25 38e2ece9bb ?

Thanks a lot!

unxavi commented 6 years ago

@GuanacoDevs and where do you merge the data from anonymous user with the new signed in user? We are talking about linking an anonymous account with a user account on the sign in process, no?

samtstern commented 6 years ago

@unxavi @GuanacoDevs I think there is some confusion. The new "silent sign-in" feature is not related to this issue at all. Anonymous upgrade has not been shipped.

GuanacoDevs commented 6 years ago

@unxavi @samtstern Apologies, I read my previous post here and not the title of the issue, got confused.

curiousily commented 6 years ago

Hey @SUPERCILEX,

Any new recommended hash that includes the changes in 4.0? Would you recommend a way to select hashes by ourselves, so I don't bother you every now and then?

Not even #1185 made it into 4.0...

Sorry to bother you again. Thanks for the hard work you're putting in!

SUPERCILEX commented 6 years ago

@unxavi @curiousily no worries about asking for new hashes. 😊 (Though if you go to the Commits tab here, you'll find the latest and greatest.) Anyway, c280098444 includes 4.0 plus the account linking stuff. Cheers! πŸ₯‚

ghost commented 6 years ago

how can I add Authenticate with Firebase Anonymously into FirebaseUI

like this

enter image description here capture

Although the user can log in anonymously, I need to give the user a different id to identify the user

SUPERCILEX commented 6 years ago

@mike1128 The point is that it's anonymous. πŸ˜‰ So basically, you just call FirebaseAuth#signInAnonymously() and you're good. There's no UI.

ghost commented 6 years ago

I want them in the same page.

So user can choose sign in with facebook or email or sign in anonymously

from one interface.

SUPERCILEX commented 6 years ago

@mike1128 This isn't something the user interacts with though. Typically, you'd call signInAnonymously() on app start if the user isn't signed in.

ghost commented 6 years ago

Thank you for your replying.

But it doesn't solve the problem.

If I call signInAnonymously(), I should give user a button.

so it's two interface or page, not one.

what i want it like this

startActivityForResult( AuthUI.getInstance() .createSignInIntentBuilder() .setAvailableProviders(Arrays.asList( new AuthUI.IdpConfig.EmailBuilder().build(), new AuthUI.IdpConfig.FacebookBuilder().build(), new AuthUI.IdpConfig.AnonymousBuilder().build())) .build(), RC_SIGN_IN);

and then Handling the sign-in response

like if (requestCode == RC_SIGN_IN) { if (resultCode == RESULT_OK) { //write uid to firestore

SUPERCILEX commented 6 years ago

I see what you're saying, but the whole point of signInAnonymously() is that there's no user interaction. So that means no button. You should call the method on app start without the user needing to do anything.

ghost commented 6 years ago

what if user want to sign in on app start? so I need give the user a choice to sign in with email or sign in with facebook, or sign in anonymous

SUPERCILEX commented 6 years ago

Oh, I think I see where you're confused. Anonymous sign-in isn't persisted anywhere. That means if the user re-installs your app one way or another they'll lose all their data. The point of my fork is that you sign in anonymously on app start so you can use the db and all that, then when the user is ready to sign-in after going through tutorials or something, you present them with a reliable sign-in method.

samtstern commented 6 years ago

Hi all,

Some basic support for this feature has been released in version 4.1.0. More to come!

SUPERCILEX commented 6 years ago

I'm going to attempt to clarify and organize what's going on so this post can be used as a starting point for any newcomers.

Current state of affairs

Public releases of FUI include support for non-conflicting upgrades. What does that mean? If a user only has an anonymous account and then signs up for a real account, you're good to go. However, anytime the user is signed in anonymously but already has a real account, you'll run into a merge conflict which can't be resolved nicely. BTW, when I say "real" account, I mean something with a stable ID like an email or phone.

What does the fork do?

Simply put, it lets you resolve merge conflicts by transferring a user's data to their real account with security rules in mind.

How do I get it?

Setup JitPack with this repo: com.github.SUPERCILEX.FirebaseUI-Android:firebase-ui-auth:60f451cfea

Documentation can be found here: https://github.com/SUPERCILEX/FirebaseUI-Android/blob/anonymous-auth/auth/README.md#account-linking

hummatli commented 4 years ago

Hi, folk. I found a method called enableAnonymousUsersAutoUpgrade(). It converts the anonymous user to a permanent one.

I don't know when it was added but I think it answers the question.

AuthUI.getInstance() .createSignInIntentBuilder()
.enableAnonymousUsersAutoUpgrade() .build()

percula commented 4 years ago

@SUPERCILEX , can you please comment on this enableAnonymousUsersAutoUpgrade()? Is it comparable to your fork? Thanks!

Edit: Just looked at the documentation. They appear to be similar. The key difference is "When linking is unsuccessful due to user collision, an error with code ErrorCodes.ANONYMOUS_UPGRADE_MERGE_CONFLICT will be returned to onActivityResult()". So instead of passing a merge conflict class, you handle it in onActivityResult()

danijorda1 commented 2 years ago

Hi @SUPERCILEX could you please create a new build with firebase-ui-auth:8.0.0 ? Phone Auth is not working in higher versions of Android with firebase-ui-auth:4.2.0

Thanks a lot!