Skip to content

Commit 796786c

Browse files
paperscissorspaperscissors
committed
fix double encryption issue - only encrypt on save, not on set
1 parent 6001f71 commit 796786c

File tree

2 files changed

+113
-16
lines changed

2 files changed

+113
-16
lines changed

‎src/Encryptable.php‎

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,11 @@ protected function encryptAttributes()
4949

5050
foreach ($this->getEncryptableAttributes() as $attribute) {
5151
if (isset($this->attributes[$attribute]) && ! empty($this->attributes[$attribute])) {
52-
// Use the field-specific encryption that handles character limits
53-
$this->attributes[$attribute] = $this->encryptValueForStorage($this->attributes[$attribute], $databaseHasLimits);
52+
// Only encrypt if the value is not already encrypted
53+
if (!$this->isValueEncrypted($this->attributes[$attribute])) {
54+
// Use the field-specific encryption that handles character limits
55+
$this->attributes[$attribute] = $this->encryptValueForStorage($this->attributes[$attribute], $databaseHasLimits);
56+
}
5457
}
5558
}
5659
}
@@ -114,7 +117,19 @@ protected function encryptValueForStorage($value, $databaseHasLimits = false)
114117
*/
115118
protected function decryptValue($value)
116119
{
117-
return $this->getEncryptionService()->decrypt($value);
120+
$decrypted = $this->getEncryptionService()->decrypt($value);
121+
122+
// Check if the decrypted value is still encrypted (double encryption scenario)
123+
if (is_string($decrypted) && $this->isValueEncrypted($decrypted)) {
124+
try {
125+
return $this->getEncryptionService()->decrypt($decrypted);
126+
} catch (\Exception $e) {
127+
// If second decryption fails, return the first decryption result
128+
return $decrypted;
129+
}
130+
}
131+
132+
return $decrypted;
118133
}
119134

120135
/**
@@ -155,8 +170,7 @@ public function __get($key)
155170

156171
/**
157172
* Dynamically set attributes.
158-
* Ensures attributes are correctly marked for encryption when set through
159-
* notification or other dynamic methods.
173+
* Ensures attributes are stored as plain text when set, to be encrypted on save.
160174
*
161175
* @param string $key
162176
* @param mixed $value
@@ -169,18 +183,9 @@ public function __set($key, $value)
169183
return parent::__set($key, $value);
170184
}
171185

172-
// Set the attribute as normal
186+
// Set the attribute as normal - do NOT encrypt here
187+
// Encryption should only happen on save via the saving event
173188
parent::__set($key, $value);
174-
175-
// If the attribute should be encrypted, make sure it's encrypted
176-
$encryptableAttributes = $this->getEncryptableAttributes();
177-
178-
if (in_array($key, $encryptableAttributes) && isset($this->attributes[$key]) && !empty($value)) {
179-
// Don't double-encrypt - only encrypt if the value is not already encrypted
180-
if (!$this->isValueEncrypted($value)) {
181-
$this->attributes[$key] = $this->encryptValue($value);
182-
}
183-
}
184189
}
185190

186191
/**

‎tests/Unit/EncryptableTest.php‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,98 @@ public function testDecryptionWorksForRelationshipLoadedModels()
391391
$this->assertEquals('123 Main Street', $array['street_1']);
392392
$this->assertEquals('Apt 4B', $array['street_2']);
393393
}
394+
395+
public function testDoubleEncryptionDecryption()
396+
{
397+
// Create a test table
398+
$this->app['db']->connection()->getSchemaBuilder()->create('test_models', function ($table) {
399+
$table->increments('id');
400+
$table->text('address')->nullable();
401+
$table->timestamps();
402+
});
403+
404+
$encryptionService = $this->app->make(EncryptionService::class);
405+
406+
// Use reflection to access compact encryption
407+
$reflection = new \ReflectionClass($encryptionService);
408+
$compactEncryptMethod = $reflection->getMethod('compactEncrypt');
409+
$compactEncryptMethod->setAccessible(true);
410+
411+
// Create a double-encrypted scenario:
412+
// 1. First encrypt with Laravel standard encryption
413+
// 2. Then encrypt that result with compact encryption
414+
$originalValue = '123 Main Street';
415+
$firstEncryption = $encryptionService->encrypt($originalValue); // Laravel standard
416+
$doubleEncrypted = $compactEncryptMethod->invokeArgs($encryptionService, [$firstEncryption]); // Compact of encrypted
417+
418+
// Manually insert into database with double encryption
419+
$id = $this->app['db']->connection()->table('test_models')->insertGetId([
420+
'address' => $doubleEncrypted,
421+
'created_at' => now(),
422+
'updated_at' => now(),
423+
]);
424+
425+
// Retrieve using Eloquent - this should handle double decryption
426+
$model = TestEncryptableModel::find($id);
427+
428+
// Check that the value is properly decrypted (should handle double encryption)
429+
$this->assertEquals($originalValue, $model->address);
430+
431+
// Verify the raw database value is the double encrypted value
432+
$rawData = $this->app['db']->connection()->table('test_models')->where('id', $id)->first();
433+
$this->assertTrue(strpos($rawData->address, 'c:') === 0);
434+
$this->assertNotEquals($originalValue, $rawData->address);
435+
}
436+
437+
public function testSingleEncryptionOnSaveOnly()
438+
{
439+
// Create a test table
440+
$this->app['db']->connection()->getSchemaBuilder()->create('test_models', function ($table) {
441+
$table->increments('id');
442+
$table->text('email')->nullable();
443+
$table->text('phone')->nullable();
444+
$table->timestamps();
445+
});
446+
447+
// Create a new model
448+
$model = new TestEncryptableModel();
449+
450+
// Set values - these should NOT be encrypted yet
451+
$model->email = '[email protected]';
452+
$model->phone = '123-456-7890';
453+
454+
// Check that the values are still plain text before save
455+
$this->assertEquals('[email protected]', $model->email);
456+
$this->assertEquals('123-456-7890', $model->phone);
457+
458+
// Check the raw attributes are also plain text
459+
$this->assertEquals('[email protected]', $model->getAttributes()['email']);
460+
$this->assertEquals('123-456-7890', $model->getAttributes()['phone']);
461+
462+
// Now save the model - this should encrypt the values
463+
$model->save();
464+
465+
// After save, when accessed, they should still return the original values
466+
$this->assertEquals('[email protected]', $model->email);
467+
$this->assertEquals('123-456-7890', $model->phone);
468+
469+
// But the raw database should be encrypted
470+
$rawData = $this->app['db']->connection()->table('test_models')->where('id', $model->id)->first();
471+
$this->assertNotEquals('[email protected]', $rawData->email);
472+
$this->assertNotEquals('123-456-7890', $rawData->phone);
473+
474+
// And they should be detectable as encrypted
475+
$reflection = new \ReflectionClass($model);
476+
$method = $reflection->getMethod('isValueEncrypted');
477+
$method->setAccessible(true);
478+
$this->assertTrue($method->invokeArgs($model, [$rawData->email]));
479+
$this->assertTrue($method->invokeArgs($model, [$rawData->phone]));
480+
481+
// Retrieve a fresh instance and verify decryption works
482+
$freshModel = TestEncryptableModel::find($model->id);
483+
$this->assertEquals('[email protected]', $freshModel->email);
484+
$this->assertEquals('123-456-7890', $freshModel->phone);
485+
}
394486
}
395487

396488
class TestEncryptableModel extends Model

0 commit comments

Comments
 (0)