Merge "Fix app links verification resetting user state" into tm-dev

services/core/java/com/android/server/pm/verify/domain/DomainVerificationService.java

@@ -317,7 +317,7 @@ public class DomainVerificationService extends SystemService
         mEnforcer.assertApprovedVerifier(callingUid, mProxy);
         final Computer snapshot = mConnection.snapshot();
         synchronized (mLock) {
-            List<String> verifiedDomains = new ArrayList<>();
+            List<String> newlyVerifiedDomains = new ArrayList<>();
 
             GetAttachedResult result = getAndValidateAttachedLocked(domainSetId, domains,
                     true /* forAutoVerify */, callingUid, null /* userId */, snapshot);
@@ -329,21 +329,28 @@ public class DomainVerificationService extends SystemService
             ArrayMap<String, Integer> stateMap = pkgState.getStateMap();
             for (String domain : domains) {
                 Integer previousState = stateMap.get(domain);
-                if (previousState != null
+                // Skip if the state hasn't changed or can't be changed
-                        && !DomainVerificationState.isModifiable(previousState)) {
+                if (previousState != null) {
-                    continue;
+                    if (previousState == state
+                            || !DomainVerificationState.isModifiable(previousState)) {
+                        continue;
+                    }
                 }
 
                 if (DomainVerificationState.isVerified(state)) {
-                    verifiedDomains.add(domain);
+                    if (previousState == null
+                            || !DomainVerificationState.isVerified(previousState)) {
+                        newlyVerifiedDomains.add(domain);
+                    }
                 }
 
                 stateMap.put(domain, state);
             }
 
-            int size = verifiedDomains.size();
+            // For newly verified domains, revoke their user states across other packages
+            int size = newlyVerifiedDomains.size();
             for (int index = 0; index < size; index++) {
-                removeUserStatesForDomain(verifiedDomains.get(index));
+                removeUserStatesForDomain(pkgState, newlyVerifiedDomains.get(index));
             }
         }
 
@@ -367,7 +374,6 @@ public class DomainVerificationService extends SystemService
                         "State must be one of NO_RESPONSE, SUCCESS, APPROVED, or DENIED");
         }
 
-        ArraySet<String> verifiedDomains = new ArraySet<>();
         if (packageName == null) {
             final Computer snapshot = mConnection.snapshot();
             synchronized (mLock) {
@@ -395,10 +401,6 @@ public class DomainVerificationService extends SystemService
                         validDomains.retainAll(autoVerifyDomains);
                     }
 
-                    if (DomainVerificationState.isVerified(state)) {
-                        verifiedDomains.addAll(validDomains);
-                    }
-
                     setDomainVerificationStatusInternal(pkgState, state, validDomains);
                 }
             }
@@ -424,19 +426,33 @@ public class DomainVerificationService extends SystemService
                     validDomains.retainAll(mCollector.collectValidAutoVerifyDomains(pkg));
                 }
 
+                ArraySet<String> newlyVerifiedDomains = null;
                 if (DomainVerificationState.isVerified(state)) {
-                    verifiedDomains.addAll(validDomains);
+                    newlyVerifiedDomains = new ArraySet<>();
+                    ArrayMap<String, Integer> stateMap = pkgState.getStateMap();
+                    int domainsSize = validDomains.size();
+                    for (int domainIndex = 0; domainIndex < domainsSize; domainIndex++) {
+                        String domain = validDomains.valueAt(domainIndex);
+                        Integer oldState = stateMap.get(domain);
+                        // Only remove if not previously verified
+                        if (oldState == null || !DomainVerificationState.isVerified(oldState)) {
+                            newlyVerifiedDomains.add(domain);
+                        }
+                    }
                 }
 
                 setDomainVerificationStatusInternal(pkgState, state, validDomains);
-            }
-        }
 
-        // Mirror SystemApi behavior of revoking user selection for approved domains.
+                // Mirror SystemApi behavior of revoking user selection for approved domains.
-        if (DomainVerificationState.isVerified(state)) {
+                // This is done in a second pass so that the previous state can be compared before
-            final int size = verifiedDomains.size();
+                // the previous method overwrites it with the new state.
-            for (int index = 0; index < size; index++) {
+                if (newlyVerifiedDomains != null) {
-                removeUserStatesForDomain(verifiedDomains.valueAt(index));
+                    int domainsSize = newlyVerifiedDomains.size();
+                    for (int domainIndex = 0; domainIndex < domainsSize; domainIndex++) {
+                        String domain = newlyVerifiedDomains.valueAt(domainIndex);
+                        removeUserStatesForDomain(pkgState, domain);
+                    }
+                }
             }
         }
 
@@ -452,7 +468,10 @@ public class DomainVerificationService extends SystemService
         }
     }
 
