From 3192015cfdbd0c99b1419a0b87a4c4b86256af9d Mon Sep 17 00:00:00 2001 From: akwizgran Date: Fri, 5 Jan 2018 18:03:56 +0000 Subject: [PATCH 1/9] Ask for Bluetooth discoverability before adding a contact. --- .../briar/android/activity/RequestCodes.java | 2 +- .../keyagreement/KeyAgreementActivity.java | 65 ++++++++++++------- 2 files changed, 42 insertions(+), 25 deletions(-) diff --git a/briar-android/src/main/java/org/briarproject/briar/android/activity/RequestCodes.java b/briar-android/src/main/java/org/briarproject/briar/android/activity/RequestCodes.java index f3d72d17c..1709fc85a 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/activity/RequestCodes.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/activity/RequestCodes.java @@ -11,7 +11,7 @@ public interface RequestCodes { int REQUEST_RINGTONE = 7; int REQUEST_PERMISSION_CAMERA = 8; int REQUEST_DOZE_WHITELISTING = 9; - int REQUEST_ENABLE_BLUETOOTH = 10; + int REQUEST_BLUETOOTH_DISCOVERABLE = 10; int REQUEST_UNLOCK = 11; int REQUEST_KEYGUARD_UNLOCK = 12; diff --git a/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/KeyAgreementActivity.java b/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/KeyAgreementActivity.java index d74c3e248..14237029c 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/KeyAgreementActivity.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/KeyAgreementActivity.java @@ -38,14 +38,17 @@ import javax.annotation.Nullable; import javax.inject.Inject; import static android.Manifest.permission.CAMERA; -import static android.bluetooth.BluetoothAdapter.ACTION_REQUEST_ENABLE; +import static android.bluetooth.BluetoothAdapter.ACTION_REQUEST_DISCOVERABLE; +import static android.bluetooth.BluetoothAdapter.ACTION_SCAN_MODE_CHANGED; import static android.bluetooth.BluetoothAdapter.ACTION_STATE_CHANGED; +import static android.bluetooth.BluetoothAdapter.EXTRA_SCAN_MODE; import static android.bluetooth.BluetoothAdapter.EXTRA_STATE; +import static android.bluetooth.BluetoothAdapter.SCAN_MODE_CONNECTABLE_DISCOVERABLE; import static android.bluetooth.BluetoothAdapter.STATE_ON; import static android.content.pm.PackageManager.PERMISSION_GRANTED; import static android.os.Build.VERSION.SDK_INT; import static android.widget.Toast.LENGTH_LONG; -import static org.briarproject.briar.android.activity.RequestCodes.REQUEST_ENABLE_BLUETOOTH; +import static org.briarproject.briar.android.activity.RequestCodes.REQUEST_BLUETOOTH_DISCOVERABLE; import static org.briarproject.briar.android.activity.RequestCodes.REQUEST_PERMISSION_CAMERA; @MethodsNotNullByDefault @@ -55,7 +58,7 @@ public abstract class KeyAgreementActivity extends BriarActivity implements KeyAgreementEventListener { private enum BluetoothState { - UNKNOWN, NO_ADAPTER, WAITING, REFUSED, ENABLED + UNKNOWN, NO_ADAPTER, WAITING, REFUSED, ENABLED, DISCOVERABLE } private static final Logger LOG = @@ -64,7 +67,7 @@ public abstract class KeyAgreementActivity extends BriarActivity implements @Inject EventBus eventBus; - private boolean isResumed = false, enableWasRequested = false; + private boolean isResumed = false, wasAdapterEnabled = false; private boolean continueClicked, gotCameraPermission; private BluetoothState bluetoothState = BluetoothState.UNKNOWN; private BroadcastReceiver bluetoothReceiver = null; @@ -85,7 +88,9 @@ public abstract class KeyAgreementActivity extends BriarActivity implements if (state == null) { showInitialFragment(IntroFragment.newInstance()); } - IntentFilter filter = new IntentFilter(ACTION_STATE_CHANGED); + IntentFilter filter = new IntentFilter(); + filter.addAction(ACTION_STATE_CHANGED); + filter.addAction(ACTION_SCAN_MODE_CHANGED); bluetoothReceiver = new BluetoothStateReceiver(); registerReceiver(bluetoothReceiver, filter); } @@ -133,45 +138,48 @@ public abstract class KeyAgreementActivity extends BriarActivity implements public void showNextScreen() { continueClicked = true; if (checkPermissions()) { - if (shouldRequestEnableBluetooth()) requestEnableBluetooth(); - else if (canShowQrCodeFragment()) showQrCodeFragment(); + if (shouldRequestBluetoothDiscoverable()) { + requestBluetoothDiscoverable(); + } else if (canShowQrCodeFragment()) { + showQrCodeFragment(); + } } } - private boolean shouldRequestEnableBluetooth() { + private boolean shouldRequestBluetoothDiscoverable() { return bluetoothState == BluetoothState.UNKNOWN || bluetoothState == BluetoothState.REFUSED; } - private void requestEnableBluetooth() { + private void requestBluetoothDiscoverable() { BluetoothAdapter bt = BluetoothAdapter.getDefaultAdapter(); if (bt == null) { setBluetoothState(BluetoothState.NO_ADAPTER); - } else if (bt.isEnabled()) { - setBluetoothState(BluetoothState.ENABLED); - } else { - enableWasRequested = true; - setBluetoothState(BluetoothState.WAITING); - Intent i = new Intent(ACTION_REQUEST_ENABLE); - startActivityForResult(i, REQUEST_ENABLE_BLUETOOTH); + return; } + setBluetoothState(BluetoothState.WAITING); + wasAdapterEnabled = bt.isEnabled(); + Intent i = new Intent(ACTION_REQUEST_DISCOVERABLE); + startActivityForResult(i, REQUEST_BLUETOOTH_DISCOVERABLE); } private void setBluetoothState(BluetoothState bluetoothState) { LOG.info("Setting Bluetooth state to " + bluetoothState); this.bluetoothState = bluetoothState; - if (enableWasRequested && bluetoothState == BluetoothState.ENABLED) { + if (!wasAdapterEnabled && bluetoothState == BluetoothState.ENABLED) { eventBus.broadcast(new BluetoothEnabledEvent()); - enableWasRequested = false; + wasAdapterEnabled = true; } if (canShowQrCodeFragment()) showQrCodeFragment(); } @Override public void onActivityResult(int request, int result, Intent data) { - // If the request was granted we'll catch the state change event - if (request == REQUEST_ENABLE_BLUETOOTH && result == RESULT_CANCELED) - setBluetoothState(BluetoothState.REFUSED); + if (request == REQUEST_BLUETOOTH_DISCOVERABLE) { + // If the request was granted we'll catch the state change event + if (result == RESULT_CANCELED) + setBluetoothState(BluetoothState.REFUSED); + } } private void showQrCodeFragment() { @@ -259,9 +267,18 @@ public abstract class KeyAgreementActivity extends BriarActivity implements @Override public void onReceive(Context context, Intent intent) { - int state = intent.getIntExtra(EXTRA_STATE, 0); - if (state == STATE_ON) setBluetoothState(BluetoothState.ENABLED); - else setBluetoothState(BluetoothState.UNKNOWN); + String action = intent.getAction(); + if (ACTION_STATE_CHANGED.equals(action)) { + int state = intent.getIntExtra(EXTRA_STATE, 0); + if (state == STATE_ON) + setBluetoothState(BluetoothState.ENABLED); + else setBluetoothState(BluetoothState.UNKNOWN); + } else if (ACTION_SCAN_MODE_CHANGED.equals(action)) { + int scanMode = intent.getIntExtra(EXTRA_SCAN_MODE, -1); + if (scanMode == SCAN_MODE_CONNECTABLE_DISCOVERABLE) + setBluetoothState(BluetoothState.DISCOVERABLE); + else setBluetoothState(BluetoothState.UNKNOWN); + } } } } From bd00fb1c04046e98ec31609117942efd71f6a9f4 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Thu, 11 Jan 2018 12:14:23 +0000 Subject: [PATCH 2/9] Ask for coarse location permission before adding a contact. --- briar-android/src/main/AndroidManifest.xml | 1 + .../briar/android/activity/RequestCodes.java | 2 +- .../keyagreement/KeyAgreementActivity.java | 116 +++++++++++------- briar-android/src/main/res/values/strings.xml | 4 + 4 files changed, 76 insertions(+), 47 deletions(-) diff --git a/briar-android/src/main/AndroidManifest.xml b/briar-android/src/main/AndroidManifest.xml index 885429cc2..b5a63032d 100644 --- a/briar-android/src/main/AndroidManifest.xml +++ b/briar-android/src/main/AndroidManifest.xml @@ -7,6 +7,7 @@ + diff --git a/briar-android/src/main/java/org/briarproject/briar/android/activity/RequestCodes.java b/briar-android/src/main/java/org/briarproject/briar/android/activity/RequestCodes.java index 1709fc85a..5d087d88b 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/activity/RequestCodes.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/activity/RequestCodes.java @@ -9,7 +9,7 @@ public interface RequestCodes { int REQUEST_WRITE_BLOG_POST = 5; int REQUEST_SHARE_BLOG = 6; int REQUEST_RINGTONE = 7; - int REQUEST_PERMISSION_CAMERA = 8; + int REQUEST_PERMISSION_CAMERA_LOCATION = 8; int REQUEST_DOZE_WHITELISTING = 9; int REQUEST_BLUETOOTH_DISCOVERABLE = 10; int REQUEST_UNLOCK = 11; diff --git a/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/KeyAgreementActivity.java b/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/KeyAgreementActivity.java index 14237029c..55d8ea5b3 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/KeyAgreementActivity.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/KeyAgreementActivity.java @@ -3,7 +3,6 @@ package org.briarproject.briar.android.keyagreement; import android.bluetooth.BluetoothAdapter; import android.content.BroadcastReceiver; import android.content.Context; -import android.content.DialogInterface.OnClickListener; import android.content.Intent; import android.content.IntentFilter; import android.os.Bundle; @@ -37,19 +36,21 @@ import java.util.logging.Logger; import javax.annotation.Nullable; import javax.inject.Inject; +import static android.Manifest.permission.ACCESS_COARSE_LOCATION; import static android.Manifest.permission.CAMERA; import static android.bluetooth.BluetoothAdapter.ACTION_REQUEST_DISCOVERABLE; import static android.bluetooth.BluetoothAdapter.ACTION_SCAN_MODE_CHANGED; import static android.bluetooth.BluetoothAdapter.ACTION_STATE_CHANGED; import static android.bluetooth.BluetoothAdapter.EXTRA_SCAN_MODE; import static android.bluetooth.BluetoothAdapter.EXTRA_STATE; +import static android.bluetooth.BluetoothAdapter.SCAN_MODE_CONNECTABLE; import static android.bluetooth.BluetoothAdapter.SCAN_MODE_CONNECTABLE_DISCOVERABLE; import static android.bluetooth.BluetoothAdapter.STATE_ON; import static android.content.pm.PackageManager.PERMISSION_GRANTED; import static android.os.Build.VERSION.SDK_INT; import static android.widget.Toast.LENGTH_LONG; import static org.briarproject.briar.android.activity.RequestCodes.REQUEST_BLUETOOTH_DISCOVERABLE; -import static org.briarproject.briar.android.activity.RequestCodes.REQUEST_PERMISSION_CAMERA; +import static org.briarproject.briar.android.activity.RequestCodes.REQUEST_PERMISSION_CAMERA_LOCATION; @MethodsNotNullByDefault @ParametersNotNullByDefault @@ -68,7 +69,7 @@ public abstract class KeyAgreementActivity extends BriarActivity implements EventBus eventBus; private boolean isResumed = false, wasAdapterEnabled = false; - private boolean continueClicked, gotCameraPermission; + private boolean continueClicked, gotCameraPermission, gotLocationPermission; private BluetoothState bluetoothState = BluetoothState.UNKNOWN; private BroadcastReceiver bluetoothReceiver = null; @@ -155,12 +156,12 @@ public abstract class KeyAgreementActivity extends BriarActivity implements BluetoothAdapter bt = BluetoothAdapter.getDefaultAdapter(); if (bt == null) { setBluetoothState(BluetoothState.NO_ADAPTER); - return; + } else { + setBluetoothState(BluetoothState.WAITING); + wasAdapterEnabled = bt.isEnabled(); + Intent i = new Intent(ACTION_REQUEST_DISCOVERABLE); + startActivityForResult(i, REQUEST_BLUETOOTH_DISCOVERABLE); } - setBluetoothState(BluetoothState.WAITING); - wasAdapterEnabled = bt.isEnabled(); - Intent i = new Intent(ACTION_REQUEST_DISCOVERABLE); - startActivityForResult(i, REQUEST_BLUETOOTH_DISCOVERABLE); } private void setBluetoothState(BluetoothState bluetoothState) { @@ -202,62 +203,83 @@ public abstract class KeyAgreementActivity extends BriarActivity implements } private boolean checkPermissions() { - if (ContextCompat.checkSelfPermission(this, CAMERA) != - PERMISSION_GRANTED) { - // Should we show an explanation? - if (ActivityCompat.shouldShowRequestPermissionRationale(this, - CAMERA)) { - OnClickListener continueListener = - (dialog, which) -> requestPermission(); - Builder builder = new Builder(this, style.BriarDialogTheme); - builder.setTitle(string.permission_camera_title); - builder.setMessage(string.permission_camera_request_body); - builder.setNeutralButton(string.continue_button, - continueListener); - builder.show(); - } else { - requestPermission(); - } - gotCameraPermission = false; - return false; - } else { - gotCameraPermission = true; + gotCameraPermission = checkPermission(CAMERA); + gotLocationPermission = checkPermission(ACCESS_COARSE_LOCATION); + if (gotCameraPermission && gotLocationPermission) return true; + // Should we show an explanation for one or both permissions? + boolean cameraRationale = shouldShowRationale(CAMERA); + boolean locationRationale = shouldShowRationale(ACCESS_COARSE_LOCATION); + if (cameraRationale && locationRationale) { + showRationale(string.permission_camera_location_title, + string.permission_camera_location_request_body); + } else if (cameraRationale) { + showRationale(string.permission_camera_title, + string.permission_camera_request_body); + } else if (locationRationale) { + showRationale(string.permission_location_title, + string.permission_location_request_body); + } else if (gotCameraPermission) { + // Location permission has been permanently denied but we can + // continue without it return true; + } else { + requestPermissions(); } + return false; } - private void requestPermission() { - ActivityCompat.requestPermissions(this, new String[] {CAMERA}, - REQUEST_PERMISSION_CAMERA); + private boolean checkPermission(String permission) { + return ContextCompat.checkSelfPermission(this, permission) + == PERMISSION_GRANTED; + } + + private boolean shouldShowRationale(String permission) { + return ActivityCompat.shouldShowRequestPermissionRationale(this, + permission); + } + + private void showRationale(@StringRes int title, @StringRes int body) { + Builder builder = new Builder(this, style.BriarDialogTheme); + builder.setTitle(title); + builder.setMessage(body); + builder.setNeutralButton(string.continue_button, + (dialog, which) -> requestPermissions()); + builder.show(); + } + + private void requestPermissions() { + ActivityCompat.requestPermissions(this, + new String[] {CAMERA, ACCESS_COARSE_LOCATION}, + REQUEST_PERMISSION_CAMERA_LOCATION); } @Override @UiThread public void onRequestPermissionsResult(int requestCode, String permissions[], int[] grantResults) { - if (requestCode == REQUEST_PERMISSION_CAMERA) { - // If request is cancelled, the result arrays are empty. - if (grantResults.length > 0 && - grantResults[0] == PERMISSION_GRANTED) { - gotCameraPermission = true; + if (requestCode == REQUEST_PERMISSION_CAMERA_LOCATION) { + // If request is cancelled, the result arrays are empty + gotCameraPermission = grantResults.length > 0 + && grantResults[0] == PERMISSION_GRANTED; + gotLocationPermission = grantResults.length > 1 + && grantResults[1] == PERMISSION_GRANTED; + if (gotCameraPermission) { showNextScreen(); } else { - if (!ActivityCompat.shouldShowRequestPermissionRationale(this, - CAMERA)) { + if (shouldShowRationale(CAMERA)) { + Toast.makeText(this, string.permission_camera_denied_toast, + LENGTH_LONG).show(); + supportFinishAfterTransition(); + } else { // The user has permanently denied the request - OnClickListener cancelListener = - (dialog, which) -> supportFinishAfterTransition(); Builder builder = new Builder(this, style.BriarDialogTheme); builder.setTitle(string.permission_camera_title); builder.setMessage(string.permission_camera_denied_body); builder.setPositiveButton(string.ok, UiUtils.getGoToSettingsListener(this)); - builder.setNegativeButton(string.cancel, cancelListener); + builder.setNegativeButton(string.cancel, + (dialog, which) -> supportFinishAfterTransition()); builder.show(); - } else { - Toast.makeText(this, string.permission_camera_denied_toast, - LENGTH_LONG).show(); - supportFinishAfterTransition(); } } } @@ -274,9 +296,11 @@ public abstract class KeyAgreementActivity extends BriarActivity implements setBluetoothState(BluetoothState.ENABLED); else setBluetoothState(BluetoothState.UNKNOWN); } else if (ACTION_SCAN_MODE_CHANGED.equals(action)) { - int scanMode = intent.getIntExtra(EXTRA_SCAN_MODE, -1); + int scanMode = intent.getIntExtra(EXTRA_SCAN_MODE, 0); if (scanMode == SCAN_MODE_CONNECTABLE_DISCOVERABLE) setBluetoothState(BluetoothState.DISCOVERABLE); + else if (scanMode == SCAN_MODE_CONNECTABLE) + setBluetoothState(BluetoothState.ENABLED); else setBluetoothState(BluetoothState.UNKNOWN); } } diff --git a/briar-android/src/main/res/values/strings.xml b/briar-android/src/main/res/values/strings.xml index 08afd7f57..92f05edcc 100644 --- a/briar-android/src/main/res/values/strings.xml +++ b/briar-android/src/main/res/values/strings.xml @@ -467,6 +467,10 @@ Camera permission To scan the QR code, Briar needs access to the camera. + Location permission + To discover Bluetooth devices, Briar needs permission to access your location.\n\nBriar does not store your location or share it with anyone. + Camera and location + To scan the QR code, Briar needs access to the camera.\n\nTo discover Bluetooth devices, Briar needs permission to access your location.\n\nBriar does not store your location or share it with anyone. You have denied access to the camera, but adding contacts requires using the camera.\n\nPlease consider granting access. Camera permission was not granted QR code From 8935ec2c2e7c57f0e57a792dd88da4665b1bc444 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Fri, 22 Jun 2018 16:23:36 +0100 Subject: [PATCH 3/9] Don't wait for state change if BT is already discoverable. --- .../android/keyagreement/KeyAgreementActivity.java | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/KeyAgreementActivity.java b/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/KeyAgreementActivity.java index 55d8ea5b3..6af2e5f28 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/KeyAgreementActivity.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/KeyAgreementActivity.java @@ -177,9 +177,16 @@ public abstract class KeyAgreementActivity extends BriarActivity implements @Override public void onActivityResult(int request, int result, Intent data) { if (request == REQUEST_BLUETOOTH_DISCOVERABLE) { - // If the request was granted we'll catch the state change event - if (result == RESULT_CANCELED) + if (result == RESULT_CANCELED) { setBluetoothState(BluetoothState.REFUSED); + } else { + // If Bluetooth is already discoverable, show the QR code - + // otherwise wait for the state or scan mode to change + BluetoothAdapter bt = BluetoothAdapter.getDefaultAdapter(); + if (bt == null) throw new AssertionError(); + if (bt.getScanMode() == SCAN_MODE_CONNECTABLE_DISCOVERABLE) + setBluetoothState(BluetoothState.DISCOVERABLE); + } } } From de611857cf1d9d5237bb6c5bc5bced47e37c17a3 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Fri, 22 Jun 2018 18:13:39 +0100 Subject: [PATCH 4/9] Discover BT devices if no address is provided. --- .../bluetooth/AndroidBluetoothPlugin.java | 85 +++++++++++++++++++ .../keyagreement/KeyAgreementConstants.java | 2 +- .../plugin/bluetooth/BluetoothPlugin.java | 36 +++++--- .../plugin/bluetooth/JavaBluetoothPlugin.java | 6 ++ 4 files changed, 114 insertions(+), 15 deletions(-) diff --git a/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothPlugin.java b/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothPlugin.java index a170cbccc..5e3973045 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothPlugin.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothPlugin.java @@ -21,13 +21,19 @@ import org.briarproject.bramble.util.AndroidUtils; import java.io.Closeable; import java.io.IOException; import java.security.SecureRandom; +import java.util.ArrayList; +import java.util.Collection; import java.util.UUID; +import java.util.concurrent.BlockingQueue; import java.util.concurrent.ExecutionException; import java.util.concurrent.Executor; +import java.util.concurrent.LinkedBlockingQueue; import java.util.logging.Logger; import javax.annotation.Nullable; +import static android.bluetooth.BluetoothAdapter.ACTION_DISCOVERY_FINISHED; +import static android.bluetooth.BluetoothAdapter.ACTION_DISCOVERY_STARTED; import static android.bluetooth.BluetoothAdapter.ACTION_SCAN_MODE_CHANGED; import static android.bluetooth.BluetoothAdapter.ACTION_STATE_CHANGED; import static android.bluetooth.BluetoothAdapter.EXTRA_SCAN_MODE; @@ -37,8 +43,12 @@ import static android.bluetooth.BluetoothAdapter.SCAN_MODE_CONNECTABLE_DISCOVERA import static android.bluetooth.BluetoothAdapter.SCAN_MODE_NONE; import static android.bluetooth.BluetoothAdapter.STATE_OFF; import static android.bluetooth.BluetoothAdapter.STATE_ON; +import static android.bluetooth.BluetoothDevice.ACTION_FOUND; +import static android.bluetooth.BluetoothDevice.EXTRA_DEVICE; +import static java.util.logging.Level.INFO; import static java.util.logging.Level.WARNING; import static org.briarproject.bramble.util.LogUtils.logException; +import static org.briarproject.bramble.util.PrivacyUtils.scrubMacAddress; @MethodsNotNullByDefault @ParametersNotNullByDefault @@ -182,6 +192,67 @@ class AndroidBluetoothPlugin extends BluetoothPlugin { } } + @Override + @Nullable + DuplexTransportConnection discoverAndConnect(String uuid) { + if (adapter == null) return null; + for (String address : discoverDevices()) { + try { + if (LOG.isLoggable(INFO)) + LOG.info("Connecting to " + scrubMacAddress(address)); + return connectTo(address, uuid); + } catch (IOException e) { + if (LOG.isLoggable(INFO)) { + LOG.info("Could not connect to " + + scrubMacAddress(address)); + } + } + } + LOG.info("Could not connect to any devices"); + return null; + } + + private Collection discoverDevices() { + Collection addresses = new ArrayList<>(); + BlockingQueue intents = new LinkedBlockingQueue<>(); + DiscoveryReceiver receiver = new DiscoveryReceiver(intents); + IntentFilter filter = new IntentFilter(); + filter.addAction(ACTION_DISCOVERY_STARTED); + filter.addAction(ACTION_DISCOVERY_FINISHED); + filter.addAction(ACTION_FOUND); + appContext.registerReceiver(receiver, filter); + try { + if (adapter.startDiscovery()) { + while (true) { + Intent i = intents.take(); + String action = i.getAction(); + if (ACTION_DISCOVERY_STARTED.equals(action)) { + LOG.info("Discovery started"); + } else if (ACTION_DISCOVERY_FINISHED.equals(action)) { + LOG.info("Discovery finished"); + break; + } else if (ACTION_FOUND.equals(action)) { + BluetoothDevice d = i.getParcelableExtra(EXTRA_DEVICE); + String address = d.getAddress(); + if (LOG.isLoggable(INFO)) + LOG.info("Discovered " + scrubMacAddress(address)); + if (!addresses.contains(address)) + addresses.add(address); + } + } + } else { + LOG.info("Could not start discovery"); + } + } catch (InterruptedException e) { + LOG.info("Interrupted while discovering devices"); + Thread.currentThread().interrupt(); + } finally { + adapter.cancelDiscovery(); + appContext.unregisterReceiver(receiver); + } + return addresses; + } + private void tryToClose(@Nullable Closeable c) { try { if (c != null) c.close(); @@ -207,4 +278,18 @@ class AndroidBluetoothPlugin extends BluetoothPlugin { } } } + + private static class DiscoveryReceiver extends BroadcastReceiver { + + private final BlockingQueue intents; + + private DiscoveryReceiver(BlockingQueue intents) { + this.intents = intents; + } + + @Override + public void onReceive(Context ctx, Intent intent) { + intents.add(intent); + } + } } diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/keyagreement/KeyAgreementConstants.java b/bramble-api/src/main/java/org/briarproject/bramble/api/keyagreement/KeyAgreementConstants.java index d329bff3f..078b824a8 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/keyagreement/KeyAgreementConstants.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/keyagreement/KeyAgreementConstants.java @@ -21,7 +21,7 @@ public interface KeyAgreementConstants { /** * The connection timeout in milliseconds. */ - long CONNECTION_TIMEOUT = 20 * 1000; + long CONNECTION_TIMEOUT = 40 * 1000; /** * The transport identifier for Bluetooth. diff --git a/bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothPlugin.java b/bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothPlugin.java index 243548c0a..9610a905c 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothPlugin.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothPlugin.java @@ -96,6 +96,9 @@ abstract class BluetoothPlugin implements DuplexPlugin, EventListener { abstract DuplexTransportConnection connectTo(String address, String uuid) throws IOException; + @Nullable + abstract DuplexTransportConnection discoverAndConnect(String uuid); + BluetoothPlugin(BluetoothConnectionLimiter connectionLimiter, Executor ioExecutor, SecureRandom secureRandom, Backoff backoff, DuplexPluginCallback callback, int maxLatency) { @@ -326,9 +329,6 @@ abstract class BluetoothPlugin implements DuplexPlugin, EventListener { @Override public KeyAgreementListener createKeyAgreementListener(byte[] commitment) { if (!isRunning()) return null; - // There's no point listening if we can't discover our own address - String address = getBluetoothAddress(); - if (address == null) return null; // No truncation necessary because COMMIT_LENGTH = 16 String uuid = UUID.nameUUIDFromBytes(commitment).toString(); if (LOG.isLoggable(INFO)) LOG.info("Key agreement UUID " + uuid); @@ -346,7 +346,8 @@ abstract class BluetoothPlugin implements DuplexPlugin, EventListener { } BdfList descriptor = new BdfList(); descriptor.add(TRANSPORT_ID_BLUETOOTH); - descriptor.add(StringUtils.macToBytes(address)); + String address = getBluetoothAddress(); + if (address != null) descriptor.add(StringUtils.macToBytes(address)); return new BluetoothKeyAgreementListener(descriptor, ss); } @@ -354,18 +355,25 @@ abstract class BluetoothPlugin implements DuplexPlugin, EventListener { public DuplexTransportConnection createKeyAgreementConnection( byte[] commitment, BdfList descriptor) { if (!isRunning()) return null; - String address; - try { - address = parseAddress(descriptor); - } catch (FormatException e) { - LOG.info("Invalid address in key agreement descriptor"); - return null; - } // No truncation necessary because COMMIT_LENGTH = 16 String uuid = UUID.nameUUIDFromBytes(commitment).toString(); - if (LOG.isLoggable(INFO)) - LOG.info("Connecting to key agreement UUID " + uuid); - DuplexTransportConnection conn = connect(address, uuid); + DuplexTransportConnection conn; + if (descriptor.size() == 1) { + if (LOG.isLoggable(INFO)) + LOG.info("Discovering address for key agreement UUID " + uuid); + conn = discoverAndConnect(uuid); + } else { + String address; + try { + address = parseAddress(descriptor); + } catch (FormatException e) { + LOG.info("Invalid address in key agreement descriptor"); + return null; + } + if (LOG.isLoggable(INFO)) + LOG.info("Connecting to key agreement UUID " + uuid); + conn = connect(address, uuid); + } if (conn != null) connectionLimiter.keyAgreementConnectionOpened(conn); return conn; } diff --git a/bramble-java/src/main/java/org/briarproject/bramble/plugin/bluetooth/JavaBluetoothPlugin.java b/bramble-java/src/main/java/org/briarproject/bramble/plugin/bluetooth/JavaBluetoothPlugin.java index c3652bfb9..c3fe9b028 100644 --- a/bramble-java/src/main/java/org/briarproject/bramble/plugin/bluetooth/JavaBluetoothPlugin.java +++ b/bramble-java/src/main/java/org/briarproject/bramble/plugin/bluetooth/JavaBluetoothPlugin.java @@ -108,6 +108,12 @@ class JavaBluetoothPlugin extends BluetoothPlugin { return wrapSocket((StreamConnection) Connector.open(url)); } + @Override + @Nullable + DuplexTransportConnection discoverAndConnect(String uuid) { + return null; // TODO + } + private String makeUrl(String address, String uuid) { return "btspp://" + address + ":" + uuid + ";name=RFCOMM"; } From efe15df940db6ecf28dbe1336f6813ebe85d1158 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Thu, 11 Oct 2018 17:19:29 +0100 Subject: [PATCH 5/9] Remove static import of R's fields. --- .../keyagreement/KeyAgreementActivity.java | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/KeyAgreementActivity.java b/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/KeyAgreementActivity.java index 6af2e5f28..043a08bfc 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/KeyAgreementActivity.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/KeyAgreementActivity.java @@ -21,8 +21,6 @@ import org.briarproject.bramble.api.nullsafety.MethodsNotNullByDefault; import org.briarproject.bramble.api.nullsafety.ParametersNotNullByDefault; import org.briarproject.bramble.api.plugin.event.BluetoothEnabledEvent; import org.briarproject.briar.R; -import org.briarproject.briar.R.string; -import org.briarproject.briar.R.style; import org.briarproject.briar.android.activity.ActivityComponent; import org.briarproject.briar.android.activity.BriarActivity; import org.briarproject.briar.android.fragment.BaseFragment; @@ -217,14 +215,14 @@ public abstract class KeyAgreementActivity extends BriarActivity implements boolean cameraRationale = shouldShowRationale(CAMERA); boolean locationRationale = shouldShowRationale(ACCESS_COARSE_LOCATION); if (cameraRationale && locationRationale) { - showRationale(string.permission_camera_location_title, - string.permission_camera_location_request_body); + showRationale(R.string.permission_camera_location_title, + R.string.permission_camera_location_request_body); } else if (cameraRationale) { - showRationale(string.permission_camera_title, - string.permission_camera_request_body); + showRationale(R.string.permission_camera_title, + R.string.permission_camera_request_body); } else if (locationRationale) { - showRationale(string.permission_location_title, - string.permission_location_request_body); + showRationale(R.string.permission_location_title, + R.string.permission_location_request_body); } else if (gotCameraPermission) { // Location permission has been permanently denied but we can // continue without it @@ -246,10 +244,10 @@ public abstract class KeyAgreementActivity extends BriarActivity implements } private void showRationale(@StringRes int title, @StringRes int body) { - Builder builder = new Builder(this, style.BriarDialogTheme); + Builder builder = new Builder(this, R.style.BriarDialogTheme); builder.setTitle(title); builder.setMessage(body); - builder.setNeutralButton(string.continue_button, + builder.setNeutralButton(R.string.continue_button, (dialog, which) -> requestPermissions()); builder.show(); } @@ -274,17 +272,19 @@ public abstract class KeyAgreementActivity extends BriarActivity implements showNextScreen(); } else { if (shouldShowRationale(CAMERA)) { - Toast.makeText(this, string.permission_camera_denied_toast, + Toast.makeText(this, + R.string.permission_camera_denied_toast, LENGTH_LONG).show(); supportFinishAfterTransition(); } else { // The user has permanently denied the request - Builder builder = new Builder(this, style.BriarDialogTheme); - builder.setTitle(string.permission_camera_title); - builder.setMessage(string.permission_camera_denied_body); - builder.setPositiveButton(string.ok, + Builder builder = + new Builder(this, R.style.BriarDialogTheme); + builder.setTitle(R.string.permission_camera_title); + builder.setMessage(R.string.permission_camera_denied_body); + builder.setPositiveButton(R.string.ok, UiUtils.getGoToSettingsListener(this)); - builder.setNegativeButton(string.cancel, + builder.setNegativeButton(R.string.cancel, (dialog, which) -> supportFinishAfterTransition()); builder.show(); } From 9515e938578567ee0c275f1a0f8d215aa085964a Mon Sep 17 00:00:00 2001 From: akwizgran Date: Thu, 11 Oct 2018 18:18:05 +0100 Subject: [PATCH 6/9] Cancel discovery after 10 seconds and try to connect. --- .../bluetooth/AndroidBluetoothPlugin.java | 25 +++++++++++++++---- .../AndroidBluetoothPluginFactory.java | 7 ++++-- .../keyagreement/KeyAgreementConstants.java | 2 +- .../briarproject/briar/android/AppModule.java | 2 +- 4 files changed, 27 insertions(+), 9 deletions(-) diff --git a/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothPlugin.java b/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothPlugin.java index 5e3973045..65e0211fa 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothPlugin.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothPlugin.java @@ -16,6 +16,7 @@ import org.briarproject.bramble.api.plugin.PluginException; import org.briarproject.bramble.api.plugin.duplex.DuplexPluginCallback; import org.briarproject.bramble.api.plugin.duplex.DuplexTransportConnection; import org.briarproject.bramble.api.system.AndroidExecutor; +import org.briarproject.bramble.api.system.Clock; import org.briarproject.bramble.util.AndroidUtils; import java.io.Closeable; @@ -23,6 +24,8 @@ import java.io.IOException; import java.security.SecureRandom; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; +import java.util.List; import java.util.UUID; import java.util.concurrent.BlockingQueue; import java.util.concurrent.ExecutionException; @@ -45,6 +48,7 @@ import static android.bluetooth.BluetoothAdapter.STATE_OFF; import static android.bluetooth.BluetoothAdapter.STATE_ON; import static android.bluetooth.BluetoothDevice.ACTION_FOUND; import static android.bluetooth.BluetoothDevice.EXTRA_DEVICE; +import static java.util.concurrent.TimeUnit.MILLISECONDS; import static java.util.logging.Level.INFO; import static java.util.logging.Level.WARNING; import static org.briarproject.bramble.util.LogUtils.logException; @@ -57,8 +61,11 @@ class AndroidBluetoothPlugin extends BluetoothPlugin { private static final Logger LOG = Logger.getLogger(AndroidBluetoothPlugin.class.getName()); + private static final int MAX_DISCOVERY_MS = 10_000; + private final AndroidExecutor androidExecutor; private final Context appContext; + private final Clock clock; private volatile boolean wasEnabledByUs = false; private volatile BluetoothStateReceiver receiver = null; @@ -68,12 +75,13 @@ class AndroidBluetoothPlugin extends BluetoothPlugin { AndroidBluetoothPlugin(BluetoothConnectionLimiter connectionLimiter, Executor ioExecutor, AndroidExecutor androidExecutor, - Context appContext, SecureRandom secureRandom, Backoff backoff, - DuplexPluginCallback callback, int maxLatency) { + Context appContext, SecureRandom secureRandom, Clock clock, + Backoff backoff, DuplexPluginCallback callback, int maxLatency) { super(connectionLimiter, ioExecutor, secureRandom, backoff, callback, maxLatency); this.androidExecutor = androidExecutor; this.appContext = appContext; + this.clock = clock; } @Override @@ -213,7 +221,7 @@ class AndroidBluetoothPlugin extends BluetoothPlugin { } private Collection discoverDevices() { - Collection addresses = new ArrayList<>(); + List addresses = new ArrayList<>(); BlockingQueue intents = new LinkedBlockingQueue<>(); DiscoveryReceiver receiver = new DiscoveryReceiver(intents); IntentFilter filter = new IntentFilter(); @@ -223,8 +231,11 @@ class AndroidBluetoothPlugin extends BluetoothPlugin { appContext.registerReceiver(receiver, filter); try { if (adapter.startDiscovery()) { - while (true) { - Intent i = intents.take(); + long now = clock.currentTimeMillis(); + long end = now + MAX_DISCOVERY_MS; + while (now < end) { + Intent i = intents.poll(end - now, MILLISECONDS); + if (i == null) break; String action = i.getAction(); if (ACTION_DISCOVERY_STARTED.equals(action)) { LOG.info("Discovery started"); @@ -239,6 +250,7 @@ class AndroidBluetoothPlugin extends BluetoothPlugin { if (!addresses.contains(address)) addresses.add(address); } + now = clock.currentTimeMillis(); } } else { LOG.info("Could not start discovery"); @@ -247,9 +259,12 @@ class AndroidBluetoothPlugin extends BluetoothPlugin { LOG.info("Interrupted while discovering devices"); Thread.currentThread().interrupt(); } finally { + LOG.info("Cancelling discovery"); adapter.cancelDiscovery(); appContext.unregisterReceiver(receiver); } + // Shuffle the addresses so we don't always try the same one first + Collections.shuffle(addresses); return addresses; } diff --git a/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothPluginFactory.java b/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothPluginFactory.java index c4b546739..53a127caa 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothPluginFactory.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothPluginFactory.java @@ -11,6 +11,7 @@ import org.briarproject.bramble.api.plugin.duplex.DuplexPlugin; import org.briarproject.bramble.api.plugin.duplex.DuplexPluginCallback; import org.briarproject.bramble.api.plugin.duplex.DuplexPluginFactory; import org.briarproject.bramble.api.system.AndroidExecutor; +import org.briarproject.bramble.api.system.Clock; import java.security.SecureRandom; import java.util.concurrent.Executor; @@ -33,17 +34,19 @@ public class AndroidBluetoothPluginFactory implements DuplexPluginFactory { private final Context appContext; private final SecureRandom secureRandom; private final EventBus eventBus; + private final Clock clock; private final BackoffFactory backoffFactory; public AndroidBluetoothPluginFactory(Executor ioExecutor, AndroidExecutor androidExecutor, Context appContext, - SecureRandom secureRandom, EventBus eventBus, + SecureRandom secureRandom, EventBus eventBus, Clock clock, BackoffFactory backoffFactory) { this.ioExecutor = ioExecutor; this.androidExecutor = androidExecutor; this.appContext = appContext; this.secureRandom = secureRandom; this.eventBus = eventBus; + this.clock = clock; this.backoffFactory = backoffFactory; } @@ -65,7 +68,7 @@ public class AndroidBluetoothPluginFactory implements DuplexPluginFactory { MAX_POLLING_INTERVAL, BACKOFF_BASE); AndroidBluetoothPlugin plugin = new AndroidBluetoothPlugin( connectionLimiter, ioExecutor, androidExecutor, appContext, - secureRandom, backoff, callback, MAX_LATENCY); + secureRandom, clock, backoff, callback, MAX_LATENCY); eventBus.addListener(plugin); return plugin; } diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/keyagreement/KeyAgreementConstants.java b/bramble-api/src/main/java/org/briarproject/bramble/api/keyagreement/KeyAgreementConstants.java index 078b824a8..4041d0ce4 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/keyagreement/KeyAgreementConstants.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/keyagreement/KeyAgreementConstants.java @@ -21,7 +21,7 @@ public interface KeyAgreementConstants { /** * The connection timeout in milliseconds. */ - long CONNECTION_TIMEOUT = 40 * 1000; + long CONNECTION_TIMEOUT = 60_000; /** * The transport identifier for Bluetooth. diff --git a/briar-android/src/main/java/org/briarproject/briar/android/AppModule.java b/briar-android/src/main/java/org/briarproject/briar/android/AppModule.java index 606647958..bbfb919ce 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/AppModule.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/AppModule.java @@ -107,7 +107,7 @@ public class AppModule { Context appContext = app.getApplicationContext(); DuplexPluginFactory bluetooth = new AndroidBluetoothPluginFactory(ioExecutor, androidExecutor, - appContext, random, eventBus, backoffFactory); + appContext, random, eventBus, clock, backoffFactory); DuplexPluginFactory tor = new AndroidTorPluginFactory(ioExecutor, scheduler, appContext, networkManager, locationUtils, eventBus, torSocketFactory, backoffFactory, resourceProvider, From 4b7a81177c9a17c0195dba6af5c9bdcc366c1fdb Mon Sep 17 00:00:00 2001 From: akwizgran Date: Mon, 15 Oct 2018 14:46:40 +0100 Subject: [PATCH 7/9] Static imports. --- .../plugin/bluetooth/BluetoothPlugin.java | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothPlugin.java b/bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothPlugin.java index 9610a905c..0fdc03cee 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothPlugin.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothPlugin.java @@ -23,7 +23,6 @@ import org.briarproject.bramble.api.plugin.event.EnableBluetoothEvent; import org.briarproject.bramble.api.properties.TransportProperties; import org.briarproject.bramble.api.settings.Settings; import org.briarproject.bramble.api.settings.event.SettingsUpdatedEvent; -import org.briarproject.bramble.util.StringUtils; import java.io.IOException; import java.security.SecureRandom; @@ -46,6 +45,9 @@ import static org.briarproject.bramble.api.plugin.BluetoothConstants.PROP_UUID; import static org.briarproject.bramble.api.plugin.BluetoothConstants.UUID_BYTES; import static org.briarproject.bramble.util.LogUtils.logException; import static org.briarproject.bramble.util.PrivacyUtils.scrubMacAddress; +import static org.briarproject.bramble.util.StringUtils.isNullOrEmpty; +import static org.briarproject.bramble.util.StringUtils.macToBytes; +import static org.briarproject.bramble.util.StringUtils.macToString; @MethodsNotNullByDefault @ParametersNotNullByDefault @@ -196,7 +198,7 @@ abstract class BluetoothPlugin implements DuplexPlugin, EventListener { address = getBluetoothAddress(); if (LOG.isLoggable(INFO)) LOG.info("Local address " + scrubMacAddress(address)); - if (!StringUtils.isNullOrEmpty(address)) { + if (!isNullOrEmpty(address)) { p.put(PROP_ADDRESS, address); changed = true; } @@ -259,9 +261,9 @@ abstract class BluetoothPlugin implements DuplexPlugin, EventListener { // Try to connect to known devices in parallel for (Entry e : contacts.entrySet()) { String address = e.getValue().get(PROP_ADDRESS); - if (StringUtils.isNullOrEmpty(address)) continue; + if (isNullOrEmpty(address)) continue; String uuid = e.getValue().get(PROP_UUID); - if (StringUtils.isNullOrEmpty(uuid)) continue; + if (isNullOrEmpty(uuid)) continue; ContactId c = e.getKey(); ioExecutor.execute(() -> { if (!isRunning() || !shouldAllowContactConnections()) return; @@ -312,9 +314,9 @@ abstract class BluetoothPlugin implements DuplexPlugin, EventListener { if (!isRunning() || !shouldAllowContactConnections()) return null; if (!connectionLimiter.canOpenContactConnection()) return null; String address = p.get(PROP_ADDRESS); - if (StringUtils.isNullOrEmpty(address)) return null; + if (isNullOrEmpty(address)) return null; String uuid = p.get(PROP_UUID); - if (StringUtils.isNullOrEmpty(uuid)) return null; + if (isNullOrEmpty(uuid)) return null; DuplexTransportConnection conn = connect(address, uuid); if (conn == null) return null; // TODO: Why don't we reset the backoff here? @@ -347,7 +349,7 @@ abstract class BluetoothPlugin implements DuplexPlugin, EventListener { BdfList descriptor = new BdfList(); descriptor.add(TRANSPORT_ID_BLUETOOTH); String address = getBluetoothAddress(); - if (address != null) descriptor.add(StringUtils.macToBytes(address)); + if (address != null) descriptor.add(macToBytes(address)); return new BluetoothKeyAgreementListener(descriptor, ss); } @@ -381,7 +383,7 @@ abstract class BluetoothPlugin implements DuplexPlugin, EventListener { private String parseAddress(BdfList descriptor) throws FormatException { byte[] mac = descriptor.getRaw(1); if (mac.length != 6) throw new FormatException(); - return StringUtils.macToString(mac); + return macToString(mac); } @Override From 3d6a336f6d26d2eb5aef9c0388498f7e33bd2744 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Thu, 18 Oct 2018 10:26:59 +0100 Subject: [PATCH 8/9] Refactor permissions code, add comments, fix corner cases. --- .../keyagreement/KeyAgreementActivity.java | 190 +++++++++++------- 1 file changed, 114 insertions(+), 76 deletions(-) diff --git a/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/KeyAgreementActivity.java b/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/KeyAgreementActivity.java index 043a08bfc..4298e89c6 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/KeyAgreementActivity.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/KeyAgreementActivity.java @@ -10,11 +10,9 @@ import android.support.annotation.StringRes; import android.support.annotation.UiThread; import android.support.v4.app.ActivityCompat; import android.support.v4.app.FragmentManager; -import android.support.v4.content.ContextCompat; import android.support.v7.app.AlertDialog.Builder; import android.support.v7.widget.Toolbar; import android.view.MenuItem; -import android.widget.Toast; import org.briarproject.bramble.api.event.EventBus; import org.briarproject.bramble.api.nullsafety.MethodsNotNullByDefault; @@ -45,8 +43,6 @@ import static android.bluetooth.BluetoothAdapter.SCAN_MODE_CONNECTABLE; import static android.bluetooth.BluetoothAdapter.SCAN_MODE_CONNECTABLE_DISCOVERABLE; import static android.bluetooth.BluetoothAdapter.STATE_ON; import static android.content.pm.PackageManager.PERMISSION_GRANTED; -import static android.os.Build.VERSION.SDK_INT; -import static android.widget.Toast.LENGTH_LONG; import static org.briarproject.briar.android.activity.RequestCodes.REQUEST_BLUETOOTH_DISCOVERABLE; import static org.briarproject.briar.android.activity.RequestCodes.REQUEST_PERMISSION_CAMERA_LOCATION; @@ -60,14 +56,37 @@ public abstract class KeyAgreementActivity extends BriarActivity implements UNKNOWN, NO_ADAPTER, WAITING, REFUSED, ENABLED, DISCOVERABLE } + private enum Permission { + UNKNOWN, GRANTED, SHOW_RATIONALE, PERMANENTLY_DENIED + } + private static final Logger LOG = Logger.getLogger(KeyAgreementActivity.class.getName()); @Inject EventBus eventBus; - private boolean isResumed = false, wasAdapterEnabled = false; - private boolean continueClicked, gotCameraPermission, gotLocationPermission; + /** + * Set to true in onPostResume() and false in onPause(). This prevents the + * QR code fragment from being shown if onRequestPermissionsResult() is + * called while the activity is paused, which could cause a crash due to + * https://issuetracker.google.com/issues/37067655. + */ + private boolean isResumed = false; + /** + * Set to true when the continue button is clicked, and false when the QR + * code fragment is shown. This prevents the QR code fragment from being + * shown automatically before the continue button has been clicked. + */ + private boolean continueClicked = false; + /** + * Records whether the Bluetooth adapter was already enabled before we + * asked for Bluetooth discoverability, so we know whether to broadcast a + * {@link BluetoothEnabledEvent}. + */ + private boolean wasAdapterEnabled = false; + private Permission cameraPermission = Permission.UNKNOWN; + private Permission locationPermission = Permission.UNKNOWN; private BluetoothState bluetoothState = BluetoothState.UNKNOWN; private BroadcastReceiver bluetoothReceiver = null; @@ -111,20 +130,40 @@ public abstract class KeyAgreementActivity extends BriarActivity implements } } + @Override + public void onStart() { + super.onStart(); + // Permissions may have been granted manually while we were stopped + cameraPermission = Permission.UNKNOWN; + locationPermission = Permission.UNKNOWN; + } + @Override protected void onPostResume() { super.onPostResume(); isResumed = true; // Workaround for // https://code.google.com/p/android/issues/detail?id=190966 - if (canShowQrCodeFragment()) showQrCodeFragment(); + showQrCodeFragmentIfAllowed(); } - private boolean canShowQrCodeFragment() { - return isResumed && continueClicked - && (SDK_INT < 23 || gotCameraPermission) - && bluetoothState != BluetoothState.UNKNOWN - && bluetoothState != BluetoothState.WAITING; + private void showQrCodeFragmentIfAllowed() { + if (isResumed && continueClicked && areEssentialPermissionsGranted()) { + if (bluetoothState == BluetoothState.UNKNOWN || + bluetoothState == BluetoothState.ENABLED) { + requestBluetoothDiscoverable(); + } else if (bluetoothState != BluetoothState.WAITING) { + showQrCodeFragment(); + } + } + } + + private boolean areEssentialPermissionsGranted() { + // If the camera permission has been granted, and the location + // permission has been granted or permanently denied, we can continue + return cameraPermission == Permission.GRANTED && + (locationPermission == Permission.GRANTED || + locationPermission == Permission.PERMANENTLY_DENIED); } @Override @@ -136,18 +175,7 @@ public abstract class KeyAgreementActivity extends BriarActivity implements @Override public void showNextScreen() { continueClicked = true; - if (checkPermissions()) { - if (shouldRequestBluetoothDiscoverable()) { - requestBluetoothDiscoverable(); - } else if (canShowQrCodeFragment()) { - showQrCodeFragment(); - } - } - } - - private boolean shouldRequestBluetoothDiscoverable() { - return bluetoothState == BluetoothState.UNKNOWN - || bluetoothState == BluetoothState.REFUSED; + if (checkPermissions()) showQrCodeFragmentIfAllowed(); } private void requestBluetoothDiscoverable() { @@ -169,7 +197,7 @@ public abstract class KeyAgreementActivity extends BriarActivity implements eventBus.broadcast(new BluetoothEnabledEvent()); wasAdapterEnabled = true; } - if (canShowQrCodeFragment()) showQrCodeFragment(); + showQrCodeFragmentIfAllowed(); } @Override @@ -189,7 +217,12 @@ public abstract class KeyAgreementActivity extends BriarActivity implements } private void showQrCodeFragment() { + // If we return to the intro fragment, the continue button needs to be + // clicked again before showing the QR code fragment continueClicked = false; + // If we return to the intro fragment, ask for Bluetooth + // discoverability again before showing the QR code fragment + bluetoothState = BluetoothState.UNKNOWN; // FIXME #824 FragmentManager fm = getSupportFragmentManager(); if (fm.findFragmentByTag(KeyAgreementFragment.TAG) == null) { @@ -208,41 +241,37 @@ public abstract class KeyAgreementActivity extends BriarActivity implements } private boolean checkPermissions() { - gotCameraPermission = checkPermission(CAMERA); - gotLocationPermission = checkPermission(ACCESS_COARSE_LOCATION); - if (gotCameraPermission && gotLocationPermission) return true; - // Should we show an explanation for one or both permissions? - boolean cameraRationale = shouldShowRationale(CAMERA); - boolean locationRationale = shouldShowRationale(ACCESS_COARSE_LOCATION); - if (cameraRationale && locationRationale) { + if (areEssentialPermissionsGranted()) return true; + // If the camera permission has been permanently denied, ask the + // user to change the setting + if (cameraPermission == Permission.PERMANENTLY_DENIED) { + Builder builder = new Builder(this, R.style.BriarDialogTheme); + builder.setTitle(R.string.permission_camera_title); + builder.setMessage(R.string.permission_camera_denied_body); + builder.setPositiveButton(R.string.ok, + UiUtils.getGoToSettingsListener(this)); + builder.setNegativeButton(R.string.cancel, + (dialog, which) -> supportFinishAfterTransition()); + builder.show(); + return false; + } + // Should we show the rationale for one or both permissions? + if (cameraPermission == Permission.SHOW_RATIONALE && + locationPermission == Permission.SHOW_RATIONALE) { showRationale(R.string.permission_camera_location_title, R.string.permission_camera_location_request_body); - } else if (cameraRationale) { + } else if (cameraPermission == Permission.SHOW_RATIONALE) { showRationale(R.string.permission_camera_title, R.string.permission_camera_request_body); - } else if (locationRationale) { + } else if (locationPermission == Permission.SHOW_RATIONALE) { showRationale(R.string.permission_location_title, R.string.permission_location_request_body); - } else if (gotCameraPermission) { - // Location permission has been permanently denied but we can - // continue without it - return true; } else { requestPermissions(); } return false; } - private boolean checkPermission(String permission) { - return ContextCompat.checkSelfPermission(this, permission) - == PERMISSION_GRANTED; - } - - private boolean shouldShowRationale(String permission) { - return ActivityCompat.shouldShowRequestPermissionRationale(this, - permission); - } - private void showRationale(@StringRes int title, @StringRes int body) { Builder builder = new Builder(this, R.style.BriarDialogTheme); builder.setTitle(title); @@ -261,35 +290,44 @@ public abstract class KeyAgreementActivity extends BriarActivity implements @Override @UiThread public void onRequestPermissionsResult(int requestCode, - String permissions[], int[] grantResults) { - if (requestCode == REQUEST_PERMISSION_CAMERA_LOCATION) { - // If request is cancelled, the result arrays are empty - gotCameraPermission = grantResults.length > 0 - && grantResults[0] == PERMISSION_GRANTED; - gotLocationPermission = grantResults.length > 1 - && grantResults[1] == PERMISSION_GRANTED; - if (gotCameraPermission) { - showNextScreen(); - } else { - if (shouldShowRationale(CAMERA)) { - Toast.makeText(this, - R.string.permission_camera_denied_toast, - LENGTH_LONG).show(); - supportFinishAfterTransition(); - } else { - // The user has permanently denied the request - Builder builder = - new Builder(this, R.style.BriarDialogTheme); - builder.setTitle(R.string.permission_camera_title); - builder.setMessage(R.string.permission_camera_denied_body); - builder.setPositiveButton(R.string.ok, - UiUtils.getGoToSettingsListener(this)); - builder.setNegativeButton(R.string.cancel, - (dialog, which) -> supportFinishAfterTransition()); - builder.show(); - } - } + String[] permissions, int[] grantResults) { + if (requestCode != REQUEST_PERMISSION_CAMERA_LOCATION) + throw new AssertionError(); + if (gotPermission(CAMERA, permissions, grantResults)) { + cameraPermission = Permission.GRANTED; + } else if (shouldShowRationale(CAMERA)) { + cameraPermission = Permission.SHOW_RATIONALE; + } else { + cameraPermission = Permission.PERMANENTLY_DENIED; } + if (gotPermission(ACCESS_COARSE_LOCATION, permissions, grantResults)) { + locationPermission = Permission.GRANTED; + } else if (shouldShowRationale(ACCESS_COARSE_LOCATION)) { + locationPermission = Permission.SHOW_RATIONALE; + } else { + locationPermission = Permission.PERMANENTLY_DENIED; + } + // If a permission dialog has been shown, showing the QR code fragment + // on this call path would cause a crash due to + // https://code.google.com/p/android/issues/detail?id=190966. + // In that case the isResumed flag prevents the fragment from being + // shown here, and showQrCodeFragmentIfAllowed() will be called again + // from onPostResume(). + if (checkPermissions()) showQrCodeFragmentIfAllowed(); + } + + private boolean gotPermission(String permission, String[] permissions, + int[] grantResults) { + for (int i = 0; i < permissions.length; i++) { + if (permission.equals(permissions[i])) + return grantResults[i] == PERMISSION_GRANTED; + } + return false; + } + + private boolean shouldShowRationale(String permission) { + return ActivityCompat.shouldShowRequestPermissionRationale(this, + permission); } private class BluetoothStateReceiver extends BroadcastReceiver { From 9c4fb4fd3407e3acd44dfa3fe503477c6debcc0b Mon Sep 17 00:00:00 2001 From: akwizgran Date: Thu, 18 Oct 2018 17:22:54 +0100 Subject: [PATCH 9/9] Remove unused string. --- briar-android/src/main/res/values/strings.xml | 1 - 1 file changed, 1 deletion(-) diff --git a/briar-android/src/main/res/values/strings.xml b/briar-android/src/main/res/values/strings.xml index 92f05edcc..897a77a7a 100644 --- a/briar-android/src/main/res/values/strings.xml +++ b/briar-android/src/main/res/values/strings.xml @@ -472,7 +472,6 @@ Camera and location To scan the QR code, Briar needs access to the camera.\n\nTo discover Bluetooth devices, Briar needs permission to access your location.\n\nBriar does not store your location or share it with anyone. You have denied access to the camera, but adding contacts requires using the camera.\n\nPlease consider granting access. - Camera permission was not granted QR code Show QR code fullscreen