Update PermissionChecker usages to avoid unnecessary attribution.

We had accidental usages of the PermissionChecker for cases where no
private data was provided to the app but the checkPermission API on
the latter also did blame data access on the app. The PermissionChecker
was designed to handle IPC calls and not for generic API checks.

To avoid future accidental incorrect PermissionChecker usages this
change renames the existing APIs of the latter to clearly indicate
that they should be used for data delivery and also adds sibling
methods for doing the same permission checks for preflight purposes.
Also the documentation is improved to furhter assist developers.

In addition, this change fixes accidental permission checker usages
that blame when they should not by using the new preflight flavor
of the permission check APIs.

Test:
    atest com.android.settingslib.location.RecentLocationAppsTest
    atest CtsPermissionTestCases
    added: LocationAccessCheckTest#notificationOnlyForAccessesSinceFeatureWasEnabled
    added: LocationAccessCheckTest#noNotificationIfFeatureDisabled
    added: LocationAccessCheckTest#noNotificationIfBlamerNotSystemOrLocationProvider
    added: LocationAccessCheckTest#testOpeningLocationSettingsDoesNotTriggerAccess

bug:141028068
Merged-in: I941dfbca9cb5560bd0facfa4671c04c89965919e
Change-Id: I941dfbca9cb5560bd0facfa4671c04c89965919e
(cherry picked from commit 150f6207288d128f678940e11167b69e05f1207b)

2 files changed, 61 insertions, 5 deletions

