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

iOS: Cannot request location always if we already have location when in use permission #490

Open
tallpants opened this issue Jul 27, 2020 · 31 comments · May be fixed by #716
Open

iOS: Cannot request location always if we already have location when in use permission #490

tallpants opened this issue Jul 27, 2020 · 31 comments · May be fixed by #716
Labels
Fund Can be funded on Polar.sh

Comments

@tallpants
Copy link

tallpants commented Jul 27, 2020

Bug report

If we have location when in use permission, requesting location always permission always returns denied, probably because of this line: https://github.com/react-native-community/react-native-permissions/blob/4017cbf0861b562bb48888f76cfa7c9a62c68ffc/ios/LocationAlways/RNPermissionHandlerLocationAlways.m#L38

Apple's documentation is pretty clear about how you're allowed to call requestAlwaysAuthorization if the current authorization status is either kCLAuthorizationStatusNotDetermined or kCLAuthorizationStatusAuthorizedWhenInUse. Calling this when we already have when-in-use authorization will prompt the user to upgrade to "always allow" location permissions.

But in the library we're assuming that if we currently have when-in-use authorization, we're not allowed to ask for always authorization.

I tested by manually calling requestAlwaysAuthorization in Objective-C after getting when-in-use authorization from react-native-permissions -- and the upgrade prompt shows up just fine.

This should be an easy fix (as far as I can tell).

Documentation for requestAlwaysAuthorization that explains this: https://developer.apple.com/documentation/corelocation/cllocationmanager/1620551-requestalwaysauthorization?changes=latest_minor&language=objc

Upvote & Fund

  • This repository is using Polar.sh so you can help prioritize & fund this issue.
  • The funding will only be unlocked once this issue is completed.
Fund with Polar
@zoontek
Copy link
Owner

zoontek commented Jul 27, 2020

@tallpants This library already handle location permission escalation. The line you give is from the check method from RNPermissionHandlerLocationAlways.m, so there no issue here.

A normal permission escalation would be:

  1. check that LOCATION_WHEN_IN_USE is granted and LOCATION_ALWAYS is denied.
  2. request LOCATION_ALWAYS

@zoontek zoontek added the invalid This doesn't seem right label Jul 27, 2020
@tallpants
Copy link
Author

tallpants commented Jul 27, 2020

@zoontek doing request for LOCATION_ALWAYS will return RESULTS.BLOCKED right now if we have LOCATION_WHEN_IN_USE permission already.

@zoontek zoontek removed the invalid This doesn't seem right label Jul 28, 2020
@zoontek
Copy link
Owner

zoontek commented Jul 28, 2020

I just checked it, it's not that easy to fix it.

I explain myself: kCLAuthorizationStatusAuthorizedWhenInUse does not means that LOCATION_ALWAYS is requestable. It means that it might be requestable.
It's fixable, but will need a bit of work / test.

@tallpants
Copy link
Author

tallpants commented Jul 28, 2020

You're right yea it's more complicated than I initially thought. Do you have any ideas on how to approach fixing it? My initial guess is that without some form of persistence (maybe to NSUserDefaults) about whether we've already requested LOCATION_ALWAYS once before in the app, it's not possible to reliably work with the existing API of check and request.

Something like this (pseudocode -- doesn't handle all the other cases):

- (void)checkWithResolver:(RCTPromiseResolveBlock)resolve rejecter:(RCTPromiseRejectBlock)reject {
  CLAuthorizationStatus authorizationStatus = [CLLocationManager authorizationStatus];
  BOOL hasRequestedLocationAlways = [NSUserDefaults standardDefaults] boolForKey:@"RNPermissionHandlerLocationAlways-hasRequested"];

  if (authorizationStatus == kCLAuthorizationStatusAuthorizedWhenInUse && hasRequestedLocationAlways) {
    return resolve(RNPermissionStatusDenied);
  } else {
    return resolve(RNPermissionStatusNotDetermined);
  }
}

... and similary setting the RNPermissionHandlerLocationAlways-hasRequested key in user defaults to true in the request method after calling requestAlwaysAuthorization (since it can only be called once).

Just an idea of course, maybe you can find a nicer way of solving.

@zoontek
Copy link
Owner

zoontek commented Jul 28, 2020

