improve FIDO integration tests

testing-android/src/androidTest/java/com/yubico/yubikit/testing/DeviceTests.java

@@ -21,18 +21,21 @@
 import com.yubico.yubikit.testing.openpgp.OpenPgpTests;
 import com.yubico.yubikit.testing.piv.PivTests;
 
+import org.junit.experimental.categories.Categories;
 import org.junit.runner.RunWith;
 import org.junit.runners.Suite;
 
 /**
  * All integration tests.
+ * Note that the YubiKey applications will be reset several times.
+ * <p>
+ * FIDO integration tests should be ran separately.
  */
 @RunWith(Suite.class)
 @Suite.SuiteClasses({
         PivTests.class,
         OpenPgpTests.class,
         OathTests.class,
-        FidoTests.class
 })
 public class DeviceTests {
 }

testing-android/src/androidTest/java/com/yubico/yubikit/testing/fido/Ctap2ConfigInstrumentedTests.java

@@ -16,43 +16,67 @@
 
 package com.yubico.yubikit.testing.fido;
 
+import com.yubico.yubikit.testing.AlwaysManualTestCategory;
 import com.yubico.yubikit.testing.framework.FidoInstrumentedTests;
 
 import org.junit.Test;
+import org.junit.experimental.categories.Categories;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
 
 /**
- * NOTE: Run the testcases in this suite manually one by one. See test case documentation
- * and reset the FIDO application where needed.
+ * Config tests.
+ * <p>
+ * These tests will change FIDO2 application configuration through authenticatorConfig. As these changes
+ * are irreversible.
+ * <p>
+ * Read documentation for each tests for more information.
  */
