From 23412ad94a4f74d93d9daf3271d4c013c6fb8e58 Mon Sep 17 00:00:00 2001 From: Arc Wang Date: Mon, 21 Nov 2022 15:04:24 +0800 Subject: [PATCH 1/5] Settings 2-pane deep link vulnerabilities Settings app must not start an deep link Activity if 1. The deep link Activity is not exported. or 2. Calling package does not have the permission to start the deep link Activity. Bug: 250589026 Test: make RunSettingsRoboTests ROBOTEST_FILTER=SettingsHomepageActivityTest Change-Id: I9a3bddfa5d9d1d2e924dd6f3e5e07dca6c11664f Merged-In: I9a3bddfa5d9d1d2e924dd6f3e5e07dca6c11664f --- .../homepage/SettingsHomepageActivity.java | 35 +++++++++++++++++++ .../SettingsHomepageActivityTest.java | 35 +++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/src/com/android/settings/homepage/SettingsHomepageActivity.java b/src/com/android/settings/homepage/SettingsHomepageActivity.java index 67390847dcb..b79a64c7df3 100644 --- a/src/com/android/settings/homepage/SettingsHomepageActivity.java +++ b/src/com/android/settings/homepage/SettingsHomepageActivity.java @@ -27,6 +27,8 @@ import android.app.ActivityManager; import android.app.settings.SettingsEnums; import android.content.ComponentName; import android.content.Intent; +import android.content.pm.ActivityInfo; +import android.content.pm.PackageManager; import android.content.pm.UserInfo; import android.content.res.Configuration; import android.os.Bundle; @@ -66,6 +68,7 @@ import com.android.settings.core.CategoryMixin; import com.android.settings.core.FeatureFlags; import com.android.settings.homepage.contextualcards.ContextualCardsFragment; import com.android.settings.overlay.FeatureFactory; +import com.android.settings.password.PasswordUtils; import com.android.settings.safetycenter.SafetyCenterManagerWrapper; import com.android.settingslib.Utils; import com.android.settingslib.core.lifecycle.HideNonSystemOverlayMixin; @@ -444,6 +447,32 @@ public class SettingsHomepageActivity extends FragmentActivity implements finish(); return; } + + if (!TextUtils.equals(PasswordUtils.getCallingAppPackageName(getActivityToken()), + getPackageName())) { + ActivityInfo targetActivityInfo = null; + try { + targetActivityInfo = getPackageManager().getActivityInfo(targetComponentName, + /* flags= */ 0); + } catch (PackageManager.NameNotFoundException e) { + Log.e(TAG, "Failed to get target ActivityInfo: " + e); + finish(); + return; + } + + if (!targetActivityInfo.exported) { + Log.e(TAG, "Must not launch an unexported Actvity for deep link"); + finish(); + return; + } + + if (!isCallingAppPermitted(targetActivityInfo.permission)) { + Log.e(TAG, "Calling app must have the permission of deep link Activity"); + finish(); + return; + } + } + targetIntent.setComponent(targetComponentName); // To prevent launchDeepLinkIntentToRight again for configuration change. @@ -485,6 +514,12 @@ public class SettingsHomepageActivity extends FragmentActivity implements } } + @VisibleForTesting + boolean isCallingAppPermitted(String permission) { + return TextUtils.isEmpty(permission) || PasswordUtils.isCallingAppPermitted( + this, getActivityToken(), permission); + } + private String getHighlightMenuKey() { final Intent intent = getIntent(); if (intent != null && TextUtils.equals(intent.getAction(), diff --git a/tests/robotests/src/com/android/settings/homepage/SettingsHomepageActivityTest.java b/tests/robotests/src/com/android/settings/homepage/SettingsHomepageActivityTest.java index 7387407f481..337b659ddd0 100644 --- a/tests/robotests/src/com/android/settings/homepage/SettingsHomepageActivityTest.java +++ b/tests/robotests/src/com/android/settings/homepage/SettingsHomepageActivityTest.java @@ -20,6 +20,8 @@ import static android.view.WindowManager.LayoutParams.SYSTEM_FLAG_HIDE_NON_SYSTE import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; @@ -40,9 +42,11 @@ import androidx.fragment.app.Fragment; import com.android.settings.R; import com.android.settings.dashboard.suggestions.SuggestionFeatureProviderImpl; import com.android.settings.testutils.shadow.ShadowActivityEmbeddingUtils; +import com.android.settings.testutils.shadow.ShadowPasswordUtils; import com.android.settings.testutils.shadow.ShadowUserManager; import com.android.settingslib.core.lifecycle.HideNonSystemOverlayMixin; +import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -69,6 +73,11 @@ public class SettingsHomepageActivityTest { MockitoAnnotations.initMocks(this); } + @After + public void tearDown() { + ShadowPasswordUtils.reset(); + } + @Test public void launch_shouldHaveAnimationForIaFragment() { final SettingsHomepageActivity activity = Robolectric.buildActivity( @@ -208,6 +217,32 @@ public class SettingsHomepageActivityTest { verify(activity).initSplitPairRules(); } + @Test + @Config(shadows = {ShadowPasswordUtils.class}) + public void isCallingAppPermitted_emptyPermission_returnTrue() { + SettingsHomepageActivity homepageActivity = spy(new SettingsHomepageActivity()); + + assertTrue(homepageActivity.isCallingAppPermitted("")); + } + + @Test + @Config(shadows = {ShadowPasswordUtils.class}) + public void isCallingAppPermitted_noGrantedPermission_returnFalse() { + SettingsHomepageActivity homepageActivity = spy(new SettingsHomepageActivity()); + + assertFalse(homepageActivity.isCallingAppPermitted("android.permission.TEST")); + } + + @Test + @Config(shadows = {ShadowPasswordUtils.class}) + public void isCallingAppPermitted_grantedPermission_returnTrue() { + SettingsHomepageActivity homepageActivity = spy(new SettingsHomepageActivity()); + String permission = "android.permission.TEST"; + ShadowPasswordUtils.addGrantedPermission(permission); + + assertTrue(homepageActivity.isCallingAppPermitted(permission)); + } + @Implements(SuggestionFeatureProviderImpl.class) public static class ShadowSuggestionFeatureProviderImpl { From 72a892754d5d7c3771cf484a7ee4b46ad9e12e98 Mon Sep 17 00:00:00 2001 From: Zhenwei Chen Date: Tue, 22 Nov 2022 04:50:15 +0800 Subject: [PATCH 2/5] Fix LoaderCallback.onLoadFinished uncalled issue When two loaders started almost at the same time, it is possible onLoadFinished is never called. Bug: 260687359 Test: Unit tests passed and manual test on device Merged-In: I41a041d5878f9930db44775408380d0d4588faba Change-Id: I41a041d5878f9930db44775408380d0d4588faba Signed-off-by: Zhenwei Chen (cherry picked from commit 41ce87729e0802a00130bff7405960ee95bcdc2d) --- .../batteryusage/PowerUsageAdvanced.java | 3 +- .../batteryusage/PowerUsageBase.java | 48 +++++- .../batteryusage/PowerUsageSummary.java | 10 +- .../batteryusage/PowerUsageBaseTest.java | 75 ++++++++- .../batteryusage/PowerUsageSummaryTest.java | 142 ++++++++++++++++-- 5 files changed, 245 insertions(+), 33 deletions(-) diff --git a/src/com/android/settings/fuelgauge/batteryusage/PowerUsageAdvanced.java b/src/com/android/settings/fuelgauge/batteryusage/PowerUsageAdvanced.java index b88d85ddd1f..db904730a27 100644 --- a/src/com/android/settings/fuelgauge/batteryusage/PowerUsageAdvanced.java +++ b/src/com/android/settings/fuelgauge/batteryusage/PowerUsageAdvanced.java @@ -55,7 +55,6 @@ public class PowerUsageAdvanced extends PowerUsageBase { private static final String KEY_REFRESH_TYPE = "refresh_type"; private static final String KEY_BATTERY_GRAPH = "battery_graph"; private static final String KEY_APP_LIST = "app_list"; - private static final int LOADER_BATTERY_USAGE_STATS = 2; @VisibleForTesting BatteryHistoryPreference mHistPref; @@ -188,7 +187,7 @@ public class PowerUsageAdvanced extends PowerUsageBase { // Uses customized battery history loader if chart design is enabled. if (mIsChartGraphEnabled && !mIsChartDataLoaded) { mIsChartDataLoaded = true; - getLoaderManager().restartLoader(LOADER_BATTERY_USAGE_STATS, bundle, + restartLoader(LoaderIndex.BATTERY_HISTORY_LOADER, bundle, mBatteryHistoryLoaderCallbacks); } else if (!mIsChartGraphEnabled) { super.restartBatteryStatsLoader(refreshType); diff --git a/src/com/android/settings/fuelgauge/batteryusage/PowerUsageBase.java b/src/com/android/settings/fuelgauge/batteryusage/PowerUsageBase.java index ccefdf2c4db..ed3a9215352 100644 --- a/src/com/android/settings/fuelgauge/batteryusage/PowerUsageBase.java +++ b/src/com/android/settings/fuelgauge/batteryusage/PowerUsageBase.java @@ -24,6 +24,7 @@ import android.os.Bundle; import android.os.UserManager; import android.util.Log; +import androidx.annotation.IntDef; import androidx.annotation.NonNull; import androidx.annotation.VisibleForTesting; import androidx.loader.app.LoaderManager; @@ -33,17 +34,19 @@ import com.android.settings.dashboard.DashboardFragment; import com.android.settings.fuelgauge.BatteryBroadcastReceiver; import com.android.settings.fuelgauge.BatteryUtils; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; + /** * Common base class for things that need to show the battery usage graph. */ public abstract class PowerUsageBase extends DashboardFragment { - private static final String TAG = "PowerUsageBase"; - private static final String KEY_REFRESH_TYPE = "refresh_type"; - private static final String KEY_INCLUDE_HISTORY = "include_history"; - - private static final int LOADER_BATTERY_USAGE_STATS = 1; + @VisibleForTesting + static final String KEY_REFRESH_TYPE = "refresh_type"; + @VisibleForTesting + static final String KEY_INCLUDE_HISTORY = "include_history"; @VisibleForTesting BatteryUsageStats mBatteryUsageStats; @@ -55,6 +58,21 @@ public abstract class PowerUsageBase extends DashboardFragment { final BatteryUsageStatsLoaderCallbacks mBatteryUsageStatsLoaderCallbacks = new BatteryUsageStatsLoaderCallbacks(); + @Retention(RetentionPolicy.SOURCE) + @IntDef({ + LoaderIndex.BATTERY_USAGE_STATS_LOADER, + LoaderIndex.BATTERY_INFO_LOADER, + LoaderIndex.BATTERY_TIP_LOADER, + LoaderIndex.BATTERY_HISTORY_LOADER + + }) + public @interface LoaderIndex { + int BATTERY_USAGE_STATS_LOADER = 0; + int BATTERY_INFO_LOADER = 1; + int BATTERY_TIP_LOADER = 2; + int BATTERY_HISTORY_LOADER = 3; + } + @Override public void onAttach(Activity activity) { super.onAttach(activity); @@ -91,10 +109,28 @@ public abstract class PowerUsageBase extends DashboardFragment { final Bundle bundle = new Bundle(); bundle.putInt(KEY_REFRESH_TYPE, refreshType); bundle.putBoolean(KEY_INCLUDE_HISTORY, isBatteryHistoryNeeded()); - getLoaderManager().restartLoader(LOADER_BATTERY_USAGE_STATS, bundle, + restartLoader(LoaderIndex.BATTERY_USAGE_STATS_LOADER, bundle, mBatteryUsageStatsLoaderCallbacks); } + protected LoaderManager getLoaderManagerForCurrentFragment() { + return LoaderManager.getInstance(this); + } + + protected void restartLoader(int loaderId, Bundle bundle, + LoaderManager.LoaderCallbacks loaderCallbacks) { + LoaderManager loaderManager = getLoaderManagerForCurrentFragment(); + Loader loader = loaderManager.getLoader( + loaderId); + if (loader != null && !loader.isReset()) { + loaderManager.restartLoader(loaderId, bundle, + loaderCallbacks); + } else { + loaderManager.initLoader(loaderId, bundle, + loaderCallbacks); + } + } + protected void onLoadFinished(@BatteryUpdateType int refreshType) { refreshUi(refreshType); } diff --git a/src/com/android/settings/fuelgauge/batteryusage/PowerUsageSummary.java b/src/com/android/settings/fuelgauge/batteryusage/PowerUsageSummary.java index bca32a78f8f..f2664927886 100644 --- a/src/com/android/settings/fuelgauge/batteryusage/PowerUsageSummary.java +++ b/src/com/android/settings/fuelgauge/batteryusage/PowerUsageSummary.java @@ -64,11 +64,6 @@ public class PowerUsageSummary extends PowerUsageBase implements @VisibleForTesting static final String KEY_BATTERY_USAGE = "battery_usage_summary"; - @VisibleForTesting - static final int BATTERY_INFO_LOADER = 1; - @VisibleForTesting - static final int BATTERY_TIP_LOADER = 2; - @VisibleForTesting PowerUsageFeatureProvider mPowerFeatureProvider; @VisibleForTesting @@ -241,7 +236,7 @@ public class PowerUsageSummary extends PowerUsageBase implements @VisibleForTesting void restartBatteryTipLoader() { - getLoaderManager().restartLoader(BATTERY_TIP_LOADER, Bundle.EMPTY, mBatteryTipsCallbacks); + restartLoader(LoaderIndex.BATTERY_TIP_LOADER, Bundle.EMPTY, mBatteryTipsCallbacks); } @VisibleForTesting @@ -274,8 +269,7 @@ public class PowerUsageSummary extends PowerUsageBase implements if (!mIsBatteryPresent) { return; } - getLoaderManager().restartLoader(BATTERY_INFO_LOADER, Bundle.EMPTY, - mBatteryInfoLoaderCallbacks); + restartLoader(LoaderIndex.BATTERY_INFO_LOADER, Bundle.EMPTY, mBatteryInfoLoaderCallbacks); } @VisibleForTesting diff --git a/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/PowerUsageBaseTest.java b/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/PowerUsageBaseTest.java index 27009301da9..6ed10cd2b9b 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/PowerUsageBaseTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/PowerUsageBaseTest.java @@ -15,18 +15,26 @@ */ package com.android.settings.fuelgauge.batteryusage; +import static com.android.settings.fuelgauge.batteryusage.PowerUsageBase.KEY_INCLUDE_HISTORY; +import static com.android.settings.fuelgauge.batteryusage.PowerUsageBase.KEY_REFRESH_TYPE; + import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.refEq; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; import android.content.Context; +import android.os.BatteryUsageStats; import android.os.Bundle; import androidx.loader.app.LoaderManager; +import androidx.loader.content.Loader; +import com.android.settings.fuelgauge.BatteryBroadcastReceiver; import com.android.settings.testutils.shadow.ShadowDashboardFragment; import com.android.settingslib.core.AbstractPreferenceController; @@ -46,14 +54,15 @@ public class PowerUsageBaseTest { @Mock private LoaderManager mLoaderManager; + @Mock + private Loader mBatteryUsageStatsLoader; private TestFragment mFragment; @Before public void setUp() { MockitoAnnotations.initMocks(this); - mFragment = spy(new TestFragment()); - doReturn(mLoaderManager).when(mFragment).getLoaderManager(); + mFragment = spy(new TestFragment(mLoaderManager)); } @Test @@ -63,7 +72,62 @@ public class PowerUsageBaseTest { verify(mLoaderManager, never()).initLoader(anyInt(), any(Bundle.class), any()); } - public static class TestFragment extends PowerUsageBase { + @Test + public void restartBatteryInfoLoader() { + final Bundle bundle = new Bundle(); + bundle.putInt(KEY_REFRESH_TYPE, BatteryBroadcastReceiver.BatteryUpdateType.BATTERY_STATUS); + bundle.putBoolean(KEY_INCLUDE_HISTORY, false); + doReturn(mBatteryUsageStatsLoader).when(mLoaderManager).getLoader( + PowerUsageBase.LoaderIndex.BATTERY_USAGE_STATS_LOADER); + doReturn(false).when(mBatteryUsageStatsLoader).isReset(); + + mFragment.restartBatteryStatsLoader( + BatteryBroadcastReceiver.BatteryUpdateType.BATTERY_STATUS); + + verify(mLoaderManager) + .restartLoader(eq(PowerUsageBase.LoaderIndex.BATTERY_USAGE_STATS_LOADER), + refEq(bundle), any()); + } + + @Test + public void restartBatteryInfoLoader_loaderReset_initLoader() { + final Bundle bundle = new Bundle(); + bundle.putInt(KEY_REFRESH_TYPE, BatteryBroadcastReceiver.BatteryUpdateType.BATTERY_STATUS); + bundle.putBoolean(KEY_INCLUDE_HISTORY, false); + doReturn(mBatteryUsageStatsLoader).when(mLoaderManager).getLoader( + PowerUsageBase.LoaderIndex.BATTERY_USAGE_STATS_LOADER); + doReturn(true).when(mBatteryUsageStatsLoader).isReset(); + + mFragment.restartBatteryStatsLoader( + BatteryBroadcastReceiver.BatteryUpdateType.BATTERY_STATUS); + + verify(mLoaderManager) + .initLoader(eq(PowerUsageBase.LoaderIndex.BATTERY_USAGE_STATS_LOADER), + refEq(bundle), any()); + } + + @Test + public void restartBatteryInfoLoader_nullLoader_initLoader() { + final Bundle bundle = new Bundle(); + bundle.putInt(KEY_REFRESH_TYPE, BatteryBroadcastReceiver.BatteryUpdateType.BATTERY_STATUS); + bundle.putBoolean(KEY_INCLUDE_HISTORY, false); + doReturn(null).when(mLoaderManager).getLoader( + PowerUsageBase.LoaderIndex.BATTERY_USAGE_STATS_LOADER); + + mFragment.restartBatteryStatsLoader( + BatteryBroadcastReceiver.BatteryUpdateType.BATTERY_STATUS); + + verify(mLoaderManager).initLoader(eq(PowerUsageBase.LoaderIndex.BATTERY_USAGE_STATS_LOADER), + refEq(bundle), any()); + } + + private static class TestFragment extends PowerUsageBase { + + private LoaderManager mLoaderManager; + + TestFragment(LoaderManager loaderManager) { + mLoaderManager = loaderManager; + } @Override public int getMetricsCategory() { @@ -94,5 +158,10 @@ public class PowerUsageBaseTest { protected List createPreferenceControllers(Context context) { return null; } + + @Override + protected LoaderManager getLoaderManagerForCurrentFragment() { + return mLoaderManager; + } } } diff --git a/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/PowerUsageSummaryTest.java b/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/PowerUsageSummaryTest.java index fa049f8fa35..04bf01937c9 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/PowerUsageSummaryTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/PowerUsageSummaryTest.java @@ -15,7 +15,6 @@ */ package com.android.settings.fuelgauge.batteryusage; -import static com.android.settings.fuelgauge.batteryusage.PowerUsageSummary.BATTERY_INFO_LOADER; import static com.android.settings.fuelgauge.batteryusage.PowerUsageSummary.KEY_BATTERY_ERROR; import static com.android.settings.fuelgauge.batteryusage.PowerUsageSummary.KEY_BATTERY_USAGE; @@ -39,14 +38,17 @@ import android.os.Bundle; import android.provider.Settings; import androidx.loader.app.LoaderManager; +import androidx.loader.content.Loader; import androidx.preference.Preference; import androidx.preference.PreferenceScreen; import com.android.settings.R; import com.android.settings.SettingsActivity; import com.android.settings.fuelgauge.BatteryBroadcastReceiver; +import com.android.settings.fuelgauge.BatteryInfo; import com.android.settings.fuelgauge.BatteryUtils; import com.android.settings.fuelgauge.batterytip.BatteryTipPreferenceController; +import com.android.settings.fuelgauge.batterytip.tips.BatteryTip; import com.android.settings.testutils.FakeFeatureFactory; import com.android.settings.testutils.XmlTestUtils; import com.android.settings.testutils.shadow.ShadowUtils; @@ -69,8 +71,6 @@ import java.util.List; // TODO: Improve this test class so that it starts up the real activity and fragment. @RunWith(RobolectricTestRunner.class) public class PowerUsageSummaryTest { - - private static final long TIME_SINCE_LAST_FULL_CHARGE_MS = 120 * 60 * 1000; private static Intent sAdditionalBatteryInfoIntent; @BeforeClass @@ -83,6 +83,10 @@ public class PowerUsageSummaryTest { @Mock private LoaderManager mLoaderManager; @Mock + private Loader mBatteryTipLoader; + @Mock + private Loader mBatteryInfoLoader; + @Mock private ContentResolver mContentResolver; @Mock private BatteryBroadcastReceiver mBatteryBroadcastReceiver; @@ -105,11 +109,9 @@ public class PowerUsageSummaryTest { mRealContext = spy(RuntimeEnvironment.application); mFeatureFactory = FakeFeatureFactory.setupForTest(); - mFragment = spy(new TestFragment(mRealContext)); + mFragment = spy(new TestFragment(mRealContext, mLoaderManager)); mFragment.initFeatureProvider(); doNothing().when(mFragment).restartBatteryStatsLoader(anyInt()); - doReturn(mock(LoaderManager.class)).when(mFragment).getLoaderManager(); - when(mFragment.getActivity()).thenReturn(mSettingsActivity); when(mFeatureFactory.powerUsageFeatureProvider.getAdditionalBatteryInfoIntent()) .thenReturn(sAdditionalBatteryInfoIntent); @@ -155,12 +157,58 @@ public class PowerUsageSummaryTest { @Test public void restartBatteryTipLoader() { //TODO: add policy logic here when BatteryTipPolicy is implemented - doReturn(mLoaderManager).when(mFragment).getLoaderManager(); + doReturn(mBatteryTipLoader).when(mLoaderManager).getLoader( + PowerUsageBase.LoaderIndex.BATTERY_TIP_LOADER); + doReturn(false).when(mBatteryTipLoader).isReset(); mFragment.restartBatteryTipLoader(); - verify(mLoaderManager) - .restartLoader(eq(PowerUsageSummary.BATTERY_TIP_LOADER), eq(Bundle.EMPTY), any()); + verify(mLoaderManager).restartLoader(eq(PowerUsageBase.LoaderIndex.BATTERY_TIP_LOADER), + eq(Bundle.EMPTY), any()); + } + + @Test + public void restartBatteryTipLoader_nullLoader_initLoader() { + doReturn(null).when(mLoaderManager).getLoader( + PowerUsageBase.LoaderIndex.BATTERY_TIP_LOADER); + + mFragment.restartBatteryTipLoader(); + + verify(mLoaderManager).initLoader(eq(PowerUsageBase.LoaderIndex.BATTERY_TIP_LOADER), + eq(Bundle.EMPTY), any()); + } + + @Test + public void restartBatteryTipLoader_loaderReset_initLoader() { + doReturn(mBatteryTipLoader).when(mLoaderManager).getLoader( + PowerUsageBase.LoaderIndex.BATTERY_TIP_LOADER); + doReturn(true).when(mBatteryTipLoader).isReset(); + + mFragment.restartBatteryTipLoader(); + + + verify(mLoaderManager).initLoader(eq(PowerUsageBase.LoaderIndex.BATTERY_TIP_LOADER), + eq(Bundle.EMPTY), any()); + } + + + @Test + public void refreshUi_contextNull_doNothing() { + doReturn(null).when(mFragment).getContext(); + + mFragment.refreshUi(BatteryBroadcastReceiver.BatteryUpdateType.MANUAL); + + verify(mFragment, never()).restartBatteryTipLoader(); + verify(mFragment, never()).restartBatteryInfoLoader(); + } + + @Test + public void refreshUi_batteryNotPresent_doNothing() { + mFragment.setIsBatteryPresent(false); + mFragment.refreshUi(BatteryBroadcastReceiver.BatteryUpdateType.MANUAL); + + verify(mFragment, never()).restartBatteryTipLoader(); + verify(mFragment, never()).restartBatteryInfoLoader(); } @Test @@ -168,10 +216,12 @@ public class PowerUsageSummaryTest { mFragment.mBatteryTipPreferenceController = mock(BatteryTipPreferenceController.class); when(mFragment.mBatteryTipPreferenceController.needUpdate()).thenReturn(false); mFragment.updateBatteryTipFlag(new Bundle()); + doNothing().when(mFragment).restartBatteryInfoLoader(); mFragment.refreshUi(BatteryBroadcastReceiver.BatteryUpdateType.MANUAL); verify(mFragment, never()).restartBatteryTipLoader(); + verify(mFragment).restartBatteryInfoLoader(); } @Test @@ -179,10 +229,12 @@ public class PowerUsageSummaryTest { mFragment.mBatteryTipPreferenceController = mock(BatteryTipPreferenceController.class); when(mFragment.mBatteryTipPreferenceController.needUpdate()).thenReturn(true); mFragment.updateBatteryTipFlag(new Bundle()); + doNothing().when(mFragment).restartBatteryInfoLoader(); mFragment.refreshUi(BatteryBroadcastReceiver.BatteryUpdateType.BATTERY_LEVEL); verify(mFragment, never()).restartBatteryTipLoader(); + verify(mFragment).restartBatteryInfoLoader(); } @Test @@ -190,10 +242,13 @@ public class PowerUsageSummaryTest { mFragment.mBatteryTipPreferenceController = mock(BatteryTipPreferenceController.class); when(mFragment.mBatteryTipPreferenceController.needUpdate()).thenReturn(true); mFragment.updateBatteryTipFlag(new Bundle()); + doNothing().when(mFragment).restartBatteryInfoLoader(); + doNothing().when(mFragment).restartBatteryTipLoader(); mFragment.refreshUi(BatteryBroadcastReceiver.BatteryUpdateType.MANUAL); verify(mFragment).restartBatteryTipLoader(); + verify(mFragment).restartBatteryInfoLoader(); } @Test @@ -217,19 +272,68 @@ public class PowerUsageSummaryTest { @Test public void restartBatteryInfoLoader_contextNull_doNothing() { when(mFragment.getContext()).thenReturn(null); - when(mFragment.getLoaderManager()).thenReturn(mLoaderManager); mFragment.restartBatteryInfoLoader(); - verify(mLoaderManager, never()).restartLoader(BATTERY_INFO_LOADER, Bundle.EMPTY, + verify(mLoaderManager, never()).restartLoader( + PowerUsageBase.LoaderIndex.BATTERY_INFO_LOADER, Bundle.EMPTY, mFragment.mBatteryInfoLoaderCallbacks); } - public static class TestFragment extends PowerUsageSummary { - private Context mContext; + @Test + public void restartBatteryInfoLoader_batteryIsNotPresent_doNothing() { + mFragment.setIsBatteryPresent(false); - public TestFragment(Context context) { + mFragment.restartBatteryInfoLoader(); + + verify(mLoaderManager, never()).restartLoader( + PowerUsageBase.LoaderIndex.BATTERY_INFO_LOADER, Bundle.EMPTY, + mFragment.mBatteryInfoLoaderCallbacks); + } + + @Test + public void restartBatteryInfoLoader() { + doReturn(mBatteryInfoLoader).when(mLoaderManager).getLoader( + PowerUsageBase.LoaderIndex.BATTERY_INFO_LOADER); + doReturn(false).when(mBatteryTipLoader).isReset(); + + mFragment.restartBatteryInfoLoader(); + + verify(mLoaderManager).restartLoader(eq(PowerUsageBase.LoaderIndex.BATTERY_INFO_LOADER), + eq(Bundle.EMPTY), any()); + } + + @Test + public void restartBatteryInfoLoader_nullLoader_initLoader() { + doReturn(null).when(mLoaderManager).getLoader( + PowerUsageBase.LoaderIndex.BATTERY_INFO_LOADER); + + mFragment.restartBatteryInfoLoader(); + + verify(mLoaderManager).initLoader(eq(PowerUsageBase.LoaderIndex.BATTERY_INFO_LOADER), + eq(Bundle.EMPTY), any()); + } + + @Test + public void restartBatteryInfoLoader_loaderReset_initLoader() { + mFragment.setIsBatteryPresent(true); + doReturn(mBatteryInfoLoader).when(mLoaderManager).getLoader( + PowerUsageBase.LoaderIndex.BATTERY_INFO_LOADER); + doReturn(true).when(mBatteryInfoLoader).isReset(); + + mFragment.restartBatteryInfoLoader(); + + verify(mLoaderManager).initLoader(eq(PowerUsageBase.LoaderIndex.BATTERY_INFO_LOADER), + eq(Bundle.EMPTY), any()); + } + + private static class TestFragment extends PowerUsageSummary { + private Context mContext; + private LoaderManager mLoaderManager; + + TestFragment(Context context, LoaderManager loaderManager) { mContext = context; + mLoaderManager = loaderManager; } @Override @@ -242,5 +346,15 @@ public class PowerUsageSummaryTest { // Override it so we can access this method in test return super.getContentResolver(); } + + public void setIsBatteryPresent(boolean isBatteryPresent) { + mIsBatteryPresent = isBatteryPresent; + } + + @Override + protected LoaderManager getLoaderManagerForCurrentFragment() { + return mLoaderManager; + } } + } From 8eef47d0ccd53ee08d92bdcc92f889846656b0e8 Mon Sep 17 00:00:00 2001 From: Alejandro Nijamkin Date: Tue, 29 Nov 2022 12:07:33 -0800 Subject: [PATCH 3/5] Brings back "Control from locked device" setting. In ag/20427460, we made ControlsTrivialPrivacyPreferenceController, which controls this setting, be UNSUPPORTED_ON_DEVICE if customizable lock screen quick affordances are enabled. This wrongly removes this setting from the Settings app and there is no new UI where the user can control that anymore. What this means is that, once users upgrade to an Android build with our feature, they will forever be stuck with whatever they last chose for "Control from locked device". This CL brings that back but changes the behaviour a bit such that, if the quick affordances feature is enabled, this setting is never disabled. Fix: 260722836 Test: Unit tests added. Manually verified that the setting is visible and enabled if the feature is enabled, even if the current selection does not include the Home quick affordance and that if the feature is off, the setting is visible but disabled if the main setting is off (old behaviour unchanged). Change-Id: I2e53123b3b7a2896699aeaa13b0c7d5a1c8a9c92 --- ...olsTrivialPrivacyPreferenceController.java | 17 ++++--- ...rivialPrivacyPreferenceControllerTest.java | 50 ++++++++++++++++++- 2 files changed, 58 insertions(+), 9 deletions(-) diff --git a/src/com/android/settings/display/ControlsTrivialPrivacyPreferenceController.java b/src/com/android/settings/display/ControlsTrivialPrivacyPreferenceController.java index 7239ac7e640..be2de59b4b5 100644 --- a/src/com/android/settings/display/ControlsTrivialPrivacyPreferenceController.java +++ b/src/com/android/settings/display/ControlsTrivialPrivacyPreferenceController.java @@ -50,9 +50,11 @@ public class ControlsTrivialPrivacyPreferenceController extends TogglePreference @Override public CharSequence getSummary() { - if (getAvailabilityStatus() == DISABLED_DEPENDENT_SETTING) { + if (!CustomizableLockScreenUtils.isFeatureEnabled(mContext) + && getAvailabilityStatus() == DISABLED_DEPENDENT_SETTING) { return mContext.getText(R.string.lockscreen_trivial_disabled_controls_summary); } + return mContext.getText(R.string.lockscreen_trivial_controls_summary); } @@ -70,21 +72,22 @@ public class ControlsTrivialPrivacyPreferenceController extends TogglePreference @Override public int getAvailabilityStatus() { - if (CustomizableLockScreenUtils.isFeatureEnabled(mContext)) { - return UNSUPPORTED_ON_DEVICE; - } - return showDeviceControlsSettingsEnabled() ? AVAILABLE : DISABLED_DEPENDENT_SETTING; } private boolean showDeviceControlsSettingsEnabled() { - return Settings.Secure.getInt(mContext.getContentResolver(), DEPENDENCY_SETTING_KEY, 0) - != 0; + return CustomizableLockScreenUtils.isFeatureEnabled(mContext) + || Settings.Secure.getInt( + mContext.getContentResolver(), DEPENDENCY_SETTING_KEY, 0) != 0; } @Override public void displayPreference(PreferenceScreen screen) { super.displayPreference(screen); + if (CustomizableLockScreenUtils.isFeatureEnabled(mContext)) { + return; + } + Preference currentPreference = screen.findPreference(getPreferenceKey()); currentPreference.setDependency("lockscreen_privacy_controls_switch"); } diff --git a/tests/robotests/src/com/android/settings/display/ControlsTrivialPrivacyPreferenceControllerTest.java b/tests/robotests/src/com/android/settings/display/ControlsTrivialPrivacyPreferenceControllerTest.java index 3d4bc2e1933..8bb3ff69001 100644 --- a/tests/robotests/src/com/android/settings/display/ControlsTrivialPrivacyPreferenceControllerTest.java +++ b/tests/robotests/src/com/android/settings/display/ControlsTrivialPrivacyPreferenceControllerTest.java @@ -21,10 +21,14 @@ import static com.google.common.truth.Truth.assertThat; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import android.content.ContentResolver; import android.content.Context; +import android.database.Cursor; +import android.database.MatrixCursor; import android.provider.Settings; import androidx.preference.Preference; @@ -40,6 +44,7 @@ import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.MockitoAnnotations; +import org.mockito.stubbing.Answer; import org.robolectric.RobolectricTestRunner; @RunWith(RobolectricTestRunner.class) @@ -62,9 +67,11 @@ public class ControlsTrivialPrivacyPreferenceControllerTest { @Before public void setUp() { MockitoAnnotations.initMocks(this); - mContext = ApplicationProvider.getApplicationContext(); + mContext = spy(ApplicationProvider.getApplicationContext()); + mContentResolver = spy(mContext.getContentResolver()); + when(mContext.getContentResolver()).thenReturn(mContentResolver); - mContentResolver = mContext.getContentResolver(); + setCustomizableLockScreenQuickAffordancesEnabled(false); mController = new ControlsTrivialPrivacyPreferenceController(mContext, TEST_KEY); } @@ -130,6 +137,18 @@ public class ControlsTrivialPrivacyPreferenceControllerTest { verify(mPreference, atLeastOnce()).setSummary(mController.getSummary()); } + @Test + public void updateStateWithCustomizableLockScreenQuickAffordancesEnabled() { + setCustomizableLockScreenQuickAffordancesEnabled(true); + Settings.Secure.putInt(mContentResolver, DEPENDENCY_SETTING_KEY, 0); + + mController.updateState(mPreference); + + verify(mPreference).setEnabled(true); + verify(mPreference, atLeastOnce()).setSummary( + mContext.getString(R.string.lockscreen_trivial_controls_summary)); + } + @Test public void getAvailabilityStatusWithoutDeviceControls() { Settings.Secure.putInt(mContentResolver, DEPENDENCY_SETTING_KEY, 0); @@ -138,6 +157,15 @@ public class ControlsTrivialPrivacyPreferenceControllerTest { BasePreferenceController.DISABLED_DEPENDENT_SETTING); } + @Test + public void getAvailabilityStatusWithCustomizableLockScreenQuickAffordancesEnabled() { + setCustomizableLockScreenQuickAffordancesEnabled(true); + Settings.Secure.putInt(mContentResolver, DEPENDENCY_SETTING_KEY, 0); + + assertThat(mController.getAvailabilityStatus()).isEqualTo( + BasePreferenceController.AVAILABLE); + } + @Test public void getAvailabilityStatusWithDeviceControls() { Settings.Secure.putInt(mContentResolver, DEPENDENCY_SETTING_KEY, 1); @@ -154,4 +182,22 @@ public class ControlsTrivialPrivacyPreferenceControllerTest { mController.displayPreference(mPreferenceScreen); verify(mPreference).setDependency(anyString()); } + + private void setCustomizableLockScreenQuickAffordancesEnabled(boolean isEnabled) { + when( + mContentResolver.query( + CustomizableLockScreenUtils.FLAGS_URI, null, null, null)) + .thenAnswer((Answer) invocation -> { + final MatrixCursor cursor = new MatrixCursor( + new String[] { + CustomizableLockScreenUtils.NAME, + CustomizableLockScreenUtils.VALUE + }); + cursor.addRow( + new Object[] { + CustomizableLockScreenUtils.ENABLED_FLAG, isEnabled ? 1 : 0 + }); + return cursor; + }); + } } From a2133c2b001db647361e307f6c6ac2b36d31fb78 Mon Sep 17 00:00:00 2001 From: Joshua Mccloskey Date: Wed, 30 Nov 2022 00:07:06 +0000 Subject: [PATCH 4/5] Revert "Fix the flicker of FingerprintSettings before FingerprintEnrollIntro." This reverts commit 3ca9965a9693894e913d4b97819b27aa0b644e21. Reason for revert: b/259709482 Change-Id: I1fcc8b3ecd42859af591a7b250edfedbbdf204e4 Merged-In: I1fcc8b3ecd42859af591a7b250edfedbbdf204e4 --- .../fingerprint/FingerprintStatusUtils.java | 4 +--- .../fingerprint/FingerprintStatusUtilsTest.java | 14 +------------- 2 files changed, 2 insertions(+), 16 deletions(-) diff --git a/src/com/android/settings/biometrics/fingerprint/FingerprintStatusUtils.java b/src/com/android/settings/biometrics/fingerprint/FingerprintStatusUtils.java index 18db774214c..71cdcf73b84 100644 --- a/src/com/android/settings/biometrics/fingerprint/FingerprintStatusUtils.java +++ b/src/com/android/settings/biometrics/fingerprint/FingerprintStatusUtils.java @@ -78,9 +78,7 @@ public class FingerprintStatusUtils { * Returns the class name of the Settings page corresponding to fingerprint settings. */ public String getSettingsClassName() { - return !hasEnrolled() && isAvailable() - ? FingerprintEnrollIntroductionInternal.class.getName() - : FingerprintSettings.class.getName(); + return FingerprintSettings.class.getName(); } /** diff --git a/tests/unit/src/com/android/settings/biometrics/fingerprint/FingerprintStatusUtilsTest.java b/tests/unit/src/com/android/settings/biometrics/fingerprint/FingerprintStatusUtilsTest.java index a5d74a03500..69e5e2f485b 100644 --- a/tests/unit/src/com/android/settings/biometrics/fingerprint/FingerprintStatusUtilsTest.java +++ b/tests/unit/src/com/android/settings/biometrics/fingerprint/FingerprintStatusUtilsTest.java @@ -179,20 +179,8 @@ public class FingerprintStatusUtilsTest { } @Test - public void getSettingsClassName_whenNotEnrolled_fingerprintOnly_returnsFingerprintEnrollInduction() { + public void getSettingsClassName_whenNotEnrolled_returnsFingerprintSettings() { when(mFingerprintManager.hasEnrolledFingerprints(anyInt())).thenReturn(false); - when(mFingerprintManager.isHardwareDetected()).thenReturn(true); - when(mFaceManager.isHardwareDetected()).thenReturn(false); - - assertThat(mFingerprintStatusUtils.getSettingsClassName()) - .isEqualTo(FingerprintEnrollIntroductionInternal.class.getName()); - } - - @Test - public void getSettingsClassName_whenNotEnrolled_fingerprintNotOnly_returnsFingerprintSettings() { - when(mFingerprintManager.hasEnrolledFingerprints(anyInt())).thenReturn(false); - when(mFingerprintManager.isHardwareDetected()).thenReturn(true); - when(mFaceManager.isHardwareDetected()).thenReturn(true); assertThat(mFingerprintStatusUtils.getSettingsClassName()) .isEqualTo(FingerprintSettings.class.getName()); From 434c8934c4aa416931a66626016d94712e47d617 Mon Sep 17 00:00:00 2001 From: Arc Wang Date: Mon, 21 Nov 2022 15:04:24 +0800 Subject: [PATCH 5/5] Settings 2-pane deep link vulnerabilities Settings app must not start an deep link Activity if 1. The deep link Activity is not exported. or 2. Calling package does not have the permission to start the deep link Activity. Bug: 250589026 Test: make RunSettingsRoboTests ROBOTEST_FILTER=SettingsHomepageActivityTest Change-Id: I9a3bddfa5d9d1d2e924dd6f3e5e07dca6c11664f Merged-In: I9a3bddfa5d9d1d2e924dd6f3e5e07dca6c11664f --- .../homepage/SettingsHomepageActivity.java | 36 +++++++++++++++++++ .../SettingsHomepageActivityTest.java | 35 ++++++++++++++++++ 2 files changed, 71 insertions(+) diff --git a/src/com/android/settings/homepage/SettingsHomepageActivity.java b/src/com/android/settings/homepage/SettingsHomepageActivity.java index 8e14c5a44ea..467f0aa10fe 100644 --- a/src/com/android/settings/homepage/SettingsHomepageActivity.java +++ b/src/com/android/settings/homepage/SettingsHomepageActivity.java @@ -27,6 +27,8 @@ import android.app.ActivityManager; import android.app.settings.SettingsEnums; import android.content.ComponentName; import android.content.Intent; +import android.content.pm.ActivityInfo; +import android.content.pm.PackageManager; import android.content.pm.UserInfo; import android.content.res.Configuration; import android.os.Bundle; @@ -43,6 +45,7 @@ import android.widget.FrameLayout; import android.widget.ImageView; import android.widget.Toolbar; +import androidx.annotation.VisibleForTesting; import androidx.core.graphics.Insets; import androidx.core.view.ViewCompat; import androidx.core.view.WindowCompat; @@ -65,6 +68,7 @@ import com.android.settings.core.CategoryMixin; import com.android.settings.core.FeatureFlags; import com.android.settings.homepage.contextualcards.ContextualCardsFragment; import com.android.settings.overlay.FeatureFactory; +import com.android.settings.password.PasswordUtils; import com.android.settingslib.Utils; import com.android.settingslib.core.lifecycle.HideNonSystemOverlayMixin; @@ -431,6 +435,32 @@ public class SettingsHomepageActivity extends FragmentActivity implements finish(); return; } + + if (!TextUtils.equals(PasswordUtils.getCallingAppPackageName(getActivityToken()), + getPackageName())) { + ActivityInfo targetActivityInfo = null; + try { + targetActivityInfo = getPackageManager().getActivityInfo(targetComponentName, + /* flags= */ 0); + } catch (PackageManager.NameNotFoundException e) { + Log.e(TAG, "Failed to get target ActivityInfo: " + e); + finish(); + return; + } + + if (!targetActivityInfo.exported) { + Log.e(TAG, "Must not launch an unexported Actvity for deep link"); + finish(); + return; + } + + if (!isCallingAppPermitted(targetActivityInfo.permission)) { + Log.e(TAG, "Calling app must have the permission of deep link Activity"); + finish(); + return; + } + } + targetIntent.setComponent(targetComponentName); // To prevent launchDeepLinkIntentToRight again for configuration change. @@ -472,6 +502,12 @@ public class SettingsHomepageActivity extends FragmentActivity implements } } + @VisibleForTesting + boolean isCallingAppPermitted(String permission) { + return TextUtils.isEmpty(permission) || PasswordUtils.isCallingAppPermitted( + this, getActivityToken(), permission); + } + private String getHighlightMenuKey() { final Intent intent = getIntent(); if (intent != null && TextUtils.equals(intent.getAction(), diff --git a/tests/robotests/src/com/android/settings/homepage/SettingsHomepageActivityTest.java b/tests/robotests/src/com/android/settings/homepage/SettingsHomepageActivityTest.java index 4d203a8a6b0..4de8b005c3c 100644 --- a/tests/robotests/src/com/android/settings/homepage/SettingsHomepageActivityTest.java +++ b/tests/robotests/src/com/android/settings/homepage/SettingsHomepageActivityTest.java @@ -20,6 +20,8 @@ import static android.view.WindowManager.LayoutParams.SYSTEM_FLAG_HIDE_NON_SYSTE import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; @@ -37,9 +39,11 @@ import androidx.fragment.app.Fragment; import com.android.settings.R; import com.android.settings.dashboard.suggestions.SuggestionFeatureProviderImpl; import com.android.settings.homepage.contextualcards.slices.BatteryFixSliceTest; +import com.android.settings.testutils.shadow.ShadowPasswordUtils; import com.android.settings.testutils.shadow.ShadowUserManager; import com.android.settingslib.core.lifecycle.HideNonSystemOverlayMixin; +import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -66,6 +70,11 @@ public class SettingsHomepageActivityTest { MockitoAnnotations.initMocks(this); } + @After + public void tearDown() { + ShadowPasswordUtils.reset(); + } + @Test public void launch_shouldHaveAnimationForIaFragment() { final SettingsHomepageActivity activity = Robolectric.buildActivity( @@ -195,6 +204,32 @@ public class SettingsHomepageActivityTest { & SYSTEM_FLAG_HIDE_NON_SYSTEM_OVERLAY_WINDOWS).isEqualTo(0); } + @Test + @Config(shadows = {ShadowPasswordUtils.class}) + public void isCallingAppPermitted_emptyPermission_returnTrue() { + SettingsHomepageActivity homepageActivity = spy(new SettingsHomepageActivity()); + + assertTrue(homepageActivity.isCallingAppPermitted("")); + } + + @Test + @Config(shadows = {ShadowPasswordUtils.class}) + public void isCallingAppPermitted_noGrantedPermission_returnFalse() { + SettingsHomepageActivity homepageActivity = spy(new SettingsHomepageActivity()); + + assertFalse(homepageActivity.isCallingAppPermitted("android.permission.TEST")); + } + + @Test + @Config(shadows = {ShadowPasswordUtils.class}) + public void isCallingAppPermitted_grantedPermission_returnTrue() { + SettingsHomepageActivity homepageActivity = spy(new SettingsHomepageActivity()); + String permission = "android.permission.TEST"; + ShadowPasswordUtils.addGrantedPermission(permission); + + assertTrue(homepageActivity.isCallingAppPermitted(permission)); + } + @Implements(SuggestionFeatureProviderImpl.class) public static class ShadowSuggestionFeatureProviderImpl {