Skip to content

Commit

Permalink
Storage runtime permissions resolves getodk#644 (getodk#2119)
Browse files Browse the repository at this point in the history
* moved view instantiation code above permission check since onResume is making calls to some elements and grabbing views from the layout won't be impacted by result of permission check.

* Added GrantPermissionRule so that the tests on API 23 and above would be granted all the necessary permissions before they run.

* Applied permission rules and checks to all Content Providers and Activities that can be launched from an intent.

* removed due to no other declarants of these permissions being present. they were added as a fix to permission dialogs that weren't showing up during debugging.

* refactored initialization statements that were pulled from onCreate of FormEntryActivity

* fixed a bug that occurred due to requestWindowFeature causing a crash

RequestWindowFeature should always be called   before making any view related calls such as SetContentView.

* Added storage permission checks to all activities so when the permission is revoked it's handled gracefully.

Entry point activities were able to handle storage permissions but all other activities weren't so they crashed when the user manually revoked the permission. Now when a user revokes a permission the app shows a dialog about it and then quits. This approach is taken because the revoking of storage permissions is an edge case in a sense and if an user actually does it, it's easier for them to restart the app via an entry point than to have permission granted code in all activities.
  • Loading branch information
jd-alexander authored and lognaturel committed May 14, 2018
1 parent eb3c9ee commit 9d323a6
Show file tree
Hide file tree
Showing 16 changed files with 547 additions and 213 deletions.
5 changes: 2 additions & 3 deletions collect_app/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -205,15 +205,15 @@ dependencies {
exclude group: 'org.apache.httpcomponents'
}

implementation group: 'com.evernote', name: 'android-job', version: '1.2.5'
implementation "com.evernote:android-job:1.2.5"
implementation "com.rarepebble:colorpicker:2.3.1"
implementation "commons-io:commons-io:2.6"
implementation "net.sf.kxml:kxml2:2.3.0"
implementation "net.sf.opencsv:opencsv:2.3"
implementation("org.opendatakit:opendatakit-javarosa:2.9.0") {
exclude module: 'joda-time'
}

implementation "com.karumi:dexter:4.2.0"
implementation "org.osmdroid:osmdroid-android:5.6.4"
implementation "org.slf4j:slf4j-android:1.6.1-RC1"
implementation "pub.devrel:easypermissions:0.2.1"
Expand Down Expand Up @@ -294,7 +294,6 @@ dependencies {
androidTestImplementation "com.squareup.okhttp3:mockwebserver:3.9.0"
}


// Must be at bottom to prevent dependency collisions
// https://developers.google.com/android/guides/google-services-plugin
apply plugin: 'com.google.gms.google-services'
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.odk.collect.android;

