Security: hold an agent who must set up MFA to the set up page on every request, not just at sign in, and fix the client portal two-step gate, which a path added to the URL or a second action sent with the set up one got past - both now go through requestIsMfaSetup(), which checks the script actually running and every key in the request; View as contact is refused while an agent still owes MFA set up, and agents are emailed when MFA is turned on

agent/user/post/profile.php

@@ -157,6 +157,11 @@ if (isset($_POST['enable_mfa'])) {
 
         logAudit("User Account", "Edit", "$session_name enabled MFA on their account");
 
+        // Setting MFA up needs only the password, so whoever holds it can do it - the email
+        // is how the real owner finds out an authenticator was added (the portal sends one too)
+        $me = userRow($session_user_id);
+        userNotifyChange($me['user_name'], $me['user_email'], 'multi-factor authentication was turned on');
+
         flashAlert("Multi-factor authentication is on - you will be asked for a code when you sign in");
 
         // Clear the mfa_token from the session to avoid re-use.

client/includes/check_login.php

@@ -105,9 +105,14 @@ if (!$portal_view_as) {
 }
 
 // Two-step sign-in required for password accounts (Admin > Client Portal) and
-// not set up yet: Your account until it is, and nothing else
+// not set up yet: Your account until it is, and nothing else. The one gate for the
+// pages and for client/post.php, judged on the script really running and every key
+// in the request (requestIsMfaSetup()).
 $portal_mfa_setup_needed = !$portal_view_as && intval($config_client_portal_require_mfa ?? 0) === 1 && $session_auth_method === 'local' && !$session_mfa_on;
-if ($portal_mfa_setup_needed && !in_array(basename($_SERVER['PHP_SELF']), ['profile.php', 'post.php'], true)) {
+if ($portal_mfa_setup_needed && !requestIsMfaSetup([
+    'client/profile.php' => [[['mfa'], []]],
+    'client/post.php' => [[[], ['csrf_token', 'portal_mfa_enable', 'verify_code']], [['logout'], []]],
+])) {
     redirect("/client/profile.php?mfa=setup");
 }
 

client/post.php

@@ -25,10 +25,8 @@ if (portalViewingAs() && !isset($_GET['export_statement_pdf'])) {
     redirect();
 }
 
-// Two-step sign-in required and not set up yet: setting it up, or signing out
-if ($portal_mfa_setup_needed && !isset($_POST['portal_mfa_enable']) && !isset($_GET['logout'])) {
-    redirect('profile.php?mfa=setup');
-}
+// Two-step sign-in required and not set up yet: client/includes/check_login.php has
+// already turned away everything but setting it up, or signing out
 
 if (isset($_POST['add_ticket'])) {
 

functions/auth.php

@@ -5,6 +5,31 @@
 
 // When provided a module name (e.g. module_support), returns the associated permission level (false=none, 1=read, 2=write, 3=full).
 // The role's levels are read once a request - a page asks dozens of times.
+/*
+ * Is this request one of the few a session may make while it still has to set up MFA?
+ *
+ * $allowed maps a script, from the app root, to the request shapes it may take - each a
+ * [GET keys, POST keys] pair, and every key in the request has to be one of them. The
+ * script is the file actually running, never PHP_SELF: a path tacked onto the end of a
+ * URL (client/tickets.php/profile.php) changes PHP_SELF but not what runs. And the keys
+ * are checked whole, because post handlers run one after another - a request that carries
+ * the set-up action and another one alongside it must not get the other one through.
+ */
+function requestIsMfaSetup(array $allowed) {
+    $root = realpath(dirname(__DIR__));
+    $script = realpath((string) ($_SERVER['SCRIPT_FILENAME'] ?? ''));
+    if ($root === false || $script === false || strpos($script, $root . DIRECTORY_SEPARATOR) !== 0) {
+        return false;
+    }
+    $script = str_replace(DIRECTORY_SEPARATOR, '/', substr($script, strlen($root) + 1));
+    foreach ($allowed[$script] ?? [] as [$get_keys, $post_keys]) {
+        if (!array_diff(array_keys($_GET), $get_keys) && !array_diff(array_keys($_POST), $post_keys)) {
+            return true;
+        }
+    }
+    return false;
+}
+
 function lookupUserPermission($module) {
     global $mysqli, $session_is_admin, $session_user_role;
     static $levels = [];

functions/portal.php

@@ -343,11 +343,17 @@ function portalViewAsAccess($agent_user_id, $contact_id) {
     if (intval($config_client_portal_enable) !== 1 || !$agent_user_id || !$contact_id) {
         return null;
     }
-    $agent = mysqli_fetch_assoc(mysqli_query($mysqli, "SELECT user_name, user_password, user_type, user_status, user_archived_at, user_role_id, role_is_admin
-        FROM users LEFT JOIN user_roles ON role_id = user_role_id WHERE user_id = $agent_user_id LIMIT 1"));
+    $agent = mysqli_fetch_assoc(mysqli_query($mysqli, "SELECT user_name, user_password, user_type, user_status, user_archived_at, user_role_id, role_is_admin,
+        user_token, user_config_force_mfa
+        FROM users LEFT JOIN user_roles ON role_id = user_role_id LEFT JOIN user_settings ON user_settings.user_id = users.user_id
+        WHERE users.user_id = $agent_user_id LIMIT 1"));
     if (!$agent || intval($agent['user_type']) !== 1 || intval($agent['user_status']) !== 1 || $agent['user_archived_at'] !== null) {
         return null;
     }
+    // Still has to set up the MFA their account requires (includes/load_user_session.php)
+    if (intval($agent['user_config_force_mfa'] ?? 0) === 1 && trim((string) $agent['user_token']) === '') {
+        return null;
+    }
     $admin = intval($agent['role_is_admin'] ?? 0) === 1;
     $level = function ($module) use ($mysqli, $admin, $agent) {
         if ($admin) {

includes/load_user_session.php

@@ -64,6 +64,17 @@ if (!hash_equals($session_stamp_current, (string) ($_SESSION['user_session_stamp
     redirect("/login.php");
 }
 
+// MFA required on this account and not set up yet: setting it up, or signing out, is all
+// this session may do - every other page, modal, AJAX call and POST goes back to the set
+// up page. Signing in only sends them there; this is what keeps them there, including when
+// an admin requires MFA, or resets it, on an agent who is already signed in.
+if ($session_user_config_force_mfa === 1 && trim((string) $session_token) === '' && !requestIsMfaSetup([
+    'agent/user/mfa_enforcement.php' => [[[], []]],
+    'agent/user/post.php' => [[[], ['csrf_token', 'enable_mfa', 'verify_code']], [['logout'], []]],
+])) {
+    redirect("/agent/user/mfa_enforcement.php");
+}
+
 // Load user client permissions (allow + deny lists)
 $user_client_access_sql = "SELECT client_id, permission_type FROM user_client_permissions WHERE user_id = $session_user_id";
 $user_client_access_result = mysqli_query($mysqli, $user_client_access_sql);