Replace DbProtection class by an Encryption service

app/Classes/DbProtection.php

@@ -1,113 +0,0 @@
-<?php
-
-namespace App\Classes;
-
-use Throwable;
-use Exception;
-use App\TwoFAccount;
-use Illuminate\Support\Facades\DB;
-use Illuminate\Support\Facades\Crypt;
-
-class DbProtection
-{
-    /**
-     * Encrypt 2FA sensitive data
-     * @return boolean
-     */
-    public static function enable() : bool
-    {
-        // All existing records have to be encrypted without exception.
-        // This means that if any of the encryption failed we have to rollback
-        // all records to their original value.
-        
-        $EncryptFailed = false;
-        $twofaccounts = DB::table('twofaccounts')->get();
-
-        $twofaccounts->each(function ($item, $key) use(&$EncryptFailed) {
-            try {
-                $item->uri = Crypt::encryptString($item->uri);
-                $item->account = Crypt::encryptString($item->account);
-            }
-            catch (Exception $e) {
-                $EncryptFailed = true;
-                return false;
-            }
-        });
-                
-        if( $EncryptFailed ) {
-            return false;
-        }
-
-        return self::tryUpdate($twofaccounts);
-    }
-
-
-    /**
-     * Decrypt 2FA sensitive data
-     * @return boolean
-     */
-    public static function disable() : bool
-    {
-        // All existing records have to be decrypted without exception.
-        // This means that if any of the encryption failed we have to rollback
-        // all records to their original value.
-        
-        $DecryptFailed = false;
-        $EncryptedTwofaccounts = DB::table('twofaccounts')->get();
-
-        $EncryptedTwofaccounts->each(function ($item, $key) use(&$DecryptFailed) {
-            try {
-                $item->uri = Crypt::decryptString($item->uri);
-                $item->account = Crypt::decryptString($item->account);
-            }
-            catch (Exception $e) {
-                $DecryptFailed = true;
-                return false;
-            }
-        });
-                
-        if( $DecryptFailed ) {
-            return false;
-        }
-
-        return DbProtection::tryUpdate($EncryptedTwofaccounts);
-    }
-
-
-    /**
-     * Try to update all records of the collection
-     * @param  Illuminate\Database\Eloquent\Collection $twofaccounts
-     * @return boolean                  
-     */
-    private static function tryUpdate(\Illuminate\Support\Collection $twofaccounts) : bool
-    {
-        // The whole collection has its sensible data encrypted/decrypted, now we update the db
-        // using a transaction to ensure rollback if an exception is thrown
-
-        DB::beginTransaction();
-
-        try {
-            $twofaccounts->each(function ($item, $key) {
-                DB::table('twofaccounts')
-                    ->where('id', $item->id)
-                    ->update([
-                        'uri' => $item->uri,
-                        'account' => $item->account
-                    ]);
-            });
-
-            DB::commit();
-        }
-        // @codeCoverageIgnoreStart
-        // Dont now how to fake that :(
-        catch (Throwable $e) {
-            DB::rollBack();
-
-            return false;
-        }
-        // @codeCoverageIgnoreEnd
-
-        return true;
-    }
-
-}

app/Exceptions/DbEncryptionException.php

@@ -0,0 +1,14 @@
+<?php
+
+namespace App\Exceptions;
+
+use Exception;
+
+/**
+ * Class DbEncryptionException.
+ *
+ * @codeCoverageIgnore
+ */
+class DbEncryptionException extends Exception
+{
+}

app/Exceptions/UndecipherableException.php

@@ -0,0 +1,14 @@
+<?php
+
+namespace App\Exceptions;
+
+use Exception;
+
+/**
+ * Class UndecipherableException.
+ *
+ * @codeCoverageIgnore
+ */
+class UndecipherableException extends Exception
+{
+}

app/Http/Controllers/SettingController.php

@@ -2,11 +2,12 @@
 
 namespace App\Http\Controllers;
 
-use App\Classes\DbProtection;
+use App\Exceptions\DbEncryptionException;
+use App\Services\DbEncryptionService;
+use App\Services\SettingServiceInterface;
 use App\Http\Requests\SettingStoreRequest;
 use App\Http\Requests\SettingUpdateRequest;
 use App\Http\Controllers\Controller;
-use App\Services\SettingServiceInterface;
 
 
 class SettingController extends Controller
@@ -17,14 +18,20 @@ class SettingController extends Controller
      */
     protected SettingServiceInterface $settingService;
 
+    /**
+     * The Settings Service instance.
+     */
+    protected DbEncryptionService $dbEncryptionService;
+
 
     /**
      * Create a new controller instance.
      * 
      */
