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

feat: add privacy-manifest config support #1406

Merged
merged 10 commits into from
Mar 13, 2024

Conversation

knaito-asial
Copy link
Contributor

@knaito-asial knaito-asial commented Mar 6, 2024

Platforms affected

ios

Motivation and Context

To support setting ios privacy manifest according to config.xml setting.

Description

Introducing tag in config.xml ios platform directive.

If user add following directive in config.xml (platform ios)

<privacy-manifest>
	<key>NSPrivacyTracking</key>
	<true/>
	<key>NSPrivacyCollectedDataTypes</key>
	<array/>
	<key>NSPrivacyAccessedAPITypes</key>
	<array/>
	<key>NSPrivacyTrackingDomains</key>
	<array/>
</privacy-manifest>

the ios project PrivacyManifest.xcprivacy is updated as config.xml.
If user does not set <privacy-manifest> tag in config.xml, the cordova template
default PrivacyManifest.xcprivacy is used.

Note: This <privacy-manifest> tag overrides whole PrivacyManifest.xcprivacy file in project each time
user execute cordova prepare.

Testing

I locally checked this feature with sample project.
I locally exec npm run coverage.

Checklist

  • I've run the tests to see all new and existing tests pass
  • I added automated test coverage as appropriate for this change
  • Commit is prefixed with (platform) if this change only applies to one platform (e.g. (android))
  • If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct keyword to close issues using keywords)
  • I've updated the documentation if necessary

@erisu erisu requested review from dpogue and erisu March 6, 2024 08:01
Copy link
Member

@erisu erisu left a comment

Choose a reason for hiding this comment

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

The suggested changes aim to establish a standard filename convention for use across platforms. This would facilitate the process for anyone intending to create a new platform by copying an existing one and making modifications without the need for renaming files.

lib/ConfigParseriOS.js Outdated Show resolved Hide resolved
lib/ConfigParseriOS.js Outdated Show resolved Hide resolved
lib/prepare.js Outdated Show resolved Hide resolved
lib/prepare.js Outdated Show resolved Hide resolved
lib/ConfigParseriOS.js Outdated Show resolved Hide resolved
lib/prepare.js Show resolved Hide resolved
@breautek
Copy link
Contributor

breautek commented Mar 6, 2024

If user does not set tag in config.xml, the cordova template
default PrivacyManifest.xcprivacy is used.

Note: This tag overrides whole PrivacyManifest.xcprivacy file in project each time
user execute cordova prepare.

How does this work with plugins? Can a plugin use <privacy-manifest-ios> within the plugin.xml or will the application's config.xml overwrite plugin entries?

I don't want to introduce scope creep but I just want to make sure this was thought about. I think it would be ideal for plugins to be able to declare their privacy entries and the results will get merged into one file for the application. Otherwise it will be a painful process for plugin authors to document and hope end users follows instructions to add the necessary privacy entries.

@erisu
Copy link
Member

erisu commented Mar 7, 2024

How does this work with plugins? Can a plugin use <privacy-manifest-ios> within the plugin.xml or will the application's config.xml overwrite plugin entries?

The PR will solely focus on adding the minimum support for the privacy-manifest tag to the config.xml for app developers.

We aim to ensure that both Config.xml (for App Developers) and Plugin.xml (for Plugin Developers) support this tag and are completed and released before Apple's Privacy Manifest requirement takes effect. However, in the event that we are unable to complete the support for plugin.xml, at least the support for config.xml will be completed and merged with this PR.

I don't want to introduce scope creep but I just want to make sure this was thought about. I think it would be ideal for plugins to be able to declare their privacy entries and the results will get merged into one file for the application. Otherwise it will be a painful process for plugin authors to document and hope end users follows instructions to add the necessary privacy entries.

As you mentioned, we're aiming to keep the scope specific for this PR. However, the ultimate goal aligns with your idea of gathering all privacy-manifest tag values and merging them into one file for the application.

lib/PlatformConfigParser.js Outdated Show resolved Hide resolved
lib/PlatformConfigParser.js Outdated Show resolved Hide resolved
@erisu erisu changed the title Config privacy manifest feat: add privacy-manifest config support Mar 7, 2024
Copy link
Member