diff --git a/hostsidetests/devicepolicy/app/DeviceAndProfileOwner/src/com/android/cts/deviceandprofileowner/PermissionsTest.java b/hostsidetests/devicepolicy/app/DeviceAndProfileOwner/src/com/android/cts/deviceandprofileowner/PermissionsTest.java
index fefe9cdb0c5..1c633c3eaff 100644
--- a/hostsidetests/devicepolicy/app/DeviceAndProfileOwner/src/com/android/cts/deviceandprofileowner/PermissionsTest.java
+++ b/hostsidetests/devicepolicy/app/DeviceAndProfileOwner/src/com/android/cts/deviceandprofileowner/PermissionsTest.java
@@ -343,8 +343,9 @@ public class PermissionsTest extends BaseDeviceAdminTest {
         PackageInfo packageInfo = mContext.getPackageManager().getPackageInfo(
                 SIMPLE_PRE_M_APP_PACKAGE_NAME, 0);
         assertEquals(PackageManager.PERMISSION_GRANTED,
-                PermissionChecker.checkPermission(mContext, PERMISSION_NAME, -1,
-                        packageInfo.applicationInfo.uid, SIMPLE_PRE_M_APP_PACKAGE_NAME));
+                PermissionChecker.checkPermissionForDataDelivery(mContext, PERMISSION_NAME,
+                        PermissionChecker.PID_UNKNOWN, packageInfo.applicationInfo.uid,
+                        SIMPLE_PRE_M_APP_PACKAGE_NAME));
     }
 
     private void assertCanSetPermissionGrantStateAppPreM(int value) throws Exception {
@@ -365,7 +366,8 @@ public class PermissionsTest extends BaseDeviceAdminTest {
 
         // For pre-M apps the access to the data might be prevented via app-ops. Hence check that
         // they are correctly set
-        boolean isGranted = (PermissionChecker.checkPermission(mContext, PERMISSION_NAME, -1,
+        boolean isGranted = (PermissionChecker.checkPermissionForDataDelivery(mContext,
+                PERMISSION_NAME, PermissionChecker.PID_UNKNOWN,
                 packageInfo.applicationInfo.uid, SIMPLE_PRE_M_APP_PACKAGE_NAME)
                 == PackageManager.PERMISSION_GRANTED);
         switch (value) {
diff --git a/tests/tests/permission/src/android/permission/cts/LocationAccessCheckTest.java b/tests/tests/permission/src/android/permission/cts/LocationAccessCheckTest.java
index db30b831596..4ecf5b69d65 100644
--- a/tests/tests/permission/src/android/permission/cts/LocationAccessCheckTest.java
+++ b/tests/tests/permission/src/android/permission/cts/LocationAccessCheckTest.java
@@ -40,6 +40,7 @@ import static org.junit.Assume.assumeTrue;
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
 
 import android.app.ActivityManager;
+import android.app.AppOpsManager;
 import android.app.UiAutomation;
 import android.content.ComponentName;
 import android.content.ContentResolver;
@@ -52,6 +53,7 @@ import android.location.Location;
 import android.location.LocationListener;
 import android.location.LocationManager;
 import android.os.Bundle;
+import android.os.Debug;
 import android.os.IBinder;
 import android.os.Looper;
 import android.os.ParcelFileDescriptor;
@@ -393,6 +395,15 @@ public class LocationAccessCheckTest {
     }
 
     /**
+     * Disable location access check
+     */
+    private void disableLocationAccessCheck() {
+        runWithShellPermissionIdentity(() -> DeviceConfig.setProperty(
+                DeviceConfig.NAMESPACE_PRIVACY,
+                PROPERTY_LOCATION_ACCESS_CHECK_ENABLED, "false", false));
+    }
+
+    /**
      * Make sure fine location can be accessed at all.
      */
     @Before
@@ -526,7 +537,7 @@ public class LocationAccessCheckTest {
         accessLocation();
         getNotification(true);
 
-        assertNull(getNotification(false));
+        assertNull(getNotification(true));
     }
 
     @Test
@@ -544,7 +555,7 @@ public class LocationAccessCheckTest {
         grantPermissionToTestApp(ACCESS_BACKGROUND_LOCATION);
 
         accessLocation();
-        assertNotNull(getNotification(false));
+        assertNotNull(getNotification(true));
     }
 
     @Test
@@ -576,6 +587,7 @@ public class LocationAccessCheckTest {
             eventually(() -> assertNull(getNotification(false)), UNEXPECTED_TIMEOUT_MILLIS);
         } finally {
             installBackgroundAccessApp();
+            getNotification(true);
         }
     }
 
@@ -602,4 +614,46 @@ public class LocationAccessCheckTest {
             installBackgroundAccessApp();
         }
     }
+
+    @Test
+    public void noNotificationIfFeatureDisabled() throws Throwable {
+        disableLocationAccessCheck();
+        accessLocation();
+        assertNull(getNotification(true));
+    }
+
+    @Test
+    public void notificationOnlyForAccessesSinceFeatureWasEnabled() throws Throwable {
+        // Disable the feature and access location in disabled state
+        disableLocationAccessCheck();
+        accessLocation();
+        assertNull(getNotification(true));
+
+        // No notification expected for accesses before enabling the feature
+        enableLocationAccessCheck();
+        assertNull(getNotification(true));
+
+        // Notification expected for access after enabling the feature
+        accessLocation();
+        assertNotNull(getNotification(true));
+    }
+
+    @Test
+    public void noNotificationIfBlamerNotSystemOrLocationProvider() throws Throwable {
+        // Blame the app for access from an untrusted for notification purposes package.
+        runWithShellPermissionIdentity(() -> {
+            AppOpsManager appOpsManager = sContext.getSystemService(AppOpsManager.class);
+            appOpsManager.noteProxyOpNoThrow(AppOpsManager.OPSTR_FINE_LOCATION, TEST_APP_PKG,
+                    sContext.getPackageManager().getPackageUid(TEST_APP_PKG, 0));
+        });
+        assertNull(getNotification(true));
+    }
+
+    @Test
+    public void testOpeningLocationSettingsDoesNotTriggerAccess() throws Throwable {
+        Intent intent = new Intent(Settings.ACTION_LOCATION_SOURCE_SETTINGS);
+        intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
+        sContext.startActivity(intent);
+        assertNull(getNotification(true));
+    }
 }