Make it possible to define DataObject specific encryption keys.

[Taitava]

Hi,

a project of mine requires that each Member can have a different encryption key than what other members use, so I created this new feature which allows actually every DataObject to define a custom key (if needed).

The TLDR guide on how to use this feature is just to create a new method in your DataObject class:

class MyDataObject extends DataObject
{
    private static $db = array(
        'MyEncryptedField' => 'EncryptedText'
    );

    public function provideEncryptionKey($field_name, $field_type)
    {
        return *A custom key here*;
    }
}

When you have this method defined, the module now overrides the ENCRYPT_AT_REST_KEY key when encrypting or decrypting a value. This allows you to create any kind of advanced logic for which key to use.

I have also updated the readme file to describe how to use this feature. Of course the default behaviour is still to use the constant defined in _ss_environment.php.

---

So, last but not least, as this might be a big change to the module and your plans may differ from this, please let me know your opinion about it. If there's anything you would like me to change in this pull request, I would gladly hear it, as it's you module anyway :). Also you can do any changes yourself if you wish.

I have to say that I haven't really had time to test this very much, but at least the little testing that I have done has worked.

P.S. I would be glad to have a look at the couple of issues that are currently open for this module, perhaps I can do something for them. I'll let you know when I have proceeded with that, but unfortunately I can't promise anything.

[Firesphere]

I'll (finally) will give this a go this week, thanks!

[Taitava]

Thanks! :)

[Firesphere]

@madmatt It looks pretty good to me and seems to work as expected, could you give it a look as well? As it's quite a major refactor on reading the keys.

@Taitava Thanks for the effort!

[Firesphere]

ping @madmatt

[madmatt]

Hi @Taitava,

I'm so sorry for missing this - my GitHub notifications are a little wonky and I never saw the pings on this. My apologies!

The code overall looks good - could we get some unit tests added for this? The module doesn't currently have a huge range of tests, so just a single one that defines different encryption keys and pushes through some of the trait code would be great :)

Only other comments are some minor PSR-2 fixes (though we could do those as a separate pass once your work is merged) and I was pretty sure you don't need to require_once() traits in SS3, was there a specific version you were using or did I just get really lucky whenever I've used them before?

Thanks!

[Taitava]

No problem, I understand that some times maintaining a module/project is not so fast :). These are free and open source projects any way :). Nice to hear that my code is suitable for this module :).

Unit tests are something I'm not familiar with - but I definitively must learn to make them, as I actually need to start using them in my work anyway. So yes, I will find some time to start learning how to make unit tests and then I'll write one for this pull request. I just don't know yet when I have time to do this.

PSR-2 is another new thing to me. If it's not too much of trouble to you, i think you could do the PSR-2 conversion if it goes easily. If not, I can do that too.

About the include_once part, I rarely use traits so I really don't know how SilverStripe's auto detection works with traits. I just remember that I was not able to get the trait included before I used the require_once command. I have probably only tested this in one version of SS3 (can't remember which specific version) so it might be a bug in that particular SS version, but I'm not sure. Hmm, actually I now Googled and found this trait-loader module, so I suspect that SS3 simply does not auto load traits by default. Or if it does, then that feature must be quite new. Anyway, I admit that using the require_once command is not very nice.

How do we continue? Do you want this PR to include the unit test before you will merge it, or is it sufficient if this gets merged without a unit test and then I would create another PR when I have the unit test ready?

[Firesphere]

To get the traits working, you'll need to declare them in the composer.json, for SS3, I think? Can't find an example right now :/

[Taitava]

Okay, that sounds good. Just one thing comes to my mind, if somebody does not use composer, then they will wonder why the trait is not working. Should there be a note in the installation section of the readme that says something like "if you don't use composer, put require_once(path-to-the-trait) somewhere in your _config.php or to some other place"? Of course it's recommended to use composer, but as it's not a requirement, I would suggest this kind of mention in the documentation. But it's just a small issue. :)

[madmatt]

Hey @Taitava, I just wanted to follow up on this PR, it's been a few years and I never got around to fixing it up or merging sorry! Are you still wanting to get this across the line? I'll close the PR for now, but please feel free to re-open or comment on it if you'd still like to get this included in the module.

[Taitava]

Hi @madmatt ! 🙂 No worries! Personally, I haven't been working on any SilverStripe projects anymore in many years, so I don't need this at the moment. If I ever come back to a project I used this module with, I can then reopen this PR for discussion, but so far it seems unlikely. 👍