From 16541e4f88bf645408de098d1368cb49dd09e4c3 Mon Sep 17 00:00:00 2001 From: Daniel Sandler Date: Fri, 13 Nov 2009 17:07:50 -0800 Subject: [PATCH] Fix a race condition determining whether password fallback mode is allowed. The fix is in LockPatternKeyguardView, whose constructor was firing off an asynchronous request to the AccountManager to find out about the specifics of the account on the device. (If it's SAML, we don't have the password in cleartext and therefore can't use it to unlock.) Unfortunately, if the AccountManager responds too quickly, we get the answer (in LPKV.run()) before the UnlockScreen has even been instantiated (later in LPKV's ctor). The fix is to create the unlock screen first and *then* ping the AccountManager for details. Bug: http://b/2216308 Change-Id: Iedc84675c0ab8a001d062d806e2bee7ed1a29758 --- .../policy/impl/KeyguardViewManager.java | 2 +- .../policy/impl/LockPatternKeyguardView.java | 42 +++++++++++++------ .../internal/policy/impl/UnlockScreen.java | 9 ++++ 3 files changed, 39 insertions(+), 14 deletions(-) diff --git a/policy/com/android/internal/policy/impl/KeyguardViewManager.java b/policy/com/android/internal/policy/impl/KeyguardViewManager.java index bac2fcada11f9..d4dc429d08b07 100644 --- a/policy/com/android/internal/policy/impl/KeyguardViewManager.java +++ b/policy/com/android/internal/policy/impl/KeyguardViewManager.java @@ -92,7 +92,7 @@ public class KeyguardViewManager implements KeyguardWindowController { * lazily. */ public synchronized void show() { - if (DEBUG) Log.d(TAG, "show()"); + if (DEBUG) Log.d(TAG, "show(); mKeyguardView==" + mKeyguardView); if (mKeyguardHost == null) { if (DEBUG) Log.d(TAG, "keyguard host is null, creating it..."); diff --git a/policy/com/android/internal/policy/impl/LockPatternKeyguardView.java b/policy/com/android/internal/policy/impl/LockPatternKeyguardView.java index 0ebd945600efa..00dc92969b0d2 100644 --- a/policy/com/android/internal/policy/impl/LockPatternKeyguardView.java +++ b/policy/com/android/internal/policy/impl/LockPatternKeyguardView.java @@ -160,7 +160,10 @@ public class LockPatternKeyguardView extends KeyguardViewBase } catch (AuthenticatorException e) { } mEnableFallback = !hasSAMLAccount; - if (mUnlockScreen instanceof UnlockScreen) { + + if (mUnlockScreen == null) { + Log.w(TAG, "no unlock screen when receiving AccountManager information"); + } else if (mUnlockScreen instanceof UnlockScreen) { ((UnlockScreen)mUnlockScreen).setEnableFallback(true); } } @@ -179,18 +182,6 @@ public class LockPatternKeyguardView extends KeyguardViewBase KeyguardWindowController controller) { super(context); - final boolean hasAccount = AccountManager.get(context).getAccounts().length > 0; - if (hasAccount) { - /* If we have a SAML account which requires web login we can not use the - fallback screen UI to ask the user for credentials. - For now we will disable fallback screen in this case. - Ultimately we could consider bringing up a web login from GLS - but need to make sure that it will work in the "locked screen" mode. */ - String[] features = new String[] {"saml"}; - AccountManager.get(context).getAccountsByTypeAndFeatures( - "com.google", features, this, null); - } - mEnableFallback = false; mRequiresSim = @@ -275,6 +266,9 @@ public class LockPatternKeyguardView extends KeyguardViewBase public void reportFailedPatternAttempt() { mUpdateMonitor.reportFailedAttempt(); final int failedAttempts = mUpdateMonitor.getFailedAttempts(); + if (DEBUG) Log.d(TAG, + "reportFailedPatternAttempt: #" + failedAttempts + + " (enableFallback=" + mEnableFallback + ")"); if (mEnableFallback && failedAttempts == (LockPatternUtils.FAILED_ATTEMPTS_BEFORE_RESET - LockPatternUtils.FAILED_ATTEMPTS_BEFORE_TIMEOUT)) { @@ -313,8 +307,28 @@ public class LockPatternKeyguardView extends KeyguardViewBase mLockScreen = createLockScreen(); addView(mLockScreen); final UnlockMode unlockMode = getUnlockMode(); + if (DEBUG) Log.d(TAG, + "LockPatternKeyguardView ctor: about to createUnlockScreenFor; mEnableFallback=" + + mEnableFallback); mUnlockScreen = createUnlockScreenFor(unlockMode); mUnlockScreenMode = unlockMode; + + // Ask the account manager if we have an account that can be used as a + // fallback in case the user forgets his pattern. The response comes + // back in run() below; don't bother asking until you've called + // createUnlockScreenFor(), else the information will go unused. + final boolean hasAccount = AccountManager.get(context).getAccounts().length > 0; + if (hasAccount) { + /* If we have a SAML account which requires web login we can not use the + fallback screen UI to ask the user for credentials. + For now we will disable fallback screen in this case. + Ultimately we could consider bringing up a web login from GLS + but need to make sure that it will work in the "locked screen" mode. */ + String[] features = new String[] {"saml"}; + AccountManager.get(context).getAccountsByTypeAndFeatures( + "com.google", features, this, null); + } + addView(mUnlockScreen); updateScreen(mMode); } @@ -475,6 +489,8 @@ public class LockPatternKeyguardView extends KeyguardViewBase mUpdateMonitor, mKeyguardScreenCallback, mUpdateMonitor.getFailedAttempts()); + if (DEBUG) Log.d(TAG, + "createUnlockScreenFor(" + unlockMode + "): mEnableFallback=" + mEnableFallback); view.setEnableFallback(mEnableFallback); return view; } else if (unlockMode == UnlockMode.SimPin) { diff --git a/policy/com/android/internal/policy/impl/UnlockScreen.java b/policy/com/android/internal/policy/impl/UnlockScreen.java index f85b62f341f49..e413d6bed6079 100644 --- a/policy/com/android/internal/policy/impl/UnlockScreen.java +++ b/policy/com/android/internal/policy/impl/UnlockScreen.java @@ -27,6 +27,7 @@ import android.widget.Button; import android.widget.TextView; import android.text.format.DateFormat; import android.text.TextUtils; +import android.util.Log; import com.android.internal.R; import com.android.internal.telephony.IccCard; import com.android.internal.widget.LinearLayoutWithDefaultTouchRecepient; @@ -45,6 +46,7 @@ class UnlockScreen extends LinearLayoutWithDefaultTouchRecepient implements KeyguardScreen, KeyguardUpdateMonitor.ConfigurationChangeCallback, KeyguardUpdateMonitor.InfoCallback, KeyguardUpdateMonitor.SimStateCallback { + private static final boolean DEBUG = false; private static final String TAG = "UnlockScreen"; // how long before we clear the wrong pattern @@ -162,6 +164,12 @@ class UnlockScreen extends LinearLayoutWithDefaultTouchRecepient mTotalFailedPatternAttempts = totalFailedAttempts; mFailedPatternAttemptsSinceLastTimeout = totalFailedAttempts % LockPatternUtils.FAILED_ATTEMPTS_BEFORE_TIMEOUT; + if (DEBUG) Log.d(TAG, + "UnlockScreen() ctor: totalFailedAttempts=" + + totalFailedAttempts + ", mFailedPat...=" + + mFailedPatternAttemptsSinceLastTimeout + ); + if (mUpdateMonitor.isInPortrait()) { LayoutInflater.from(context).inflate(R.layout.keyguard_screen_unlock_portrait, this, true); } else { @@ -239,6 +247,7 @@ class UnlockScreen extends LinearLayoutWithDefaultTouchRecepient } public void setEnableFallback(boolean state) { + if (DEBUG) Log.d(TAG, "setEnableFallback(" + state + ")"); mEnableFallback = state; }