From 5ba7e52054de33054ba8fa1154a2452f8e8077ad Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Lesimple?=
 <stephane.lesimple+bastion@ovhcloud.com>
Date: Tue, 31 Oct 2023 16:18:26 +0000
Subject: [PATCH] refacto: osh.pl: move hardcoded plugin special cases to
 config

---
 bin/plugin/open/help.json                 |  3 ++-
 bin/plugin/open/info.json                 |  3 ++-
 bin/plugin/open/selfMFAResetPassword.json |  3 ++-
 bin/plugin/open/selfMFAResetTOTP.json     |  3 ++-
 bin/plugin/open/selfMFASetupPassword.json |  3 ++-
 bin/plugin/open/selfMFASetupTOTP.json     |  2 ++
 bin/shell/osh.pl                          | 26 ++++++++---------------
 7 files changed, 21 insertions(+), 22 deletions(-)

diff --git a/bin/plugin/open/help.json b/bin/plugin/open/help.json
index f3fcbef..a1e4a59 100644
--- a/bin/plugin/open/help.json
+++ b/bin/plugin/open/help.json
@@ -1,5 +1,6 @@
 {
     "interactive": [
         "help" , {"pr" : ["<enter>"]}
-    ]
+    ],
+    "mfa_setup_not_required": true
 }
diff --git a/bin/plugin/open/info.json b/bin/plugin/open/info.json
index 7fde235..303b50d 100644
--- a/bin/plugin/open/info.json
+++ b/bin/plugin/open/info.json
@@ -1,5 +1,6 @@
 {
     "interactive": [
         "info" , {"pr" : ["<enter>"]}
-    ]
+    ],
+    "mfa_setup_not_required": true
 }
diff --git a/bin/plugin/open/selfMFAResetPassword.json b/bin/plugin/open/selfMFAResetPassword.json
index 9398ff8..fed5fe7 100644
--- a/bin/plugin/open/selfMFAResetPassword.json
+++ b/bin/plugin/open/selfMFAResetPassword.json
@@ -2,5 +2,6 @@
     "interactive": [
         "selfMFAResetPassword" , {"pr" : ["<enter>"]}
     ],
-    "master_only": true
+    "master_only": true,
+    "mfa_required": "any"
 }
diff --git a/bin/plugin/open/selfMFAResetTOTP.json b/bin/plugin/open/selfMFAResetTOTP.json
index b1a3bdf..67d0945 100644
--- a/bin/plugin/open/selfMFAResetTOTP.json
+++ b/bin/plugin/open/selfMFAResetTOTP.json
@@ -2,5 +2,6 @@
     "interactive": [
         "selfMFAResetTOTP" , {"pr" : ["<enter>"]}
     ],
-    "master_only": true
+    "master_only": true,
+    "mfa_required": "any"
 }
diff --git a/bin/plugin/open/selfMFASetupPassword.json b/bin/plugin/open/selfMFASetupPassword.json
index b6e718a..a5e5959 100644
--- a/bin/plugin/open/selfMFASetupPassword.json
+++ b/bin/plugin/open/selfMFASetupPassword.json
@@ -4,5 +4,6 @@
     ],
     "master_only": true,
     "execution_mode_on_freebsd": "system",
-    "terminal_mode": "noecho"
+    "terminal_mode": "noecho",
+    "mfa_setup_not_required": true
 }
diff --git a/bin/plugin/open/selfMFASetupTOTP.json b/bin/plugin/open/selfMFASetupTOTP.json
index 266c5ec..d7a1e3f 100644
--- a/bin/plugin/open/selfMFASetupTOTP.json
+++ b/bin/plugin/open/selfMFASetupTOTP.json
@@ -3,5 +3,7 @@
         "selfMFASetupTOTP" , {"pr" : ["<enter>"]}
     ],
     "master_only": true,
+    "mfa_required": "any-if-configured",
+    "mfa_setup_not_required": true,
     "terminal_mode": "noecho"
 }
diff --git a/bin/shell/osh.pl b/bin/shell/osh.pl
index ec64acd..8e93b95 100755
--- a/bin/shell/osh.pl
+++ b/bin/shell/osh.pl
@@ -740,7 +740,9 @@ if ($realm && $ENV{'LC_BASTION_DETAILS'}) {
     }
 }
 
-if ($mfaPolicy ne 'disabled' && !grep { $osh_command eq $_ } qw{ selfMFASetupPassword selfMFASetupTOTP help info }) {
+if ($mfaPolicy ne 'disabled'
+    && !OVH::Bastion::plugin_config(plugin => $osh_command, key => "mfa_setup_not_required")->value)
+{
 
     if (($mfaPolicy eq 'password-required' && !$hasMfaPasswordBypass) || $isMfaPasswordRequired) {
         main_exit(OVH::Bastion::EXIT_MFA_PASSWORD_SETUP_REQUIRED, 'mfa_password_setup_required',
@@ -961,23 +963,13 @@ if ($osh_command) {
         # TODO: autodetect if the MFA check is done outside of the code by sshd+PAM, to avoid re-asking for it here
         my $MFArequiredForPlugin = OVH::Bastion::plugin_config(plugin => $osh_command, key => "mfa_required")->value;
         $MFArequiredForPlugin ||= 'none';    # no config means none
-          # some plugins need an explicit MFA check before being called (mainly plugins manipulating authentication factors)
-          # if the user wants to reset one of its MFA tokens, force require MFA
-        if (   (grep { $osh_command eq $_ } qw{ selfMFAResetPassword selfMFAResetTOTP })
-            && ($MFArequiredForPlugin eq 'none'))
-        {
 
-            # enforce MFA in those cases, even if it's not configured
-            $MFArequiredForPlugin = 'any';
-        }
-
-        # if the user wants to setup TOTP, if it happens to be already set (or any other factor), require it too
-        # note: this is not needed for selfMFASetupPassword, because `passwd` does the job of asking the previous password
-        elsif ($osh_command eq 'selfMFASetupTOTP'
-            && ($isMfaTOTPConfigured || $isMfaPasswordConfigured)
-            && ($MFArequiredForPlugin eq 'none'))
-        {
-            $MFArequiredForPlugin = 'any';
+        # These kind of plugins will require MFA if we have at least one already configured, none otherwise.
+        # This is mainly used by selfMFASetupTOTP, to ensure that the current TOTP is asked before allowing the user
+        # to setup a new one. Note that this is not used by selfMFASetupPassword, as `passwd` already asks for
+        # the current password before allowing to change it
+        if ($MFArequiredForPlugin eq 'any-if-configured') {
+            $MFArequiredForPlugin = (($isMfaTOTPConfigured || $isMfaPasswordConfigured) ? 'any' : 'none');
         }
 
         if (!grep { $MFArequiredForPlugin eq $_ } qw{ password totp any none }) {