App wants to use your confidential info stored in firebase_auth with macos flutter #10582
Comments
|
I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight. |
|
@ncooke3 no rush. But just curious if you are able to reproduce? Or if you have an ETA on when you can get to this? |
|
Hi @notandyvee, so if you build and run the app in release mode, does the issue also reproduce? |
|
Also, what is your macOS version? |
Just tried it. Interestingly it does not happen when running it in release mode with the firebase auth keychain created in the development mode. I guess thats not surprising as I noticed it also worked normally when in profile mode.
MacOS 12.6 |
|
Hmm interesting... Question, is your project configured in such a way that you can edit the source code in Firebase Auth? Specifically, |
|
I'm not gonna lie. I don't really fully understand the setup. Left it mostly as the flutter default. But I can see the file you are talking about. Just not in the same path as what you posted. So not sure if I can edit it. Do you have a specific root folder I can search in to look? |
|
No worries–– and that looks right. It should let you edit it (you might have to "unlock" it in a dialog box that pops up when you try to do so). If it does, locate the Replace it with the following: - (NSDictionary *)genericPasswordQueryWithKey:(NSString *)key {
NSMutableDictionary *query = @{
(__bridge id)kSecClass : (__bridge id)kSecClassGenericPassword,
(__bridge id)kSecAttrAccount : [kAccountPrefix stringByAppendingString:key],
(__bridge id)kSecAttrService : _service,
}.mutableCopy;
if (@available(iOS 13.0, macOS 10.15, macCatalyst 13.0, tvOS 13.0, watchOS 6.0, *)) {
query[(__bridge id)kSecUseDataProtectionKeychain] = (__bridge id)kCFBooleanTrue;
}
return [query copy];
}And then try reproducing the issue again. |
|
Sorry for the delay @ncooke3 . Pushed a new testflight build. The pop up is gone, but because now the existing keychain wasn't recognized. I verified that firebase_auth keychain exists. But had to re-login. When logging in the resulting keychain looks like this. Edit: How do I undo the change when doing things like this? After xcode is closed and re-open I mean. |
|
Sorry, what exactly do you mean by "but because now the existing keychain wasn't recognized."? I'm trying to determine if this helped or not. Did successive calls to the new keychain yield a pop up?
You can copy and paste the implementation from here: firebase-ios-sdk/FirebaseAuth/Sources/Storage/FIRAuthKeychainServices.m Lines 228 to 234 in c24031a Podfile.lock shows is being used.
|
|
Damn. Apologies for the wasted cycles here. It was not clear what was happening. So to be clear, the change you made should stop using the old keychain row, which will be of type "login" and my app should now be using the "iCloud" keychain? In essence a re-auth is expected? When I noticed that I stopped testing thinking it was not expected since it "appeared" wrong. Assuming my assumption above is correct, I tested a few scenarios with your test change on
Your method change did not yield pop ups when using builds in development and testflight. When updating a testflight build no popups occur either. It's like they are now sharing the same keychain properly. This is great news. Thank you. Now whats the next steps here for this fix? Is it safe to use this code temporarily to avoid the popups? Or should I wait for a proper prod fix? Again, extremely appreciative in such quick turn around here 🙏🏻 . |
|
No worries– macOS keychain debugging is pretty confusing!
Yes, the new flag will use the data protection keychain rather than the traditional file based keychain. This is the only way that I know of to make the macOS keychain behave like the iOS one. Apple released a good tech note on this recently that helped fill in the gaps of my keychain knowledge so check it out if you're interested. There are two inevitable drawbacks worth calling out with the updated implementation:
I'll check with the Auth team after the holidays and move to getting this out in a release! In the meantime, I think it's safe to use this local edit to unblock yourself–– all of Firebase's other keychain usage now use this new attribute (I must have missed this particular API surface). I would just do some extra manual testing with the test flight builds to ensure auth is functioning as expected after updating (apart from the need to re-auth). Let me know if you notice any interesting behavior! |
Thanks for the link. Does actually help in understanding why this was a problem.
That's a great question. The setup I have should automatically respond to auth changes. I don't log users out automatically, so it would have to be in response to the auth SDK callback. Definitely confirm with the auth team. I can take the one time hit as I'm waiting to release so it doesn't bother me at the moment.
Question about that. So I'm cool doing that. What happens when I update the firebase libs before this fix is included? Would those files be deleted and a fresh copy from the release be used? Meaning, would I need to manually edit Coming from the Android world, editing libraries like that is magical haha. Have no idea what to expect. |
Yep– if you update the Firebase libs before the fix is scheduled, those files will be deleted and a fresh copy from the release will be cached for your app's xcworkspace. So, you're correct that you will have to manually edit
For reasoning above, no – the local change would not persist. |
|
We've been seeing this issue in a TestFlight downloaded Flutter iOS app to an M1 MacOS. Can see in Podfile.lock that we're on 10.3.0. The error exposed is: Just wanted to give a heads up. |
|
This thread solved it for us: google/GoogleSignIn-iOS#165 Specifically, adding a capability of "Keychain Sharing", and then a key with Ps. The code in #10582 (comment) didn't do anything to fix this specific issue. |
|
@larssn the issue you are referencing seems similar but unrelated at first glance. I had also turned on keychain sharing but my issue is not a google sign in issue. Does this mean if I use google sign in I need to add the keychain sharing capability with |
|
Tbh I haven't looked into it in detail. We're two people who need TestFlight access on iOS to our app, and all I concluded was that the above fixed it. You might be right that it's an entirely different issue. |
|
Hi @notandyvee, checking in to see if the original solution is working as expected? |
|
Hey @notandyvee. We need more information to resolve this issue but there hasn't been an update in 5 weekdays. I'm marking the issue as stale and if there are no new updates in the next 5 days I will close it automatically. If you have more information that will help us get to the bottom of this, just add a comment! |
|
Sorry for the late response @ncooke3 .
Yes. Your solution fixed the issue I was having when updating the method as you instructed. |
|
Hi @notandyvee, apologies for the delay. I have since staged this fix in the next release, which should release sometime during the week of February 7th. Thanks for reporting the issue and don't hesitate to reach out on this thread if you run into anymore trouble! |
Description
As the title suggests, when I first submitted my app to testflight I noticed the dialog below. I found this issue on the firebase ios sdk repo. It should have been fixed. What I've done:
My observations.
I was hestitant to post this here. But since a previously opened issue is also happening to me I figured it makes sense.
Any help avoiding this keychain issue on update? My fear is every time a user updates the app in production they will be prompted to enter their keychain info. Currently I am testing with testflight, which should be a final build anyways.
Reproducing the issue
Firebase SDK Version
10.0.0
Xcode Version
13.4.1
Installation Method
CocoaPods
Firebase Product(s)
Authentication
Targeted Platforms
macOS
Relevant Log Output
No response
If using Swift Package Manager, the project's Package.resolved
No response
If using CocoaPods, the project's Podfile.lock
Expand
Podfile.locksnippetThe text was updated successfully, but these errors were encountered: