invertase / react-native-firebase

🔥 A well-tested feature-rich modular Firebase implementation for React Native. Supports both iOS & Android platforms for all Firebase services.
https://rnfirebase.io
Other
11.66k stars 2.21k forks source link

[🐛] Missing firebase-js-sdk API #7483

Open Nantris opened 10 months ago

Nantris commented 10 months ago

Issue

Unimplemented JS SDK features include:

Also possibly the following:

I located these by searching for the string not implemented in the portions of the project we use. I've also made an effort to document the others but this may not be comprehensive, or may contain false positives for the auth ones.


Nantris commented 10 months ago

Originally mentioned in #7433.

Nantris commented 10 months ago

I also noticed uploadBytes is not implemented also. Is there anything else we can use in the meantime, or any plan to implement this in the immediate future? It would be a shame to have to revert adding react-native-firebase to our app.

Unfortunately I lack the knowledge and time to help implement it.

mikehardy commented 10 months ago

@Slapbox we handle the non-modular put API which is very nearly uploadBytes by signature, it handles the same type of input which tells me that under the covers we are already doing all the necessary things from javascript to native and back

So my first take on it is that supporting/implementing uploadBytes is probably just exporting the correct symbol and API shape, and should be nearly trivial. Those are famous last words but it really does look that way to me

Need to get react-native 0.73 unexpected incompatibility #7500 fixed before I can take a look at uploadBytes but I'll check it out with priority tomorrow

Nantris commented 10 months ago

Those are famous last words but it really does look that way to me

Too true!

I really appreciate your attention on this a lot. Thank you for the great project and the great assistance!


Workaround:

I just came back to update that it seems that uploadBytesResumable is implemented and seems so far to be a viable workaround for us, but they return different types. We're just lucky to have an implementation that can readily support both without rewriting code. Always those famous last words though.

mikehardy commented 10 months ago

The implementation of uploadBytes (if necessary? Do let me know if uploadBytesResumable doesn't work out...) is likely to just internally do the UploadTask await for then -> then dereference final snapshot -> map final metadata + ref into an UploadResult -> return that

Which is to say it may reduce some boilerplate by handling that for you but it's just bending around the same information and should be working exactly the same under the covers in the native layer

mikehardy commented 9 months ago

Coming over from #4166 -

getMultiFactorResolver is definitely implemented and working, TOTP I don't think was a focus of the original implementer (and in fact I think it was actually added as a firebase feature after the implementation of MFA here but my memory is a bit vague on the timing, apologies if wrong)

It appears uploadBytes is actually not a blocker now, is that correct?

Auth-related things obviously would be though - no one is going to change their well-considered hard-won MFA auth flows without a serious (and unwanted) battle - I understand that

So the MFA Resolver should kick out a list of hints and TOTP should be included (I think?) despite not being implemented yet here

At that point I think an implementation of TotpMultiFactorGenerator is all that is needed, and I don't recall that being very difficult.

Auth unenroll and token listeners are already present and working (in fact, native has a super-listener beyond the JS SDK of a "user change listener" which is the superset of token changes and user changes, but token listening by itself is I think all implemented.

So if I'm following along on your migration feasibility audit correctly it sounds like the TotpMultiFactorGenerator is really the blocker at the moment? If so, an implementation of that is something I could schedule

Nantris commented 9 months ago

Thank you as always for your great work and assistance @mikehardy!

It appears uploadBytes is actually not a blocker now, is that correct?

I'm 99% sure it's not a blocker, but I've not done extensive testing of every part of our application. We're doing a major rewrite so nothing is headed to production yet. Right now I'm just calling it "Good enough to keep moving in development."

If I encounter any portion of our code that can't use uploadBytesResumable as a drop-in replacement, it should be possible to adapt it on our end, but it would be good to have implemented in RNF eventually.


Regarding TotpMultiFactorGenerator - it's not a blocker for us at present, but in the future it would be if not implemented. We're planning to add TOTP support in the next few months, so I was scouting ahead to make sure the path is clear. I wanted to highlight that one because, whether we were going to use it or not, it's a security method without any possible workaround.

I think you're right that it didn't exist when getMultiFactorResolver was added. It's a rather new method; I'm pretty sure it's less than a year old.

github-actions[bot] commented 8 months ago

Hello 👋, to help manage issues we automatically close stale issues.

This issue has been automatically marked as stale because it has not had activity for quite some time.Has this issue been fixed, or does it still require attention?

This issue will be closed in 15 days if no further activity occurs.

Thank you for your contributions.

Nantris commented 8 months ago

Not stale.

mikehardy commented 8 months ago

Not only not stale, there's progress :-)

github-actions[bot] commented 7 months ago

Hello 👋, to help manage issues we automatically close stale issues.

This issue has been automatically marked as stale because it has not had activity for quite some time.Has this issue been fixed, or does it still require attention?

This issue will be closed in 15 days if no further activity occurs.

Thank you for your contributions.

Nantris commented 7 months ago

Not stale. Thanks to the team for the continued progress on this!

github-actions[bot] commented 6 months ago

Hello 👋, to help manage issues we automatically close stale issues.

This issue has been automatically marked as stale because it has not had activity for quite some time.Has this issue been fixed, or does it still require attention?

This issue will be closed in 15 days if no further activity occurs.

Thank you for your contributions.

Nantris commented 6 months ago

Not stale.

github-actions[bot] commented 5 months ago

Hello 👋, to help manage issues we automatically close stale issues.

This issue has been automatically marked as stale because it has not had activity for quite some time.Has this issue been fixed, or does it still require attention?

This issue will be closed in 15 days if no further activity occurs.

Thank you for your contributions.

Nantris commented 5 months ago

Bump.

mikehardy commented 5 months ago

Yep - usually not a fan of bumps but this actually tracks a very worthwhile chunk of work and I want it to stay open as well I am not sure our "pin" actually avoids the stale bot (separate issue...) but hopefully it does.

aem commented 1 month ago

Curious if there's an update specifically on the TotpMultiFactorGenerator compatibility, and if that work can be prioritized. Unfortunately I don't personally have native mobile experience so I'm hesitant to try and contribute it, but not having access to TOTP second factors is tough given the relative insecurity of SMS.

aem commented 1 month ago

Would it maybe be helpful to split that out into its own issue so its not tied to the Storage + Database module work? I'm happy to do a deeper write up on the multi-factor work if that would help move it along