Refactor the icons recording & Update tests

app/Helpers/Helpers.php

@@ -6,17 +6,6 @@ use Illuminate\Support\Str;
 
 class Helpers
 {
-    /**
-     * Generate a unique filename
-     *
-     * @param  string  $extension
-     * @return string The filename
-     */
-    public static function getUniqueFilename(string $extension): string
-    {
-        return Str::random(40) . '.' . $extension;
-    }
-
     /**
      * Clean a version number string
      *

app/Models/TwoFAccount.php

@@ -21,6 +21,7 @@ use Illuminate\Support\Facades\Crypt;
 use Illuminate\Support\Facades\Http;
 use Illuminate\Support\Facades\Log;
 use Illuminate\Support\Facades\Storage;
+use Illuminate\Support\Facades\Validator;
 use Illuminate\Validation\ValidationException;
 use OTPHP\Factory;
 use OTPHP\HOTP;
@@ -29,6 +30,7 @@ use ParagonIE\ConstantTime\Base32;
 use Spatie\EloquentSortable\Sortable;
 use Spatie\EloquentSortable\SortableTrait;
 use SteamTotp\SteamTotp;
+use Illuminate\Support\Str;
 
 class TwoFAccount extends Model implements Sortable
 {
@@ -423,7 +425,7 @@ class TwoFAccount extends Model implements Sortable
             $this->enforceAsSteam();
         }
         if ($this->generator->hasParameter('image')) {
-            $this->icon = $this->storeImageAsIcon($this->generator->getParameter('image'));
+            self::setIcon($this->generator->getParameter('image'));
         }
 
         if (!$this->icon && Settings::get('getOfficialIcons') && !$skipIconFetching) {
@@ -534,16 +536,93 @@ class TwoFAccount extends Model implements Sortable
         }
     }
 
+    /**
+     * Store and set the provided icon
+     * 
+     * @param  \Psr\Http\Message\StreamInterface|\Illuminate\Http\File|\Illuminate\Http\UploadedFile|string|resource  $data
+     * @param  string|null  $extension The resource extension, without the dot
+     */
+    public function setIcon($data, $extension = null): void
+    {
+        $isRemoteData = Str::startsWith($data, ['http://', 'https://']) && Validator::make(
+            [$data],
+            ['url']
+        )->passes();
+
+        if ($isRemoteData) {
+            $icon = $this->storeRemoteImageAsIcon($data);
+        } else {
+            $icon = $extension ? $this->storeFileDataAsIcon($data, $extension) : null;
+        }
+
+        $this->icon = $icon ?: $this->icon;
+    }
+
+    /**
+     * Store img data as an icon file.
+     *
+     * @param  \Psr\Http\Message\StreamInterface|\Illuminate\Http\File|\Illuminate\Http\UploadedFile|string|resource  $content
+     * @param  string  $extension The file extension, without the dot
+     * @return string|null The filename of the stored icon or null if the operation fails
+     */
+    private function storeFileDataAsIcon($content, $extension): string|null
+    {
+        $filename = self::getUniqueFilename($extension);
+
+        if (Storage::disk('icons')->put($filename, $content)) {
+            if (self::isValidIcon($filename, 'icons')) {
+                Log::info(sprintf('Image %s successfully stored for import', $filename));
+
+                return $filename;
+            } else {
+                Storage::disk('icons')->delete($filename);
+            }
+        }
+
+        return null;
+    }
+
+
+    /**
+     * Generate a unique filename
+     *
+     * @param  string  $extension
+     * @return string The filename
+     */
+    private function getUniqueFilename(string $extension): string
+    {
+        return Str::random(40) . '.' . $extension;
+    }
+
+    /**
+     * Validate a file is a valid image
+     *
+     * @param  string  $filename
+     * @param  string  $disk
+     * @return  bool
+     */
+    private function isValidIcon($filename, $disk): bool
+    {
+        return in_array(Storage::disk($disk)->mimeType($filename), [
+            'image/png',
+            'image/jpeg',
+            'image/webp',
+            'image/bmp',
+            'image/x-ms-bmp',
+            'image/svg+xml'
+        ]) && (Storage::disk($disk)->mimeType($filename) !== 'image/svg+xml' ? getimagesize(Storage::disk($disk)->path($filename)) : true);
+    }
+
     /**
      * Gets the image resource pointed by the image url and store it as an icon
      *
      * @return string|null The filename of the stored icon or null if the operation fails
      */
-    private function storeImageAsIcon(string $url)
+    private function storeRemoteImageAsIcon(string $url): string|null
     {
         try {
             $path_parts  = pathinfo($url);
-            $newFilename = Helpers::getUniqueFilename($path_parts['extension']);
+            $newFilename = self::getUniqueFilename($path_parts['extension']);
 
             try {
                 $response = Http::retry(3, 100)->get($url);
@@ -555,10 +634,7 @@ class TwoFAccount extends Model implements Sortable
                 Log::error(sprintf('Cannot fetch imageLink at "%s"', $url));
             }
 
-            if (
+            if (self::isValidIcon($newFilename, 'imagesLink')) {
-                in_array(Storage::disk('imagesLink')->mimeType($newFilename), ['image/png', 'image/jpeg', 'image/webp', 'image/bmp'])
-                && getimagesize(Storage::disk('imagesLink')->path($newFilename))
-            ) {
                 // Should be a valid image, we move it to the icons disk
                 if (Storage::disk('icons')->put($newFilename, Storage::disk('imagesLink')->get($newFilename))) {
                     Storage::disk('imagesLink')->delete($newFilename);
@@ -570,7 +646,7 @@ class TwoFAccount extends Model implements Sortable
                 throw new \Exception('Unsupported mimeType or missing image on storage');
             }
 
-            return $newFilename;
+            return Storage::disk('icons')->exists($newFilename) ? $newFilename : null;
         }
         // @codeCoverageIgnoreStart
         catch (\Exception | \Throwable $ex) {

app/Services/LogoService.php

@@ -98,8 +98,7 @@ class LogoService
         try {
             $response = Http::retry(3, 100)->get(self::TFA_URL);
 
-            $coll = collect(json_decode(htmlspecialchars_decode($response->body()), true))
+            $coll = collect(json_decode(htmlspecialchars_decode($response->body()), true)) /* @phpstan-ignore-line */
-                /** @phpstan-ignore-line */
                 ->mapWithKeys(function ($item, $key) {
                     return [
                         strtolower(head($item)) => $item[1]['domain'],

app/Services/Migrators/AegisMigrator.php

@@ -38,7 +38,7 @@ class AegisMigrator extends Migrator
      * @param  mixed  $migrationPayload
      * @return \Illuminate\Support\Collection<int|string, \App\Models\TwoFAccount> The converted accounts
      */
-    public function migrate(mixed $migrationPayload) : Collection
+    public function migrate(mixed $migrationPayload): Collection
     {
         $json = json_decode(htmlspecialchars_decode($migrationPayload), true);
 
@@ -67,27 +67,21 @@ class AegisMigrator extends Migrator
                 if (Arr::has($otp_parameters, 'icon') && Arr::has($otp_parameters, 'icon_mime')) {
                     switch ($otp_parameters['icon_mime']) {
                         case 'image/svg+xml':
-                            $extension = 'svg';
+                            $parameters['iconExt'] = 'svg';
                             break;
 
                         case 'image/png':
-                            $extension = 'png';
+                            $parameters['iconExt'] = 'png';
                             break;
 
                         case 'image/jpeg':
-                            $extension = 'jpg';
+                            $parameters['iconExt'] = 'jpg';
                             break;
 
                         default:
                             throw new \Exception();
                     }
-
+                    $parameters['iconData'] = base64_decode($otp_parameters['icon']);
-                    $filename = Helpers::getUniqueFilename($extension);
-
-                    if (Storage::disk('icons')->put($filename, base64_decode($otp_parameters['icon']))) {
-                        $parameters['icon'] = $filename;
-                        Log::info(sprintf('Image %s successfully stored for import', $filename));
-                    }
                 }
             } catch (\Exception) {
                 // we do nothing
@@ -96,6 +90,9 @@ class AegisMigrator extends Migrator
             try {
                 $twofaccounts[$key] = new TwoFAccount;
                 $twofaccounts[$key]->fillWithOtpParameters($parameters);
+                if (Arr::has($parameters, 'iconExt') && Arr::has($parameters, 'iconData')) {
+                    $twofaccounts[$key]->setIcon($parameters['iconData'], $parameters['iconExt']);
+                }
             } catch (\Exception $exception) {
                 Log::error(sprintf('Cannot instanciate a TwoFAccount object with OTP parameters from imported item #%s', $key));
                 Log::debug($exception->getMessage());

tests/Api/v1/Requests/TwoFAccountStoreRequestTest.php

@@ -96,6 +96,12 @@ class TwoFAccountStoreRequestTest extends TestCase
                 'otp_type'  => 'totp',
                 'algorithm' => 'md5',
             ]],
+            [[
+                'account'   => 'MyAccount',
+                'otp_type'  => 'totp',
+                'algorithm' => 'md5',
+                'secret'    => 'eee',
+            ]],
         ];
     }
 
@@ -136,13 +142,18 @@ class TwoFAccountStoreRequestTest extends TestCase
             [[
                 'account'  => 'MyAccount',
                 'otp_type' => 'totp',
-                'secret'   => 'notaBase32String',
+                'secret'   => true,
             ]],
             [[
                 'account'  => 'MyAccount',
                 'otp_type' => 'totp',
                 'secret'   => 123456,
             ]],
+            [[
+                'account'  => 'MyAccount',
+                'otp_type' => 'totp',
+                'secret'   => '1.0',
+            ]],
             [[
                 'account'  => 'MyAccount',
                 'otp_type' => 'totp',

tests/Api/v1/Requests/TwoFAccountUpdateRequestTest.php

@@ -72,7 +72,7 @@ class TwoFAccountUpdateRequestTest extends TestCase
                 'account'   => 'MyAccount',
                 'icon'      => null,
                 'otp_type'  => 'hotp',
-                'secret'    => 'A4GRFHZVRBGY7UIW',
+                'secret'    => 'eeee',
                 'digits'    => 10,
                 'algorithm' => 'sha1',
                 'period'    => null,
@@ -136,7 +136,7 @@ class TwoFAccountUpdateRequestTest extends TestCase
                 'account'   => 'MyAccount',
                 'icon'      => null,
                 'otp_type'  => 'hotp',
-                'secret'    => 'notaBase32String',
+                'secret'    => 1000,
                 'digits'    => 6,
                 'algorithm' => 'sha1',
                 'period'    => null,

tests/Data/OtpTestData.php

@@ -34,7 +34,25 @@ class OtpTestData
 
     const IMAGE = 'https%3A%2F%2Fen.opensuse.org%2Fimages%2F4%2F44%2FButton-filled-colour.png';
 
-    const ICON = 'test.png';
+    const ICON_PNG = 'test.png';
+
+    const ICON_PNG_DATA = 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAACXBIWXMAAA7EAAAOxAGVKw4bAAAAC0lEQVQImWP4DwQACfsD/eNV8pwAAAAASUVORK5CYII=';
+
+    const ICON_JPEG = 'test.jpg';
+
+    const ICON_JPEG_DATA = '/9j/4AAQSkZJRgABAQEAYABgAAD/2wBDAFA3PEY8MlBGQUZaVVBfeMiCeG5uePWvuZHI////////////////////////////////////////////////////2wBDAVVaWnhpeOuCguv/////////////////////////////////////////////////////////////////////////wAARCAABAAEDASIAAhEBAxEB/8QAHwAAAQUBAQEBAQEAAAAAAAAAAAECAwQFBgcICQoL/8QAtRAAAgEDAwIEAwUFBAQAAAF9AQIDAAQRBRIhMUEGE1FhByJxFDKBkaEII0KxwRVS0fAkM2JyggkKFhcYGRolJicoKSo0NTY3ODk6Q0RFRkdISUpTVFVWV1hZWmNkZWZnaGlqc3R1dnd4eXqDhIWGh4iJipKTlJWWl5iZmqKjpKWmp6ipqrKztLW2t7i5usLDxMXGx8jJytLT1NXW19jZ2uHi4+Tl5ufo6erx8vP09fb3+Pn6/8QAHwEAAwEBAQEBAQEBAQAAAAAAAAECAwQFBgcICQoL/8QAtREAAgECBAQDBAcFBAQAAQJ3AAECAxEEBSExBhJBUQdhcRMiMoEIFEKRobHBCSMzUvAVYnLRChYkNOEl8RcYGRomJygpKjU2Nzg5OkNERUZHSElKU1RVVldYWVpjZGVmZ2hpanN0dXZ3eHl6goOEhYaHiImKkpOUlZaXmJmaoqOkpaanqKmqsrO0tba3uLm6wsPExcbHyMnK0tPU1dbX2Nna4uPk5ebn6Onq8vP09fb3+Pn6/9oADAMBAAIRAxEAPwC7RRRQB//Z';
+
+    const ICON_WEBP = 'test.webp';
+
+    const ICON_WEBP_DATA = 'UklGRiQAAABXRUJQVlA4IBgAAAAwAQCdASoBAAEAB0CWJaQAA3AA/u9gAAA=';
+
+    const ICON_BMP = 'test.bmp';
+
+    const ICON_BMP_DATA = 'Qk2OAAAAAAAAAIoAAAB8AAAAAQAAAAEAAAABACAAAwAAACAAAAATCwAAEwsAAAAAAAAAAAAAAAD/AAD/AAD/AAAAAAAA/0JHUnMAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAMJXy/w==';
+
+    const ICON_SVG = 'test.svg';
+
+    const ICON_SVG_DATA = '<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 1024 1024"><circle cx="512" cy="512" r="512" style="fill:#000e9c"/><path d="m700.2 466.5 61.2-106.3c23.6 41.6 37.2 89.8 37.2 141.1 0 68.8-24.3 131.9-64.7 181.4H575.8l48.7-84.6h-64.4l75.8-131.7 64.3.1zm-55.4-125.2L448.3 682.5l.1.2H290.1c-40.5-49.5-64.7-112.6-64.7-181.4 0-51.4 13.6-99.6 37.3-141.3l102.5 178.2 113.3-197h166.3z" style="fill:#fff"/></svg>';
 
     const TOTP_FULL_CUSTOM_URI_NO_IMG = 'otpauth://totp/' . self::SERVICE . ':' . self::ACCOUNT . '?secret=' . self::SECRET . '&issuer=' . self::SERVICE . '&digits=' . self::DIGITS_CUSTOM . '&period=' . self::PERIOD_CUSTOM . '&algorithm=' . self::ALGORITHM_CUSTOM;
 
@@ -57,7 +75,7 @@ class OtpTestData
     const ARRAY_OF_FULL_VALID_PARAMETERS_FOR_CUSTOM_TOTP = [
         'service'   => self::SERVICE,
         'account'   => self::ACCOUNT,
-        'icon'      => self::ICON,
+        'icon'      => self::ICON_PNG,
         'otp_type'  => 'totp',
         'secret'    => self::SECRET,
         'digits'    => self::DIGITS_CUSTOM,
@@ -87,7 +105,7 @@ class OtpTestData
     const ARRAY_OF_FULL_VALID_PARAMETERS_FOR_CUSTOM_HOTP = [
         'service'   => self::SERVICE,
         'account'   => self::ACCOUNT,
-        'icon'      => self::ICON,
+        'icon'      => self::ICON_PNG,
         'otp_type'  => 'hotp',
         'secret'    => self::SECRET,
         'digits'    => self::DIGITS_CUSTOM,
@@ -112,5 +130,4 @@ class OtpTestData
         'period'    => self::PERIOD_DEFAULT,
         'counter'   => null,
     ];
-
 }

tests/Feature/Models/TwoFAccountModelTest.php

@@ -7,9 +7,8 @@ use Tests\Data\OtpTestData;
 use Tests\FeatureTestCase;
 use Illuminate\Support\Facades\Storage;
 use Illuminate\Http\Testing\FileFactory;
+use Illuminate\Http\UploadedFile;
 use Illuminate\Support\Facades\Http;
-use App\Helpers\Helpers;
-use Mockery\MockInterface;
 use Tests\Data\HttpRequestTestData;
 
 /**
@@ -28,9 +27,10 @@ class TwoFAccountModelTest extends FeatureTestCase
     protected $customHotpTwofaccount;
 
     /**
-     * 
+     * Helpers $helpers;
+
      */
-    const ICON_NAME = 'oDBngpjQaQAgLtHqGuYiPRqftCXv6Sj4hSAXARpA.png';
+    protected $helpers;
 
     /**
      * @test
@@ -43,7 +43,7 @@ class TwoFAccountModelTest extends FeatureTestCase
         $this->customTotpTwofaccount->legacy_uri = OtpTestData::TOTP_FULL_CUSTOM_URI;
         $this->customTotpTwofaccount->service    = OtpTestData::SERVICE;
         $this->customTotpTwofaccount->account    = OtpTestData::ACCOUNT;
-        $this->customTotpTwofaccount->icon       = OtpTestData::ICON;
+        $this->customTotpTwofaccount->icon       = OtpTestData::ICON_PNG;
         $this->customTotpTwofaccount->otp_type   = 'totp';
         $this->customTotpTwofaccount->secret     = OtpTestData::SECRET;
         $this->customTotpTwofaccount->digits     = OtpTestData::DIGITS_CUSTOM;
@@ -56,7 +56,7 @@ class TwoFAccountModelTest extends FeatureTestCase
         $this->customHotpTwofaccount->legacy_uri = OtpTestData::HOTP_FULL_CUSTOM_URI;
         $this->customHotpTwofaccount->service    = OtpTestData::SERVICE;
         $this->customHotpTwofaccount->account    = OtpTestData::ACCOUNT;
-        $this->customHotpTwofaccount->icon       = OtpTestData::ICON;
+        $this->customHotpTwofaccount->icon       = OtpTestData::ICON_PNG;
         $this->customHotpTwofaccount->otp_type   = 'hotp';
         $this->customHotpTwofaccount->secret     = OtpTestData::SECRET;
         $this->customHotpTwofaccount->digits     = OtpTestData::DIGITS_CUSTOM;
@@ -80,21 +80,10 @@ class TwoFAccountModelTest extends FeatureTestCase
 
     /**
      * @test
-     * 
-     * @runInSeparateProcess
-     * @preserveGlobalState disabled
      */
     public function test_fill_with_custom_totp_uri_returns_correct_value()
     {
-        $this->mock('alias:' . Helpers::class, function (MockInterface $helper) {
+        $file = (new FileFactory)->image('file.png', 10, 10);
-            $helper->shouldReceive('getUniqueFilename')
-                ->andReturn(self::ICON_NAME);
-
-            $helper->shouldReceive('isValidImage')
-                ->andReturn(true);
-        });
-
-        $file = (new FileFactory)->image(self::ICON_NAME, 10, 10);
 
         Http::preventStrayRequests();
         Http::fake([
@@ -107,9 +96,6 @@ class TwoFAccountModelTest extends FeatureTestCase
         $twofaccount = new TwoFAccount;
         $twofaccount->fillWithURI(OtpTestData::TOTP_FULL_CUSTOM_URI);
 
-        Storage::disk('icons')->assertExists(self::ICON_NAME);
-        Storage::disk('imagesLink')->assertMissing(self::ICON_NAME);
-
         $this->assertEquals('totp', $twofaccount->otp_type);
         $this->assertEquals(OtpTestData::TOTP_FULL_CUSTOM_URI, $twofaccount->legacy_uri);
         $this->assertEquals(OtpTestData::SERVICE, $twofaccount->service);
@@ -119,7 +105,10 @@ class TwoFAccountModelTest extends FeatureTestCase
         $this->assertEquals(OtpTestData::PERIOD_CUSTOM, $twofaccount->period);
         $this->assertEquals(null, $twofaccount->counter);
         $this->assertEquals(OtpTestData::ALGORITHM_CUSTOM, $twofaccount->algorithm);
-        $this->assertEquals(self::ICON_NAME, $twofaccount->icon);
+        $this->assertNotNull($twofaccount->icon);
+
+        Storage::disk('icons')->assertExists($twofaccount->icon);
+        Storage::disk('imagesLink')->assertMissing($twofaccount->icon);
     }
 
     /**
@@ -144,21 +133,10 @@ class TwoFAccountModelTest extends FeatureTestCase
 
     /**
      * @test
-     * 
-     * @runInSeparateProcess
-     * @preserveGlobalState disabled
      */
     public function test_fill_with_custom_hotp_uri_returns_correct_value()
     {
-        $this->mock('alias:' . Helpers::class, function (MockInterface $helper) {
+        $file = (new FileFactory)->image('file.png', 10, 10);
-            $helper->shouldReceive('getUniqueFilename')
-                ->andReturn(self::ICON_NAME);
-
-            $helper->shouldReceive('isValidImage')
-                ->andReturn(true);
-        });
-
-        $file = (new FileFactory)->image(self::ICON_NAME, 10, 10);
 
         Http::preventStrayRequests();
         Http::fake([
@@ -171,9 +149,6 @@ class TwoFAccountModelTest extends FeatureTestCase
         $twofaccount = new TwoFAccount;
         $twofaccount->fillWithURI(OtpTestData::HOTP_FULL_CUSTOM_URI);
 
-        Storage::disk('icons')->assertExists(self::ICON_NAME);
-        Storage::disk('imagesLink')->assertMissing(self::ICON_NAME);
-
         $this->assertEquals('hotp', $twofaccount->otp_type);
         $this->assertEquals(OtpTestData::HOTP_FULL_CUSTOM_URI, $twofaccount->legacy_uri);
         $this->assertEquals(OtpTestData::SERVICE, $twofaccount->service);
@@ -183,7 +158,10 @@ class TwoFAccountModelTest extends FeatureTestCase
         $this->assertEquals(null, $twofaccount->period);
         $this->assertEquals(OtpTestData::COUNTER_CUSTOM, $twofaccount->counter);
         $this->assertEquals(OtpTestData::ALGORITHM_CUSTOM, $twofaccount->algorithm);
-        $this->assertEquals(self::ICON_NAME, $twofaccount->icon);
+        $this->assertNotNull($twofaccount->icon);
+
+        Storage::disk('icons')->assertExists($twofaccount->icon);
+        Storage::disk('imagesLink')->assertMissing($twofaccount->icon);
     }
 
     /**
@@ -450,20 +428,9 @@ class TwoFAccountModelTest extends FeatureTestCase
 
     /**
      * @test
-     * 
-     * @runInSeparateProcess
-     * @preserveGlobalState disabled
      */
     public function test_getOTP_for_totp_returns_the_same_password()
     {
-        $this->mock('alias:' . Helpers::class, function (MockInterface $helper) {
-            $helper->shouldReceive('getUniqueFilename')
-                ->andReturn(self::ICON_NAME);
-
-            $helper->shouldReceive('isValidImage')
-                ->andReturn(true);
-        });
-
         Http::preventStrayRequests();
         Http::fake([
             'https://en.opensuse.org/images/4/44/Button-filled-colour.png' => Http::response(HttpRequestTestData::ICON_PNG, 200),
@@ -491,19 +458,9 @@ class TwoFAccountModelTest extends FeatureTestCase
 
     /**
      * @test
-     * 
-     * @runInSeparateProcess
-     * @preserveGlobalState disabled
      */
     public function test_getOTP_for_hotp_returns_the_same_password()
     {
-        $this->mock('alias:' . Helpers::class, function (MockInterface $helper) {
-            $helper->shouldReceive('getUniqueFilename')
-                ->andReturn(self::ICON_NAME);
-
-            $helper->shouldReceive('isValidImage')
-                ->andReturn(true);
-        });
 
         Http::preventStrayRequests();
         Http::fake([
@@ -555,7 +512,7 @@ class TwoFAccountModelTest extends FeatureTestCase
         $twofaccount = new TwoFAccount;
 
         $this->expectException(\App\Exceptions\InvalidSecretException::class);
-        $otp_from_uri = $twofaccount->fillWithURI('otpauth://totp/' . OtpTestData::ACCOUNT . '?secret=0')->getOTP();
+        $otp_from_uri = $twofaccount->fillWithURI('otpauth://totp/' . OtpTestData::ACCOUNT . '?secret=1.0')->getOTP();
     }
 
     /**
@@ -607,16 +564,9 @@ class TwoFAccountModelTest extends FeatureTestCase
 
     /**
      * @test
-     * 
-     * @runInSeparateProcess
-     * @preserveGlobalState disabled
      */
     public function test_fill_succeed_when_image_fetching_fails()
     {
-        $this->mock('alias:' . Helpers::class, function (MockInterface $helper) {
-            $helper->shouldReceive('getUniqueFilename')
-                ->andReturn(self::ICON_NAME);
-        });
 
         Http::preventStrayRequests();
 
@@ -626,8 +576,8 @@ class TwoFAccountModelTest extends FeatureTestCase
         $twofaccount = new TwoFAccount;
         $twofaccount->fillWithURI(OtpTestData::TOTP_FULL_CUSTOM_URI);
 
-        Storage::disk('icons')->assertMissing(self::ICON_NAME);
+        Storage::disk('icons')->assertDirectoryEmpty('/');
-        Storage::disk('imagesLink')->assertMissing(self::ICON_NAME);
+        Storage::disk('imagesLink')->assertDirectoryEmpty('/');
     }
 
     /**
@@ -675,7 +625,7 @@ class TwoFAccountModelTest extends FeatureTestCase
         $twofaccount->legacy_uri = OtpTestData::TOTP_FULL_CUSTOM_URI;
         $twofaccount->service    = OtpTestData::SERVICE;
         $twofaccount->account    = OtpTestData::ACCOUNT;
-        $twofaccount->icon       = OtpTestData::ICON;
+        $twofaccount->icon       = OtpTestData::ICON_PNG;
         $twofaccount->otp_type   = 'totp';
         $twofaccount->secret     = OtpTestData::SECRET;
         $twofaccount->digits     = OtpTestData::DIGITS_CUSTOM;
@@ -696,7 +646,7 @@ class TwoFAccountModelTest extends FeatureTestCase
         $twofaccount->legacy_uri = OtpTestData::TOTP_FULL_CUSTOM_URI;
         $twofaccount->service    = OtpTestData::SERVICE;
         $twofaccount->account    = OtpTestData::ACCOUNT;
-        $twofaccount->icon       = OtpTestData::ICON;
+        $twofaccount->icon       = OtpTestData::ICON_PNG;
         $twofaccount->otp_type   = 'totp';
         $twofaccount->secret     = OtpTestData::SECRET;
         $twofaccount->digits     = OtpTestData::DIGITS_CUSTOM;
@@ -707,4 +657,84 @@ class TwoFAccountModelTest extends FeatureTestCase
 
         $this->assertFalse($twofaccount->equals($this->customHotpTwofaccount));
     }
+
+    /**
+     * @test
+     * 
+     * @dataProvider iconResourceProvider
+     */
+    public function test_set_icon_stores_and_set_the_icon($res, $ext)
+    {
+        Storage::fake('imagesLink');
+        Storage::fake('icons');
+
+        $previousIcon = $this->customTotpTwofaccount->icon;
+        $this->customTotpTwofaccount->setIcon($res, $ext);
+
+        $this->assertNotEquals($previousIcon, $this->customTotpTwofaccount->icon);
+
+        Storage::disk('icons')->assertExists($this->customTotpTwofaccount->icon);
+        Storage::disk('imagesLink')->assertMissing($this->customTotpTwofaccount->icon);
+    }
+
+    /**
+     * Provide data for Icon store tests
+     */
+    public function iconResourceProvider()
+    {
+        return [
+            'PNG' => [
+                base64_decode(OtpTestData::ICON_PNG_DATA),
+                'png',
+            ],
+            'JPG' => [
+                base64_decode(OtpTestData::ICON_JPEG_DATA),
+                'jpg',
+            ],
+            'WEBP' => [
+                base64_decode(OtpTestData::ICON_WEBP_DATA),
+                'webp',
+            ],
+            'BMP' => [
+                base64_decode(OtpTestData::ICON_BMP_DATA),
+                'bmp',
+            ],
+            'SVG' => [
+                OtpTestData::ICON_SVG_DATA,
+                'svg',
+            ],
+        ];
+    }
+
+    /**
+     * @test
+     * 
+     * @dataProvider invalidIconResourceProvider
+     */
+    public function test_set_invalid_icon_ends_without_error($res, $ext)
+    {
+        Storage::fake('imagesLink');
+        Storage::fake('icons');
+
+        $previousIcon = $this->customTotpTwofaccount->icon;
+        $this->customTotpTwofaccount->setIcon($res, $ext);
+
+        $this->assertEquals($previousIcon, $this->customTotpTwofaccount->icon);
+
+        Storage::disk('icons')->assertMissing($this->customTotpTwofaccount->icon);
+        Storage::disk('imagesLink')->assertMissing($this->customTotpTwofaccount->icon);
+    }
+
+    /**
+     * Provide data for Icon store tests
+     */
+    public function invalidIconResourceProvider()
+    {
+        return [
+            'INVALID_PNG' => [
+                'lkjdslfkjslkdfjlskdjflksjf',
+                'png',
+            ],
+        ];
+    }
 }

tests/Feature/Services/TwoFAccountServiceTest.php

@@ -41,7 +41,7 @@ class TwoFAccountServiceTest extends FeatureTestCase
         $this->customTotpTwofaccount->legacy_uri = OtpTestData::TOTP_FULL_CUSTOM_URI;
         $this->customTotpTwofaccount->service    = OtpTestData::SERVICE;
         $this->customTotpTwofaccount->account    = OtpTestData::ACCOUNT;
-        $this->customTotpTwofaccount->icon       = OtpTestData::ICON;
+        $this->customTotpTwofaccount->icon       = OtpTestData::ICON_PNG;
         $this->customTotpTwofaccount->otp_type   = 'totp';
         $this->customTotpTwofaccount->secret     = OtpTestData::SECRET;
         $this->customTotpTwofaccount->digits     = OtpTestData::DIGITS_CUSTOM;
@@ -54,7 +54,7 @@ class TwoFAccountServiceTest extends FeatureTestCase
         $this->customHotpTwofaccount->legacy_uri = OtpTestData::HOTP_FULL_CUSTOM_URI;
         $this->customHotpTwofaccount->service    = OtpTestData::SERVICE;
         $this->customHotpTwofaccount->account    = OtpTestData::ACCOUNT;
-        $this->customHotpTwofaccount->icon       = OtpTestData::ICON;
+        $this->customHotpTwofaccount->icon       = OtpTestData::ICON_PNG;
         $this->customHotpTwofaccount->otp_type   = 'hotp';
         $this->customHotpTwofaccount->secret     = OtpTestData::SECRET;
         $this->customHotpTwofaccount->digits     = OtpTestData::DIGITS_CUSTOM;
@@ -212,7 +212,7 @@ class TwoFAccountServiceTest extends FeatureTestCase
         $parameters = [
             'service'   => OtpTestData::SERVICE,
             'account'   => OtpTestData::ACCOUNT,
-            'icon'      => OtpTestData::ICON,
+            'icon'      => OtpTestData::ICON_PNG,
             'otp_type'  => 'totp',
             'secret'    => OtpTestData::SECRET,
             'digits'    => OtpTestData::DIGITS_DEFAULT,

tests/Unit/HelpersTest.php

@@ -10,19 +10,6 @@ use Tests\TestCase;
  */
 class HelpersTest extends TestCase
 {
-    /**
-     * @test
-     */
-    public function test_getUniqueFilename_returns_filename()
-    {
-        $ext = 'jpg';
-        $filename = Helpers::getUniqueFilename($ext);
-
-        $this->assertIsString($filename);
-        $this->assertStringEndsWith('.' . $ext, $filename);
-        $this->assertEquals(41 + strlen($ext), strlen($filename));
-    }
-
     /**
      * @test
      * 
@@ -95,4 +82,57 @@ class HelpersTest extends TestCase
             ],
         ];
     }
+
+    /**
+     * @test
+     * 
+     * @dataProvider  toBase32PaddedStringProvider
+     */
+    public function test_toBase32Format_returns_base32_formated_string($str, $expected)
+    {
+        $base32str = Helpers::PadToBase32Format($str);
+
+        $this->assertEquals($expected, $base32str);
+    }
+
+    /**
+     * Provide data for cleanVersionNumber() tests
+     */
+    public function toBase32PaddedStringProvider()
+    {
+        return [
+            'SHORT_STRING' => [
+                'eeee',
+                'EEEE====',
+            ],
+            'LONG_STRING' => [
+                'eeeezzzztt',
+                'EEEEZZZZTT======',
+            ],
+            'EXACT_LENGTH_STRING' => [
+                'eeeezzzz',
+                'EEEEZZZZ',
+            ],
+            'EXACT_LONG_LENGTH_STRING' => [
+                'eeeezzzzeeeezzzzeeeezzzz',
+                'EEEEZZZZEEEEZZZZEEEEZZZZ',
+            ],
+            'NO_STRING' => [
+                '',
+                '',
+            ],
+            'BOOL_STRING' => [
+                true,
+                '1=======',
+            ],
+            'INT_STRING' => [
+                10,
+                '10======',
+            ],
+            'FLOAT_STRING' => [
+                0.1,
+                '0.1=====',
+            ],
+        ];
+    }
 }

tests/Unit/TwoFAccountModelTest.php

@@ -3,6 +3,7 @@
 namespace Tests\Unit;
 
 use App\Events\TwoFAccountDeleted;
+use App\Helpers\Helpers;
 use App\Models\TwoFAccount;
 use App\Services\SettingService;
 use Illuminate\Support\Facades\Crypt;
@@ -49,16 +50,16 @@ class TwoFAccountModelTest extends ModelTestCase
         });
 
         $twofaccount = TwoFAccount::factory()->make([
-            $attribute => 'string',
+            $attribute => 'STRING==',
         ]);
 
-        $this->assertEquals('string', Crypt::decryptString($twofaccount->getAttributes()[$attribute]));
+        $this->assertEquals('STRING==', Crypt::decryptString($twofaccount->getAttributes()[$attribute]));
     }
 
     /**
      * Provide attributes to test for encryption
      */
-    public function provideSensitiveAttributes() : array
+    public function provideSensitiveAttributes(): array
     {
         return [
             [
@@ -111,4 +112,30 @@ class TwoFAccountModelTest extends ModelTestCase
 
         $this->assertEquals(__('errors.indecipherable'), $twofaccount->$attribute);
     }
+
+    /**
+     * @test
+     * 
+     * @runInSeparateProcess
+     * @preserveGlobalState disabled
+     */
+    public function test_secret_is_uppercased_and_padded_at_setup()
+    {
+        $settingService = $this->mock(SettingService::class, function (MockInterface $settingService) {
+            $settingService->shouldReceive('get')
+                ->with('useEncryption')
+                ->andReturn(false);
+        });
+
+        $helpers = $this->mock('alias:' . Helpers::class, function (MockInterface $helpers) {
+            $helpers->shouldReceive('PadToBase32Format')
+                ->andReturn('YYYY====');
+        });
+
+        $twofaccount = TwoFAccount::factory()->make([
+            'secret' => 'yyyy',
+        ]);
+
+        $this->assertEquals('YYYY====', $twofaccount->secret);
+    }
 }