capawesome-team / capacitor-firebase

⚡️ Firebase plugins for Capacitor. Supports Android, iOS and the Web.
https://capawesome.io/plugins/firebase/
Apache License 2.0
365 stars 91 forks source link

bug: authStateChange never fires. #140

Closed braincomb closed 1 year ago

braincomb commented 2 years ago

Plugin(s): "@capacitor-firebase/authentication": "^0.5.0"

Platform(s): Android

Current behavior: When using const result = await FirebaseAuthentication.signInWithGoogle() for example, the login is successful and result.user is returned. However, authStateChange is never fired. Note that this works in web, but NOT on Android.

Expected behavior: authStateChange needs to be fired on any change in auth state, e.g. re-opening app or signing out.

Steps to reproduce:

Related code:

  // capacitor.config.json
  "plugins": {
    "FirebaseAuthentication": {
      "skipNativeAuth": false,
      "providers": ["twitter.com", "google.com", "facebook.com"]
    },
  }
// main.ts
import { FirebaseAuthentication } from "@capacitor-firebase/authentication";
FirebaseAuthentication.removeAllListeners().then(() => {
  FirebaseAuthentication.addListener("authStateChange", (result) => {
    if (result.user) {
      // this never fires
    } else {
      // this never fires
    }
  });
});
  // Login.vue
  const signInWithGoogle = async () => {
    try {
      const result: SignInResult = await FirebaseAuthentication.signInWithGoogle();

      if (result.user) {
        return result.user;
      }
    } catch (err) {
      console.log(err);
    }
  };

Other information: Some potentially relevant logs from Android Studio:

V/Capacitor/FirebaseAuthenticationPlugin: Notifying listeners for event authStateChange D/Capacitor/FirebaseAuthenticationPlugin: No listeners found for event authStateChange V/Capacitor/Plugin: To native (Capacitor plugin): callbackId: 81763094, pluginId: FirebaseAuthentication, methodName: addListener V/Capacitor: callback: 81763094, pluginId: FirebaseAuthentication, methodName: addListener, methodData: {"eventName":"authStateChange"}

Also, it looks like there's a similar issue that was closed but with no real solution, https://github.com/capawesome-team/capacitor-firebase/issues/56

Capacitor doctor:

Latest Dependencies:

  @capacitor/cli: 3.6.0
  @capacitor/core: 3.6.0
  @capacitor/android: 3.6.0
  @capacitor/ios: 3.6.0

Installed Dependencies:

  @capacitor/cli: 3.4.3
  @capacitor/core: 3.4.3
  @capacitor/android: 3.4.3
  @capacitor/ios: 3.4.3
robingenz commented 2 years ago

Hi @braincomb, the problem with #56 was that i could not reproduce it. In my demo app the listener works. Could you try the demo app and see if it works for you?

braincomb commented 2 years ago

Hi @robingenz,

I'll try the demo app when I get a chance, but so far the only difference that I can see is that you do not initiliazeApp firebase app if the platform is native Capacitor. I don't know if that makes any difference, but I do require to initialize JS SDK, since I'm also using Firestore.

Additionally, I noticed that Notifying listeners for event authStateChange happens before the listener is added in the Android Studio logs. I don't know if this has any significance.

In the end I think I will end up having to use skipNativeAuth=true with JS SDK auth, since that also gives me AdditionalUserInfo that I need.

Thank you.

robingenz commented 2 years ago

skipNativeAuth should not matter in this case. I have tested it with both.

aronsuarez commented 2 years ago

same problem here but with iOS, it works on the Web

jaythegeek commented 2 years ago

I too am having the same issue. Vue App. Calling the addListener and it is attaching it. It just never gets fired on IOS. Any ideas?

robingenz commented 2 years ago

Please provide a minimal, reproducible example so that I can debug the it.

aaronksaunders commented 1 year ago

I have been stumped by this same issue all day and was forced to create a workaround, it works correctly when running in the browser but not on the device. In the browser, the listener is called at startup, on device it is not.

this work around fails because I don't have access to the js authenticated user when restarting the app with a previously logged in user

    await FirebaseAuthentication.removeAllListeners();

    if (Capacitor.isNativePlatform()) {
        const result = await FirebaseAuthentication.getCurrentUser();

        //  This always returns null, even if the result is not null
        console.log("js user", await getAuth().currentUser);

        callback(result?.user);
        return
    }

    // this never fires on the device
    FirebaseAuthentication.addListener("authStateChange", (result) => {
        if (result.user) {

            //  This works correctly in browser
            console.log("js user", await getAuth().currentUser);

            callback(result.user);
            debugger;
        } else {
            callback(null);
        }
    });
