Merge pull request #91 from privacyidea/fix_2step_dialog

Fix 2step dialog
privacyidea · Dec 6, 2019 · 2d20fd6 · 2d20fd6
2 parents 85a6d4c + c2d8b25
commit 2d20fd6
Show file tree

Hide file tree

Showing 27 changed files with 144 additions and 111 deletions.
diff --git a/app/build.gradle b/app/build.gradle
@@ -12,7 +12,7 @@ android {
         def versionMajor = 2
         def versionMinor = 0
         def versionPatch = 3
-        def versionAdditive = 0
+        def versionAdditive = 1
 
         versionCode versionMajor * 10000 + versionMinor * 1000 + versionPatch * 100 + versionAdditive
         versionName "${versionMajor}.${versionMinor}.${versionPatch}b${versionAdditive}"
@@ -76,8 +76,11 @@ dependencies {
     testImplementation 'org.mockito:mockito-core:1.10.19'
     testImplementation 'org.json:json:20180813'
     testImplementation 'com.google.code.gson:gson:2.8.5'
-    androidTestImplementation 'com.squareup.okhttp3:mockwebserver:3.14.0'
 
+    /* Mock Server */
+    androidTestImplementation 'com.squareup.okhttp3:mockwebserver:4.2.1'
+    androidTestImplementation("com.squareup.okhttp3:okhttp-tls:4.2.1")
+    /* *********** */
     androidTestImplementation 'androidx.annotation:annotation:1.1.0'
     androidTestImplementation 'androidx.test:runner:1.2.0'
     androidTestImplementation 'androidx.test:rules:1.2.0'

diff --git a/app/src/androidTest/java/it/netknights/piauthenticator/AboutTest.java b/app/src/androidTest/java/it/netknights/piauthenticator/AboutTest.java
@@ -26,19 +26,19 @@
 import android.view.ViewGroup;
 import android.view.ViewParent;
 
+import androidx.test.espresso.DataInteraction;
+import androidx.test.espresso.ViewInteraction;
+import androidx.test.filters.LargeTest;
+import androidx.test.internal.runner.junit4.AndroidJUnit4ClassRunner;
+import androidx.test.rule.ActivityTestRule;
+
 import org.hamcrest.Description;
 import org.hamcrest.Matcher;
 import org.hamcrest.TypeSafeMatcher;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
-import androidx.test.espresso.DataInteraction;
-import androidx.test.espresso.ViewInteraction;
-import androidx.test.filters.LargeTest;
-import androidx.test.internal.runner.junit4.AndroidJUnit4ClassRunner;
-import androidx.test.rule.ActivityTestRule;
-
 import it.netknights.piauthenticator.utils.AppConstants;
 import it.netknights.piauthenticator.viewcontroller.MainActivity;
 
@@ -51,11 +51,11 @@
 import static androidx.test.espresso.matcher.ViewMatchers.withClassName;
 import static androidx.test.espresso.matcher.ViewMatchers.withId;
 import static androidx.test.espresso.matcher.ViewMatchers.withText;
+import static androidx.test.platform.app.InstrumentationRegistry.getInstrumentation;
 import static org.hamcrest.Matchers.allOf;
 import static org.hamcrest.Matchers.anything;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.startsWith;
-import static androidx.test.platform.app.InstrumentationRegistry.getInstrumentation;
 
 /**
  * Check that the AboutActivity displays the info labels and license correctly
@@ -177,14 +177,14 @@ public void aboutTest() {
         textView9.check(matches(withText("Apache License 2.0")));
 
         ViewInteraction textView10 = onView(
-                allOf(withId(R.id.about_row_title), withText("OTP Authenticator by Bruno Bierbaumer"),
+                allOf(withId(R.id.about_row_title), withText("OTP Authenticator"),
                         childAtPosition(
                                 childAtPosition(
                                         withId(R.id.listView_about),
                                         1),
                                 0),
                         isDisplayed()));
-        textView10.check(matches(withText("OTP Authenticator by Bruno Bierbaumer")));
+        textView10.check(matches(withText("OTP Authenticator")));
 
         ViewInteraction textView11 = onView(
                 allOf(withId(R.id.about_row_title), withText("ZXing Embedded"),

diff --git a/app/src/androidTest/java/it/netknights/piauthenticator/AnyStringMatcher.java b/app/src/androidTest/java/it/netknights/piauthenticator/AnyStringMatcher.java
@@ -23,10 +23,10 @@
 import android.view.View;
 import android.widget.TextView;
 
-import org.hamcrest.Description;
-
 import androidx.test.espresso.matcher.BoundedMatcher;
 
+import org.hamcrest.Description;
+
 public class AnyStringMatcher extends BoundedMatcher<View, TextView> {
 
     static AnyStringMatcher withAnyString() {

diff --git a/app/src/androidTest/java/it/netknights/piauthenticator/DetailAndMenuTest.java b/app/src/androidTest/java/it/netknights/piauthenticator/DetailAndMenuTest.java
@@ -24,6 +24,14 @@
 import android.view.ViewGroup;
 import android.view.ViewParent;
 
+import androidx.test.espresso.DataInteraction;
+import androidx.test.espresso.ViewInteraction;
+import androidx.test.espresso.matcher.ViewMatchers;
+import androidx.test.filters.LargeTest;
+import androidx.test.internal.runner.junit4.AndroidJUnit4ClassRunner;
+import androidx.test.platform.app.InstrumentationRegistry;
+import androidx.test.rule.ActivityTestRule;
+
 import org.hamcrest.Description;
 import org.hamcrest.Matcher;
 import org.hamcrest.TypeSafeMatcher;
@@ -33,14 +41,6 @@
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
-import androidx.test.espresso.DataInteraction;
-import androidx.test.espresso.ViewInteraction;
-import androidx.test.espresso.matcher.ViewMatchers;
-import androidx.test.filters.LargeTest;
-import androidx.test.internal.runner.junit4.AndroidJUnit4ClassRunner;
-import androidx.test.platform.app.InstrumentationRegistry;
-import androidx.test.rule.ActivityTestRule;
-
 import it.netknights.piauthenticator.viewcontroller.MainActivity;
 
 import static androidx.test.espresso.Espresso.onData;

diff --git a/app/src/androidTest/java/it/netknights/piauthenticator/NormalToken.java b/app/src/androidTest/java/it/netknights/piauthenticator/NormalToken.java
@@ -26,6 +26,12 @@
 import android.view.ViewGroup;
 import android.view.ViewParent;
 
+import androidx.test.espresso.ViewInteraction;
+import androidx.test.espresso.intent.Intents;
+import androidx.test.espresso.matcher.ViewMatchers;
+import androidx.test.internal.runner.junit4.AndroidJUnit4ClassRunner;
+import androidx.test.rule.ActivityTestRule;
+
 import org.hamcrest.Description;
 import org.hamcrest.Matcher;
 import org.hamcrest.Matchers;
@@ -37,12 +43,6 @@
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
-import androidx.test.espresso.ViewInteraction;
-import androidx.test.espresso.intent.Intents;
-import androidx.test.espresso.matcher.ViewMatchers;
-import androidx.test.internal.runner.junit4.AndroidJUnit4ClassRunner;
-import androidx.test.rule.ActivityTestRule;
-
 import it.netknights.piauthenticator.viewcontroller.MainActivity;
 
 import static androidx.test.espresso.Espresso.onData;
@@ -61,7 +61,6 @@
 import static androidx.test.espresso.matcher.ViewMatchers.withId;
 import static androidx.test.espresso.matcher.ViewMatchers.withText;
 import static it.netknights.piauthenticator.AnyStringMatcher.withAnyString;
-import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.CoreMatchers.not;
 import static org.hamcrest.Matchers.allOf;
 import static org.hamcrest.Matchers.anything;
@@ -194,9 +193,9 @@ public void twoStepQR() {
 
         // The result should be shown
         ViewInteraction textView = onView(
-                allOf(IsInstanceOf.instanceOf(android.widget.TextView.class), withText("Phone secret"),
+                allOf(IsInstanceOf.instanceOf(android.widget.TextView.class), withText(R.string.TwoStepFinishDialogTitle),
                         isDisplayed()));
-        textView.check(matches(withText("Phone secret")));
+        textView.check(matches(withText(R.string.TwoStepFinishDialogTitle)));
 
         ViewInteraction textView2 = onView(
                 allOf(withId(android.R.id.message), withAnyString(),

diff --git a/app/src/androidTest/java/it/netknights/piauthenticator/PushRolloutAuth.java b/app/src/androidTest/java/it/netknights/piauthenticator/PushRolloutAuth.java
@@ -27,6 +27,12 @@
 import android.view.ViewGroup;
 import android.view.ViewParent;
 
+import androidx.test.espresso.ViewInteraction;
+import androidx.test.espresso.intent.Intents;
+import androidx.test.espresso.matcher.ViewMatchers;
+import androidx.test.internal.runner.junit4.AndroidJUnit4ClassRunner;
+import androidx.test.rule.ActivityTestRule;
+
 import org.hamcrest.Description;
 import org.hamcrest.Matcher;
 import org.hamcrest.TypeSafeMatcher;
@@ -46,17 +52,13 @@
 import java.security.spec.InvalidKeySpecException;
 import java.security.spec.PKCS8EncodedKeySpec;
 
-import androidx.test.espresso.ViewInteraction;
-import androidx.test.espresso.intent.Intents;
-import androidx.test.espresso.matcher.ViewMatchers;
-import androidx.test.internal.runner.junit4.AndroidJUnit4ClassRunner;
-import androidx.test.rule.ActivityTestRule;
-
 import it.netknights.piauthenticator.utils.PKCS1ToSubjectPublicKeyInfo;
 import it.netknights.piauthenticator.utils.Util;
 import it.netknights.piauthenticator.viewcontroller.MainActivity;
 import okhttp3.mockwebserver.MockResponse;
 import okhttp3.mockwebserver.MockWebServer;
+import okhttp3.tls.HandshakeCertificates;
+import okhttp3.tls.internal.TlsUtil;
 
 import static androidx.test.espresso.Espresso.onView;
 import static androidx.test.espresso.action.ViewActions.click;
@@ -95,11 +97,13 @@ public class PushRolloutAuth {
     @Before
     public void setup() throws IOException, NoSuchAlgorithmException, InvalidKeySpecException {
         server = new MockWebServer();
-
+        // Setup 'https'
+        HandshakeCertificates hsc = TlsUtil.localhost();
+        server.useHttps(hsc.sslSocketFactory(), false);
 
         server.start();
-        url = server.url("/ttype/push").toString();
 
+        url = server.url("/ttype/push").toString();
 
         privateKeyServer = KeyFactory.getInstance("RSA").generatePrivate(
                 new PKCS8EncodedKeySpec(
@@ -151,6 +155,7 @@ public void pushQR() throws NoSuchAlgorithmException, InvalidKeyException, Signa
                                         withClassName(is("android.widget.ScrollView")),
                                         0),
                                 3)));
+        sleep(5000);
         appCompatButton3.perform(scrollTo(), click());
 
         // Validate token is there - but unfinished

diff --git a/app/src/androidTest/java/it/netknights/piauthenticator/ToastMatcher.java b/app/src/androidTest/java/it/netknights/piauthenticator/ToastMatcher.java
@@ -21,11 +21,11 @@
 
 import android.view.WindowManager;
 
+import androidx.test.espresso.Root;
+
 import org.hamcrest.Description;
 import org.hamcrest.TypeSafeMatcher;
 
-import androidx.test.espresso.Root;
-
 public class ToastMatcher extends TypeSafeMatcher<Root> {
     @Override
     protected boolean matchesSafely(Root root) {

diff --git a/app/src/main/java/it/netknights/piauthenticator/interfaces/MainActivityInterface.java b/app/src/main/java/it/netknights/piauthenticator/interfaces/MainActivityInterface.java
@@ -22,25 +22,26 @@
 
 import android.content.DialogInterface;
 
+import androidx.annotation.Nullable;
+
 import java.security.PublicKey;
 
 import it.netknights.piauthenticator.model.FirebaseInitConfig;
 import it.netknights.piauthenticator.model.Token;
 import it.netknights.piauthenticator.utils.SecretKeyWrapper;
 
 public interface MainActivityInterface {
-    void makeAlertDialog(String title, String message, String positiveBtnText, boolean cancelable,
-                         DialogInterface.OnClickListener positiveBtnListener);
-
-    void makeAlertDialog(int titleID, int messageID, int positiveBtnTextID, boolean cancelable,
-                         DialogInterface.OnClickListener positiveBtnListener);
-
-    void makeAlertDialog(String title, String message);
 
     void makeAlertDialog(int titleID, String message);
 
     void makeAlertDialog(int titleID, int messageID);
 
+    void makeAlertDialog(int titleID, String message, int positiveBtnTextID, boolean cancelable,
+                         @Nullable DialogInterface.OnClickListener positiveBtnListener);
+
+    void makeAlertDialog(int titleID, int messageID, int positiveBtnTextID, boolean cancelable,
+                         @Nullable DialogInterface.OnClickListener positiveBtnListener);
+
     void makeDeviceNotSupportedDialog();
 
     void makeToast(String message);

diff --git a/app/src/main/java/it/netknights/piauthenticator/interfaces/PresenterTaskInterface.java b/app/src/main/java/it/netknights/piauthenticator/interfaces/PresenterTaskInterface.java
@@ -27,7 +27,7 @@
 public interface PresenterTaskInterface {
     void updateTaskStatus(int statusCode, Token token);
 
-    void makeAlertDialog(String title, String message);
+    void twoStepFinished(String messageToShow);
 
     PublicKey generatePublicKeyFor(String serial);
 

diff --git a/app/src/main/java/it/netknights/piauthenticator/interfaces/PushAuthCallbackInterface.java b/app/src/main/java/it/netknights/piauthenticator/interfaces/PushAuthCallbackInterface.java
@@ -25,5 +25,5 @@
 public interface PushAuthCallbackInterface {
     void authenticationFinished(boolean success, Token token);
 
-    void handleError(int statusCode, Token token);
+    void handleAuthError(int statusCode, Token token);
 }
diff --git a/app/src/main/java/it/netknights/piauthenticator/presenter/Presenter.java b/app/src/main/java/it/netknights/piauthenticator/presenter/Presenter.java
@@ -515,10 +515,10 @@ public void timerProgress(int progress) {
         if (progress < 3 || progress > 27 && progress < 33 || progress > 57) {
             refreshOTPs();
         }
-        // Check for expired pendingAuths every 30s
-        if (progress == 30 || progress == 0) {
+        // Check for expired pendingAuths every 5s
+        if (progress % 5 == 0) {
+            checkForExpiredAuths();
         }
-        checkForExpiredAuths();
     }
 
     private void checkForExpiredAuths() {
@@ -689,8 +689,9 @@ public void updateTaskStatus(int statusCode, Token token) {
     }
 
     @Override
-    public void makeAlertDialog(String title, String message) {
-        mainActivityInterface.makeAlertDialog(title, message);
+    public void twoStepFinished(String messageToShow) {
+        mainActivityInterface.makeAlertDialog(R.string.TwoStepFinishDialogTitle, messageToShow, R.string.ButtonOK,
+                false, (dialog, which) -> dialog.dismiss());
     }
 
     @Override
@@ -703,9 +704,11 @@ public void authenticationFinished(boolean success, Token token) {
         if (success) {
             mainActivityInterface.makeToast(R.string.AuthenticationSuccessful);
             // Remove the notification if still present
-            mainActivityInterface.cancelNotification(token.getPendingAuths().get(0).getNotificationID());
-            // In case of success, remove the pendingAuth from the token (always the first from within the App)
-            token.getPendingAuths().remove(0);
+            if (!token.getPendingAuths().isEmpty()) {
+                mainActivityInterface.cancelNotification(token.getPendingAuths().get(0).getNotificationID());
+                // In case of success, remove the pendingAuth from the token (always the first from within the App)
+                token.getPendingAuths().remove(0);
+            }
             token.state = FINISHED;
             tokenListInterface.notifyChange();
         } else {
@@ -717,7 +720,7 @@ public void authenticationFinished(boolean success, Token token) {
 
     /**
      * Cancel the running Authentication Task and remove the pair from the runningAuthentications List
-     *
+     * Sets the tokens 'lastAuthHadError' so the authentication can be dismissed
      * @param token token of the pair
      */
     private void deleteRunningAuthenticationFor(Token token) {
@@ -732,11 +735,12 @@ private void deleteRunningAuthenticationFor(Token token) {
             toDelete.second.cancel(true);
             runningAuthentications.remove(toDelete);
         }
+        token.lastAuthHadError = true;
         token.state = FINISHED;
     }
 
     @Override
-    public void handleError(int statusCode, Token token) {
+    public void handleAuthError(int statusCode, Token token) {
         token.lastAuthHadError = true;
         switch (statusCode) {
             case STATUS_ENDPOINT_UNKNOWN_HOST: {

diff --git a/app/src/main/java/it/netknights/piauthenticator/services/PushAuthService.java b/app/src/main/java/it/netknights/piauthenticator/services/PushAuthService.java
@@ -259,7 +259,7 @@ private void rebuildNotificationWithErrorReason(String reason) {
     }
 
     @Override
-    public void handleError(int statusCode, Token token) {
+    public void handleAuthError(int statusCode, Token token) {
         switch (statusCode) {
             case STATUS_ENDPOINT_UNKNOWN_HOST: {
                 // TODO just fail the authentication?

diff --git a/app/src/main/java/it/netknights/piauthenticator/tasks/PushAuthTask.java b/app/src/main/java/it/netknights/piauthenticator/tasks/PushAuthTask.java
@@ -144,11 +144,11 @@ protected void onProgressUpdate(Integer... values) {
         switch (code) {
             case PA_INVALID_SIGNATURE:
                 logprint("authentication failed due to invalid signature :(");
-                pushAuthCallbackInterface.handleError(code, token);
+                pushAuthCallbackInterface.handleAuthError(code, token);
                 break;
             case PA_SIGNING_FAILURE:
                 logprint("authentication failed due to signing failure :(");
-                pushAuthCallbackInterface.handleError(code, token);
+                pushAuthCallbackInterface.handleAuthError(code, token);
                 break;
             case PA_AUTHENTICATION_FINISHED:
                 if (success) {
@@ -160,7 +160,7 @@ protected void onProgressUpdate(Integer... values) {
                 }
                 break;
             default: {
-                pushAuthCallbackInterface.handleError(code, token);
+                pushAuthCallbackInterface.handleAuthError(code, token);
                 break;
             }
         }