Improved two factor login flow.

assets/less/classes/input/text.less

@@ -19,4 +19,8 @@
     &--monospace {
         font-family: @mio-font-mono;
     }
+
+    &--centre {
+        text-align: center;
+    }
 }

database/2019_03_10_161059_add_tfa_login_token_table.php

@@ -0,0 +1,30 @@
+<?php
+namespace Misuzu\DatabaseMigrations\AddTfaLoginTokenTable;
+
+use PDO;
+
+function migrate_up(PDO $conn): void
+{
+    $conn->exec("
+        CREATE TABLE `msz_auth_tfa` (
+            `user_id`       INT UNSIGNED    NOT NULL,
+            `tfa_token`     CHAR(32)        NOT NULL,
+            `tfa_created`   TIMESTAMP       NOT NULL DEFAULT CURRENT_TIMESTAMP,
+            UNIQUE INDEX    `auth_tfa_token_unique`     (`tfa_token`),
+            INDEX           `auth_tfa_user_foreign`     (`user_id`),
+            INDEX           `auth_tfa_created_index`    (`tfa_created`),
+            CONSTRAINT `auth_tfa_user_foreign`
+                FOREIGN KEY (`user_id`)
+                REFERENCES `msz_users` (`user_id`)
+                ON UPDATE CASCADE
+                ON DELETE CASCADE
+        );
+    ");
+}
+
+function migrate_down(PDO $conn): void
+{
+    $conn->exec("
+        DROP TABLE `msz_auth_tfa`;
+    ");
+}

misuzu.php

@@ -57,6 +57,7 @@ require_once 'src/Forum/validate.php';
 require_once 'src/Net/geoip.php';
 require_once 'src/Net/ip.php';
 require_once 'src/Parsers/parse.php';
+require_once 'src/Users/auth.php';
 require_once 'src/Users/avatar.php';
 require_once 'src/Users/background.php';
 require_once 'src/Users/login_attempt.php';
@@ -208,6 +209,15 @@ if (PHP_SAPI === 'cli') {
                         'run' => $runLowFreq,
                         'command' => 'forum_count_synchronise',
                     ],
+                    [
+                        'name' => 'Clean up expired tfa tokens.',
+                        'type' => 'sql',
+                        'run' => true,
+                        'command' => "
+                            DELETE FROM `msz_auth_tfa`
+                            WHERE `tfa_created` < NOW() - INTERVAL 15 MINUTE
+                        ",
+                    ],
                 ];
 
                 foreach ($cronTasks as $cronTask) {

public/auth/login.php

@@ -56,41 +56,27 @@ while (!empty($login->value('array'))) {
     }
 
     $loginPassword = $login->password->value('string', '');
-
-    if (isset($login->tfa)) {
-        $loginPassword = openssl_decrypt($loginPassword, 'aes-256-ecb', config_get_default('insecure', 'Auth', 'password_key'));
-    }
-
     if (!password_verify($loginPassword, $userData['password'])) {
         user_login_attempt_record(false, $userData['user_id'], $ipAddress, $userAgent);
         $notices[] = $loginFailedError;
         break;
     }
 
+    if (user_password_needs_rehash($userData['password'])) {
+        user_password_set($userData['user_id'], $loginPassword);
+    }
+
     if ($loginPermission > 0 && !perms_check_user(MSZ_PERMS_GENERAL, $userData['user_id'], $loginPermission)) {
         $notices[] = "Login succeeded, but you're not allowed to browse the site right now.";
         user_login_attempt_record(true, $userData['user_id'], $ipAddress, $userAgent);
         break;
     }
 
-    if (!empty($userData['user_totp_key'])) {
+    if ($userData['totp_enabled']) {
-        $currentCode = totp_generate($userData['user_totp_key']);
+        header(sprintf('Location: %s', url('auth-two-factor', [
-
+            'token' => user_auth_tfa_token_create($userData['user_id']),
-        if (isset($login->tfa) && $currentCode !== $login->tfa->value('string', '')) {
+        ])));
-            $notices[] = "Invalid two factor code, {$attemptsRemainingError}.";
+        return;
-            user_login_attempt_record(false, $userData['user_id'], $ipAddress, $userAgent);
-        }
-
-        if (!isset($login->tfa) || !empty($notices)) {
-            echo tpl_render('auth.twofactor', [
-                'login_notices' => $notices,
-                'login_username' => $loginUsername,
-                'login_redirect' => $loginRedirect,
-                // red flags, this should be fine but probably replaced with a token system going forward
-                'login_password' => openssl_encrypt($loginPassword, 'aes-256-ecb', config_get_default('insecure', 'Auth', 'password_key')),
-            ]);
-            return;
-        }
     }
 
     user_login_attempt_record(true, $userData['user_id'], $ipAddress, $userAgent);
@@ -103,10 +89,6 @@ while (!empty($login->value('array'))) {
 
     user_session_start($userData['user_id'], $sessionKey);
 
-    if (user_password_needs_rehash($userData['password'])) {
-        user_password_set($userData['user_id'], $loginPassword);
-    }
-
     $cookieLife = strtotime(user_session_current('session_expires'));
     $cookieValue = base64url_encode(user_session_cookie_pack($userData['user_id'], $sessionKey));
     setcookie('msz_auth', $cookieValue, $cookieLife, '/', '', true, true);
@@ -121,7 +103,7 @@ while (!empty($login->value('array'))) {
 
 $welcomeMode = RequestVar::get()->welcome->value('bool', false);
 $loginUsername = $login->username->value('string') ?? RequestVar::get()->username->value('string', '');
-$loginRedirect = $welcomeMode ? '/' : RequestVar::get()->redirect->value('string') ?? $_SERVER['HTTP_REFERER'] ?? '/';
+$loginRedirect = $welcomeMode ? url('index') : RequestVar::get()->redirect->value('string') ?? $_SERVER['HTTP_REFERER'] ?? url('index');
 $sitePrivateMessage = $siteIsPrivate ? config_get_default('', 'Private', 'message') : '';
 $canResetPassword = $siteIsPrivate ? boolval(config_get_default(false, 'Private', 'password_reset')) : true;
 $canRegisterAccount = !$siteIsPrivate;

public/auth/twofactor.php

@@ -0,0 +1,85 @@
+<?php
+use Misuzu\Request\RequestVar;
+
+require_once '../../misuzu.php';
+
+if (user_session_active()) {
+    header(sprintf('Location: %s', url('index')));
+    return;
+}
+
+$twofactor = RequestVar::post()->twofactor;
+$notices = [];
+$ipAddress = ip_remote_address();
+$remainingAttempts = user_login_attempts_remaining($ipAddress);
+$tokenInfo = user_auth_tfa_token_info(
+    RequestVar::get()->token->value('string') ?? $twofactor->token->value('string', '')
+);
+
+// checking user_totp_key specifically because there's a fringe chance that
+//  there's a token present, but totp is actually disabled
+if (empty($tokenInfo['user_totp_key'])) {
+    header(sprintf('Location: %s', url('auth-login')));
+    return;
+}
+
+while (!empty($twofactor->value('array'))) {
+    if (!csrf_verify('twofactor', $_POST['csrf'] ?? '')) {
+        $notices[] = 'Was unable to verify the request, please try again!';
+        break;
+    }
+
+    $userAgent = $_SERVER['HTTP_USER_AGENT'] ?? '';
+    $redirect = $twofactor->redirect->value('string', '');
+
+    if ($twofactor->code->empty()) {
+        $notices[] = 'Code field was empty.';
+        break;
+    }
+
+    if ($remainingAttempts < 1) {
+        $notices[] = 'There are too many failed login attempts from your IP address, please try again later.';
+        break;
+    }
+
+    $currentCode = totp_generate($tokenInfo['user_totp_key']);
+
+    if ($currentCode !== $twofactor->code->value('string', '')) {
+        $notices[] = sprintf(
+            "Invalid two factor code, %d attempt%s remaining",
+            $remainingAttempts - 1,
+            $remainingAttempts === 2 ? '' : 's'
+        );
+        user_login_attempt_record(false, $tokenInfo['user_id'], $ipAddress, $userAgent);
+        break;
+    }
+
+    user_login_attempt_record(true, $tokenInfo['user_id'], $ipAddress, $userAgent);
+    $sessionKey = user_session_create($tokenInfo['user_id'], $ipAddress, $userAgent);
+
+    if (empty($sessionKey)) {
+        $notices[] = "Something broke while creating a session for you, please tell an administrator or developer about this!";
+        break;
+    }
+
+    user_auth_tfa_token_invalidate($tokenInfo['tfa_token']);
+    user_session_start($tokenInfo['user_id'], $sessionKey);
+
+    $cookieLife = strtotime(user_session_current('session_expires'));
+    $cookieValue = base64url_encode(user_session_cookie_pack($tokenInfo['user_id'], $sessionKey));
+    setcookie('msz_auth', $cookieValue, $cookieLife, '/', '', true, true);
+
+    if (!is_local_url($redirect)) {
+        $redirect = url('index');
+    }
+
+    header("Location: {$redirect}");
+    return;
+}
+
+echo tpl_render('auth.twofactor', [
+    'twofactor_notices' => $notices,
+    'twofactor_redirect' => RequestVar::get()->redirect->value('string') ?? url('index'),
+    'twofactor_attempts_remaining' => $remainingAttempts,
+    'twofactor_token' => $tokenInfo['tfa_token'],
+]);

src/Users/auth.php

@@ -0,0 +1,56 @@
+<?php
+define('MSZ_AUTH_TFA_TOKENS_SIZE', 16); // * 2
+
+function user_auth_tfa_token_generate(): string
+{
+    return bin2hex(random_bytes(MSZ_AUTH_TFA_TOKENS_SIZE));
+}
+
+function user_auth_tfa_token_create(int $userId): string
+{
+    if ($userId < 1) {
+        return '';
+    }
+
+    $token = user_auth_tfa_token_generate();
+
+    $createToken = db_prepare('
+        INSERT INTO `msz_auth_tfa`
+            (`user_id`, `tfa_token`)
+        VALUES
+            (:user_id, :token)
+    ');
+    $createToken->bindValue('user_id', $userId);
+    $createToken->bindValue('token', $token);
+
+    if (!$createToken->execute()) {
+        return '';
+    }
+
+    return $token;
+}
+
+function user_auth_tfa_token_invalidate(string $token): void
+{
+    $deleteToken = db_prepare('
+        DELETE FROM `msz_auth_tfa`
+        WHERE `tfa_token` = :token
+    ');
+    $deleteToken->bindValue('token', $token);
+    $deleteToken->execute();
+}
+
+function user_auth_tfa_token_info(string $token): array
+{
+    $getTokenInfo = db_prepare('
+        SELECT
+            at.`user_id`, at.`tfa_token`, at.`tfa_created`, u.`user_totp_key`
+        FROM `msz_auth_tfa` AS at
+        LEFT JOIN `msz_users` AS u
+        ON u.`user_id` = at.`user_id`
+        WHERE at.`tfa_token` = :token
+        AND at.`tfa_created` >= NOW() - INTERVAL 15 MINUTE
+    ');
+    $getTokenInfo->bindValue('token', $token);
+    return db_fetch($getTokenInfo);
+}

src/Users/user.php

@@ -59,7 +59,7 @@ function user_create(
 function user_find_for_login(string $usernameOrMail): array
 {
     $getUser = db_prepare('
-        SELECT `user_id`, `password`, `user_totp_key`
+        SELECT `user_id`, `password`, `user_totp_key` IS NOT NULL AS `totp_enabled`
         FROM `msz_users`
         WHERE LOWER(`email`) = LOWER(:email)
         OR LOWER(`username`) = LOWER(:username)
@@ -126,7 +126,9 @@ function user_totp_info(int $userId): array
     }
 
     $getTwoFactorInfo = db_prepare('
-        SELECT `username`, `user_totp_key` IS NOT NULL AS `totp_enabled`
+        SELECT
+            `username`, `user_totp_key`,
+            `user_totp_key` IS NOT NULL AS `totp_enabled`
         FROM `msz_users`
         WHERE `user_id` = :user_id
     ');

src/url.php

@@ -19,6 +19,7 @@ define('MSZ_URLS', [
     'auth-reset'                        => ['/auth/password.php',       ['user' => '<user>']],
     'auth-logout'                       => ['/auth/logout.php',         ['token' => '{logout}']],
     'auth-resolve-user'                 => ['/auth/login.php',          ['resolve_user' => '<username>']],
+    'auth-two-factor'                   => ['/auth/twofactor.php',      ['token' => '<token>']],
 
     'changelog-index'                   => ['/changelog.php'],
     'changelog-change'                  => ['/changelog.php',           ['c' => '<change>']],

templates/auth/twofactor.twig

@@ -5,18 +5,17 @@
 {% set title = 'Two Factor Authentication' %}
 
 {% block content %}
-    <form class="container auth__container auth__twofactor" method="post" action="{{ url('auth-login') }}">
+    <form class="container auth__container auth__twofactor" method="post" action="{{ url('auth-two-factor') }}">
         {{ container_title('<i class="fas fa-user-shield fa-fw"></i> Two Factor Authentication') }}
 
-        {{ input_csrf('login') }}
+        {{ input_csrf('twofactor') }}
-        {{ input_hidden('login[redirect]', login_redirect) }}
+        {{ input_hidden('twofactor[redirect]', twofactor_redirect) }}
-        {{ input_hidden('login[username]', login_username) }}
+        {{ input_hidden('twofactor[token]', twofactor_token) }}
-        {{ input_hidden('login[password]', login_password) }}
 
-        {% if login_notices|length > 0 %}
+        {% if twofactor_notices|length > 0 %}
             <div class="warning auth__warning">
                 <div class="warning__content">
-                    {% for notice in login_notices %}
+                    {% for notice in twofactor_notices %}
                         <p class="auth__warning__paragraph">{{ notice }}</p>
                     {% endfor %}
                 </div>
@@ -28,7 +27,7 @@
                Code
             </div>
             <div class="auth__label__value">
-                {{ input_text('login[tfa]', 'input__text--monospace auth__label__input', '', 'text', '', true, {'maxlength':6}, 1) }}
+                {{ input_text('twofactor[code]', 'input__text--monospace input__text--centre auth__label__input', '', 'text', '', true, {'maxlength':6}, 1) }}
             </div>
         </label>