From 1eb438576d862df8bf8c7221a6bebee7e8dd85f3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dominik=20Sch=C3=BCrmann?= <dominik@dominikschuermann.de>
Date: Mon, 6 Jul 2015 16:21:48 +0200
Subject: Better YubiKey UX flow for error handling

---
 .../keychain/ui/NfcOperationActivity.java          |  29 +++-
 .../keychain/ui/base/BaseNfcActivity.java          |  23 ++-
 OpenKeychain/src/main/res/layout/nfc_activity.xml  | 158 ------------------
 .../src/main/res/layout/nfc_operation_activity.xml | 179 +++++++++++++++++++++
 OpenKeychain/src/main/res/values/strings.xml       |  30 ++--
 5 files changed, 240 insertions(+), 179 deletions(-)
 delete mode 100644 OpenKeychain/src/main/res/layout/nfc_activity.xml
 create mode 100644 OpenKeychain/src/main/res/layout/nfc_operation_activity.xml

(limited to 'OpenKeychain')

diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/NfcOperationActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/NfcOperationActivity.java
index cacfd2e52..8802cd621 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/NfcOperationActivity.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/NfcOperationActivity.java
@@ -9,7 +9,9 @@ package org.sufficientlysecure.keychain.ui;
 import android.content.Intent;
 import android.os.AsyncTask;
 import android.os.Bundle;
+import android.view.View;
 import android.view.WindowManager;
+import android.widget.Button;
 import android.widget.TextView;
 import android.widget.ViewAnimator;
 
