delatbabel / elocryptfive

Automatically encrypt and decrypt Laravel 5 Eloquent values
MIT License
87 stars 28 forks source link

Laravel 5.5 - data is stored as decrypted after model update #22

Open bjauy opened 7 years ago

bjauy commented 7 years ago

I have been testing this package for few days on Laravel 5.5. Recently, I've checked table content and some of the rows weren't encrypted at all.

After few tests in artisan tinker I've nailed the issue to updates that make the data unencrypted. Model creation works as expected.

The problem happens also in default L5.5 installation, using MySQL (MariaDB 10.1.26 exactly):

  1. Database and package setup:

    composer create-project --prefer-dist laravel/laravel laravel55
    cd laravel55
    composer require delatbabel/elocryptfive
    php artisan make:auth
    php artisan migrate:fresh

    (if there is an error with key length, change encoding values in config/database.php to 'utf8' and 'utf8_general_ci' accordingly)

  2. Add below to app/User.php:

use Delatbabel\Elocrypt\Elocrypt;

class User extends Authenticatable
{
    use Elocrypt;

   [...]

   protected $encrypts = [ 'name', ];

   [...]
  1. Test in tinker:
>>> User::create(['name' => 'test', 'email' => 'test@example.com', 'password' => Hash::make('test')]);

The value for name is encrypted.

>>> User::find(1)->update(['email' => 'example@example.com']);

After the update table preview shows data as unecrypted.

dokicro commented 7 years ago

Hey, I have the same problem did you found any solutions?

bjauy commented 7 years ago

Yes, you need to override getDirty() method in your models which depend on Elocrypt trait:

public function getDirty()
{
    $dirty = [];

    foreach ($this->attributes as $key => $value) {
        if (! $this->originalIsEquivalent($key, $value)) {
                    $dirty[$key] = $value;
        }
    }

    return $dirty;
}
dokicro commented 7 years ago

Thanks 👍

delatbabel commented 7 years ago

Hrm, interesting. I wonder if it can be over-ridden in the trait and if doing so will cause breakages on prior versions of Laravel? I haven't started migrating any projects to L5.5 yet although it's a planned change with the new LTS release. I will keep this ticket open for monitoring and if anyone has any further information.

bjauy commented 7 years ago

IIRC originalIsNumericallyEquivalent method was renamed (and refactored) to originalIsEquivalent in L5.5, so simply adding getDirty would break the trait for users of older versions.

Maybe we could just add second, optional trait with just getDirty() overriden? This, and an information in the readme should be enough IMHO.

Users of older Laravel versions wouldn't be affected by the changes.

I can prepare pull request if you don't mind and if the change is ok for you.

schonhose commented 7 years ago

I just want to chime in that I am also looking to migrate from Laravel 5.1 to 5.5. I use this package to encrypt data in my database so I welcome any ideas on how to make it work on 5.5.

mrgrain commented 6 years ago

There is also an other bug with Laravel 5.5: If an attribute is casted, the getDirty will fail as well in certain cases. Essentially Laravel is casting the encrypted values which will result in different encrypted values being regarded as the same. E.g. object, array or json will be tried to be read from JSON. However, since the provided input is encrypted, the casted value evaluates to false no matter what.

Our solution is to overwrite the castAttribute method


    /**
     * Decrypt encrypted data before it is processed by cast attribute
     * @param $key
     * @param $value
     *
     * @return mixed
     */
    protected function castAttribute($key, $value)
    {
        return parent::castAttribute($key, $this->doDecryptAttribute($key, $value));
    }
delatbabel commented 6 years ago

Thanks for the information. I'm continuing to track this. I'm about to migrate one project from 5.1 LTS to 5.5 LTS so I will try to build something that works for both LTS versions.

mrgrain commented 6 years ago

Instead of trying to fix it for both versions at the same time (or forks or whatever), @delatbabel you could just release a new version which requires Laravel 5.5

mbardelmeijer commented 6 years ago

@delatbabel any update on this?

austinheap commented 6 years ago

I ended up 1) forking this, 2) fixing all the Laravel 5.5 issues, 3) dropping support for Laravel below 5.5, and 4) a bunch of random cleanup & unit tests. Available below:

ITwrx commented 6 years ago

@mrgrain where should one override the castAttribute method? thanks

mrgrain commented 6 years ago

@ITwrx Overwrite it in the model that needs to be encrypted. We have a base model for all encrypted models which uses Elocrypt's trait and overrides the castAttribute method (amongst other things).

Or you could simply create your own EncryptedModel trait which extends the original Elocrypt one.

ITwrx commented 6 years ago

@mrgrain cool, thanks.

delatbabel commented 6 years ago

Hi all, firstly sorry about the delay. I have had a long period of illness, just getting back to health now. I'll certainly look at integrating the patches into a Laravel 5.5 branch shortly.

ITwrx commented 6 years ago

glad you're back. stay well.