Merge pull request #2796 from k9mail/compose-error-without-key
Show error if encryption is enabled but no key is configured
This commit is contained in:
commit
7a9ea6c885
6 changed files with 85 additions and 36 deletions
|
@ -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;
|
||||
}
|
||||
|
||||
|
|
|
@ -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() {
|
||||
|
|
|
@ -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()) {
|
||||
|
|
|
@ -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];
|
||||
|
|
|
@ -1146,7 +1146,7 @@ Please submit bug reports, contribute new features and ask questions at
|
|||
<string name="address_type_other">Other</string>
|
||||
<string name="address_type_mobile">Mobile</string>
|
||||
<string name="compose_error_no_draft_folder">No Drafts folder configured for this account!</string>
|
||||
<string name="compose_error_no_signing_key">No key configured for signing! Please check your settings.</string>
|
||||
<string name="compose_error_no_key_configured">No key configured for this account! Check your settings.</string>
|
||||
<string name="crypto_mode_disabled">Don\'t encrypt</string>
|
||||
<string name="crypto_mode_opportunistic">Encrypt if possible</string>
|
||||
<string name="crypto_mode_private">Encrypt</string>
|
||||
|
|
|
@ -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<MimeMessage> captor = ArgumentCaptor.forClass(MimeMessage.class);
|
||||
verify(mockCallback).onMessageBuildSuccess(captor.capture(), eq(false));
|
||||
verifyNoMoreInteractions(mockCallback);
|
||||
|
||||
MimeMessage message = captor.getValue();
|
||||
assertEquals("text/plain", message.getMimeType());
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
Loading…
Reference in a new issue