Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix android bluetooth permission request above api version 30 #993

Closed
wants to merge 2 commits into from

Conversation

rufman
Copy link

@rufman rufman commented Feb 10, 2023

The BLUETOOTH permission was removed in api version 30 in favor of BLUETOOTH_SCAN, BLUETOOTH_ADVERTISE and BLUETOOTH_CONNECT.

This means that manifests targetting api version above 30 will not have this permission set. For these versions the manifest should be checked for the new permissions.

This change ensures that the documented behavior of the bluetooth permission always returning true is true.

Fixes issues #884

✨ What kind of change does this PR introduce? (Bug fix, feature, docs update...)

Bug fix for Android api versions above 30 requesting Bluetooth permissions.

⤵️ What is the current behavior?

Permission.bluetooth return denied instead of the documented always allowed (https://pub.dev/documentation/permission_handler_platform_interface/latest/permission_handler_platform_interface/Permission/bluetooth-constant.html)

🆕 What is the new behavior (if this is a feature change)?

This commit brings the code in line with the documented behavior

💥 Does this PR introduce a breaking change?

No.

🐛 Recommendations for testing

Test on api versions after 30 with a manifest that only includes one of the new permissions, and either no BLUETOOTH permission, or `

📝 Links to relevant issues/docs

🤔 Checklist before submitting

  • I made sure all projects build.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
  • I updated CHANGELOG.md to add a description of the change.
  • I followed the style guide lines (code style guide).
  • I updated the relevant documentation.
  • I rebased onto current master.

The BLUETOOTH permission was removed in api version 30 in favor of
BLUETOOTH_SCAN, BLUETOOTH_ADVERTISE and BLUETOOTH_CONNECT.

This means that manifests targetting api version above 30 will not have
this permission set. For these versions the manifest should be checked
for the new permissions.

This change ensures that the documented behavior of the bluetooth
permission always returning `true` is true.

Fixes issues Baseflow#884
Comment on lines +515 to 516
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only one of these should remain. If both are kept this results in an error

@C-8
Copy link

C-8 commented Jun 21, 2023

Is there an update on this?
Right now this workaround #884 is not even working anymore, since it throws error
Element uses-permission#android.permission.BLUETOOTH at AndroidManifest.xml:6:3-66 duplicated with element declared at AndroidManifest.xml:3:3-5:36

List<String> names = PermissionUtils.getManifestNames(context, PermissionConstants.PERMISSION_GROUP_BLUETOOTH);
boolean missingInManifest = names == null || names.isEmpty();
if (missingInManifest) {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) {
Copy link
Contributor

@wujek-srujek wujek-srujek Jul 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have just spent some time with this library and getting to know its code and testing it on various Android devices, and here is the result of my research: this change (and PR) seems to be unnecessary, at least with version 10.4.2.

As you can see here https://github.com/Baseflow/flutter-permission-handler/blob/permission_handler_v10.4.2/permission_handler_android/android/src/main/java/com/baseflow/permissionhandler/PermissionManager.java#L322-L324 the checkBluetoothPermissionStatus method edited here is used if the new permissions are checked on a device with API level < S (31) - in this case the library will simply fall back to checking for the old Permissions.BLUETOOTH (code before this PR). If the current device is 31 or higher, the method will not be called and the permissions will be checked by the code starting here: https://github.com/Baseflow/flutter-permission-handler/blob/permission_handler_v10.4.2/permission_handler_android/android/src/main/java/com/baseflow/permissionhandler/PermissionManager.java#L327.

Similarly, requestPermissions (not touched in this PR) works correctly - on pre-31 devices it will first determine the status of the permissions (by using checkBluetoothPermissionStatus and the fallback logic as described above) and because Permissions.BLUETOOTH is a 'normal' permission, as long as it is in the manifest it will be 'granted' (not having it in the manifest is an error), so requesting it is a no-op - it will not put the permission to permissionsToRequest list. On 31 and later devices, it will correctly request the new permissions.

Which means - as long as the Flutter part of you application uses new permissions, it will all work even on pre-31 devices as the library is smart enough to fallback to the old permission if new ones are used on old devices. Tested with both a 33 and 23 device.

As a side note, irrelevant to this PR, the same logic applies to the new Permissions.POST_NOTIFICATIONS introduced in API level 33 - if used on a pre-33 device, this library falls back to using the NotificationManagerCompat for the check, and 33 and later check the permission - see https://github.com/Baseflow/flutter-permission-handler/blob/permission_handler_v10.4.2/permission_handler_android/android/src/main/java/com/baseflow/permissionhandler/PermissionManager.java#L479-L492.

@wujek-srujek
Copy link
Contributor

Is there an update on this? Right now this workaround #884 is not even working anymore, since it throws error Element uses-permission#android.permission.BLUETOOTH at AndroidManifest.xml:6:3-66 duplicated with element declared at AndroidManifest.xml:3:3-5:36

I have a test app running on devices with API level 23 and 33 and I don't need any workarounds. I always use the new permissions, it works even on old devices (see my other rather lengthy comment) and the lib implements fallbacks. I think this PR should be closed as it is unnecessary.

@mvanbeusekom
Copy link
Member

Agreed with @wujek-srujek, closing this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants