Floating-Dartists / matomo-tracker

A fully cross-platform wrap of the Matomo tracking client for Flutter, using the Matomo API.
https://pub.dev/packages/matomo_tracker
MIT License
26 stars 28 forks source link

PII in the user agent string can cause tracking failure #39

Closed luckyrat closed 1 year ago

luckyrat commented 1 year ago

I noticed that on an iPhone, no events were being tracked because the User-Agent HTTP header is being set to an invalid value.

This problem won't affect every device but is potentially not limited to iPhones.

In the specific case, the iPhone running ios 12 set its device name to Firstname Lastname’s iPhone as a default configuration. It is therefore likely that many/most user's of iPhones have a device name in this format. I have no newer hardware available to verify if Apple have changed this behaviour in recent years but either way, the name may well persist across iOS upgrades to even the most up to date phones.

Note that the apostrophe is not a standard ASCII apostrophe and thus it is not valid to be used in a HTTP header so Dart throws an error rather than submit the tracking request.

I have fixed the issue through two different approaches.

1) Stop setting the User-Agent HTTP Header to the device-specific value. Although I have only confirmed this works with existing users known to Matomo, I would expect that it works fine for new users too, since we already supply the ua parameter. 2) Remove the iOS device name and equivalents for other operating systems from the User agent string.

Either fix on its own would resolve this issue but I think the two combined are required to help protect against other unexpected device variables crashing the tracking process and to protect library users and end users from an unexpected leak of personal information.

TesteurManiak commented 1 year ago

IMO it would be better to remove invalid characters from the user agent using a Regex, what do you @Pierre-Monier and @MeixDev ?

MeixDev commented 1 year ago

From what I remember from the Matomo documentation, it's been a long time, the User Agent is used to determine the device from which you send the request through matomo's very own device-detector library.

Therefore I believe that setting the User-Agent according to the device still is important. Replacing non-ASCII characters through a RegEx operation feels like the way to go for me.

Luckyrat is also right in the fact that we already provide the ua parameter though. If this parameter replaces the User-Agent in the eyes of matomo and is confirmed to also work for new users, I would be fine with dropping the HTTP Header.

luckyrat commented 1 year ago

I've just tried an existing user on my test device (Android 12) and then wiped the stored data (uninstalled). Repeating the same with a fresh installation of the app produces identical device information in Matomo for the new visitor so as far as I can see, there is no need for the HTTP header.

I can't see how it would behave differently for different devices but I'm unsure about the backwards-compatibility with older versions of Matomo server. I've only tested with the latest version but hazard a guess that the behaviour won't have changed since the introduction of the ua parameter. Do you have an agreed minimum Matomo server version that this package must support?

I considered a RegEx but would prefer to avoid that per-request overhead unless it is necessary so figured it made sense to start with the efficient approach and only add the additional complexity if we find it causes an unexpected problem.

TesteurManiak commented 1 year ago

If I'm refering to Matomo's changelog it doesn't seem like anything User-Agent related has been deprecated so far, I guess that it wouldn't cause any issue to stop setting it in the send method and only rely on the ua property used in MatomoEvent.

TesteurManiak commented 1 year ago

Fixed in #40