AltBeacon / android-beacon-library

Allows Android apps to interact with BLE beacons
Apache License 2.0
2.83k stars 834 forks source link

Beacon scanning not working with Android 12 Bluetooth permissions #1063

Closed mannodermaus closed 2 years ago

mannodermaus commented 2 years ago

Hey David! Long time no see, I hope you're doing well. 🙏

In the process of upgrading my application to Android 12, I was updating the permission declarations to the new Bluetooth permissions introduced in that OS version. Unfortunately, I'm unable to find my nearby beacon when using AltBeacon to range for it. Replacing the AltBeacon usage with some vanilla Android code (using BluetoothLeScannerdirectly), I did find it, however.

In the PR for Android 12, it was noted that scanning didn't work unless ACCESS_FINE_LOCATION was also granted at runtime. Being a replacement for this old way, the new Bluetooth permissions should be able to work without that extra declaration though, so I went back into AltBeacon itself to see what I could find. Eventually, I landed inside this conditional inside CycledLeScanner, which I identified as the culprit. The permission check in this method does not take the new permissions into account. I believe that checkLocationPermission could be updated to include an SDK check and choose FINE/COARSE or the new BLUETOOTH_SCAN permission depending on the result. No problem from my end to file a PR for this change if it sounds good to you as well.

Expected behavior

Devices running Android 12 should be able to range for beacons without also requiring the old-style permissions.

Actual behavior

A declaration of ACCESS_FINE_LOCATION is required in order to make beacon ranging work, even on Android 12.

Steps to reproduce this behavior

Launch the reference application on an Android 12 device and try to find a nearby beacon with it.

Mobile device model and OS version

Pixel Pro 6, Android 12

Android Beacon Library version

2.19

davidgyoung commented 2 years ago

Hi, Marcel.

Thank you for pointing out that line. At a minimum I think it should be changed to put a warning in the log in an else condition.

However, I am not sure it should be removed. In my tests of an app targeting Android 12 (See here: #1043), it is necessary to have both BLUETOOTH_SCAN and ACCESS_FINE_LOCATION in order to detect common beacon formats like iBeacon, AltBeacon and Eddystone. Android’s documentation says that the manifest must include one of these two:

<uses-permission android:name="android.permission.BLUETOOTH_SCAN"
   android:usesPermissionFlags="neverForLocation" />

or

 <!-- Needed only if your app uses Bluetooth scan results to derive physical location. -->
    <uses-permission android:name="android.permission.ACCESS_FINE_LOCATION" />

And says:

Note: If you include neverForLocation in your android:usesPermissionFlags, some BLE beacons are filtered from the scan results.

See: https://developer.android.com/guide/topics/connectivity/bluetooth/permissions?hl=it_ch&skip_cache=false

Can you explain exactly what you have in your manifest and what kind of beacons you are scanning for? It sounds to me like what you describe is not supposed to work.

mannodermaus commented 2 years ago

Thanks for the quick response! Granted, our setup ranges for beacons with a non-standard, custom layout (i.e. we use setBeaconLayout() with our own format), so I'm not sure if it would be able to find any of the common formats as I don't own any of them. It's possible that AltBeacon, Eddystone and iBeacon all do require the location under the hood for some reason - our custom beacons don't seem to have that same requirement.

I'm adding the full excerpt for the BLE-related permissions from our manifest below. Note that our manifest still declares ACCESS_FINE_LOCATION as well because of unrelated GPS functionality elsewhere in the app. During my testing, I made sure to deny that location access before trying to scan with just BLUETOOTH_SCAN, causing checkPermission() to yield false for ACCESS_FINE_LOCATION.

    <uses-permission
        android:name="android.permission.BLUETOOTH"
        android:maxSdkVersion="30" />
    <uses-permission
        android:name="android.permission.BLUETOOTH_ADMIN"
        android:maxSdkVersion="30" />

    <uses-permission
        android:name="android.permission.BLUETOOTH_SCAN"
        android:usesPermissionFlags="neverForLocation" />
    <uses-permission android:name="android.permission.BLUETOOTH_CONNECT" />

    <uses-permission android:name="android.permission.ACCESS_FINE_LOCATION" />
    <uses-permission android:name="android.permission.ACCESS_COARSE_LOCATION" />

Regarding the proposal for CycledLeScanner, I wasn't trying to get rid of the permission check itself because I agree that it should be retained. The proposed change could look something like this:

    // I guess this could be renamed to 'checkScanPermissions' or sth when doing this
    private boolean checkLocationPermission() {
        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S && checkPermission(Manifest.permission.BLUETOOTH_SCAN)) {
            return true;
        }

        return checkPermission(Manifest.permission.ACCESS_COARSE_LOCATION) || checkPermission(Manifest.permission.ACCESS_FINE_LOCATION);
    }

