From f99363c06c8d9ea000a90421000c3760a9982892 Mon Sep 17 00:00:00 2001 From: Vivek Maskara Date: Tue, 15 May 2018 12:55:37 +0530 Subject: [PATCH] Fix security exception crash while accessing network location provider (#1498) * Fix security exception crash while accessing network location provider * Added java docs --- .../location/LocationServiceManager.java | 33 ++++++++++++---- .../nrw/commons/nearby/NearbyActivity.java | 39 +++++++++++++++++-- 2 files changed, 61 insertions(+), 11 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/location/LocationServiceManager.java b/app/src/main/java/fr/free/nrw/commons/location/LocationServiceManager.java index 73ded852f..49c422633 100644 --- a/app/src/main/java/fr/free/nrw/commons/location/LocationServiceManager.java +++ b/app/src/main/java/fr/free/nrw/commons/location/LocationServiceManager.java @@ -1,6 +1,7 @@ package fr.free.nrw.commons.location; import android.Manifest; +import android.annotation.SuppressLint; import android.app.Activity; import android.content.Context; import android.content.pm.PackageManager; @@ -10,9 +11,10 @@ import android.location.LocationManager; import android.os.Bundle; import android.support.v4.app.ActivityCompat; import android.support.v4.content.ContextCompat; -import android.util.Log; +import java.util.HashSet; import java.util.List; +import java.util.Set; import java.util.concurrent.CopyOnWriteArrayList; import timber.log.Timber; @@ -29,6 +31,7 @@ public class LocationServiceManager implements LocationListener { private Location lastLocation; private final List locationListeners = new CopyOnWriteArrayList<>(); private boolean isLocationManagerRegistered = false; + private Set locationExplanationDisplayed = new HashSet<>(); /** * Constructs a new instance of LocationServiceManager. @@ -51,7 +54,6 @@ public class LocationServiceManager implements LocationListener { /** * Returns whether the location permission is granted. - * * @return true if the location permission is granted */ public boolean isLocationPermissionGranted() { @@ -73,10 +75,23 @@ public class LocationServiceManager implements LocationListener { LOCATION_REQUEST); } + /** + * The permission explanation dialog box is now displayed just once for a particular activity. We are subscribing + * to updates from multiple providers so its important to show the dialog just once. Otherwise it will be displayed + * once for every provider, which in our case currently is 2. + * @param activity + * @return + */ public boolean isPermissionExplanationRequired(Activity activity) { - return !activity.isFinishing() && - ActivityCompat.shouldShowRequestPermissionRationale(activity, - Manifest.permission.ACCESS_FINE_LOCATION); + if (activity.isFinishing()) { + return false; + } + boolean showRequestPermissionRationale = ActivityCompat.shouldShowRequestPermissionRationale(activity, Manifest.permission.ACCESS_FINE_LOCATION); + if (showRequestPermissionRationale && !locationExplanationDisplayed.contains(activity)) { + locationExplanationDisplayed.add(activity); + return true; + } + return false; } /** @@ -84,8 +99,9 @@ public class LocationServiceManager implements LocationListener { * (e.g. when Location permission just granted) * @return last known LatLng */ + @SuppressLint("MissingPermission") public LatLng getLKL() { - if (ContextCompat.checkSelfPermission(context, Manifest.permission.ACCESS_FINE_LOCATION) == PackageManager.PERMISSION_GRANTED) { + if (isLocationPermissionGranted()) { Location lastKL = locationManager.getLastKnownLocation(LocationManager.GPS_PROVIDER); if (lastKL == null) { lastKL = locationManager.getLastKnownLocation(LocationManager.NETWORK_PROVIDER); @@ -107,9 +123,10 @@ public class LocationServiceManager implements LocationListener { * Registers a LocationManager to listen for current location. */ public void registerLocationManager() { - if (!isLocationManagerRegistered) + if (!isLocationManagerRegistered) { isLocationManagerRegistered = requestLocationUpdatesFromProvider(LocationManager.NETWORK_PROVIDER) && requestLocationUpdatesFromProvider(LocationManager.GPS_PROVIDER); + } } /** @@ -142,7 +159,7 @@ public class LocationServiceManager implements LocationListener { * @return LOCATION_SIGNIFICANTLY_CHANGED if location changed significantly * LOCATION_SLIGHTLY_CHANGED if location changed slightly */ - protected LocationChangeType isBetterLocation(Location location, Location currentBestLocation) { + private LocationChangeType isBetterLocation(Location location, Location currentBestLocation) { if (currentBestLocation == null) { // A new location is always better than no location diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyActivity.java b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyActivity.java index 04886fda4..35e15b0d9 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyActivity.java +++ b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyActivity.java @@ -322,7 +322,7 @@ public class NearbyActivity extends NavigationBaseActivity implements LocationUp protected void onStart() { super.onStart(); locationManager.addLocationListener(this); - locationManager.registerLocationManager(); + registerLocationUpdates(); } @Override @@ -400,7 +400,7 @@ public class NearbyActivity extends NavigationBaseActivity implements LocationUp return; } - locationManager.registerLocationManager(); + registerLocationUpdates(); LatLng lastLocation = locationManager.getLastLocation(); if (curLatLng != null && curLatLng.equals(lastLocation)) { //refresh view only if location has changed @@ -450,6 +450,39 @@ public class NearbyActivity extends NavigationBaseActivity implements LocationUp } } + /** + * This method first checks if the location permissions has been granted and then register the location manager for updates. + */ + private void registerLocationUpdates() { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { + if (locationManager.isLocationPermissionGranted()) { + locationManager.registerLocationManager(); + } else { + // Should we show an explanation? + if (locationManager.isPermissionExplanationRequired(this)) { + new AlertDialog.Builder(this) + .setMessage(getString(R.string.location_permission_rationale_nearby)) + .setPositiveButton("OK", (dialog, which) -> { + requestLocationPermissions(); + dialog.dismiss(); + }) + .setNegativeButton("Cancel", (dialog, id) -> { + showLocationPermissionDeniedErrorDialog(); + dialog.cancel(); + }) + .create() + .show(); + + } else { + // No explanation needed, we can request the permission. + requestLocationPermissions(); + } + } + } else { + locationManager.registerLocationManager(); + } + } + private void populatePlaces(NearbyController.NearbyPlacesInfo nearbyPlacesInfo) { List placeList = nearbyPlacesInfo.placeList; LatLng[] boundaryCoordinates = nearbyPlacesInfo.boundaryCoordinates; @@ -530,7 +563,7 @@ public class NearbyActivity extends NavigationBaseActivity implements LocationUp locationManager.removeLocationListener(this); } else { lockNearbyView = false; - locationManager.registerLocationManager(); + registerLocationUpdates(); locationManager.addLocationListener(this); } }