Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[8.x] Add encrypted string Eloquent cast #34937

Merged
merged 2 commits into from
Oct 23, 2020

Conversation

jasonmccreary
Copy link
Contributor

I know this has been attempted and closed before. Please hear me out...

First, Laravel has evolved since this was first attempted way back in Laravel 5.3. The framework includes many more quality of life improvements which help developers with every day tasks. I consider encryption to be one of those tasks.

Second, the common reason for closing this before was that it may be done with a custom cast or accessor/mutator. Be that as it may, this creates just that tiny bit of friction which may prevent developers from securing their data. As encryption is a first-class feature within Laravel, so too should be persisting encrypted data.

For those reasons, I am reviving this as a fresh PR to include a built-in Eloquent cast of encrypted.

This cast will automatically handle the encryption and decryption of simple string. For anything more complex, I defer to custom model casts or future PRs.

Example:

public $casts = [
    'access_token' => 'encrypted',
];

jasonmccreary and others added 2 commits October 22, 2020 14:34
Co-authored-by: Jess Archer <jess@jessarcher.com>
@jasonmccreary
Copy link
Contributor Author

I am not tied to the naming or implementation of this cast. Only that it is built-in. So I welcome any feedback.

If merged, I am glad to submit a PR to update the docs as well.

@tomschlick
Copy link
Contributor

I like the idea of a cast here, but how would you handle an instance of say a json string needing encrypted? Would that necessitate a use case for encrypted_json / encrypted_array casts? Or should those cases be handled manually through getXAttribute() calls?

@jasonmccreary
Copy link
Contributor Author

@tomschlick, more complex encryption would definitely require a custom cast. However, I could see future additions for encrypted:json, encrypted:object, etc similar to the format of the date casts. 👍

@driesvints driesvints changed the title Add encrypted string Eloquent cast [8.x] Add encrypted string Eloquent cast Oct 23, 2020
@Patabugen
Copy link

I love this - just a suggestion, but might this work smoother alongside other casts by having a list of encrypted fields, then the other casting can be the same and we don't end up wanting to create an encrypted and non-encrypted version of each one:

public $encrypted = [
    'access_token',
    'launch_date,
];

public $dates = [
    'launch_date'
];

@jasonmccreary
Copy link
Contributor Author

@Patabugen this was the approach in #29663. I do think that offers more flexibility, per @tomschlick's point. However, given how it was closed, I thought this was a good start to cover the common use case.

@tontonsb
Copy link
Contributor

tontonsb commented Oct 24, 2020

Damn, I am a bit late on this one. One of the objections on previous PRs was that this binds Eloquent and Laravel's encryption. Doesn't your implementation harm the Laravel-less use of Eloquent? Or is it somehow indirectly required already in illuminate/database?

Thanks to the custom cast classes introduced in Laravel 7, this could be accomplished by adding the more Laravel-specific casts in a separate location, like Support or simply Casts. Here's the API of my encryption and hashing package:

protected $casts = [
    // hashes the value when assigning to $model->password
    'password' => Password::class,

    // encrypts on write, decrypts on read
    'classified' => Encrypted::class,

    // encrypts on write, decrypts & typecasts on read
    'secret' => Encrypted::class.':integer',
];

I think pretty much the same solution could be shipped with Laravel. @jasonmccreary do you see any reasons against putting the encryption cast in a separate class and outside of the database component?

@tomschlick on most cases there should be no issues whatsoever because encryption serializes and decryption deserializes. You get whatever you put in initially. And when you need the typecasts... Doing a combined cast seems a bit wrong and that's the main reason why I haven't even attempted to make a PR out of my encryption cast package. Chainable casts would be the real solution to problems like that.

@RichardStyles
Copy link

I created a similar Encryption Cast package for Eloquent. However, it creates 4096-bit RSA keys to do the encryption/decryption rather than the built-in Crypt class. This then allows you to rotate your app:key without losing access to your encrypted data.

https://github.com/RichardStyles/EloquentEncryption

@mikemand
Copy link
Contributor

mikemand commented Oct 27, 2020

I love this - just a suggestion, but might this work smoother alongside other casts by having a list of encrypted fields, then the other casting can be the same and we don't end up wanting to create an encrypted and non-encrypted version of each one:

public $encrypted = [
    'access_token',
    'launch_date,
];
public $dates = [
    'launch_date'
];

Correct me if I'm wrong, but the $dates property was deprecated in Laravel 8. Everything is handled by $casts now.

@jasonmccreary
Copy link
Contributor Author

@mikemand indeed. This wasn't the best example, but aimed to demonstrate the use case for compound or chained casts.

@mikemand
Copy link
Contributor

@jasonmccreary Ah, I see. I like the syntax you created in #34948. Great work on this. I can depreciate my own Encrypt cast now. 👍

@tontonsb
Copy link
Contributor

@jasonmccreary @mikemand Is it really deprecated? I still see it in the docs: https://laravel.com/docs/8.x/eloquent-mutators#date-mutators

And I don't see any note on that in the upgrade guide.

@mikemand
Copy link
Contributor

@tontonsb It doesn't look like it was documented beyond the docblock for some reason: c0e0bb5 I remember reading somewhere that it was being depreciated, but I don't remember where that was; maybe Taylor's Twitter.

@jadamec
Copy link

jadamec commented Nov 18, 2020

Works great, however, when User has an encrypted email, Login actually doesn't work - "The account with the specified email address does not exist.".

protected $casts = [
  'name' => 'encrypted',
  'email' => 'encrypted',
  'email_verified_at' => 'datetime',
];

@RichardStyles
Copy link

RichardStyles commented Nov 18, 2020

@jadamec Whenever you encrypt data you have to be aware that this removes/limits simple searching from your DB as you can't query against it easily.
As the encryption key etc is in your APP level, not the DB level so the query builder parameters will be using the unencrypted term. The query builder doesn't cast the values, and depending on your encryption method these may not be the same each time if it did.
You might be able to go to a username based approach and have this unencrypted.
I'd advise to only encrypt what is absolutely necessary for details such as SSN (USA) or NI (UK) which people can't change.

@askmrsinh
Copy link

askmrsinh commented Jun 14, 2023

For note, it seems like you can't hide and encrypted cast a field at the same time as of today. So, something like this in a model might give you errors or null values:

protected $hidden = [
    'refresh_token',
    'access_token',
];

protected $casts = [
    'refresh_token' => 'encrypted', // is also hidden for serialization
    'access_token' => 'encrypted', // is also hidden for serialization
    'expires_at' => 'datetime',
];

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants