ambta/DoctrineEncryptBundle

ext/mcrypt is deprecated in php 7.1, will be removed in 7.2

bruno-ds opened this issue · 7 comments

Hello,
since ext/mcrypt is deprecated in php 7.1, will be removed in 7.2, it would be great to use OpenSSL instead of mcrypt.
I'm starting a fork (https://github.com/Combodo/DoctrineEncryptBundle) in order to do this, please tell me if you'r interested in merging my changes or if I make it an autonomous fork.

Hi brunocombodo,

Thank you for your comment. Feel free to fork, we will review submitted pull request from time to time and merge the changes if they are good :).

Due the amount of work we currently have we are unable to keep up the work on this repo ourselves so all help is welcome.

@bruno-ds happy you're forking.

will there be an upgrade / migration requirement ? or will this simply be a dependency switch ?

@jayesbe I just pushed a new version wich needs custom configuration on each field.
(because with the current code, the encrypted colums where always updated on a flush)

In short word : it is not a drop in replacement, see : https://github.com/Combodo/DoctrineEncryptBundle#scope-of-this-fork and https://github.com/Combodo/DoctrineEncryptBundle#this-verstion-30-drop-support-from-many-functionnalities-of-the-20-

PS : I'm not an expert either in encryption or in open source projects, so you should re-read the code ans use it at your own risks ;)

/** @var string store the decryptedVersion of $host */
private $hostDecrypted; 

why do you need this ?

why can't getHostDecrypted() decrypt when called ?

getHostDecrypted() {
    return decrypt($this->host); // pseudo 
}

the original cod modified the field with the decrypted value, so doctrine detected it and on a flush operation it updated the field in database even if nothing has changed (see #24).
In order to correct this behaviour wich can be ressource consuming I had to store the decrypted value elsewhere, so I did it there.

PS : By respect for ambta I invite you not to pollute their issue tracker and to create an issue on the fork's issue tracker .


EDIT : I just understood your question : you're rigth, maybe it could be better, so the decrypt version could be lazy loaded... if you have time I invite you to make a PR, if not, maybe I'll look at it if I have performance issue.

@bruno-ds your fork is missing an issues section.

@jayesbe sorry, this is corrected