Skip to content
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

Release & Debug version of the app asking keychain permissions #8811

Closed
ollyde opened this issue May 28, 2022 · 64 comments · Fixed by #9531
Closed

Release & Debug version of the app asking keychain permissions #8811

ollyde opened this issue May 28, 2022 · 64 comments · Fixed by #9531
Assignees
Labels
platform: macos Issues / PRs which are specifically for MacOS. plugin: core plugin: messaging resolution: fixed A fix has been merged or is pending merge from a PR. type: bug Something isn't working

Comments

@ollyde
Copy link

ollyde commented May 28, 2022

Related to firebase/firebase-ios-sdk#5540

When launching the app customers are reporting that they are asking for sensitive key-chain access.

We had to include Firebase Core in order to get FCM working..

Anyway around this? It's not acceptable to ask 8 times for Keychain access everytime the customer launches and many refuse to use "Always allow" which is completely reasonable.

@google-oss-bot
Copy link

I found a few problems with this issue:

  • I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.
  • This issue does not seem to follow the issue template. Make sure you provide all the required information.

@paulb777
Copy link
Member

Sorry about the trouble.

@ollyde
Copy link
Author

ollyde commented May 29, 2022

We had to disable it and resort to Web-sockets and local notifications for MacOS. It's fairly strange for the plugin to ask for chain access 8 times, feels very wrong.

Flutter 3.0.1

firebase_core: ^1.17.0
firebase_messaging: ^11.4.0

@paulb777
Copy link
Member

This might be a Flutter specific issue, since we haven't had seen this issue with ObjC/Swift apps. I'll transfer the issue.

@paulb777 paulb777 transferred this issue from firebase/firebase-ios-sdk May 31, 2022
@ollyde
Copy link
Author

ollyde commented May 31, 2022

@paulb777 disabling the library stops with Platform.isMac stops it asking for keychain access. We're still compiling to MacOS though.

@paulb777
Copy link
Member

One more thought since the issue sounds similar to a development-only workflow we've seen for macOS. How is the app distributed? We would expect App Store signing to obviate the need for any keychain permission popups.

@ollyde
Copy link
Author

ollyde commented Jun 1, 2022

@paulb777 yes I originally thought that but release mode and signing made no difference. When released via the App Store the MacOS app was asked for keychain 8 times on launch.

@darshankawar darshankawar added triage Issue is currently being triaged. and removed api: messaging labels Jun 1, 2022
@darshankawar
Copy link

@OllyDixon
If I understand correctly, the keychain permission is being asked for macOS after adding firebase_core / firebase_messaging plugin ?

@darshankawar darshankawar added the blocked: customer-response Waiting for customer response, e.g. more information was requested. label Jun 1, 2022
@ollyde
Copy link
Author

ollyde commented Jun 1, 2022

@darshankawar indeed. Multiple times on every launch. Something that is not acceptable for a user journey.

@google-oss-bot google-oss-bot added Needs Attention This issue needs maintainer attention. and removed blocked: customer-response Waiting for customer response, e.g. more information was requested. labels Jun 1, 2022
@darshankawar
Copy link

@darshankawar darshankawar added blocked: customer-response Waiting for customer response, e.g. more information was requested. and removed Needs Attention This issue needs maintainer attention. labels Jun 1, 2022
@google-oss-bot google-oss-bot added the Stale Issue with no recent activity label Jun 10, 2022
@google-oss-bot
Copy link

Hey @OllyDixon. We need more information to resolve this issue but there hasn't been an update in 7 weekdays. I'm marking the issue as stale and if there are no new updates in the next 7 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!

@ollyde
Copy link
Author

ollyde commented Jun 10, 2022

@darshankawar sorry for the delay. We removed firebase in this project and are using an alternative push notifications system because of this bug.

I’m sure the sample app will give the same bug for MacOS :-)

@google-oss-bot google-oss-bot added Needs Attention This issue needs maintainer attention. and removed blocked: customer-response Waiting for customer response, e.g. more information was requested. Stale Issue with no recent activity labels Jun 10, 2022
@maheshmnj
Copy link
Member

@OllyDixon Incase you have more info for us regarding how to reproduce the issue, feel free to write back.

@TammiLion
Copy link

TammiLion commented Aug 17, 2022

Made a sample project and managed to reproduce it. Some (for me easily overlooked steps). In Xcode setting the signing to your own signing (so not Sign to run Locally). Adding the Firebase.configure() into the AppDelegate.swift (if this is wrong please let me know). In our project we use Firebase Crashlytics and Firebase Messaging and I saw the same for @maheshmnj so I also added it in the pubspec. I made a video of the entire process but the interesting stuff only happens at the end of the video (besides uploading the repo to git :p oh no google api secret exposed!!!).

Repo with sample project: https://github.com/TammiLion/macos_keychain_firebase_attempt2

Here is the google drive link to the 40+ min video. https://drive.google.com/file/d/1oXZEAc0U7abGL2Fe1ygDUPUXf-WOFVBL/view?usp=sharing
(Google drive is processing it, so it might not be available immediately)
The "interesting" stuff start around the 33 minute mark. This is the point where I realize I also can't reproduce it immediately just from a sample project. I then read the log and notice some firebase errors (can't find GoogleService-Info.plist and missing Firebase.configure in AppDelegate) I then open the XCode workspace of the project in which I can reproduce it and start following the steps to add the GoogleService-Info.plist and the Firebase.configure() to the AppDelegate.swift.

You might ask yourself, why add those things when they are not in the MacOS installation documentation (flutter documentation without MacOS specifics - https://firebase.google.com/docs/flutter/setup?platform=ios#next_steps).
(flutter documentation that is archived but does mention MacOS - https://firebase.flutter.dev/docs/manual-installation/macos/)
While it is not there it does show in the logs and is listed as a step when in the Firebase Console you go to Project Overview > Select MacOS > See SDK Instructions > Step 4 "Add Initialization Code". Which is the kind of stuff you start doing when you are having trouble with getting Firebase up and running. So maybe it should not be done! Just let me know :D

Schermafbeelding 2022-08-17 om 19 15 20

Schermafbeelding 2022-08-17 om 13 59 09

Schermafbeelding 2022-08-17 om 19 14 32

@russellwheatley
Copy link
Member

@TammiLion Thank you for sharing your setup. The video is only 26 seconds long, but I didn't need to watch it anyway.

You do not need to add FirebaseApp.configure() to your AppDelegate. The firebase_core plugin already takes care of configuration here.

@tommienu did you also add FirebaseApp.configure() to your AppDelegate?

@tommienu
Copy link

@tommienu did you also add FirebaseApp.configure() to your AppDelegate?

Nope, here's my AppDelegate.swift:

import Cocoa

@NSApplicationMain
class AppDelegate: FlutterAppDelegate {
  override func applicationShouldTerminateAfterLastWindowClosed(_ sender: NSApplication) -> Bool {
    NSApp.setActivationPolicy(.accessory)
    return false
  }
}

I believe I've configured this according to the documentation.

@tommienu
Copy link

tommienu commented Aug 17, 2022

FWIW, this sounds like a regression from this old issue from 2020. Which was fixed in Firebase release 6.26.0, according to this comment.

Understanding what changed there might assist with understanding why it's now resurfacing.

Edit: Please notice this post @ncooke3. I believe this will assist while debugging firebase/firebase-ios-sdk#9392.

@GroovinChip
Copy link

Folks, I have been following this conversation closely over the last few days and I'm super confused as to whether there is a resolution for this issue or not. @russellwheatley seems to feel that this is a user configuration issue, yet @TammiLion is able to reproduce the issue, and @tommienu thinks there is a regression. Do we know what's going on here yet or is this still an ongoing investigation?

@TammiLion
Copy link

TammiLion commented Aug 17, 2022

@russellwheatley thanks for the answer. I removed the Firebase.configure() from the AppDelegate.swift in the sample project (also committed it to git). Unfortunately the problem still occurs.

I also tested with App sandbox allowed/disallowed and it made no difference.

Here's the actual video (once again, might not be immediately available due to drive processing it, will also edit the old comment): https://drive.google.com/file/d/1oXZEAc0U7abGL2Fe1ygDUPUXf-WOFVBL/view?usp=sharing

@ncooke3
Copy link
Member

ncooke3 commented Aug 18, 2022

Hi everyone,

Apologies for the trouble here and thank you for all the useful information. The firebase-ios-sdk team is aware of the issue and we have been actively working on a fix to resolve this. This issue has popped up in a few GitHub issues so I'm going to centralize communications in firebase-ios-sdk/#9392, but I will also update this thread when I have a more substantial update.

I will share another update soon!

@darshankawar darshankawar added blocked: firebase-sdk and removed blocked: customer-response Waiting for customer response, e.g. more information was requested. labels Aug 18, 2022
@ncooke3
Copy link
Member

ncooke3 commented Sep 6, 2022

Hi everyone, I have an update here.

We have a fix that is currently planned for release in Firebase 9.6.0 next week. Thanks!

@ncooke3
Copy link
Member

ncooke3 commented Sep 13, 2022

Hi everyone,

Just a quick fyi that Firebase 9.6.0 has been released. This release should unblock this issue @darshankawar.

Please continue the conversation here or in firebase-ios-sdk/#9392 if the issue persists.

As noted in this comment and in the release notes, please enable the Keychain Sharing capability for your macOS targets to make sure everything works properly.

Thanks!

@darshankawar
Copy link

@OllyDixon Please check above comment and re-verify to confirm.

@darshankawar darshankawar added the blocked: customer-response Waiting for customer response, e.g. more information was requested. label Sep 14, 2022
@ollyde
Copy link
Author

ollyde commented Sep 14, 2022

@ncooke3 do customers need to enable keychain sharing for their apps? As that won’t be an option. I don’t see other MacOS apps requesting this; and if they do I reject.

Apple will reject our apps without valid reason for keychain access.

What is the reason for us enabling it? I don’t see the point and it’s not explained. Non of our apps ‘SDKs’ use the keychain ..

If this is the case the original issue is not resolved. Apple provides a very simple push notifications service/key like on iOS; why is Firebase not utilizing this? Why is it using the keychain in the first place?

@google-oss-bot google-oss-bot added Needs Attention This issue needs maintainer attention. and removed blocked: customer-response Waiting for customer response, e.g. more information was requested. labels Sep 14, 2022
@ncooke3
Copy link
Member

ncooke3 commented Sep 15, 2022

Hi @OllyDixon

Thank you for the questions. I have answered each one below.

do customers need to enable keychain sharing for their apps?

  1. A developer building a Mac app using Firebase needs to do two things to ensure their app’s users do not see keychain access prompts:
    • Use a Firebase version equal or greater than 9.6.0
    • Add the Keychain Sharing capability to the provisioning profile they use to sign their app

I don’t see other MacOS apps requesting this; and if they do I reject.

  1. I understand your concern. It is likely the case that such Mac apps either do not access the keychain or use the solution introduced in Firebase 9.6.0.

Apple will reject our apps without valid reason for keychain access.

  1. There is a reason that the Keychain Sharing capability needs to be added. Firebase uses the keychain to store SDK data. For example, Firebase Installations is an
    SDK that is depended on by other Firebase products. Firebase Installations manages Installation IDs, which map to an installed instance of a Firebase app. The implementation of this SDK is shared across platforms. At the time of its implementation, the Keychain was chosen to store these unique identifiers.

What is the reason for us enabling it? I don’t see the point and it’s not explained. Non of our apps ‘SDKs’ use the keychain ..

  1. Please refer to these places for additional documentation regarding this issue:

    Additionally, the Firebase team is planning on moving away from using the keychain as a storage container where possible. It will take some work to do this, so it would be targeted for a future Firebase 10.x.0 release. For now, the fix in Firebase 9.6.0 will unblock your Mac app(s) from prompting users for permission to access the keychain. All you need to do is upgrade and add the Keychain Sharing capability to the provisioning profile you use to sign your Mac app(s).

Apple provides a very simple push notifications service/key like on iOS; why is Firebase not utilizing this?

  1. I understand, but custom solutions are sometimes needed to reach the desired functionality. And for what it's worth, Firebase Messaging does make use of APNs.

Why is it using the keychain in the first place?

  1. Please refer to answer 4.

Please let me know if you have further questions.

@ollyde
Copy link
Author

ollyde commented Sep 15, 2022

@ncooke3

Thanks for the info.

There is a reason that the keychain Sharing capability needs to be added. Firebase uses the keychain to store SDK data. For

Why not use library folder/cache or app folder for this. Storing it inside the key-chain seems a bit strange? You're asking us expose sensitive data, our key-chains might contain passwords and other items that we don't want to expose to any SDK's inside our app.

Apple will reject our apps without valid reason for keychain access.

If we get an awkward reviewer which sometimes happens, we still have no reason to use the keychain. Our answer can't be 'cause some SDK inside our app uses it' they won't accept that.

@ncooke3
Copy link
Member

ncooke3 commented Sep 15, 2022

Hi @OllyDixon,

Why not use library folder/cache or app folder for this.

Firebase uses the keychain to store various bits of SDK data. For example, Firebase Auth allows you to manage user accounts. Account information is a good example of something that the Keychain is designed to store. Storing such information in the keychain is done to protect such information.

When SDKs are designed, a storage solution is chosen based on the requirements at that time. That said, we are working on reviewing our use of the Keychain, and if we find that the Keychain's security is not needed, then we will migrate away as necessary. We will be working on this and any resulting changes can be expected in a Firebase 10.X.0 release.

Storing it inside the key-chain seems a bit strange?

The Keychain can store all sorts of data. And it has its own trade-offs just like any other storage solution (File System, User Defaults, database, etc.).

While I was not involved in its design, I believe Firebase Installation tokens were stored in the keychain rather than an unprotected storage container out of an abundance of caution. And as I said in my previous answer, it's worth us examining whether or not the SDK needs to be that defensive. From an implementation and testability perspective, the Keychain can be difficult to deal with because of its API surface. So there are multiple benefits to not using the Keychain (as long as the data does not pose a risk by being stored in an unsecure container).

You're asking us expose sensitive data, our key-chains might contain passwords and other items that we don't want to expose to any SDK's inside our app.

Nobody is asking that. Just because an SDK uses the keychain does not mean that it can access such items. Each Keychain items has a dictionary of attributes that are unique to that item. These are defined by you, the developer, and Firebase has no idea what those attributes are. It is only aware of the Keychain items that it creates.

I'm happy to point you to code in the Firebase SDK that accesses the keychain. Everything is here.

If we get an awkward reviewer which sometimes happens, we still have no reason to use the keychain. Our answer can't be 'cause some SDK inside our app uses it' they won't accept that.

I understand. I think the justification can be more detailed as this is ultimately something that is trying to improve your app's user experience. Firebase Installations provides a table that provides a justification for why the SDK is used for each product. For instance, if you are using Firebase Messaging, it uses Installations to target devices for message delivery. The keychain is used to securely store the ID used to target that particular device.


Apologies if you are still not convinced. The fix in Firebase 9.6.0 was intended to quickly unblock Firebase developers targeting macOS apps. I will have more info in the coming weeks regarding what Firebase SDKs may be updated to move away from using the Keychain.

@ollyde
Copy link
Author

ollyde commented Sep 16, 2022

@ncooke3

Thanks for the reply.

Still not convinced; have many more questions and security pokes; but I won’t take anymore time.

Really it’s a shame Google can’t give us a simple messaging service without using Firebase (which was GCM on Android and APN on Apple). Forcing developers to go though Firebase to increase sales; seems antitrust. Us developers always get screwed by this mobile duopoly 🤪

For us the keychain issue is still a deal breaker and a risk not worth taking. That keychain code could change at anytime exposing users sensitive data.

We decided to go with our own solution on MacOS 😎

Thanks for your help.

@darshankawar darshankawar removed the Needs Attention This issue needs maintainer attention. label Sep 16, 2022
@GroovinChip
Copy link

GroovinChip commented Sep 25, 2022

@ncooke3 it seems like package:cloud_firestore is still using version 9.5.0 of the SDK. When can we expect to see an update that uses 9.6.0?

image

@darshankawar
Copy link

Check this PR : #9531

@GroovinChip
Copy link

GroovinChip commented Sep 27, 2022

@darshankawar that's great, but firebase_core needs to be published to pub with 9.6.0 because cloud_firestore depends on it from pub, so depending on firebase_auth and cloud_firestore from git and running pod install still brings in the 9.5.0 sdk. Really it would just be better if all packages were updated on pub. When can we expect this to happen?

@darshankawar darshankawar added the resolution: fixed A fix has been merged or is pending merge from a PR. label Sep 28, 2022
@firebase firebase locked and limited conversation to collaborators Oct 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
platform: macos Issues / PRs which are specifically for MacOS. plugin: core plugin: messaging resolution: fixed A fix has been merged or is pending merge from a PR. type: bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.