Merge pull request #14602 from uberbrady/fix_saving_encrypted_custom_fields

Re-enabled updating encrypted custom fields via API [sc-41465]

app/Http/Controllers/Api/AssetsController.php

@@ -665,25 +665,26 @@ class AssetsController extends Controller
             $model = AssetModel::find($asset->model_id);
             
             // Update custom fields
+            $problems_updating_encrypted_custom_fields = false;
             if (($model) && (isset($model->fieldset))) {
                 foreach ($model->fieldset->fields as $field) {
                     $field_val = $request->input($field->db_column, null);
 
                     if ($request->has($field->db_column)) {
-                        if ($field->field_encrypted == '1') {
-                            if (Gate::allows('admin')) {
-                                $asset->{$field->db_column} = Crypt::encrypt($field_val);
-                            }
-                        }
                         if ($field->element == 'checkbox') {
                             if(is_array($field_val)) {
                                 $field_val = implode(',', $field_val);
-                                $asset->{$field->db_column} = $field_val;
                             }
                         }
-                        else {
+                        if ($field->field_encrypted == '1') {
-                            $asset->{$field->db_column} = $field_val;
+                            if (Gate::allows('admin')) {
+                                $field_val = Crypt::encrypt($field_val);
+                            } else {
+                                $problems_updating_encrypted_custom_fields = true;
+                                continue;
+                            }
                         }
+                        $asset->{$field->db_column} = $field_val;
                     }
                 }
             }
@@ -709,8 +710,11 @@ class AssetsController extends Controller
                     $asset->image = $asset->getImageUrl();
                 }
 
-                return response()->json(Helper::formatStandardApiResponse('success', $asset, trans('admin/hardware/message.update.success')));
+                if ($problems_updating_encrypted_custom_fields) {
-                return response()->json(Helper::formatStandardApiResponse('success', (new AssetsTransformer)->transformAsset($asset), trans('admin/hardware/message.update.success')));
+                    return response()->json(Helper::formatStandardApiResponse('success', $asset, trans('admin/hardware/message.update.encrypted_warning')));
+                } else {
+                    return response()->json(Helper::formatStandardApiResponse('success', $asset, trans('admin/hardware/message.update.success')));
+                }
             }
 
             return response()->json(Helper::formatStandardApiResponse('error', null, $asset->getErrors()), 200);

database/factories/AssetFactory.php

@@ -4,6 +4,7 @@ namespace Database\Factories;
 
 use App\Models\Asset;
 use App\Models\AssetModel;
+use App\Models\CustomField;
 use App\Models\Location;
 use App\Models\Statuslabel;
 use App\Models\Supplier;
@@ -353,6 +354,16 @@ class AssetFactory extends Factory
         return $this->state(['requestable' => false]);
     }
 
