-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Firebase UI does not work with Safari on Mac or iPhone. Recent regression. #977
Comments
Does not occur on Safari 16. Could be a regression or behavior change in Safari 16.1 and how iframe messages are handled/passed. |
I see this with Safari 16.0 too.. But the modular demo app and the auth-compat demo app from firebase-js-sdk are working ok for me. |
I am also seeing failing following behaviour, tested with Safari Version 16.1 (17614.2.9.1.8, 17614) (Current Beta) using firebasejs 9.11.0 Compat and firebasejs-ui 6.0.1. On our webpage app the behaviour seen is -
It seems that the Callback "signInSuccessWithAuthResult" from uiconfig in ".fbui.start('id, uiconfig)" is not executed. The same failing behaviour is seen on the demo app https://fir-ui-demo-84a6c.firebaseapp.com/ as linked by koush, if you select "Sign in with Redirect". |
I ended up stripping firebase ui out and building my own ui using my existing ui framework (vue/vuetify). The oauth callback process going redirecting to some random firebase page before redirecting back to the app, iframe intercommuncation, etc, seems unnecessarily complex and fragile. I'm not sure how much of this is due to firebase-js vs firebase-ui. In any case, it's easy to build the callback urls per platform, which is what I did. Then, covert the tokens into credentials using firebase-js in the callback page, roughly: const state = JSON.parse(searchParams.get('state')!);
const { provider } = state;
let credential: OAuthCredential | undefined;
if (provider === 'google') {
credential = GoogleAuthProvider.credential(searchParams.get('id_token'));
}
else if (provider === 'apple') {
const oauthProvider = new OAuthProvider('apple.com');
credential = oauthProvider.credential({
idToken: searchParams.get('id_token')!,
rawNonce: state['nonce'],
});
}
else if (provider === 'email') {
if (!isSignInWithEmailLink(auth, window.location.href))
throw new Error('Invalid sign in link.');
const email = localStorage.getItem('emailForSignIn');
const password = localStorage.getItem('passwordForSignIn');
if (!email || !password)
throw new Error('Please open this link in the same browser that initiated the Sign In.');
localStorage.removeItem('emailForSignIn');
localStorage.removeItem('passwordForSignIn');
signInWithEmailLink(auth, email, window.location.href).then(async userCredential => {
try {
await updatePassword(userCredential.user, password);
await getIdTokenAndRedirect(userCredential, state);
}
catch (e) {
console.error('error', e);
error.value = e.message || e.toString();
}
});
}
else {
throw new Error('unknown provider: ' + provider);
}
async function checkCredential() {
const userCredential = await signInWithCredential(auth, credential!);
await getIdTokenAndRedirect(userCredential, state);
}
if (credential) {
checkCredential()
.catch(e => {
console.error('error', e);
error.value = e.message || e.toString();
});
} This lets me continue using my existing firebase ids, to which my app is currently tied, but also provides me an offramp from firebase itself, as I now am collecting and mapping the native oauth provider ids. |
It would be really nice if someone from the Firebase team could respond here. This is now impacting everyone who upgrades to iOS 16.1 and MacOS Ventura, and I expect it will eventually trickle down to users on older OS versions as well. The only option we have at the moment is apparently just to rip out |
After further digging I've found a potential workaround, which is to update the var uiConfig = {
callbacks: {
signInSuccessWithAuthResult: () => false,
},
'signInOptions': [{
provider: firebase.auth.GoogleAuthProvider.PROVIDER_ID,
customParameters: {
prompt: 'select_account' // Forces account selection
},
}],
'signInFlow': 'popup',
};
var ui = new firebaseui.auth.AuthUI(firebase.auth());
ui.start('#firebaseui-auth-container', uiConfig); Got the critical hint from: firebase/firebase-js-sdk#6443 (comment) I hope this works for others! |
This appears to work for our web app. However, our mobile application utilizes a webview and unfortunately the popup required for this solution does not work. Still hoping for some action/input by the Firebase team. @firebase-ops , @bojeil-google , @jamesdaniels ? |
Want to clarify that there are 2 different issues here:
This happens in the firebase ui demo - https://fir-ui-demo-84a6c.firebaseapp.com/ on Safari 16.0+. In this case, the app is in the same domain as the oauth handler (which shows up as "https://fir-ui-demo-84a6c.firebaseapp.com/__auth/handler")
This issue happens when the app is hosted in a different domain than <project>.firebaseapp.com. The Oauth handler is accessed at <project>.firebaseapp.com, so when performing a signin with redirect the flow is like this: app domain -> <project>.firebaseapp.com/__auth/handler -> IDP page (example - accounts.google.com) -> back to <project]>.firebaseapp.com/__auth/handler with IDP response -> put response in browser storage and go back to app domain -> cross origin iframe to read the IDP response that was put into browser storage, to complete sign in. This cross-domain browser storage access which is failing likely due to Safari ITP feature. This current github issue is tracking scenario 1. I am able to repro this error if I deploy the UI web demo using the CDN, but if I build it using npm, there is no error. Will dig into this some more. |
As @koush mentioned in the issue description, looks like this might be due to the event being received as a window Message event (with the relevant fields in event.data, instead of event). @koush can you share the steps to get the detailed stacktrace and the variable value? I only see a cryptic stacktrace without the actual function names and adding log messages/building locally doesn't repro the issue. I did verify that in the working case, the AuthEvent delivered has the fields to be destructured into. (in https://github.com/firebase/firebase-js-sdk/blob/b6c231a282313aeda59c447c24f71fdad35240bc/packages/auth/src/core/strategies/abstract_popup_redirect_operation.ts#L86) |
Our team switched to the const uiConfig = {
// ...
'signInFlow': 'popup',
}; Our team was unable to log into the firebaseui demo using either |
The destructing error is fixed by pulling in the fix in js-sdk - firebase/firebase-js-sdk#5644. |
@prameshj do you mean #982? If you do, could you please provide any update on when this will go into a new release? I wanted to install the library with a fix using EDIT: I was able to rebuild the library, but the issue still persists when using it.. |
My firebase code is not displaying the
|
Hoping to release by next week. The demo page has been redeployed - https://fir-ui-demo-84a6c.firebaseapp.com/. |
Thanks @prameshj! I can confirm I'm not seeing issue #1 anymore, neither on desktop Safari 16.1 nor iPadOS Safari 16.1.1. Also, I was able to fix issue #2 by following the official mitigation #3. So all seems good now, with these fixes. |
I can also confirm the same, although using mitigation nr1! Thank you @prameshj!! 🎉 |
Thanks for sharing folks! firebase-ui 6.0.2 has been published on npm - https://www.npmjs.com/package/firebaseui I am closing this issue since the demo app has also been fixed. Thanks! |
[REQUIRED] Describe the problem
Firebase UI does not work with Safari on Mac or iPhone.
Steps to reproduce:
Additionally: these steps work fine with Chrome on the same Mac.
Relevant Code:
N/A
Seen in console:
TypeError: Right side of assignment cannot be destructured
The event it is receiving seems to be a window Message event, and not the JSON contained within
data
which is what contains the fields to be destructured.Source file:
https://github.com/firebase/firebase-js-sdk/blob/b6c231a282313aeda59c447c24f71fdad35240bc/packages/auth/src/core/strategies/abstract_popup_redirect_operation.ts#L86
The text was updated successfully, but these errors were encountered: