From 28791f5ff06e0f0cfcf4bb5b32d29fae47bf4dcb Mon Sep 17 00:00:00 2001 From: Weng Su Date: Thu, 15 Dec 2022 15:53:04 +0800 Subject: [PATCH] Safely remove Wi-Fi user credentials - Show "(In use)" if a credential is used by Wi-Fi networks - Show Wi-Fi networks in use in the details dialog - Disable "Uninstall" button if the credential is in use Bug: 258542666 Test: manual test atest -c UserCredentialsSettingsTest Change-Id: I1fb29b58698d918f987b9a16c195127cf270c17e --- res/layout/user_credential.xml | 16 ++ res/values/strings.xml | 14 +- .../settings/UserCredentialsSettings.java | 109 ++++++++-- .../settings/UserCredentialsSettingsTest.java | 195 ++++++++++++++++++ 4 files changed, 313 insertions(+), 21 deletions(-) create mode 100644 tests/unit/src/com/android/settings/UserCredentialsSettingsTest.java diff --git a/res/layout/user_credential.xml b/res/layout/user_credential.xml index f441bdac825..fa7abb3d7d5 100644 --- a/res/layout/user_credential.xml +++ b/res/layout/user_credential.xml @@ -39,6 +39,22 @@ android:orientation="vertical" android:paddingTop="10dp"> + + + + Credentials are not available for this user Installed for VPN and apps + + Installed for Wi\u2011Fi - Installed for Wi-Fi + Installed for Wi\u2011Fi (In use) Remove all the contents? @@ -5807,14 +5809,16 @@ Permanently remove the user CA certificate? + + Being used by - This entry contains: + This entry contains - one user key + 1 user key - one user certificate + 1 user certificate - one CA certificate + 1 CA certificate %d CA certificates diff --git a/src/com/android/settings/UserCredentialsSettings.java b/src/com/android/settings/UserCredentialsSettings.java index c45b5efb860..73f1d9e6c77 100644 --- a/src/com/android/settings/UserCredentialsSettings.java +++ b/src/com/android/settings/UserCredentialsSettings.java @@ -43,12 +43,14 @@ import android.view.View; import android.view.ViewGroup; import android.widget.TextView; +import androidx.annotation.VisibleForTesting; import androidx.appcompat.app.AlertDialog; import androidx.fragment.app.DialogFragment; import androidx.fragment.app.Fragment; import androidx.recyclerview.widget.RecyclerView; import com.android.settings.core.instrumentation.InstrumentedDialogFragment; +import com.android.settings.wifi.helper.SavedWifiHelper; import com.android.settingslib.RestrictedLockUtils; import com.android.settingslib.RestrictedLockUtils.EnforcedAdmin; import com.android.settingslib.RestrictedLockUtilsInternal; @@ -74,6 +76,9 @@ public class UserCredentialsSettings extends SettingsPreferenceFragment private static final String KEYSTORE_PROVIDER = "AndroidKeyStore"; + @VisibleForTesting + protected SavedWifiHelper mSavedWifiHelper; + @Override public int getMetricsCategory() { return SettingsEnums.USER_CREDENTIALS; @@ -88,15 +93,23 @@ public class UserCredentialsSettings extends SettingsPreferenceFragment @Override public void onClick(final View view) { final Credential item = (Credential) view.getTag(); - if (item != null) { - CredentialDialogFragment.show(this, item); + if (item == null) return; + if (item.isInUse()) { + item.setUsedByNames(mSavedWifiHelper.getCertificateNetworkNames(item.alias)); } + showCredentialDialogFragment(item); } @Override public void onCreate(@Nullable Bundle savedInstanceState) { super.onCreate(savedInstanceState); getActivity().setTitle(R.string.user_credentials); + mSavedWifiHelper = SavedWifiHelper.getInstance(getContext(), getSettingsLifecycle()); + } + + @VisibleForTesting + protected void showCredentialDialogFragment(Credential item) { + CredentialDialogFragment.show(this, item); } protected void announceRemoval(String alias) { @@ -112,7 +125,9 @@ public class UserCredentialsSettings extends SettingsPreferenceFragment } } - public static class CredentialDialogFragment extends InstrumentedDialogFragment { + /** The fragment to show the credential information. */ + public static class CredentialDialogFragment extends InstrumentedDialogFragment + implements DialogInterface.OnShowListener { private static final String TAG = "CredentialDialogFragment"; private static final String ARG_CREDENTIAL = "credential"; @@ -162,17 +177,23 @@ public class UserCredentialsSettings extends SettingsPreferenceFragment dialog.dismiss(); } }; - // TODO: b/127865361 - // a safe means of clearing wifi certificates. Configs refer to aliases - // directly so deleting certs will break dependent access points. - // However, Wi-Fi used to remove this certificate from storage if the network - // was removed, regardless if it is used in more than one network. - // It has been decided to allow removing certificates from this menu, as we - // assume that the user who manually adds certificates must have a way to - // manually remove them. builder.setNegativeButton(R.string.trusted_credentials_remove_label, listener); } - return builder.create(); + AlertDialog dialog = builder.create(); + dialog.setOnShowListener(this); + return dialog; + } + + /** + * Override for the negative button enablement on demand. + */ + @Override + public void onShow(DialogInterface dialogInterface) { + final Credential item = (Credential) getArguments().getParcelable(ARG_CREDENTIAL); + if (item.isInUse()) { + ((AlertDialog) getDialog()).getButton(AlertDialog.BUTTON_NEGATIVE) + .setEnabled(false); + } } @Override @@ -300,6 +321,9 @@ public class UserCredentialsSettings extends SettingsPreferenceFragment while (aliases.hasMoreElements()) { String alias = aliases.nextElement(); Credential c = new Credential(alias, uid); + if (!c.isSystem()) { + c.setInUse(mSavedWifiHelper.isCertificateInUse(alias)); + } Key key = null; try { key = keyStore.getKey(alias, null); @@ -423,12 +447,13 @@ public class UserCredentialsSettings extends SettingsPreferenceFragment } ((TextView) view.findViewById(R.id.alias)).setText(item.alias); - ((TextView) view.findViewById(R.id.purpose)).setText(item.isSystem() - ? R.string.credential_for_vpn_and_apps - : R.string.credential_for_wifi); + updatePurposeView(view.findViewById(R.id.purpose), item); view.findViewById(R.id.contents).setVisibility(expanded ? View.VISIBLE : View.GONE); if (expanded) { + updateUsedByViews(view.findViewById(R.id.credential_being_used_by_title), + view.findViewById(R.id.credential_being_used_by_content), item); + for (int i = 0; i < credentialViewTypes.size(); i++) { final View detail = view.findViewById(credentialViewTypes.keyAt(i)); detail.setVisibility(item.storedTypes.contains(credentialViewTypes.valueAt(i)) @@ -438,6 +463,30 @@ public class UserCredentialsSettings extends SettingsPreferenceFragment return view; } + @VisibleForTesting + protected static void updatePurposeView(TextView purpose, Credential item) { + int subTextResId = R.string.credential_for_vpn_and_apps; + if (!item.isSystem()) { + subTextResId = (item.isInUse()) + ? R.string.credential_for_wifi_in_use + : R.string.credential_for_wifi; + } + purpose.setText(subTextResId); + } + + @VisibleForTesting + protected static void updateUsedByViews(TextView title, TextView content, Credential item) { + List usedByNames = item.getUsedByNames(); + if (usedByNames.size() > 0) { + title.setVisibility(View.VISIBLE); + content.setText(String.join("\n", usedByNames)); + content.setVisibility(View.VISIBLE); + } else { + title.setVisibility(View.GONE); + content.setVisibility(View.GONE); + } + } + static class AliasEntry { public String alias; public int uid; @@ -468,6 +517,16 @@ public class UserCredentialsSettings extends SettingsPreferenceFragment */ final int uid; + /** + * Indicate whether or not this credential is in use. + */ + boolean mIsInUse; + + /** + * The list of networks which use this credential. + */ + List mUsedByNames = new ArrayList<>(); + /** * Should contain some non-empty subset of: *
    @@ -524,10 +583,28 @@ public class UserCredentialsSettings extends SettingsPreferenceFragment return UserHandle.getAppId(uid) == Process.SYSTEM_UID; } - public String getAlias() { return alias; } + public String getAlias() { + return alias; + } public EnumSet getStoredTypes() { return storedTypes; } + + public void setInUse(boolean inUse) { + mIsInUse = inUse; + } + + public boolean isInUse() { + return mIsInUse; + } + + public void setUsedByNames(List names) { + mUsedByNames = new ArrayList<>(names); + } + + public List getUsedByNames() { + return new ArrayList(mUsedByNames); + } } } diff --git a/tests/unit/src/com/android/settings/UserCredentialsSettingsTest.java b/tests/unit/src/com/android/settings/UserCredentialsSettingsTest.java new file mode 100644 index 00000000000..c4cdf8015b0 --- /dev/null +++ b/tests/unit/src/com/android/settings/UserCredentialsSettingsTest.java @@ -0,0 +1,195 @@ +/* + * Copyright (C) 2022 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; + +import static com.google.common.truth.Truth.assertThat; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import android.content.Context; +import android.os.Looper; +import android.os.Process; +import android.view.View; +import android.widget.TextView; + +import androidx.test.annotation.UiThreadTest; +import androidx.test.core.app.ApplicationProvider; +import androidx.test.ext.junit.runners.AndroidJUnit4; + +import com.android.settings.testutils.ResourcesUtils; +import com.android.settings.wifi.helper.SavedWifiHelper; + +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.Spy; +import org.mockito.junit.MockitoJUnit; +import org.mockito.junit.MockitoRule; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +@RunWith(AndroidJUnit4.class) +public class UserCredentialsSettingsTest { + static final String TEST_ALIAS = "test_alias"; + static final String TEST_USER_BY_NAME = "test_used_by_name"; + + static final String TEXT_PURPOSE_SYSTEM = "credential_for_vpn_and_apps"; + static final String TEXT_PURPOSE_WIFI = "credential_for_wifi"; + static final String TEXT_PURPOSE_WIFI_IN_USE = "credential_for_wifi_in_use"; + + @Rule + public final MockitoRule mMockitoRule = MockitoJUnit.rule(); + @Spy + final Context mContext = ApplicationProvider.getApplicationContext(); + @Mock + SavedWifiHelper mSavedWifiHelper; + @Mock + View mView; + + UserCredentialsSettings mSettings; + UserCredentialsSettings.Credential mSysCredential = + new UserCredentialsSettings.Credential(TEST_ALIAS, Process.SYSTEM_UID); + UserCredentialsSettings.Credential mWifiCredential = + new UserCredentialsSettings.Credential(TEST_ALIAS, Process.WIFI_UID); + List mUsedByNames = Arrays.asList(TEST_USER_BY_NAME); + TextView mPurposeView = new TextView(ApplicationProvider.getApplicationContext()); + TextView mUsedByTitleView = new TextView(ApplicationProvider.getApplicationContext()); + TextView mUsedByContentView = new TextView(ApplicationProvider.getApplicationContext()); + + @Before + @UiThreadTest + public void setUp() { + when(mSavedWifiHelper.isCertificateInUse(any(String.class))).thenReturn(false); + when(mSavedWifiHelper.getCertificateNetworkNames(any(String.class))) + .thenReturn(new ArrayList<>()); + when(mView.getTag()).thenReturn(mWifiCredential); + + if (Looper.myLooper() == null) { + Looper.prepare(); + } + mSettings = spy(new UserCredentialsSettings()); + when(mSettings.getContext()).thenReturn(mContext); + mSettings.mSavedWifiHelper = mSavedWifiHelper; + doNothing().when(mSettings) + .showCredentialDialogFragment(any(UserCredentialsSettings.Credential.class)); + } + + @Test + @UiThreadTest + public void onClick_noCredentialInTag_doNothing() { + when(mView.getTag()).thenReturn(null); + + mSettings.onClick(mView); + + verify(mSavedWifiHelper, never()).getCertificateNetworkNames(any(String.class)); + verify(mSettings, never()) + .showCredentialDialogFragment(any(UserCredentialsSettings.Credential.class)); + } + + @Test + @UiThreadTest + public void onClick_credentialInNotUse_notSetUsedByNamesThenShowDialog() { + mWifiCredential.setInUse(false); + when(mView.getTag()).thenReturn(mWifiCredential); + + mSettings.onClick(mView); + + verify(mSavedWifiHelper, never()).getCertificateNetworkNames(any(String.class)); + verify(mSettings) + .showCredentialDialogFragment(any(UserCredentialsSettings.Credential.class)); + } + + @Test + @UiThreadTest + public void onClick_credentialInUse_setUsedByNamesThenShowDialog() { + mWifiCredential.setInUse(true); + when(mView.getTag()).thenReturn(mWifiCredential); + when(mSavedWifiHelper.getCertificateNetworkNames(any(String.class))) + .thenReturn(mUsedByNames); + + mSettings.onClick(mView); + + verify(mSavedWifiHelper).getCertificateNetworkNames(any(String.class)); + assertThat(mWifiCredential.getUsedByNames()).isEqualTo(mUsedByNames); + verify(mSettings) + .showCredentialDialogFragment(any(UserCredentialsSettings.Credential.class)); + } + + @Test + @UiThreadTest + public void updatePurposeView_getSystemCert_setTextCorrectly() { + mSettings.updatePurposeView(mPurposeView, mSysCredential); + + assertThat(mPurposeView.getText()).isEqualTo(getResString(TEXT_PURPOSE_SYSTEM)); + } + + @Test + @UiThreadTest + public void updatePurposeView_getWifiCert_setTextCorrectly() { + mWifiCredential.setInUse(false); + + mSettings.updatePurposeView(mPurposeView, mWifiCredential); + + assertThat(mPurposeView.getText()).isEqualTo(getResString(TEXT_PURPOSE_WIFI)); + } + + @Test + @UiThreadTest + public void updatePurposeView_isWifiCertInUse_setTextCorrectly() { + mWifiCredential.setInUse(true); + + mSettings.updatePurposeView(mPurposeView, mWifiCredential); + + assertThat(mPurposeView.getText()).isEqualTo(getResString(TEXT_PURPOSE_WIFI_IN_USE)); + } + + @Test + @UiThreadTest + public void updateUsedByViews_noUsedByName_hideViews() { + mWifiCredential.setUsedByNames(new ArrayList<>()); + + mSettings.updateUsedByViews(mUsedByTitleView, mUsedByContentView, mWifiCredential); + + assertThat(mUsedByTitleView.getVisibility()).isEqualTo(View.GONE); + assertThat(mUsedByContentView.getVisibility()).isEqualTo(View.GONE); + } + + @Test + @UiThreadTest + public void updateUsedByViews_hasUsedByName_showViews() { + mWifiCredential.setUsedByNames(mUsedByNames); + + mSettings.updateUsedByViews(mUsedByTitleView, mUsedByContentView, mWifiCredential); + + assertThat(mUsedByTitleView.getVisibility()).isEqualTo(View.VISIBLE); + assertThat(mUsedByContentView.getVisibility()).isEqualTo(View.VISIBLE); + assertThat(mUsedByContentView.getText().toString().contains(TEST_USER_BY_NAME)).isTrue(); + } + + static String getResString(String name) { + return ResourcesUtils.getResourcesString(ApplicationProvider.getApplicationContext(), name); + } +}