From 7fc4815b15c23bb7d9e5e6e856a4452f2747607d Mon Sep 17 00:00:00 2001
From: netkas <netkas.nosial@gmail.com>
Date: Wed, 12 Mar 2025 17:53:04 -0400
Subject: [PATCH] Refactor SettingsSetOtp and OneTimePasswordManager to improve
 parameter handling, enhance UUID validation, and ensure proper type casting
 for password verification and OTP creation

https://github.com/nosial/Socialbox-PHP/issues/65
---
 .../Settings/SettingsSetOtp.php               | 10 +-------
 .../Managers/OneTimePasswordManager.php       | 25 +++++++++++--------
 2 files changed, 16 insertions(+), 19 deletions(-)

diff --git a/src/Socialbox/Classes/StandardMethods/Settings/SettingsSetOtp.php b/src/Socialbox/Classes/StandardMethods/Settings/SettingsSetOtp.php
index 5089d5d..3928657 100644
--- a/src/Socialbox/Classes/StandardMethods/Settings/SettingsSetOtp.php
+++ b/src/Socialbox/Classes/StandardMethods/Settings/SettingsSetOtp.php
@@ -4,12 +4,10 @@
 
     use Exception;
     use Socialbox\Abstracts\Method;
-    use Socialbox\Classes\Cryptography;
     use Socialbox\Enums\Flags\SessionFlags;
     use Socialbox\Enums\StandardError;
     use Socialbox\Exceptions\CryptographyException;
     use Socialbox\Exceptions\DatabaseOperationException;
-    use Socialbox\Exceptions\Standard\InvalidRpcArgumentException;
     use Socialbox\Exceptions\Standard\MissingRpcArgumentException;
     use Socialbox\Exceptions\Standard\StandardRpcException;
     use Socialbox\Interfaces\SerializableInterface;
@@ -26,7 +24,6 @@
          */
         public static function execute(ClientRequest $request, RpcRequest $rpcRequest): ?SerializableInterface
         {
-
             try
             {
                 $peer = $request->getPeer();
@@ -65,14 +62,9 @@
                     throw new MissingRpcArgumentException('password');
                 }
 
-                if(!Cryptography::validateSha512($rpcRequest->getParameter('password')))
-                {
-                    throw new InvalidRpcArgumentException('password', 'The provided password is not a valid SHA-512 hash');
-                }
-
                 try
                 {
-                    if(!PasswordManager::verifyPassword($peer, $rpcRequest->getParameter('password')))
+                    if(!PasswordManager::verifyPassword($peer, (string)$rpcRequest->getParameter('password')))
                     {
                         return $rpcRequest->produceError(StandardError::FORBIDDEN, 'The provided password is incorrect');
                     }
diff --git a/src/Socialbox/Managers/OneTimePasswordManager.php b/src/Socialbox/Managers/OneTimePasswordManager.php
index c9fa89c..2f5fb2b 100644
--- a/src/Socialbox/Managers/OneTimePasswordManager.php
+++ b/src/Socialbox/Managers/OneTimePasswordManager.php
@@ -2,12 +2,14 @@
 
     namespace Socialbox\Managers;
 
-    use DateTime;
+    use InvalidArgumentException;
     use PDOException;
+    use Random\RandomException;
     use Socialbox\Classes\Configuration;
     use Socialbox\Classes\Cryptography;
     use Socialbox\Classes\Database;
     use Socialbox\Classes\OtpCryptography;
+    use Socialbox\Classes\Validator;
     use Socialbox\Exceptions\CryptographyException;
     use Socialbox\Exceptions\DatabaseOperationException;
     use Socialbox\Objects\Database\PeerDatabaseRecord;
@@ -27,6 +29,10 @@
             {
                 $peerUuid = $peerUuid->getUuid();
             }
+            elseif(!Validator::validateUuid($peerUuid))
+            {
+                throw new InvalidArgumentException('The given internal peer UUID is not a valid UUID V4');
+            }
 
             try
             {
@@ -45,18 +51,15 @@
         /**
          * Creates and stores a new OTP (One-Time Password) secret for the specified peer, and generates a key URI.
          *
-         * @param string|PeerDatabaseRecord $peer The unique identifier of the peer, either as a string UUID
+         * @param PeerDatabaseRecord $peerDatabaseRecord The unique identifier of the peer, either as a string UUID
          *                                          or an instance of RegisteredPeerRecord.
          * @return string The generated OTP key URI that can be used for applications like authenticator apps.
+         * @throws CryptographyException If there is an error during the encryption of the OTP secret.
          * @throws DatabaseOperationException If there is an error during the database operation.
+         * @throws RandomException If there is an error during the generation of the OTP secret.
          */
-        public static function createOtp(string|PeerDatabaseRecord $peer): string
+        public static function createOtp(PeerDatabaseRecord $peerDatabaseRecord): string
         {
-            if(is_string($peer))
-            {
-                $peer = RegisteredPeerManager::getPeer($peer);
-            }
-
             $secret = OtpCryptography::generateSecretKey(Configuration::getSecurityConfiguration()->getOtpSecretKeyLength());
             $encryptionKey = Configuration::getCryptographyConfiguration()->getRandomInternalEncryptionKey();
             $encryptedSecret = Cryptography::encryptMessage($secret, $encryptionKey, Configuration::getCryptographyConfiguration()->getEncryptionKeysAlgorithm());
@@ -64,7 +67,8 @@
             try
             {
                 $stmt = Database::getConnection()->prepare("INSERT INTO authentication_otp (peer_uuid, secret) VALUES (:peer_uuid, :secret)");
-                $stmt->bindParam(':peer_uuid', $peer);
+                $peerUuid = $peerDatabaseRecord->getUuid();
+                $stmt->bindParam(':peer_uuid', $peerUuid);
                 $stmt->bindParam(':secret', $encryptedSecret);
                 $stmt->execute();
             }
@@ -73,7 +77,7 @@
                 throw new DatabaseOperationException('An error occurred while creating the OTP secret in the database', $e);
             }
 
-            return OtpCryptography::generateKeyUri($peer->getAddress(), $secret,
+            return OtpCryptography::generateKeyUri($peerDatabaseRecord->getAddress(), $secret,
                 Configuration::getInstanceConfiguration()->getDomain(),
                 Configuration::getSecurityConfiguration()->getOtpTimeStep(),
                 Configuration::getSecurityConfiguration()->getOtpDigits(),
@@ -173,6 +177,7 @@
          *
          * @param string|PeerDatabaseRecord $peerUuid The peer's UUID or an instance of RegisteredPeerRecord whose OTP record's last updated timestamp needs to be retrieved
          * @return int The last updated timestamp of the OTP record, or 0 if no such record exists
+         * @throws DatabaseOperationException if the database operation fails.
          */
         public static function getLastUpdated(string|PeerDatabaseRecord $peerUuid): int
         {