Problem/Motivation
The existing module uses phpseclib to get some information about the keys. This function is the sole procedural function in the .module file.
Proposed resolution
I'd like to move the key_asymmetric_get_key_properties() function into a KeyPair service with the intention of expanding the service to handle other key management activities. Doing so will provide a strong API for such things that will allow us to remove the dependency on phpseclib.
Remaining tasks
API changes
Add a new KeyPair class with a getKeyProperties() method that is identical in function to the key_asymmetric_get_key_properties() function. Use the new service where the existing function was formerly called.
The update should include a wrapper function in the key_asymmetric.module file for backwards compatibility, marked
as deprecated.
Issue fork key_asymmetric-3489430
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
john franklin commented@roderik, can you take a minute to review this?
Comment #4
roderikThis looks good. One naming nit in MR.
At this point, if people do not want to be dependent on phpseclib, they need to overwrite the
key_asymmetric.key_pairservice with their own, and they need to extend the KeyPair class because there is no interface yet.Two separate areas of future extension:
1) so that other services don't need to extend KeyPair:
2) If we want this to be code-free, i..e people just install another similar service that uses another library, and it (usually) gets picked up automatically, we need to make the current KeyPair service just one of potentially many, and
applies(): boolmethod (our implementation being e.g.{return class_exists('phpseclib3\Crypt\PublicKeyLoader';}Those can be later extensions, when this actually gets implemented by another service / needs to be swappable. Nothing in this issue stands in the way of it.
Comment #5
roderikNeeds Work for at least the proposed class name change. If you don't agree, you can merge.
Comment #6
john franklin commentedThank you for reviewing the MR. I think #1 makes sense and I'll update the MR to go that route. #2 is more complicated than I was intending.
For clarity, my road map is:
step 1, refactor code into KeyPair class, no implementation changes (this MR)
step 2, change the class implementation of KeyPair to replace phpseclib with base PHP OpenSSL calls in a way that doesn't change the behavior of KeyPair. (near future MR, possible new major version)
step 3, add new functionality to KeyPair, such as key generation, again using base PHP OpenSSL calls. (future MR)
Comment #8
john franklin commentedComment #10
roderik> step 2, change the class implementation of KeyPair to replace phpseclib with base PHP OpenSSL calls
Ah. That makes a lot of sense (and cancels the need for all the things I just made up on the spot while reviewing).