[WOOMOB-3851] Request the Android 17 local network permission for POS phone-as-reader - #16418
[WOOMOB-3851] Request the Android 17 local network permission for POS phone-as-reader#16418kidinov wants to merge 7 commits into
Conversation
kidinov
added
type: bug
Project manifest changes for WooCommerceThe following changes in the --- ./build/reports/diff_manifest/WooCommerce/vanillaRelease/base_manifest.txt 2026-08-14 15:29:14.057744198 +0000
+++ ./build/reports/diff_manifest/WooCommerce/vanillaRelease/head_manifest.txt 2026-08-14 15:29:16.736610451 +0000
@@ -19,6 +19,7 @@
<uses-permission android:name="android.permission.ACTION_OPEN_DOCUMENT" />
<uses-permission android:name="android.permission.NFC" />
<uses-permission android:name="android.permission.POST_NOTIFICATIONS" />
+ <uses-permission android:name="android.permission.ACCESS_LOCAL_NETWORK" />
<uses-permission android:name="android.permission.CAMERA" />
<queries>Go to https://buildkite.com/automattic/woocommerce-android/builds/41583/canvas?sid=01a000e1-ad01-4024-8776-2755947e1fed, click on the |
|
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
AI Code Review - Found 1 potential issue
The permission plumbing is clean and consistent across both the store (CardReaderModeActivity/CardReaderModeViewModel) and POS (WooPosCardReaderConnectionController) paths, the onResume/recheckPermissions re-entry is idempotent (sessionStarted and the is Connecting guards prevent duplicate sessions/connections), and the new states/events are wired through with matching tests and previews.
Investigated the github-advanced-security[bot] alerts (3x InlinedApi: ACCESS_LOCAL_NETWORK requires API 37, min is 26): these are false positives. Manifest.permission.ACCESS_LOCAL_NETWORK is a compile-time String constant that is inlined at build time, and every runtime request is gated behind WooPermissionUtils.hasLocalNetworkPermission(), which short-circuits to true when !SystemVersionUtils.isAtLeastCinnamonBun(). So the .launch(ACCESS_LOCAL_NETWORK) calls are never reached below API 37. No code change needed for correctness; if lint runs at error level in CI you may want a baseline entry or a scoped @SuppressLint("InlinedApi") to keep the build green.
Automatic review · claude-opus-4-8 · Workflow run
How to reply to a finding
Reply on this review (or inline at the line the finding refers to) with one of:
@claude addressed- I made the change. Bot verifies against the next diff before marking resolved.@claude rejected: <reason>- Will not fix; reason gets quoted on the next review.@claude not-applicable- Finding does not apply (wrong file, already covered elsewhere, etc.).
The bot honours these on the next review pass.
| isLocalNetworkPermissionPermanentlyDenied = !granted && !shouldShowRationale | ||
| val currentState = _state.value | ||
| if (granted && currentState is WooPosCardReaderConnectionState.MissingLocalNetworkPermission) { | ||
| onPhoneConnectClicked(currentState.phone) |
There was a problem hiding this comment.
AI Code Review [follow-up]
Issue: When local-network permission is granted (here and in recheckPermissions() on line 214), you reconnect with the phone captured in the MissingLocalNetworkPermission state. While that state is active, shouldIgnoreDiscoveryUpdates() still lets latestDiscoveredPhones refresh, so a phone that re-advertised on a new host/port during the permission prompt leaves the stored address stale. The first connect would then fail and only recover on retry (which does call refreshAddressOf).
Suggestion: Refresh the address before reconnecting, mirroring onPhoneRetryConnectClicked, e.g. onPhoneConnectClicked(latestDiscoveredPhones.refreshAddressOf(currentState.phone)) in both spots.
…etwork-permission # Conflicts: # WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/cardreader/connection/WooPosCardReaderConnectionController.kt
3 participants
Description
Fixes WOOMOB-3851
On Android 17,
ACCESS_LOCAL_NETWORKbecomes a runtime permission for apps targeting SDK 37. Nothing requested it, so Remote Tap to Pay broke on the targetSdk 37 bump: the phone'sNsdManager.registerServicethrewSecurityException: Missing local network permission, and the tablet's TLS socket to the phone timed out. This declares the permission and asks for it on both sides.The phone treats it as a hard requirement, gated before the session starts, next to the existing location gate. The tablet only asks when the merchant actually picks a phone reader — mDNS browsing is not gated by the permission, so putting it in the pre-discovery requirement chain would have made Bluetooth-only merchants answer a prompt they never need.
Below API 37
hasLocalNetworkPermission()returns true, so nothing changes on Android 16 and earlier.Test Steps
On an Android 17 (API 37) device or emulator:
On Android 16 or earlier, run the same flows and confirm no new prompt appears and pairing still works.
Images/gif
Phone, Android 17 — before vs after
Phone, Android 17 — system prompt and the blocked variant
ACCESS_LOCAL_NETWORKsits in the Nearby devices runtime group, so Android renders it with the same wording as the Bluetooth prompt. Whether a system prompt appears depends on whether the app already holds that group: with Bluetooth granted the request is auto-granted silently, and without it the prompt is shown. Reader mode never asks for Bluetooth, so the phone always prompts.POS tablet, Android 17 — asked only when a phone reader is picked
Since #16406 a returning phone-reader merchant skips the Bluetooth gates, so the tablet holds no Nearby devices permission and gets the system prompt too.
Android 16 regression check — real phone + tablet, no new prompt
RELEASE-NOTES.txtif necessary. Use the "[Internal]" label for non-user-facing changes.