-public class Ctap2ConfigInstrumentedTests extends FidoInstrumentedTests {
+@RunWith(Categories.class)
+@Suite.SuiteClasses(Ctap2ConfigInstrumentedTests.ConfigTests.class)
+@Categories.ExcludeCategory(AlwaysManualTestCategory.class)
+public class Ctap2ConfigInstrumentedTests {
 
-    @Test
-    public void testReadWriteEnterpriseAttestation() throws Throwable {
-        withCtap2Session(Ctap2ConfigTests::testReadWriteEnterpriseAttestation);
-    }
+    public static class ConfigTests extends FidoInstrumentedTests {
+        @Test
+        public void testReadWriteEnterpriseAttestation() throws Throwable {
+            withCtap2Session(Ctap2ConfigTests::testReadWriteEnterpriseAttestation);
+        }
 
-    @Test
-    public void testToggleAlwaysUv() throws Throwable {
-        withCtap2Session(Ctap2ConfigTests::testToggleAlwaysUv);
-    }
+        /**
+         * Toggles the {@code alwaysUv} option to opposite value. It is not possible to set this
+         * option to `false` on a FIPS approved YubiKey.
+         *
+         * @throws Throwable if an error occurs
+         */
+        @Test
+        @Category(AlwaysManualTestCategory.class)
+        public void testToggleAlwaysUv() throws Throwable {
+            withCtap2Session(Ctap2ConfigTests::testToggleAlwaysUv);
+        }
 
-    /**
-     * Reset the FIDO application after calling this test case.
-     *
-     * @throws Throwable on any error
-     */
-    @Test
-    public void testSetForcePinChange() throws Throwable {
-        withCtap2Session(Ctap2ConfigTests::testSetForcePinChange);
-    }
+        /**
+         * Sets the {@code forcePinChange} flag, verifies that and then changes the PIN twice so
+         * that the device uses the {@code TestUtil.PIN}.
+         *
+         * @throws Throwable if an error occurs
+         */
+        @Test
+        public void testSetForcePinChange() throws Throwable {
+            withCtap2Session(Ctap2ConfigTests::testSetForcePinChange);
+        }
 
-    /**
-     * Reset the FIDO application after calling this test case.
-     *
-     * @throws Throwable on any error
-     */
-    @Test
-    public void testSetMinPinLength() throws Throwable {
-        withCtap2Session(Ctap2ConfigTests::testSetMinPinLength);
+        /**
+         * Changes the {@code minPinLength} value. This change is irreversible and after running
+         * this test, the YubiKey should be reset.
+         *
+         * @throws Throwable if an error occurs
+         */
+        @Test
+        @Category(AlwaysManualTestCategory.class)
+        public void testSetMinPinLength() throws Throwable {
+            withCtap2Session(Ctap2ConfigTests::testSetMinPinLength);
+        }
     }
 }

testing-android/src/androidTest/java/com/yubico/yubikit/testing/fido/Ctap2SessionResetInstrumentedTests.java

@@ -16,9 +16,11 @@
 
 package com.yubico.yubikit.testing.fido;
 
+import com.yubico.yubikit.testing.AlwaysManualTestCategory;
 import com.yubico.yubikit.testing.framework.FidoInstrumentedTests;
 
 import org.junit.Test;
+import org.junit.experimental.categories.Category;
 
 /**
  * Tests FIDO Reset.
@@ -32,6 +34,7 @@
  */
 public class Ctap2SessionResetInstrumentedTests extends FidoInstrumentedTests {
     @Test
+    @Category(AlwaysManualTestCategory.class)
     public void testReset() throws Throwable {
         withCtap2Session(Ctap2SessionTests::testReset);
     }

testing-android/src/androidTest/java/com/yubico/yubikit/testing/fido/FidoTests.java

@@ -16,20 +16,31 @@
 
 package com.yubico.yubikit.testing.fido;
 
+import com.yubico.yubikit.testing.AlwaysManualTestCategory;
+
+import org.junit.experimental.categories.Categories;
 import org.junit.runner.RunWith;
 import org.junit.runners.Suite;
 
-@RunWith(Suite.class)
+/**
+ * Setup YubiKey before running the integration tests:
+ * <ul>
+ *     <li>reset FIDO app</li>
+ *     <li>optionally set PIN to `11234567`</li>
+ * </ul>
+ */
+@RunWith(Categories.class)
 @Suite.SuiteClasses({
         BasicWebAuthnClientInstrumentedTests.class,
-        Ctap2BioEnrollmentInstrumentedTests.class,
         Ctap2ClientPinInstrumentedTests.class,
-        Ctap2ConfigInstrumentedTests.class,
         Ctap2CredentialManagementInstrumentedTests.class,
         Ctap2SessionInstrumentedTests.class,
-        Ctap2SessionResetInstrumentedTests.class,
         EnterpriseAttestationInstrumentedTests.class,
         UvDiscouragedInstrumentedTests.class,
+        Ctap2ConfigInstrumentedTests.class,
+        Ctap2BioEnrollmentInstrumentedTests.class,
+        Ctap2SessionResetInstrumentedTests.class,
 })
+@Categories.ExcludeCategory(AlwaysManualTestCategory.class)
 public class FidoTests {
 }

testing-android/src/androidTest/java/com/yubico/yubikit/testing/fido/UvDiscouragedInstrumentedTests.java

@@ -17,22 +17,25 @@
 package com.yubico.yubikit.testing.fido;
 
 import com.yubico.yubikit.fido.client.PinRequiredClientError;
+import com.yubico.yubikit.testing.AlwaysManualTestCategory;
 import com.yubico.yubikit.testing.framework.FidoInstrumentedTests;
 
 import org.junit.Test;
+import org.junit.experimental.categories.Category;
 
 public class UvDiscouragedInstrumentedTests extends FidoInstrumentedTests {
     @Test
+    @Category(AlwaysManualTestCategory.class)
     public void testMakeCredentialGetAssertion() throws Throwable {
-        withCtap2Session(BasicWebAuthnClientTests::testUvDiscouragedMakeCredentialGetAssertionWithoutPin);
+        withCtap2Session(BasicWebAuthnClientTests::testUvDiscouragedMakeCredentialGetAssertionNoPin);
     }
 
     /**
      * Run this test only on devices with PIN set
      * Expected to fail with PinRequiredClientError
      */
     @Test(expected = PinRequiredClientError.class)
-    public void testMakeCredentialGetAssertionOnProtected() throws Throwable {
+    public void testMakeCredentialGetAssertionOnProtectedWithPin() throws Throwable {
         withCtap2Session(BasicWebAuthnClientTests::testUvDiscouragedMakeCredentialGetAssertionWithPin);
     }
 }

testing-android/src/main/java/com/yubico/yubikit/testing/AlwaysManualTestCategory.java

@@ -0,0 +1,20 @@
+/*
+ * Copyright (C) 2024 Yubico.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *       http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.yubico.yubikit.testing;
+
+public interface AlwaysManualTestCategory {
+}

testing-android/src/main/java/com/yubico/yubikit/testing/TestActivity.java

@@ -79,6 +79,20 @@ protected void onCreate(@Nullable Bundle savedInstanceState) {
     @Override
     protected void onResume() {
         super.onResume();
+        startNfcDiscovery();
+    }
+
+    @Override
+    protected void onPause() {
+        stopNfcDiscovery();
+        super.onPause();
+    }
+
+    private void stopNfcDiscovery() {
+        yubiKitManager.stopNfcDiscovery(this);
+    }
+
+    private void startNfcDiscovery() {
         try {
             yubiKitManager.startNfcDiscovery(new NfcConfiguration().timeout(1000 * 60 * 5), this, sessionQueue::add);
         } catch (NfcNotAvailable e) {
@@ -90,12 +104,6 @@ protected void onResume() {
         }
     }
 
-    @Override
-    protected void onPause() {
-        yubiKitManager.stopNfcDiscovery(this);
-        super.onPause();
-    }
-
     private void setBusy(boolean busy) {
         runOnUiThread(() -> {
             if (busy) {
@@ -148,6 +156,8 @@ public synchronized void returnSession(YubiKeyDevice device) throws InterruptedE
                 ((NfcYubiKeyDevice) device).remove(lock::release);
             } else {
                 lock.release();
+                stopNfcDiscovery();
+                startNfcDiscovery();
             }
 
             lock.acquire();

testing/src/main/java/com/yubico/yubikit/testing/fido/BasicWebAuthnClientTests.java

@@ -77,11 +77,6 @@ public static void testMakeCredentialGetAssertionTokenUvOnly(Ctap2Session sessio
     }
 
     public static void testMakeCredentialGetAssertion(Ctap2Session session) throws Throwable {
-
-        // // Ctap2ClientPinTests.ensureDefaultPinSet(session);
-
-        char[] pin = TestData.PIN;
-
         BasicWebAuthnClient webauthn = new BasicWebAuthnClient(session);
         List<byte[]> deleteCredIds = new ArrayList<>();
 
@@ -100,7 +95,7 @@ public static void testMakeCredentialGetAssertion(Ctap2Session session) throws T
                 TestData.CLIENT_DATA_JSON_CREATE,
                 creationOptionsNonRk,
                 Objects.requireNonNull(creationOptionsNonRk.getRp().getId()),
-                pin,
+                TestData.PIN,
                 null,
                 null
         );
@@ -123,7 +118,7 @@ public static void testMakeCredentialGetAssertion(Ctap2Session session) throws T
                 TestData.CLIENT_DATA_JSON_CREATE,
                 creationOptionsRk,
                 Objects.requireNonNull(creationOptionsRk.getRp().getId()),
-                pin,
+                TestData.PIN,
                 null,
                 null
         );
@@ -148,7 +143,7 @@ public static void testMakeCredentialGetAssertion(Ctap2Session session) throws T
                     TestData.CLIENT_DATA_JSON_GET,
                     requestOptions,
                     TestData.RP_ID,
-                    pin,
+                    TestData.PIN,
                     null
             );
             AuthenticatorAssertionResponse response = (AuthenticatorAssertionResponse) credential.getResponse();
@@ -168,7 +163,7 @@ public static void testUvDiscouragedMakeCredentialGetAssertionWithPin(Ctap2Sessi
         testUvDiscouragedMakeCredentialGetAssertion(session);
     }
 
-    public static void testUvDiscouragedMakeCredentialGetAssertionWithoutPin(Ctap2Session session) throws Throwable {
+    public static void testUvDiscouragedMakeCredentialGetAssertionNoPin(Ctap2Session session) throws Throwable {
         assumeFalse("Device has no PIN set",
                 Boolean.TRUE.equals(session.getCachedInfo().getOptions().get("clientPin")));
         testUvDiscouragedMakeCredentialGetAssertion(session);
@@ -286,9 +281,6 @@ private static void testUvDiscouragedMakeCredentialGetAssertion(Ctap2Session ses
     }
 
     public static void testGetAssertionMultipleUsersRk(Ctap2Session session) throws Throwable {
-
-        // Ctap2ClientPinTests.ensureDefaultPinSet(session);
-
         BasicWebAuthnClient webauthn = new BasicWebAuthnClient(session);
         List<byte[]> deleteCredIds = new ArrayList<>();
 
@@ -367,8 +359,6 @@ public static void testGetAssertionMultipleUsersRk(Ctap2Session session) throws
 
     public static void testGetAssertionWithAllowList(Ctap2Session session) throws Throwable {
 
-        // Ctap2ClientPinTests.ensureDefaultPinSet(session);
-
         BasicWebAuthnClient webauthn = new BasicWebAuthnClient(session);
 
         // Make 2 new credentials
@@ -457,8 +447,6 @@ public static void testGetAssertionWithAllowList(Ctap2Session session) throws Th
 
     public static void testMakeCredentialWithExcludeList(Ctap2Session session) throws Throwable {
 
-        // Ctap2ClientPinTests.ensureDefaultPinSet(session);
-
         BasicWebAuthnClient webauthn = new BasicWebAuthnClient(session);
         List<PublicKeyCredentialDescriptor> excludeList = new ArrayList<>();
 
@@ -525,7 +513,6 @@ public static void testMakeCredentialWithExcludeList(Ctap2Session session) throw
     }
 
     public static void testMakeCredentialKeyAlgorithms(Ctap2Session session) throws Throwable {
-        // Ctap2ClientPinTests.ensureDefaultPinSet(session);
         BasicWebAuthnClient webauthn = new BasicWebAuthnClient(session);
         List<PublicKeyCredentialParameters> allCredParams = Arrays.asList(
                 TestData.PUB_KEY_CRED_PARAMS_ES256,
@@ -601,18 +588,14 @@ public static void testMakeCredentialKeyAlgorithms(Ctap2Session session) throws
     }
 
     public static void testClientPinManagement(Ctap2Session session) throws Throwable {
-        // Ctap2ClientPinTests.ensureDefaultPinSet(session);
-
         BasicWebAuthnClient webauthn = new BasicWebAuthnClient(session);
         assertTrue(webauthn.isPinSupported());
         assertTrue(webauthn.isPinConfigured());
 
-        char[] otherPin = "123123".toCharArray();
-
-        webauthn.changePin(TestData.PIN, otherPin);
+        webauthn.changePin(TestData.PIN, TestData.OTHER_PIN);
 
         try {
-            webauthn.changePin(TestData.PIN, otherPin);
+            webauthn.changePin(TestData.PIN, TestData.OTHER_PIN);
             fail("Wrong PIN was accepted");
         } catch (ClientError e) {
             assertThat(e.getErrorCode(), equalTo(ClientError.Code.BAD_REQUEST));
@@ -621,14 +604,13 @@ public static void testClientPinManagement(Ctap2Session session) throws Throwabl
                     is(CtapException.ERR_PIN_INVALID));
         }
 
-        webauthn.changePin(otherPin, TestData.PIN);
+        webauthn.changePin(TestData.OTHER_PIN, TestData.PIN);
     }
 
 
     public static void testClientCredentialManagement(Ctap2Session session) throws Throwable {
         assumeTrue("Credential management not supported",
                 CredentialManagement.isSupported(session.getCachedInfo()));
-        // Ctap2ClientPinTests.ensureDefaultPinSet(session);
         BasicWebAuthnClient webauthn = new BasicWebAuthnClient(session);
         PublicKeyCredentialCreationOptions creationOptions = getCreateOptions(null, true,
                 Collections.singletonList(TestData.PUB_KEY_CRED_PARAMS_ES256),