-    public function __construct(SettingServiceInterface $SettingServiceInterface)
+    public function __construct(SettingServiceInterface $SettingServiceInterface, DbEncryptionService $dbEncryptionService)
     {
         $this->settingService = $SettingServiceInterface;
+        $this->dbEncryptionService = $dbEncryptionService;
     }
 
 
@@ -96,35 +103,26 @@ class SettingController extends Controller
     {
         $validated = $request->validated();
 
-        $this->settingService->set($settingName, $validated['value']);
+        // The useEncryption setting impacts records in DB so we delegate the work to the
+        // dedicated db encryption service
+        if( $settingName === 'useEncryption')
+        {
+            try {
+                $this->dbEncryptionService->setTo($validated['value']);
+            }
+            catch(DbEncryptionException $ex) {
+                return response()->json([
+                    'message' => $ex->getMessage()
+                ], 400);
+            }
+        }
+        else $this->settingService->set($settingName, $validated['value']);
 
         return response()->json([
             'key' => $settingName,
             'value' => $validated['value']
         ], 200);
 
-        // The useEncryption option impacts the [existing] content of the database.
-        // Encryption/Decryption of the data is done only if the user change the value of the option
-        // to prevent successive encryption
-
-        if( $request->has('useEncryption'))
-        {
-            if( $request->useEncryption && !$this->settingService->get('useEncryption') ) {
-
-                // user enabled the encryption
-                if( !DbProtection::enable() ) {
-                    return response()->json(['message' => __('errors.error_during_encryption')], 400);
-                }
-            }
-            else if( !$request->useEncryption && $this->settingService->get('useEncryption') ) {
-
-                // user disabled the encryption
-                if( !DbProtection::disable() ) {
-                    return response()->json(['message' => __('errors.error_during_decryption')], 400);
-                }
-            }
-        }
-
     }
 
 

app/Http/Controllers/TwoFAccountController.php

@@ -3,6 +3,7 @@
 namespace App\Http\Controllers;
 
 use App\TwoFAccount;
+use App\Exceptions\UndecipherableException;
 use App\Http\Requests\TwoFAccountReorderRequest;
 use App\Http\Requests\TwoFAccountStoreRequest;
 use App\Http\Requests\TwoFAccountUpdateRequest;
@@ -160,7 +161,14 @@ class TwoFAccountController extends Controller
 
         // The request input is the ID of an existing account
         if ( $id ) {
-            $otp = $this->twofaccountService->getOTP((int) $id);
+            try {
+                $otp = $this->twofaccountService->getOTP((int) $id);
+            }
+            catch (UndecipherableException $ex) {
+                return response()->json([
+                    'message' => __('errors.cannot_decipher_secret')
+                ], 400);
+            }
         }
 
         // The request input is an uri

app/Services/DbEncryptionService.php

@@ -0,0 +1,108 @@
+<?php
+
+namespace App\Services;
+
+use Throwable;
+use Exception;
+use App\TwoFAccount;
+use App\Exceptions\DbEncryptionException;
+use App\Services\SettingServiceInterface;
+use Illuminate\Support\Facades\DB;
+use Illuminate\Support\Facades\Crypt;
+use Illuminate\Support\Collection;
+
+class DbEncryptionService
+{
+
+    /**
+     * The Settings Service instance.
+     */
+    protected SettingServiceInterface $settingService;
+
+
+    /**
+     * Settings service constructor
+     * 
+     */
+    public function __construct(SettingServiceInterface $SettingServiceInterface)
+    {
+        $this->settingService = $SettingServiceInterface;
+    }
+
+
+    /**
+     * Enable or Disable encryption of 2FAccounts sensible data
+     * 
+     * @return void
+     * @throws DbEncryptionException Something failed, everything have been rolledback
+     */
+    public function setTo(bool $state) : void
+    {
+        // We don't want the records to be encrypted/decrypted multiple successive times
+        $isInUse = $this->settingService->get('useEncryption');
+
+        if ($isInUse === !$state) {
+            if ($this->updateRecords($state)) {
+                $this->settingService->set('useEncryption', $state);
+            }
+            else {
+                throw new DbEncryptionException($state === true ? __('errors.error_during_encryption') : __('errors.error_during_decryption'));
+            }
+        }
+    }
+
+
+    /**
+     * Encrypt/Decrypt accounts in database
+     * 
+     * @param boolean $encrypted Whether the record should be encrypted or not
+     * @return boolean Whether the operation completed successfully
+     */
+    private function updateRecords(bool $encrypted) : bool
+    {        
+        $success = true;
+        $twofaccounts = DB::table('twofaccounts')->get();
+
+        $twofaccounts->each(function ($item, $key) use(&$success, $encrypted) {
+            try {
+                $item->legacy_uri   = $encrypted ? Crypt::encryptString($item->legacy_uri)  : Crypt::decryptString($item->legacy_uri);
+                $item->account      = $encrypted ? Crypt::encryptString($item->account)     : Crypt::decryptString($item->account);
+                $item->secret       = $encrypted ? Crypt::encryptString($item->secret)      : Crypt::decryptString($item->secret);
+            }
+            catch (Exception $e) {
+                $success = false;
+                // Exit the each iteration
+                return false;
+            }
+        });
+
+        if ($success) {
+            // The whole collection has now its sensible data encrypted/decrypted
+            // We update the db using a transaction that can rollback everything if an error occured
+            DB::beginTransaction();
+
+            try {
+                $twofaccounts->each(function ($item, $key) {
+                    DB::table('twofaccounts')
+                        ->where('id', $item->id)
+                        ->update([
+                            'legacy_uri' => $item->legacy_uri,
+                            'account'    => $item->account,
+                            'secret'     => $item->secret
+                        ]);
+                });
+
+                DB::commit();
+                return true;
+            }
+            // @codeCoverageIgnoreStart
+            // Dont now how to fake that :(
+            catch (Throwable $e) {
+                DB::rollBack();
+
+                return false;
+            }
+        }
+        else return false;
+    }
+}

