Skip to content

[google_maps_flutter] update ios privacy manifest #6511

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

Merged
merged 9 commits into from
Apr 16, 2024

Conversation

rrpadilla
Copy link
Contributor

@rrpadilla rrpadilla commented Apr 13, 2024

The installation instructions for Google Maps Platform SDKs for iOS now have download links and instructions for manually adding Privacy Manifest files for use with the latest versions of the SDKs. You can read more about this here.

This PR synchronizes the iOS privacy manifest with the file provided by Google for the Maps SDK for iOS.
The privacy manifest was downloaded from: https://developers.google.com/maps/documentation/ios-sdk/config#add-apple-privacy-manifest-file.

As you can see in the new PrivacyInfo.xcprivacy file Google Maps SDK is using the following NSPrivacyAccessedAPITypes:

  • NSPrivacyAccessedAPICategoryDiskSpace
  • NSPrivacyAccessedAPICategorySystemBootTime
  • NSPrivacyAccessedAPICategoryFileTimestamp
  • NSPrivacyAccessedAPICategoryUserDefaults

Related issues:

Fixes flutter/flutter#94491

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@rrpadilla rrpadilla requested a review from cbracken as a code owner April 13, 2024 06:48
@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact "@test-exemption-reviewer" in the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Copy link
Member

@cbracken cbracken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're going to do a little digging to figure out why the Maps SDK team is requesting these steps: https://developers.google.com/maps/documentation/ios-sdk/config#add-apple-privacy-manifest-file

@jmagman
Copy link
Member

jmagman commented Apr 15, 2024

@rrpadilla Did you get an App Store warning about the GoogleMaps frameworks specifically? And if you make the google_maps_flutter manifest changes in this PR and resubmit your app, the warning goes away? I wouldn't expect the App Store validator would accept the google_maps_flutter manifest as a proxy for an embedded GoogleMaps framework, but I may be misunderstanding the mechanism...

@stuartmorgan-g
Copy link
Contributor

@rrpadilla Did you get an App Store warning about the GoogleMaps frameworks specifically?

Capturing from discussion elsewhere: it looks like the vendored frameworks in the Pod are static frameworks, and then we're also forcing static build in google_maps_flutter (I'm not sure why offhand; maybe because of the Info.plist key lookup?), so by the time it gets to the App Store validator it's all in Runner.

Copy link
Contributor

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we do move forward with this (it's error-prone in the long term, and the static frameworks really should be including this information directly), we will need to make updating the SDK version part of this PR. According to the docs:

Important: These instructions for the Apple Privacy Manifest File apply only to the latest SDK version.

That will also mean changing the min supported iOS version in our podspec.

We will need to do that regardless though, because older versions won't be compliant with the new rules anyway.

@rrpadilla
Copy link
Contributor Author

@rrpadilla Did you get an App Store warning about the GoogleMaps frameworks specifically? And if you make the google_maps_flutter manifest changes in this PR and resubmit your app, the warning goes away? I wouldn't expect the App Store validator would accept the google_maps_flutter manifest as a proxy for an embedded GoogleMaps framework, but I may be misunderstanding the mechanism...

The first time I submitted the app to apple store connect I get an email about ITMS-91053: Missing API declaration, specifically the 4 listed below.

NSPrivacyAccessedAPICategoryFileTimestamp
NSPrivacyAccessedAPICategorySystemBootTime
NSPrivacyAccessedAPICategoryUserDefaults
NSPrivacyAccessedAPICategoryDiskSpace

I created a privacy file as recommended here and here. This is the privacy file I uploaded and the warning goes away. Basically I just copied the NSPrivacyAccessedAPITypes entries from the privacy manifest provided by google here.

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
    <key>NSPrivacyTracking</key>
    <false/>
    <key>NSPrivacyTrackingDomains</key>
    <array/>
    <key>NSPrivacyCollectedDataTypes</key>
    <array/>
    <key>NSPrivacyAccessedAPITypes</key>
	<array>
		<dict>
			<key>NSPrivacyAccessedAPIType</key>
			<string>NSPrivacyAccessedAPICategoryDiskSpace</string>
			<key>NSPrivacyAccessedAPITypeReasons</key>
			<array>
				<string>85F4.1</string>
			</array>
		</dict>
		<dict>
			<key>NSPrivacyAccessedAPIType</key>
			<string>NSPrivacyAccessedAPICategoryFileTimestamp</string>
			<key>NSPrivacyAccessedAPITypeReasons</key>
			<array>
				<string>C617.1</string>
			</array>
		</dict>
		<dict>
			<key>NSPrivacyAccessedAPIType</key>
			<string>NSPrivacyAccessedAPICategorySystemBootTime</string>
			<key>NSPrivacyAccessedAPITypeReasons</key>
			<array>
				<string>35F9.1</string>
			</array>
		</dict>
		<dict>
			<key>NSPrivacyAccessedAPIType</key>
			<string>NSPrivacyAccessedAPICategoryUserDefaults</string>
			<key>NSPrivacyAccessedAPITypeReasons</key>
			<array>
				<string>1C8F.1</string>
			</array>
		</dict>
	</array>
</dict>
</plist>

How do I know Google Maps is causing this warning?

I build a flutter app with the google_maps_flutter plugin only and when I run the commands listed below (recommended by @stuartmorgan here). Then I confirmed the GoogleMapsPrivacy.bundle was using those entries.

nm build/ios/Release-iphoneos/Runner.app/Runner | grep mach_absolute_time
nm build/ios/Release-iphoneos/Runner.app/Runner | grep systemUptime
nm build/ios/Release-iphoneos/Runner.app/Runner | grep NSFileCreationDate
nm build/ios/Release-iphoneos/Runner.app/Runner | grep NSFileModificationDate
nm build/ios/Release-iphoneos/Runner.app/Runner | grep fileModificationDate
nm build/ios/Release-iphoneos/Runner.app/Runner | grep NSURLContentModificationDateKey
nm build/ios/Release-iphoneos/Runner.app/Runner | grep NSURLCreationDateKey
nm build/ios/Release-iphoneos/Runner.app/Runner | grep getattrlist
nm build/ios/Release-iphoneos/Runner.app/Runner | grep fstatat
nm build/ios/Release-iphoneos/Runner.app/Runner | grep "stat$"

As you can see the Google Maps Flutter iOS privacy file is empty.

@stuartmorgan-g
Copy link
Contributor

As you can see the Google Maps Flutter iOS privacy file is empty.

Which should be correct, because the manifest for the plugin should only describe what the code in the plugin does, and its dependencies should have their own manifests that describe what they do. These Google Maps SDK instructions are very unusual.

@rrpadilla
Copy link
Contributor Author

Which should be correct, because the manifest for the plugin should only describe what the code in the plugin does, and its dependencies should have their own manifests that describe what they do. These Google Maps SDK instructions are very unusual.

@stuartmorgan What is your recommendation?

@stuartmorgan-g
Copy link
Contributor

My recommendation is that we wait for this to land anything.

@stuartmorgan-g
Copy link
Contributor

While we wait for that, feel free to update the PR to change the podspec as described in my earlier comment, as that will need to be done for any version of a PR. That will also involve changing the directories in example to remove iOS versions that are no longer applicable, and moving their native tests forward.

@stuartmorgan-g
Copy link
Contributor

That will also involve changing the directories in example to remove iOS versions that are no longer applicable, and moving their native tests forward.

Actually, I can go ahead and do these changes and push them to the PR. They are bit tricky to get right.

@rrpadilla
Copy link
Contributor Author

That will also involve changing the directories in example to remove iOS versions that are no longer applicable, and moving their native tests forward.

Actually, I can go ahead and do these changes and push them to the PR. They are bit tricky to get right.

@stuartmorgan Thanks.

Maybe we have to do this in the podspec file.

  # Version 8.4 and the versions up to 9.0, not including 9.0 and higher.
  s.dependency 'GoogleMaps', '~> 8.4'
  s.static_framework = true
  s.platform = :ios, '15.0'

@stuartmorgan-g
Copy link
Contributor

Based on offline conversation, we've decided to go ahead with incorporating the stand-alone privacy bundle's manifest into the plugin's manifest for now. The expectation is that future versions of the Google Maps SDK will have it built in as expected, at which point we can unwind the workaround†.

I've pushed the following changes:

  • Updated the podspec to require 8.4, and correspondingly iOS 14.
    • I've updated the CHANGELOG accordingly, and also made it a minor version bump since this is now much more than a bugfix change. Arguably it should be a major version bump since it can break people's builds, but Flutter 3.16 will give a clear error message explaining why and what the next steps are, and since earlier versions of the plugin won't be compatible with the App Store starting on May 1st anyway, the incremental impact of the breakage is, IMO, small. So I'm not doing a major version bump since that would just make the plumbing more complicated for us, and actually make it harder for people to get the fix they will absolutely need.
  • I've removed the no-longer-useful ios12 and ios13 example directories, and folded the tests from ios12 into ios14. (Technically I actually deleted ios14, renamed ios12, and then back-ported the minor changes into ios12 to minimize potential errors.)
    • I left the multi-OS-version example structure in place because we'll need it again soon; the SDK release notes have already indicated that sometime this year iOS 14 will be dropped, so we'll want to go back to supporting a range. (@cbracken feel free to set up a quick VC with me to explain the unusual test structure and the context around it.)
  • Since we are no longer supporting older versions of the SDK, I updated the code that was doing dynamic symbol lookup to get new features to just use them normally, and resolved a TODO about a deprecated API.
  • Since we no longer need it, I removed the arm64 simulator workaround, fixing [google_maps_flutter] Update minimum GoogleMaps dependency to a version that supports ARM simulators flutter#94491

(†If the version that includes the manifest is also the version that drops iOS 14, which may well be the case based on the timings listed in the SDK release notes, we'll need to think about whether we want to unwind the workaround at the cost of losing iOS 14 support.)

Copy link
Member

@cbracken cbracken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm modulo one comment.

@stuartmorgan-g
Copy link
Contributor

stuartmorgan-g commented Apr 16, 2024

@jmagman Do you want to sign off on this as well? (In particular, dropping iOS 12 and 13 support without a breaking version change, just the tooling error.)

Copy link
Member

@jmagman jmagman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I knew this day would eventually come. 🙂

LGTM!

@rrpadilla thanks for bringing the unusual GoogleMaps case to our attention.

@stuartmorgan-g stuartmorgan-g added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 16, 2024
@auto-submit auto-submit bot merged commit 18c4ca4 into flutter:main Apr 16, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 17, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Apr 17, 2024
flutter/packages@90c876d...d39830e

2024-04-16 [email protected] Manual roll Flutter from 2e748e8 to 3882afb (16 revisions) (flutter/packages#6549)
2024-04-16 [email protected] [image_picker] Add limit parameter to pickMultiImage and pickMultipleMedia to ios and Android (flutter/packages#6201)
2024-04-16 [email protected] [camera] Remove iOS thread-safe result class (flutter/packages#6498)
2024-04-16 [email protected] [google_maps_flutter] update ios privacy manifest (flutter/packages#6511)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
gilnobrega pushed a commit to gilnobrega/flutter that referenced this pull request Apr 22, 2024
flutter/packages@90c876d...d39830e

2024-04-16 [email protected] Manual roll Flutter from 2e748e8 to 3882afb (16 revisions) (flutter/packages#6549)
2024-04-16 [email protected] [image_picker] Add limit parameter to pickMultiImage and pickMultipleMedia to ios and Android (flutter/packages#6201)
2024-04-16 [email protected] [camera] Remove iOS thread-safe result class (flutter/packages#6498)
2024-04-16 [email protected] [google_maps_flutter] update ios privacy manifest (flutter/packages#6511)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
TecHaxter pushed a commit to TecHaxter/flutter_packages that referenced this pull request May 22, 2024
The installation instructions for Google Maps Platform SDKs for iOS now have download links and instructions for **manually** adding Privacy Manifest files for use with the latest versions of the SDKs. You can read more about this [here](googlemaps/google-maps-ios-utils#463 (comment)).

This PR synchronizes the iOS privacy manifest with the file provided by Google for the Maps SDK for iOS. 
The privacy manifest was downloaded from: [https://developers.google.com/maps/documentation/ios-sdk/config#add-apple-privacy-manifest-file](https://developers.google.com/maps/documentation/ios-sdk/config#add-apple-privacy-manifest-file).

As you can see in the new **[PrivacyInfo.xcprivacy](packages/google_maps_flutter/google_maps_flutter_ios/ios/Resources/PrivacyInfo.xcprivacy)** file Google Maps SDK is using the following **NSPrivacyAccessedAPITypes**:

- **NSPrivacyAccessedAPICategoryDiskSpace**
- **NSPrivacyAccessedAPICategorySystemBootTime**
- **NSPrivacyAccessedAPICategoryFileTimestamp**
- **NSPrivacyAccessedAPICategoryUserDefaults**

*Related issues:*
- [flutter/flutter/issues/145269](flutter/flutter#145269)
- [flutter/flutter/issues/143232](flutter/flutter#143232)
- [flutter/flutter/issues/131940](flutter/flutter#131940 (comment))

Fixes flutter/flutter#94491
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App p: google_maps_flutter platform-ios
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[google_maps_flutter] Update minimum GoogleMaps dependency to a version that supports ARM simulators
4 participants