From 055fb59ec34a986ea9f5620554b51c438d809361 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Mon, 8 Sep 2014 14:32:55 +0200 Subject: Properly handle uneditable key types in EditKeyFragment More specifically, return an OperationResultParcel for stripped and divert-to-card keys, with an appropriate error message in the log. part of #828 --- .../keychain/pgp/PgpKeyOperation.java | 4 ++ .../keychain/service/OperationResultParcel.java | 6 +++ .../keychain/ui/EditKeyFragment.java | 48 ++++++++++++++++++---- OpenKeychain/src/main/res/values/strings.xml | 5 +++ 4 files changed, 54 insertions(+), 9 deletions(-) (limited to 'OpenKeychain') diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java index ddb00305a..3f5528b05 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java @@ -381,6 +381,10 @@ public class PgpKeyOperation { * This is a natural workflow since pgp keyrings are immutable data structures: Old semantics * are changed by adding new certificates, which implicitly override older certificates. * + * Note that this method does not care about any "special" type of master key. If unlocking + * with a passphrase fails, the operation will fail with an unlocking error. More specific + * handling of errors should be done in UI code! + * */ public EditKeyResult modifySecretKeyRing(CanonicalizedSecretKeyRing wsKR, SaveKeyringParcel saveParcel, String passphrase) { diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java index 438443644..fefc3e4b1 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java @@ -427,6 +427,12 @@ public class OperationResultParcel implements Parcelable { MSG_CON_SUCCESS (R.string.msg_con_success), MSG_CON_WARN_DELETE_PUBLIC (R.string.msg_con_warn_delete_public), MSG_CON_WARN_DELETE_SECRET (R.string.msg_con_warn_delete_secret), + + // messages used in UI code + MSG_EK_ERROR_DIVERT (R.string.msg_ek_error_divert), + MSG_EK_ERROR_DUMMY (R.string.msg_ek_error_dummy), + MSG_EK_ERROR_NOT_FOUND (R.string.msg_ek_error_not_found), + ; private final int mMsgId; diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyFragment.java index 830e856aa..3de01adcd 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyFragment.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyFragment.java @@ -35,7 +35,6 @@ import android.view.View.OnClickListener; import android.view.ViewGroup; import android.widget.AdapterView; import android.widget.ListView; -import android.widget.Toast; import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.R; @@ -46,9 +45,13 @@ import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException; import org.sufficientlysecure.keychain.provider.CachedPublicKeyRing; import org.sufficientlysecure.keychain.provider.KeychainContract; import org.sufficientlysecure.keychain.provider.ProviderHelper; +import org.sufficientlysecure.keychain.provider.ProviderHelper.NotFoundException; import org.sufficientlysecure.keychain.service.KeychainIntentService; import org.sufficientlysecure.keychain.service.KeychainIntentServiceHandler; import org.sufficientlysecure.keychain.service.OperationResultParcel; +import org.sufficientlysecure.keychain.service.OperationResultParcel.LogLevel; +import org.sufficientlysecure.keychain.service.OperationResultParcel.LogType; +import org.sufficientlysecure.keychain.service.OperationResultParcel.OperationLog; import org.sufficientlysecure.keychain.service.PassphraseCacheService; import org.sufficientlysecure.keychain.service.SaveKeyringParcel; import org.sufficientlysecure.keychain.ui.adapter.SubkeysAdapter; @@ -169,15 +172,26 @@ public class EditKeyFragment extends LoaderFragment implements Uri secretUri = KeychainContract.KeyRings.buildUnifiedKeyRingUri(mDataUri); CachedPublicKeyRing keyRing = new ProviderHelper(getActivity()).getCachedPublicKeyRing(secretUri); + long masterKeyId = keyRing.getMasterKeyId(); + + // check if this is a master secret key we can work with + switch (keyRing.getSecretKeyType(masterKeyId)) { + case GNU_DUMMY: + finishWithError(LogType.MSG_EK_ERROR_DUMMY); + return; + case DIVERT_TO_CARD: + finishWithError(LogType.MSG_EK_ERROR_DIVERT); + break; + } - mSaveKeyringParcel = new SaveKeyringParcel(keyRing.getMasterKeyId(), - keyRing.getFingerprint()); + mSaveKeyringParcel = new SaveKeyringParcel(masterKeyId, keyRing.getFingerprint()); mPrimaryUserId = keyRing.getPrimaryUserIdWithFallback(); + } catch (NotFoundException e) { + finishWithError(LogType.MSG_EK_ERROR_NOT_FOUND); + return; } catch (PgpGeneralException e) { - Log.e(Constants.TAG, "PgpGeneralException", e); - Toast.makeText(getActivity(), R.string.error_no_secret_key_found, Toast.LENGTH_SHORT).show(); - getActivity().finish(); + finishWithError(LogType.MSG_EK_ERROR_NOT_FOUND); return; } @@ -185,9 +199,7 @@ public class EditKeyFragment extends LoaderFragment implements mCurrentPassphrase = PassphraseCacheService.getCachedPassphrase(getActivity(), mSaveKeyringParcel.mMasterKeyId); } catch (PassphraseCacheService.KeyNotFoundException e) { - Log.e(Constants.TAG, "Key not found!", e); - Toast.makeText(getActivity(), R.string.error_no_secret_key_found, Toast.LENGTH_SHORT).show(); - getActivity().finish(); + finishWithError(LogType.MSG_EK_ERROR_NOT_FOUND); return; } @@ -564,4 +576,22 @@ public class EditKeyFragment extends LoaderFragment implements getActivity().startService(intent); } + + /** Closes this activity, returning a result parcel with a single error log entry. */ + void finishWithError(LogType reason) { + + // Prepare the log + OperationLog log = new OperationLog(); + log.add(LogLevel.ERROR, reason, 0); + + // Prepare an intent with an EXTRA_RESULT + Intent intent = new Intent(); + intent.putExtra(OperationResultParcel.EXTRA_RESULT, + new OperationResultParcel(OperationResultParcel.RESULT_ERROR, log)); + + // Finish with result + getActivity().setResult(EditKeyActivity.RESULT_OK, intent); + getActivity().finish(); + } + } diff --git a/OpenKeychain/src/main/res/values/strings.xml b/OpenKeychain/src/main/res/values/strings.xml index 24effb7f1..2a89f15bc 100644 --- a/OpenKeychain/src/main/res/values/strings.xml +++ b/OpenKeychain/src/main/res/values/strings.xml @@ -756,6 +756,11 @@ "Exception deleting public cache file" "Exception deleting secret cache file" + + "editing of nfc keys is not (yet) supported" + "cannot edit keyring with stripped master key!" + "key not found" + "Click to clear cached passphrases" "OpenKeychain has cached %d passphrases" -- cgit v1.2.3 From ceea2667e8544b58acfc59f8d3284f4dcbd7cf7a Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Mon, 8 Sep 2014 14:50:16 +0200 Subject: introduce SingletonResult for... singleton results --- .../keychain/service/OperationResults.java | 58 +++++++++++++++++++++- .../keychain/ui/EditKeyFragment.java | 8 +-- OpenKeychain/src/main/res/values/strings.xml | 6 +-- 3 files changed, 62 insertions(+), 10 deletions(-) (limited to 'OpenKeychain') diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResults.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResults.java index 822c069cc..ad8fb2f7a 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResults.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResults.java @@ -33,13 +33,69 @@ import com.github.johnpersano.supertoasts.util.Style; import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.R; import org.sufficientlysecure.keychain.pgp.CanonicalizedKeyRing; -import org.sufficientlysecure.keychain.pgp.KeyRing; import org.sufficientlysecure.keychain.pgp.UncachedKeyRing; import org.sufficientlysecure.keychain.ui.LogDisplayActivity; import org.sufficientlysecure.keychain.ui.LogDisplayFragment; public abstract class OperationResults { + /** This is a simple subclass meant to contain only a single log message. This log + * message is also shown without a log button in the createNotify SuperToast. */ + public static class SingletonResult extends OperationResultParcel { + + /** Construct from a parcel - trivial because we have no extra data. */ + public SingletonResult(Parcel source) { + super(source); + } + + public SingletonResult(int result, LogLevel level, LogType reason) { + super(result, new OperationLog()); + // Prepare the log + mLog.add(level, reason, 0); + } + + @Override + public SuperCardToast createNotify(final Activity activity) { + + // there is exactly one error msg - use that one + String str = activity.getString(mLog.iterator().next().mType.getMsgId()); + int color; + + // Determine color by result type + if (cancelled()) { + color = Style.RED; + } else if (success()) { + if (getLog().containsWarnings()) { + color = Style.ORANGE; + } else { + color = Style.GREEN; + } + } else { + color = Style.RED; + } + + SuperCardToast toast = new SuperCardToast(activity, SuperToast.Type.STANDARD, + Style.getStyle(color, SuperToast.Animations.POPUP)); + toast.setText(str); + toast.setDuration(SuperToast.Duration.EXTRA_LONG); + toast.setIndeterminate(false); + toast.setSwipeToDismiss(true); + return toast; + + } + + public static Creator CREATOR = new Creator() { + public SingletonResult createFromParcel(final Parcel source) { + return new SingletonResult(source); + } + + public SingletonResult[] newArray(final int size) { + return new SingletonResult[size]; + } + }; + + } + public static class ImportKeyResult extends OperationResultParcel { public final int mNewKeys, mUpdatedKeys, mBadKeys, mSecret; diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyFragment.java index 3de01adcd..703755457 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyFragment.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyFragment.java @@ -51,7 +51,7 @@ import org.sufficientlysecure.keychain.service.KeychainIntentServiceHandler; import org.sufficientlysecure.keychain.service.OperationResultParcel; import org.sufficientlysecure.keychain.service.OperationResultParcel.LogLevel; import org.sufficientlysecure.keychain.service.OperationResultParcel.LogType; -import org.sufficientlysecure.keychain.service.OperationResultParcel.OperationLog; +import org.sufficientlysecure.keychain.service.OperationResults.SingletonResult; import org.sufficientlysecure.keychain.service.PassphraseCacheService; import org.sufficientlysecure.keychain.service.SaveKeyringParcel; import org.sufficientlysecure.keychain.ui.adapter.SubkeysAdapter; @@ -580,14 +580,10 @@ public class EditKeyFragment extends LoaderFragment implements /** Closes this activity, returning a result parcel with a single error log entry. */ void finishWithError(LogType reason) { - // Prepare the log - OperationLog log = new OperationLog(); - log.add(LogLevel.ERROR, reason, 0); - // Prepare an intent with an EXTRA_RESULT Intent intent = new Intent(); intent.putExtra(OperationResultParcel.EXTRA_RESULT, - new OperationResultParcel(OperationResultParcel.RESULT_ERROR, log)); + new SingletonResult(SingletonResult.RESULT_ERROR, LogLevel.ERROR, reason)); // Finish with result getActivity().setResult(EditKeyActivity.RESULT_OK, intent); diff --git a/OpenKeychain/src/main/res/values/strings.xml b/OpenKeychain/src/main/res/values/strings.xml index 2a89f15bc..64affdfde 100644 --- a/OpenKeychain/src/main/res/values/strings.xml +++ b/OpenKeychain/src/main/res/values/strings.xml @@ -757,9 +757,9 @@ "Exception deleting secret cache file" - "editing of nfc keys is not (yet) supported" - "cannot edit keyring with stripped master key!" - "key not found" + "Editing of nfc keys is not (yet) supported!" + "Cannot edit keyring with stripped master key!" + "Key not found!" "Click to clear cached passphrases" -- cgit v1.2.3