Skip to content
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

9.0.0 Auth doesn't work: GoogleAuthProvider is failing FederatedAuthProvider and BaseOAuthProvider instanceof assertions #5420

Closed
jpike88 opened this issue Aug 31, 2021 · 7 comments · Fixed by #5427
Assignees

Comments

@jpike88
Copy link

jpike88 commented Aug 31, 2021

  • Operating System version: macOS
  • Browser version: Chrome
  • Firebase SDK version: 9.0.0-beta.7
  • Firebase Product: auth
  • Environment: Cordova + iOS 15, same issue on Cordova + Android 11 Chromium

(I use fuse-box as a bundler, and my code is being output to ES2015 code, and I have countless other libraries in my codebase that run fine.)

Things were working with Firebase 8.x, but after upgrading to 9 and migrating to the required code:


import { GoogleAuthProvider, FacebookAuthProvider } from '@firebase/auth';
import {
	getRedirectResult,
	signInWithRedirect,
	getAuth,
} from '@firebase/auth/cordova';

const auth = getAuth(Environment.firebaseInstance);
			const provider = new GoogleAuthProvider();
			try {
				await signInWithRedirect(auth, provider);
			} catch (error) {
				// swallow this, just return.
				return;
			}
			const result = await getRedirectResult(auth);
			const credential = GoogleAuthProvider.credentialFromResult(result);
			await this.handleSSOAuthResult(
				AuthProviders.Google,
				result.user.email,
				credential.idToken
			);

This code doesn't work, an assertion fails here:

https://github.com/firebase/firebase-js-sdk/blame/509c18fbc1a0d4f85a0ca8c7cfef8e3b0f3183ec/packages/auth/src/platform_browser/strategies/redirect.ts#L95

See attached image for what inspecting breakpointed variables looks like:

131309455-afbc6865-f401-4d56-a9c3-e50c95712028

see this comment for detailed further investigation

Current workaround, on postinstall do a find+sed on the @firebase folder, and replace all instances of provider instanceof FederatedAuthProvider and provider instanceof BaseOAuthProvider with true in order to disable the asserts, and everything works properly.

@jpike88 jpike88 changed the title Critical: GoogleAuthProvider is failing FederatedAuthProvider and BaseOAuthProvider instanceof assertions Critical, Auth doesn't work: GoogleAuthProvider is failing FederatedAuthProvider and BaseOAuthProvider instanceof assertions Aug 31, 2021
@jpike88 jpike88 changed the title Critical, Auth doesn't work: GoogleAuthProvider is failing FederatedAuthProvider and BaseOAuthProvider instanceof assertions 9.0.0 Auth doesn't work: GoogleAuthProvider is failing FederatedAuthProvider and BaseOAuthProvider instanceof assertions Aug 31, 2021
@jbalidiong jbalidiong added the v9 label Aug 31, 2021
@jbalidiong
Copy link
Contributor

Hi @jpike88, thanks for reporting this behavior. If I can replicate the issue, I can have a better look into it. Please share a minimal, but complete sample of a project that I can run locally.

@jpike88
Copy link
Author

jpike88 commented Aug 31, 2021

am arranging this as a priority within the hour... hang tight

@jpike88
Copy link
Author

jpike88 commented Aug 31, 2021

Note: In making this, I'm almost certain the issue is specific to the @firebase/auth/cordova module. It actually seemed to work ok when I was importing everything from @firebase/auth and not @firebase/auth/cordova.

Ready.

Instructions:

Run npm ci to install everything.

Then use ts-node or tsc in the base folder, and then run:
node fuse default

and then go to localhost:4444.

Look for a file called app.component.ts and that will contain the code I'm testing. You can put your test firebase credentials in there. Also, notice I put a little code in to fool the library into thinking the environment is Cordova so the same error is fired.

You'll see a button called CLICK ME, click it and look at the console.

Archive.zip

@jpike88
Copy link
Author

jpike88 commented Aug 31, 2021

I found the issue, I was using the GoogleLoginProvider exposed via @firebase/auth, and mixing that with the methods I was pulling out of @firebase/auth/cordova. The assertions failed because even though the superclasses had the same name, they weren't actually on the same prototype hierarchy.

I should have used the GoogleLoginProvider from @firebase/auth/cordova, that solves the issue.

But this is super annoying of a design flaw! There should be a warning if people are accidentally mixing libraries like that, they seem to have an identical API and it's easy to get stuck on thinking the bundler was the problem.

@jbalidiong
Copy link
Contributor

Glad to know that it is a library mix-up issue and not the SDK itself. As a suggestion, I recommend adding the validation of libraries used to prevent this particular scenario.

@jpike88
Copy link
Author

jpike88 commented Aug 31, 2021

Well I think it warrants at least a note in the documentation. Otherwise it's like 'don't press this red button, press the other red button'.

@Feiyang1
Copy link
Member

Feiyang1 commented Sep 1, 2021

@sam-gc We could produce a better error message in case people mix up the submodule imports. Firestore does something similar by checking the expected instance type and throw if it doesn't match. See https://github.com/firebase/firebase-js-sdk/blob/master/packages/firestore/src/util/input_validation.ts#L166-L172

Reopening ..

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
5 participants