From 45bfac00e58059eed71fd22e2767c8e0437b6251 Mon Sep 17 00:00:00 2001 From: Andy Stadler Date: Mon, 24 Jan 2011 11:08:31 -0800 Subject: [PATCH] Rework account security bootstrap procedure * Bug to fix was that it was looping if the user canceled password or encryption setup (instead of simply re-notifying.) * Solution was to refactor all code into a single sequence of checks, rerun the sequence each time, and set markers to prevent looping. * Rework needed after the changes in I54f39bc9 which added encryption support but introduced the looping behavior. * Removed a UI-thread DB access Bug: 3346641 Change-Id: I0791d7a16287efecc7121b5ffa0db26ca2b05151 --- .../email/activity/setup/AccountSecurity.java | 217 ++++++++++-------- 1 file changed, 127 insertions(+), 90 deletions(-) diff --git a/src/com/android/email/activity/setup/AccountSecurity.java b/src/com/android/email/activity/setup/AccountSecurity.java index a5c4fe3f5..d4414fa3a 100644 --- a/src/com/android/email/activity/setup/AccountSecurity.java +++ b/src/com/android/email/activity/setup/AccountSecurity.java @@ -18,6 +18,7 @@ package com.android.email.activity.setup; import com.android.email.R; import com.android.email.SecurityPolicy; +import com.android.email.Utility; import com.android.email.activity.ActivityHelper; import com.android.email.provider.EmailContent.Account; import com.android.email.provider.EmailContent.HostAuth; @@ -26,6 +27,7 @@ import android.app.Activity; import android.app.admin.DevicePolicyManager; import android.content.Context; import android.content.Intent; +import android.os.AsyncTask; import android.os.Bundle; /** @@ -47,6 +49,11 @@ public class AccountSecurity extends Activity { private static final int REQUEST_PASSWORD = 2; private static final int REQUEST_ENCRYPTION = 3; + private boolean mTriedAddAdministrator = false; + private boolean mTriedSetPassword = false; + private boolean mTriedSetEncryption = false; + private Account mAccount; + /** * Used for generating intent for this activity (which is intended to be launched * from a notification.) @@ -67,114 +74,144 @@ public class AccountSecurity extends Activity { ActivityHelper.debugSetWindowFlags(this); Intent i = getIntent(); - long accountId = i.getLongExtra(EXTRA_ACCOUNT_ID, -1); + final long accountId = i.getLongExtra(EXTRA_ACCOUNT_ID, -1); SecurityPolicy security = SecurityPolicy.getInstance(this); security.clearNotification(accountId); - if (accountId != -1) { - // TODO: spin up a thread to do this in the background, because of DB ops - Account account = Account.restoreAccountWithId(this, accountId); - if (account != null) { - if (account.mSecurityFlags != 0) { - // This account wants to control security - if (!security.isActiveAdmin()) { - // retrieve name of server for the format string - HostAuth hostAuth = - HostAuth.restoreHostAuthWithId(this, account.mHostAuthKeyRecv); - if (hostAuth != null) { - // try to become active - must happen here in activity, to get result - Intent intent = new Intent(DevicePolicyManager.ACTION_ADD_DEVICE_ADMIN); - intent.putExtra(DevicePolicyManager.EXTRA_DEVICE_ADMIN, - security.getAdminComponent()); - intent.putExtra(DevicePolicyManager.EXTRA_ADD_EXPLANATION, - this.getString(R.string.account_security_policy_explanation_fmt, - hostAuth.mAddress)); - startActivityForResult(intent, REQUEST_ENABLE); - // keep this activity on stack to process result - return; - } - } else { - // already active - try to set actual policies, finish, and return - boolean startedActivity = setActivePolicies(); - if (startedActivity) { - // keep this activity on stack to process result - return; - } - } - } - } + if (accountId == -1) { + finish(); + return; } - finish(); + + // Let onCreate exit, while background thread retrieves account. + // Then start the security check/bootstrap process. + new AsyncTask() { + @Override + protected Account doInBackground(Void... params) { + return Account.restoreAccountWithId(AccountSecurity.this, accountId); + } + + @Override + protected void onPostExecute(Account result) { + mAccount = result; + if (mAccount != null && mAccount.mSecurityFlags != 0) { + // This account wants to control security + tryAdvanceSecurity(mAccount); + return; + } + finish(); + } + }.execute(); } /** - * Handle the eventual result of the user allowing us to become an active device admin + * After any of the activities return, try to advance to the "next step" */ @Override protected void onActivityResult(int requestCode, int resultCode, Intent data) { - boolean startedActivity = false; - switch (requestCode) { - case REQUEST_PASSWORD: - case REQUEST_ENCRYPTION: - // Force the result code and just check the DPM to check for actual success - resultCode = Activity.RESULT_OK; - //$FALL-THROUGH$ - case REQUEST_ENABLE: - if (resultCode == Activity.RESULT_OK) { - // now active - try to set actual policies - startedActivity = setActivePolicies(); - } else { - // failed - repost notification, and exit - final long accountId = getIntent().getLongExtra(EXTRA_ACCOUNT_ID, -1); - if (accountId != -1) { - new Thread() { - @Override - public void run() { - SecurityPolicy.getInstance(AccountSecurity.this) - .policiesRequired(accountId); - } - }.start(); - } - } - } - if (!startedActivity) { - finish(); - } + tryAdvanceSecurity(mAccount); super.onActivityResult(requestCode, resultCode, data); } /** - * Now that we are connected as an active device admin, try to set the device to the - * correct security level, and ask for a password if necessary. - * @return true if we started another activity (and should not finish(), as we're waiting for - * their result.) + * Walk the user through the required steps to become an active administrator and with + * the requisite security settings for the given account. + * + * These steps will be repeated each time we return from a given attempt (e.g. asking the + * user to choose a device pin/password). In a typical activation, we may repeat these + * steps a few times. It may go as far as step 5 (password) or step 6 (encryption), but it + * will terminate when step 2 (isActive()) succeeds. + * + * If at any point we do not advance beyond a given user step, (e.g. the user cancels + * instead of setting a password) we simply repost the security notification, and exit. + * We never want to loop here. */ - private boolean setActivePolicies() { - SecurityPolicy sp = SecurityPolicy.getInstance(this); - // check current security level - if sufficient, we're done! - if (sp.isActive(null)) { - Account.clearSecurityHoldOnAllAccounts(this); - return false; + private void tryAdvanceSecurity(Account account) { + SecurityPolicy security = SecurityPolicy.getInstance(this); + + // Step 1. Check if we are an active device administrator, and stop here to activate + if (!security.isActiveAdmin()) { + if (mTriedAddAdministrator) { + repostNotification(account, security); + finish(); + } else { + mTriedAddAdministrator = true; + // retrieve name of server for the format string + HostAuth hostAuth = HostAuth.restoreHostAuthWithId(this, account.mHostAuthKeyRecv); + if (hostAuth == null) { + repostNotification(account, security); + finish(); + } else { + // try to become active - must happen here in activity, to get result + Intent intent = new Intent(DevicePolicyManager.ACTION_ADD_DEVICE_ADMIN); + intent.putExtra(DevicePolicyManager.EXTRA_DEVICE_ADMIN, + security.getAdminComponent()); + intent.putExtra(DevicePolicyManager.EXTRA_ADD_EXPLANATION, + this.getString(R.string.account_security_policy_explanation_fmt, + hostAuth.mAddress)); + startActivityForResult(intent, REQUEST_ENABLE); + } + } + return; } - // set current security level - sp.setActivePolicies(); - // check current security level - if sufficient, we're done! - int inactiveReasons = sp.getInactiveReasons(null); - if (inactiveReasons == 0) { + + // Step 2. Check if the current aggregate security policy is being satisfied by the + // DevicePolicyManager (the current system security level). + if (security.isActive(null)) { Account.clearSecurityHoldOnAllAccounts(this); - return false; + finish(); + return; } - // If password or encryption required, launch relevant intent + + // Step 3. Try to assert the current aggregate security requirements with the system. + security.setActivePolicies(); + + // Step 4. Recheck the security policy, and determine what changes are needed (if any) + // to satisfy the requirements. + int inactiveReasons = security.getInactiveReasons(null); + + // Step 5. If password is needed, try to have the user set it if ((inactiveReasons & SecurityPolicy.INACTIVE_NEED_PASSWORD) != 0) { - // launch the activity to have the user set a new password. - Intent intent = new Intent(DevicePolicyManager.ACTION_SET_NEW_PASSWORD); - startActivityForResult(intent, REQUEST_PASSWORD); - return true; - } else if ((inactiveReasons & SecurityPolicy.INACTIVE_NEED_ENCRYPTION) != 0) { - // launch the activity to start up encryption. - Intent intent = new Intent(DevicePolicyManager.ACTION_START_ENCRYPTION); - startActivityForResult(intent, REQUEST_ENCRYPTION); - return true; + if (mTriedSetPassword) { + repostNotification(account, security); + finish(); + } else { + mTriedSetPassword = true; + // launch the activity to have the user set a new password. + Intent intent = new Intent(DevicePolicyManager.ACTION_SET_NEW_PASSWORD); + startActivityForResult(intent, REQUEST_PASSWORD); + } + return; } - return false; + + // Step 6. If encryption is needed, try to have the user set it + if ((inactiveReasons & SecurityPolicy.INACTIVE_NEED_ENCRYPTION) != 0) { + if (mTriedSetEncryption) { + repostNotification(account, security); + finish(); + } else { + mTriedSetEncryption = true; + // launch the activity to start up encryption. + Intent intent = new Intent(DevicePolicyManager.ACTION_START_ENCRYPTION); + startActivityForResult(intent, REQUEST_ENCRYPTION); + } + return; + } + + // Step 7. No problems were found, so clear holds and exit + Account.clearSecurityHoldOnAllAccounts(this); + finish(); + } + + /** + * Mark an account as not-ready-for-sync and post a notification to bring the user back here + * eventually. + */ + private void repostNotification(final Account account, final SecurityPolicy security) { + Utility.runAsync(new Runnable() { + @Override + public void run() { + security.policiesRequired(account.mId); + } + }); } }