@@ -49,6 +51,7 @@ public class NfcOperationActivity extends BaseNfcActivity {
 
     public ViewAnimator vAnimator;
     public TextView vErrorText;
+    public Button vErrorTryAgainButton;
 
     private RequiredInputParcel mRequiredInput;
     private Intent mServiceIntent;
@@ -68,7 +71,20 @@ public class NfcOperationActivity extends BaseNfcActivity {
 
         vAnimator = (ViewAnimator) findViewById(R.id.view_animator);
         vAnimator.setDisplayedChild(0);
-        vErrorText = (TextView) findViewById(R.id.nfc_activity_error_text);
+        vErrorText = (TextView) findViewById(R.id.nfc_activity_3_error_text);
+        vErrorTryAgainButton = (Button) findViewById(R.id.nfc_activity_3_error_try_again);
+        vErrorTryAgainButton.setOnClickListener(new View.OnClickListener() {
+            @Override
+            public void onClick(View v) {
+                resumeTagHandling();
+
+                // obtain passphrase for this subkey
+                if (mRequiredInput.mType != RequiredInputParcel.RequiredInputType.NFC_MOVE_KEY_TO_CARD) {
+                    obtainYubiKeyPin(mRequiredInput);
+                }
+                vAnimator.setDisplayedChild(0);
+            }
+        });
 
         Intent intent = getIntent();
         Bundle data = intent.getExtras();
@@ -84,7 +100,7 @@ public class NfcOperationActivity extends BaseNfcActivity {
 
     @Override
     protected void initLayout() {
-        setContentView(R.layout.nfc_activity);
+        setContentView(R.layout.nfc_operation_activity);
     }
 
     @Override
@@ -221,8 +237,7 @@ public class NfcOperationActivity extends BaseNfcActivity {
                     if (isNfcConnected()) {
                         try {
                             Thread.sleep(200);
-                        } catch (InterruptedException e) {
-                            // never mind
+                        } catch (InterruptedException ignored) {
                         }
                     } else {
                         return null;
@@ -239,7 +254,9 @@ public class NfcOperationActivity extends BaseNfcActivity {
 
     @Override
     protected void onNfcError(String error) {
-        vErrorText.setText(error);
+        pauseTagHandling();
+
+        vErrorText.setText(error + "\n\n" + getString(R.string.nfc_try_again_text));
         vAnimator.setDisplayedChild(3);
     }
 
@@ -270,8 +287,6 @@ public class NfcOperationActivity extends BaseNfcActivity {
         // clear (invalid) passphrase
         PassphraseCacheService.clearCachedPassphrase(
                 this, mRequiredInput.getMasterKeyId(), mRequiredInput.getSubKeyId());
-
-        obtainYubiKeyPin(mRequiredInput);
     }
 
 }
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/base/BaseNfcActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/base/BaseNfcActivity.java
index 02e8271cb..65f8a451c 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/base/BaseNfcActivity.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/base/BaseNfcActivity.java
@@ -29,6 +29,7 @@ import android.content.Intent;
 import android.content.IntentFilter;
 import android.nfc.NfcAdapter;
 import android.nfc.Tag;
+import android.nfc.TagLostException;
 import android.nfc.tech.IsoDep;
 import android.os.AsyncTask;
 import android.os.Bundle;
@@ -72,6 +73,7 @@ public abstract class BaseNfcActivity extends BaseActivity {
     protected boolean mPw3Validated;
     private NfcAdapter mNfcAdapter;
     private IsoDep mIsoDep;
+    private boolean mTagHandlingEnabled;
 
     private static final int TIMEOUT = 100000;
 
@@ -168,10 +170,20 @@ public abstract class BaseNfcActivity extends BaseActivity {
         }.execute();
     }
 
+    protected void pauseTagHandling() {
+        mTagHandlingEnabled = false;
+    }
+
+    protected void resumeTagHandling() {
+        mTagHandlingEnabled = true;
+    }
+
     @Override
     protected void onCreate(Bundle savedInstanceState) {
         super.onCreate(savedInstanceState);
 
+        mTagHandlingEnabled = true;
+
         Intent intent = getIntent();
         String action = intent.getAction();
         if (NfcAdapter.ACTION_TAG_DISCOVERED.equals(action)) {
@@ -186,7 +198,8 @@ public abstract class BaseNfcActivity extends BaseActivity {
      */
     @Override
     public void onNewIntent(final Intent intent) {
-        if (NfcAdapter.ACTION_TAG_DISCOVERED.equals(intent.getAction())) {
+        if (NfcAdapter.ACTION_TAG_DISCOVERED.equals(intent.getAction())
+                && mTagHandlingEnabled) {
             handleIntentInBackground(intent);
         }
     }
@@ -194,6 +207,11 @@ public abstract class BaseNfcActivity extends BaseActivity {
     private void handleNfcError(Exception e) {
         Log.e(Constants.TAG, "nfc error", e);
 
+        if (e instanceof TagLostException) {
+            onNfcError(getString(R.string.error_nfc_tag_lost));
+            return;
+        }
+
         short status;
         if (e instanceof CardException) {
             status = ((CardException) e).getResponseCode();
@@ -202,7 +220,8 @@ public abstract class BaseNfcActivity extends BaseActivity {
         }
         // When entering a PIN, a status of 63CX indicates X attempts remaining.
         if ((status & (short)0xFFF0) == 0x63C0) {
-            onNfcError(getString(R.string.error_pin, status & 0x000F));
+            int tries = status & 0x000F;
+            onNfcError(getResources().getQuantityString(R.plurals.error_pin, tries, tries));
             return;
         }
 
diff --git a/OpenKeychain/src/main/res/layout/nfc_activity.xml b/OpenKeychain/src/main/res/layout/nfc_activity.xml
deleted file mode 100644
index 992f8c14d..000000000
--- a/OpenKeychain/src/main/res/layout/nfc_activity.xml
+++ /dev/null
@@ -1,158 +0,0 @@
-<?xml version="1.0" encoding="utf-8"?>
-<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
-    xmlns:custom="http://schemas.android.com/apk/res-auto"
-    xmlns:tools="http://schemas.android.com/tools"
-    android:layout_width="match_parent"
-    android:layout_height="wrap_content">
-
-    <org.sufficientlysecure.keychain.ui.widget.ToolableViewAnimator
-        android:id="@+id/view_animator"
-        android:layout_width="match_parent"
-        android:layout_height="match_parent"
-        android:animateLayoutChanges="true"
-        android:inAnimation="@anim/fade_in"
-        android:measureAllChildren="false"
-        android:minHeight="?listPreferredItemHeightSmall"
-        android:outAnimation="@anim/fade_out"
-        android:paddingBottom="16dp"
-        android:paddingLeft="24dp"
-        android:paddingRight="24dp"
-        android:paddingTop="16dp"
-        custom:initialView="3">
-
-        <RelativeLayout
-            android:layout_width="match_parent"
-            android:layout_height="wrap_content">
-
-            <TextView
-                android:id="@+id/nfc_activity_text"
-                android:layout_width="wrap_content"
-                android:layout_height="wrap_content"
-                android:text="@string/nfc_text"
-                android:textAppearance="@android:style/TextAppearance.Medium" />
-
-            <ImageView
-                android:id="@+id/nfc_image"
-                android:layout_width="wrap_content"
-                android:layout_height="wrap_content"
-                android:layout_alignParentLeft="true"
-                android:layout_alignParentStart="true"
-                android:layout_below="@+id/nfc_activity_text"
-                android:layout_marginTop="8dp"
-                android:adjustViewBounds="true"
-                android:background="@android:color/transparent"
-                android:src="@drawable/yubikey_phone" />
-        </RelativeLayout>
-
-        <RelativeLayout
-            android:layout_width="match_parent"
-            android:layout_height="wrap_content">
-
-            <TextView
-                android:id="@+id/nfc_activity_text2"
-                android:layout_width="wrap_content"
-                android:layout_height="wrap_content"
-                android:text="@string/nfc_wait"
-                android:textAppearance="@android:style/TextAppearance.Medium" />
-
-            <ProgressBar
-                style="?android:attr/progressBarStyleLarge"
-                android:layout_width="wrap_content"
-                android:layout_height="wrap_content"
-                android:layout_centerHorizontal="true"
-                android:layout_centerVertical="true"
-                android:padding="8dp" />
-
-            <!-- placeholder to retain dialog size -->
-            <ImageView
-                android:id="@+id/nfc_image_placeholder1"
-                android:layout_width="wrap_content"
-                android:layout_height="wrap_content"
-                android:layout_alignParentLeft="true"
-                android:layout_alignParentStart="true"
-                android:layout_below="@+id/nfc_activity_text2"
-                android:layout_marginTop="8dp"
-                android:adjustViewBounds="true"
-                android:background="@android:color/transparent"
-                android:src="@drawable/yubikey_phone"
-                android:visibility="invisible" />
-
-        </RelativeLayout>
-
-        <RelativeLayout
-            android:layout_width="match_parent"
-            android:layout_height="wrap_content">
-
-            <TextView
-                android:id="@+id/nfc_activity_text3"
-                android:layout_width="wrap_content"
-                android:layout_height="wrap_content"
-                android:text="@string/nfc_finished"
-                android:textAppearance="@android:style/TextAppearance.Medium" />
-
-            <!-- placeholder to retain dialog size -->
-            <ImageView
-                android:id="@+id/nfc_image_placeholder2"
-                android:layout_width="wrap_content"
-                android:layout_height="wrap_content"
-                android:layout_alignParentLeft="true"
-                android:layout_alignParentStart="true"
-                android:layout_below="@+id/nfc_activity_text3"
-                android:layout_marginTop="8dp"
-                android:adjustViewBounds="true"
-                android:background="@android:color/transparent"
-                android:src="@drawable/yubikey_phone"
-                android:visibility="invisible" />
-
-            <ImageView
-                android:layout_width="wrap_content"
-                android:layout_height="wrap_content"
-                android:layout_centerHorizontal="true"
-                android:layout_centerVertical="true"
-                android:adjustViewBounds="true"
-                android:background="@android:color/transparent"
-                android:src="@drawable/ic_check_circle_black_48dp" />
-
-        </RelativeLayout>
-
-        <RelativeLayout
-            android:layout_width="match_parent"
-            android:layout_height="wrap_content">
-
-            <TextView
-                android:id="@+id/nfc_activity_text_placeholder"
-                android:layout_width="wrap_content"
-                android:layout_height="wrap_content"
-                android:text=""
-                android:textAppearance="@android:style/TextAppearance.Medium" />
-
-            <!-- placeholder to retain dialog size -->
-            <ImageView
-                android:id="@+id/nfc_image_placeholder3"
-                android:layout_width="wrap_content"
-                android:layout_height="wrap_content"
-                android:layout_alignParentLeft="true"
-                android:layout_alignParentStart="true"
-                android:layout_below="@+id/nfc_activity_text_placeholder"
-                android:layout_marginTop="8dp"
-                android:adjustViewBounds="true"
-                android:background="@android:color/transparent"
-                android:src="@drawable/yubikey_phone"
-                android:visibility="invisible" />
-
-            <TextView
-                android:id="@+id/nfc_activity_error_text"
-                android:layout_width="wrap_content"
-                android:layout_height="wrap_content"
-                android:layout_centerHorizontal="true"
-                android:layout_centerVertical="true"
-                android:textAppearance="@android:style/TextAppearance.Medium"
-                android:textColor="@color/android_red_dark"
-                tools:text="Error text" />
-
-
-        </RelativeLayout>
-
-    </org.sufficientlysecure.keychain.ui.widget.ToolableViewAnimator>
-
-</LinearLayout>
\ No newline at end of file
diff --git a/OpenKeychain/src/main/res/layout/nfc_operation_activity.xml b/OpenKeychain/src/main/res/layout/nfc_operation_activity.xml
new file mode 100644
index 000000000..8b043861f
--- /dev/null
+++ b/OpenKeychain/src/main/res/layout/nfc_operation_activity.xml
@@ -0,0 +1,179 @@
+<?xml version="1.0" encoding="utf-8"?>
+<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
+    xmlns:custom="http://schemas.android.com/apk/res-auto"
+    xmlns:tools="http://schemas.android.com/tools"
+    android:layout_width="match_parent"
+    android:layout_height="wrap_content">
+
+    <org.sufficientlysecure.keychain.ui.widget.ToolableViewAnimator
+        android:id="@+id/view_animator"
+        android:layout_width="match_parent"
+        android:layout_height="match_parent"
+        android:animateLayoutChanges="true"
+        android:inAnimation="@anim/fade_in"
+        android:measureAllChildren="false"
+        android:minHeight="?listPreferredItemHeightSmall"
+        android:outAnimation="@anim/fade_out"
+        custom:initialView="3">
+
+        <RelativeLayout
+            android:layout_width="match_parent"
+            android:layout_height="wrap_content">
+
+            <TextView
+                android:id="@+id/nfc_activity_0_text"
+                android:layout_width="wrap_content"
+                android:layout_height="wrap_content"
+                android:layout_marginLeft="24dp"
+                android:layout_marginRight="24dp"
+                android:layout_marginTop="24dp"
+                android:text="@string/nfc_text"
+                android:textAppearance="@android:style/TextAppearance.Medium" />
+
+            <ImageView
+                android:id="@+id/nfc_activity_0_image"
+                android:layout_width="wrap_content"
+                android:layout_height="wrap_content"
+                android:layout_alignParentLeft="true"
+                android:layout_alignParentStart="true"
+                android:layout_below="@+id/nfc_activity_0_text"
+                android:layout_margin="24dp"
+                android:adjustViewBounds="true"
+                android:background="@android:color/transparent"
+                android:src="@drawable/yubikey_phone" />
+        </RelativeLayout>
+
+        <RelativeLayout
+            android:layout_width="match_parent"
+            android:layout_height="wrap_content">
+
+            <TextView
+                android:id="@+id/nfc_activity_1_text"
+                android:layout_width="wrap_content"
+                android:layout_height="wrap_content"
+                android:layout_marginLeft="24dp"
+                android:layout_marginRight="24dp"
+                android:layout_marginTop="24dp"
+                android:text="@string/nfc_wait"
+                android:textAppearance="@android:style/TextAppearance.Medium" />
+
+            <ProgressBar
+                style="?android:attr/progressBarStyleLarge"
+                android:layout_width="wrap_content"
+                android:layout_height="wrap_content"
+                android:layout_centerHorizontal="true"
+                android:layout_centerVertical="true"
+                android:padding="8dp" />
+
+            <!-- placeholder to retain dialog size -->
+            <ImageView
+                android:id="@+id/nfc_activity_1_placeholder"
+                android:layout_width="wrap_content"
+                android:layout_height="wrap_content"
+                android:layout_alignParentLeft="true"
+                android:layout_alignParentStart="true"
+                android:layout_below="@+id/nfc_activity_1_text"
+                android:layout_margin="24dp"
+                android:adjustViewBounds="true"
+                android:background="@android:color/transparent"
+                android:src="@drawable/yubikey_phone"
+                android:visibility="invisible" />
+
+        </RelativeLayout>
+
+        <RelativeLayout
+            android:layout_width="match_parent"
+            android:layout_height="wrap_content">
+
+            <TextView
+                android:id="@+id/nfc_activity_2_text"
+                android:layout_width="wrap_content"
+                android:layout_height="wrap_content"
+                android:layout_marginLeft="24dp"
+                android:layout_marginRight="24dp"
+                android:layout_marginTop="24dp"
+                android:text="@string/nfc_finished"
+                android:textAppearance="@android:style/TextAppearance.Medium" />
+
+            <!-- placeholder to retain dialog size -->
+            <ImageView
+                android:id="@+id/nfc_activity_2_placeholder"
+                android:layout_width="wrap_content"
+                android:layout_height="wrap_content"
+                android:layout_alignParentLeft="true"
+                android:layout_alignParentStart="true"
+                android:layout_below="@+id/nfc_activity_2_text"
+                android:layout_margin="24dp"
+                android:adjustViewBounds="true"
+                android:background="@android:color/transparent"
+                android:src="@drawable/yubikey_phone"
+                android:visibility="invisible" />
+
+            <ImageView
+                android:layout_width="wrap_content"
+                android:layout_height="wrap_content"
+                android:layout_centerHorizontal="true"
+                android:layout_centerVertical="true"
+                android:adjustViewBounds="true"
+                android:background="@android:color/transparent"
+                android:src="@drawable/ic_check_circle_black_48dp" />
+
+        </RelativeLayout>
+
+        <RelativeLayout
+            android:layout_width="match_parent"
+            android:layout_height="wrap_content">
+
+            <TextView
+                android:id="@+id/nfc_activity_3_text_placeholder"
+                android:layout_width="wrap_content"
+                android:layout_height="wrap_content"
+                android:layout_alignEnd="@+id/nfc_activity_3_placeholder"
+                android:layout_alignRight="@+id/nfc_activity_3_placeholder"
+                android:layout_marginLeft="24dp"
+                android:layout_marginTop="24dp"
+                android:text=""
+                android:textAppearance="@android:style/TextAppearance.Medium" />
+
+            <!-- placeholder to retain dialog size -->
+            <ImageView
+                android:id="@+id/nfc_activity_3_placeholder"
+                android:layout_width="wrap_content"
+                android:layout_height="wrap_content"
+                android:layout_alignParentLeft="true"
+                android:layout_alignParentStart="true"
+                android:layout_below="@+id/nfc_activity_3_text_placeholder"
+                android:layout_margin="24dp"
+                android:adjustViewBounds="true"
+                android:background="@android:color/transparent"
+                android:src="@drawable/yubikey_phone"
+                android:visibility="invisible" />
+
+            <TextView
+                android:id="@+id/nfc_activity_3_error_text"
+                android:layout_width="wrap_content"
+                android:layout_height="wrap_content"
+                android:layout_centerHorizontal="true"
+                android:layout_centerVertical="true"
+                android:layout_margin="24dp"
+                android:textAppearance="@android:style/TextAppearance.Medium"
+                android:textColor="@color/android_red_dark"
+                tools:text="Error text" />
+
+            <Button
+                android:id="@+id/nfc_activity_3_error_try_again"
+                style="?android:attr/borderlessButtonStyle"
+                android:layout_width="wrap_content"
+                android:layout_height="48dp"
+                android:layout_alignParentEnd="true"
+                android:layout_alignParentRight="true"
+                android:layout_below="@+id/nfc_activity_3_placeholder"
+                android:layout_margin="8dp"
+                android:text="@string/error_nfc_try_again"
+                android:textColor="@color/accent" />
+
+        </RelativeLayout>
+
+    </org.sufficientlysecure.keychain.ui.widget.ToolableViewAnimator>
+
+</LinearLayout>
\ No newline at end of file
diff --git a/OpenKeychain/src/main/res/values/strings.xml b/OpenKeychain/src/main/res/values/strings.xml
index bccdb4f00..c3a887fd1 100644
--- a/OpenKeychain/src/main/res/values/strings.xml
+++ b/OpenKeychain/src/main/res/values/strings.xml
@@ -263,7 +263,8 @@
     <string name="yubikey_pin_for">"Enter PIN to access YubiKey for '%s'"</string>
     <string name="nfc_text">"Hold YubiKey against the NFC marker at the back of your device."</string>
     <string name="nfc_wait">"Keep the YubiKey at the back!"</string>
-    <string name="nfc_finished">"Please take away the YubiKey now."</string>
+    <string name="nfc_finished">"Take away the YubiKey now."</string>
+    <string name="nfc_try_again_text">"Take away the YubiKey now and press TRY AGAIN."</string>
     <string name="file_delete_confirmation_title">"Delete original files?"</string>
     <string name="file_delete_confirmation">"The following files will be deleted:%s"</string>
     <string name="file_delete_successful">"%1$d out of %2$d files have been deleted.%3$s"</string>
@@ -1388,17 +1389,22 @@
     <string name="btn_import">"Import"</string>
     <string name="snack_yubi_other">Different key stored on YubiKey!</string>
     <string name="error_nfc">"NFC Error: %s"</string>
-    <string name="error_pin">"NFC: Incorrect PIN; %d tries remaining."</string>
-    <string name="error_nfc_terminated">"NFC: Smart card in termination state"</string>
-    <string name="error_nfc_wrong_length">"NFC: Wrong length for sent / received data"</string>
-    <string name="error_nfc_conditions_not_satisfied">"NFC: Conditions of use not satisfied"</string>
-    <string name="error_nfc_security_not_satisfied">"NFC: Security status not satisfied"</string>
-    <string name="error_nfc_authentication_blocked">"NFC: PIN blocked after too many attempts"</string>
-    <string name="error_nfc_data_not_found">"NFC: Key or object not found"</string>
-    <string name="error_nfc_unknown">"NFC: Unknown Error"</string>
-    <string name="error_nfc_bad_data">"NFC: Card reported invalid data"</string>
-    <string name="error_nfc_chaining_error">"NFC: Card expected last command in a chain"</string>
-    <string name="error_nfc_header">"NFC: Card reported invalid %s byte"</string>
+    <plurals name="error_pin">
+        <item quantity="one">"Incorrect PIN!\n%d try remaining."</item>
+        <item quantity="other">"Incorrect PIN!\n%d tries remaining."</item>
+    </plurals>
+    <string name="error_nfc_terminated">"YubiKey in termination state"</string>
+    <string name="error_nfc_wrong_length">"Wrong length for sent / received data"</string>
+    <string name="error_nfc_conditions_not_satisfied">"Conditions of use not satisfied"</string>
+    <string name="error_nfc_security_not_satisfied">"Security status not satisfied"</string>
+    <string name="error_nfc_authentication_blocked">"PIN blocked after too many attempts"</string>
+    <string name="error_nfc_data_not_found">"Key or object not found"</string>
+    <string name="error_nfc_unknown">"Unknown Error"</string>
+    <string name="error_nfc_bad_data">"YubiKey reported invalid data"</string>
+    <string name="error_nfc_chaining_error">"YubiKey expected last command in a chain"</string>
+    <string name="error_nfc_header">"YubiKey reported invalid %s byte"</string>
+    <string name="error_nfc_tag_lost">"YubiKey has been taken off too early. Keep the YubiKey at the back until the operation finishes."</string>
+    <string name="error_nfc_try_again">"Try again"</string>
     <string name="error_pin_nodefault">Default PIN was rejected!</string>
     <string name="error_temp_file">Error creating temporary file.</string>
     <string name="btn_delete_original">Delete original file</string>
-- 
cgit v1.2.3