Moved TOTP codes out of the main users table.

database/2025_02_08_000526_create_users_totp_table.php

@@ -0,0 +1,33 @@
+<?php
+use Index\Base32;
+use Index\Db\DbConnection;
+use Index\Db\Migration\DbMigration;
+
+final class CreateUsersTotpTable_20250208_000526 implements DbMigration {
+    public function migrate(DbConnection $conn): void {
+        $conn->execute(<<<SQL
+            CREATE TABLE msz_users_totp (
+                user_id      INT UNSIGNED NOT NULL,
+                totp_secret  BINARY(16)   NOT NULL,
+                totp_created TIMESTAMP    NOT NULL DEFAULT NOW(),
+                PRIMARY KEY (user_id),
+                INDEX users_totp_created_index (totp_created),
+                CONSTRAINT users_totp_users_foreign
+                    FOREIGN KEY (user_id)
+                    REFERENCES msz_users (user_id)
+                    ON UPDATE CASCADE
+                    ON DELETE CASCADE
+            ) COLLATE='utf8mb4_bin';
+        SQL);
+
+        $result = $conn->query('SELECT user_id, user_totp_key FROM msz_users WHERE user_totp_key IS NOT NULL');
+        $stmt = $conn->prepare('INSERT INTO msz_users_totp (user_id, totp_secret) VALUES (?, ?)');
+        while($result->next()) {
+            $stmt->addParameter(1, $result->getString(0));
+            $stmt->addParameter(2, Base32::decode($result->getString(1)));
+            $stmt->execute();
+        }
+
+        $conn->execute('ALTER TABLE msz_users DROP COLUMN user_totp_key');
+    }
+}

public-legacy/auth/login.php

@@ -118,7 +118,7 @@ while(!empty($_POST['login']) && is_array($_POST['login'])) {
         break;
     }
 
