Skip to content

Commit

Permalink
android: handle device disconnect while the app is in the background
Browse files Browse the repository at this point in the history
The ACTION_USB_DEVICE_ATTACHED intent in MainActivity was not handled by the app while
it was in the background.

Resulting bug:
- when you disconnect an unlocked device while the app is in the
  background, and bring the app to the foreground again, the previous
  unlocked keystore is visible, as the backend didn't register the
  disconnet.
- same when the user disconnects the device while the app is in the
  background, and reconnects it before bringing it to the
  foreground. Ths is a special case, simply calling `updateDevice()`
  in `onResume()` would not handle this correctly, as a device would
  be connected, and an intermediate disconnect could not be detected.

To fix this, we detect the DETACH intent in the service, and trigger
the backend to handle it.
  • Loading branch information
benma committed Nov 18, 2024
1 parent f200fd0 commit ba5aea7
Show file tree
Hide file tree
Showing 6 changed files with 81 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Changelog

## Unreleased
- Android: handle device disconnect while the app is in the background

# 4.46.0
- Android: enable export logs feature
Expand Down
14 changes: 12 additions & 2 deletions backend/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,8 @@ type Backend struct {

devices map[string]device.Interface

usbManager *usb.Manager

accountsAndKeystoreLock locker.Locker
accounts AccountsList
// keystore is nil if no keystore is connected.
Expand Down Expand Up @@ -612,13 +614,14 @@ func (backend *Backend) OnDeviceUninit(f func(string)) {
// Start starts the background services. It returns a channel of events to handle by the library
// client.
func (backend *Backend) Start() <-chan interface{} {
usb.NewManager(
backend.usbManager = usb.NewManager(
backend.arguments.MainDirectoryPath(),
backend.arguments.BitBox02DirectoryPath(),
backend.socksProxy,
backend.environment.DeviceInfos,
backend.Register,
backend.Deregister).Start()
backend.Deregister)
backend.usbManager.Start()

httpClient, err := backend.socksProxy.GetHTTPClient()
if err != nil {
Expand All @@ -638,6 +641,13 @@ func (backend *Backend) Start() <-chan interface{} {
return backend.events
}

// UsbUpdate triggers a scan of the USB devices to detect connects/disconnects.
func (backend *Backend) UsbUpdate() {
if backend.usbManager != nil {
backend.usbManager.Update()
}
}

// DevicesRegistered returns a map of device IDs to device of registered devices.
func (backend *Backend) DevicesRegistered() map[string]device.Interface {
return backend.devices
Expand Down
10 changes: 10 additions & 0 deletions backend/bridgecommon/bridgecommon.go
Original file line number Diff line number Diff line change
Expand Up @@ -359,3 +359,13 @@ func Shutdown() {
log.Info("Shutdown called, but backend not running")
}
}

// UsbUpdate wraps backend.UsbUpdate.
func UsbUpdate() {
mu.RLock()
defer mu.RUnlock()
if globalBackend == nil {
return
}
globalBackend.UsbUpdate()
}
16 changes: 15 additions & 1 deletion backend/devices/usb/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ type Manager struct {
onRegister func(device.Interface) error
onUnregister func(string)

updateCh chan struct{}

socksProxy socksproxy.SocksProxy

log *logrus.Entry
Expand All @@ -111,6 +113,7 @@ func NewManager(
deviceInfos: deviceInfos,
onRegister: onRegister,
onUnregister: onUnregister,
updateCh: make(chan struct{}),
socksProxy: socksProxy,

log: logging.Get().WithGroup("manager"),
Expand Down Expand Up @@ -255,8 +258,19 @@ func (manager *Manager) checkIfRemoved(deviceID string) bool {
return true
}

// Update triggers a scan of the USB devices to detect connects/disconnects.
func (manager *Manager) Update() {
go func() {
manager.updateCh <- struct{}{}
}()
}

func (manager *Manager) listen() {
for {
select {
case <-manager.updateCh:
case <-time.After(time.Second):
}
for deviceID, device := range manager.devices {
// Check if device was removed.
if manager.checkIfRemoved(deviceID) {
Expand Down Expand Up @@ -310,11 +324,11 @@ func (manager *Manager) listen() {
manager.log.WithError(err).Error("Failed to execute on-register")
}
}
time.Sleep(time.Second)
}
}

// Start listens for inserted/removed devices forever.
func (manager *Manager) Start() {
go manager.listen()
manager.Update()
}
5 changes: 5 additions & 0 deletions backend/mobileserver/mobileserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,11 @@ func UsingMobileDataChanged() {
bridgecommon.UsingMobileDataChanged()
}

// UsbUpdate exposes `bridgecommon.UsbUpdate` to Java/Kotlin.
func UsbUpdate() {
bridgecommon.UsbUpdate()
}

type goLogHook struct {
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,18 @@
import android.app.NotificationManager;
import android.app.PendingIntent;
import android.app.Service;
import android.content.BroadcastReceiver;
import android.content.Context;
import android.content.Intent;
import android.content.IntentFilter;
import android.os.Binder;
import android.os.Build;
import android.os.IBinder;
import android.hardware.usb.UsbManager;

import androidx.core.app.NotificationCompat;
import androidx.lifecycle.ViewModelProvider;
import androidx.lifecycle.ViewModelStoreOwner;

import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;
Expand All @@ -31,6 +37,20 @@ public class GoService extends Service {

private final int notificationId = 8;

private ViewModelStoreOwner viewModelStoreOwner;

private final BroadcastReceiver usbStateReceiver = new BroadcastReceiver() {
@Override
public void onReceive(Context context, Intent intent) {
String action = intent.getAction();
if (viewModelStoreOwner != null && UsbManager.ACTION_USB_DEVICE_DETACHED.equals(action)) {
GoViewModel viewModel = new ViewModelProvider(viewModelStoreOwner).get(GoViewModel.class);
viewModel.setDevice(null);
Mobileserver.usbUpdate();
}
}
};

@Override
public void onCreate() {
Util.log("GoService onCreate()");
Expand Down Expand Up @@ -68,12 +88,26 @@ public void onCreate() {
// focus. This is needed to avoid timeouts when the backend is polling the BitBox for e.g.
// an address verification.
startForeground(notificationId, notification);

// Register USB broadcast receiver to detect USB disconnects, even while the app is in the
// background.
IntentFilter filter = new IntentFilter();
filter.addAction(UsbManager.ACTION_USB_DEVICE_DETACHED);

if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
registerReceiver(usbStateReceiver, filter, Context.RECEIVER_EXPORTED);
} else {
registerReceiver(usbStateReceiver, filter);
}

Util.log("GoService onCreate completed");
}

@Override
public void onDestroy() {
Util.log("GoService onDestroy()");
super.onDestroy();
unregisterReceiver(usbStateReceiver);
// It would be nice to call MobileServer.shutdown() here, but that function
// is currently incomplete and can lead to unpredictable results.
}
Expand All @@ -98,6 +132,10 @@ GoService getService() {
}
}

public void setViewModelStoreOwner(ViewModelStoreOwner owner) {
this.viewModelStoreOwner = owner;
}

@Override
public IBinder onBind(Intent intent) {
return binder;
Expand Down

0 comments on commit ba5aea7

Please sign in to comment.