robingenz commented 1 year ago

Hi @braincomb, i think i got your problem now. Does this help you: https://github.com/capawesome-team/capacitor-firebase/issues/159#issuecomment-1194667877 ?

aaronksaunders commented 1 year ago

@robingenz - sample project in vuejs where the listener is not firing when app starts up on device

https://github.com/aaronksaunders/mobile-gauth

robingenz commented 1 year ago

@aaronksaunders Thank you! I will have a look.

robingenz commented 1 year ago

@aaronksaunders Have you tested your demo? It works on the Web but not on iOS, i just get a blank white screen without error message on xcode or safari devtools:

grafik
aaronksaunders commented 1 year ago

@robingenz that is the bug... the state change is never firing at startup so the app doesn't now if it should show the home screen or the login screen

robingenz commented 1 year ago

@aaronksaunders The auth state change listener is firing. See my screenshot. You can debug it with XCode yourself.

grafik

Maybe it is a problem with Capacitor or your code. I'll try to find out more. In any case, the plugin is doing its job.

aaronksaunders commented 1 year ago

@robingenz - I debugged it in your code also, the problem is that it is firing before the Ionic app is completely initialized and able to set the listener up.

just because it is firing in the plugin doesn't demonstrate that it is actually properly notifying its listeners...

do you have a sample application showing that in fact listeners sent in code are being called in the Ionic application? I will be happy to follow that example but what you have shown here doesn't validate that listeners are 1) being registered properly on the device and two are actually being called

braincomb commented 1 year ago

@aaronksaunders thank you for taking the time to set up a reproducible project; I am of the same assessment that the event may be firing from the plugin but it either 1) doesn't propagate to the WebView layer, or 2) as you said fires before the listener is set up. This can be observed from the logs in my original post from Android Studio:

V/Capacitor/FirebaseAuthenticationPlugin: Notifying listeners for event authStateChange
D/Capacitor/FirebaseAuthenticationPlugin: No listeners found for event authStateChange
V/Capacitor/Plugin: To native (Capacitor plugin): callbackId: 81763094, pluginId: FirebaseAuthentication, methodName: addListener
V/Capacitor: callback: 81763094, pluginId: FirebaseAuthentication, methodName: addListener, methodData: {"eventName":"authStateChange"}
aaronksaunders commented 1 year ago

@braincomb thanks for doing the work also, I was just about to do the same with the IOS plugin, but it already confirms my suspicion that the authstate change is firing at startup of the plugin and the web code hasn't attached the listener yet. Hopefully, this will be enough proof for @robingenz to take another look at this.

leochaddad commented 1 year ago

@aaronksaunders did you come up with any temporary solution? I'm experiencing the exact same problem.

robingenz commented 1 year ago

I will look at it again and possibly consult with the Capacitor team to see what solution they recommend.

aaronksaunders commented 1 year ago

@leochaddad - I figured out a workaround to the problem for my vuejs application, I set skipNativeAuth to false

and then I just use the JS SDK authListener since it does fire.

    const initialized = ref(false);

    const app = initializeApp(firebaseConfig);
    if (Capacitor.isNativePlatform()) {
        auth = initializeAuth(app, {
            persistence: indexedDBLocalPersistence,
        });
    } else {
        auth = getAuth(app);
    }

    auth.onAuthStateChanged(async (user: any) => {
        console.log("user - onAuthStateChanged", user);
        initialized.value = true;
    })

I don't start the app completely until I know authStateChanged has been called

const { initialized } = useFirebaseService();

const app = createApp(App)
  .use(IonicVue)
  .use(router);

watch(initialized, (value) => {
  console.log("initialized", value);
  // mount the app
  console.log('mounting the app...');
  router.isReady().then(() => {
    app.mount('#app');
  });
})
jaythegeek commented 1 year ago

Sorry I wasn't able to give a reproducible, I have used a similar method to @aaronksaunders - I am authed wooohooo... Then comes the fun part, on web I can use Firestore DB perfectly as the token is sent with the request and handled as per the Firebase docs.

However, trying to fire a web based firestore request from iOS causes a not authed error - I cannot figure out why this would be the case, I know the firestore connection works because I am able to read data etc.

