From 0dffe3afd7a2fdfb394573aa0d8d06dd90e9fe12 Mon Sep 17 00:00:00 2001
From: James Lemieux <jplemieux@google.com>
Date: Thu, 30 Jan 2014 12:00:03 -0800
Subject: [PATCH] Keep on getting a couldn't sign-in notification

b/11551107

This is caused by ImapConnection.doLogin() only throwing
AuthenticationFailedExceptions and not other varieties of exceptions.

While fixing this bug, I discovered that the ConversationListFooterView
that is displayed in response to a authentication failure contains a
button called "Sign In" that didn't actually do anything. I made it
navigate to the incoming account settings fragment where the user is
free to change the relevant account credentials.

Change-Id: I2c772ecab18f3e57059eceeae01de08f1fdab4c2
---
 AndroidManifest.xml                           | 19 +++--
 .../emailcommon/utility/IntentUtilities.java  |  2 +-
 .../email/MessagingExceptionStrings.java      | 57 ---------------
 .../email/activity/setup/AccountSettings.java |  7 +-
 .../setup/HeadlessAccountSettingsLoader.java  | 70 +++++++++++++++++++
 .../email/mail/store/ImapConnection.java      | 47 +++++++++----
 .../android/email/mail/store/ImapFolder.java  |  4 +-
 .../android/email/mail/store/ImapStore.java   | 17 ++---
 .../email/mail/store/imap/ImapConstants.java  |  5 ++
 .../android/email/provider/EmailProvider.java | 16 +++++
 .../service/PopImapSyncAdapterService.java    | 19 ++---
 11 files changed, 167 insertions(+), 96 deletions(-)
 delete mode 100644 src/com/android/email/MessagingExceptionStrings.java
 create mode 100644 src/com/android/email/activity/setup/HeadlessAccountSettingsLoader.java

diff --git a/AndroidManifest.xml b/AndroidManifest.xml
index dcf228f94..bafc088cf 100644
--- a/AndroidManifest.xml
+++ b/AndroidManifest.xml
@@ -266,10 +266,8 @@
             android:exported="true"
             >
             <intent-filter>
-                <action
-                    android:name="com.android.email.activity.setup.ACCOUNT_MANAGER_ENTRY" />
-                <category
-                    android:name="android.intent.category.DEFAULT" />
+                <action android:name="com.android.email.activity.setup.ACCOUNT_MANAGER_ENTRY" />
+                <category android:name="android.intent.category.DEFAULT" />
             </intent-filter>
             <intent-filter>
                 <action android:name="android.intent.action.EDIT" />
@@ -285,6 +283,19 @@
                 <category android:name="android.intent.category.DEFAULT" />
             </intent-filter>
         </activity>
+        <!-- a Headless Activity to load the account from the account id before navigating to the
+        Incoming Account Settings fragment -->
+        <activity
+            android:name=".activity.setup.HeadlessAccountSettingsLoader"
+            android:theme="@android:style/Theme.NoDisplay"
+            >
+            <intent-filter>
+                <action android:name="android.intent.action.VIEW" />
+                <category android:name="android.intent.category.DEFAULT" />
+                <data android:host="com.android.email.INCOMING_SETTINGS" />
+                <data android:scheme="auth"/>
+            </intent-filter>
+        </activity>
         <activity
             android:name=".provider.FolderPickerActivity"
             android:label="@string/folder_picker_title"
diff --git a/emailcommon/src/com/android/emailcommon/utility/IntentUtilities.java b/emailcommon/src/com/android/emailcommon/utility/IntentUtilities.java
index bd1c01304..d69e61672 100644
--- a/emailcommon/src/com/android/emailcommon/utility/IntentUtilities.java
+++ b/emailcommon/src/com/android/emailcommon/utility/IntentUtilities.java
@@ -98,7 +98,7 @@ public final class IntentUtilities {
     }
 
     /**
-     * Retrieve the account ID.
+     * Retrieve the account ID from the underlying URI.
      */
     public static long getAccountIdFromIntent(Intent intent) {
         return getLongFromIntent(intent, ACCOUNT_ID_PARAM);
diff --git a/src/com/android/email/MessagingExceptionStrings.java b/src/com/android/email/MessagingExceptionStrings.java
deleted file mode 100644
index cb2c6070c..000000000
--- a/src/com/android/email/MessagingExceptionStrings.java
+++ /dev/null
@@ -1,57 +0,0 @@
-/*
- * Copyright (C) 2011 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.email;
-
-import com.android.emailcommon.mail.MessagingException;
-
-import android.content.Context;
-
-/**
- * @return the error message associated with this exception.
- */
-public class MessagingExceptionStrings {
-    public static String getErrorString(Context context, MessagingException e) {
-        return context.getResources().getString(getErrorStringResourceId(e));
-    }
-
-    /**
-     * @return the resource ID of the error message associated with this exception.
-     */
-    private static int getErrorStringResourceId(MessagingException e) {
-        switch (e.getExceptionType()) {
-            case MessagingException.IOERROR:
-                return R.string.account_setup_failed_ioerror;
-            case MessagingException.ATTACHMENT_NOT_FOUND:
-                return R.string.attachment_not_found;
-            case MessagingException.TLS_REQUIRED:
-                return R.string.account_setup_failed_tls_required;
-            case MessagingException.AUTH_REQUIRED:
-                return R.string.account_setup_failed_auth_required;
-            case MessagingException.GENERAL_SECURITY:
-                return R.string.account_setup_failed_security;
-                // TODO Generate a unique string for this case, which is the case
-                // where the security policy needs to be updated.
-            case MessagingException.SECURITY_POLICIES_REQUIRED:
-                return R.string.account_setup_failed_security;
-            case MessagingException.ACCESS_DENIED:
-                return R.string.account_setup_failed_access_denied;
-            case MessagingException.CLIENT_CERTIFICATE_ERROR:
-                return R.string.account_setup_failed_certificate_inaccessible;
-        }
-        return R.string.status_network_error; // default
-    }
-}
diff --git a/src/com/android/email/activity/setup/AccountSettings.java b/src/com/android/email/activity/setup/AccountSettings.java
index 2e9ef22d2..052d6aaed 100644
--- a/src/com/android/email/activity/setup/AccountSettings.java
+++ b/src/com/android/email/activity/setup/AccountSettings.java
@@ -31,8 +31,8 @@ import android.net.Uri;
 import android.os.AsyncTask;
 import android.os.Bundle;
 import android.preference.PreferenceActivity;
-import android.text.TextUtils;
 import android.text.SpannableString;
+import android.text.TextUtils;
 import android.text.method.LinkMovementMethod;
 import android.text.util.Linkify;
 import android.view.KeyEvent;
@@ -87,6 +87,7 @@ public class AccountSettings extends PreferenceActivity implements FeedbackEnabl
             "AccountSettings.for_account_reason";
     private static final String EXTRA_TITLE = "AccountSettings.title";
     public static final String EXTRA_NO_ACCOUNTS = "AccountSettings.no_account";
+    public static final String EXTRA_ACCOUNT = "AccountSettings.account";
 
     // Intent extras for launch directly from system account manager
     // NOTE: This string must match the one in res/xml/account_preferences.xml
@@ -206,6 +207,9 @@ public class AccountSettings extends PreferenceActivity implements FeedbackEnabl
                 startActivity(setupIntent);
                 finish();
                 return;
+            } else if (i.hasExtra(EXTRA_ACCOUNT)) {
+                final Account account = i.getParcelableExtra(EXTRA_ACCOUNT);
+                mSetupData = new SetupDataFragment(SetupDataFragment.FLOW_MODE_EDIT, account);
             } else {
                 // Otherwise, we're called from within the Email app and look for our extras
                 mRequestedAccountId = IntentUtilities.getAccountIdFromIntent(i);
@@ -337,6 +341,7 @@ public class AccountSettings extends PreferenceActivity implements FeedbackEnabl
         // a security vulnerability.
         return (TextUtils.equals(AccountSettingsFragment.class.getName(), fragmentName) ||
                 TextUtils.equals(GeneralPreferences.class.getName(), fragmentName) ||
+                TextUtils.equals(AccountSetupIncomingFragment.class.getName(), fragmentName) ||
                 TextUtils.equals(AccountSettingsEditQuickResponsesFragment.class.getName(),
                         fragmentName) ||
                 TextUtils.equals(DebugFragment.class.getName(), fragmentName));
diff --git a/src/com/android/email/activity/setup/HeadlessAccountSettingsLoader.java b/src/com/android/email/activity/setup/HeadlessAccountSettingsLoader.java
new file mode 100644
index 000000000..ba43f9d7a
--- /dev/null
+++ b/src/com/android/email/activity/setup/HeadlessAccountSettingsLoader.java
@@ -0,0 +1,70 @@
+package com.android.email.activity.setup;
+
+import android.app.Activity;
+import android.content.Context;
+import android.content.Intent;
+import android.os.AsyncTask;
+import android.os.Bundle;
+import android.preference.PreferenceActivity;
+
+import com.android.emailcommon.provider.Account;
+import com.android.emailcommon.utility.IntentUtilities;
+
+/**
+ * This activity is headless. It exists to load the Account object from  the given account ID and
+ * then starts the {@link AccountSettings} activity with the appropriate fragment showing in place.
+ */
+public class HeadlessAccountSettingsLoader extends Activity {
+
+    @Override
+    public void onCreate(Bundle savedInstanceState) {
+        super.onCreate(savedInstanceState);
+
+        final Intent i = getIntent();
+        final long accountID = IntentUtilities.getAccountIdFromIntent(i);
+
+        if ("incoming".equals(i.getData().getLastPathSegment())) {
+            new LoadAccountIncomingSettingsAsyncTask(getApplicationContext())
+                    .executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR, accountID);
+        }
+    }
+
+    /**
+     * Asynchronously loads the Account object from its ID and then navigates to the AccountSettings
+     * fragment.
+     */
+    private class LoadAccountIncomingSettingsAsyncTask extends AsyncTask<Long, Void, Account> {
+        private final Context mContext;
+
+        private LoadAccountIncomingSettingsAsyncTask(Context context) {
+            mContext = context;
+        }
+
+        protected Account doInBackground(Long... params) {
+            return Account.restoreAccountWithId(mContext, params[0]);
+        }
+
+        protected void onPostExecute(Account result) {
+            // create an Intent to view a new activity
+            final Intent intent = new Intent(Intent.ACTION_VIEW);
+            intent.setFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
+
+            // we are navigating explicitly to the AccountSettings activity
+            intent.setClass(mContext, AccountSettings.class);
+
+            // place the account in the intent as an extra
+            intent.putExtra(AccountSettings.EXTRA_ACCOUNT, result);
+
+            // these extras show the "incoming fragment" in the AccountSettings activity by default
+            intent.putExtra(PreferenceActivity.EXTRA_SHOW_FRAGMENT,
+                    AccountSetupIncomingFragment.class.getCanonicalName());
+            intent.putExtra(PreferenceActivity.EXTRA_SHOW_FRAGMENT_ARGUMENTS,
+                    AccountSetupIncomingFragment.getArgs(true));
+            intent.putExtra(PreferenceActivity.EXTRA_NO_HEADERS, true);
+
+            mContext.startActivity(intent);
+
+            finish();
+         }
+    }
+}
\ No newline at end of file
diff --git a/src/com/android/email/mail/store/ImapConnection.java b/src/com/android/email/mail/store/ImapConnection.java
index 88a8eaa91..2e9019edd 100644
--- a/src/com/android/email/mail/store/ImapConnection.java
+++ b/src/com/android/email/mail/store/ImapConnection.java
@@ -97,9 +97,10 @@ class ImapConnection {
      * Generates and returns the phrase to be used for authentication. This will be a LOGIN with
      * username and password, or an OAUTH authentication string, with username and access token.
      * Currently, these are the only two auth mechanisms supported.
-     * @return
+     *
      * @throws IOException
      * @throws AuthenticationFailedException
+     * @return the login command string to sent to the IMAP server
      */
     String getLoginPhrase() throws MessagingException, IOException {
         // build the LOGIN string once (instead of over-and-over again.)
@@ -197,7 +198,7 @@ class ImapConnection {
 
     /**
      * Closes the connection and releases all resources. This connection can not be used again
-     * until {@link #setStore(ImapStore, String, String)} is called.
+     * until {@link #setStore(ImapStore)} is called.
      */
     void close() {
         if (mTransport != null) {
@@ -255,7 +256,7 @@ class ImapConnection {
     }
 
     boolean isTransportOpenForTest() {
-        return mTransport != null ? mTransport.isOpen() : false;
+        return mTransport != null && mTransport.isOpen();
     }
 
     ImapResponse readResponse() throws IOException, MessagingException {
@@ -315,8 +316,7 @@ class ImapConnection {
         return tag;
     }
 
-    List<ImapResponse> executeSimpleCommand(String command) throws IOException,
-            MessagingException {
+    List<ImapResponse> executeSimpleCommand(String command) throws IOException, MessagingException {
         return executeSimpleCommand(command, false);
     }
 
@@ -328,17 +328,25 @@ class ImapConnection {
      * @throws MessagingException
      */
     List<ImapResponse> getCommandResponses() throws IOException, MessagingException {
-        ArrayList<ImapResponse> responses = new ArrayList<ImapResponse>();
+        final List<ImapResponse> responses = new ArrayList<ImapResponse>();
         ImapResponse response;
         do {
             response = mParser.readResponse();
             responses.add(response);
         } while (!response.isTagged());
+
         if (!response.isOk()) {
             final String toString = response.toString();
             final String alert = response.getAlertTextOrEmpty().getString();
+            final String responseCode = response.getResponseCodeOrEmpty().getString();
             destroyResponses();
-            throw new ImapException(toString, alert);
+
+            // if the response code indicates an error occurred within the server, indicate that
+            if (ImapConstants.UNAVAILABLE.equals(responseCode)) {
+                throw new MessagingException(MessagingException.SERVER_ERROR, alert);
+            }
+
+            throw new ImapException(toString, alert, responseCode);
         }
         return responses;
     }
@@ -476,8 +484,7 @@ class ImapConnection {
     /**
      * Logs into the IMAP server
      */
-    private void doLogin()
-            throws IOException, MessagingException, AuthenticationFailedException {
+    private void doLogin() throws IOException, MessagingException, AuthenticationFailedException {
         try {
             if (mImapStore.getUseOAuth()) {
                 // SASL authentication can take multiple steps. Currently the only SASL
@@ -490,10 +497,17 @@ class ImapConnection {
             if (MailActivityEmail.DEBUG) {
                 LogUtils.d(Logging.LOG_TAG, ie, "ImapException");
             }
-            throw new AuthenticationFailedException(ie.getAlertText(), ie);
 
-        } catch (MessagingException me) {
-            throw new AuthenticationFailedException(null, me);
+            final String code = ie.getResponseCode();
+            final String alertText = ie.getAlertText();
+
+            // if the response code indicates expired or bad credentials, throw a special exception
+            if (ImapConstants.AUTHENTICATIONFAILED.equals(code) ||
+                    ImapConstants.EXPIRED.equals(code)) {
+                throw new AuthenticationFailedException(alertText, ie);
+            }
+
+            throw new MessagingException(alertText, ie);
         }
     }
 
@@ -542,8 +556,15 @@ class ImapConnection {
             sendCommand("", true);
             response = readResponse();
         }
-        return response;
 
+        // if the response code indicates an error occurred within the server, indicate that
+        final String responseCode = response.getResponseCodeOrEmpty().getString();
+        if (ImapConstants.UNAVAILABLE.equals(responseCode)) {
+            final String alert = response.getAlertTextOrEmpty().getString();
+            throw new MessagingException(MessagingException.SERVER_ERROR, alert);
+        }
+
+        return response;
     }
 
     /**
diff --git a/src/com/android/email/mail/store/ImapFolder.java b/src/com/android/email/mail/store/ImapFolder.java
index 4f9588b00..30857360c 100644
--- a/src/com/android/email/mail/store/ImapFolder.java
+++ b/src/com/android/email/mail/store/ImapFolder.java
@@ -59,10 +59,10 @@ import java.text.SimpleDateFormat;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Date;
-import java.util.Locale;
 import java.util.HashMap;
 import java.util.LinkedHashSet;
 import java.util.List;
+import java.util.Locale;
 import java.util.TimeZone;
 
 class ImapFolder extends Folder {
@@ -1236,7 +1236,7 @@ class ImapFolder extends Folder {
             mConnection = null; // To prevent close() from returning the connection to the pool.
             close(false);
         }
-        return new MessagingException("IO Error", ioe);
+        return new MessagingException(MessagingException.IOERROR, "IO Error", ioe);
     }
 
     @Override
diff --git a/src/com/android/email/mail/store/ImapStore.java b/src/com/android/email/mail/store/ImapStore.java
index b14e58d34..82bb289b4 100644
--- a/src/com/android/email/mail/store/ImapStore.java
+++ b/src/com/android/email/mail/store/ImapStore.java
@@ -627,29 +627,26 @@ public class ImapStore extends Store {
     static class ImapException extends MessagingException {
         private static final long serialVersionUID = 1L;
 
-        String mAlertText;
+        private final String mAlertText;
+        private final String mResponseCode;
 
-        public ImapException(String message, String alertText, Throwable throwable) {
-            super(message, throwable);
-            mAlertText = alertText;
-        }
-
-        public ImapException(String message, String alertText) {
+        public ImapException(String message, String alertText, String responseCode) {
             super(message);
             mAlertText = alertText;
+            mResponseCode = responseCode;
         }
 
         public String getAlertText() {
             return mAlertText;
         }
 
-        public void setAlertText(String alertText) {
-            mAlertText = alertText;
+        public String getResponseCode() {
+            return mResponseCode;
         }
     }
 
     public void closeConnections() {
-        ImapConnection connection = null;
+        ImapConnection connection;
         while ((connection = mConnectionPool.poll()) != null) {
             connection.close();
         }
diff --git a/src/com/android/email/mail/store/imap/ImapConstants.java b/src/com/android/email/mail/store/imap/ImapConstants.java
index 5a3290420..9f4d59290 100644
--- a/src/com/android/email/mail/store/imap/ImapConstants.java
+++ b/src/com/android/email/mail/store/imap/ImapConstants.java
@@ -96,4 +96,9 @@ public final class ImapConstants {
     public static final String XOAUTH2 = "XOAUTH2";
     public static final String APPENDUID = "APPENDUID";
     public static final String NIL = "NIL";
+
+    /** response codes within IMAP responses */
+    public static final String EXPIRED = "EXPIRED";
+    public static final String AUTHENTICATIONFAILED = "AUTHENTICATIONFAILED";
+    public static final String UNAVAILABLE = "UNAVAILABLE";
 }
diff --git a/src/com/android/email/provider/EmailProvider.java b/src/com/android/email/provider/EmailProvider.java
index e20e487d8..e4ab1a0fc 100644
--- a/src/com/android/email/provider/EmailProvider.java
+++ b/src/com/android/email/provider/EmailProvider.java
@@ -98,6 +98,7 @@ import com.android.emailcommon.service.EmailServiceStatus;
 import com.android.emailcommon.service.IEmailService;
 import com.android.emailcommon.service.SearchParams;
 import com.android.emailcommon.utility.AttachmentUtilities;
+import com.android.emailcommon.utility.IntentUtilities;
 import com.android.emailcommon.utility.Utility;
 import com.android.ex.photo.provider.PhotoContract;
 import com.android.mail.preferences.MailPrefs;
@@ -171,6 +172,9 @@ public class EmailProvider extends ContentProvider {
     private static final String EMAIL_ATTACHMENT_MIME_TYPE =
         "vnd.android.cursor.item/email-attachment";
 
+    /** The base of the URI that navigates to the settings page to alter email auth credentials */
+    private static Uri BASE_AUTH_URI;
+
     /** Appended to the notification URI for delete operations */
     private static final String NOTIFICATION_OP_DELETE = "delete";
     /** Appended to the notification URI for insert operations */
@@ -1001,6 +1005,8 @@ public class EmailProvider extends ContentProvider {
             UIPROVIDER_RECENT_FOLDERS_NOTIFIER =
                     Uri.parse("content://" + uiNotificationAuthority + "/uirecentfolders");
 
+            BASE_AUTH_URI = Uri.parse("auth://" + EmailContent.EMAIL_PACKAGE_NAME +
+                    ".INCOMING_SETTINGS/incoming/");
 
             // All accounts
             sURIMatcher.addURI(EmailContent.AUTHORITY, "account", ACCOUNT);
@@ -3024,6 +3030,12 @@ public class EmailProvider extends ContentProvider {
                 .appendQueryParameter("account", account).build().toString();
     }
 
+    private static String getExternalUriStringReathentication(long accountId) {
+        final Uri.Builder builder = BASE_AUTH_URI.buildUpon();
+        IntentUtilities.setAccountId(builder, accountId);
+        return builder.build().toString();
+    }
+
     private static String getBits(int bitField) {
         StringBuilder sb = new StringBuilder(" ");
         for (int i = 0; i < 32; i++, bitField >>= 1) {
@@ -3125,6 +3137,10 @@ public class EmailProvider extends ContentProvider {
             values.put(UIProvider.AccountColumns.COMPOSE_URI,
                     getExternalUriStringEmail2("compose", id));
         }
+        if (projectionColumns.contains(UIProvider.AccountColumns.REAUTHENTICATION_INTENT_URI)) {
+            values.put(UIProvider.AccountColumns.REAUTHENTICATION_INTENT_URI,
+                    getExternalUriStringReathentication(accountId));
+        }
         if (projectionColumns.contains(UIProvider.AccountColumns.MIME_TYPE)) {
             values.put(UIProvider.AccountColumns.MIME_TYPE, EMAIL_APP_MIME_TYPE);
         }
diff --git a/src/com/android/email/service/PopImapSyncAdapterService.java b/src/com/android/email/service/PopImapSyncAdapterService.java
index 4099be5c2..4b62860e0 100644
--- a/src/com/android/email/service/PopImapSyncAdapterService.java
+++ b/src/com/android/email/service/PopImapSyncAdapterService.java
@@ -144,24 +144,27 @@ public class PopImapSyncAdapterService extends Service {
                             UIProvider.LastSyncResult.SUCCESS);
                 }
             } catch (MessagingException e) {
-                int cause = e.getExceptionType();
-                // XXX It's no good to put the MessagingException.cause here, that's not the
-                // same set of values that we use in EmailServiceStatus.
-                switch(cause) {
+                final int type = e.getExceptionType();
+                // type must be translated into the domain of values used by EmailServiceStatus
+                switch(type) {
                     case MessagingException.IOERROR:
-                        EmailServiceStatus.syncMailboxStatus(resolver, extras, mailboxId, cause, 0,
+                        EmailServiceStatus.syncMailboxStatus(resolver, extras, mailboxId, type, 0,
                                 UIProvider.LastSyncResult.CONNECTION_ERROR);
                         syncResult.stats.numIoExceptions++;
                         break;
                     case MessagingException.AUTHENTICATION_FAILED:
-                        EmailServiceStatus.syncMailboxStatus(resolver, extras, mailboxId, cause, 0,
+                        EmailServiceStatus.syncMailboxStatus(resolver, extras, mailboxId, type, 0,
                                 UIProvider.LastSyncResult.AUTH_ERROR);
                         syncResult.stats.numAuthExceptions++;
                         break;
+                    case MessagingException.SERVER_ERROR:
+                        EmailServiceStatus.syncMailboxStatus(resolver, extras, mailboxId, type, 0,
+                                UIProvider.LastSyncResult.SERVER_ERROR);
+                        break;
 
                     default:
-                    EmailServiceStatus.syncMailboxStatus(resolver, extras, mailboxId, cause, 0,
-                            UIProvider.LastSyncResult.INTERNAL_ERROR);
+                        EmailServiceStatus.syncMailboxStatus(resolver, extras, mailboxId, type, 0,
+                                UIProvider.LastSyncResult.INTERNAL_ERROR);
                 }
             }
         } finally {