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: privacy manifest files #677

Merged
merged 4 commits into from
Apr 10, 2024
Merged

feat: privacy manifest files #677

merged 4 commits into from
Apr 10, 2024

Conversation

levibostian
Copy link
Contributor

@levibostian levibostian commented Mar 28, 2024

Closes: https://linear.app/customerio/issue/MBL-187/update-values-based-on-linear-doc
Part of: https://linear.app/customerio/issue/MBL-189/release-ios

Create privacy manifest files for all target modules that customers could decide to install in an app. Update package manager config files to include these new resource files.

Testing criteria CocoaPods:

  • pod update, opened app in xcode
  • Verified that in 'Pods' project settings, there are new targets added for each module (example: CustomerIODataPipelines-CustomerIODataPipelines_Privacy). I then verified in each of the _Privacy targets > Build phases > compile sources list, the privacy manifest files were listed and they resource to a valid path.
  • App compiled without any new warnings.

Testing criteria SPM:

  • Opened sample app in xcode.
  • Fixed all warnings xcode gave me while resolving the CIO package.
  • Compiled app without any new warnings.

Copy link

github-actions bot commented Mar 28, 2024

Sample app builds 📱

Below you will find the list of the latest versions of the sample apps. It's recommended to always download the latest builds of the sample apps to accurately test the pull request.


  • CocoaPods-FCM: spr/main/371fc1ed (1712686243)
  • APN-UIKit: spr/main/371fc1ed (1712686218)

Closes: https://linear.app/customerio/issue/MBL-187/update-values-based-on-linear-doc
Part of: https://linear.app/customerio/issue/MBL-189/release-ios

Create privacy manifest files for all target modules that customers could decide to install in an app. Update package manager config files to include these new resource files.

Testing criteria CocoaPods: 
* `pod update`, opened app in xcode
* Verified that in 'Pods' project settings, there are new targets added for each module (example: `CustomerIODataPipelines-CustomerIODataPipelines_Privacy`). I then verified in each of the `_Privacy targets > Build phases > compile sources` list, the privacy manifest files were listed and they resource to a valid path. 
* App compiled without any new warnings. 

Testing criteria SPM: 
* Opened sample app in xcode. 
* Fixed all warnings xcode gave me while resolving the CIO package. 
* Compiled app without any new warnings. 

commit-id:371fc1ed
@levibostian
Copy link
Contributor Author

Do not merge until the Linear ticket is unblocked.

It's still ready for a technical review.

@levibostian
Copy link
Contributor Author

Notes for reviewers:

  • The values in the manifest files came from internal docs we created. See ticket for link to the docs.
  • See description for testing criteria on how I validated this.

<key>NSPrivacyCollectedDataType</key>
<string>NSPrivacyCollectedDataTypeUserID</string>
<key>NSPrivacyCollectedDataTypeLinked</key>
<false/> <!-- because of anonymous profiles, linking to a person is optional -->
Copy link
Contributor

Choose a reason for hiding this comment

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

added a comment in notion about it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am keeping this value as-is until we give others on team time to read documentation and give suggestions for this value.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Since to utilize basically any functionality of Customer.io, we need to link usage data to a user typically email or a device. This is the rational for marking this as true. If we mark this as false, it means that Customer.io does not have any data linked to a user anywhere in our systems.

Copy link
Contributor

Choose a reason for hiding this comment

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

Added comment on notion too. We only need a profile to function, so even if they use aliases to create profiles, sending push notifications and in-app messages should still be possible based on the data linked to the user, which I believe aligns with Apple's privacy details. For sending email, it's true that linking an actual email is necessary, but we don’t force it and it likely falls under the customer app's privacy? 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

From https://developer.apple.com/app-store/app-privacy-details/#linked-data

Additionally, in order for data not to be linked to a particular user’s identity, you must avoid certain activities after collection:
You must not attempt to link the data back to the user’s identity.
You must not tie the data to other datasets that enable it to be linked to a particular user’s identity.

I believe we do both of these activities, which I why I tend to go for true

Copy link
Contributor Author

@levibostian levibostian Apr 3, 2024

Choose a reason for hiding this comment

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