Any ideas on that, seems like the current authed token is not actually passed back and used within iOS unless I am missing something

aaronksaunders commented 1 year ago

@jaythegeek - not sure what you are doing but if you followed my approach it should work fine. I just retested my code and I am able to query the database on web and on device.

check out project here - https://github.com/aaronksaunders/mobile-gauth

jaythegeek commented 1 year ago

@aaronksaunders do you have a security rule on that query such as;

match /users/{userId} {
      allow write: if request.auth.uid == userId;
      allow read: if true;
}

The request.auth is not set when performing this from iOS but does work from web, it's strange.

I am imagining your query doesnt have a rule on it like pretty much all of my reads, so it works fine, but just not the authed calls for writes etc.

Any further suggestions would be appreciated, I have no hair as it is! 😂

Looking over your code now and we are pretty much on the same page :/

aaronksaunders commented 1 year ago

@jaythegeek I have a security rule - also the way my code is written, I can't even get to the query until firebase js SDK fires the authChangedListener, that is why I don't think you are doing exactly what I am doing

jaythegeek commented 1 year ago

@aaronksaunders I have literally taken your firebase.ts and swapped out the config, then the write I am doing to firestore is like this:

import { useFirebaseService } from "@/services/firebase";
const { db } = useFirebaseService();
// other imports

function updateUser(){
  await setDoc(
        doc(db, "users", user.id),
        {
          thing: 'other thing',
        },
        { merge: true }
      );
 }

All my reads and anything without a security rule works great, if I turn that particular security rule off, it works perfectly.

aaronksaunders commented 1 year ago

@jaythegeek can you show me what the output is in your console log?

also, show me how you are ensuring the js initialization has been completed before you render the first component of your app.

happy to move this chat to a discussion since we are off-topic here, this is all a workaround because the plugin doesn't work

jaythegeek commented 1 year ago

@aaronksaunders I have opened a discussion - Yes frustrating have to work around, this is the core function of the plugin :/ @robingenz if we figure it out, I'll come back and update here as it may help to resolve the underlying issue we are working around.

robingenz commented 1 year ago

Yes, feel free to let me know, I appreciate any help. PRs are also welcome! I'm a bit busy at the moment as I only maintain these open source projects in my spare time and I'm currently updating more than 15 plugins to Capacitor 4. I will try to work on this issue in a timely manner. If you can't find a solution and want me to invest extra time and prioritize your issue you are welcome to sponsor this issue once, see https://github.com/sponsors/capawesome-team?frequency=one-time.

jaythegeek commented 1 year ago

@robingenz I appreciate your position, if we can fix it we will. I am snowed under too and this particular problem is causing me a delay, so I am sure, in the event a few more hours doesn't solve it, we can come to an arrangement 😄

aaronksaunders commented 1 year ago

@robingenz - I appreciate the work, you asked for a sample showing the bug, and I provided it to you... not sure what else there is you are expecting other than I fix the bug and submit a PR.

The fact is the plugin doesn't work, IMHO this is not a bug, it is a missing feature and everything I am doing is a hack to make it work

robingenz commented 1 year ago

@aaronksaunders That's right I have all the info I need to solve the problem. Thanks for that! I just wanted to let you know why it's waiting so long and what the options are. The issue is just waiting for me to find time for it. That's all! As I said, I do this in my spare time, which is limited. You can send a PR if you want to speed up the process. Or you can sponsor the project. If neither is an option, I can only do it when I find time.

robingenz commented 1 year ago

I have tried a few things but unfortunately had no success. I don't get when the app is ready and a listener is active. There is a load method but it fires too early.

There is one thing I still don't understand: What reason would there even be to listen to the listener at the launch of the app? If you want to know if a user is logged in (to send him to the login page, for example), you can simply call getCurrentUser(). The listener would in that case only slow down your app launch, because you wait for the plugin to notify you instead of just actively asking the plugin.

Can someone explain to me why you should rely on the event listener when opening the app?

To be clear: We are only talking about the app launch here. Once the app is launched, the listener works fine for me.

aaronksaunders commented 1 year ago

Mostly because that is the pattern that is used in the Firebase JavaScript API documentation I believe. Pretty much most of the web apps follow the approach of checking for a user before routing vs route to login and the redirect

If you are suggesting an alternative approach then you should probably provide a sample for people who use the plugin to follow.

Either way it is creating one approach for running on the web and another for running native.

I was trying to take a look at the pattern followed by the react native firebase plug-in, but got distracted. Maybe you are correct that the native api’s aren’t structured such that you can check at startup

robingenz commented 1 year ago

Pretty much most of the web apps follow the approach of checking for a user before routing vs route to login and the redirect

This approach can be implemented in different ways. For example, this is my implementation in the demo app (Angular Guard):

import { Injectable } from '@angular/core';
import { CanActivate, Router } from '@angular/router';
import { FirebaseAuthenticationService } from '@app/core';

@Injectable({
  providedIn: 'root',
})
export class AuthGuard implements CanActivate {
  constructor(
    private readonly firebaseAuthenticationService: FirebaseAuthenticationService,
    private readonly router: Router
  ) {}

  public async canActivate(): Promise<boolean> {
    const user = await this.firebaseAuthenticationService.getCurrentUser();
    if (user) {
      return true;
    }
    this.router.navigate(['/login']);
    return false;
  }
}

So my recommendation is: If you want to know if a user is logged in at the start of the app, the best way is to check this with getCurrentUser() and not wait for the event listener.

If you are suggesting an alternative approach then you should probably provide a sample for people who use the plugin to follow.

Yes, I am planning several blog posts in the future and would like to take up this topic there.


However, I started a discussion in the Capacitor repository. Maybe someone has an idea there.

TadasMil commented 1 year ago

@robingenz Don't want to be rude, but getCurrentUser function on Android always returns user object even when user list inside of Authentication table of Firebase is empty. For Web, it works fine. Is this the correct implementation of getCurrentUser? Seems strange when you have no users at all, but on Android user is being returned with getCurrentUser lmo.

robingenz commented 1 year ago

@TadasMil If no user is signed in, getCurrentUser should return:

{
  "user": null,
}

If this is not the case for you then please create a new issue.

robingenz commented 1 year ago

I maybe found a fix. Could someone test this dev build (Capacitor 4)?

npm i @capacitor-firebase/authentication@1.0.0-dev.f101fe0.1661165211
Guiditox commented 1 year ago

I maybe found a fix. Could someone test this dev build (Capacitor 4)?

npm i @capacitor-firebase/authentication@1.0.0-dev.f101fe0.1661165211

I did and it works perfect!

Thank you very much for contributing to the community. Excellent plugin!

robingenz commented 1 year ago

@Guiditox Thank you for testing!

braincomb commented 1 year ago

I maybe found a fix. Could someone test this dev build (Capacitor 4)?

npm i @capacitor-firebase/authentication@1.0.0-dev.f101fe0.1661165211

Hi @robingenz

Tested on Android and I can confirm the listener is firing at start-up as expected.

However, the result is always null. Is it possible because I'm using the JS SDK method of signing in?

      const result = await FirebaseAuthentication.signInWithGoogle();

      const credential = GoogleAuthProvider.credential(result.credential?.idToken);
      const auth = getAuth();
      const userCredential = await signInWithCredential(auth, credential);

I was forced to use the above method because const result = await FirebaseAuthentication.signInWithGoogle(); doesn't return result.user for me.

So it's possible I'm doing something wrong here, but the original issue seems to be fixed, so thank you!

robingenz commented 1 year ago

@braincomb Yes, if you use 'skipNativeAuth=true' this listener will fire 'null' and no user is returned (only the credentials for the Firebase JS SDK).

aaronksaunders commented 1 year ago

That’s the bug…

On Mon, Aug 1, 2022 at 6:09 AM Robin Genz @.***> wrote:

@aaronksaunders https://github.com/aaronksaunders Have you tested your demo? It works on the Web but not on iOS, i just get a blank white screen without error message on xcode or safari devtools: [image: grafik] https://user-images.githubusercontent.com/13857929/182125752-8254f8d8-b068-46b6-a904-0f59abcf60f8.png

— Reply to this email directly, view it on GitHub https://github.com/capawesome-team/capacitor-firebase/issues/140#issuecomment-1200995553, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEAFGITPEHCXASNVV4VGKLVW6O5RANCNFSM53MKITUA . You are receiving this because you were mentioned.Message ID: @.***>

--

--

Aaron K. Saunders CEO Clearly Innovative Inc @.*** www.clearlyinnovative.com

This email message and any attachment(s) are for the sole use of the intended recipient(s) and may contain proprietary and/or confidential information which may be privileged or otherwise protected from disclosure. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient(s), please contact the sender by reply email and destroy the original message and any copies of the message as well as any attachment(s) to the original message.