Merge "Fix bugs in AppOp restrictions" into sc-dev

This commit is contained in:
Soonil Nagarkar
2021-05-13 17:47:44 +00:00
committed by Android (Google) Code Review
3 changed files with 86 additions and 77 deletions

View File

@@ -146,7 +146,8 @@ import java.util.concurrent.CopyOnWriteArrayList;
* The service class that manages LocationProviders and issues location
* updates and alerts.
*/
public class LocationManagerService extends ILocationManager.Stub {
public class LocationManagerService extends ILocationManager.Stub implements
LocationProviderManager.StateChangedListener {
/**
* Controls lifecycle of LocationManagerService.
@@ -228,7 +229,7 @@ public class LocationManagerService extends ILocationManager.Stub {
private static final String ATTRIBUTION_TAG = "LocationService";
private final Object mLock = new Object();
final Object mLock = new Object();
private final Context mContext;
private final Injector mInjector;
@@ -257,7 +258,7 @@ public class LocationManagerService extends ILocationManager.Stub {
new CopyOnWriteArrayList<>();
@GuardedBy("mLock")
private @Nullable OnProviderLocationTagsChangeListener mOnProviderLocationTagsChangeListener;
@Nullable OnProviderLocationTagsChangeListener mOnProviderLocationTagsChangeListener;
LocationManagerService(Context context, Injector injector) {
mContext = context.createAttributionContext(ATTRIBUTION_TAG);
@@ -331,9 +332,8 @@ public class LocationManagerService extends ILocationManager.Stub {
synchronized (mProviderManagers) {
Preconditions.checkState(getLocationProviderManager(manager.getName()) == null);
manager.startManager();
manager.setOnProviderLocationTagsChangeListener(
mOnProviderLocationTagsChangeListener);
manager.startManager(this);
if (realProvider != null) {
// custom logic wrapping all non-passive providers
if (manager != mPassiveManager) {
@@ -1356,6 +1356,44 @@ public class LocationManagerService extends ILocationManager.Stub {
ipw.decreaseIndent();
}
@Override
public void onStateChanged(String provider, AbstractLocationProvider.State oldState,
AbstractLocationProvider.State newState) {
if (!Objects.equals(oldState.identity, newState.identity)) {
refreshAppOpsRestrictions(UserHandle.USER_ALL);
}
OnProviderLocationTagsChangeListener listener;
synchronized (mLock) {
listener = mOnProviderLocationTagsChangeListener;
}
if (listener != null) {
if (!oldState.extraAttributionTags.equals(newState.extraAttributionTags)
|| !Objects.equals(oldState.identity, newState.identity)) {
if (oldState.identity != null) {
listener.onLocationTagsChanged(
new LocationManagerInternal.LocationTagInfo(
oldState.identity.getUid(),
oldState.identity.getPackageName(),
Collections.emptySet()));
}
if (newState.identity != null) {
ArraySet<String> attributionTags = new ArraySet<>(
newState.extraAttributionTags.size() + 1);
attributionTags.addAll(newState.extraAttributionTags);
attributionTags.add(newState.identity.getAttributionTag());
listener.onLocationTagsChanged(
new LocationManagerInternal.LocationTagInfo(
newState.identity.getUid(),
newState.identity.getPackageName(),
attributionTags));
}
}
}
}
private void refreshAppOpsRestrictions(int userId) {
if (userId == UserHandle.USER_ALL) {
final int[] runningUserIds = mInjector.getUserInfoHelper().getRunningUserIds();
@@ -1367,27 +1405,34 @@ public class LocationManagerService extends ILocationManager.Stub {
Preconditions.checkArgument(userId >= 0);
ArraySet<String> packages = new ArraySet<>();
for (LocationProviderManager manager : mProviderManagers) {
packages.add(manager.getIdentity().getPackageName());
}
packages.add(mContext.getPackageName());
packages.addAll(mInjector.getSettingsHelper().getIgnoreSettingsPackageWhitelist());
String[] allowedPackages = packages.toArray(new String[0]);
boolean enabled = mInjector.getSettingsHelper().isLocationEnabled(userId);
String[] allowedPackages = null;
if (!enabled) {
ArraySet<String> packages = new ArraySet<>();
for (LocationProviderManager manager : mProviderManagers) {
CallerIdentity identity = manager.getIdentity();
if (identity != null) {
packages.add(identity.getPackageName());
}
}
packages.add(mContext.getPackageName());
packages.addAll(mInjector.getSettingsHelper().getIgnoreSettingsPackageWhitelist());
allowedPackages = packages.toArray(new String[0]);
}
AppOpsManager appOpsManager = Objects.requireNonNull(
mContext.getSystemService(AppOpsManager.class));
appOpsManager.setUserRestrictionForUser(
AppOpsManager.OP_COARSE_LOCATION,
enabled,
!enabled,
LocationManagerService.this,
allowedPackages,
userId);
appOpsManager.setUserRestrictionForUser(
AppOpsManager.OP_FINE_LOCATION,
enabled,
!enabled,
LocationManagerService.this,
allowedPackages,
userId);
@@ -1467,11 +1512,6 @@ public class LocationManagerService extends ILocationManager.Stub {
@Nullable OnProviderLocationTagsChangeListener listener) {
synchronized (mLock) {
mOnProviderLocationTagsChangeListener = listener;
final int providerCount = mProviderManagers.size();
for (int i = 0; i < providerCount; i++) {
final LocationProviderManager manager = mProviderManagers.get(i);
manager.setOnProviderLocationTagsChangeListener(listener);
}
}
}
}

View File

@@ -55,8 +55,6 @@ import android.location.LastLocationRequest;
import android.location.Location;
import android.location.LocationManager;
import android.location.LocationManagerInternal;
import android.location.LocationManagerInternal.LocationTagInfo;
import android.location.LocationManagerInternal.OnProviderLocationTagsChangeListener;
import android.location.LocationManagerInternal.ProviderEnabledListener;
import android.location.LocationRequest;
import android.location.LocationResult;
@@ -90,7 +88,6 @@ import android.util.TimeUtils;
import com.android.internal.annotations.GuardedBy;
import com.android.internal.util.Preconditions;
import com.android.internal.util.function.pooled.PooledLambda;
import com.android.server.FgThread;
import com.android.server.LocalServices;
import com.android.server.location.LocationPermissions;
@@ -122,7 +119,6 @@ import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Objects;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.function.Predicate;
@@ -171,6 +167,11 @@ public class LocationProviderManager extends
private static final int STATE_STOPPING = 1;
private static final int STATE_STOPPED = 2;
public interface StateChangedListener {
void onStateChanged(String provider, AbstractLocationProvider.State oldState,
AbstractLocationProvider.State newState);
}
protected interface LocationTransport {
void deliverOnLocationChanged(LocationResult locationResult,
@@ -1315,7 +1316,7 @@ public class LocationProviderManager extends
private @Nullable OnAlarmListener mDelayedRegister;
@GuardedBy("mLock")
private @Nullable OnProviderLocationTagsChangeListener mOnLocationTagsChangeListener;
private @Nullable StateChangedListener mStateChangedListener;
public LocationProviderManager(Context context, Injector injector,
String name, @Nullable PassiveLocationProviderManager passiveManager) {
@@ -1354,10 +1355,11 @@ public class LocationProviderManager extends
return TAG;
}
public void startManager() {
public void startManager(@Nullable StateChangedListener listener) {
synchronized (mLock) {
Preconditions.checkState(mState == STATE_STOPPED);
mState = STATE_STARTED;
mStateChangedListener = listener;
mUserHelper.addListener(mUserChangedListener);
mSettingsHelper.addOnLocationEnabledChangedListener(mLocationEnabledChangedListener);
@@ -1396,6 +1398,7 @@ public class LocationProviderManager extends
mEnabled.clear();
mLastLocations.clear();
mStateChangedListener = null;
mState = STATE_STOPPED;
}
}
@@ -1476,19 +1479,6 @@ public class LocationProviderManager extends
}
}
/**
* Registers a listener for the location tags of the provider.
*
* @param listener The listener
*/
public void setOnProviderLocationTagsChangeListener(
@Nullable OnProviderLocationTagsChangeListener listener) {
Preconditions.checkArgument(mOnLocationTagsChangeListener == null || listener == null);
synchronized (mLock) {
mOnLocationTagsChangeListener = listener;
}
}
public void setMockProvider(@Nullable MockLocationProvider provider) {
synchronized (mLock) {
Preconditions.checkState(mState != STATE_STOPPED);
@@ -2292,31 +2282,10 @@ public class LocationProviderManager extends
updateRegistrations(Registration::onProviderPropertiesChanged);
}
if (mOnLocationTagsChangeListener != null) {
if (!oldState.extraAttributionTags.equals(newState.extraAttributionTags)
|| !Objects.equals(oldState.identity, newState.identity)) {
if (oldState.identity != null) {
FgThread.getHandler().sendMessage(PooledLambda.obtainMessage(
OnProviderLocationTagsChangeListener::onLocationTagsChanged,
mOnLocationTagsChangeListener, new LocationTagInfo(
oldState.identity.getUid(), oldState.identity.getPackageName(),
Collections.emptySet())
));
}
if (newState.identity != null) {
ArraySet<String> attributionTags = new ArraySet<>(
newState.extraAttributionTags.size() + 1);
attributionTags.addAll(newState.extraAttributionTags);
attributionTags.add(newState.identity.getAttributionTag());
FgThread.getHandler().sendMessage(PooledLambda.obtainMessage(
OnProviderLocationTagsChangeListener::onLocationTagsChanged,
mOnLocationTagsChangeListener, new LocationTagInfo(
newState.identity.getUid(), newState.identity.getPackageName(),
attributionTags)
));
}
}
if (mStateChangedListener != null) {
StateChangedListener listener = mStateChangedListener;
FgThread.getExecutor().execute(
() -> listener.onStateChanged(mName, oldState, newState));
}
}

View File

@@ -61,8 +61,6 @@ import android.location.ILocationListener;
import android.location.LastLocationRequest;
import android.location.Location;
import android.location.LocationManagerInternal;
import android.location.LocationManagerInternal.LocationTagInfo;
import android.location.LocationManagerInternal.OnProviderLocationTagsChangeListener;
import android.location.LocationManagerInternal.ProviderEnabledListener;
import android.location.LocationRequest;
import android.location.LocationResult;
@@ -133,6 +131,8 @@ public class LocationProviderManagerTest {
private Random mRandom;
@Mock
private LocationProviderManager.StateChangedListener mStateChangedListener;
@Mock
private LocationManagerInternal mInternal;
@Mock
@@ -167,14 +167,14 @@ public class LocationProviderManagerTest {
mInjector.getUserInfoHelper().startUser(OTHER_USER);
mPassive = new PassiveLocationProviderManager(mContext, mInjector);
mPassive.startManager();
mPassive.startManager(null);
mPassive.setRealProvider(new PassiveLocationProvider(mContext));
mProvider = new TestProvider(PROPERTIES, PROVIDER_IDENTITY);
mProvider.setProviderAllowed(true);
mManager = new LocationProviderManager(mContext, mInjector, NAME, mPassive);
mManager.startManager();
mManager.startManager(mStateChangedListener);
mManager.setRealProvider(mProvider);
}
@@ -219,18 +219,18 @@ public class LocationProviderManagerTest {
}
@Test
public void testAttributionTags() {
OnProviderLocationTagsChangeListener listener = mock(
OnProviderLocationTagsChangeListener.class);
mManager.setOnProviderLocationTagsChangeListener(listener);
public void testStateChangedListener() {
mProvider.setExtraAttributionTags(Collections.singleton("extra"));
ArgumentCaptor<LocationTagInfo> captor = ArgumentCaptor.forClass(LocationTagInfo.class);
verify(listener, times(2)).onLocationTagsChanged(captor.capture());
ArgumentCaptor<AbstractLocationProvider.State> captorOld = ArgumentCaptor.forClass(
AbstractLocationProvider.State.class);
ArgumentCaptor<AbstractLocationProvider.State> captorNew = ArgumentCaptor.forClass(
AbstractLocationProvider.State.class);
verify(mStateChangedListener, timeout(TIMEOUT_MS).times(2)).onStateChanged(eq(NAME),
captorOld.capture(), captorNew.capture());
assertThat(captor.getAllValues().get(0).getTags()).isEmpty();
assertThat(captor.getAllValues().get(1).getTags()).containsExactly("extra", "attribution");
assertThat(captorOld.getAllValues().get(1).extraAttributionTags).isEmpty();
assertThat(captorNew.getAllValues().get(1).extraAttributionTags).containsExactly("extra");
}
@Test