From e97164cbb0a0cfd313771a45303aca624a766f85 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Tue, 26 Sep 2017 20:12:47 +0200 Subject: [PATCH] Show error if encryption is enabled but no key is configured --- .../activity/compose/ComposeCryptoStatus.java | 19 ++--- .../k9/activity/compose/RecipientMvpView.java | 4 +- .../activity/compose/RecipientPresenter.java | 7 +- .../fsck/k9/message/PgpMessageBuilder.java | 14 ++-- k9mail/src/main/res/values/strings.xml | 2 +- .../k9/message/PgpMessageBuilderTest.java | 75 +++++++++++++++---- 6 files changed, 85 insertions(+), 36 deletions(-) diff --git a/k9mail/src/main/java/com/fsck/k9/activity/compose/ComposeCryptoStatus.java b/k9mail/src/main/java/com/fsck/k9/activity/compose/ComposeCryptoStatus.java index b1d226599..ec88c16f3 100644 --- a/k9mail/src/main/java/com/fsck/k9/activity/compose/ComposeCryptoStatus.java +++ b/k9mail/src/main/java/com/fsck/k9/activity/compose/ComposeCryptoStatus.java @@ -117,7 +117,7 @@ public class ComposeCryptoStatus { return CryptoSpecialModeDisplayType.SIGN_ONLY; } - if (canEncrypt() && isPgpInlineModeEnabled()) { + if (allRecipientsCanEncrypt() && isPgpInlineModeEnabled()) { return CryptoSpecialModeDisplayType.PGP_INLINE; } @@ -126,7 +126,7 @@ public class ComposeCryptoStatus { public boolean shouldUsePgpMessageBuilder() { // CryptoProviderState.ERROR will be handled as an actual error, see SendErrorState - return cryptoProviderState != CryptoProviderState.UNCONFIGURED && openPgpKeyId != null; + return cryptoProviderState != CryptoProviderState.UNCONFIGURED; } public boolean isEncryptionEnabled() { @@ -155,7 +155,7 @@ public class ComposeCryptoStatus { return cryptoProviderState == CryptoProviderState.OK; } - boolean canEncrypt() { + boolean allRecipientsCanEncrypt() { return recipientAutocryptStatus != null && recipientAutocryptStatus.type.canEncrypt(); } @@ -168,11 +168,7 @@ public class ComposeCryptoStatus { } boolean canEncryptAndIsMutual() { - return canEncrypt() && recipientAutocryptStatus.type.isMutual(); - } - - boolean isEncryptionEnabledError() { - return isEncryptionEnabled() && !canEncrypt(); + return allRecipientsCanEncrypt() && recipientAutocryptStatus.type.isMutual(); } boolean hasAutocryptPendingIntent() { @@ -258,6 +254,7 @@ public class ComposeCryptoStatus { public enum SendErrorState { PROVIDER_ERROR, + KEY_CONFIG_ERROR, ENABLED_ERROR } @@ -267,7 +264,11 @@ public class ComposeCryptoStatus { return SendErrorState.PROVIDER_ERROR; } - if (isEncryptionEnabledError()) { + if (openPgpKeyId == null && (isEncryptionEnabled() || isSignOnly())) { + return SendErrorState.KEY_CONFIG_ERROR; + } + + if (isEncryptionEnabled() && !allRecipientsCanEncrypt()) { return SendErrorState.ENABLED_ERROR; } diff --git a/k9mail/src/main/java/com/fsck/k9/activity/compose/RecipientMvpView.java b/k9mail/src/main/java/com/fsck/k9/activity/compose/RecipientMvpView.java index 5723a274d..8fcc26ec8 100644 --- a/k9mail/src/main/java/com/fsck/k9/activity/compose/RecipientMvpView.java +++ b/k9mail/src/main/java/com/fsck/k9/activity/compose/RecipientMvpView.java @@ -349,8 +349,8 @@ public class RecipientMvpView implements OnFocusChangeListener, OnClickListener Toast.makeText(activity, R.string.error_crypto_provider_ui_required, Toast.LENGTH_LONG).show(); } - public void showErrorMissingSignKey() { - Toast.makeText(activity, R.string.compose_error_no_signing_key, Toast.LENGTH_LONG).show(); + public void showErrorNoKeyConfigured() { + Toast.makeText(activity, R.string.compose_error_no_key_configured, Toast.LENGTH_LONG).show(); } public void showErrorInlineAttach() { diff --git a/k9mail/src/main/java/com/fsck/k9/activity/compose/RecipientPresenter.java b/k9mail/src/main/java/com/fsck/k9/activity/compose/RecipientPresenter.java index 1b27ff637..4735d2b1b 100644 --- a/k9mail/src/main/java/com/fsck/k9/activity/compose/RecipientPresenter.java +++ b/k9mail/src/main/java/com/fsck/k9/activity/compose/RecipientPresenter.java @@ -624,7 +624,7 @@ public class RecipientPresenter implements PermissionPingCallback { return; } - if (currentCryptoStatus.isEncryptionEnabledError()) { + if (currentCryptoStatus.isEncryptionEnabled() && !currentCryptoStatus.allRecipientsCanEncrypt()) { recipientMvpView.showOpenPgpEnabledErrorDialog(false); return; } @@ -688,6 +688,9 @@ public class RecipientPresenter implements PermissionPingCallback { case PROVIDER_ERROR: recipientMvpView.showErrorOpenPgpConnection(); break; + case KEY_CONFIG_ERROR: + recipientMvpView.showErrorNoKeyConfigured(); + break; default: throw new AssertionError("not all error states handled, this is a bug!"); } @@ -894,7 +897,7 @@ public class RecipientPresenter implements PermissionPingCallback { return; } if (enableEncryption) { - if (!cachedCryptoStatus.canEncrypt()) { + if (!cachedCryptoStatus.allRecipientsCanEncrypt()) { onCryptoModeChanged(CryptoMode.CHOICE_ENABLED); recipientMvpView.showOpenPgpEnabledErrorDialog(true); } else if (cachedCryptoStatus.canEncryptAndIsMutual()) { diff --git a/k9mail/src/main/java/com/fsck/k9/message/PgpMessageBuilder.java b/k9mail/src/main/java/com/fsck/k9/message/PgpMessageBuilder.java index d485d7c1e..98f308ff9 100644 --- a/k9mail/src/main/java/com/fsck/k9/message/PgpMessageBuilder.java +++ b/k9mail/src/main/java/com/fsck/k9/message/PgpMessageBuilder.java @@ -87,20 +87,22 @@ public class PgpMessageBuilder extends MessageBuilder { throw new IllegalStateException("PgpMessageBuilder must have cryptoStatus set before building!"); } + Long openPgpKeyId = cryptoStatus.getOpenPgpKeyId(); try { - if (!cryptoStatus.isProviderStateOk()) { - throw new MessagingException("OpenPGP Provider is not ready!"); - } - currentProcessedMimeMessage = build(); } catch (MessagingException me) { queueMessageBuildException(me); return; } - Long openPgpKeyId = cryptoStatus.getOpenPgpKeyId(); if (openPgpKeyId == null) { - throw new IllegalArgumentException("PgpMessageBuilder requires a configured key id!"); + queueMessageBuildSuccess(currentProcessedMimeMessage); + return; + } + + if (!cryptoStatus.isProviderStateOk()) { + queueMessageBuildException(new MessagingException("OpenPGP Provider is not ready!")); + return; } Address address = currentProcessedMimeMessage.getFrom()[0]; diff --git a/k9mail/src/main/res/values/strings.xml b/k9mail/src/main/res/values/strings.xml index b50147752..d56e7cae7 100644 --- a/k9mail/src/main/res/values/strings.xml +++ b/k9mail/src/main/res/values/strings.xml @@ -1146,7 +1146,7 @@ Please submit bug reports, contribute new features and ask questions at Other Mobile No Drafts folder configured for this account! - No key configured for signing! Please check your settings. + No key configured for this account! Check your settings. Don\'t encrypt Encrypt if possible Encrypt diff --git a/k9mail/src/test/java/com/fsck/k9/message/PgpMessageBuilderTest.java b/k9mail/src/test/java/com/fsck/k9/message/PgpMessageBuilderTest.java index e70b8dccb..1736faa2c 100644 --- a/k9mail/src/test/java/com/fsck/k9/message/PgpMessageBuilderTest.java +++ b/k9mail/src/test/java/com/fsck/k9/message/PgpMessageBuilderTest.java @@ -85,33 +85,76 @@ public class PgpMessageBuilderTest { } @Test - public void build__withCryptoProviderNotOk__shouldThrow() throws MessagingException { - cryptoStatusBuilder.setCryptoMode(CryptoMode.SIGN_ONLY); - CryptoProviderState[] cryptoProviderStates = { - CryptoProviderState.LOST_CONNECTION, CryptoProviderState.UNCONFIGURED, - CryptoProviderState.UNINITIALIZED, CryptoProviderState.ERROR - }; + public void build__withCryptoProviderUnconfigured__shouldThrow() throws MessagingException { + cryptoStatusBuilder.setCryptoMode(CryptoMode.NO_CHOICE); - for (CryptoProviderState state : cryptoProviderStates) { - cryptoStatusBuilder.setCryptoProviderState(state); - pgpMessageBuilder.setCryptoStatus(cryptoStatusBuilder.build()); + cryptoStatusBuilder.setCryptoProviderState(CryptoProviderState.UNCONFIGURED); + pgpMessageBuilder.setCryptoStatus(cryptoStatusBuilder.build()); - Callback mockCallback = mock(Callback.class); - pgpMessageBuilder.buildAsync(mockCallback); + Callback mockCallback = mock(Callback.class); + pgpMessageBuilder.buildAsync(mockCallback); - verify(mockCallback).onMessageBuildException(any(MessagingException.class)); - verifyNoMoreInteractions(mockCallback); - } + verify(mockCallback).onMessageBuildException(any(MessagingException.class)); + verifyNoMoreInteractions(mockCallback); } - @Test(expected = RuntimeException.class) - public void buildCleartext__withNoSigningKey__shouldThrow() { + @Test + public void build__withCryptoProviderUninitialized__shouldThrow() throws MessagingException { + cryptoStatusBuilder.setCryptoMode(CryptoMode.NO_CHOICE); + + cryptoStatusBuilder.setCryptoProviderState(CryptoProviderState.UNINITIALIZED); + pgpMessageBuilder.setCryptoStatus(cryptoStatusBuilder.build()); + + Callback mockCallback = mock(Callback.class); + pgpMessageBuilder.buildAsync(mockCallback); + + verify(mockCallback).onMessageBuildException(any(MessagingException.class)); + verifyNoMoreInteractions(mockCallback); + } + + @Test + public void build__withCryptoProviderError__shouldThrow() throws MessagingException { + cryptoStatusBuilder.setCryptoMode(CryptoMode.NO_CHOICE); + + cryptoStatusBuilder.setCryptoProviderState(CryptoProviderState.ERROR); + pgpMessageBuilder.setCryptoStatus(cryptoStatusBuilder.build()); + + Callback mockCallback = mock(Callback.class); + pgpMessageBuilder.buildAsync(mockCallback); + + verify(mockCallback).onMessageBuildException(any(MessagingException.class)); + verifyNoMoreInteractions(mockCallback); + } + + @Test + public void build__withCryptoProviderLostConnection__shouldThrow() throws MessagingException { + cryptoStatusBuilder.setCryptoMode(CryptoMode.NO_CHOICE); + + cryptoStatusBuilder.setCryptoProviderState(CryptoProviderState.LOST_CONNECTION); + pgpMessageBuilder.setCryptoStatus(cryptoStatusBuilder.build()); + + Callback mockCallback = mock(Callback.class); + pgpMessageBuilder.buildAsync(mockCallback); + + verify(mockCallback).onMessageBuildException(any(MessagingException.class)); + verifyNoMoreInteractions(mockCallback); + } + + @Test + public void buildCleartext__withNoSigningKey__shouldBuildTrivialMessage() { cryptoStatusBuilder.setCryptoMode(CryptoMode.NO_CHOICE); cryptoStatusBuilder.setOpenPgpKeyId(null); pgpMessageBuilder.setCryptoStatus(cryptoStatusBuilder.build()); Callback mockCallback = mock(Callback.class); pgpMessageBuilder.buildAsync(mockCallback); + + ArgumentCaptor captor = ArgumentCaptor.forClass(MimeMessage.class); + verify(mockCallback).onMessageBuildSuccess(captor.capture(), eq(false)); + verifyNoMoreInteractions(mockCallback); + + MimeMessage message = captor.getValue(); + assertEquals("text/plain", message.getMimeType()); } @Test