The core already has such helper method (https://github.com/react-native-community/react-native-permissions/blob/master/ios/RNPermissions.m#L324)

It might work, but we have to flag as requested only if the result isn't whenInUse.

@rolfb
Copy link

rolfb commented Aug 3, 2020

Would love the ability to call requestAlwaysAuthorization twice to get to Always faster too.

@mikehardy
Copy link

This is under discussion here as well: https://github.com/transistorsoft/react-native-background-geolocation-android/issues/938#issuecomment-668042805

By you @rolfb :-), just cross-linking here as you've got great research posted in the other issue

@rolfb
Copy link

rolfb commented Aug 3, 2020

Cheers, @mikehardy!

The other one is private though :-)

@mikehardy
Copy link

Ah true enough - it affects the public version, but either way may result in knowledge here :)

@rolfb
Copy link

rolfb commented Aug 5, 2020

It seems like you have to explicitly request WhenInUse first, then you can request Always directly after. If you request Always first and get While in use (as read from device settings) you can't request Always to trigger the "Change to always" alert and will have to wait for the system to trigger it. This is probably clear to most, just wanted to summarize.

@jdegger
Copy link

jdegger commented Aug 10, 2020

For reference, the background-geolocation-android private repo owner @christocracy posted this as a solution:

docs clearly state it must either be undetermined or when in use when requesting.

The code added to make this work is the || clause.

if ((status == kCLAuthorizationStatusNotDetermined) || ((lastStatus == kCLAuthorizationStatusAuthorizedWhenInUse) && ([desiredRequest isEqualToString:ALWAYS_STRING]) )) {
  .
  .
  .
}

I have tested by commenting-out the check for kCLAuthorizationStatusNotDetermined in this repo and this seems to work fine.

@tallpants
Copy link
Author

@jdegger that'll work for the initial request -- but to keep the API consistent we also need to track manually when we've asked for it already and return DENIED -- because requestAlwaysAuthorization will just not do anything if it's called a second time.

@christocracy
Copy link

we also need to track manually when we've asked for it already

That's correct. The updated logic in background-geolocationplugin keeps track of a config.didRequestUpgradeLocationAuthorization, stored in NSUseDefaults.

Note that if user resets location authorization to Ask Next Time, we can once again request an authorization upgrade from WhenInUse -> Always.

BOOL isRequestUpgradeAlways = NO;

if (status == kCLAuthorizationStatusNotDetermined) {
    // Reset upgrade-to-always state in UserDefaults
    config.didRequestUpgradeLocationAuthorization = NO;
} else if ((status == kCLAuthorizationStatusAuthorizedWhenInUse) && ([desiredRequest isEqualToString:ALWAYS_STRING]) && !config.didRequestUpgradeLocationAuthorization ) {
    isRequestUpgradeAlways = YES;
}

if ((status == kCLAuthorizationStatusNotDetermined) || isRequestUpgradeAlways ) {
    // Update upgrade-to-always state in UserDefaults
    if (isRequestUpgradeAlways) {
        config.didRequestUpgradeLocationAuthorization = YES;
    }
    // Request authorization
    .
    .
    .
}

@elkinjosetm
Copy link

Is there any update on this issue?

@zoontek
Copy link
Owner

zoontek commented Sep 21, 2020

I personally don't have any time to work on open source these days.
For react-native-permissions, iOS 14 / Android 11 support will probably be the main focus area, so this task will probably receive a low priority.

@christocracy
Copy link

@zoontek I'm the author of react-native-background-geolocation. Requesting Always permission with Android 11 / targetSdkVersion 30 is really tricky.

  1. Manifest.permission.ACCESS_BACKGROUND_LOCATION must be requested on its own (cannot be combined with ACCESS_COARSE_LOCATION / ACCESS_FINE_LOCATION.
  2. Android offers a method to determine if the user has already been asked for ACCESS_BACKGROUND_PERMISSION :
    /// Returns true if the Dialog should be shown
    @TargetApi(29)
    public boolean shouldShow(Activity activity) {
        if (activity == null || misDialogActive.get()) return false;
        return ActivityCompat.shouldShowRequestPermissionRationale(activity, Manifest.permission.ACCESS_BACKGROUND_LOCATION);
    }
  1. When above method returns true, it's your responsibility to compose a custom dialog explaining why the app requires Allow all the time permission. For background-geolocation, I've created a new config option backgroundPermissionRationale for customizing the text-elements on the dialog.
backgroundPermissionRationale: {
  title: "Allow access to this device's location in the background?",
  message: "In order to X, Y and Z, please enable 'Allow all the time permission",
  okButton: "Change to Allow all the time"
}

In the background-geolocation library, I show a Dialog instance with custom layout styled to look similar to the Android System Dialog:

@christocracy
Copy link

@zoontek Maybe another way to handle this without your library producing a native Android Dialog would be to offer an event like onRationalize(permission), where the develop would take full responsibility for producing their own RN dialog and re-executing their permission-request.

@zoontek
Copy link
Owner

zoontek commented Sep 21, 2020

@christocracy I personally hate apps that request Always permission because, with the exception of some apps that use location extensively, it's often not required except to track users.

Thanks for the informations, I will check this when I will have some time (we just launched our product, I'm really busy these days 😅). I'll try to choose the easiest solution, but I'm afraid of these changes.

@mikehardy
Copy link

@zoontek I agree with you in general and in spirit, however I have one of those apps that nevertheless does something useful with the always permission, in a transparent + delete-able and even optional way, such that if it's allowed it could help the user, but we respect that people hate it (since normally they receive no value for their data). Which is just to say, sure, but...sometimes there's reasons, so I'd love to see the feature. Recognizing I may need to code it up :-), although I'm pretty busy myself at the moment

@christocracy
Copy link

christocracy commented Sep 21, 2020

I personally hate apps that request Always

My plugin was originally designed for tracking first-responders in disaster-zones (hurricanes, earthquakes), where life depended on it. It is ideally suited for fleet-tracking use-cases (eg: Jogging App, Delivery service app, Taxi / Trucking apps).

I make no moral judgements on my customers' usage of Always authorization though I do let those who I find using it in Social app that it's not a good idea.

I welcome these strict new permission authorization mechanisms introduced by both Google and iOS.

@zoontek
Copy link
Owner

zoontek commented Sep 21, 2020

@christocracy @mikehardy Guys it's not against you, I also worked on several apps that required background geolocation, it's just that I see this feature abused :)

I will try to add a light API to handle these cases, but not an ultra complete one with X fancy options 😅

@tallpants
Copy link
Author

tallpants commented Oct 6, 2020

@zoontek would you be willing to accept a PR that fixes only the iOS permission escalation issue (where you can't request location always authorization after you've been granted location when in use authorization) -- which is what the original issue was about, and then we can track the problems on Android and a new API for extra cases and so on in a different issue?

I'm hoping we can merge it and release a minor version since it's technically a bug with the library, instead of saving it for 3.0.

@tallpants
Copy link
Author

PR opened for this here: #529

@taylorkline
Copy link

taylorkline commented Oct 23, 2020

I have ran into this issue as well. We have two use-cases in our app, one that only needs whenInUse auth, and another, rare use-case that requires always.

Apple's own guidelines state,

Request authorization only when your user needs location services to perform a task in your app. If it's not clear to the user why your app is using location services, the user may deny your request.

Therefore this current bug does not allow us React Native users to follow Apple's guidelines.

I acknowledge that @zoontek doesn't have time to prioritize this issue right now, but wanted to make a note here.

@tallpants
Copy link
Author

@taylorkline we'd all like to see this fixed but for now you'll have to consider this a missing feature. I'd suggest using the changes in my PR linked above along with patch-package in the meantime. Realistically I think this'll only be fixed in the next major release because it's not possible to fit this permission into the existing API.

@mkraina
Copy link

mkraina commented Dec 30, 2020

Hi guys, I've came up with possible solution proposed here in @tallpants PR
So far it works just fine for me, hopefully our QA won't find out any flaws with this
here is the react-native-permissions+3.0.0.patch generated via patch-package (using changes from the PR)

index 29c9075..c7e2f7e 100644
--- a/node_modules/react-native-permissions/ios/LocationAlways/RNPermissionHandlerLocationAlways.m
+++ b/node_modules/react-native-permissions/ios/LocationAlways/RNPermissionHandlerLocationAlways.m
@@ -35,7 +35,13 @@ - (void)checkWithResolver:(void (^ _Nonnull)(RNPermissionStatus))resolve
       return resolve(RNPermissionStatusNotDetermined);
     case kCLAuthorizationStatusRestricted:
       return resolve(RNPermissionStatusRestricted);
-    case kCLAuthorizationStatusAuthorizedWhenInUse:
+    case kCLAuthorizationStatusAuthorizedWhenInUse: {
+        BOOL requestedBefore = [RNPermissions isFlaggedAsRequested:[[self class] handlerUniqueId]];
+        if (requestedBefore) {
+            return resolve(RNPermissionStatusDenied);
+        }
+        return resolve(RNPermissionStatusNotDetermined);
+    }
     case kCLAuthorizationStatusDenied:
       return resolve(RNPermissionStatusDenied);
     case kCLAuthorizationStatusAuthorizedAlways:
@@ -48,7 +54,11 @@ - (void)requestWithResolver:(void (^ _Nonnull)(RNPermissionStatus))resolve
   if (![CLLocationManager locationServicesEnabled]) {
     return resolve(RNPermissionStatusNotAvailable);
   }
-  if ([CLLocationManager authorizationStatus] != kCLAuthorizationStatusNotDetermined) {
+
+  CLAuthorizationStatus authorizationStatus = [CLLocationManager authorizationStatus];
+  BOOL requestedBefore = [RNPermissions isFlaggedAsRequested:[[self class] handlerUniqueId]];
+
+  if (authorizationStatus != kCLAuthorizationStatusNotDetermined && (authorizationStatus == kCLAuthorizationStatusAuthorizedWhenInUse && requestedBefore)) {
     return [self checkWithResolver:resolve rejecter:reject];
   }
 
@@ -57,14 +67,35 @@ - (void)requestWithResolver:(void (^ _Nonnull)(RNPermissionStatus))resolve
 
   _locationManager = [CLLocationManager new];
   [_locationManager setDelegate:self];
+
+  [[NSNotificationCenter defaultCenter] addObserver:self
+                                        selector:@selector(handleAppStateDidChange:)
+                                        name:UIApplicationDidBecomeActiveNotification
+                                        object:nil];
+    
   [_locationManager requestAlwaysAuthorization];
+  [RNPermissions flagAsRequested:[[self class] handlerUniqueId]];
 }
 
-- (void)locationManager:(CLLocationManager *)manager didChangeAuthorizationStatus:(CLAuthorizationStatus)status {
-  if (status != kCLAuthorizationStatusNotDetermined) {
-    [_locationManager setDelegate:nil];
+- (void)updatePermission {
+    if(_resolve == nil || _reject == nil) return;
     [self checkWithResolver:_resolve rejecter:_reject];
-  }
+    [_locationManager setDelegate:nil];
+    _resolve = nil;
+    _reject = nil;
+    [[NSNotificationCenter defaultCenter] removeObserver:self name:UIApplicationDidBecomeActiveNotification object:nil];
+}
+
+- (void)locationManager:(CLLocationManager *)manager didChangeAuthorizationStatus:(CLAuthorizationStatus)status {
+    if (status != kCLAuthorizationStatusNotDetermined && status != kCLAuthorizationStatusAuthorizedWhenInUse) {
+        [self updatePermission];
+    }
+}
+
+- (void)handleAppStateDidChange:(NSNotification *)notification {
+    if([notification.name isEqualToString:UIApplicationDidBecomeActiveNotification]) {
+        [self updatePermission];
+    }
 }
 
 @end

@tallpants
Copy link
Author

@zoontek we're revisiting this at our org -- the PR I made for this was left hanging because of your concern over a promise that might be unresolved indefinitely (which was definitely not ideal I agree).

We're willing to do the work to switch to the UIApplicationDidBecomeActiveNotification approach suggested by so many others here (as well as used by Expo in their location module) if you'd be willing to review / merge and release it -- we're happy to be responsible for maintaining that code. Does that work for you?

@zoontek
Copy link
Owner

zoontek commented Sep 15, 2022

@tallpants Yes, that could do the trick!

@tallpants
Copy link
Author

@zoontek here's the new PR!

#716

@tallpants
Copy link
Author

ping @zoontek -- just following up on this!

@KaiValar
Copy link

Just fyi, the guys of Expo can do this job with their package location, maybe you can look how they did it https://github.com/expo/expo/tree/master/packages/expo-location

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fund Can be funded on Polar.sh
Projects
None yet
10 participants