Improve certificate failure notifications.

The commit that introduced those notifications also introduced a rather
... interesting design pattern: The CertificateValidationException
notified the user of its pure existance - it's no longer a 'message'
only, but defines policy. As this is more than unusual, replace this
pattern by the MessagingController treating
CertificateValidationException specially when accessing remote folders.

Also make clear which account failed when constructing the notification.

res/values-de/strings.xml

@@ -214,7 +214,7 @@ Um Fehler zu melden, neue Funktionen vorzuschlagen oder Fragen zu stellen, besuc
     <string name="notification_action_read">Gelesen</string>
     <string name="notification_action_delete">Löschen</string>
 
-    <string name="notification_certificate_error_title">Zertifikatsproblem</string>
+    <string name="notification_certificate_error_title">Zertifikatsproblem (<xliff:g id="account">%s</xliff:g>)</string>
     <string name="notification_certificate_error_text">Servereinstellungen überprüfen</string>
 
     <string name="notification_bg_sync_ticker">Neue Nachrichten in <xliff:g id="account">%s</xliff:g>:<xliff:g id="folder">%s</xliff:g> werden abgerufen</string>

res/values-fr/strings.xml

@@ -245,7 +245,7 @@ de plus</string>
     <string name="notification_action_reply">Répondre</string>
     <string name="notification_action_read">Lire</string>
     <string name="notification_action_delete">Supprimer</string>
-    <string name="notification_certificate_error_title">Erreur de certificat</string>
+    <string name="notification_certificate_error_title">Erreur de certificat (<xliff:g id="account">%s</xliff:g>)</string>
     <string name="notification_certificate_error_text">Vérifier les paramètres serveur</string>
 
     <string name="notification_bg_sync_ticker">Vérification des messages\u00A0: <xliff:g id="account">%s</xliff:g>:<xliff:g id="folder">%s</xliff:g></string>

res/values/strings.xml

@@ -221,7 +221,7 @@ Please submit bug reports, contribute new features and ask questions at
     <string name="notification_action_reply">Reply</string>
     <string name="notification_action_read">Read</string>
     <string name="notification_action_delete">Delete</string>
-    <string name="notification_certificate_error_title">Certificate error</string>
+    <string name="notification_certificate_error_title">Certificate error for <xliff:g id="account">%s</xliff:g></string>
     <string name="notification_certificate_error_text">Check your server settings</string>
 
     <string name="notification_bg_sync_ticker">Checking mail: <xliff:g id="account">%s</xliff:g>:<xliff:g id="folder">%s</xliff:g></string>

src/com/fsck/k9/K9.java

@@ -361,9 +361,9 @@ public class K9 extends Application {
     // Must not conflict with an account number
     public static final int FETCHING_EMAIL_NOTIFICATION      = -5000;
     public static final int SEND_FAILED_NOTIFICATION      = -1500;
+    public static final int CERTIFICATE_EXCEPTION_NOTIFICATION_INCOMING = -2000;
+    public static final int CERTIFICATE_EXCEPTION_NOTIFICATION_OUTGOING = -2500;
     public static final int CONNECTIVITY_ID = -3;
-    public static final int CERTIFICATE_EXCEPTION_NOTIFICATION_INCOMING = -4;
-    public static final int CERTIFICATE_EXCEPTION_NOTIFICATION_OUTGOING = -5;
 
 
     public static class Intents {

src/com/fsck/k9/activity/setup/AccountSetupCheckSettings.java

@@ -108,7 +108,11 @@ public class AccountSetupCheckSettings extends K9Activity implements OnClickList
                         finish();
                         return;
                     }
-                    clearCertificateErrorNotifications();
+
+                    final MessagingController ctrl = MessagingController.getInstance(getApplication());
+                    ctrl.clearCertificateErrorNotifications(AccountSetupCheckSettings.this,
+                            mAccount, mCheckIncoming, mCheckOutgoing);
+
                     if (mCheckIncoming) {
                         store = mAccount.getRemoteStore();
 
@@ -199,19 +203,6 @@ public class AccountSetupCheckSettings extends K9Activity implements OnClickList
         });
     }
 
-    private void clearCertificateErrorNotifications() {
-        final Application app = getApplication();
-        final NotificationManager notifMgr = (NotificationManager) app
-                .getSystemService(Context.NOTIFICATION_SERVICE);
-        final String uuid = mAccount.getUuid();
-        if (mCheckOutgoing){
-            notifMgr.cancel(uuid, K9.CERTIFICATE_EXCEPTION_NOTIFICATION_OUTGOING);
-        }
-        if (mCheckIncoming){
-            notifMgr.cancel(uuid, K9.CERTIFICATE_EXCEPTION_NOTIFICATION_INCOMING);
-        }
-    }
-
     private void showErrorDialog(final int msgResId, final Object... args) {
         mHandler.post(new Runnable() {
             public void run() {

src/com/fsck/k9/activity/setup/AccountSetupIncoming.java

@@ -2,6 +2,7 @@
 package com.fsck.k9.activity.setup;
 
 import android.app.Activity;
+import android.content.Context;
 import android.content.Intent;
 import android.os.Bundle;
 import android.text.Editable;
@@ -33,7 +34,7 @@ import java.util.HashMap;
 import java.util.Map;
 
 public class AccountSetupIncoming extends K9Activity implements OnClickListener {
-    public static final String EXTRA_ACCOUNT = "account";
+    private static final String EXTRA_ACCOUNT = "account";
     private static final String EXTRA_MAKE_DEFAULT = "makeDefault";
 
     private static final int[] POP3_PORTS = {
@@ -90,10 +91,14 @@ public class AccountSetupIncoming extends K9Activity implements OnClickListener
     }
 
     public static void actionEditIncomingSettings(Activity context, Account account) {
+        context.startActivity(intentActionEditIncomingSettings(context, account));
+    }
+
+    public static Intent intentActionEditIncomingSettings(Context context, Account account) {
         Intent i = new Intent(context, AccountSetupIncoming.class);
         i.setAction(Intent.ACTION_EDIT);
         i.putExtra(EXTRA_ACCOUNT, account.getUuid());
-        context.startActivity(i);
+        return i;
     }
 
     @Override

src/com/fsck/k9/activity/setup/AccountSetupOutgoing.java

@@ -26,7 +26,7 @@ import java.net.URLEncoder;
 
 public class AccountSetupOutgoing extends K9Activity implements OnClickListener,
     OnCheckedChangeListener {
-    public static final String EXTRA_ACCOUNT = "account";
+    private static final String EXTRA_ACCOUNT = "account";
 
     private static final String EXTRA_MAKE_DEFAULT = "makeDefault";
 
@@ -74,10 +74,14 @@ public class AccountSetupOutgoing extends K9Activity implements OnClickListener,
     }
 
     public static void actionEditOutgoingSettings(Context context, Account account) {
+        context.startActivity(intentActionEditOutgoingSettings(context, account));
+    }
+
+    public static Intent intentActionEditOutgoingSettings(Context context, Account account) {
         Intent i = new Intent(context, AccountSetupOutgoing.class);
         i.setAction(Intent.ACTION_EDIT);
         i.putExtra(EXTRA_ACCOUNT, account.getUuid());
-        context.startActivity(i);
+        return i;
     }
 
     @Override

src/com/fsck/k9/controller/MessagingController.java

@@ -2,6 +2,8 @@ package com.fsck.k9.controller;
 
 import java.io.CharArrayWriter;
 import java.io.PrintWriter;
+import java.security.cert.CertPathValidatorException;
+import java.security.cert.CertificateException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -51,6 +53,8 @@ import com.fsck.k9.activity.FolderList;
 import com.fsck.k9.activity.MessageList;
 import com.fsck.k9.activity.MessageReference;
 import com.fsck.k9.activity.NotificationDeleteConfirmation;
+import com.fsck.k9.activity.setup.AccountSetupIncoming;
+import com.fsck.k9.activity.setup.AccountSetupOutgoing;
 import com.fsck.k9.helper.Contacts;
 import com.fsck.k9.helper.power.TracingPowerManager;
 import com.fsck.k9.helper.power.TracingPowerManager.TracingWakeLock;
@@ -62,6 +66,7 @@ import com.fsck.k9.mail.Folder.FolderType;
 import com.fsck.k9.mail.Folder.OpenMode;
 import com.fsck.k9.mail.Message;
 import com.fsck.k9.mail.Message.RecipientType;
+import com.fsck.k9.mail.CertificateValidationException;
 import com.fsck.k9.mail.MessagingException;
 import com.fsck.k9.mail.Part;
 import com.fsck.k9.mail.PushReceiver;
@@ -1151,6 +1156,7 @@ public class MessagingController implements Runnable {
             for (MessagingListener l : getListeners(listener)) {
                 l.synchronizeMailboxFailed(account, folder, rootMessage);
             }
+            notifyUserIfCertificateProblem(mApplication, e, account, true);
             addErrorMessage(account, null, e);
             Log.e(K9.LOG_TAG, "Failed synchronizing folder " + account.getDescription() + ":" + folder + " @ " + new Date());
 
@@ -1994,6 +2000,7 @@ public class MessagingController implements Runnable {
                 }
             }
         } catch (MessagingException me) {
+            notifyUserIfCertificateProblem(mApplication, me, account, true);
             addErrorMessage(account, null, me);
             Log.e(K9.LOG_TAG, "Could not process command '" + processingCommand + "'", me);
             throw me;
@@ -2602,6 +2609,60 @@ public class MessagingController implements Runnable {
         }
     }
 
+    private void notifyUserIfCertificateProblem(Context context, Exception e,
+            Account account, boolean incoming) {
+        if (!(e instanceof CertificateValidationException)) {
+            return;
+        }
+
+        CertificateValidationException cve = (CertificateValidationException) e;
+        if (!cve.needsUserAttention()) {
+            return;
+        }
+
+        final int id = incoming
+                ? K9.CERTIFICATE_EXCEPTION_NOTIFICATION_INCOMING + account.getAccountNumber()
+                : K9.CERTIFICATE_EXCEPTION_NOTIFICATION_OUTGOING + account.getAccountNumber();
+        final Intent i = incoming
+                ? AccountSetupIncoming.intentActionEditIncomingSettings(context, account)
+                : AccountSetupOutgoing.intentActionEditOutgoingSettings(context, account);
+        final PendingIntent pi = PendingIntent.getActivity(context,
+                account.getAccountNumber(), i, PendingIntent.FLAG_UPDATE_CURRENT);
+        final String title = context.getString(
+                R.string.notification_certificate_error_text, account.getName());
+
+        final NotificationCompat.Builder builder = new NotificationCompat.Builder(context);
+        builder.setSmallIcon(R.drawable.stat_notify_email_generic);
+        builder.setWhen(System.currentTimeMillis());
+        builder.setAutoCancel(true);
+        builder.setTicker(title);
+        builder.setContentTitle(title);
+        builder.setContentText(context.getString(R.string.notification_certificate_error_text));
+        builder.setContentIntent(pi);
+
+        configureNotification(builder, null, null,
+                K9.NOTIFICATION_LED_FAILURE_COLOR,
+                K9.NOTIFICATION_LED_BLINK_FAST, true);
+
+        final NotificationManager nm = (NotificationManager)
+                context.getSystemService(Context.NOTIFICATION_SERVICE);
+        nm.notify(null, id, builder.build());
+    }
+
+    public void clearCertificateErrorNotifications(Context context,
+            final Account account, boolean incoming, boolean outgoing) {
+        final NotificationManager nm = (NotificationManager)
+                context.getSystemService(Context.NOTIFICATION_SERVICE);
+
+        if (incoming) {
+            nm.cancel(null, K9.CERTIFICATE_EXCEPTION_NOTIFICATION_INCOMING + account.getAccountNumber());
+        }
+        if (outgoing) {
+            nm.cancel(null, K9.CERTIFICATE_EXCEPTION_NOTIFICATION_OUTGOING + account.getAccountNumber());
+        }
+    }
+
+
     static long uidfill = 0;
     static AtomicBoolean loopCatch = new AtomicBoolean();
     public void addErrorMessage(Account account, String subject, Throwable t) {
@@ -2986,6 +3047,7 @@ public class MessagingController implements Runnable {
             for (MessagingListener l : getListeners(listener)) {
                 l.loadMessageForViewFailed(account, folder, uid, e);
             }
+            notifyUserIfCertificateProblem(mApplication, e, account, true);
             addErrorMessage(account, null, e);
             return false;
         } finally {
@@ -3134,6 +3196,7 @@ public class MessagingController implements Runnable {
                     for (MessagingListener l : getListeners(listener)) {
                         l.loadAttachmentFailed(account, message, part, tag, me.getMessage());
                     }
+                    notifyUserIfCertificateProblem(mApplication, me, account, true);
                     addErrorMessage(account, null, me);
 
                 } finally {
@@ -3308,23 +3371,6 @@ public class MessagingController implements Runnable {
                 builder.build());
     }
 
-    public void notify(String tag, int id, String title, String text, PendingIntent pi) {
-        final NotificationManager notifMgr = (NotificationManager) mApplication
-                .getSystemService(Context.NOTIFICATION_SERVICE);
-        NotificationCompat.Builder builder = new NotificationCompat.Builder(mApplication);
-        builder.setSmallIcon(R.drawable.stat_notify_email_generic);
-        builder.setWhen(System.currentTimeMillis());
-        builder.setAutoCancel(true);
-        builder.setTicker(title);
-        builder.setContentTitle(title);
-        builder.setContentText(text);
-        builder.setContentIntent(pi);
-        configureNotification(builder, null, null,
-                K9.NOTIFICATION_LED_FAILURE_COLOR,
-                K9.NOTIFICATION_LED_BLINK_FAST, true);
-        notifMgr.notify(tag, id, builder.build());
-    }
-
     /**
      * Display an ongoing notification while checking for new messages on the server.
      *
@@ -3510,6 +3556,7 @@ public class MessagingController implements Runnable {
                             localFolder.moveMessages(new Message[] { message }, (LocalFolder) localStore.getFolder(account.getDraftsFolderName()));
                         }
 
+                        notifyUserIfCertificateProblem(mApplication, e, account, false);
                         message.setFlag(Flag.X_SEND_FAILED, true);
                         Log.e(K9.LOG_TAG, "Failed to send message", e);
                         for (MessagingListener l : getListeners()) {
@@ -4123,7 +4170,6 @@ public class MessagingController implements Runnable {
                     throw new UnavailableAccountException(e);
                 } catch (Exception e) {
                     Log.e(K9.LOG_TAG, "emptyTrash failed", e);
-
                     addErrorMessage(account, null, e);
                 } finally {
                     closeFolder(localFolder);

src/com/fsck/k9/mail/CertificateValidationException.java

@@ -1,15 +1,6 @@
 
 package com.fsck.k9.mail;
 
-import android.app.Application;
-import android.app.PendingIntent;
-import android.content.Intent;
-import com.fsck.k9.Account;
-import com.fsck.k9.K9;
-import com.fsck.k9.R;
-import com.fsck.k9.activity.setup.AccountSetupIncoming;
-import com.fsck.k9.activity.setup.AccountSetupOutgoing;
-import com.fsck.k9.controller.MessagingController;
 import java.security.cert.CertPathValidatorException;
 import java.security.cert.CertificateException;
 
@@ -20,44 +11,20 @@ public class CertificateValidationException extends MessagingException {
         super(message);
     }
 
-    public CertificateValidationException(final String message,
+    public CertificateValidationException(final String message, Throwable throwable) {
-            Throwable throwable, final Account account, final boolean incoming) {
         super(message, throwable);
-        /*
+    }
-         * We get here because of an SSLException. We are only interested in
+
-         * creating a notification if the underlying cause is certificate
+    public boolean needsUserAttention() {
-         * related.
+        Throwable throwable = getCause();
-         */
+
+        /* user attention is required if the certificate was deemed invalid */
         while (throwable != null
                 && !(throwable instanceof CertPathValidatorException)
                 && !(throwable instanceof CertificateException)) {
             throwable = throwable.getCause();
         }
-        if (throwable == null)
+
-            return;
+        return throwable != null;
-        final Application app = K9.app;
-        final String title = app
-                .getString(R.string.notification_certificate_error_title);
-        final String text = app
-                .getString(R.string.notification_certificate_error_text);
-        final Class<?> className;
-        final String extraName;
-        final int id;
-        if (incoming) {
-            className = AccountSetupIncoming.class;
-            extraName = AccountSetupIncoming.EXTRA_ACCOUNT;
-            id = K9.CERTIFICATE_EXCEPTION_NOTIFICATION_INCOMING;
-        } else {
-            className = AccountSetupOutgoing.class;
-            extraName = AccountSetupOutgoing.EXTRA_ACCOUNT;
-            id = K9.CERTIFICATE_EXCEPTION_NOTIFICATION_OUTGOING;
-        }
-        final Intent i = new Intent(app, className);
-        final String uuid = account.getUuid();
-        i.setAction(Intent.ACTION_EDIT);
-        i.putExtra(extraName, uuid);
-        final PendingIntent pi = PendingIntent.getActivity(app, 0, i, 0);
-        MessagingController controller = MessagingController.getInstance(app);
-        controller.notify(uuid, id, title, text, pi);
     }
 }

src/com/fsck/k9/mail/store/ImapStore.java

@@ -428,11 +428,6 @@ public class ImapStore extends Store {
         public void setCombinedPrefix(String prefix) {
             mCombinedPrefix = prefix;
         }
-
-        @Override
-        public Account getAccount() {
-            return mAccount;
-        }
     }
 
     private static final SimpleDateFormat RFC3501_DATE = new SimpleDateFormat("dd-MMM-yyyy", Locale.US);
@@ -2627,7 +2622,7 @@ public class ImapStore extends Store {
 
 
             } catch (SSLException e) {
-                throw new CertificateValidationException(e.getMessage(), e, mSettings.getAccount(), true);
+                throw new CertificateValidationException(e.getMessage(), e);
             } catch (GeneralSecurityException gse) {
                 throw new MessagingException(
                     "Unable to open connection to IMAP server due to security error.", gse);

src/com/fsck/k9/mail/store/Pop3Store.java

@@ -391,7 +391,7 @@ public class Pop3Store extends Store {
 
                 mCapabilities = getCapabilities();
             } catch (SSLException e) {
-                throw new CertificateValidationException(e.getMessage(), e, mAccount, true);
+                throw new CertificateValidationException(e.getMessage(), e);
             } catch (GeneralSecurityException gse) {
                 throw new MessagingException(
                     "Unable to open connection to POP server due to security error.", gse);

src/com/fsck/k9/mail/store/WebDavStore.java

@@ -848,7 +848,7 @@ public class WebDavStore extends Store {
                                       response.getStatusLine().toString());
             }
         } catch (SSLException e) {
-            throw new CertificateValidationException(e.getMessage(), e, mAccount, true);
+            throw new CertificateValidationException(e.getMessage(), e);
         } catch (IOException ioe) {
             Log.e(K9.LOG_TAG, "IOException: " + ioe + "\nTrace: " + processException(ioe));
             throw new MessagingException("IOException", ioe);

src/com/fsck/k9/mail/transport/SmtpTransport.java

@@ -389,7 +389,7 @@ public class SmtpTransport extends Transport {
                 }
             }
         } catch (SSLException e) {
-            throw new CertificateValidationException(e.getMessage(), e, mAccount, false);
+            throw new CertificateValidationException(e.getMessage(), e);
         } catch (GeneralSecurityException gse) {
             throw new MessagingException(
                 "Unable to open connection to SMTP server due to security error.", gse);

src/com/fsck/k9/mail/transport/imap/ImapSettings.java

@@ -1,6 +1,5 @@
 package com.fsck.k9.mail.transport.imap;
 
-import com.fsck.k9.Account;
 import com.fsck.k9.mail.store.ImapStore;
 import com.fsck.k9.mail.store.ImapStore.AuthType;
 import com.fsck.k9.mail.store.ImapStore.ImapConnection;
@@ -34,7 +33,4 @@ public interface ImapSettings {
     String getCombinedPrefix();
 
     void setCombinedPrefix(String prefix);
-
-    Account getAccount();
-
 }