-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
[🐛] IllegalArgumentException: Given link is not a valid email link #8360
Comments
So to make sure I understand, according to your code, you do check by calling the method they indicate, right before you then try to use the URL and get the error message indicating there is a problem? 🤔 Our implementation here appears to be a javascript interpretation of the rules vs calling the underlying native method: react-native-firebase/packages/auth/lib/index.js Lines 363 to 369 in 082a1e4
I wonder if the javascript interpretation lacks fidelity somehow... The immediate workaround of course is to catch that error + check for that specific error code and work with the user somehow But something else is apparently more deeply wrong firebase-js-sdk appears to be just checking for 'mode=signIn' in the URL as their validation - Which is basically what we are doing, so ... looks okay from that perspective - whatever link made it past your check should have been valid Frustrating that our code has a failure listener for the task but the exception bubbles out anyway - I think that's an upstream bug to be honest: Lines 448 to 459 in 082a1e4
Since you appear to have a reproduction case - if you reached right into those lines and altered them to be this instead: try { // <-- add this try
firebaseAuth
.signInWithEmailLink(email, emailLink)
.addOnSuccessListener(
authResult -> {
Log.d(TAG, "signInWithEmailLink:onComplete:success");
promiseWithAuthResult(authResult, promise);
})
.addOnFailureListener(
exception -> {
Log.e(TAG, "signInWithEmailLink:onComplete:failure", exception);
promiseRejectAuthException(promise, exception);
});
// add this catch block with the same code as addOnFailureListener to handle exceptions outside of Task
} catch (Exception e) {
Log.e(TAG, "signInWithEmailLink:onComplete:failure", exception);
promiseRejectAuthException(promise, exception);
} ...I bet it would not crash, and the error would bubble up the normal way through the javascript call chain |
my full code is currently if (isSignInWithEmailLink(auth, url)) {
try {
const firebaseCredential = await signInWithEmailLink(auth,
route.params.email,
url
);
const idToken = await firebaseCredential.user.getIdToken();
...
} catch (e: any) {
setLoading(false);
setWaiting(false);
switch (e.code) {
case 'auth/invalid-action-code':
case 'auth/expired-action-code':
showAlert(t('code_expired_title'), t('code_expired_message'));
break;
case 'auth/invalid-email':
showAlert(t('invalid_email_title'), t('invalid_email_message'));
break;
case 'auth/user-disabled':
showAlert(t('user_disabled_title'), t('user_disabled_message'));
break;
default:
showAlert(t('generic_error_title'), t('generic_error_message'));
}
}
} else {
/// log error
} error is not catched |
1- I asked a question what's the answer to the question? how did it go when you tried the proposed solution? |
You are right, first we check Here is an example url we use: |
To be honest, you posted such a small amount of information in your issue above that I'm tempted to say it's sun spots. It could be anything. Perhaps you upgraded from some older version to the current version, and if you look at the changelog here https://github.com/invertase/react-native-firebase/blob/main/CHANGELOG.md you might see that firebase-android-sdk changed, and if you consult their release notes you may find that they changed the implementation of this method. I don't know though because you didn't include any information that might help determine a difference. You don't talk about frequency, and you don't provide an example link that would reproduce the failure, so who can say for sure? No one All I can say is that: 1- the try/catch seems to work, which is good, we can patch that in |
@mikehardy if i add the
|
@Willham12 that's surprising since firebase-ios-sdk and firebase-js-sdk apparently don't require that, I wonder why Android is different 🤔. Does Android generate sign in links with the apiKey URL param already, such that it can consume it's own links at least? If it does, then perhaps this is a case of people using links generated in firebase-ios-sdk or firebase-js-sdk to log in via firebase-android-sdk somehow 🤔 . Including a full reproduction scenario that includes generating the links that would show this problem would help. Any reproduction scenario really would be helpful It does seem strange to me to include the API key in the URL though - is that the API key from (for instance) the google-services.json file? The API key for the whole firebase project? Not the sort of thing you'd pass around normally, and I would expect security concerns to be handled by the "token" that is the OOB code |
It's the Web API Key (known as Firebase-provisioned API key) from the firebase console. We are using the firebase admin sdk to generate a link with |
Interesting, thanks for the context.
I linked the code for official firebase-js-sdk and react-native-firebase above. It is not verifying apiKey. For some reason firebase-android-sdk appears to be requiring it, though firebase-ios-sdk (which does not open source that code for verification...) does not seem to. So this appears to be android-specific. It must be triggered rarely because so few people re-implement the web auth thing for templating reasons and just rely on the built-in templates that firebase automatically provides. Based on your reporting, it seems those firebase-provided templates include the apiKey so most people just don't suffer this problem. That needs to be verified - easy enough. So now the actions seem to be:
Does that sound right @Willham12 ? It seems like you have already basically implemented this - if you posted a PR with what you have I could work with you to shepherd it to merge + release really quickly |
Crashed on Android, ios working fine.
"@react-native-firebase/app": "21.11.0",
"@react-native-firebase/auth": "21.11.0",
The text was updated successfully, but these errors were encountered: