From 2a9409e91457c175be5ee36d3e676a70dc52bdca Mon Sep 17 00:00:00 2001 From: Maggie Date: Wed, 21 Mar 2018 11:47:28 -0700 Subject: [PATCH] Fix test location provider bug LocationManager.isProviderEnabled should return true for a test provider if: 1. A test provider is added through LocationManager.addTestProvider 2. The test provider is enabled through LocationManager.setTestProviderEnabled This fix changes the logic if LocationManager.isProviderEnabled and LocationManager.setProviderEnabledForUser to allow users get/set test location provider status. Bug: 72647543 Bug: 77238940 Test: Manual Test: run cts --module CtsLocationTestCases -t android.location.cts.LocationManagerTest Change-Id: Ib241e5b487fd873c1fa0d7ac34b82b8592fc0906 --- .../android/location/ILocationManager.aidl | 4 + .../android/location/LocationManager.java | 80 ++---- .../server/LocationManagerService.java | 229 +++++++++++++++++- 3 files changed, 240 insertions(+), 73 deletions(-) diff --git a/location/java/android/location/ILocationManager.aidl b/location/java/android/location/ILocationManager.aidl index fa3f99a90cc8b..1276881704cf6 100644 --- a/location/java/android/location/ILocationManager.aidl +++ b/location/java/android/location/ILocationManager.aidl @@ -89,6 +89,10 @@ interface ILocationManager ProviderProperties getProviderProperties(String provider); String getNetworkProviderPackage(); + boolean isProviderEnabledForUser(String provider, int userId); + boolean setProviderEnabledForUser(String provider, boolean enabled, int userId); + boolean isLocationEnabledForUser(int userId); + void setLocationEnabledForUser(boolean enabled, int userId); void addTestProvider(String name, in ProviderProperties properties, String opPackageName); void removeTestProvider(String provider, String opPackageName); void setTestProviderLocation(String provider, in Location loc, String opPackageName); diff --git a/location/java/android/location/LocationManager.java b/location/java/android/location/LocationManager.java index a523958080d36..38286a3248650 100644 --- a/location/java/android/location/LocationManager.java +++ b/location/java/android/location/LocationManager.java @@ -1249,40 +1249,11 @@ public class LocationManager { @SystemApi @RequiresPermission(WRITE_SECURE_SETTINGS) public void setLocationEnabledForUser(boolean enabled, UserHandle userHandle) { - final List allProvidersList = getAllProviders(); - // Update all providers on device plus gps and network provider when disabling location. - Set allProvidersSet = new ArraySet<>(allProvidersList.size() + 2); - allProvidersSet.addAll(allProvidersList); - // When disabling location, disable gps and network provider that could have been enabled by - // location mode api. - if (enabled == false) { - allProvidersSet.add(GPS_PROVIDER); - allProvidersSet.add(NETWORK_PROVIDER); + try { + mService.setLocationEnabledForUser(enabled, userHandle.getIdentifier()); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); } - if (allProvidersSet.isEmpty()) { - return; - } - // to ensure thread safety, we write the provider name with a '+' or '-' - // and let the SettingsProvider handle it rather than reading and modifying - // the list of enabled providers. - final String prefix = enabled ? "+" : "-"; - StringBuilder locationProvidersAllowed = new StringBuilder(); - for (String provider : allProvidersSet) { - checkProvider(provider); - if (provider.equals(PASSIVE_PROVIDER)) { - continue; - } - locationProvidersAllowed.append(prefix); - locationProvidersAllowed.append(provider); - locationProvidersAllowed.append(","); - } - // Remove the trailing comma - locationProvidersAllowed.setLength(locationProvidersAllowed.length() - 1); - Settings.Secure.putStringForUser( - mContext.getContentResolver(), - Settings.Secure.LOCATION_PROVIDERS_ALLOWED, - locationProvidersAllowed.toString(), - userHandle.getIdentifier()); } /** @@ -1295,22 +1266,11 @@ public class LocationManager { */ @SystemApi public boolean isLocationEnabledForUser(UserHandle userHandle) { - final String allowedProviders = Settings.Secure.getStringForUser( - mContext.getContentResolver(), Settings.Secure.LOCATION_PROVIDERS_ALLOWED, - userHandle.getIdentifier()); - if (allowedProviders == null) { - return false; + try { + return mService.isLocationEnabledForUser(userHandle.getIdentifier()); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); } - final List providerList = Arrays.asList(allowedProviders.split(",")); - for(String provider : getAllProviders()) { - if (provider.equals(PASSIVE_PROVIDER)) { - continue; - } - if (providerList.contains(provider)) { - return true; - } - } - return false; } /** @@ -1362,9 +1322,12 @@ public class LocationManager { @SystemApi public boolean isProviderEnabledForUser(String provider, UserHandle userHandle) { checkProvider(provider); - String allowedProviders = Settings.Secure.getStringForUser(mContext.getContentResolver(), - Settings.Secure.LOCATION_PROVIDERS_ALLOWED, userHandle.getIdentifier()); - return TextUtils.delimitedStringContains(allowedProviders, ',', provider); + + try { + return mService.isProviderEnabledForUser(provider, userHandle.getIdentifier()); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); + } } /** @@ -1383,16 +1346,13 @@ public class LocationManager { public boolean setProviderEnabledForUser( String provider, boolean enabled, UserHandle userHandle) { checkProvider(provider); - // to ensure thread safety, we write the provider name with a '+' or '-' - // and let the SettingsProvider handle it rather than reading and modifying - // the list of enabled providers. - if (enabled) { - provider = "+" + provider; - } else { - provider = "-" + provider; + + try { + return mService.setProviderEnabledForUser( + provider, enabled, userHandle.getIdentifier()); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); } - return Settings.Secure.putStringForUser(mContext.getContentResolver(), - Settings.Secure.LOCATION_PROVIDERS_ALLOWED, provider, userHandle.getIdentifier()); } /** diff --git a/services/core/java/com/android/server/LocationManagerService.java b/services/core/java/com/android/server/LocationManagerService.java index 26b83f5ae0c36..fb5fba0d097e5 100644 --- a/services/core/java/com/android/server/LocationManagerService.java +++ b/services/core/java/com/android/server/LocationManagerService.java @@ -1344,13 +1344,7 @@ public class LocationManagerService extends ILocationManager.Stub { * @param provider the name of the location provider */ private boolean isAllowedByCurrentUserSettingsLocked(String provider) { - if (mEnabledProviders.contains(provider)) { - return true; - } - if (mDisabledProviders.contains(provider)) { - return false; - } - return isLocationProviderEnabledForUser(provider, mCurrentUserId); + return isAllowedByUserSettingsLockedForUser(provider, mCurrentUserId); } /** @@ -1359,13 +1353,33 @@ public class LocationManagerService extends ILocationManager.Stub { * processes belonging to background users. * * @param provider the name of the location provider - * @param uid the requestor's UID + * @param userId the user id to query */ - private boolean isAllowedByUserSettingsLocked(String provider, int uid) { + private boolean isAllowedByUserSettingsLockedForUser(String provider, int userId) { + if (mEnabledProviders.contains(provider)) { + return true; + } + if (mDisabledProviders.contains(provider)) { + return false; + } + return isLocationProviderEnabledForUser(provider, userId); + } + + + /** + * Returns "true" if access to the specified location provider is allowed by the specified + * user's settings. Access to all location providers is forbidden to non-location-provider + * processes belonging to background users. + * + * @param provider the name of the location provider + * @param uid the requestor's UID + * @param userId the user id to query + */ + private boolean isAllowedByUserSettingsLocked(String provider, int uid, int userId) { if (!isCurrentProfile(UserHandle.getUserId(uid)) && !isUidALocationProvider(uid)) { return false; } - return isAllowedByCurrentUserSettingsLocked(provider); + return isAllowedByUserSettingsLockedForUser(provider, userId); } /** @@ -1572,7 +1586,8 @@ public class LocationManagerService extends ILocationManager.Stub { continue; } if (allowedResolutionLevel >= getMinimumResolutionLevelForProviderUse(name)) { - if (enabledOnly && !isAllowedByUserSettingsLocked(name, uid)) { + if (enabledOnly + && !isAllowedByUserSettingsLocked(name, uid, mCurrentUserId)) { continue; } if (criteria != null && !LocationProvider.propertiesMeetCriteria( @@ -2098,7 +2113,7 @@ public class LocationManagerService extends ILocationManager.Stub { oldRecord.disposeLocked(false); } - boolean isProviderEnabled = isAllowedByUserSettingsLocked(name, uid); + boolean isProviderEnabled = isAllowedByUserSettingsLocked(name, uid, mCurrentUserId); if (isProviderEnabled) { applyRequirementsLocked(name); } else { @@ -2219,7 +2234,7 @@ public class LocationManagerService extends ILocationManager.Stub { LocationProviderInterface provider = mProvidersByName.get(name); if (provider == null) return null; - if (!isAllowedByUserSettingsLocked(name, uid)) return null; + if (!isAllowedByUserSettingsLocked(name, uid, mCurrentUserId)) return null; Location location; if (allowedResolutionLevel < RESOLUTION_LEVEL_FINE) { @@ -2539,6 +2554,173 @@ public class LocationManagerService extends ILocationManager.Stub { return null; } + /** + * Returns the current location enabled/disabled status for a user + * + * @param userId the id of the user + * @return true if location is enabled + */ + @Override + public boolean isLocationEnabledForUser(int userId) { + // Check INTERACT_ACROSS_USERS permission if userId is not current user id. + checkInteractAcrossUsersPermission(userId); + + long identity = Binder.clearCallingIdentity(); + try { + synchronized (mLock) { + final String allowedProviders = Settings.Secure.getStringForUser( + mContext.getContentResolver(), + Settings.Secure.LOCATION_PROVIDERS_ALLOWED, + userId); + if (allowedProviders == null) { + return false; + } + final List providerList = Arrays.asList(allowedProviders.split(",")); + for(String provider : mRealProviders.keySet()) { + if (provider.equals(LocationManager.PASSIVE_PROVIDER) + || provider.equals(LocationManager.FUSED_PROVIDER)) { + continue; + } + if (providerList.contains(provider)) { + return true; + } + } + return false; + } + } finally { + Binder.restoreCallingIdentity(identity); + } + } + + /** + * Enable or disable location for a user + * + * @param enabled true to enable location, false to disable location + * @param userId the id of the user + */ + @Override + public void setLocationEnabledForUser(boolean enabled, int userId) { + mContext.enforceCallingPermission( + android.Manifest.permission.WRITE_SECURE_SETTINGS, + "Requires WRITE_SECURE_SETTINGS permission"); + + // Check INTERACT_ACROSS_USERS permission if userId is not current user id. + checkInteractAcrossUsersPermission(userId); + + long identity = Binder.clearCallingIdentity(); + try { + synchronized (mLock) { + final Set allRealProviders = mRealProviders.keySet(); + // Update all providers on device plus gps and network provider when disabling + // location + Set allProvidersSet = new ArraySet<>(allRealProviders.size() + 2); + allProvidersSet.addAll(allRealProviders); + // When disabling location, disable gps and network provider that could have been + // enabled by location mode api. + if (enabled == false) { + allProvidersSet.add(LocationManager.GPS_PROVIDER); + allProvidersSet.add(LocationManager.NETWORK_PROVIDER); + } + if (allProvidersSet.isEmpty()) { + return; + } + // to ensure thread safety, we write the provider name with a '+' or '-' + // and let the SettingsProvider handle it rather than reading and modifying + // the list of enabled providers. + final String prefix = enabled ? "+" : "-"; + StringBuilder locationProvidersAllowed = new StringBuilder(); + for (String provider : allProvidersSet) { + if (provider.equals(LocationManager.PASSIVE_PROVIDER) + || provider.equals(LocationManager.FUSED_PROVIDER)) { + continue; + } + locationProvidersAllowed.append(prefix); + locationProvidersAllowed.append(provider); + locationProvidersAllowed.append(","); + } + // Remove the trailing comma + locationProvidersAllowed.setLength(locationProvidersAllowed.length() - 1); + Settings.Secure.putStringForUser( + mContext.getContentResolver(), + Settings.Secure.LOCATION_PROVIDERS_ALLOWED, + locationProvidersAllowed.toString(), + userId); + } + } finally { + Binder.restoreCallingIdentity(identity); + } + } + + /** + * Returns the current enabled/disabled status of a location provider and user + * + * @param provider name of the provider + * @param userId the id of the user + * @return true if the provider exists and is enabled + */ + @Override + public boolean isProviderEnabledForUser(String provider, int userId) { + // Check INTERACT_ACROSS_USERS permission if userId is not current user id. + checkInteractAcrossUsersPermission(userId); + + // Fused provider is accessed indirectly via criteria rather than the provider-based APIs, + // so we discourage its use + if (LocationManager.FUSED_PROVIDER.equals(provider)) return false; + + int uid = Binder.getCallingUid(); + synchronized (mLock) { + LocationProviderInterface p = mProvidersByName.get(provider); + return p != null + && isAllowedByUserSettingsLocked(provider, uid, userId); + } + } + + /** + * Enable or disable a single location provider. + * + * @param provider name of the provider + * @param enabled true to enable the provider. False to disable the provider + * @param userId the id of the user to set + * @return true if the value was set, false on errors + */ + @Override + public boolean setProviderEnabledForUser(String provider, boolean enabled, int userId) { + mContext.enforceCallingPermission( + android.Manifest.permission.WRITE_SECURE_SETTINGS, + "Requires WRITE_SECURE_SETTINGS permission"); + + // Check INTERACT_ACROSS_USERS permission if userId is not current user id. + checkInteractAcrossUsersPermission(userId); + + // Fused provider is accessed indirectly via criteria rather than the provider-based APIs, + // so we discourage its use + if (LocationManager.FUSED_PROVIDER.equals(provider)) return false; + + long identity = Binder.clearCallingIdentity(); + try { + synchronized (mLock) { + // No such provider exists + if (!mProvidersByName.containsKey(provider)) return false; + + // If it is a test provider, do not write to Settings.Secure + if (mMockProviders.containsKey(provider)) { + setTestProviderEnabled(provider, enabled); + return true; + } + + // to ensure thread safety, we write the provider name with a '+' or '-' + // and let the SettingsProvider handle it rather than reading and modifying + // the list of enabled providers. + String providerChange = (enabled ? "+" : "-") + provider; + return Settings.Secure.putStringForUser( + mContext.getContentResolver(), Settings.Secure.LOCATION_PROVIDERS_ALLOWED, + providerChange, userId); + } + } finally { + Binder.restoreCallingIdentity(identity); + } + } + /** * Read location provider status from Settings.Secure * @@ -2559,6 +2741,23 @@ public class LocationManagerService extends ILocationManager.Stub { } } + /** + * Method for checking INTERACT_ACROSS_USERS permission if specified user id is not the same as + * current user id + * + * @param userId the user id to get or set value + */ + private void checkInteractAcrossUsersPermission(int userId) { + int uid = Binder.getCallingUid(); + if (UserHandle.getUserId(uid) != userId) { + if (ActivityManager.checkComponentPermission( + android.Manifest.permission.INTERACT_ACROSS_USERS, uid, -1, true) + != PERMISSION_GRANTED) { + throw new SecurityException("Requires INTERACT_ACROSS_USERS permission"); + } + } + } + /** * Returns "true" if the UID belongs to a bound location provider. * @@ -3076,7 +3275,11 @@ public class LocationManagerService extends ILocationManager.Stub { if (!canCallerAccessMockLocation(opPackageName)) { return; } + setTestProviderEnabled(provider, enabled); + } + /** Enable or disable a test location provider. */ + private void setTestProviderEnabled(String provider, boolean enabled) { synchronized (mLock) { MockProvider mockProvider = mMockProviders.get(provider); if (mockProvider == null) {