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

Reconsider background tracking mode impl for iOS13 #136

Open
js opened this issue Jun 7, 2019 · 21 comments
Open

Reconsider background tracking mode impl for iOS13 #136

js opened this issue Jun 7, 2019 · 21 comments

Comments

@js
Copy link

js commented Jun 7, 2019

iOS 13 changes the way background location permissions work.

If I understand the current implementation correctly this is how the mapbox-events-ios SDK works

  • If location auth status is authorizedAlways the SDK will startUpdating its CLLocationManager until:
    ** 5 minutes has passed
    ** OR when the user leaves a 3km wide geofence

in iOS13 the app can still ask for always access and either always or while in use is granted, but it'll still appear as authorizedAlways to the app until location is actually requested while in the background, at which point (or perhaps even sometime later "when convinient") the OS will ask the user if they want to allow background location updates.

Seems to be this will leave a surprise for many mapbox users. Let's say an fictional hiking app asks for requestAlwaysAuthorization or has done so at a previous time, but it won't actually starts it's own CLLocationManager in the background until the users decides to start a hike through some interaction in the app.
However, since the app now seemingly has .authorizedAlways mapbox-event-sdk will start its background location tracker and when the app is in the background the user will get asked if they want to allow background location updates, completely out of context as they've only intended to allow background location access when they've actually started a hike.

https://developer.apple.com/videos/play/wwdc2019/705/

@alfwatt
Copy link
Contributor

alfwatt commented Jun 14, 2019

@js thanks for the heads up, we've been tracking this and hope to have a fix in the SDK and Maps before iOS 13 is in public beta.

@alfwatt alfwatt self-assigned this Jun 17, 2019
@js
Copy link
Author

js commented Aug 12, 2019

The public release of iOS 13 is (probably) not many weeks away, any updates on this?

@js
Copy link
Author

js commented Sep 5, 2019

iOS 13 is most likely being released to the public in a week, yet I have seen no intention from Mapbox to resolve this issue?

@alfwatt Could you please give us any indication as to what your plans are to resolve this issue please? It will have a pretty big negative impact on our app, and indirectly on mapbox and the (lack of) data you'll collect.

@julianrex
Copy link
Contributor

julianrex commented Sep 6, 2019

/cc @chloekraw @benlevin47

@chloekraw
Copy link

cc: @MargaretMeiLee @angelanavarro

@alfwatt
Copy link
Contributor

alfwatt commented Sep 6, 2019

@js We've been tracking the iOS 13 changes closely and have been working on an update to our telemetry collection practices. We'll provide an update as soon as possible, as we understand your concern and agree it's an important issue.

@js
Copy link
Author

js commented Sep 12, 2019

@alfwatt iOS 13 will be publicly released on September 19th, in seven days, please give us ample time to submit and get our updates reviewed with your updated SDK.

@alfwatt
Copy link
Contributor

alfwatt commented Sep 12, 2019

@js sorry we don't have a fix ready, we're discussing with our execs how to proceed. What we've seen in our testing is that the prompt in iOS 13.1 doesn't display all the locations which are sent to the application but only the significant location change notifications, e.g. here's the 3-day alert for our testing app compared with the last two days worth of collection on my device:

IMG_1902
IMG_1903
IMG_1904

@js
Copy link
Author

js commented Sep 16, 2019

@alfwatt Yes this matches my observations as well. But the issue here isn't really the granularity of the data collected as such, but entirely about the fact that an app will now explicitly make the user aware that it's collecting data in the background (which in itself is probably good).

In an app where background location tracking is entirely context-based (such as a workout app) and opt-in through an action (such as starting a workout) then the appearance of a dialog such as the above when the user didn't expect it will almost certainly cause the user to revoke the background location permission, which is bad news for the app as it no longer gets background access and we've essentially betrayed the users trust here (!), but also bad news for mapbox and your data collection.

@js
Copy link
Author

js commented Sep 16, 2019

Also, mapbox events seems to be monitoring outside the 3000 meter geofence, as evident in in your screenshot of your "MB Events" app, which I have also observed.

The circle annotationehere roughly indicates a 3km radius around your location in the screenshot. Should it not stop collection data when exiting that region? Yet it seems like the breadcrumbs of go beyond this. I have similar iOS13 alerts where the locations are well beyond a 3km geofence…
sf3k

@js
Copy link
Author

js commented Sep 17, 2019

As I'm digging into what exactly it is mapbox is doing here, I'm not sure where I got 3000m from, it's actually when leaving a 300m radius and then it attempts to track it at most for five minutes it seems? Am I reading this and understanding that intention correctly?

The problem is that it keeps tracking the location beyond these two, a casual read through the implementation seems to point at significant location monitoring and visit monitoring is still active after the five minute limit.

Please correct me if I misunderstand anything here, I'd very much like to understand
1) why our app seemingly tracks the users location beyond what is reasonably expected (see multiple previous comments)
2) excessive background battery usage, likely related to the above

Here's what seems to be happening, please correct me in what I misunderstand:

  1. Once the CLLocationManager in your MMELocationManager gets an accurate (within 300meters) location it'll start monitoring for a region with that as the center and a 300 meter radius
  2. When that region is exited the location manager is started again as well as a 5 minute timer
  3. if the timeout is reached, there is an attempt at checking backgroundLocationServiceTimeoutAllowedDate instead if simply relying on the timer being fired, presumably to avoid it being called beyond the 5 minute timeout if the timer was (re)created multiple times? But backgroundLocationServiceTimeoutAllowedDate is reset each time -startBackgroundTimeoutTimer is called?

Assuming I read everything correctly there's a few issues in the above imho:

Number two above has some issues when a new location arrives, which will happen (?) since the location manager is started again when exiting the region:

  • if the user is moving, then the timer is reset whenever a new location arrives as they move
  • if the accuracy is below 300 meters, a new region is created centered at the newly arrived location update, essentially moving the entire process back to step one (!).
  • The timer is re-created multiple times essentially voiding the five minute cutoff and causing it to track the user beyond those five minutes

Apart from the timeout issue mentioned in number 3, it doesn't actually stop all the location monitoring that was started only regular location updates, region moniting, visit monitoring and significant location monitoring is still active. Seems like it should be calling stopUpdatingLocation on self instead?

@ChristianSteffens
Copy link

I can confirm - at least for our App - that disabling the Mapbox Metrics (via the UserDefault MGLMapboxMetricsEnabled) does indeed fix the issue. Our App doesn't request "Always Permission", only "When In Use". But still our users got the warning / alert dialog under iOS 13 - disabling the Mapbox metrics from start "fixed" the issue...

@alfwatt
Copy link
Contributor

alfwatt commented Sep 27, 2019

@ChristianSteffens if you app has either NSLocationAlwaysAndWhenInUseUsageDescription or NSLocationAlwaysUsageDescription set in it's Info.plist the Events SDK will attempt to collect background location.

If your app does not use background location you can remove those keys from your Info.plist to prevent any background location collection.

@ChristianSteffens
Copy link

🤔, i.e. the mere existence of these two (2) keys will result in background location tracking? Even if the App doesn't have the necessary permissions?

@JustinGanzer
Copy link

JustinGanzer commented Nov 21, 2019

@alfwatt @ChristianSteffens

If your app does not use background location you can remove those keys from your Info.plist to prevent any background location collection.

Beware that this will cause errors if I'm not mistaken. Because Apple identifies that MapBox has code references to the "Always" permission you'll receive a warning email from app store connect informing you that you should indeed provide these strings.

The email will read as follows:

ITMS-90683: Missing Purpose String in Info.plist - Your app's code references one or more APIs that access sensitive user data. The app's Info.plist file should contain a NSLocationAlwaysUsageDescription key with a user-facing purpose string explaining clearly and completely why your app needs the data. Starting Spring 2019, all apps submitted to the App Store that access user data are required to include a purpose string. If you're using external libraries or SDKs, they may reference APIs that require a purpose string. While your app might not use these APIs, a purpose string is still required. You can contact the developer of the library or SDK and request they release a version of their code that doesn't contain the APIs. Learn more (https://developer.apple.com/documentation/uikit/core_app/protecting_the_user_s_privacy).

So you either have a warning and complications with Apple or provide the necessary strings and have MapBox attempting to pull in the unwanted location updates. The only logical solution to me appears to have control over what permission MapBox-Events uses and when...or fork and throw out all code references to the always permission.

@epologee
Copy link

epologee commented Feb 3, 2020

It's February '20. This issue is a few months old by now, and in the mean time NYT published their opinion piece, that moved the needle some more towards general suspicion of those three-day-pop-ups.

Have there been any developments for the Mapbox SDK, or is there a sanctioned way of disabling the region monitoring?

@alfwatt
Copy link
Contributor

alfwatt commented Feb 3, 2020

@epologee please check out the latest information in the readme, Release 0.10.0 contains some changes to the region monitoring for iOS 13: https://github.com/mapbox/mapbox-events-ios#-foreground-and-background-location-collection

@epologee
Copy link

epologee commented Feb 3, 2020

Hi Alf, thanks for the reply. I still think this issue is a valid open one. @JustinGanzer already mentioned that section of the readme, above. However the suggestion to remove keys from the Plist to fix this, does not apply.

  • Yes, our app uses background location, asks for "always permission", and therefore has the proper key/value-pairs in the Info.plist that can't be removed.
  • No, we do not want to allow Mapbox background telemetry gathering, because iOS 13' mini-map dialog gives users the assumption that they're somehow being tracked outside of the focus area of the app.

The Mapbox SDK is not actively used in the background. It's an entirely non-map related feature in the app.

I know that the terms state that we should not limit the data that the Mapbox SDK sends "back home", however there's no requirement to do so in the background, or at least, I can't spot it directly. If I'm mistaken here, please let me know.

@JustinGanzer
Copy link

@epologee Their terms are kept short and readable, though it does say

Not interfere with or limit the data [ie in any way] that the Mapbox SDK sends to us

At least that is how it sounds to me and as such leaves no room for interpretation of "no requirement to do so in background" anywhere.

However the sdk is open to anyone and you're not using their service if you're hosting your own map data server or don't pull any map data from mapbox because you're not using it display a map at all? (Though I wonder what it is one does with mapbox with no maps).

Those are the only two solutions I see here.

@SjoerdPerfors
Copy link

SjoerdPerfors commented Feb 5, 2020

If you use the navigator SDK which includes this one:

This SDK uses Mapbox Navigator, a private binary, as a dependency. The Mapbox Navigator binary may be used with a Mapbox account and under the Mapbox TOS. If you do not wish to use this binary, make sure you swap out this dependency in libandroid-navigation/build.gradle. Code in this repo falls under the MIT license.

So you cannot simply fork it and disable this. If you need a license free navigation SDK fork an older version or use one of the existing forks.

@alfwatt
Copy link
Contributor

alfwatt commented Feb 6, 2020

@JustinGanzer we're looking at the issue with the CLLocationSymbols being in the frameworks causing App Store warnings, we hope to resolve that issue in the next release.

@alfwatt alfwatt removed their assignment Feb 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants