CredentialService: Separate persistance and crypto

[rabbl]

The credentialService has two main functionalities at the moment which are persistance and crypto. For a better unit-testing experience I propose to separate this functionalities to a crypto-service and a persistance-service, connected by a factory or something similar.

The other way could be that the persistance execution moves to the controller and the persistance query moves to en entity-Repository. Create an entity (with properties for encrypted and plain-information) would be a nice improvement too.

[MichaelThessel]

Separating the service into 2 sounds good to me. Definitely more testable. As for moving business logic into the controller not so much. I would like to keep the controllers as lean as possible.

[rabbl]

Proposal: maintain the CredentialService with the following changes

• add credentialEntity (https://github.com/MichaelThessel/pwx/issues/32)
• as doctrine does the persistence of the Entity, there is no need to add an extra persistence class
• add a cryptoService-Class which pipes credentialObject for encrypting and decrypting

[MichaelThessel]

Sounds good to me. Feel free to implement it.

[rabbl]

I started separating a AES-Factory and a RSA-Service including first tests. Here: https://github.com/MichaelThessel/pwx/commit/ed7d7049560ab866d94784d2210c44ee39d3f10f Here: https://github.com/MichaelThessel/pwx/commit/d307c15006991e38a9db4b51475782c0f0f085cd

What should happen in the else cases here or here

[MichaelThessel]

I think you are doing the mistake of mixing the encryption service with the credential service. Ideally the encryption service would know nothing about the credentials. I think if we go for Doctrine ORM we don't need the credential service anymore. We would use the CredentialRepository to retrieve entities. The entity itself would have preUpdate and postLoad event handlers that handle the encryption. If we then add a CredentialFactory to create new Credential entities we should have used all the basic patterns :) and implemented a nice level of abstraction. Hope that makes sense. Let me know if not.

[rabbl]

Sounds nice. Some days ago I asked about doing persistence in the controller. What I have meant was exactly that. Using Doctrine ORM in the controller to persist and delete the credentials. Using the preUpdate and postLoad-events sounds good. So what is the best approach, injecting a dependency in the Entity or make it by use-statements and instantiate the class in the entity?

Nevertheless i finished my idea and have with https://github.com/rabbl/pwx/commit/2bf66d4ee60f3aba1e7ee514843c410d54055dca a version with separation of services that does have the same behavior like the old one. So i think a first step is done having smaller controller and unittests. It could be the basis of your thoughts.

Let me know what do you think.

[MichaelThessel]

I think injecting the encryption service into the entity might be best. I will have a look at the pull request in soon.