The above is what I was compiling into the library on a local feature branch in my fork to validate the ranging on my end. I'm intrigued by your saying that this shouldn't technically work. I'll do some more testing on my end to see what other factor might've skewed the results in my favor here!

davidgyoung commented 2 years ago

Ok, understood about the custom beacon layout — even though this is not the most common use case, the library must support this without location permission being granted, so I agree a change is warranted.

I cannot remember exactly why I added code to not start the scan if the permission is not granted. I vaguely remember that trying to start the scan with permission denied caused a crash on some model and OS version.

Assuming my memory is correct, we probably need to leave this check in place, but we can alter it as you suggest and add warnings in the logs that scans will not be performed (and if location permission is not granted) that common beacon formats may be filtered out by Android.

Based on the Android docs, what you are doing now will probably work fine — but if you were using AltBeacon, iBeacon or Eddystone I suspect it would not. Since those formats are the primary use cases for the library we must be extra careful not to break anything with them.

davidgyoung commented 2 years ago

Here is the PR that added these permission checks. The description says that a SecurityException is thrown which blocks the main thread.

https://github.com/AltBeacon/android-beacon-library/issues/326

So I believe these checks need to be retained. Unfortunately Android’s increasingly complex permissions model is not helping us here.

mannodermaus commented 2 years ago

Gotcha! I realize that these benefits would benefit our app but are far from standard usage, so we could raise the obscurity level of enabling this very high if necessary. I don't necessarily foresee a blocker with the proposed extension of checkLocationPermission() as nothing is taken away. But as you said, this is Android and we all know how it behaves sometimes.

Re #326, luckily I do own a Nexus 5 on Android 6.0 myself, so I'll be able to verify the correct behavior of the location permission check even after the proposed change - albeit only with our weird, custom beacon layout. Depending on how much feedback we could get and how high you estimate the potential danger here, the additional permission check for BLUETOOTH_SCAN could be tested in a beta version or become an opt-in setting or configuration flag. If we need to make it a system property that you'd just have to know about in order to enable it, I'd be fine with that as well. Kotlin's debug mode for coroutines does the same thing and it works fairly well for them.

mdivya-symplr commented 1 year ago

@davidgyoung Being new to android, I would like to clarify on following thing: When you meant for BLUETOOTH_SCAN and ACCESS_FINE_LOCATION, Does this mean beacon scanning works only when we allow nearby devices and PRECISE Location? I have observed that for Android 12+, beacons are getting detected only when we allow Precise location but doesn't work for Approximate location. Thanks in advance.

However, I am not sure it should be removed. In my tests of an app targeting Android 12 (See here: https://github.com/AltBeacon/android-beacon-library/pull/1043), it is necessary to have both BLUETOOTH_SCAN and ACCESS_FINE_LOCATION in order to detect common beacon formats like iBeacon, AltBeacon and Eddystone. Android’s documentation says that the manifest must include one of these two:

davidgyoung commented 1 year ago

Yes, beacon scanning works only when we allow nearby devices and PRECISE Location.

mdivya-symplr commented 1 year ago

@davidgyoung Thanks for confirming

mdivya-symplr commented 1 year ago

@davidgyoung Is there an enhancement coming in future to support Approximate location?

davidgyoung commented 1 year ago

My understanding is that it is not possible to detect popular bluetooth beacon formats (iBeacon, Eddystone) on when only approximate location permission is granted on Android 12+ because the operating system filters out those bluetooth detections. The operating system will only pass through those detections to third party apps if precise location permission is granted.

mdivya-symplr commented 1 year ago

@davidgyoung In Android 11 do we need any other permissions apart from ACCESS_COARSE_LOCATION for beacon scan?

For Android 12+, with org.altbeacon:android-beacon-library:2.19.3 by requesting nearby devices permission and precise location beacons are scanning whereas in android 11 beacons are not detecting.

davidgyoung commented 1 year ago

You need ACCESS_FINE_LOCATION instead of ACCESS_COARSE_LOCATION on Android 11. If you want to scan in the background you also need ACCESS_BACKGROUND_LOCATION.

AishwaryaYML commented 1 year ago

@davidgyoung Not able to scan BLE devices in background with just BLUETOOTH_SCAN permission in Android 12. Any leads here? Is ACCESS_BACKGROUND_LOCATION permission mandatory along with BLUETOOTH_SCAN to scan BLE devices in background?

davidgyoung commented 1 year ago

Yes, ACCESS_BACKGROUND_LOCATION permission is needed if you want to scan for iBeacon, AltBeacon or Eddystone advertisements in the background.

If you obtain only BLUETOOTH_SCAN permission, you can only read advertisements in the background that don't match one of the location beacon formats above. Android will filter out detections matching these formats unless you have the appropriate location permission.