@dpogue dpogue left a comment

Choose a reason for hiding this comment

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

Formatting and code style comments aside, the overall implementation looks good to me 👍

tests/spec/unit/prepare.spec.js Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 96.15385% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 78.50%. Comparing base (af6335e) to head (e655949).

Files Patch % Lines
lib/prepare.js 95.65% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1406      +/-   ##
==========================================
+ Coverage   78.24%   78.50%   +0.25%     
==========================================
  Files          15       16       +1     
  Lines        1788     1814      +26     
==========================================
+ Hits         1399     1424      +25     
- Misses        389      390       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@erisu
Copy link
Member

erisu commented Mar 12, 2024

I tested the following steps:

% cordova create cordovaTest com.erisu.cordovaTest cordovaTest && $_
% cordova platform add ../../cordova-ios-7.1.0-dev.tgz
% cordova build ios
% cordova run ios

I confirmed the following were successful:

  • Adding platform (CLI)
  • Building (CLI)
  • Deploy to Simulator (CLI)
  • Building (Xcode)
  • Deploy to Simulator (Xcode)

I confirmed that the PrivacyInfo manifest exists in the app's scope, within Xcode.

Default PrivacyInfo Settings:

xcode-privacyinfo

I added the following configuration snippet to projects config.xml

    <platform name="ios">
        <privacy-manifest>
            <key>NSPrivacyTracking</key>
            <true/>
            <key>NSPrivacyCollectedDataTypes</key>
            <array/>
            <key>NSPrivacyAccessedAPITypes</key>
            <array/>
            <key>NSPrivacyTrackingDomains</key>
            <array/>
        </privacy-manifest>
    </platform>

I prepared the project with the following command:

% cordova prepare

I confirmed within Xcode that the PrivacyInfo manifest content was updated and reflected the change.

Updated PrivacyInfo Settings after Cordova Prepare:

xcode-privacyinfo-after-change

I confirmed the following were successful with the changes in config.xml:

  • Preparing project (CLI)
  • Building (CLI)
  • Deploy to Simulator (CLI)
  • Building (Xcode)
  • Deploy to Simulator (Xcode)

Important Notice

  • This PR does not handle setting PrivacyInfo from plugin.
  • It does not handle merging of config.xml with of template defaults.
  • It replaced the entire PrivacyInfo Manifest with the content defined in config.xml, if set.

E.g. of the last item:

Settings in config.xml

    <platform name="ios">
        <privacy-manifest>
            <key>NSPrivacyTracking</key>
            <false/>
        </privacy-manifest>
    </platform>

Running: cordova prepare

Generates the following:

xcode-privacyinfo-after-change-with-missing-items

Noticed that the following items are no longer listed: NSPrivacyCollectedDataTypes, NSPrivacyAccessedAPITypes, NSPrivacyTrackingDomains

Copy link
Member

@erisu erisu left a comment

Choose a reason for hiding this comment

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

LGTM

I will currently give it an "Approve" mark and will wait awhile to see if others have any additional feedback before merging.

Copy link
Contributor

@breautek breautek left a comment

Choose a reason for hiding this comment

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

lgtm

@erisu erisu merged commit c97845a into apache:master Mar 13, 2024
9 checks passed
@BenLaKnet
Copy link

BenLaKnet commented Mar 22, 2024

When this version could be used or tested?

@phyr0s
Copy link

phyr0s commented Mar 25, 2024

Hello, @erisu
When new version 7.1.0 will be released with this change?
Thanks

ishiguro-m-wing added a commit to dejiren/cordova-ios that referenced this pull request Mar 29, 2024
This reverts commit 1974dbb.

Revert "feat: add privacy-manifest config support (apache#1406)"

This reverts commit c97845a.

Revert "ci(gh-action): add Apache RAT & package license checker workflow w/ license header additions (apache#1408)"

This reverts commit d316558.

Revert "fix: use PROVISIONING_PROFILE_SPECIFIER for manual codesigning (apache#1405)"

This reverts commit af6335e.

Revert "feat: add PrivacyInfo.xcprivacy for CordovaLib & app template (apache#1383)"

This reverts commit b400b70.

