Browse Source

Fix issues detected by static code analysis

Bubka 2 years ago
parent
commit
6a41c77144

+ 1 - 1
app/Api/v1/Controllers/TwoFAccountController.php

@@ -106,7 +106,7 @@ class TwoFAccountController extends Controller
      * Convert a migration resource to a valid TwoFAccounts collection
      *
      * @param  \App\Api\v1\Requests\TwoFAccountImportRequest  $request
-     * @return \Illuminate\Http\JsonResponse
+     * @return \Illuminate\Http\JsonResponse|\App\Api\v1\Resources\TwoFAccountCollection
      */
     public function migrate(TwoFAccountImportRequest $request)
     {

+ 19 - 7
app/Factories/MigratorFactory.php

@@ -21,7 +21,7 @@ class MigratorFactory implements MigratorFactoryInterface
      * @param string $migrationPayload The migration payload used to infer the migrator type
      * @return Migrator
      */
-    public function create($migrationPayload) : Migrator
+    public function create(string $migrationPayload) : Migrator
     {
         if ($this->isAegisJSON($migrationPayload)) {
             return App::make(AegisMigrator::class);
@@ -41,9 +41,12 @@ class MigratorFactory implements MigratorFactoryInterface
 
 
     /**
+     * Determine if a payload comes from Google Authenticator
      * 
+     * @param string $migrationPayload The payload to analyse
+     * @return bool
      */
-    private function isGoogleAuth($migrationPayload) : bool
+    private function isGoogleAuth(string $migrationPayload) : bool
     {
         // - Google Auth migration URI : a string starting with otpauth-migration://offline?data= on a single line
 
@@ -57,9 +60,12 @@ class MigratorFactory implements MigratorFactoryInterface
 
 
     /**
+     * Determine if a payload is a plain text content
      * 
+     * @param string $migrationPayload The payload to analyse
+     * @return bool
      */
-    private function isPlainText($migrationPayload) : bool
+    private function isPlainText(string $migrationPayload) : bool
     {
         // - Plain text : one or more otpauth URIs (otpauth://[t|h]otp/...), one per line
 
@@ -73,9 +79,12 @@ class MigratorFactory implements MigratorFactoryInterface
 
 
     /**
+     * Determine if a payload comes from Aegis Authenticator in JSON format
      * 
+     * @param string $migrationPayload The payload to analyse
+     * @return bool
      */
-    private function isAegisJSON($migrationPayload) : mixed
+    private function isAegisJSON(string $migrationPayload) : mixed
     {
         // - Aegis JSON : is a JSON object with the key db.entries full of objects like
         //      {
@@ -108,7 +117,7 @@ class MigratorFactory implements MigratorFactoryInterface
                         'db.entries.*.issuer' => 'required',
                         'db.entries.*.info' => 'required'
                     ]
-                ));
+                )) > 0;
             }
         }
 
@@ -117,9 +126,12 @@ class MigratorFactory implements MigratorFactoryInterface
 
 
     /**
+     * Determine if a payload comes from 2FAS Authenticator
      * 
+     * @param string $migrationPayload The payload to analyse
+     * @return bool
      */
-    private function is2FASv2($migrationPayload) : mixed
+    private function is2FASv2(string $migrationPayload) : mixed
     {
         // - 2FAS JSON : is a JSON object with the key 'schemaVersion' == 2 and a key 'services' full of objects like
         // {
@@ -156,7 +168,7 @@ class MigratorFactory implements MigratorFactoryInterface
                         'services.*.name' => 'required',
                         'services.*.otp' => 'required'
                     ]
-                ));
+                )) > 0;
             }
         }
 

+ 1 - 2
app/Helpers/Helpers.php

@@ -9,8 +9,7 @@ class Helpers
     /**
      * Generate a unique filename
      * 
-     * @param string $ids twofaccount ids to delete
-     * 
+     * @param string $extension
      * @return string The filename
      */
     public static function getUniqueFilename(string $extension): string

+ 2 - 1
app/Services/Migrators/AegisMigrator.php

@@ -49,6 +49,8 @@ class AegisMigrator extends Migrator
             throw new InvalidMigrationDataException('Aegis');
         }
 
+        $twofaccounts = array();
+
         foreach ($json['db']['entries'] as $key => $otp_parameters) {
 
             $parameters = array();
@@ -81,7 +83,6 @@ class AegisMigrator extends Migrator
                         
                         default:
                             throw new \Exception();
-                            break;
                     }
 
                     $filename = Helpers::getUniqueFilename($extension);

+ 4 - 1
app/Services/Migrators/Migrator.php

@@ -12,11 +12,14 @@ abstract class Migrator
      * @param  mixed  $migrationPayload
      * @return \Illuminate\Support\Collection The converted accounts
      */
-    abstract protected function migrate(mixed $migrationPayload) : Collection;
+    abstract public function migrate(mixed $migrationPayload) : Collection;
 
 
     /**
      * Pad a string to 8 chars min
+     * 
+     * @param string $string
+     * @return string The padded string
      */
     protected function padToValidBase32Secret(string $string)
     {

+ 2 - 0
app/Services/Migrators/TwoFASMigrator.php

@@ -80,6 +80,8 @@ class TwoFASMigrator extends Migrator
             Log::error('Aegis JSON migration data cannot be read');
             throw new InvalidMigrationDataException('2FAS Auth');
         }
+        
+        $twofaccounts = array();
 
         foreach ($json['services'] as $key => $otp_parameters) {
 

+ 3 - 5
app/Services/TwoFAccountService.php

@@ -10,7 +10,7 @@ use Illuminate\Support\Collection;
 class TwoFAccountService
 {
     /**
-     * @var $migrator The Migration service
+     * @var MigratorFactoryInterface $migratorFactory The Migration service
      */
     protected $migratorFactory;
 
@@ -50,11 +50,10 @@ class TwoFAccountService
     /**
      * Convert a migration payload to a set of TwoFAccount objects
      * 
-     * @param string $migrationUri migration uri provided by Google Authenticator export feature
-     * 
+     * @param string $migrationPayload Migration payload from 2FA apps export feature
      * @return \Illuminate\Support\Collection The converted accounts
      */
-    public function migrate($migrationPayload) : Collection
+    public function migrate(string $migrationPayload) : Collection
     {
         $migrator = $this->migratorFactory->create($migrationPayload);
         $twofaccounts = $migrator->migrate($migrationPayload);
@@ -67,7 +66,6 @@ class TwoFAccountService
      * Delete one or more twofaccounts
      * 
      * @param int|array|string $ids twofaccount ids to delete
-     * 
      * @return int The number of deleted
      */
     public static function delete($ids) : int