Merge pull request #121 from domdomegg/fix-permissions-117

Fix permissions on API levels 23 and above

commons/app/build.gradle

@@ -1,23 +1,25 @@
 apply plugin: 'com.android.application'
 
 dependencies {
-    compile fileTree(dir: 'libs', include: '*.jar')
+    compile fileTree(include: '*.jar', dir: 'libs')
-    compile 'com.google.code.gson:gson:1.4'
     compile 'fr.avianey.com.viewpagerindicator:library:2.4.1.1@aar'
     compile 'in.yuvi:http.fluent:1.3'
     compile 'com.android.volley:volley:1.0.0'
     compile 'com.nostra13.universalimageloader:universal-image-loader:1.8.4'
     compile 'ch.acra:acra:4.5.0'
     compile 'org.mediawiki:api:1.3'
-    compile 'de.keyboardsurfer.android.widget:crouton:1.8.5@aar'
+    compile 'commons-codec:commons-codec:1.10'
-    compile group: 'commons-codec', name: 'commons-codec', version: '1.10'
     compile 'com.android.support:support-v4:23.4.0'
     compile 'com.android.support:appcompat-v7:23.4.0'
+    compile 'com.android.support:design:23.4.0'
+
+    //noinspection GradleDependency - old version has required feature
+    compile 'com.google.code.gson:gson:1.4'
 }
 
 android {
     compileSdkVersion 23
-    buildToolsVersion "23.0.2"
+    buildToolsVersion "23.0.3"
 
     useLibrary  'org.apache.http.legacy'
 

commons/app/src/main/AndroidManifest.xml

@@ -8,16 +8,16 @@
         android:targetSdkVersion="23" />
 
     <uses-permission android:name="android.permission.INTERNET" />
-    <uses-permission android:name="android.permission.AUTHENTICATE_ACCOUNTS"/>
-    <uses-permission android:name="android.permission.GET_ACCOUNTS"/>
-    <uses-permission android:name="android.permission.USE_CREDENTIALS"/>
-    <uses-permission android:name="android.permission.MANAGE_ACCOUNTS"/>
     <uses-permission android:name="android.permission.READ_EXTERNAL_STORAGE"/>
     <uses-permission android:name="android.permission.READ_SYNC_SETTINGS"/>
     <uses-permission android:name="android.permission.READ_SYNC_STATS"/>
     <uses-permission android:name="android.permission.WRITE_SYNC_SETTINGS"/>
     <uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE"/>
     <uses-permission android:name="android.permission.ACCESS_FINE_LOCATION" />
+    <uses-permission android:name="android.permission.AUTHENTICATE_ACCOUNTS"/>
+    <uses-permission android:name="android.permission.GET_ACCOUNTS"/>
+    <uses-permission android:name="android.permission.USE_CREDENTIALS"/>
+    <uses-permission android:name="android.permission.MANAGE_ACCOUNTS"/>
 
     <application
         android:name=".CommonsApplication"

commons/app/src/main/java/fr/free/nrw/commons/auth/LoginActivity.java

@@ -1,26 +1,39 @@
 package fr.free.nrw.commons.auth;
 
+import android.accounts.Account;
+import android.accounts.AccountAuthenticatorActivity;
+import android.accounts.AccountAuthenticatorResponse;
+import android.accounts.AccountManager;
+import android.app.Activity;
+import android.app.ProgressDialog;
+import android.content.ContentResolver;
+import android.content.Intent;
+import android.net.Uri;
+import android.os.AsyncTask;
+import android.os.Bundle;
+import android.support.design.widget.Snackbar;
+import android.support.v4.app.NavUtils;
+import android.text.Editable;
+import android.text.TextWatcher;
+import android.util.Log;
+import android.view.KeyEvent;
+import android.view.Menu;
+import android.view.MenuItem;
+import android.view.View;
+import android.view.inputmethod.EditorInfo;
+import android.widget.Button;
+import android.widget.EditText;
+import android.widget.TextView;
+import android.widget.Toast;
+
 import java.io.IOException;
 
-import android.content.*;
-import android.net.Uri;
-import android.text.*;
-import android.view.inputmethod.EditorInfo;
-import de.keyboardsurfer.android.widget.crouton.*;
-import android.os.*;
-import android.accounts.*;
-import android.app.*;
-import android.util.*;
-import android.view.*;
-import android.widget.*;
-import android.support.v4.app.NavUtils;
-
-import fr.free.nrw.commons.*;
-import fr.free.nrw.commons.WelcomeActivity;
-import fr.free.nrw.commons.modifications.ModificationsContentProvider;
 import fr.free.nrw.commons.CommonsApplication;
 import fr.free.nrw.commons.EventLog;
-import fr.free.nrw.commons.contributions.*;
+import fr.free.nrw.commons.R;
+import fr.free.nrw.commons.WelcomeActivity;
+import fr.free.nrw.commons.contributions.ContributionsContentProvider;
+import fr.free.nrw.commons.modifications.ModificationsContentProvider;
 
 
 public class LoginActivity extends AccountAuthenticatorActivity {
@@ -79,7 +92,7 @@ public class LoginActivity extends AccountAuthenticatorActivity {
                 } else if(result.equals("NotExists") || result.equals("Illegal") || result.equals("NotExists")) {
                     response = R.string.login_failed_username;
                     passwordEdit.setText("");
-                } else if(result.equals("EmptyPass") || result.equals("WrongPass")) {
+                } else if(result.equals("EmptyPass") || result.equals("WrongPass") || result.equals("WrongPluginPass")) {
                     response = R.string.login_failed_password;
                     passwordEdit.setText("");
                 } else if(result.equals("Throttled")) {
@@ -88,10 +101,11 @@ public class LoginActivity extends AccountAuthenticatorActivity {
                     response = R.string.login_failed_blocked;
                 } else {
                     // Should never really happen
+                    Log.d("Commons", "Login failed with reason: " + result);
                     response = R.string.login_failed_generic;
                 }
-                Crouton.makeText(context, response, Style.ALERT, R.id.loginErrors).show();
+                Toast.makeText(getApplicationContext(), response, Toast.LENGTH_LONG).show();
-                dialog.dismiss();
+                dialog.cancel();
             }
 
         }

commons/app/src/main/java/fr/free/nrw/commons/upload/GPSExtractor.java

@@ -9,6 +9,7 @@ import android.location.LocationManager;
 import android.media.ExifInterface;
 import android.os.Bundle;
 import android.preference.PreferenceManager;
+import android.support.design.widget.Snackbar;
 import android.util.Log;
 import java.io.IOException;
 
@@ -71,7 +72,7 @@ public class GPSExtractor {
      * Extracts geolocation of image from EXIF data.
      * @return coordinates of image as string (needs to be passed as a String in API query)
      */
-    public String getCoords() {
+    public String getCoords(boolean useGPS) {
 
         ExifInterface exif;
         String latitude = "";
@@ -87,25 +88,27 @@ public class GPSExtractor {
             return null;
         }
 
-        if (exif.getAttribute(ExifInterface.TAG_GPS_LATITUDE) == null) {
+        if (exif.getAttribute(ExifInterface.TAG_GPS_LATITUDE) == null && useGPS) {
+            registerLocationManager();
+
             imageCoordsExists = false;
-            Log.d(TAG, "Picture has no GPS info");
+            Log.d(TAG, "EXIF data has no location info");
 
             //Check what user's preference is for automatic location detection
             boolean gpsPrefEnabled = gpsPreferenceEnabled();
 
             if (gpsPrefEnabled) {
                 Log.d(TAG, "Current location values: Lat = " + currentLatitude + " Long = " + currentLongitude);
-                String currentCoords = String.valueOf(currentLatitude) + "|" + String.valueOf(currentLongitude);
+                return String.valueOf(currentLatitude) + "|" + String.valueOf(currentLongitude);
-                return currentCoords;
             } else {
-                //Otherwise treat as if no coords found
+                // No coords found
                 return null;
             }
-
+        } else if(exif.getAttribute(ExifInterface.TAG_GPS_LATITUDE) == null) {
+            return null;
         } else {
             imageCoordsExists = true;
-            Log.d(TAG, "Picture has GPS info");
+            Log.d(TAG, "EXIF data has location info");
 
             latitude = exif.getAttribute(ExifInterface.TAG_GPS_LATITUDE);
             latitude_ref = exif.getAttribute(ExifInterface.TAG_GPS_LATITUDE_REF);

commons/app/src/main/java/fr/free/nrw/commons/upload/ShareActivity.java

@@ -1,14 +1,22 @@
 package fr.free.nrw.commons.upload;
 
+import android.Manifest;
 import android.content.ContentResolver;
 import android.content.Intent;
+import android.content.pm.PackageManager;
 import android.net.Uri;
+import android.os.Build;
 import android.os.Bundle;
 import android.os.Environment;
+import android.support.design.widget.Snackbar;
+import android.support.v4.app.ActivityCompat;
 import android.support.v4.app.NavUtils;
+import android.support.v4.content.ContextCompat;
 import android.util.Log;
 import android.view.MenuItem;
+import android.view.View;
 import android.widget.ImageView;
+import android.widget.TextView;
 import android.widget.Toast;
 
 import com.nostra13.universalimageloader.core.ImageLoader;
@@ -19,9 +27,10 @@ import java.util.List;
 import fr.free.nrw.commons.CommonsApplication;
 import fr.free.nrw.commons.EventLog;
 import fr.free.nrw.commons.R;
-import fr.free.nrw.commons.auth.*;
+import fr.free.nrw.commons.auth.AuthenticatedActivity;
+import fr.free.nrw.commons.auth.WikiAccountAuthenticator;
 import fr.free.nrw.commons.category.CategorizationFragment;
-import fr.free.nrw.commons.contributions.*;
+import fr.free.nrw.commons.contributions.Contribution;
 import fr.free.nrw.commons.modifications.CategoryModifier;
 import fr.free.nrw.commons.modifications.ModificationsContentProvider;
 import fr.free.nrw.commons.modifications.ModifierSequence;
@@ -56,6 +65,12 @@ public  class       ShareActivity
     private boolean cacheFound;
 
     private GPSExtractor imageObj;
+    private String filePath;
+    private String decimalCoords;
+
+    private boolean useNewPermissions = false;
+    private boolean storagePermission = false;
+    private boolean locationPermission = false;
 
     public ShareActivity() {
         super(WikiAccountAuthenticator.COMMONS_ACCOUNT_TYPE);
@@ -167,6 +182,11 @@ public  class       ShareActivity
         finish();
     }
 
+    /**
+     * Initiates retrieval of image coordinates or user coordinates, and caching of coordinates.
+     * Then initiates the calls to MediaWiki API through an instance of MwVolleyApi.
+     */
+
     @Override
     public void onCreate(Bundle savedInstanceState) {
         super.onCreate(savedInstanceState);
@@ -197,31 +217,124 @@ public  class       ShareActivity
             contribution = savedInstanceState.getParcelable("contribution");
         }
         requestAuthToken();
-    }
 
-    /**
-     * Initiates retrieval of image coordinates or user coordinates, and caching of coordinates.
-     * Then initiates the calls to MediaWiki API through an instance of MwVolleyApi.
-     */
-    @Override
-    public void onResume() {
-        super.onResume();
 
         Log.d(TAG, "Uri: " + mediaUriString);
         Log.d(TAG, "Ext storage dir: " + Environment.getExternalStorageDirectory());
 
-        //convert image Uri to file path
+        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
-        String filePath = FileUtils.getPath(this, mediaUri);
+            useNewPermissions = true;
-        Log.d(TAG, "Filepath: " + filePath);
+            if(ContextCompat.checkSelfPermission(this, Manifest.permission.READ_EXTERNAL_STORAGE) == PackageManager.PERMISSION_GRANTED) {
+                storagePermission = true;
+            }
+            if (ContextCompat.checkSelfPermission(this, Manifest.permission.ACCESS_FINE_LOCATION) == PackageManager.PERMISSION_GRANTED) {
+                locationPermission = true;
+            }
+        }
 
+        // Check storage permissions if marshmallow or newer
+        if (useNewPermissions && (!storagePermission || !locationPermission)) {
+            if (!storagePermission && !locationPermission) {
+                String permissionRationales = getResources().getString(R.string.storage_permission_rationale) + "\n" + getResources().getString(R.string.location_permission_rationale);
+                Snackbar snackbar = Snackbar.make(findViewById(android.R.id.content), permissionRationales,
+                        Snackbar.LENGTH_INDEFINITE)
+                        .setAction(R.string.ok, new View.OnClickListener() {
+                            @Override
+                            public void onClick(View view) {
+                                ActivityCompat.requestPermissions(ShareActivity.this,
+                                        new String[]{Manifest.permission.READ_EXTERNAL_STORAGE, Manifest.permission.ACCESS_FINE_LOCATION}, 3);
+                            }
+                        });
+                snackbar.show();
+                View snackbarView = snackbar.getView();
+                TextView textView = (TextView) snackbarView.findViewById(android.support.design.R.id.snackbar_text);
+                textView.setMaxLines(3);
+            } else if (!storagePermission) {
+                Snackbar.make(findViewById(android.R.id.content), R.string.storage_permission_rationale,
+                        Snackbar.LENGTH_INDEFINITE)
+                        .setAction(R.string.ok, new View.OnClickListener() {
+                            @Override
+                            public void onClick(View view) {
+                                ActivityCompat.requestPermissions(ShareActivity.this,
+                                        new String[]{Manifest.permission.READ_EXTERNAL_STORAGE}, 1);
+                            }
+                        }).show();
+            } else if (!locationPermission) {
+                Snackbar.make(findViewById(android.R.id.content), R.string.location_permission_rationale,
+                        Snackbar.LENGTH_INDEFINITE)
+                        .setAction(R.string.ok, new View.OnClickListener() {
+                            @Override
+                            public void onClick(View view) {
+                                ActivityCompat.requestPermissions(ShareActivity.this,
+                                        new String[]{Manifest.permission.ACCESS_FINE_LOCATION}, 2);
+                            }
+                        }).show();
+            }
+        } else if (useNewPermissions && storagePermission && !locationPermission) {
+            getFileMetadata();
+        } else if(!useNewPermissions || (storagePermission && locationPermission)) {
+            getFileMetadata();
+            getLocationData();
+        }
+    }
+
+    @Override
+    public void onRequestPermissionsResult(int requestCode,
+                                           String permissions[], int[] grantResults) {
+        switch (requestCode) {
+            // 1 = Storage
+            case 1: {
+                if (grantResults.length > 0
+                        && grantResults[0] == PackageManager.PERMISSION_GRANTED) {
+                    getFileMetadata();
+                }
+                return;
+            }
+            // 2 = Location
+            case 2: {
+                if (grantResults.length > 0
+                        && grantResults[0] == PackageManager.PERMISSION_GRANTED) {
+                    getLocationData();
+                }
+                return;
+            }
+            // 3 = Storage + Location
+            case 3: {
+                if (grantResults.length > 1
+                        && grantResults[0] == PackageManager.PERMISSION_GRANTED) {
+                    getFileMetadata();
+                }
+                if (grantResults.length > 1
+                        && grantResults[1] == PackageManager.PERMISSION_GRANTED) {
+                    getLocationData();
+                }
+            }
+        }
+    }
+
+    public void getFileMetadata() {
+        filePath = FileUtils.getPath(this, mediaUri);
+        Log.d(TAG, "Filepath: " + filePath);
         Log.d(TAG, "Calling GPSExtractor");
         imageObj = new GPSExtractor(filePath, this);
-        imageObj.registerLocationManager();
 
         if (filePath != null && !filePath.equals("")) {
-            //Gets image coords if exist, otherwise gets last known coords
+            // Gets image coords from exif data
-            String decimalCoords = imageObj.getCoords();
+            decimalCoords = imageObj.getCoords(false);
+            useImageCoords();
+        }
+    }
 
+    public void getLocationData() {
+        if(imageObj == null) {
+            imageObj = new GPSExtractor(filePath, this);
+        }
+
+        decimalCoords = imageObj.getCoords(true);
+        useImageCoords();
+    }
+
+    public void useImageCoords() {
         if(decimalCoords != null) {
             Log.d(TAG, "Decimal coords of image: " + decimalCoords);
 
@@ -237,12 +350,11 @@ public  class       ShareActivity
             List displayCatList = app.cacheData.findCategory();
             boolean catListEmpty = displayCatList.isEmpty();
 
-                //if no categories found in cache, call MW API to match image coords with nearby Commons categories
+            // If no categories found in cache, call MediaWiki API to match image coords with nearby Commons categories
             if (catListEmpty) {
                 cacheFound = false;
                 apiCall.request(decimalCoords);
                 Log.d(TAG, "displayCatList size 0, calling MWAPI" + displayCatList.toString());
-
             } else {
                 cacheFound = true;
                 Log.d(TAG, "Cache found, setting categoryList in MwVolleyApi to " + displayCatList.toString());
@@ -250,12 +362,17 @@ public  class       ShareActivity
             }
         }
     }
-    }
 
     @Override
     public void onPause() {
         super.onPause();
+        try {
             imageObj.unregisterLocationManager();
+            Log.d(TAG, "Unregistered locationManager");
+        }
+        catch (NullPointerException e) {
+            Log.d(TAG, "locationManager does not exist, not unregistered");
+        }
     }
 
     @Override

commons/app/src/main/res/values/strings.xml

@@ -148,4 +148,9 @@
     <string name="detail_license_empty">Unknown license</string>
     <string name="provider_campaigns">Campaigns</string>
     <string name="menu_refresh">Refresh</string>
+
+    <string name="storage_permission_rationale">Recommended: Storage for photo metadata</string>
+    <string name="location_permission_rationale">Optional: Current location for category suggestions</string>
+    <string name="ok">OK</string>
+    <string name="back">Back</string>
 </resources>

commons/build.gradle

@@ -5,7 +5,7 @@ buildscript {
         mavenCentral()
     }
     dependencies {
-        classpath 'com.android.tools.build:gradle:2.1.0'
+        classpath 'com.android.tools.build:gradle:2.1.2'
     }
 }