-    private void removeUserStatesForDomain(@NonNull String domain) {
+    private void removeUserStatesForDomain(@NonNull DomainVerificationPkgState owningPkgState,
+            @NonNull String domain) {
+        SparseArray<DomainVerificationInternalUserState> owningUserStates =
+                owningPkgState.getUserStates();
         synchronized (mLock) {
             final int size = mAttachedPkgStates.size();
             for (int index = 0; index < size; index++) {
@@ -460,6 +479,15 @@ public class DomainVerificationService extends SystemService
                 SparseArray<DomainVerificationInternalUserState> array = pkgState.getUserStates();
                 int arraySize = array.size();
                 for (int arrayIndex = 0; arrayIndex < arraySize; arrayIndex++) {
+                    int userId = array.keyAt(arrayIndex);
+                    DomainVerificationInternalUserState owningUserState =
+                            owningUserStates.get(userId);
+                    if (owningUserState != null && !owningUserState.isLinkHandlingAllowed()) {
+                        // Skip users where the owning package has their link handling disabled,
+                        // since revoking those users would lead to no apps being able to handle
+                        // the domain.
+                        continue;
+                    }
                     array.valueAt(arrayIndex).removeHost(domain);
                 }
             }
@@ -527,6 +555,8 @@ public class DomainVerificationService extends SystemService
         mConnection.scheduleWriteSettings();
     }
 
+    @CheckResult
+    @DomainVerificationManager.Error
     public int setDomainVerificationUserSelection(@NonNull UUID domainSetId,
             @NonNull Set<String> domains, boolean enabled, @UserIdInt int userId)
             throws NameNotFoundException {

services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationPackageTest.kt

@@ -78,6 +78,8 @@ class DomainVerificationPackageTest {
         private val DOMAIN_4 = "four.$DOMAIN_BASE"
 
         private const val USER_ID = 0
+        private const val USER_ID_SECONDARY = 10
+        private val USER_IDS = listOf(USER_ID, USER_ID_SECONDARY)
     }
 
     private val pkg1 = mockPkgState(PKG_ONE, UUID_ONE, SIGNATURE_ONE)
@@ -813,6 +815,173 @@ class DomainVerificationPackageTest {
         assertExpectedState(serviceAfter)
     }
 
+    @Test
+    fun verifiedUnapproved_unverifiedSelected_approvalCausesUnselect_systemApi() {
+        verifiedUnapproved_unverifiedSelected_approvalCausesUnselect {
+            setDomainVerificationStatus(it.domainSetId, setOf(DOMAIN_1, DOMAIN_2), STATE_SUCCESS)
+        }
+    }
+
+    @Test
+    fun verifiedUnapproved_unverifiedSelected_approvalCausesUnselect_internalApi() {
+        verifiedUnapproved_unverifiedSelected_approvalCausesUnselect {
+            setDomainVerificationStatusInternal(it.packageName, STATE_SUCCESS,
+                    ArraySet(setOf(DOMAIN_1, DOMAIN_2)))
+        }
+    }
+
+    private fun verifiedUnapproved_unverifiedSelected_approvalCausesUnselect(
+            setStatusBlock: DomainVerificationService.(PackageStateInternal) -> Unit
+    ) {
+        /*
+            Domains tested:
+                1: Becomes verified in package 1, but package 1 disabled in secondary user, only
+                    disables selection for package 2 in main user
+                2: Becomes verified in package 1, unselected by package 2, remains unselected
+                3: Is autoVerify, but unverified, selected by package 2, remains selected
+                4: Non-autoVerify, selected by package 2, remains selected
+         */
+
+        val pkg1 = mockPkgState(
+            PKG_ONE,
+            UUID_ONE,
+            SIGNATURE_ONE,
+            autoVerifyDomains = listOf(DOMAIN_1, DOMAIN_2, DOMAIN_3),
+            otherDomains = listOf(DOMAIN_4)
+        )
+        val pkg2 = mockPkgState(
+            PKG_TWO,
+            UUID_TWO,
+            SIGNATURE_TWO,
+            autoVerifyDomains = emptyList(),
+            otherDomains = listOf(DOMAIN_1, DOMAIN_2, DOMAIN_3, DOMAIN_4)
+        )
+
+        val service = makeService(pkg1, pkg2)
+        service.addPackage(pkg1)
+        service.addPackage(pkg2)
+
+        // Approve domain 1, 3, and 4 for package 2 for both users
+        USER_IDS.forEach {
+            assertThat(
+                service.setDomainVerificationUserSelection(
+                    UUID_TWO,
+                    setOf(DOMAIN_1, DOMAIN_3, DOMAIN_4),
+                    true,
+                    it
+                )
+            ).isEqualTo(DomainVerificationManager.STATUS_OK)
+        }
+
+        // But disable the owner package link handling in the secondary user
+        service.setDomainVerificationLinkHandlingAllowed(pkg1.packageName, false,
+            USER_ID_SECONDARY
+        )
+
+        service.assertState(
+            pkg1,
+            verifyState = listOf(
+                DOMAIN_1 to STATE_NO_RESPONSE,
+                DOMAIN_2 to STATE_NO_RESPONSE,
+                DOMAIN_3 to STATE_NO_RESPONSE,
+            ),
+            userState2LinkHandlingAllowed = false
+        )
+
+        service.assertState(
+            pkg2,
+            verifyState = null,
+            userState1DomainState1 = DOMAIN_STATE_SELECTED,
+            userState1DomainState3 = DOMAIN_STATE_SELECTED,
+            userState1DomainState4 = DOMAIN_STATE_SELECTED,
+            userState2DomainState1 = DOMAIN_STATE_SELECTED,
+            userState2DomainState3 = DOMAIN_STATE_SELECTED,
+            userState2DomainState4 = DOMAIN_STATE_SELECTED,
+        )
+
+        // Verify the owner package
+        service.setStatusBlock(pkg1)
+
+        // Assert that package 1 is now verified, but link handling disabled in secondary user
+        service.assertState(
+            pkg1,
+            verifyState = listOf(
+                DOMAIN_1 to STATE_SUCCESS,
+                DOMAIN_2 to STATE_SUCCESS,
+                DOMAIN_3 to STATE_NO_RESPONSE,
+            ),
+            userState1DomainState1 = DOMAIN_STATE_VERIFIED,
+            userState1DomainState2 = DOMAIN_STATE_VERIFIED,
+            userState1DomainState3 = DOMAIN_STATE_NONE,
+            userState1DomainState4 = DOMAIN_STATE_NONE,
+            userState2LinkHandlingAllowed = false,
+            userState2DomainState1 = DOMAIN_STATE_VERIFIED,
+            userState2DomainState2 = DOMAIN_STATE_VERIFIED,
+            userState2DomainState3 = DOMAIN_STATE_NONE,
+            userState2DomainState4 = DOMAIN_STATE_NONE,
+        )
+
+        // Assert package 2 maintains selected in user where package 1 had link handling disabled
+        service.assertState(
+            pkg2,
+            verifyState = null,
+            userState1DomainState1 = DOMAIN_STATE_NONE,
+            userState1DomainState3 = DOMAIN_STATE_SELECTED,
+            userState1DomainState4 = DOMAIN_STATE_SELECTED,
+            userState2DomainState1 = DOMAIN_STATE_SELECTED,
+            userState2DomainState3 = DOMAIN_STATE_SELECTED,
+            userState2DomainState4 = DOMAIN_STATE_SELECTED,
+        )
+    }
+
+    fun DomainVerificationService.assertState(
+        pkg: PackageStateInternal,
+        verifyState: List<Pair<String, Int>>?,
+        userState1LinkHandlingAllowed: Boolean = true,
+        userState1DomainState1: Int = DOMAIN_STATE_NONE,
+        userState1DomainState2: Int = DOMAIN_STATE_NONE,
+        userState1DomainState3: Int = DOMAIN_STATE_NONE,
+        userState1DomainState4: Int = DOMAIN_STATE_NONE,
+        userState2LinkHandlingAllowed: Boolean = true,
+        userState2DomainState1: Int = DOMAIN_STATE_NONE,
+        userState2DomainState2: Int = DOMAIN_STATE_NONE,
+        userState2DomainState3: Int = DOMAIN_STATE_NONE,
+        userState2DomainState4: Int = DOMAIN_STATE_NONE,
+    ) {
+        if (verifyState == null) {
+            // If no auto verify domains, the info itself will be null
+            assertThat(getDomainVerificationInfo(pkg.packageName)).isNull()
+        } else {
+            getInfo(pkg.packageName).run {
+                assertThat(hostToStateMap).containsExactlyEntriesIn(verifyState.associate { it })
+            }
+        }
+
+        getUserState(pkg.packageName, USER_ID).run {
+            assertThat(isLinkHandlingAllowed).isEqualTo(userState1LinkHandlingAllowed)
+            assertThat(hostToStateMap).containsExactlyEntriesIn(
+                mapOf(
+                    DOMAIN_1 to userState1DomainState1,
+                    DOMAIN_2 to userState1DomainState2,
+                    DOMAIN_3 to userState1DomainState3,
+                    DOMAIN_4 to userState1DomainState4,
+                )
+            )
+        }
+
+        getUserState(pkg.packageName, USER_ID_SECONDARY).run {
+            assertThat(isLinkHandlingAllowed).isEqualTo(userState2LinkHandlingAllowed)
+            assertThat(hostToStateMap).containsExactlyEntriesIn(
+                mapOf(
+                    DOMAIN_1 to userState2DomainState1,
+                    DOMAIN_2 to userState2DomainState2,
+                    DOMAIN_3 to userState2DomainState3,
+                    DOMAIN_4 to userState2DomainState4,
+                )
+            )
+        }
+    }
+
     private fun DomainVerificationService.getInfo(pkgName: String) =
             getDomainVerificationInfo(pkgName)
                     .also { assertThat(it).isNotNull() }!!