From b4d4e8578cf584560464df5597e9aba40041bbd6 Mon Sep 17 00:00:00 2001 From: flashwave Date: Thu, 27 Jul 2023 12:44:50 +0000 Subject: [PATCH] Rewrote TFA session code. --- public-legacy/auth/login.php | 4 +- public-legacy/auth/twofactor.php | 21 ++++---- src/Auth/TwoFactorAuthSessions.php | 48 +++++++++++++++++ src/MisuzuContext.php | 7 +++ src/Users/UserAuthSession.php | 86 ------------------------------ 5 files changed, 66 insertions(+), 100 deletions(-) create mode 100644 src/Auth/TwoFactorAuthSessions.php delete mode 100644 src/Users/UserAuthSession.php diff --git a/public-legacy/auth/login.php b/public-legacy/auth/login.php index 6617694..2215358 100644 --- a/public-legacy/auth/login.php +++ b/public-legacy/auth/login.php @@ -3,7 +3,6 @@ namespace Misuzu; use RuntimeException; use Misuzu\Users\User; -use Misuzu\Users\UserAuthSession; use Misuzu\Users\UserSession; if(UserSession::hasCurrent()) { @@ -117,8 +116,9 @@ while(!empty($_POST['login']) && is_array($_POST['login'])) { } if($userInfo->hasTOTP()) { + $tfaToken = $msz->getTFASessions()->createToken($userInfo); url_redirect('auth-two-factor', [ - 'token' => UserAuthSession::create($userInfo)->getToken(), + 'token' => $tfaToken, ]); return; } diff --git a/public-legacy/auth/twofactor.php b/public-legacy/auth/twofactor.php index 80e11c5..c930b44 100644 --- a/public-legacy/auth/twofactor.php +++ b/public-legacy/auth/twofactor.php @@ -4,7 +4,6 @@ namespace Misuzu; use RuntimeException; use Misuzu\Users\User; use Misuzu\Users\UserSession; -use Misuzu\Users\UserAuthSession; if(UserSession::hasCurrent()) { url_redirect('index'); @@ -17,23 +16,21 @@ $userAgent = $_SERVER['HTTP_USER_AGENT'] ?? ''; $twofactor = !empty($_POST['twofactor']) && is_array($_POST['twofactor']) ? $_POST['twofactor'] : []; $notices = []; +$tfaSessions = $msz->getTFASessions(); $loginAttempts = $msz->getLoginAttempts(); $remainingAttempts = $loginAttempts->countRemainingAttempts($ipAddress); -try { - $tokenInfo = UserAuthSession::byToken( - !empty($_GET['token']) && is_string($_GET['token']) ? $_GET['token'] : ( - !empty($twofactor['token']) && is_string($twofactor['token']) ? $twofactor['token'] : '' - ) - ); -} catch(RuntimeException $ex) {} +$tokenString = !empty($_GET['token']) && is_string($_GET['token']) ? $_GET['token'] : ( + !empty($twofactor['token']) && is_string($twofactor['token']) ? $twofactor['token'] : '' +); -if(empty($tokenInfo) || $tokenInfo->hasExpired()) { +$tokenUserId = $tfaSessions->getTokenUserId($tokenString); +if(empty($tokenUserId)) { url_redirect('auth-login'); return; } -$userInfo = $tokenInfo->getUser(); +$userInfo = User::byId((int)$tokenUserId); // checking user_totp_key specifically because there's a fringe chance that // there's a token present, but totp is actually disabled @@ -72,7 +69,7 @@ while(!empty($twofactor)) { } $loginAttempts->recordAttempt(true, $ipAddress, $countryCode, $userAgent, ClientInfo::fromRequest(), $userInfo); - $tokenInfo->delete(); + $tfaSessions->deleteToken($tokenString); try { $sessionInfo = UserSession::create($userInfo, $ipAddress, $countryCode); @@ -97,5 +94,5 @@ Template::render('auth.twofactor', [ 'twofactor_notices' => $notices, 'twofactor_redirect' => !empty($_GET['redirect']) && is_string($_GET['redirect']) ? $_GET['redirect'] : url('index'), 'twofactor_attempts_remaining' => $remainingAttempts, - 'twofactor_token' => $tokenInfo->getToken(), + 'twofactor_token' => $tokenString, ]); diff --git a/src/Auth/TwoFactorAuthSessions.php b/src/Auth/TwoFactorAuthSessions.php new file mode 100644 index 0000000..51c377b --- /dev/null +++ b/src/Auth/TwoFactorAuthSessions.php @@ -0,0 +1,48 @@ +cache = new DbStatementCache($dbConn); + } + + private static function generateToken(): string { + return XString::random(32); + } + + public function createToken(User|string $userInfo): string { + if($userInfo instanceof User) + $userInfo = (string)$userInfo->getId(); + + $token = self::generateToken(); + + $stmt = $this->cache->get('INSERT INTO msz_auth_tfa (user_id, tfa_token) VALUES (?, ?)'); + $stmt->addParameter(1, $userInfo); + $stmt->addParameter(2, $token); + $stmt->execute(); + + return $token; + } + + public function getTokenUserId(string $token): string { + $stmt = $this->cache->get('SELECT user_id FROM msz_auth_tfa WHERE tfa_token = ? AND tfa_created > NOW() - INTERVAL 15 MINUTE'); + $stmt->addParameter(1, $token); + $stmt->execute(); + $result = $stmt->getResult(); + + return $result->next() ? $result->getString(0) : ''; + } + + public function deleteToken(string $token): void { + $stmt = $this->cache->get('DELETE FROM msz_auth_tfa WHERE tfa_token = ?'); + $stmt->addParameter(1, $token); + $stmt->execute(); + } +} diff --git a/src/MisuzuContext.php b/src/MisuzuContext.php index 17393f9..bf0e79b 100644 --- a/src/MisuzuContext.php +++ b/src/MisuzuContext.php @@ -4,6 +4,7 @@ namespace Misuzu; use Misuzu\Template; use Misuzu\Auth\LoginAttempts; use Misuzu\Auth\RecoveryTokens; +use Misuzu\Auth\TwoFactorAuthSessions; use Misuzu\AuditLog\AuditLog; use Misuzu\Changelog\Changelog; use Misuzu\Comments\Comments; @@ -44,6 +45,7 @@ class MisuzuContext { private ModNotes $modNotes; private Bans $bans; private Warnings $warnings; + private TwoFactorAuthSessions $tfaSessions; public function __construct(IDbConnection $dbConn, IConfig $config) { $this->dbConn = $dbConn; @@ -58,6 +60,7 @@ class MisuzuContext { $this->modNotes = new ModNotes($this->dbConn); $this->bans = new Bans($this->dbConn); $this->warnings = new Warnings($this->dbConn); + $this->tfaSessions = new TwoFactorAuthSessions($this->dbConn); } public function getDbConn(): IDbConnection { @@ -125,6 +128,10 @@ class MisuzuContext { return $this->warnings; } + public function getTFASessions(): TwoFactorAuthSessions { + return $this->tfaSessions; + } + private array $activeBansCache = []; public function tryGetActiveBan(User|string|null $userInfo = null): ?BanInfo { diff --git a/src/Users/UserAuthSession.php b/src/Users/UserAuthSession.php deleted file mode 100644 index fdfab56..0000000 --- a/src/Users/UserAuthSession.php +++ /dev/null @@ -1,86 +0,0 @@ -user_id < 1 ? -1 : $this->user_id; - } - public function getUser(): User { - if($this->user === null) - $this->user = User::byId($this->getUserId()); - return $this->user; - } - - public function getToken(): string { - return $this->tfa_token; - } - - public function getCreationTime(): int { - return $this->tfa_created === null ? -1 : $this->tfa_created; - } - public function getExpiresTime(): int { - return $this->getCreationTime() + self::TOKEN_LIFETIME; - } - public function hasExpired(): bool { - return $this->getExpiresTime() <= time(); - } - - public function delete(): void { - DB::prepare('DELETE FROM `' . DB::PREFIX . self::TABLE . '` WHERE `tfa_token` = :token') - ->bind('token', $this->tfa_token) - ->execute(); - } - - public static function generateToken(): string { - return bin2hex(random_bytes(self::TOKEN_WIDTH)); - } - - public static function create(User $user, bool $return = true): ?self { - $token = self::generateToken(); - $created = DB::prepare('INSERT INTO `' . DB::PREFIX . self::TABLE . '` (`user_id`, `tfa_token`) VALUES (:user, :token)') - ->bind('user', $user->getId()) - ->bind('token', $token) - ->execute(); - - if(!$created) - throw new RuntimeException('Failed to create auth session.'); - if(!$return) - return null; - - $object = self::byToken($token); - $object->user = $user; - return $object; - } - - private static function byQueryBase(): string { - return sprintf(self::QUERY_SELECT, sprintf(self::SELECT, self::TABLE)); - } - public static function byToken(string $token): self { - $object = DB::prepare(self::byQueryBase() . ' WHERE `tfa_token` = :token') - ->bind('token', $token) - ->fetchObject(self::class); - - if(!$object) - throw new RuntimeException('Could not find auth session token.'); - - return $object; - } -}