remove nfc retries

android/app/src/main/kotlin/com/yubico/authenticator/AppContextManager.kt

@@ -22,11 +22,13 @@ import com.yubico.yubikit.core.YubiKeyDevice
  * Provides behavior to run when a YubiKey is inserted/tapped for a specific view of the app.
  */
 abstract class AppContextManager {
-    abstract suspend fun processYubiKey(device: YubiKeyDevice)
+    abstract suspend fun processYubiKey(device: YubiKeyDevice): Boolean
 
     open fun dispose() {}
 
     open fun onPause() {}
+
+    open fun onError() {}
 }
 
 class ContextDisposedException : Exception()

android/app/src/main/kotlin/com/yubico/authenticator/MainActivity.kt

@@ -321,6 +321,7 @@ class MainActivity : FlutterFragmentActivity() {
             appMethodChannel.nfcStateChanged(NfcState.ONGOING)
         }
 
+        deviceManager.scpKeyParams = null
         // If NFC and FIPS check for SCP11b key
         if (device.transport == Transport.NFC && deviceInfo.fipsCapable != 0) {
             logger.debug("Checking for usable SCP11b key...")
@@ -340,6 +341,7 @@ class MainActivity : FlutterFragmentActivity() {
                 }
             } catch (e: Exception) {
                 logger.debug("Exception while getting scp keys: ", e)
+                contextManager?.onError()
                 if (device is NfcYubiKeyDevice) {
                     appMethodChannel.nfcStateChanged(NfcState.FAILURE)
                 }
@@ -373,9 +375,12 @@ class MainActivity : FlutterFragmentActivity() {
 
         contextManager?.let {
             try {
-                it.processYubiKey(device)
+                val requestHandled = it.processYubiKey(device)
-                if (!switchedContext && device is NfcYubiKeyDevice) {
+                if (requestHandled) {
                     appMethodChannel.nfcStateChanged(NfcState.SUCCESS)
+                }
+                if (!switchedContext && device is NfcYubiKeyDevice) {
+
                     device.remove {
                         appMethodChannel.nfcStateChanged(NfcState.IDLE)
                     }

android/app/src/main/kotlin/com/yubico/authenticator/device/DeviceManager.kt

@@ -32,6 +32,7 @@ import com.yubico.yubikit.core.smartcard.scp.ScpKeyParams
 import com.yubico.yubikit.management.Capability
 import kotlinx.coroutines.CancellationException
 import org.slf4j.LoggerFactory
+import java.io.IOException
 
 interface DeviceListener {
     // a USB device is connected
@@ -174,7 +175,6 @@ class DeviceManager(
 
     fun setDeviceInfo(deviceInfo: Info?) {
         appViewModel.setDeviceInfo(deviceInfo)
-        this.scpKeyParams = null
     }
 
     fun isUsbKeyConnected(): Boolean {
@@ -189,16 +189,12 @@ class DeviceManager(
     suspend fun <T> withKey(
         onUsb: suspend (UsbYubiKeyDevice) -> T,
         onNfc: suspend () -> com.yubico.yubikit.core.util.Result<T, Throwable>,
-        onCancelled: () -> Unit,
+        onCancelled: () -> Unit
-        retryOnNfcFailure: Boolean
     ): T =
         appViewModel.connectedYubiKey.value?.let {
             onUsb(it)
-        } ?: if (retryOnNfcFailure == true) {
+        } ?: onNfc(onNfc, onCancelled)
-            onNfcWithRetries(onNfc, onCancelled)
+
-        } else {
-            onNfc(onNfc, onCancelled)
-        }
 
     private suspend fun <T> onNfc(
         onNfc: suspend () -> com.yubico.yubikit.core.util.Result<T, Throwable>,
@@ -210,42 +206,12 @@ class DeviceManager(
         }
 
         try {
-            return onNfc.invoke().value
+            return onNfc.invoke().value.also {
+                appMethodChannel.nfcStateChanged(NfcState.SUCCESS)
+            }
         } catch (e: Exception) {
             appMethodChannel.nfcStateChanged(NfcState.FAILURE)
             throw e
         }
     }
-
-    private suspend fun <T> onNfcWithRetries(
-        onNfc: suspend () -> com.yubico.yubikit.core.util.Result<T, Throwable>,
-        onCancelled: () -> Unit
-    ): T {
-
-        nfcOverlayManager.show {
-            logger.debug("NFC action with retries was cancelled")
-            onCancelled.invoke()
-        }
-
-        while (true) {
-            try {
-                return onNfc.invoke().value
-            } catch (e: Exception) {
-
-                logger.debug("NFC action failed, asking to try again. Failure: ", e)
-                if (e is CancellationException) {
-                    throw e
-                }
-
-                if (e is ContextDisposedException) {
-                    // the key does not have the needed context anymore
-                    // we cannot continue
-                    appMethodChannel.nfcStateChanged(NfcState.FAILURE)
-                    throw e
-                }
-
-                appMethodChannel.nfcStateChanged(NfcState.FAILURE)
-            }
-        }
-    }
 }

android/app/src/main/kotlin/com/yubico/authenticator/fido/FidoConnectionHelper.kt

@@ -30,17 +30,19 @@ import kotlin.coroutines.suspendCoroutine
 
 class FidoConnectionHelper(private val deviceManager: DeviceManager) {
     private var pendingAction: FidoAction? = null
-    private var deviceInfoTimer: TimerTask? = null
 
-    fun invokePending(fidoSession: YubiKitFidoSession) {
+    fun invokePending(fidoSession: YubiKitFidoSession): Boolean {
+        var requestHandled = true
         pendingAction?.let { action ->
-            action.invoke(Result.success(fidoSession))
             pendingAction = null
+            // it is the pending action who handles this request
+            requestHandled = false
+            action.invoke(Result.success(fidoSession))
         }
+        return requestHandled
     }
 
     fun cancelPending() {
-        deviceInfoTimer?.cancel()
         pendingAction?.let { action ->
             action.invoke(Result.failure(CancellationException()))
             pendingAction = null
@@ -49,7 +51,6 @@ class FidoConnectionHelper(private val deviceManager: DeviceManager) {
 
     suspend fun <T> useSession(
         updateDeviceInfo: Boolean = false,
-        retryOnNfcFailure: Boolean = true,
         block: (YubiKitFidoSession) -> T
     ): T {
         FidoManager.updateDeviceInfo.set(updateDeviceInfo)
@@ -59,8 +60,7 @@ class FidoConnectionHelper(private val deviceManager: DeviceManager) {
             onCancelled = {
                 pendingAction?.invoke(Result.failure(CancellationException()))
                 pendingAction = null
-            },
+            }
-            retryOnNfcFailure = retryOnNfcFailure
         )
     }
 

android/app/src/main/kotlin/com/yubico/authenticator/fido/FidoManager.kt

@@ -127,8 +127,6 @@ class FidoManager(
             pinStore
         )
 
-
-
     init {
         pinRetries = null
 
@@ -176,6 +174,12 @@ class FidoManager(
         }
     }
 
+    override fun onError() {
+        super.onError()
+        logger.debug("Cancel any pending action because of upstream error")
+        connectionHelper.cancelPending()
+    }
+
     override fun dispose() {
         super.dispose()
         deviceManager.removeDeviceListener(this)
@@ -186,15 +190,16 @@ class FidoManager(
         coroutineScope.cancel()
     }
 
-    override suspend fun processYubiKey(device: YubiKeyDevice) {
+    override suspend fun processYubiKey(device: YubiKeyDevice): Boolean {
+        var requestHandled = true
         try {
             if (device.supportsConnection(FidoConnection::class.java)) {
                 device.withConnection<FidoConnection, Unit> { connection ->
-                    processYubiKey(connection, device)
+                    requestHandled = processYubiKey(connection, device)
                 }
             } else {
                 device.withConnection<SmartCardConnection, Unit> { connection ->
-                    processYubiKey(connection, device)
+                    requestHandled = processYubiKey(connection, device)
                 }
             }
 
@@ -207,10 +212,14 @@ class FidoManager(
 
             // Clear any cached FIDO state
             fidoViewModel.clearSessionState()
-        }
+            throw e
         }
 
-    private fun processYubiKey(connection: YubiKeyConnection, device: YubiKeyDevice) {
+        return requestHandled
+    }
+
+    private fun processYubiKey(connection: YubiKeyConnection, device: YubiKeyDevice): Boolean {
+        var requestHandled = true
         val fidoSession =
             if (connection is FidoConnection) {
                 YubiKitFidoSession(connection)
@@ -229,7 +238,7 @@ class FidoManager(
         val sameDevice = currentSession == previousSession
 
         if (device is NfcYubiKeyDevice && (sameDevice || resetHelper.inProgress)) {
-            connectionHelper.invokePending(fidoSession)
+            requestHandled = connectionHelper.invokePending(fidoSession)
         } else {
 
             if (!sameDevice) {
@@ -253,6 +262,8 @@ class FidoManager(
                 Session(infoData, pinStore.hasPin(), pinRetries)
             )
         }
+
+        return requestHandled
     }
 
     private fun getPinPermissionsCM(fidoSession: YubiKitFidoSession): Int {

android/app/src/main/kotlin/com/yubico/authenticator/management/ManagementConnectionHelper.kt

@@ -39,8 +39,7 @@ class ManagementConnectionHelper(
             onCancelled = {
                 action?.invoke(Result.failure(CancellationException()))
                 action = null
-            },
+            }
-            retryOnNfcFailure = false
         )
 
     private suspend fun <T> useSessionUsb(

android/app/src/main/kotlin/com/yubico/authenticator/oath/OathManager.kt

@@ -110,6 +110,15 @@ class OathManager(
     private val updateDeviceInfo = AtomicBoolean(false)
     private var deviceInfoTimer: TimerTask? = null
 
+    override fun onError() {
+        super.onError()
+        logger.debug("Cancel any pending action because of upstream error")
+        pendingAction?.let { action ->
+            action.invoke(Result.failure(CancellationException()))
+            pendingAction = null
+        }
+    }
+
     override fun onPause() {
         deviceInfoTimer?.cancel()
         // cancel any pending actions, except for addToAny
@@ -217,7 +226,8 @@ class OathManager(
         coroutineScope.cancel()
     }
 
-    override suspend fun processYubiKey(device: YubiKeyDevice) {
+    override suspend fun processYubiKey(device: YubiKeyDevice): Boolean {
+        var requestHandled = true
         try {
             device.withConnection<SmartCardConnection, Unit> { connection ->
                 val session = getOathSession(connection)
@@ -227,6 +237,8 @@ class OathManager(
                     if (pendingAction != null) {
                         pendingAction?.let { action ->
                             pendingAction = null
+                            // it is the pending action who handles this request
+                            requestHandled = false
                             action.invoke(Result.success(session))
                         }
                     } else {
@@ -235,7 +247,7 @@ class OathManager(
                             try {
                                 oathViewModel.updateCredentials(calculateOathCodes(session))
                             } catch (error: Exception) {
-                                logger.error("Failed to refresh codes", error)
+                                logger.error("Failed to refresh codes: ", error)
                                 throw error
                             }
                         }
@@ -263,7 +275,9 @@ class OathManager(
                         if (addToAny) {
                             // Special "add to any YubiKey" action, process
                             addToAny = false
+                            requestHandled = false
                             action.invoke(Result.success(session))
+                            requestHandled = true
                         } else {
                             // Awaiting an action for a different device? Fail it and stop processing.
                             action.invoke(Result.failure(IllegalStateException("Wrong deviceId")))
@@ -305,8 +319,17 @@ class OathManager(
             logger.error("Failed to connect to CCID: ", e)
             // Clear any cached OATH state
             oathViewModel.clearSession()
+            // Remove any pending action
+            pendingAction?.let { action ->
+                logger.error("Cancelling pending action")
+                pendingAction = null
+                action.invoke(Result.failure(CancellationException()))
+            }
+
             throw e
         }
+
+        return requestHandled
     }
 
     private suspend fun addAccountToAny(
@@ -316,7 +339,7 @@ class OathManager(
         val credentialData: CredentialData =
             CredentialData.parseUri(URI.create(uri))
         addToAny = true
-        return useOathSession(retryOnNfcFailure = false) { session ->
+        return useOathSession { session ->
             // We need to check for duplicates here since we haven't yet read the credentials
             if (session.credentials.any { it.id.contentEquals(credentialData.id) }) {
                 throw IllegalArgumentException()
@@ -346,7 +369,7 @@ class OathManager(
         logger.trace("Adding following accounts: {}", uris)
 
         addToAny = true
-        return useOathSession(retryOnNfcFailure = false) { session ->
+        return useOathSession { session ->
             var successCount = 0
             for (index in uris.indices) {
 
@@ -398,7 +421,16 @@ class OathManager(
             val remembered = keyManager.isRemembered(it.deviceId)
             if (unlocked) {
                 oathViewModel.setSessionState(Session(it, remembered))
+
+                try {
                     oathViewModel.updateCredentials(calculateOathCodes(it))
+                } catch (e: Exception) {
+                    // after unlocking there was problem getting the codes
+                    // to avoid incomplete session, just reset it so that the user has to
+                    // unlock it again
+                    oathViewModel.clearSession()
+                    throw e
+                }
             }
 
             jsonSerializer.encodeToString(mapOf("unlocked" to unlocked, "remembered" to remembered))
@@ -682,7 +714,6 @@ class OathManager(
     private suspend fun <T> useOathSession(
         unlock: Boolean = true,
         updateDeviceInfo: Boolean = false,
-        retryOnNfcFailure: Boolean = true,
         block: (YubiKitOathSession) -> T
     ): T {
         // callers can decide whether the session should be unlocked first
@@ -695,8 +726,7 @@ class OathManager(
             onCancelled = {
                 pendingAction?.invoke(Result.failure(CancellationException()))
                 pendingAction = null
-            },
+            }
-            retryOnNfcFailure = retryOnNfcFailure
         )
     }
 
@@ -722,7 +752,6 @@ class OathManager(
                         block.invoke(it.value)
                     })
                 }
-
                 // here the coroutine is suspended and waits till pendingAction is
                 // invoked - the pending action result will resume this coroutine
             }

lib/android/overlay/nfc/nfc_overlay.dart

@@ -28,7 +28,7 @@ import 'views/nfc_content_widget.dart';
 import 'views/nfc_overlay_icons.dart';
 import 'views/nfc_overlay_widget.dart';
 
-final _log = Logger('android.tap_request_dialog');
+final _log = Logger('android.nfc_overlay');
 const _channel = MethodChannel('com.yubico.authenticator.channel.nfc_overlay');
 
 final nfcOverlay =
@@ -41,6 +41,7 @@ class _NfcOverlayNotifier extends Notifier<int> {
   @override
   int build() {
     ref.listen(androidNfcState, (previous, current) {
+      _log.debug('Received nfc state: $current');
       processingViewTimeout?.cancel();
       final notifier = ref.read(nfcEventNotifier.notifier);
 
@@ -62,6 +63,8 @@ class _NfcOverlayNotifier extends Notifier<int> {
           break;
         case NfcState.failure:
           notifier.send(showFailed());
+          notifier
+              .send(const NfcHideViewEvent(delay: Duration(milliseconds: 800)));
           break;
         case NfcState.disabled:
           _log.debug('Received state: disabled');
@@ -125,7 +128,7 @@ class _NfcOverlayNotifier extends Notifier<int> {
   }
 
   NfcEvent showFailed() {
-    ref.read(nfcOverlayWidgetProperties.notifier).update(hasCloseButton: true);
+    ref.read(nfcOverlayWidgetProperties.notifier).update(hasCloseButton: false);
     return NfcSetViewEvent(
         child: NfcContentWidget(
           title: l10n.s_nfc_ready_to_scan,

lib/app/views/main_page.dart

@@ -59,19 +59,11 @@ class MainPage extends ConsumerWidget {
     // If the current device changes, we need to pop any open dialogs.
     ref.listen<AsyncValue<YubiKeyData>>(currentDeviceDataProvider,
         (prev, next) {
-      var canPop = true;
+      final serial = next.value?.info.serial;
-      if ((next.value != null) && (prev?.value != null)) {
+      if (serial != null && serial == prev?.value?.info.serial) {
-        // if there is change only in fipsApproved, don't pop anything
+        return;
-        var nextInfo = next.value!.info;
-        var prevInfo = prev!.value!.info;
-
-        canPop =
-            prevInfo.copyWith(fipsApproved: nextInfo.fipsApproved) != nextInfo;
-      } else if (next.hasValue && (prev != null && prev.isLoading)) {
-        canPop = false;
       }
 
-      if (canPop) {
       Navigator.of(context).popUntil((route) {
         return route.isFirst ||
             [
@@ -85,7 +77,6 @@ class MainPage extends ConsumerWidget {
               'android_qr_scanner_view',
             ].contains(route.settings.name);
       });
-      }
     });
 
     final deviceNode = ref.watch(currentDeviceProvider);