-
Notifications
You must be signed in to change notification settings - Fork 6
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
signupNewUser implementation #33
base: master
Are you sure you want to change the base?
Conversation
Nice work! Can we get a test for it? |
I don't see any similar test, what do you have in mind? |
Is there one in the https://github.com/GitLiveApp/firebase-kotlin-sdk maybe you can lift it from there |
simple tests added for createUserWithEmailAndPassword, signInWithEmailAndPassword, and signInAnonymously.
I've added the email property in order to properly run the sign in test. Please, have a look @nbransby . |
Thanks for the reminder I will try and have a look this weekend |
Just a friendly reminder this PR is waiting for approval :-), and issue 17 is directly resolved by this. |
Tests are running now but failing, also we should add tests to verify the email property is never an empty string - of course the tests will fail with the current implementation but then you'll need to get them passing because if we add the email property it will need to work 100% |
this is the behavior we would need to verify: getEmailReturns the main email address of the user, as stored in the Firebase project's user database. Unlike the email property from instances of UserInfo corresponding to authentication providers (like Github), which is not modifiable, this email address can be updated at any time by calling updateEmail. This field will be automatically populated on account creation if the AuthCredential used on signInWithCredential contained such information, or if the account was created with createUserWithEmailAndPassword. However, this is not true if the setting "Multiple Accounts per Email" is enabled in the Firebase Console - in that case this will be null unless the account was created with createUserWithEmailAndPassword or updateEmail has been called. If the user also has a password, this email address can be used to sign in into the account using signInWithEmailAndPassword. This email address is displayed in the Users section of the Firebase console. |
I'll allocate the time and implement it soon |
…se-java-sdk into signup_new_user
email is nullable again, check for empty string cancelation of token refresher after signout
So I implemented a couple more things to make the email property a bit more complete. Feel free to give feedback on that, then. Other than that, ActionCodeSettings and signInWithCredential can be implemented in the future, as there is other unrelated stuff to be done. That doesn't mean I can't contribute to its implementation in the future, though, just don't think this PR needs to implement the whole deal ✌️. |
Thanks @JacobCube. Looks like there is a conflict to resolve. I think we need a test for signing in with a custom token and then testing the email matches because looking at the code in this scenario it would appear they would get null even if there was an email set on the user. Would be good to run the same test on android first and verify the behavior as its not clear from the docs, if it does return the correct email on android I believe that must mean they make a second request to the REST api to populate the users email on sign in with custom token. |
I may look into it soon, I recently found out that implementing pictureURL might be helpful for our KMP project as well, so I might as well implement that here if no one objects:-). I saw an REST API for the custom token, I just didn't want to bother with all the various tokens, but I can implement what will be readily available. Thank you for the feedback @nbransby. |
Hey, unfortunately, I'm unable to write a test for signInWithCustomToken, as creating a custom token requires Admin SDK on BE. I could make a Firebase Function for that on this project, but it's unavailable due to the project plan (Spark). I'll just add a call to retrieve the user information to make it full. @nbransby any tips or ideas? |
Maybe its possible to get the test running locally via the emulator? https://firebase.google.com/docs/emulator-suite/connect_auth#emulated_custom_token_authentication Also it would make sense to add the test to the firebase-kotlin-sdk, then it can also confirm the android behavior (if the android sdk doesn't populate the email address on signInWithCustomToken then we don't need to support it in java either) |
… photoUrl and displayName implementation
@nbransby I tried running it on the emulator and the closest I got was to use the idToken from the emulator (which should obviously not work) -> "third-party tokens cannot be used with signInWithCustomToken". Maybe I just misunderstood something. I could see running the Firebase function locally on the emulator and generating a custom token like that, then, using that custom token to sign in with the emulator. Is that what you meant? Outside of that, I've added a call that returns user information within signInWithCustomToken which seems to work. Will appreciate any help. |
Hey!
I'm currently working on a compose multiplatform project and need this EP to be implemented, so here it is.
I pretty much just copied the solution for the verifyPassword, so there shouldn't be anything new or pricky.