-
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
[Feature Request] Sign In With Apple #2884
Comments
This is in the pipeline 👍 but will have to be done as part of v7.0.0, the OAuth flow needs a full rework as theres been several breaking changes and our implementation is now quite far behind the Web SDK. #2673 is a part of it, but there still a few bits to do. v7.0.0 though won't be far away like v6 was, hopefully end of this year/early Q1 2020. |
Looking forward to this! The deadline for inclusion (from Apple) is April 2020: https://developer.apple.com/news/?id=09122019b |
The deadline for existing apps is April 2020. Apple won't accept any new apps lacking sign in with Apple 😢 |
April is luckily still a fair bit away, that said I'm just now expanding my work project's login to other providers so I'm listening in on this if you need any testing etc - esp. if the modules ship source so you can depend on a commit hash and just do a build postinstall I can test whatever when this comes through |
Oh! I think I misunderstood you @mcjohnalds - do you mean to say it is already not possible to create a new app on the App Store with federated login that does not have apple? And it's just existing apps grandfathered to 2020? Edited to add - yep - new apps already not allowed - ouch! from the link above
Also edited to add, the loophole is you must also allow email auth, I believe, so you are not exclusively using third-party/social login:
|
@mikehardy yep - new apps already not allowed |
So, what happens when you enable email authentication? According to the text block in their document you would be allowed:
|
@mikehardy My app uses email authentication and was rejected. edit: Mine offers phone authentication too. |
@HeribertoAlves that is terrible! Am I just being dense in my reading of their own rules? I would appeal immediately with that passage quoted and state explicitly that your backend provider still has the integration listed as beta, and you can't rely on beta software. I could rant (uselessly of course) on the app store gatekeeper monopoly thing but the text seems so clear to me here. Please note that my intention here is constructive: I am literally about to attempt this same approval process. So I might be denied as well and I'm motivated to fix it but will likely need help. Who on this thread has Obj-C or Android or OAuth skills? I'm strong on Android but not on ObjC. OAuth is relatively new for me but the underlying APIs exist anyway so should be possible, and there's an existing PR to base off of to get the feature done and acceptable for merge if Apple won't approve for now. |
I have had a similar issue with getting my app to past review from Apple -- even when adding in email and phone authentication they still rejected it because it had "3rd party auth providers without login with apple". I monkey-patched together a working version of expo-apple-authentication and @react-native-firebase/auth that you can find here: https://gist.github.com/nhahn/010305dcff1404bfdeb6e2174b74e5a6 You can apply the patches to your code using patch-package and it should hopefully work |
@nhahn I just read your gist, the expo module source, and the apple website related doc - that looks viable - fantastic - a couple questions if you have time?
|
Reading even more deeply: seems supported at the native level (i.e., via expo-apple-signin) only for iOS and only iOS13+ even: docs say those symbols used in expo-apple-signin are only for iOS13+ https://developer.apple.com/documentation/authenticationservices/asauthorizationcontroller But I don't see in your example App.tsx that you are checking platform or whether it's available via https://github.com/expo/expo/blob/master/docs/pages/versions/unversioned/sdk/apple-authentication.md#appleauthenticationisavailableasync mostly I'm just trying to confirm if my understanding is correct If so, this is a great way to get react-native-firebase apps that use social auth through the App Store Review, but we'll need to expand it a bit to be universal (e.g., by back-filling Android + ios<13 support via some other mechanism) |
|
ok - well I am understanding it then - that's an obvious critical basis for moving forward :-). Thanks for replying on holiday! Agreed on android flow being radically different, that was my take as well This seems fine then to start, and via separate channel (the react-native-community Discord server) I pinged @brentvatne to see if he was interested in the nonce stuff as an expo-apple-signin patch, he'll respond here or there I'm sure and we'll see. Cheers! |
I also pinged @Salakar via react-native-firebase Discord chat server to check the gist and see what he thought. I'll PR here in react-native-firebase repo on v5 and v6 branch pending feedback. I don't like making changes to v5 but my commitment there is to make sure the branch is still useful, and I'm still on v5 and implementing social auth now so I need it :-) |
I have chatted with @brentvatne and:
This should be propposed functionality soon, I'll link related PRs here once expo-apple-signin has the change needed to build it on this side and test and @Salakar or @Ehesp has a look at the gist to make sure the RNFB part doesn't have a major issue Cheers! |
# Why invertase/react-native-firebase#2884 # How Based off of diff from https://gist.github.com/nhahn/010305dcff1404bfdeb6e2174b74e5a6 but less opinionated about the format of the nonce - we just pass the string along to the request. # Test Plan User has tested diff provided. I did not test this code.
@mikehardy is there any way to use |
Literally struggling with this same thing myself, existentially, while also having respect for the expo team. I just don't have unimodules yet and do I want to add it? Not particularly as I'm not sure how it's going to affect my plist with permissions etc (I think they merged a change for that but it's just not on my radar really whereas I've already perfected my react-native-permissions usage.) So, basically no, not without a fork. I'm going to personally use it just for now because I need to launch social auth for iOS, but I also need a cross-platform apple signin solution and expo-apple-signin isn't that whereas an internal react-native-firebase implementation - using the firebase SDK APIs which are pretty rich for apple auth on android - could work. I'd add horsepower to that once the time is right on react-native-firebase v6 (a week or three?) but need to move now. Pragmatism with an eye towards the future. |
@jonstuebe if I'm not mistaken, you tried a different more focused library (I see you github-forked it anyway). And it looked tempting. Any results to report? I have expo-apple-authentication more or less integrated but it's beastly and requires me to carry like 20 new patch-package patches. |
so only login in with email and password is ok if i get it right ? all the rest would get rejected? |
Based on the other replies yes. I almost have a version 2 of the expo-apple version working locally (changed to conform to final form of the expo-apple PR + w/typescript support in react-native-firebase) There will be a standalone react-native-firebase compatible version shortly as well. This is the current "stop work, do this" priority for me. I'll post a gist of what I'm using shortly, based on the gist from @nhahn |
For anyone following along, I updated the react-native-firebase@5.5.6 patch in my gist https://gist.github.com/mikehardy/83c9535d71cec4a8764bfda5a492c25f - I had neglected to carry the nonce-related changes to the android native side but when I went to test that today obviously it failed. I now have this submitted to the App Store, we'll see. Passes all my testing android+iOS at least. |
@Salakar - i was just joking, firebase is great even though it does tons of stuff :P that's one of the selling features! @mikehardy - appreciate the feedback. we're going to look at documenting the minimal unimodule setup and making the minimal setup requirements as lean as possible for people that are more hesitant to install react-native-unimodules itself in their project. |
App based on my gist is Apple approved as of today, with Facebook Google and Apple sign in. Share and enjoy until there is a formal release with the necessary changes |
Hey everyone, we've just released invertase/react-native-apple-authentication for this, its up to the usual standard for our libraries. The feature still needs implementing in RN Firebase Auth, which will be done this week for a v6.2.0 release (we can do this without any breaking changes and will revist the API in v7), but if you want to try it out now for v6 then I've made an example here with a v6.1.0 patch file included;
If you want to remain with the expo one + unimodules then that also works too, our one is aimed at being a pure React Native module with no added extras as we don't use unimodules. If you're using unimodules already for other stuff then it probably makes sense to stick with it. |
Hey all, v6.2.0 release is out with this in. I've also written up a guide for integrating this with |
@Salakar I have a problem. More details... I didn't know where to put the error, link here, sorry about anything. |
Still polishing up my work project before I make a PR to react-native-firebase v5.x.x but I confirm that if you use the 'react-native-firebase@5.5.6' patch from my gist above (and linked here https://gist.github.com/mikehardy/83c9535d71cec4a8764bfda5a492c25f) it integrates fine with The PR will actually even be simpler as @Salakar shows in his v6 support released in 6.2.0 that instead of plumbing the nonce all the way through I can just use the otherwise unused 'secret' field: https://github.com/invertase/react-native-firebase/pull/2979/files |
My PR for the v5.x.x branch is up as #2980 - our plan is to merge it and actually do a release. It will likely be a v5.6.0 because it requires react-native 0.60 and higher in order to get Xcode 11 support, which is how iOS 13 is supported. That's a hard requirement for apple sign-in so if you want it, there's no way around it. In the meantime people can depend on this commit hash if they want it in their projects, and just follow the firebase instructions from invertase/react-native-apple-authentication (with only change being a different way to import firebase for v5 vs v6) and it works I won't be commenting here anymore, everything's out there to make it work I believe, and you can subscribe in the relevant places if you need status package.json dependency until merged/published: "react-native-firebase": "git+https://github.com/mikehardy/react-native-firebase.git#c3d73b95892b9d65b485e141780e9d824c3730f6", |
Is this done for Android as well ? |
@damandeepsingh93 no it is not. The two solutions I'm aware of (expo or invertase apple auth for react native) are both iOS only. In my experience (my own app's user base) there is almost no value in implementing it at all (except Apple says you have to), and for android in particular zero, so I am certainly not motivated at least, vs other things on my roadmap. Any support would have to come from the community as PRs |
Since I can't get react-native-firebase v6 to work properly no matter what I try, I'll give @mikehardy 's solution a shot. Thanks! |
@skizzo it's all merged in now, works pretty much out of the box with RNFBv5.6.x and the invertase react-native-apple-authentication package, it's what I'm still using (haven't had the time to upgrade to RNFBv6 yet 😮 ) |
It does indeed work with |
Looks like Firebase now supports sign in with apple:
Would be great to support that here and update the docs! Let me know if there's anything we can do to help 😄
The text was updated successfully, but these errors were encountered: