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
This commit is contained in:
James Lemieux 2014-01-30 12:00:03 -08:00
parent 094656be6e
commit 0dffe3afd7
11 changed files with 167 additions and 96 deletions

View File

@ -266,10 +266,8 @@
android:exported="true" android:exported="true"
> >
<intent-filter> <intent-filter>
<action <action android:name="com.android.email.activity.setup.ACCOUNT_MANAGER_ENTRY" />
android:name="com.android.email.activity.setup.ACCOUNT_MANAGER_ENTRY" /> <category android:name="android.intent.category.DEFAULT" />
<category
android:name="android.intent.category.DEFAULT" />
</intent-filter> </intent-filter>
<intent-filter> <intent-filter>
<action android:name="android.intent.action.EDIT" /> <action android:name="android.intent.action.EDIT" />
@ -285,6 +283,19 @@
<category android:name="android.intent.category.DEFAULT" /> <category android:name="android.intent.category.DEFAULT" />
</intent-filter> </intent-filter>
</activity> </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 <activity
android:name=".provider.FolderPickerActivity" android:name=".provider.FolderPickerActivity"
android:label="@string/folder_picker_title" android:label="@string/folder_picker_title"

View File

@ -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) { public static long getAccountIdFromIntent(Intent intent) {
return getLongFromIntent(intent, ACCOUNT_ID_PARAM); return getLongFromIntent(intent, ACCOUNT_ID_PARAM);

View File

@ -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
}
}

View File

@ -31,8 +31,8 @@ import android.net.Uri;
import android.os.AsyncTask; import android.os.AsyncTask;
import android.os.Bundle; import android.os.Bundle;
import android.preference.PreferenceActivity; import android.preference.PreferenceActivity;
import android.text.TextUtils;
import android.text.SpannableString; import android.text.SpannableString;
import android.text.TextUtils;
import android.text.method.LinkMovementMethod; import android.text.method.LinkMovementMethod;
import android.text.util.Linkify; import android.text.util.Linkify;
import android.view.KeyEvent; import android.view.KeyEvent;
@ -87,6 +87,7 @@ public class AccountSettings extends PreferenceActivity implements FeedbackEnabl
"AccountSettings.for_account_reason"; "AccountSettings.for_account_reason";
private static final String EXTRA_TITLE = "AccountSettings.title"; private static final String EXTRA_TITLE = "AccountSettings.title";
public static final String EXTRA_NO_ACCOUNTS = "AccountSettings.no_account"; 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 // Intent extras for launch directly from system account manager
// NOTE: This string must match the one in res/xml/account_preferences.xml // 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); startActivity(setupIntent);
finish(); finish();
return; return;
} else if (i.hasExtra(EXTRA_ACCOUNT)) {
final Account account = i.getParcelableExtra(EXTRA_ACCOUNT);
mSetupData = new SetupDataFragment(SetupDataFragment.FLOW_MODE_EDIT, account);
} else { } else {
// Otherwise, we're called from within the Email app and look for our extras // Otherwise, we're called from within the Email app and look for our extras
mRequestedAccountId = IntentUtilities.getAccountIdFromIntent(i); mRequestedAccountId = IntentUtilities.getAccountIdFromIntent(i);
@ -337,6 +341,7 @@ public class AccountSettings extends PreferenceActivity implements FeedbackEnabl
// a security vulnerability. // a security vulnerability.
return (TextUtils.equals(AccountSettingsFragment.class.getName(), fragmentName) || return (TextUtils.equals(AccountSettingsFragment.class.getName(), fragmentName) ||
TextUtils.equals(GeneralPreferences.class.getName(), fragmentName) || TextUtils.equals(GeneralPreferences.class.getName(), fragmentName) ||
TextUtils.equals(AccountSetupIncomingFragment.class.getName(), fragmentName) ||
TextUtils.equals(AccountSettingsEditQuickResponsesFragment.class.getName(), TextUtils.equals(AccountSettingsEditQuickResponsesFragment.class.getName(),
fragmentName) || fragmentName) ||
TextUtils.equals(DebugFragment.class.getName(), fragmentName)); TextUtils.equals(DebugFragment.class.getName(), fragmentName));

View File

@ -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();
}
}
}

View File

@ -97,9 +97,10 @@ class ImapConnection {
* Generates and returns the phrase to be used for authentication. This will be a LOGIN with * 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. * username and password, or an OAUTH authentication string, with username and access token.
* Currently, these are the only two auth mechanisms supported. * Currently, these are the only two auth mechanisms supported.
* @return *
* @throws IOException * @throws IOException
* @throws AuthenticationFailedException * @throws AuthenticationFailedException
* @return the login command string to sent to the IMAP server
*/ */
String getLoginPhrase() throws MessagingException, IOException { String getLoginPhrase() throws MessagingException, IOException {
// build the LOGIN string once (instead of over-and-over again.) // 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 * 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() { void close() {
if (mTransport != null) { if (mTransport != null) {
@ -255,7 +256,7 @@ class ImapConnection {
} }
boolean isTransportOpenForTest() { boolean isTransportOpenForTest() {
return mTransport != null ? mTransport.isOpen() : false; return mTransport != null && mTransport.isOpen();
} }
ImapResponse readResponse() throws IOException, MessagingException { ImapResponse readResponse() throws IOException, MessagingException {
@ -315,8 +316,7 @@ class ImapConnection {
return tag; return tag;
} }
List<ImapResponse> executeSimpleCommand(String command) throws IOException, List<ImapResponse> executeSimpleCommand(String command) throws IOException, MessagingException {
MessagingException {
return executeSimpleCommand(command, false); return executeSimpleCommand(command, false);
} }
@ -328,17 +328,25 @@ class ImapConnection {
* @throws MessagingException * @throws MessagingException
*/ */
List<ImapResponse> getCommandResponses() throws IOException, MessagingException { List<ImapResponse> getCommandResponses() throws IOException, MessagingException {
ArrayList<ImapResponse> responses = new ArrayList<ImapResponse>(); final List<ImapResponse> responses = new ArrayList<ImapResponse>();
ImapResponse response; ImapResponse response;
do { do {
response = mParser.readResponse(); response = mParser.readResponse();
responses.add(response); responses.add(response);
} while (!response.isTagged()); } while (!response.isTagged());
if (!response.isOk()) { if (!response.isOk()) {
final String toString = response.toString(); final String toString = response.toString();
final String alert = response.getAlertTextOrEmpty().getString(); final String alert = response.getAlertTextOrEmpty().getString();
final String responseCode = response.getResponseCodeOrEmpty().getString();
destroyResponses(); 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; return responses;
} }
@ -476,8 +484,7 @@ class ImapConnection {
/** /**
* Logs into the IMAP server * Logs into the IMAP server
*/ */
private void doLogin() private void doLogin() throws IOException, MessagingException, AuthenticationFailedException {
throws IOException, MessagingException, AuthenticationFailedException {
try { try {
if (mImapStore.getUseOAuth()) { if (mImapStore.getUseOAuth()) {
// SASL authentication can take multiple steps. Currently the only SASL // SASL authentication can take multiple steps. Currently the only SASL
@ -490,10 +497,17 @@ class ImapConnection {
if (MailActivityEmail.DEBUG) { if (MailActivityEmail.DEBUG) {
LogUtils.d(Logging.LOG_TAG, ie, "ImapException"); LogUtils.d(Logging.LOG_TAG, ie, "ImapException");
} }
throw new AuthenticationFailedException(ie.getAlertText(), ie);
} catch (MessagingException me) { final String code = ie.getResponseCode();
throw new AuthenticationFailedException(null, me); 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); sendCommand("", true);
response = readResponse(); 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;
} }
/** /**

View File

@ -59,10 +59,10 @@ import java.text.SimpleDateFormat;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Arrays; import java.util.Arrays;
import java.util.Date; import java.util.Date;
import java.util.Locale;
import java.util.HashMap; import java.util.HashMap;
import java.util.LinkedHashSet; import java.util.LinkedHashSet;
import java.util.List; import java.util.List;
import java.util.Locale;
import java.util.TimeZone; import java.util.TimeZone;
class ImapFolder extends Folder { class ImapFolder extends Folder {
@ -1236,7 +1236,7 @@ class ImapFolder extends Folder {
mConnection = null; // To prevent close() from returning the connection to the pool. mConnection = null; // To prevent close() from returning the connection to the pool.
close(false); close(false);
} }
return new MessagingException("IO Error", ioe); return new MessagingException(MessagingException.IOERROR, "IO Error", ioe);
} }
@Override @Override

View File

@ -627,29 +627,26 @@ public class ImapStore extends Store {
static class ImapException extends MessagingException { static class ImapException extends MessagingException {
private static final long serialVersionUID = 1L; private static final long serialVersionUID = 1L;
String mAlertText; private final String mAlertText;
private final String mResponseCode;
public ImapException(String message, String alertText, Throwable throwable) { public ImapException(String message, String alertText, String responseCode) {
super(message, throwable);
mAlertText = alertText;
}
public ImapException(String message, String alertText) {
super(message); super(message);
mAlertText = alertText; mAlertText = alertText;
mResponseCode = responseCode;
} }
public String getAlertText() { public String getAlertText() {
return mAlertText; return mAlertText;
} }
public void setAlertText(String alertText) { public String getResponseCode() {
mAlertText = alertText; return mResponseCode;
} }
} }
public void closeConnections() { public void closeConnections() {
ImapConnection connection = null; ImapConnection connection;
while ((connection = mConnectionPool.poll()) != null) { while ((connection = mConnectionPool.poll()) != null) {
connection.close(); connection.close();
} }

View File

@ -96,4 +96,9 @@ public final class ImapConstants {
public static final String XOAUTH2 = "XOAUTH2"; public static final String XOAUTH2 = "XOAUTH2";
public static final String APPENDUID = "APPENDUID"; public static final String APPENDUID = "APPENDUID";
public static final String NIL = "NIL"; 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";
} }

View File

@ -98,6 +98,7 @@ import com.android.emailcommon.service.EmailServiceStatus;
import com.android.emailcommon.service.IEmailService; import com.android.emailcommon.service.IEmailService;
import com.android.emailcommon.service.SearchParams; import com.android.emailcommon.service.SearchParams;
import com.android.emailcommon.utility.AttachmentUtilities; import com.android.emailcommon.utility.AttachmentUtilities;
import com.android.emailcommon.utility.IntentUtilities;
import com.android.emailcommon.utility.Utility; import com.android.emailcommon.utility.Utility;
import com.android.ex.photo.provider.PhotoContract; import com.android.ex.photo.provider.PhotoContract;
import com.android.mail.preferences.MailPrefs; import com.android.mail.preferences.MailPrefs;
@ -171,6 +172,9 @@ public class EmailProvider extends ContentProvider {
private static final String EMAIL_ATTACHMENT_MIME_TYPE = private static final String EMAIL_ATTACHMENT_MIME_TYPE =
"vnd.android.cursor.item/email-attachment"; "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 */ /** Appended to the notification URI for delete operations */
private static final String NOTIFICATION_OP_DELETE = "delete"; private static final String NOTIFICATION_OP_DELETE = "delete";
/** Appended to the notification URI for insert operations */ /** Appended to the notification URI for insert operations */
@ -1001,6 +1005,8 @@ public class EmailProvider extends ContentProvider {
UIPROVIDER_RECENT_FOLDERS_NOTIFIER = UIPROVIDER_RECENT_FOLDERS_NOTIFIER =
Uri.parse("content://" + uiNotificationAuthority + "/uirecentfolders"); Uri.parse("content://" + uiNotificationAuthority + "/uirecentfolders");
BASE_AUTH_URI = Uri.parse("auth://" + EmailContent.EMAIL_PACKAGE_NAME +
".INCOMING_SETTINGS/incoming/");
// All accounts // All accounts
sURIMatcher.addURI(EmailContent.AUTHORITY, "account", ACCOUNT); sURIMatcher.addURI(EmailContent.AUTHORITY, "account", ACCOUNT);
@ -3024,6 +3030,12 @@ public class EmailProvider extends ContentProvider {
.appendQueryParameter("account", account).build().toString(); .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) { private static String getBits(int bitField) {
StringBuilder sb = new StringBuilder(" "); StringBuilder sb = new StringBuilder(" ");
for (int i = 0; i < 32; i++, bitField >>= 1) { for (int i = 0; i < 32; i++, bitField >>= 1) {
@ -3125,6 +3137,10 @@ public class EmailProvider extends ContentProvider {
values.put(UIProvider.AccountColumns.COMPOSE_URI, values.put(UIProvider.AccountColumns.COMPOSE_URI,
getExternalUriStringEmail2("compose", id)); 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)) { if (projectionColumns.contains(UIProvider.AccountColumns.MIME_TYPE)) {
values.put(UIProvider.AccountColumns.MIME_TYPE, EMAIL_APP_MIME_TYPE); values.put(UIProvider.AccountColumns.MIME_TYPE, EMAIL_APP_MIME_TYPE);
} }

View File

@ -144,24 +144,27 @@ public class PopImapSyncAdapterService extends Service {
UIProvider.LastSyncResult.SUCCESS); UIProvider.LastSyncResult.SUCCESS);
} }
} catch (MessagingException e) { } catch (MessagingException e) {
int cause = e.getExceptionType(); final int type = e.getExceptionType();
// XXX It's no good to put the MessagingException.cause here, that's not the // type must be translated into the domain of values used by EmailServiceStatus
// same set of values that we use in EmailServiceStatus. switch(type) {
switch(cause) {
case MessagingException.IOERROR: case MessagingException.IOERROR:
EmailServiceStatus.syncMailboxStatus(resolver, extras, mailboxId, cause, 0, EmailServiceStatus.syncMailboxStatus(resolver, extras, mailboxId, type, 0,
UIProvider.LastSyncResult.CONNECTION_ERROR); UIProvider.LastSyncResult.CONNECTION_ERROR);
syncResult.stats.numIoExceptions++; syncResult.stats.numIoExceptions++;
break; break;
case MessagingException.AUTHENTICATION_FAILED: case MessagingException.AUTHENTICATION_FAILED:
EmailServiceStatus.syncMailboxStatus(resolver, extras, mailboxId, cause, 0, EmailServiceStatus.syncMailboxStatus(resolver, extras, mailboxId, type, 0,
UIProvider.LastSyncResult.AUTH_ERROR); UIProvider.LastSyncResult.AUTH_ERROR);
syncResult.stats.numAuthExceptions++; syncResult.stats.numAuthExceptions++;
break; break;
case MessagingException.SERVER_ERROR:
EmailServiceStatus.syncMailboxStatus(resolver, extras, mailboxId, type, 0,
UIProvider.LastSyncResult.SERVER_ERROR);
break;
default: default:
EmailServiceStatus.syncMailboxStatus(resolver, extras, mailboxId, cause, 0, EmailServiceStatus.syncMailboxStatus(resolver, extras, mailboxId, type, 0,
UIProvider.LastSyncResult.INTERNAL_ERROR); UIProvider.LastSyncResult.INTERNAL_ERROR);
} }
} }
} finally { } finally {