[Thread] add @RequiresPermissions for unregisterXxxCallback APIs
Per API council review suggestions in b/309952666, add the same
permission requirements for the unregister APIs.
Bug: 309952666
Test: atest CtsThreadNetworkTestCases
Change-Id: Idc481c7676ddc5d748fe7803f74dd63981186c09
diff --git a/framework-t/api/system-current.txt b/framework-t/api/system-current.txt
index b285d85..05cf9e8 100644
--- a/framework-t/api/system-current.txt
+++ b/framework-t/api/system-current.txt
@@ -500,8 +500,8 @@
method @RequiresPermission(allOf={android.Manifest.permission.ACCESS_NETWORK_STATE, "android.permission.THREAD_NETWORK_PRIVILEGED"}) public void registerOperationalDatasetCallback(@NonNull java.util.concurrent.Executor, @NonNull android.net.thread.ThreadNetworkController.OperationalDatasetCallback);
method @RequiresPermission(android.Manifest.permission.ACCESS_NETWORK_STATE) public void registerStateCallback(@NonNull java.util.concurrent.Executor, @NonNull android.net.thread.ThreadNetworkController.StateCallback);
method @RequiresPermission("android.permission.THREAD_NETWORK_PRIVILEGED") public void scheduleMigration(@NonNull android.net.thread.PendingOperationalDataset, @NonNull java.util.concurrent.Executor, @NonNull android.os.OutcomeReceiver<java.lang.Void,android.net.thread.ThreadNetworkException>);
- method public void unregisterOperationalDatasetCallback(@NonNull android.net.thread.ThreadNetworkController.OperationalDatasetCallback);
- method public void unregisterStateCallback(@NonNull android.net.thread.ThreadNetworkController.StateCallback);
+ method @RequiresPermission(allOf={android.Manifest.permission.ACCESS_NETWORK_STATE, "android.permission.THREAD_NETWORK_PRIVILEGED"}) public void unregisterOperationalDatasetCallback(@NonNull android.net.thread.ThreadNetworkController.OperationalDatasetCallback);
+ method @RequiresPermission(android.Manifest.permission.ACCESS_NETWORK_STATE) public void unregisterStateCallback(@NonNull android.net.thread.ThreadNetworkController.StateCallback);
field public static final int DEVICE_ROLE_CHILD = 2; // 0x2
field public static final int DEVICE_ROLE_DETACHED = 1; // 0x1
field public static final int DEVICE_ROLE_LEADER = 4; // 0x4
diff --git a/thread/framework/java/android/net/thread/ThreadNetworkController.java b/thread/framework/java/android/net/thread/ThreadNetworkController.java
index ec39db4..5c5fda9 100644
--- a/thread/framework/java/android/net/thread/ThreadNetworkController.java
+++ b/thread/framework/java/android/net/thread/ThreadNetworkController.java
@@ -226,15 +226,17 @@
* @param callback the callback which has been registered with {@link #registerStateCallback}
* @throws IllegalArgumentException if {@code callback} hasn't been registered
*/
+ @RequiresPermission(permission.ACCESS_NETWORK_STATE)
public void unregisterStateCallback(@NonNull StateCallback callback) {
requireNonNull(callback, "callback cannot be null");
synchronized (mStateCallbackMapLock) {
- StateCallbackProxy callbackProxy = mStateCallbackMap.remove(callback);
+ StateCallbackProxy callbackProxy = mStateCallbackMap.get(callback);
if (callbackProxy == null) {
throw new IllegalArgumentException("callback hasn't been registered");
}
try {
mControllerService.unregisterStateCallback(callbackProxy);
+ mStateCallbackMap.remove(callback);
} catch (RemoteException e) {
e.rethrowFromSystemServer();
}
@@ -334,15 +336,21 @@
* #registerOperationalDatasetCallback}
* @throws IllegalArgumentException if {@code callback} hasn't been registered
*/
+ @RequiresPermission(
+ allOf = {
+ permission.ACCESS_NETWORK_STATE,
+ "android.permission.THREAD_NETWORK_PRIVILEGED"
+ })
public void unregisterOperationalDatasetCallback(@NonNull OperationalDatasetCallback callback) {
requireNonNull(callback, "callback cannot be null");
synchronized (mOpDatasetCallbackMapLock) {
- OperationalDatasetCallbackProxy callbackProxy = mOpDatasetCallbackMap.remove(callback);
+ OperationalDatasetCallbackProxy callbackProxy = mOpDatasetCallbackMap.get(callback);
if (callbackProxy == null) {
throw new IllegalArgumentException("callback hasn't been registered");
}
try {
mControllerService.unregisterOperationalDatasetCallback(callbackProxy);
+ mOpDatasetCallbackMap.remove(callback);
} catch (RemoteException e) {
e.rethrowFromSystemServer();
}
diff --git a/thread/service/java/com/android/server/thread/ThreadNetworkControllerService.java b/thread/service/java/com/android/server/thread/ThreadNetworkControllerService.java
index 6c9a775..f7872c4 100644
--- a/thread/service/java/com/android/server/thread/ThreadNetworkControllerService.java
+++ b/thread/service/java/com/android/server/thread/ThreadNetworkControllerService.java
@@ -418,12 +418,12 @@
@Override
public void registerStateCallback(IStateCallback stateCallback) throws RemoteException {
enforceAllCallingPermissionsGranted(permission.ACCESS_NETWORK_STATE);
-
mHandler.post(() -> mOtDaemonCallbackProxy.registerStateCallback(stateCallback));
}
@Override
public void unregisterStateCallback(IStateCallback stateCallback) throws RemoteException {
+ enforceAllCallingPermissionsGranted(permission.ACCESS_NETWORK_STATE);
mHandler.post(() -> mOtDaemonCallbackProxy.unregisterStateCallback(stateCallback));
}
@@ -438,6 +438,8 @@
@Override
public void unregisterOperationalDatasetCallback(IOperationalDatasetCallback callback)
throws RemoteException {
+ enforceAllCallingPermissionsGranted(
+ permission.ACCESS_NETWORK_STATE, PERMISSION_THREAD_NETWORK_PRIVILEGED);
mHandler.post(() -> mOtDaemonCallbackProxy.unregisterDatasetCallback(callback));
}
diff --git a/thread/tests/cts/src/android/net/thread/cts/ThreadNetworkControllerTest.java b/thread/tests/cts/src/android/net/thread/cts/ThreadNetworkControllerTest.java
index e17dd02..362ff39 100644
--- a/thread/tests/cts/src/android/net/thread/cts/ThreadNetworkControllerTest.java
+++ b/thread/tests/cts/src/android/net/thread/cts/ThreadNetworkControllerTest.java
@@ -101,7 +101,7 @@
public void tearDown() throws Exception {
if (mManager != null) {
leaveAndWait();
- dropPermissions();
+ dropAllPermissions();
}
}
@@ -128,7 +128,7 @@
getInstrumentation().getUiAutomation().adoptShellPermissionIdentity(allPermissions);
}
- private static void dropPermissions() {
+ private static void dropAllPermissions() {
getInstrumentation().getUiAutomation().dropShellPermissionIdentity();
}
@@ -217,16 +217,21 @@
for (ThreadNetworkController controller : getAllControllers()) {
SettableFuture<Integer> deviceRole = SettableFuture.create();
+ StateCallback callback = deviceRole::set;
- controller.registerStateCallback(mExecutor, role -> deviceRole.set(role));
+ try {
+ controller.registerStateCallback(mExecutor, callback);
- assertThat(deviceRole.get()).isEqualTo(DEVICE_ROLE_STOPPED);
+ assertThat(deviceRole.get()).isEqualTo(DEVICE_ROLE_STOPPED);
+ } finally {
+ controller.unregisterStateCallback(callback);
+ }
}
}
@Test
public void registerStateCallback_noPermissions_throwsSecurityException() throws Exception {
- dropPermissions();
+ dropAllPermissions();
for (ThreadNetworkController controller : getAllControllers()) {
assertThrows(
@@ -252,6 +257,26 @@
}
@Test
+ public void unregisterStateCallback_noPermissions_throwsSecurityException() throws Exception {
+ for (ThreadNetworkController controller : getAllControllers()) {
+ SettableFuture<Integer> deviceRole = SettableFuture.create();
+ StateCallback callback = role -> deviceRole.set(role);
+ grantPermissions(permission.ACCESS_NETWORK_STATE);
+ controller.registerStateCallback(mExecutor, callback);
+
+ try {
+ dropAllPermissions();
+ assertThrows(
+ SecurityException.class,
+ () -> controller.unregisterStateCallback(callback));
+ } finally {
+ grantPermissions(permission.ACCESS_NETWORK_STATE);
+ controller.unregisterStateCallback(callback);
+ }
+ }
+ }
+
+ @Test
public void unregisterStateCallback_callbackRegistered_success() throws Exception {
grantPermissions(permission.ACCESS_NETWORK_STATE);
for (ThreadNetworkController controller : getAllControllers()) {
@@ -282,7 +307,7 @@
grantPermissions(permission.ACCESS_NETWORK_STATE);
for (ThreadNetworkController controller : getAllControllers()) {
SettableFuture<Integer> deviceRole = SettableFuture.create();
- StateCallback callback = role -> deviceRole.set(role);
+ StateCallback callback = deviceRole::set;
controller.registerStateCallback(mExecutor, callback);
controller.unregisterStateCallback(callback);
@@ -300,12 +325,70 @@
for (ThreadNetworkController controller : getAllControllers()) {
SettableFuture<ActiveOperationalDataset> activeFuture = SettableFuture.create();
SettableFuture<PendingOperationalDataset> pendingFuture = SettableFuture.create();
+ var callback = newDatasetCallback(activeFuture, pendingFuture);
- controller.registerOperationalDatasetCallback(
- mExecutor, newDatasetCallback(activeFuture, pendingFuture));
+ try {
+ controller.registerOperationalDatasetCallback(mExecutor, callback);
- assertThat(activeFuture.get()).isNull();
- assertThat(pendingFuture.get()).isNull();
+ assertThat(activeFuture.get()).isNull();
+ assertThat(pendingFuture.get()).isNull();
+ } finally {
+ controller.unregisterOperationalDatasetCallback(callback);
+ }
+ }
+ }
+
+ @Test
+ public void registerOperationalDatasetCallback_noPermissions_throwsSecurityException()
+ throws Exception {
+ dropAllPermissions();
+
+ for (ThreadNetworkController controller : getAllControllers()) {
+ SettableFuture<ActiveOperationalDataset> activeFuture = SettableFuture.create();
+ SettableFuture<PendingOperationalDataset> pendingFuture = SettableFuture.create();
+ var callback = newDatasetCallback(activeFuture, pendingFuture);
+
+ assertThrows(
+ SecurityException.class,
+ () -> controller.registerOperationalDatasetCallback(mExecutor, callback));
+ }
+ }
+
+ @Test
+ public void unregisterOperationalDatasetCallback_callbackRegistered_success() throws Exception {
+ grantPermissions(permission.ACCESS_NETWORK_STATE, PERMISSION_THREAD_NETWORK_PRIVILEGED);
+ for (ThreadNetworkController controller : getAllControllers()) {
+ SettableFuture<ActiveOperationalDataset> activeFuture = SettableFuture.create();
+ SettableFuture<PendingOperationalDataset> pendingFuture = SettableFuture.create();
+ var callback = newDatasetCallback(activeFuture, pendingFuture);
+ controller.registerOperationalDatasetCallback(mExecutor, callback);
+
+ controller.unregisterOperationalDatasetCallback(callback);
+ }
+ }
+
+ @Test
+ public void unregisterOperationalDatasetCallback_noPermissions_throwsSecurityException()
+ throws Exception {
+ dropAllPermissions();
+
+ for (ThreadNetworkController controller : getAllControllers()) {
+ SettableFuture<ActiveOperationalDataset> activeFuture = SettableFuture.create();
+ SettableFuture<PendingOperationalDataset> pendingFuture = SettableFuture.create();
+ var callback = newDatasetCallback(activeFuture, pendingFuture);
+ grantPermissions(permission.ACCESS_NETWORK_STATE, PERMISSION_THREAD_NETWORK_PRIVILEGED);
+ controller.registerOperationalDatasetCallback(mExecutor, callback);
+
+ try {
+ dropAllPermissions();
+ assertThrows(
+ SecurityException.class,
+ () -> controller.unregisterOperationalDatasetCallback(callback));
+ } finally {
+ grantPermissions(
+ permission.ACCESS_NETWORK_STATE, PERMISSION_THREAD_NETWORK_PRIVILEGED);
+ controller.unregisterOperationalDatasetCallback(callback);
+ }
}
}
@@ -343,7 +426,7 @@
@Test
public void join_withoutPrivilegedPermission_throwsSecurityException() throws Exception {
- dropPermissions();
+ dropAllPermissions();
for (ThreadNetworkController controller : getAllControllers()) {
ActiveOperationalDataset activeDataset = newRandomizedDataset("TestNet", controller);
@@ -408,7 +491,7 @@
@Test
public void leave_withoutPrivilegedPermission_throwsSecurityException() {
- dropPermissions();
+ dropAllPermissions();
for (ThreadNetworkController controller : getAllControllers()) {
assertThrows(SecurityException.class, () -> controller.leave(mExecutor, v -> {}));