This PR thread has been discussed in a Notion comment.
I suggest reviewing this updated section in Notion and comment there if you have more feedback.

<array>

<!--
Section for SDKs that capture "Such as screen name, handle, account ID, assigned user ID, customer number, or other user- or account-level ID that can be used to identify a particular user or account."
Copy link
Contributor

Choose a reason for hiding this comment

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

do we have to specify the things we capture, like app name etc?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, you do not need to specify what you capture. Instead specify what categories that you capture.

Copy link
Contributor Author

@levibostian levibostian Apr 1, 2024

Choose a reason for hiding this comment

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

I may have misunderstood your question.

Looking at the docs for NSPrivacyCollectedDataTypes, I do not see anything in there regarding app name, app version.

If you see a category in this apple doc that is missing in this file, please mention the missing category in our notion doc.

Copy link
Contributor

Choose a reason for hiding this comment

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

It was based on this where it mentions adding custom data types but maybe we can skip these as I am unsure.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm open to skipping adding any custom data types for now. Apples documents aren't always the most clear, so it will be tough to determine what qualifies as a custom data type that we might need to declare.

This section is describing what information our SDK collects about the app user.

It's my understanding that this section is for what our SDK requires, not what a customer can do with our SDK. Therefore, if something is optional, use `false`.
Customers must create their own privacy file for what their app does and provide that to Apple. That's where customers report what they might be using our SDK for.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Customers do not need to declare their dependancies privacy requirements based on the second line in this call out at the top of this page.
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Re-reviewing the comments in this file and the documentation you linked to, I think the comments in this file are up-to-date and accurate.

Do you have a suggestion of a change to make to these comments?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the issue is around the interpretation of "required". Since their app is collecting an email address and passing it to us, they would have to declare that. If we ever created a UI to collect the email and link to a user then we'd have to declare the email address. Therefore in the first case email isn't a "required" part of our SDK.

So this does work, but the wording made that tough to interpret. I provided a suggestion to the line that might make that more clear.

Sources/DataPipeline/Resources/PrivacyInfo.xcprivacy Outdated Show resolved Hide resolved
This section is describing what information our SDK collects about the app user.

It's my understanding that this section is for what our SDK requires, not what a customer can do with our SDK. Therefore, if something is optional, use `false`.
Customers must create their own privacy file for what their app does and provide that to Apple. That's where customers report what they might be using our SDK for.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the issue is around the interpretation of "required". Since their app is collecting an email address and passing it to us, they would have to declare that. If we ever created a UI to collect the email and link to a user then we'd have to declare the email address. Therefore in the first case email isn't a "required" part of our SDK.

So this does work, but the wording made that tough to interpret. I provided a suggestion to the line that might make that more clear.

Sources/DataPipeline/Resources/PrivacyInfo.xcprivacy Outdated Show resolved Hide resolved
levibostian and others added 2 commits April 3, 2024 11:33
Co-authored-by: Scott Wittrock (Customer.io) <153670820+scotttwittrockcio@users.noreply.github.com>
Co-authored-by: Scott Wittrock (Customer.io) <153670820+scotttwittrockcio@users.noreply.github.com>
Copy link

codecov bot commented Apr 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 56.93%. Comparing base (40ea74b) to head (b4ecc69).
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #677      +/-   ##
==========================================
- Coverage   56.98%   56.93%   -0.06%     
==========================================
  Files         139      139              
  Lines        3871     3871              
==========================================
- Hits         2206     2204       -2     
- Misses       1665     1667       +2     

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

@kvdesa
Copy link

kvdesa commented Apr 9, 2024

Any updates on when this will be released?

@levibostian
Copy link
Contributor Author

@kvdesa Unfortunately, we do not have a target date when to expect this feature. Know that it's a high priority of our team and we're working to release it as soon as we are able.

@levibostian levibostian merged commit e5c6dc7 into main Apr 10, 2024
10 checks passed
@levibostian levibostian deleted the spr/main/371fc1ed branch April 10, 2024 15:37
github-actions bot pushed a commit that referenced this pull request Apr 10, 2024
## [3.1.0](3.0.2...3.1.0) (2024-04-10)

### Features

* privacy manifest files ([#677](#677)) ([e5c6dc7](e5c6dc7))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants