- Issue created by @John Franklin
- Merge request !5Issue #3489430: Create a KeyPair class and update the code to use it. β (Open) created by John Franklin
- πΊπΈUnited States John Franklin
@roderik, can you take a minute to review this?
- π³π±Netherlands roderik Amsterdam,NL / Budapest,HU
This 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_pair
service 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:
- define an interface
- make KeyPair implement the interface
- mention the interface (not KeyPair) in the constructor method signatures
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
- give each service a
applies(): bool
method (our implementation being e.g.{return class_exists('phpseclib3\Crypt\PublicKeyLoader';}
- add an extra service (e.g. KeyPairManager) which is a
service collector β
and also implements
getKeyProperties() { foreach ($this->sortedKeypairProviders as $provider) { if ($provider->applies()) { return $provider->getKeyProperties(); } } return some-default; }
- tag the current KeyPair service to be such a provider
- use the new KeyPairManager whereever we now use KeyPair, in our code.
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.
- π³π±Netherlands roderik Amsterdam,NL / Budapest,HU
Needs Work for at least the proposed class name change. If you don't agree, you can merge.
- πΊπΈUnited States John Franklin
Thank 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)