+    public function hasEncryptedCustomField(CustomField $field = null)
+    {
+        return $this->state(function () use ($field) {
+            return [
+                'model_id' => AssetModel::factory()->hasEncryptedCustomField($field),
+            ];
+        });
+    }
+
+
     /**
      * This allows bypassing model level validation if you want to purposefully
      * create an asset in an invalid state. Validation is turned back on

database/factories/AssetModelFactory.php

@@ -3,6 +3,7 @@
 namespace Database\Factories;
 
 use App\Models\AssetModel;
+use App\Models\CustomField;
 use App\Models\CustomFieldset;
 use App\Models\Depreciation;
 use App\Models\Manufacturer;
@@ -429,4 +430,13 @@ class AssetModelFactory extends Factory
             ];
         });
     }
+
+    public function hasEncryptedCustomField(CustomField $field = null)
+    {
+        return $this->state(function () use ($field) {
+            return [
+                'fieldset_id' => CustomFieldset::factory()->hasEncryptedCustomField($field),
+            ];
+        });
+    }
 }

database/factories/CustomFieldsetFactory.php

@@ -3,6 +3,7 @@
 namespace Database\Factories;
 
 use App\Models\CustomFieldset;
+use App\Models\CustomField;
 use Illuminate\Database\Eloquent\Factories\Factory;
 
 class CustomFieldsetFactory extends Factory
@@ -43,4 +44,13 @@ class CustomFieldsetFactory extends Factory
             ];
         });
     }
+
+    public function hasEncryptedCustomField(CustomField $field = null)
+    {
+        return $this->afterCreating(function (CustomFieldset $fieldset) use ($field) {
+            $field = $field ?? CustomField::factory()->testEncrypted()->create();
+
+            $fieldset->fields()->attach($field, ['order' => '1', 'required' => false]);
+        });
+    }
 }

resources/lang/en-US/admin/hardware/message.php

@@ -17,6 +17,7 @@ return [
     'update' => [
         'error'   			=> 'Asset was not updated, please try again',
         'success' 			=> 'Asset updated successfully.',
+        'encrypted_warning' => 'Asset updated successfully, but encrypted custom fields were not due to permissions',
         'nothing_updated'	=>  'No fields were selected, so nothing was updated.',
         'no_assets_selected'  =>  'No assets were selected, so nothing was updated.',
         'assets_do_not_exist_or_are_invalid' => 'Selected assets cannot be updated.',

tests/Feature/Api/Assets/AssetStoreTest.php

@@ -5,10 +5,12 @@ namespace Tests\Feature\Api\Assets;
 use App\Models\Asset;
 use App\Models\AssetModel;
 use App\Models\Company;
+use App\Models\CustomField;
 use App\Models\Location;
 use App\Models\Statuslabel;
 use App\Models\Supplier;
 use App\Models\User;
+use Illuminate\Support\Facades\Crypt;
 use Illuminate\Testing\Fluent\AssertableJson;
 use Tests\TestCase;
 
@@ -479,4 +481,55 @@ class AssetStoreTest extends TestCase
                 $json->has('messages.company_id')->etc();
             });
     }
+
+    public function testEncryptedCustomFieldCanBeStored()
+    {
+        $this->markIncompleteIfMySQL('Custom Fields tests do not work on MySQL');
+
+        $status = Statuslabel::factory()->create();
+        $field = CustomField::factory()->testEncrypted()->create();
+        $superuser = User::factory()->superuser()->create();
+        $assetData = Asset::factory()->hasEncryptedCustomField($field)->make();
+
+        $response = $this->actingAsForApi($superuser)
+            ->postJson(route('api.assets.store'), [
+                $field->db_column_name() => 'This is encrypted field',
+                'model_id' => $assetData->model->id,
+                'status_id' => $status->id,
+                'asset_tag' => '1234',
+            ])
+            ->assertStatusMessageIs('success')
+            ->assertOk()
+            ->json();
+
+        $asset = Asset::findOrFail($response['payload']['id']);
+        $this->assertEquals('This is encrypted field', Crypt::decrypt($asset->{$field->db_column_name()}));
+    }
+
+    public function testPermissionNeededToStoreEncryptedField()
+    {
+        // @todo:
+        $this->markTestIncomplete();
+
+        $status = Statuslabel::factory()->create();
+        $field = CustomField::factory()->testEncrypted()->create();
+        $normal_user = User::factory()->editAssets()->create();
+        $assetData = Asset::factory()->hasEncryptedCustomField($field)->make();
+
+        $response = $this->actingAsForApi($normal_user)
+            ->postJson(route('api.assets.store'), [
+                $field->db_column_name() => 'Some Other Value Entirely!',
+                'model_id' => $assetData->model->id,
+                'status_id' => $status->id,
+                'asset_tag' => '1234',
+            ])
+            // @todo: this is 403 unauthorized
+            ->assertStatusMessageIs('success')
+            ->assertOk()
+            ->assertMessagesAre('Asset updated successfully, but encrypted custom fields were not due to permissions')
+            ->json();
+
+        $asset = Asset::findOrFail($response['payload']['id']);
+        $this->assertEquals('This is encrypted field', Crypt::decrypt($asset->{$field->db_column_name()}));
+    }
 }

tests/Feature/Api/Assets/AssetUpdateTest.php

@@ -0,0 +1,55 @@
+<?php
+
+namespace Tests\Feature\Api\Assets;
+
+use App\Models\Asset;
+use App\Models\CustomField;
+use App\Models\User;
+use Illuminate\Support\Facades\Crypt;
+use Tests\TestCase;
+
+class AssetUpdateTest extends TestCase
+{
+    public function testEncryptedCustomFieldCanBeUpdated()
+    {
+        $this->markIncompleteIfMySQL('Custom Fields tests do not work on MySQL');
+
+        $field = CustomField::factory()->testEncrypted()->create();
+        $asset = Asset::factory()->hasEncryptedCustomField($field)->create();
+        $superuser = User::factory()->superuser()->create();
+
+        $this->actingAsForApi($superuser)
+            ->patchJson(route('api.assets.update', $asset->id), [
+                $field->db_column_name() => 'This is encrypted field'
+            ])
+            ->assertStatusMessageIs('success')
+            ->assertOk();
+
+        $asset->refresh();
+        $this->assertEquals('This is encrypted field', Crypt::decrypt($asset->{$field->db_column_name()}));
+    }
+
+    public function testPermissionNeededToUpdateEncryptedField()
+    {
+        $this->markIncompleteIfMySQL('Custom Fields tests do not work on MySQL');
+
+        $field = CustomField::factory()->testEncrypted()->create();
+        $asset = Asset::factory()->hasEncryptedCustomField($field)->create();
+        $normal_user = User::factory()->editAssets()->create();
+
+        $asset->{$field->db_column_name()} = Crypt::encrypt("encrypted value should not change");
+        $asset->save();
+
+        // test that a 'normal' user *cannot* change the encrypted custom field
+        $this->actingAsForApi($normal_user)
+            ->patchJson(route('api.assets.update', $asset->id), [
+                $field->db_column_name() => 'Some Other Value Entirely!'
+            ])
+            ->assertStatusMessageIs('success')
+            ->assertOk()
+            ->assertMessagesAre('Asset updated successfully, but encrypted custom fields were not due to permissions');
+
+        $asset->refresh();
+        $this->assertEquals("encrypted value should not change", Crypt::decrypt($asset->{$field->db_column_name()}));
+    }
+}

tests/Support/CanSkipTests.php

@@ -0,0 +1,13 @@
+<?php
+
+namespace Tests\Support;
+
+trait CanSkipTests
+{
+    public function markIncompleteIfMySQL($message = 'Test skipped due to database driver being MySQL.')
+    {
+        if (config('database.default') === 'mysql') {
+            $this->markTestIncomplete($message);
+        }
+    }
+}

tests/Support/CustomTestMacros.php

@@ -87,5 +87,18 @@ trait CustomTestMacros
                 return $this;
             }
         );
+
+        TestResponse::macro(
+            'assertMessagesAre',
+            function (string $message) {
+                Assert::assertEquals(
+                    $message,
+                    $this['messages'],
+                    "Response messages was not {$message}"
+                );
+
+                return $this;
+            }
+        );
     }
 }

tests/TestCase.php

@@ -7,6 +7,7 @@ use Illuminate\Foundation\Testing\LazilyRefreshDatabase;
 use Illuminate\Foundation\Testing\TestCase as BaseTestCase;
 use RuntimeException;
 use Tests\Support\AssertsAgainstSlackNotifications;
+use Tests\Support\CanSkipTests;
 use Tests\Support\CustomTestMacros;
 use Tests\Support\InteractsWithAuthentication;
 use Tests\Support\InitializesSettings;
@@ -14,6 +15,7 @@ use Tests\Support\InitializesSettings;
 abstract class TestCase extends BaseTestCase
 {
     use AssertsAgainstSlackNotifications;
+    use CanSkipTests;
     use CreatesApplication;
     use CustomTestMacros;
     use InteractsWithAuthentication;