app/Services/TwoFAccountService.php

@@ -5,6 +5,7 @@ namespace App\Services;
 use App\TwoFAccount;
 use App\Exceptions\InvalidSecretException;
 use App\Exceptions\InvalidOtpParameterException;
+use App\Exceptions\UndecipherableException;
 use App\Services\Dto\OtpDto;
 use App\Services\Dto\TwoFAccountDto;
 use OTPHP\TOTP;
@@ -118,11 +119,17 @@ class TwoFAccountService
      * @return OtpDto an OTP DTO
      * 
      * @throws InvalidSecretException The secret is not a valid base32 encoded string
+     * @throws UndecipherableException The secret cannot be deciphered
      */
     public function getOTP($data) : OtpDto
     {
         $this->initTokenWith($data);
         $OtpDto = new OtpDto();
+
+        // Early exit if the model returned an undecipherable secret
+        if (strtolower($this->token->getSecret()) === __('errors.indecipherable')) {
+            throw new UndecipherableException();
+        }
         
         try {
             if ( $this->tokenOtpType() === 'totp' ) {

app/TwoFAccount.php

@@ -15,11 +15,6 @@ class TwoFAccount extends Model implements Sortable
 
     use SortableTrait;
 
-    /**
-     * A human understandable value to return when attribute decryption fails
-     */
-    private const INDECIPHERABLE = '*indecipherable*';
-
 
     /**
      * model's array form.
@@ -199,7 +194,7 @@ class TwoFAccount extends Model implements Sortable
                 return Crypt::decryptString($value);
             }
             catch (Exception $e) {
-                return self::INDECIPHERABLE;
+                return __('errors.indecipherable');
             }
         }
         else {

resources/js/views/Accounts.vue

@@ -84,7 +84,7 @@
                                 <div class="tfa-cell tfa-content is-size-3 is-size-4-mobile" @click.stop="showAccount(account)">  
                                     <div class="tfa-text has-ellipsis">
                                         <img :src="'/storage/icons/' + account.icon" v-if="account.icon && $root.appSettings.showAccountsIcons">
-                                        {{ displayService(account.service) }}<font-awesome-icon class="has-text-danger is-size-5 ml-2" v-if="$root.appSettings.useEncryption && account.isConsistent === false" :icon="['fas', 'exclamation-circle']" />
+                                        {{ displayService(account.service) }}<font-awesome-icon class="has-text-danger is-size-5 ml-2" v-if="$root.appSettings.useEncryption && account.account === $t('errors.indecipherable')" :icon="['fas', 'exclamation-circle']" />
                                         <span class="is-family-primary is-size-6 is-size-7-mobile has-text-grey ">{{ account.account }}</span>
                                     </div>
                                 </div>

resources/lang/en/errors.php

@@ -28,5 +28,7 @@ return [
     'error_during_decryption' => 'Decryption failed, your database is still protected. This is mainly caused by an integrity issue of encrypted data for one or more accounts.',
     'qrcode_cannot_be_read' => 'This QR code is unreadable',
     'too_many_ids' => 'too many ids were included in the query parameter, max 100 allowed',
-    'delete_user_setting_only' => 'Only user-created setting can be deleted'
+    'delete_user_setting_only' => 'Only user-created setting can be deleted',
+    'indecipherable' => '*indecipherable*',
+    'cannot_decipher_secret' => 'The secret cannot be deciphered. This is mainly caused by a wrong APP_KEY set in the .env configuration file of 2Fauth or a corrupted data stored in database.',
 ];