Closed koush closed 1 year ago
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 webpage app works as expected in current versions of other browsers.
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 firebaseui-web
and write our own auth layer, which isn't exactly ideal.
After further digging I've found a potential workaround, which is to update the uiConfig
to use a popup
sign-in workflow instead of the default redirect
. Example:
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: https://github.com/firebase/firebase-js-sdk/issues/6443#issuecomment-1187798276
I hope this works for others!
After further digging I've found a potential workaround, which is to update the
uiConfig
to use apopup
sign-in workflow instead of the defaultredirect
.
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:
1) signInWithRedirect and signInWithPopup stuck in a loop with the console error "TypeError: Right side of assignment cannot be destructured"
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")
2) signInWithRedirect does not return any error/warning, but login does not complete. This is tracked in https://github.com/firebase/firebase-js-sdk/issues/6716
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. The workarounds in https://github.com/firebase/firebase-js-sdk/issues/6443#issuecomment-1187798276 apply to this issue. As mentioned above, this issue is tracked in https://github.com/firebase/firebase-js-sdk/issues/6716 since it is not specific to ui-web.
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 popup
sign-in workflow and this effectively solved our issue. It's not a fix per se, but a viable workaround for some until the regression is addressed.
const uiConfig = {
// ...
'signInFlow': 'popup',
};
Our team was unable to log into the firebaseui demo using either redirect
or popup
workflows, yet the popup workflow still worked just fine within our own app. So don't rule it out until you try the workaround yourself.
The destructing error is fixed by pulling in the fix in js-sdk - https://github.com/firebase/firebase-js-sdk/issues/5644. We have an open PR to update the auth-compat SDK version and re-deploy the demo app.
@prameshj do you mean https://github.com/firebase/firebaseui-web/pull/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 npm i github:firebase/firebaseui-web#2b8be764b293a2c8586a98f30f8d84eb6fd3ef3b
, but the installation process takes a very long time and results in a Gulp error.
EDIT: I was able to rebuild the library, but the issue still persists when using it..
#firebaseui-auth-container login
buttonI added FirebaseUI-web authentication by <div id="firebaseui-auth-container"></div>
in my html page. The script is written in js/Auth.js
file. My browser is unable to load the script from js/Auth.js
. I added <script type="modules" src="js/Auth.js"></script>
in my html
@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
npm i github:firebase/firebaseui-web#2b8be764b293a2c8586a98f30f8d84eb6fd3ef3b
, but the installation process takes a very long time and results in a Gulp error.EDIT: I was able to rebuild the library, but the issue still persists when using it..
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.
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