Revert "fix: WASM MIME type error by specifying it in Info.plist template (apache#1374)"

This reverts commit 902df96.

Revert "chore: update package & package-lock (apache#1404)"

This reverts commit 2091208.

Revert "chore: bump 7.1.0-dev for next minor release (apache#1403)"

This reverts commit 766adcf.

Revert "chore(deps-dev): bump @babel/traverse from 7.21.4 to 7.23.2 (apache#1382)"

This reverts commit 92017bb.

Revert "chore: Update Slack signup link in SUPPORT_QUESTION.md (apache#1380)"

This reverts commit be4c884.

Revert "chore: bump version 7.0.2-dev"

This reverts commit a1f3ace.
ishiguro-m-wing pushed a commit to dejiren/cordova-ios that referenced this pull request Mar 29, 2024
* feat: privacy manifest settings in config.xml
* refac: remove unused code and tidy up
* refac: update class name PlatformConfigParser
* feat: add elementtree in package.json
* dev: change privacy manifest tag name to privacy-manifest from privacy-manifest-ios
* test: add test codes
* refactor: Modified to match Linter.
* refac: improve PlatformConfigParser
* refac: remove unnecessary blank line

---------

Co-authored-by: エリス <erisu@users.noreply.github.com>
@BenLaKnet
Copy link

How to build this version to test ios manifest ?

@Jeanlo
Copy link

Jeanlo commented Apr 3, 2024

So, how do you exactly use this feature to comply with Apple guidelines? Because I keep receiving the email, they keep telling me that I need to fill: NSPrivacyAccessedAPICategoryDiskSpace, NSPrivacyAccessedAPICategoryUserDefaults, NSPrivacyAccessedAPICategoryFileTimestamp.

I've set-up those values in my config.xml like this:

<privacy-manifest>
            <key>NSPrivacyAccessedAPITypes</key>
            <array>
                <dict>
                    <key>NSPrivacyAccessedAPITypeReasons</key>
                    <array>
                        <string>7D9E.1</string>
                    </array>
                    <key>NSPrivacyAccessedAPIType</key>
                    <string>NSPrivacyAccessedAPICategoryDiskSpace</string>
                </dict>
                <dict>
                    <key>NSPrivacyAccessedAPIType</key>
                    <string>NSPrivacyAccessedAPICategoryFileTimestamp</string>
                    <key>NSPrivacyAccessedAPITypeReasons</key>
                    <array>
                        <string>C617.1</string>
                    </array>
                </dict>
                <dict>
                    <key>NSPrivacyAccessedAPITypeReasons</key>
                    <array>
                        <string>1C8F.1</string>
                    </array>
                    <key>NSPrivacyAccessedAPIType</key>
                    <string>NSPrivacyAccessedAPICategoryUserDefaults</string>
                </dict>
            </array>
</privacy-manifest>

I have also tried using some of the examples above, and the email keeps telling me the same thing.

@BenLaKnet
Copy link

BenLaKnet commented Apr 5, 2024

I do not see files generated, but when I uploaded a new version with 7.1.1.nightly-, I did not receive an email.


<platform name="ios">
        <privacy-manifest>
            <key>NSPrivacyAccessedAPITypes</key>
            <array>
				<dict>
					<key>NSPrivacyAccessedAPIType</key>
					<string>NSPrivacyAccessedAPICategoryFileTimestamp</string>
					<key>NSPrivacyAccessedAPITypeReasons</key>
					<array>
						<string>C617.1</string>
					</array>
				</dict>
				<dict>
					<key>NSPrivacyAccessedAPIType</key>
					<string>NSPrivacyAccessedAPICategoryDiskSpace</string>
					<key>NSPrivacyAccessedAPITypeReasons</key>
					<array>
						<string>E174.1</string> <!-- autre valeur 85F4.1 -->
					</array>
				</dict>
				<dict>
					<key>NSPrivacyAccessedAPIType</key>
					<string>NSPrivacyAccessedAPICategoryUserDefaults</string>
					<key>NSPrivacyAccessedAPITypeReasons</key>
					<array>
						<string>CA92.1</string>
					</array>
				</dict>
			</array>
        </privacy-manifest>
    </platform>

Is this possible?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants