From 25f29bf126b71b94ac180b2c6664b22682886a12 Mon Sep 17 00:00:00 2001 From: Fan Zhang Date: Tue, 24 Oct 2017 09:27:02 -0700 Subject: [PATCH] Only allow Settings app launch search result page Bug: 68199963 Test: robotest Change-Id: I0018e9c60b0dd46fc2420a563a93b706bf252dc4 --- AndroidManifest.xml | 6 ++ ...ntDisplayAlwaysOnPreferenceController.java | 2 +- ...playNotificationsPreferenceController.java | 8 +-- .../AutoBrightnessPreferenceController.java | 2 +- ...stGestureSettingsPreferenceController.java | 2 +- .../DoubleTapPowerPreferenceController.java | 2 +- .../DoubleTapScreenPreferenceController.java | 2 +- .../PickupGesturePreferenceController.java | 2 +- .../LocationPreferenceController.java | 2 +- ...dgingNotificationPreferenceController.java | 2 +- .../AccessibilityServiceResultLoader.java | 2 +- .../search/DatabaseIndexingUtils.java | 25 +++++--- .../search/InputDeviceResultLoader.java | 4 +- .../search/IntentSearchViewHolder.java | 5 +- .../search/SearchFeatureProvider.java | 11 ++++ .../search/SearchFeatureProviderImpl.java | 13 ++++ .../search/SearchResultTrampoline.java | 62 +++++++++++++++++++ .../settings/search/indexing/IndexData.java | 2 +- ...AssistContextPreferenceControllerTest.java | 16 ++--- ...splayAlwaysOnPreferenceControllerTest.java | 1 - ...NotificationsPreferenceControllerTest.java | 6 +- ...utoBrightnessPreferenceControllerTest.java | 13 ++-- ...oubleTapPowerPreferenceControllerTest.java | 1 - .../LocationPreferenceControllerTest.java | 3 +- ...gNotificationPreferenceControllerTest.java | 18 +++--- .../search/DatabaseIndexingUtilsTest.java | 2 +- .../search/IntentSearchViewHolderTest.java | 6 +- .../search/SearchFeatureProviderImplTest.java | 27 +++++--- 28 files changed, 178 insertions(+), 69 deletions(-) create mode 100644 src/com/android/settings/search/SearchResultTrampoline.java diff --git a/AndroidManifest.xml b/AndroidManifest.xml index e1d22257e43..69dc2ae8b29 100644 --- a/AndroidManifest.xml +++ b/AndroidManifest.xml @@ -219,6 +219,12 @@ android:theme="@style/Theme.Settings.NoActionBar"> + + + controllers = provider.getPreferenceControllers(context); - if (controllers == null ) { + if (controllers == null) { return null; } @@ -106,7 +113,7 @@ public class DatabaseIndexingUtils { /** * @param uriMap Map between the {@link PreferenceControllerMixin} keys * and the controllers themselves. - * @param key The look-up key + * @param key The look-up key * @return The Payload from the {@link PreferenceControllerMixin} specified by the key, * if it exists. Otherwise null. */ diff --git a/src/com/android/settings/search/InputDeviceResultLoader.java b/src/com/android/settings/search/InputDeviceResultLoader.java index 61e1ad1671e..e5e6553a01a 100644 --- a/src/com/android/settings/search/InputDeviceResultLoader.java +++ b/src/com/android/settings/search/InputDeviceResultLoader.java @@ -107,7 +107,7 @@ public class InputDeviceResultLoader extends AsyncLoader info = pm.queryIntentActivities(intent, 0 /* flags */); if (info != null && !info.isEmpty()) { - fragment.startActivity(intent); + fragment.startActivityForResult(intent, REQUEST_CODE_NO_OP); } else { Log.e(TAG, "Cannot launch search result, title: " + result.title + ", " + intent); diff --git a/src/com/android/settings/search/SearchFeatureProvider.java b/src/com/android/settings/search/SearchFeatureProvider.java index 956808d3857..4df8203d796 100644 --- a/src/com/android/settings/search/SearchFeatureProvider.java +++ b/src/com/android/settings/search/SearchFeatureProvider.java @@ -16,6 +16,8 @@ */ package com.android.settings.search; +import android.annotation.NonNull; +import android.content.ComponentName; import android.content.Context; import android.view.View; @@ -32,6 +34,15 @@ public interface SearchFeatureProvider { */ boolean isEnabled(Context context); + /** + * Ensures the caller has necessary privilege to launch search result page. + * + * @throws IllegalArgumentException when caller is null + * @throws SecurityException when caller is not allowed to launch search result page + */ + void verifyLaunchSearchResultPageCaller(Context context, @NonNull ComponentName caller) + throws SecurityException, IllegalArgumentException; + /** * Returns a new loader to search in index database. */ diff --git a/src/com/android/settings/search/SearchFeatureProviderImpl.java b/src/com/android/settings/search/SearchFeatureProviderImpl.java index 420b8470d91..8c4883eecec 100644 --- a/src/com/android/settings/search/SearchFeatureProviderImpl.java +++ b/src/com/android/settings/search/SearchFeatureProviderImpl.java @@ -17,6 +17,7 @@ package com.android.settings.search; +import android.content.ComponentName; import android.content.Context; import android.text.TextUtils; import android.util.Log; @@ -42,6 +43,18 @@ public class SearchFeatureProviderImpl implements SearchFeatureProvider { return true; } + @Override + public void verifyLaunchSearchResultPageCaller(Context context, ComponentName caller) { + if (caller == null) { + throw new IllegalArgumentException("ExternalSettingsTrampoline intents " + + "must be called with startActivityForResult"); + } + final String packageName = caller.getPackageName(); + if (!TextUtils.equals(packageName, context.getPackageName())) { + throw new SecurityException("Only Settings app can launch search result page"); + } + } + @Override public DatabaseResultLoader getDatabaseSearchLoader(Context context, String query) { return new DatabaseResultLoader(context, cleanQuery(query), getSiteMapManager()); diff --git a/src/com/android/settings/search/SearchResultTrampoline.java b/src/com/android/settings/search/SearchResultTrampoline.java new file mode 100644 index 00000000000..3bbe6bd58a7 --- /dev/null +++ b/src/com/android/settings/search/SearchResultTrampoline.java @@ -0,0 +1,62 @@ +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.settings.search; + +import static com.android.settings.SettingsActivity.EXTRA_SHOW_FRAGMENT_ARGUMENTS; + +import android.app.Activity; +import android.content.Intent; +import android.os.Bundle; + +import com.android.settings.SettingsActivity; +import com.android.settings.SubSettings; +import com.android.settings.overlay.FeatureFactory; + +/** + * A trampoline activity that launches setting result page. + */ +public class SearchResultTrampoline extends Activity { + + @Override + protected void onCreate(Bundle savedInstanceState) { + super.onCreate(savedInstanceState); + + // First make sure caller has privilege to launch a search result page. + FeatureFactory.getFactory(this) + .getSearchFeatureProvider() + .verifyLaunchSearchResultPageCaller(this, getCallingActivity()); + // Didn't crash, proceed and launch the result as a subsetting. + final Intent intent = getIntent(); + + // Hack to take EXTRA_FRAGMENT_ARG_KEY from intent and set into + // EXTRA_SHOW_FRAGMENT_ARGUMENTS. This is necessary because intent could be from external + // caller and args may not persisted. + final String settingKey = intent.getStringExtra(SettingsActivity.EXTRA_FRAGMENT_ARG_KEY); + final Bundle args = new Bundle(); + args.putString(SettingsActivity.EXTRA_FRAGMENT_ARG_KEY, settingKey); + intent.putExtra(EXTRA_SHOW_FRAGMENT_ARGUMENTS, args); + + // Reroute request to SubSetting. + intent.setClass(this /* context */, SubSettings.class) + .addFlags(Intent.FLAG_ACTIVITY_FORWARD_RESULT); + startActivity(intent); + + // Done. + finish(); + } + +} diff --git a/src/com/android/settings/search/indexing/IndexData.java b/src/com/android/settings/search/indexing/IndexData.java index 0e1fa2dc546..9dad3fe5caf 100644 --- a/src/com/android/settings/search/indexing/IndexData.java +++ b/src/com/android/settings/search/indexing/IndexData.java @@ -273,7 +273,7 @@ public class IndexData { || TextUtils.equals(mIntentTargetPackage, SearchIndexableResources.SUBSETTING_TARGET_PACKAGE)) { // Action is null, we will launch it as a sub-setting - intent = DatabaseIndexingUtils.buildSubsettingIntent(context, mClassName, mKey, + intent = DatabaseIndexingUtils.buildSearchResultPageIntent(context, mClassName, mKey, mScreenTitle); } else { intent = new Intent(mIntentAction); diff --git a/tests/robotests/src/com/android/settings/applications/assist/AssistContextPreferenceControllerTest.java b/tests/robotests/src/com/android/settings/applications/assist/AssistContextPreferenceControllerTest.java index 6c8ad9ba827..c71bc2937dd 100644 --- a/tests/robotests/src/com/android/settings/applications/assist/AssistContextPreferenceControllerTest.java +++ b/tests/robotests/src/com/android/settings/applications/assist/AssistContextPreferenceControllerTest.java @@ -16,14 +16,21 @@ package com.android.settings.applications.assist; +import static com.google.common.truth.Truth.assertThat; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyString; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + import android.content.ContentResolver; import android.content.Context; import android.provider.Settings; import android.support.v7.preference.PreferenceScreen; import android.support.v7.preference.TwoStatePreference; -import com.android.settings.testutils.SettingsRobolectricTestRunner; import com.android.settings.TestConfig; +import com.android.settings.testutils.SettingsRobolectricTestRunner; import com.android.settingslib.core.lifecycle.Lifecycle; import org.junit.Before; @@ -35,13 +42,6 @@ import org.robolectric.RuntimeEnvironment; import org.robolectric.annotation.Config; import org.robolectric.util.ReflectionHelpers; -import static com.google.common.truth.Truth.assertThat; -import static org.mockito.Matchers.any; -import static org.mockito.Matchers.anyString; -import static org.mockito.Matchers.eq; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - @RunWith(SettingsRobolectricTestRunner.class) @Config(manifest = TestConfig.MANIFEST_PATH, sdk = TestConfig.SDK_VERSION) public class AssistContextPreferenceControllerTest { diff --git a/tests/robotests/src/com/android/settings/display/AmbientDisplayAlwaysOnPreferenceControllerTest.java b/tests/robotests/src/com/android/settings/display/AmbientDisplayAlwaysOnPreferenceControllerTest.java index 386980709da..dfe81dbc4c7 100644 --- a/tests/robotests/src/com/android/settings/display/AmbientDisplayAlwaysOnPreferenceControllerTest.java +++ b/tests/robotests/src/com/android/settings/display/AmbientDisplayAlwaysOnPreferenceControllerTest.java @@ -17,7 +17,6 @@ package com.android.settings.display; import static com.google.common.truth.Truth.assertThat; - import static org.mockito.Matchers.anyInt; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; diff --git a/tests/robotests/src/com/android/settings/display/AmbientDisplayNotificationsPreferenceControllerTest.java b/tests/robotests/src/com/android/settings/display/AmbientDisplayNotificationsPreferenceControllerTest.java index 242a05d4fae..e1ce6945c13 100644 --- a/tests/robotests/src/com/android/settings/display/AmbientDisplayNotificationsPreferenceControllerTest.java +++ b/tests/robotests/src/com/android/settings/display/AmbientDisplayNotificationsPreferenceControllerTest.java @@ -17,9 +17,7 @@ package com.android.settings.display; import static com.android.internal.logging.nano.MetricsProto.MetricsEvent.ACTION_AMBIENT_DISPLAY; - import static com.google.common.truth.Truth.assertThat; - import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.eq; @@ -33,11 +31,11 @@ import android.provider.Settings; import android.support.v14.preference.SwitchPreference; import com.android.internal.hardware.AmbientDisplayConfiguration; +import com.android.settings.TestConfig; +import com.android.settings.core.instrumentation.MetricsFeatureProvider; import com.android.settings.search.InlinePayload; import com.android.settings.search.InlineSwitchPayload; import com.android.settings.testutils.SettingsRobolectricTestRunner; -import com.android.settings.TestConfig; -import com.android.settings.core.instrumentation.MetricsFeatureProvider; import com.android.settings.testutils.shadow.ShadowSecureSettings; import org.junit.Before; diff --git a/tests/robotests/src/com/android/settings/display/AutoBrightnessPreferenceControllerTest.java b/tests/robotests/src/com/android/settings/display/AutoBrightnessPreferenceControllerTest.java index 02f46f248fa..b46441dff65 100644 --- a/tests/robotests/src/com/android/settings/display/AutoBrightnessPreferenceControllerTest.java +++ b/tests/robotests/src/com/android/settings/display/AutoBrightnessPreferenceControllerTest.java @@ -16,14 +16,20 @@ package com.android.settings.display; +import static android.provider.Settings.System.SCREEN_BRIGHTNESS_MODE; +import static android.provider.Settings.System.SCREEN_BRIGHTNESS_MODE_AUTOMATIC; +import static android.provider.Settings.System.SCREEN_BRIGHTNESS_MODE_MANUAL; +import static com.google.common.truth.Truth.assertThat; + import android.content.ContentResolver; import android.content.Context; import android.provider.Settings; -import com.android.settings.testutils.SettingsRobolectricTestRunner; + import com.android.settings.TestConfig; import com.android.settings.search.InlinePayload; import com.android.settings.search.InlineSwitchPayload; import com.android.settings.search.ResultPayload; +import com.android.settings.testutils.SettingsRobolectricTestRunner; import org.junit.Before; import org.junit.Test; @@ -34,11 +40,6 @@ import org.mockito.MockitoAnnotations; import org.robolectric.annotation.Config; import org.robolectric.shadows.ShadowApplication; -import static android.provider.Settings.System.SCREEN_BRIGHTNESS_MODE; -import static android.provider.Settings.System.SCREEN_BRIGHTNESS_MODE_AUTOMATIC; -import static android.provider.Settings.System.SCREEN_BRIGHTNESS_MODE_MANUAL; -import static com.google.common.truth.Truth.assertThat; - @RunWith(SettingsRobolectricTestRunner.class) @Config(manifest = TestConfig.MANIFEST_PATH, sdk = TestConfig.SDK_VERSION) public class AutoBrightnessPreferenceControllerTest { diff --git a/tests/robotests/src/com/android/settings/gestures/DoubleTapPowerPreferenceControllerTest.java b/tests/robotests/src/com/android/settings/gestures/DoubleTapPowerPreferenceControllerTest.java index 0c804b364e6..1f5ca209c38 100644 --- a/tests/robotests/src/com/android/settings/gestures/DoubleTapPowerPreferenceControllerTest.java +++ b/tests/robotests/src/com/android/settings/gestures/DoubleTapPowerPreferenceControllerTest.java @@ -35,7 +35,6 @@ import com.android.settings.search.InlineSwitchPayload; import com.android.settings.search.ResultPayload; import com.android.settings.testutils.SettingsRobolectricTestRunner; import com.android.settings.testutils.shadow.SettingsShadowResources; -import com.android.settings.testutils.shadow.SettingsShadowSystemProperties; import com.android.settings.testutils.shadow.ShadowSecureSettings; import org.junit.After; diff --git a/tests/robotests/src/com/android/settings/location/LocationPreferenceControllerTest.java b/tests/robotests/src/com/android/settings/location/LocationPreferenceControllerTest.java index 87691a584d2..355d6d0a2d1 100644 --- a/tests/robotests/src/com/android/settings/location/LocationPreferenceControllerTest.java +++ b/tests/robotests/src/com/android/settings/location/LocationPreferenceControllerTest.java @@ -16,7 +16,6 @@ package com.android.settings.location; import static com.google.common.truth.Truth.assertThat; - import static org.mockito.ArgumentMatchers.nullable; import static org.mockito.Matchers.any; import static org.mockito.Mockito.verify; @@ -34,11 +33,11 @@ import android.support.v7.preference.Preference; import android.support.v7.preference.PreferenceScreen; import com.android.settings.R; +import com.android.settings.TestConfig; import com.android.settings.search.InlineListPayload; import com.android.settings.search.InlinePayload; import com.android.settings.search.ResultPayload; import com.android.settings.testutils.SettingsRobolectricTestRunner; -import com.android.settings.TestConfig; import com.android.settings.testutils.shadow.ShadowSecureSettings; import com.android.settingslib.core.lifecycle.Lifecycle; diff --git a/tests/robotests/src/com/android/settings/notification/BadgingNotificationPreferenceControllerTest.java b/tests/robotests/src/com/android/settings/notification/BadgingNotificationPreferenceControllerTest.java index c1a7d055463..ac158b6a728 100644 --- a/tests/robotests/src/com/android/settings/notification/BadgingNotificationPreferenceControllerTest.java +++ b/tests/robotests/src/com/android/settings/notification/BadgingNotificationPreferenceControllerTest.java @@ -16,6 +16,14 @@ package com.android.settings.notification; +import static android.provider.Settings.Secure.NOTIFICATION_BADGING; +import static com.google.common.truth.Truth.assertThat; +import static org.mockito.Mockito.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + import android.content.ContentResolver; import android.content.Context; import android.provider.Settings; @@ -38,16 +46,6 @@ import org.robolectric.RobolectricTestRunner; import org.robolectric.annotation.Config; import org.robolectric.shadows.ShadowApplication; -import static android.provider.Settings.Secure.NOTIFICATION_BADGING; - -import static com.google.common.truth.Truth.assertThat; - -import static org.mockito.Mockito.any; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - @RunWith(RobolectricTestRunner.class) @Config(manifest = TestConfig.MANIFEST_PATH, sdk = TestConfig.SDK_VERSION) public class BadgingNotificationPreferenceControllerTest { diff --git a/tests/robotests/src/com/android/settings/search/DatabaseIndexingUtilsTest.java b/tests/robotests/src/com/android/settings/search/DatabaseIndexingUtilsTest.java index 3bfa93665bc..126abb91df8 100644 --- a/tests/robotests/src/com/android/settings/search/DatabaseIndexingUtilsTest.java +++ b/tests/robotests/src/com/android/settings/search/DatabaseIndexingUtilsTest.java @@ -23,10 +23,10 @@ import android.content.Context; import android.util.ArrayMap; import com.android.internal.hardware.AmbientDisplayConfiguration; -import com.android.settings.testutils.SettingsRobolectricTestRunner; import com.android.settings.TestConfig; import com.android.settings.core.PreferenceControllerMixin; import com.android.settings.deviceinfo.SystemUpdatePreferenceController; +import com.android.settings.testutils.SettingsRobolectricTestRunner; import org.junit.Before; import org.junit.Test; diff --git a/tests/robotests/src/com/android/settings/search/IntentSearchViewHolderTest.java b/tests/robotests/src/com/android/settings/search/IntentSearchViewHolderTest.java index a3826f66d6c..4ec080c4207 100644 --- a/tests/robotests/src/com/android/settings/search/IntentSearchViewHolderTest.java +++ b/tests/robotests/src/com/android/settings/search/IntentSearchViewHolderTest.java @@ -221,7 +221,8 @@ public class IntentSearchViewHolderTest { assertThat(mHolder.summaryView.getText()).isEqualTo(SUMMARY); assertThat(mHolder.summaryView.getVisibility()).isEqualTo(View.VISIBLE); verify(mFragment).onSearchResultClicked(eq(mHolder), any(SearchResult.class)); - verify(mFragment).startActivity(result.payload.getIntent()); + verify(mFragment).startActivityForResult(result.payload.getIntent(), + IntentSearchViewHolder.REQUEST_CODE_NO_OP); } @Test @@ -237,7 +238,8 @@ public class IntentSearchViewHolderTest { assertThat(mHolder.summaryView.getText()).isEqualTo(SUMMARY); assertThat(mHolder.summaryView.getVisibility()).isEqualTo(View.VISIBLE); verify(mFragment).onSearchResultClicked(eq(mHolder), any(SearchResult.class)); - verify(mFragment, never()).startActivity(any(Intent.class)); + verify(mFragment, never()).startActivityForResult(result.payload.getIntent(), + IntentSearchViewHolder.REQUEST_CODE_NO_OP); } private SearchResult getSearchResult(String title, String summary, Drawable icon) { diff --git a/tests/robotests/src/com/android/settings/search/SearchFeatureProviderImplTest.java b/tests/robotests/src/com/android/settings/search/SearchFeatureProviderImplTest.java index 30ffaf8b3ec..050d7aa5bbe 100644 --- a/tests/robotests/src/com/android/settings/search/SearchFeatureProviderImplTest.java +++ b/tests/robotests/src/com/android/settings/search/SearchFeatureProviderImplTest.java @@ -17,8 +17,10 @@ package com.android.settings.search; +import static com.google.common.truth.Truth.assertThat; + import android.app.Activity; -import android.view.Menu; +import android.content.ComponentName; import com.android.settings.TestConfig; import com.android.settings.dashboard.SiteMapManager; @@ -27,23 +29,16 @@ import com.android.settings.testutils.SettingsRobolectricTestRunner; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; -import org.mockito.Answers; -import org.mockito.Mock; import org.mockito.MockitoAnnotations; import org.robolectric.Robolectric; import org.robolectric.annotation.Config; -import static com.google.common.truth.Truth.assertThat; - @RunWith(SettingsRobolectricTestRunner.class) @Config(manifest = TestConfig.MANIFEST_PATH, sdk = TestConfig.SDK_VERSION) public class SearchFeatureProviderImplTest { private SearchFeatureProviderImpl mProvider; private Activity mActivity; - @Mock(answer = Answers.RETURNS_DEEP_STUBS) - private Menu menu; - @Before public void setUp() { MockitoAnnotations.initMocks(this); @@ -76,4 +71,20 @@ public class SearchFeatureProviderImplTest { assertThat(loader.mQuery).isEqualTo(query.trim()); } + @Test(expected = IllegalArgumentException.class) + public void verifyLaunchSearchResultPageCaller_nullCaller_shouldCrash() { + mProvider.verifyLaunchSearchResultPageCaller(mActivity, null /* caller */); + } + + @Test(expected = SecurityException.class) + public void everifyLaunchSearchResultPageCaller_badCaller_shouldCrash() { + final ComponentName cn = new ComponentName("pkg", "class"); + mProvider.verifyLaunchSearchResultPageCaller(mActivity, cn); + } + + @Test + public void verifyLaunchSearchResultPageCaller_goodCaller_shouldNotCrash() { + final ComponentName cn = new ComponentName(mActivity.getPackageName(), "class"); + mProvider.verifyLaunchSearchResultPageCaller(mActivity, cn); + } }