-    if($userInfo->hasTOTP) {
+    if($msz->usersCtx->totps->hasUserTotp($userInfo)) {
         $tfaToken = $msz->authCtx->tfaSessions->createToken($userInfo);
         Tools::redirect($msz->urls->format('auth-two-factor', ['token' => $tfaToken, 'redirect' => $loginRedirect]));
         return;

public-legacy/auth/password.php

@@ -71,7 +71,8 @@ while($canResetPassword) {
 
         // also disables two factor auth to prevent getting locked out of account entirely
         // this behaviour should really be replaced with recovery keys...
-        $msz->usersCtx->users->updateUser($userInfo, password: $passwordNew, totpKey: '');
+        $msz->usersCtx->users->updateUser($userInfo, password: $passwordNew);
+        $msz->usersCtx->totps->deleteUserTotp($userInfo);
 
         $msz->createAuditLog('PASSWORD_RESET', [], $userInfo);
 

public-legacy/auth/twofactor.php

@@ -2,7 +2,7 @@
 namespace Misuzu;
 
 use RuntimeException;
-use Misuzu\TOTPGenerator;
+use Misuzu\TotpGenerator;
 use Misuzu\Auth\AuthTokenCookie;
 
 if(!isset($msz) || !($msz instanceof \Misuzu\MisuzuContext))
@@ -31,11 +31,8 @@ if(empty($tokenUserId)) {
     return;
 }
 
-$userInfo = $msz->usersCtx->users->getUser($tokenUserId, 'id');
+$totpInfo = $msz->usersCtx->totps->getUserTotp($tokenUserId);
-
+if($totpInfo === null) {
-// checking user_totp_key specifically because there's a fringe chance that
-//  there's a token present, but totp is actually disabled
-if(!$userInfo->hasTOTP) {
     Tools::redirect($msz->urls->format('auth-login'));
     return;
 }
@@ -60,30 +57,30 @@ while(!empty($twofactor)) {
     }
 
     $clientInfo = ClientInfo::fromRequest();
-    $totp = new TOTPGenerator($userInfo->totpKey);
+    $generator = $totpInfo->createGenerator();
 
-    if(!in_array($twofactor['code'], $totp->generateRange())) {
+    if(!in_array($twofactor['code'], $generator->generateRange())) {
         $notices[] = sprintf(
             "Invalid two factor code, %d attempt%s remaining",
             $remainingAttempts - 1,
             $remainingAttempts === 2 ? '' : 's'
         );
-        $msz->authCtx->loginAttempts->recordAttempt(false, $ipAddress, $countryCode, $userAgent, $clientInfo, $userInfo);
+        $msz->authCtx->loginAttempts->recordAttempt(false, $ipAddress, $countryCode, $userAgent, $clientInfo, $tokenUserId);
         break;
     }
 
-    $msz->authCtx->loginAttempts->recordAttempt(true, $ipAddress, $countryCode, $userAgent, $clientInfo, $userInfo);
+    $msz->authCtx->loginAttempts->recordAttempt(true, $ipAddress, $countryCode, $userAgent, $clientInfo, $tokenUserId);
     $msz->authCtx->tfaSessions->deleteToken($tokenString);
 
     try {
-        $sessionInfo = $msz->authCtx->sessions->createSession($userInfo, $ipAddress, $countryCode, $userAgent, $clientInfo);
+        $sessionInfo = $msz->authCtx->sessions->createSession($tokenUserId, $ipAddress, $countryCode, $userAgent, $clientInfo);
     } catch(RuntimeException $ex) {
         $notices[] = "Something broke while creating a session for you, please tell an administrator or developer about this!";
         break;
     }
 
     $tokenBuilder = $msz->authInfo->tokenInfo->toBuilder();
-    $tokenBuilder->setUserId($userInfo);
+    $tokenBuilder->setUserId($tokenUserId);
     $tokenBuilder->setSessionToken($sessionInfo);
     $tokenBuilder->removeImpersonatedUserId();
     $tokenInfo = $tokenBuilder->toInfo();

public-legacy/settings/account.php

@@ -15,6 +15,7 @@ if(!$msz->authInfo->loggedIn)
 $errors = [];
 $userInfo = $msz->authInfo->userInfo;
 $isRestricted = $msz->usersCtx->hasActiveBan($userInfo);
+$hasTotp = $msz->usersCtx->totps->hasUserTotp($userInfo);
 $isVerifiedRequest = CSRF::validateRequest();
 
 if(!$isRestricted && $isVerifiedRequest && !empty($_POST['role'])) {
@@ -47,28 +48,29 @@ if(!$isRestricted && $isVerifiedRequest && !empty($_POST['role'])) {
     }
 }
 
-if($isVerifiedRequest && isset($_POST['tfa']['enable']) && $userInfo->hasTOTP !== (bool)$_POST['tfa']['enable']) {
+if($isVerifiedRequest && isset($_POST['tfa']['enable']) && $msz->usersCtx->totps->hasUserTotp($userInfo) !== (bool)$_POST['tfa']['enable']) {
-    $totpKey = '';
-
     if((bool)$_POST['tfa']['enable']) {
-        $totpKey = TOTPGenerator::generateKey();
+        $totpInfo = $msz->usersCtx->totps->createUserTotp($userInfo);
+        $totpSecret = $totpInfo->encodedSecret;
         $totpIssuer = $msz->siteInfo->name;
         $totpQrcode = (new QRCode(new QROptions([
             'version'    => 5,
             'outputType' => QRCode::OUTPUT_IMAGE_JPG,
             'eccLevel'   => QRCode::ECC_L,
         ])))->render(sprintf('otpauth://totp/%s:%s?%s', $totpIssuer, $userInfo->name, http_build_query([
-            'secret' => $totpKey,
+            'secret' => $totpSecret,
             'issuer' => $totpIssuer,
         ])));
 
+        $hasTotp = true;
         Template::set([
-            'settings_2fa_code' => $totpKey,
+            'settings_2fa_code' => $totpSecret,
             'settings_2fa_image' => $totpQrcode,
         ]);
+    } else {
+        $hasTotp = false;
+        $msz->usersCtx->totps->deleteUserTotp($userInfo);
     }
-
-    $msz->usersCtx->users->updateUser(userInfo: $userInfo, totpKey: $totpKey);
 }
 
 if($isVerifiedRequest && !empty($_POST['current_password'])) {
@@ -122,4 +124,5 @@ Template::render('settings.account', [
     'settings_user' => $userInfo,
     'settings_roles' => $userRoles,
     'is_restricted' => $isRestricted,
+    'has_totp' => $hasTotp,
 ]);

public-legacy/settings/data.php

@@ -143,7 +143,7 @@ if(isset($_POST['action']) && is_string($_POST['action'])) {
                             $tmpFiles[] = db_to_zip($archive, $userInfo, 'perms_calculated',       ['user_id:s:n', 'forum_id:s:n', 'perms_category:s', 'perms_calculated:i']);
                             $tmpFiles[] = db_to_zip($archive, $userInfo, 'profile_fields_values',  ['field_id:s', 'user_id:s', 'format_id:s', 'field_value:s']);
                             $tmpFiles[] = db_to_zip($archive, $userInfo, 'sessions',               ['session_id:s', 'user_id:s', 'session_key:n', 'session_remote_addr_first:a', 'session_remote_addr_last:a:n', 'session_user_agent:s', 'session_country:s', 'session_expires:t', 'session_expires_bump:b', 'session_created:t', 'session_active:t:n']);
-                            $tmpFiles[] = db_to_zip($archive, $userInfo, 'users',                  ['user_id:s', 'user_name:s', 'user_password:n', 'user_email:s', 'user_remote_addr_first:a', 'user_remote_addr_last:a', 'user_super:b', 'user_country:s', 'user_colour:i:n', 'user_created:t', 'user_active:t:n', 'user_deleted:t:n', 'user_display_role_id:s:n', 'user_totp_key:n', 'user_about_content:s:n', 'user_about_parser:i', 'user_signature_content:s:n', 'user_signature_parser:i', 'user_birthdate:s:n', 'user_background_settings:i:n', 'user_title:s:n']);
+                            $tmpFiles[] = db_to_zip($archive, $userInfo, 'users',                  ['user_id:s', 'user_name:s', 'user_password:n', 'user_email:s', 'user_remote_addr_first:a', 'user_remote_addr_last:a', 'user_super:b', 'user_country:s', 'user_colour:i:n', 'user_created:t', 'user_active:t:n', 'user_deleted:t:n', 'user_display_role_id:s:n', 'user_about_content:s:n', 'user_about_parser:i', 'user_signature_content:s:n', 'user_signature_parser:i', 'user_birthdate:s:n', 'user_background_settings:i:n', 'user_title:s:n']);
                             $tmpFiles[] = db_to_zip($archive, $userInfo, 'users_bans',             ['ban_id:s', 'user_id:s', 'mod_id:n', 'ban_severity:i', 'ban_reason_public:s', 'ban_reason_private:s', 'ban_created:t', 'ban_expires:t:n']);
                             $tmpFiles[] = db_to_zip($archive, $userInfo, 'users_password_resets',  ['reset_id:s', 'user_id:s', 'reset_remote_addr:a', 'reset_requested:t', 'reset_code:n']);
                             $tmpFiles[] = db_to_zip($archive, $userInfo, 'users_warnings',         ['warn_id:s', 'user_id:s', 'mod_id:n', 'warn_body:s', 'warn_created:t']);

src/TotpGenerator.php

@@ -2,17 +2,14 @@
 namespace Misuzu;
 
 use InvalidArgumentException;
-use Index\Base32;
 
-class TOTPGenerator {
+class TotpGenerator {
     public const DIGITS = 6;
     public const INTERVAL = 30000;
 
-    public function __construct(private string $secretKey) {}
+    public function __construct(
-
+        #[\SensitiveParameter] private string $secret
-    public static function generateKey(): string {
+    ) {}
-        return Base32::encode(random_bytes(16));
-    }
 
     public static function timecode(?int $timestamp = null): int {
         $timestamp ??= time();
@@ -22,7 +19,7 @@ class TOTPGenerator {
     public function generate(?int $timecode = null): string {
         $timecode ??= self::timecode();
 
-        $hash = hash_hmac('sha1', pack('J', $timecode), Base32::decode($this->secretKey), true);
+        $hash = hash_hmac('sha1', pack('J', $timecode), $this->secret, true);
         $offset = ord($hash[strlen($hash) - 1]) & 0x0F;
 
         $bin = 0;

src/Users/UserInfo.php

@@ -21,7 +21,6 @@ class UserInfo {
         public private(set) ?int $lastActiveTime,
         public private(set) ?int $deletedTime,
         public private(set) ?string $displayRoleId,
-        #[\SensitiveParameter] public private(set) ?string $totpKey,
         public private(set) ?string $aboutBody,
         public private(set) int $aboutBodyParser,
         public private(set) ?string $signatureBody,
@@ -46,14 +45,13 @@ class UserInfo {
             lastActiveTime: $result->getIntegerOrNull(10),
             deletedTime: $result->getIntegerOrNull(11),
             displayRoleId: $result->getStringOrNull(12),
-            totpKey: $result->getStringOrNull(13),
+            aboutBody: $result->getStringOrNull(13),
-            aboutBody: $result->getStringOrNull(14),
+            aboutBodyParser: $result->getInteger(14),
-            aboutBodyParser: $result->getInteger(15),
+            signatureBody: $result->getStringOrNull(15),
-            signatureBody: $result->getStringOrNull(16),
+            signatureBodyParser: $result->getInteger(16),
-            signatureBodyParser: $result->getInteger(17),
+            birthdateRaw: $result->getStringOrNull(17),
-            birthdateRaw: $result->getStringOrNull(18),
+            backgroundSettings: $result->getIntegerOrNull(18),
-            backgroundSettings: $result->getIntegerOrNull(19),
+            title: $result->getString(19),
-            title: $result->getString(20),
         );
     }
 
@@ -93,10 +91,6 @@ class UserInfo {
         get => $this->deletedTime === null ? null : CarbonImmutable::createFromTimestampUTC($this->deletedTime);
     }
 
-    public bool $hasTOTP {
-        get => $this->totpKey !== null;
-    }
-
     public bool $isAboutBodyPlain {
         get => $this->aboutBodyParser === Parser::PLAIN;
     }

src/Users/UserTotpInfo.php

@@ -0,0 +1,35 @@
+<?php
+namespace Misuzu\Users;
+
+use Carbon\CarbonImmutable;
+use Index\Base32;
+use Index\Db\DbResult;
+use Misuzu\TotpGenerator;
+
+class UserTotpInfo {
+    public function __construct(
+        public private(set) ?string $userId,
+        #[\SensitiveParameter] public private(set) string $secret,
+        public private(set) int $createdTime
+    ) {}
+
+    public static function fromResult(DbResult $result): UserTotpInfo {
+        return new UserTotpInfo(
+            userId: $result->getString(0),
+            secret: $result->getString(1),
+            createdTime: $result->getInteger(2),
+        );
+    }
+
+    public CarbonImmutable $createdAt {
+        get => CarbonImmutable::createFromTimestampUTC($this->createdTime);
+    }
+
+    public string $encodedSecret {
+        get => Base32::encode($this->secret);
+    }
+
+    public function createGenerator(): TotpGenerator {
+        return new TotpGenerator($this->secret);
+    }
+}

src/Users/UserTotpsData.php

@@ -0,0 +1,64 @@
+<?php
+namespace Misuzu\Users;
+
+use RuntimeException;
+use Index\Db\{DbConnection,DbStatementCache};
+
+class UserTotpsData {
+    private DbStatementCache $cache;
+
+    public function __construct(DbConnection $dbConn) {
+        $this->cache = new DbStatementCache($dbConn);
+    }
+
+    public function hasUserTotp(UserInfo|string $userInfo): bool {
+        $stmt = $this->cache->get(<<<SQL
+            SELECT COUNT(*)
+            FROM msz_users_totp
+            WHERE user_id = ?
+        SQL);
+        $stmt->nextParameter($userInfo instanceof UserInfo ? $userInfo->id : $userInfo);
+        $stmt->execute();
+
+        $result = $stmt->getResult();
+        return $result->next() && $result->getBoolean(0);
+    }
+
+    public function getUserTotp(UserInfo|string $userInfo): ?UserTotpInfo {
+        $stmt = $this->cache->get(<<<SQL
+            SELECT user_id, totp_secret, UNIX_TIMESTAMP(totp_created)
+            FROM msz_users_totp
+            WHERE user_id = ?
+        SQL);
+        $stmt->nextParameter($userInfo instanceof UserInfo ? $userInfo->id : $userInfo);
+        $stmt->execute();
+
+        $result = $stmt->getResult();
+        return $result->next() ? UserTotpInfo::fromResult($result) : null;
+    }
+
+    public function deleteUserTotp(UserInfo|string $userInfo): void {
+        $stmt = $this->cache->get(<<<SQL
+            DELETE FROM msz_users_totp
+            WHERE user_id = ?
+        SQL);
+        $stmt->nextParameter($userInfo instanceof UserInfo ? $userInfo->id : $userInfo);
+        $stmt->execute();
+    }
+
+    public function createUserTotp(UserInfo|string $userInfo): UserTotpInfo {
+        $stmt = $this->cache->get(<<<SQL
+            REPLACE INTO msz_users_totp (
+                user_id, totp_secret
+            ) VALUES (?, RANDOM_BYTES(16))
+        SQL);
+        $stmt->nextParameter($userInfo instanceof UserInfo ? $userInfo->id : $userInfo);
+        $stmt->execute();
+
+        $totpInfo = $this->getUserTotp($userInfo);
+        if($totpInfo === null)
+            throw new RuntimeException('failed to create totp');
+
+        return $totpInfo;
+    }
+}

src/Users/UsersContext.php

@@ -10,6 +10,7 @@ class UsersContext {
     public private(set) BansData $bans;
     public private(set) WarningsData $warnings;
     public private(set) ModNotesData $modNotes;
+    public private(set) UserTotpsData $totps;
 
     /** @var array<string, UserInfo> */
     private array $userInfos = [];
@@ -29,6 +30,7 @@ class UsersContext {
         $this->bans = new BansData($dbConn);
         $this->warnings = new WarningsData($dbConn);
         $this->modNotes = new ModNotesData($dbConn);
+        $this->totps = new UserTotpsData($dbConn);
     }
 
     public function getUserInfo(string $value, int|string|null $select = null): UserInfo {

src/Users/UsersData.php

@@ -169,7 +169,7 @@ class UsersData {
                 UNIX_TIMESTAMP(user_created),
                 UNIX_TIMESTAMP(user_active),
                 UNIX_TIMESTAMP(user_deleted),
-                user_display_role_id, user_totp_key, user_about_content,
+                user_display_role_id, user_about_content,
                 user_about_parser, user_signature_content,
                 user_signature_parser, user_birthdate,
                 user_background_settings, user_title
@@ -267,7 +267,7 @@ class UsersData {
                 UNIX_TIMESTAMP(user_created),
                 UNIX_TIMESTAMP(user_active),
                 UNIX_TIMESTAMP(user_deleted),
-                user_display_role_id, user_totp_key, user_about_content,
+                user_display_role_id, user_about_content,
                 user_about_parser, user_signature_content,
                 user_signature_parser, user_birthdate,
                 user_background_settings, user_title
@@ -339,7 +339,6 @@ class UsersData {
         ?string $countryCode = null,
         ?Colour $colour = null,
         RoleInfo|string|null $displayRoleInfo = null,
-        ?string $totpKey = null,
         ?string $aboutBody = null,
         ?int $aboutBodyParser = null,
         ?string $signatureBody = null,
@@ -394,11 +393,6 @@ class UsersData {
             $values[] = $displayRoleInfo;
         }
 
-        if($totpKey !== null) {
-            $fields[] = 'user_totp_key = ?';
-            $values[] = $totpKey === '' ? null : $totpKey;
-        }
-
         if($aboutBody !== null && $aboutBodyParser !== null) {
             if(self::validateProfileAbout($aboutBodyParser, $aboutBody) !== '')
                 throw new InvalidArgumentException('$aboutBody and $aboutBodyParser contain invalid data!');

templates/settings/account.twig

@@ -130,7 +130,7 @@
             {% endif %}
 
             <div class="settings__two-factor__settings">
-                {% if settings_user.hasTOTP %}
+                {% if has_totp %}
                     <div class="settings__two-factor__settings__status">
                         <i class="fas fa-lock fa-fw"></i> Two Factor Authentication is enabled!
                     </div>