Fix security exception crash while accessing network location provider (#1498)

* Fix security exception crash while accessing network location provider

* Added java docs

Author: Vivek Maskara

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.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<LocationUpdateListener> locationListeners = new CopyOnWriteArrayList<>();
     private boolean isLocationManagerRegistered = false;
+    private Set<Activity> locationExplanationDisplayed = new HashSet<>();
 
     /**
      * Constructs a new instance of LocationServiceManager.
@@ -51,7 +54,6 @@ public boolean isProviderEnabled() {
 
     /**
      * Returns whether the location permission is granted.
-     *
      * @return true if the location permission is granted
      */
     public boolean isLocationPermissionGranted() {
@@ -73,19 +75,33 @@ public void requestPermissions(Activity activity) {
                 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;
     }
 
     /**
      * Gets the last known location in cases where there wasn't time to register a listener
      * (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 LatLng getLastLocation() {
      * 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 @@ private boolean requestLocationUpdatesFromProvider(String locationProvider) {
      * @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

app/src/main/java/fr/free/nrw/commons/nearby/NearbyActivity.java

@@ -322,7 +322,7 @@ protected void onActivityResult(int requestCode, int resultCode, Intent data) {
     protected void onStart() {
         super.onStart();
         locationManager.addLocationListener(this);
-        locationManager.registerLocationManager();
+        registerLocationUpdates();
     }
 
     @Override
@@ -400,7 +400,7 @@ private void refreshView(LocationServiceManager.LocationChangeType locationChang
             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 @@ private void refreshView(LocationServiceManager.LocationChangeType locationChang
         }
     }
 
+    /**
+     * 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<Place> placeList = nearbyPlacesInfo.placeList;
         LatLng[] boundaryCoordinates = nearbyPlacesInfo.boundaryCoordinates;
@@ -530,7 +563,7 @@ private void lockNearbyView(boolean lock) {
             locationManager.removeLocationListener(this);
         } else {
             lockNearbyView = false;
-            locationManager.registerLocationManager();
+            registerLocationUpdates();
             locationManager.addLocationListener(this);
         }
     }