import android.Manifest;
import android.app.Instrumentation.ActivityResult;
import android.content.Context;
import android.content.Intent;
Expand All @@ -13,6 +14,7 @@
import android.support.test.espresso.intent.rule.IntentsTestRule;
import android.support.test.espresso.matcher.BoundedMatcher;
import android.support.test.espresso.matcher.ViewMatchers;
import android.support.test.rule.GrantPermissionRule;
import android.support.test.runner.AndroidJUnit4;
import android.view.View;
import android.widget.SeekBar;
Expand Down Expand Up @@ -106,6 +108,9 @@ public class AllWidgetsFormTest {
@Rule
public MockitoRule rule = MockitoJUnit.rule();

@Rule
public GrantPermissionRule permissionRule = GrantPermissionRule.grant(Manifest.permission.READ_EXTERNAL_STORAGE, Manifest.permission.WRITE_EXTERNAL_STORAGE);

@Mock
private ActivityAvailability activityAvailability;

Expand Down
2 changes: 1 addition & 1 deletion collect_app/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ the specific language governing permissions and limitations under the License.
<uses-permission android:name="android.permission.ACCESS_FINE_LOCATION" />
<uses-permission android:name="android.permission.ACCESS_COARSE_LOCATION" />
<uses-permission android:name="android.permission.READ_EXTERNAL_STORAGE" />
<uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE" />
<uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE"/>
<uses-permission android:name="android.permission.GET_ACCOUNTS" />
<uses-permission android:name="android.permission.USE_CREDENTIALS" />
<uses-permission android:name="android.permission.ACCESS_NETWORK_STATE" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,16 @@

import android.os.Bundle;
import android.support.annotation.Nullable;
import android.support.v7.app.AlertDialog;
import android.support.v7.app.AppCompatActivity;

import org.odk.collect.android.R;
import org.odk.collect.android.utilities.ThemeUtils;

import static org.odk.collect.android.utilities.PermissionUtils.checkIfStoragePermissionsGranted;
import static org.odk.collect.android.utilities.PermissionUtils.finishAllActivities;
import static org.odk.collect.android.utilities.PermissionUtils.isEntryPointActivity;

public abstract class CollectAbstractActivity extends AppCompatActivity {

private boolean isInstanceStateSaved;
Expand All @@ -32,6 +38,27 @@ protected void onCreate(@Nullable Bundle savedInstanceState) {
themeUtils = new ThemeUtils(this);
setTheme(themeUtils.getAppTheme());
super.onCreate(savedInstanceState);

/**
* If a user has revoked the storage permission then this check ensures the app doesn't quit unexpectedly and
* informs the user of the implications of their decision before exiting. The app can't function with these permissions
* so if a user wishes to grant them they just restart.
*
* This code won't run on activities that are entry points to the app because those activities
* are able to handle permission checks and requests by themselves.
*/
if (!checkIfStoragePermissionsGranted(this) && !isEntryPointActivity(this)) {
AlertDialog.Builder builder = new AlertDialog.Builder(this, R.style.Theme_AppCompat_Light_Dialog);

builder.setTitle(R.string.storage_runtime_permission_denied_title)
.setMessage(R.string.storage_runtime_permission_denied_desc)
.setPositiveButton(android.R.string.ok, (dialogInterface, i) -> {
finishAllActivities(this);
})
.setIcon(R.drawable.sd)
.setCancelable(false)
.show();
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,17 @@
import org.odk.collect.android.application.Collect;
import org.odk.collect.android.dao.FormsDao;
import org.odk.collect.android.listeners.DiskSyncListener;
import org.odk.collect.android.listeners.PermissionListener;
import org.odk.collect.android.provider.FormsProviderAPI.FormsColumns;
import org.odk.collect.android.tasks.DiskSyncTask;
import org.odk.collect.android.utilities.ApplicationConstants;
import org.odk.collect.android.utilities.VersionHidingCursorAdapter;

import timber.log.Timber;

import static org.odk.collect.android.utilities.PermissionUtils.finishAllActivities;
import static org.odk.collect.android.utilities.PermissionUtils.requestStoragePermissions;

/**
* Responsible for displaying all the valid forms in the forms directory. Stores the path to
* selected form for use by {@link MainMenuActivity}.
Expand All @@ -55,20 +59,36 @@ public class FormChooserList extends FormListActivity implements

@Override
public void onCreate(Bundle savedInstanceState) {

// must be at the beginning of any activity that can be called from an external intent
try {
Collect.createODKDirs();
} catch (RuntimeException e) {
createErrorDialog(e.getMessage(), EXIT);
return;
}

super.onCreate(savedInstanceState);
setContentView(R.layout.chooser_list_layout);

setTitle(getString(R.string.enter_data));

requestStoragePermissions(this, new PermissionListener() {
@Override
public void granted() {
// must be at the beginning of any activity that can be called from an external intent
try {
Collect.createODKDirs();
Collect.getInstance().getActivityLogger().open();
init();
} catch (RuntimeException e) {
createErrorDialog(e.getMessage(), EXIT);
return;
}
}

@Override
public void denied() {
// The activity has to finish because ODK Collect cannot function without these permissions.
finishAllActivities(FormChooserList.this);
}
});
}

private void init() {
setupAdapter();

// DiskSyncTask checks the disk for any forms not already in the content provider
// that is, put here by dragging and dropping onto the SDCard
diskSyncTask = (DiskSyncTask) getLastCustomNonConfigurationInstance();
Expand Down Expand Up @@ -116,24 +136,26 @@ public void onItemClick(AdapterView<?> parent, View view, int position, long id)
intent.putExtra(ApplicationConstants.BundleKeys.FORM_MODE, ApplicationConstants.FormModes.EDIT_SAVED);
startActivity(intent);
}

finish();
}
}

@Override
protected void onResume() {
diskSyncTask.setDiskSyncListener(this);
super.onResume();

if (diskSyncTask.getStatus() == AsyncTask.Status.FINISHED) {
syncComplete(diskSyncTask.getStatusMessage());
if (diskSyncTask != null) {
diskSyncTask.setDiskSyncListener(this);
if (diskSyncTask.getStatus() == AsyncTask.Status.FINISHED) {
syncComplete(diskSyncTask.getStatusMessage());
}
}
}

@Override
protected void onPause() {
diskSyncTask.setDiskSyncListener(null);
if (diskSyncTask != null) {
diskSyncTask.setDiskSyncListener(null);
}
super.onPause();
}

Expand Down
Loading

0 comments on commit 9d323a6

Please sign in to comment.