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

This commit is contained in:
Winson Chiu
2022-06-14 16:43:30 +00:00
committed by Android (Google) Code Review
2 changed files with 220 additions and 21 deletions

View File

@@ -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
&& !DomainVerificationState.isModifiable(previousState)) {
continue;
// Skip if the state hasn't changed or can't be changed
if (previousState != null) {
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.
if (DomainVerificationState.isVerified(state)) {
final int size = verifiedDomains.size();
for (int index = 0; index < size; index++) {
removeUserStatesForDomain(verifiedDomains.valueAt(index));
// Mirror SystemApi behavior of revoking user selection for approved domains.
// This is done in a second pass so that the previous state can be compared before
// the previous method overwrites it with the new state.
if (newlyVerifiedDomains != null) {
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 {

View File

@@ -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() }!!