From 50af081a20e3ef4404df8602ccf0b39c8a7ecb17 Mon Sep 17 00:00:00 2001
From: Wolfgang Bumiller <w.bumiller@proxmox.com>
Date: Tue, 6 Jun 2023 12:03:17 +0200
Subject: [PATCH] tfa: improve UX for recovery keys and when none are left

If we get an empty challenge, tell the user to contact an
administrator as it means no 2nd factors and no recovery
keys are available.

Currently if only 1 key was available and it had a high ID,
we'd show something like: "Recovery keys available: 9,
Warning, less than 4 keys available."
Let's start off with the warning, and then be explicit about
the IDs.

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 src/window/TfaWindow.js | 75 ++++++++++++++++++++++++++++++++---------
 1 file changed, 59 insertions(+), 16 deletions(-)

diff --git a/src/window/TfaWindow.js b/src/window/TfaWindow.js
index 22ac50d..3646e0e 100644
--- a/src/window/TfaWindow.js
+++ b/src/window/TfaWindow.js
@@ -45,11 +45,17 @@ Ext.define('Proxmox.window.TfaLoginWindow', {
 
 	    let lastTabId = me.getLastTabUsed();
 	    let initialTab = -1, i = 0;
+	    let count2nd = 0;
+	    let hasRecovery = false;
 	    for (const k of ['webauthn', 'totp', 'recovery', 'u2f', 'yubico']) {
 		const available = !!challenge[k];
 		vm.set(`availableChallenge.${k}`, available);
 
 		if (available) {
+		    count2nd++;
+		    if (k === 'recovery') {
+			hasRecovery = true;
+		    }
 		    if (i === lastTabId) {
 			initialTab = i;
 		    } else if (initialTab < 0) {
@@ -58,15 +64,32 @@ Ext.define('Proxmox.window.TfaLoginWindow', {
 		}
 		i++;
 	    }
+	    if (!count2nd || (count2nd === 1 && hasRecovery && !challenge.recovery.length)) {
+		// no 2nd factors available (and if recovery keys are configured they're empty)
+		me.lookup('cannotLogin').setVisible(true);
+		me.lookup('recoveryKey').setVisible(false);
+		view.down('tabpanel').setActiveTab(2); // recovery
+		return;
+	    }
 	    view.down('tabpanel').setActiveTab(initialTab);
 
 	    if (challenge.recovery) {
-		me.lookup('availableRecovery').update(Ext.String.htmlEncode(
-		    gettext('Available recovery keys: ') + view.challenge.recovery.join(', '),
-		));
-		me.lookup('availableRecovery').setVisible(true);
-		if (view.challenge.recovery.length <= 3) {
-		    me.lookup('recoveryLow').setVisible(true);
+		if (!view.challenge.recovery.length) {
+		    me.lookup('recoveryEmpty').setVisible(true);
+		    me.lookup('recoveryKey').setVisible(false);
+		} else {
+		    let idList = view
+			    .challenge
+			    .recovery
+			    .map((id) => Ext.String.format(gettext('ID {0}'), id))
+			    .join(', ');
+		    me.lookup('availableRecovery').update(Ext.String.htmlEncode(
+			Ext.String.format(gettext('Available recovery keys: {0}'), idList),
+		    ));
+		    me.lookup('availableRecovery').setVisible(true);
+		    if (view.challenge.recovery.length <= 3) {
+			me.lookup('recoveryLow').setVisible(true);
+		    }
 		}
 	    }
 
@@ -363,6 +386,36 @@ Ext.define('Proxmox.window.TfaLoginWindow', {
 		    disabled: '{!availableChallenge.recovery}',
 		},
 		items: [
+		    {
+			xtype: 'box',
+			reference: 'cannotLogin',
+			hidden: true,
+			html: '<i class="fa fa-exclamation-triangle warning"></i>'
+			    + Ext.String.format(
+				gettext('No second factor left! Please contact an administrator!'),
+				4,
+			    ),
+		    },
+		    {
+			xtype: 'box',
+			reference: 'recoveryEmpty',
+			hidden: true,
+			html: '<i class="fa fa-exclamation-triangle warning"></i>'
+			    + Ext.String.format(
+				gettext('No more recovery keys left! Please generate a new set!'),
+				4,
+			    ),
+		    },
+		    {
+			xtype: 'box',
+			reference: 'recoveryLow',
+			hidden: true,
+			html: '<i class="fa fa-exclamation-triangle warning"></i>'
+			    + Ext.String.format(
+				gettext('Less than {0} recovery keys available. Please generate a new set after login!'),
+				4,
+			    ),
+		    },
 		    {
 			xtype: 'box',
 			reference: 'availableRecovery',
@@ -379,16 +432,6 @@ Ext.define('Proxmox.window.TfaLoginWindow', {
 			regex: /^[0-9a-f]{4}(-[0-9a-f]{4}){3}$/,
 			regexText: gettext('Does not look like a valid recovery key'),
 		    },
-		    {
-			xtype: 'box',
-			reference: 'recoveryLow',
-			hidden: true,
-			html: '<i class="fa fa-exclamation-triangle warning"></i>'
-			    + Ext.String.format(
-				gettext('Less than {0} recovery keys available. Please generate a new set after login!'),
-				4,
-			    ),
-		    },
 		],
 	    },
 	    {