checkdomain / comodo Goto Github PK

Sectigo, who bought Comodo, told us that we should start using the !AutoApplyOrder end point in place of the !AutoApplySSL endpoint.

Info on the !AutoApplyOrder endpoint can be found at https://sectigo.com/partner/pci/resources/Sectigo-AutoApplyOrder-v2.2.1.pdf .

Interestingly, it appears to be 100% backwards compatible to the !AutoApplySSL endpoint. Said by the linked doc:

AutoApplyOrder is fully backward-compatible with AutoApplySSL, accepting all the same parameters and able to request all the same certificates.
Changing to AutoApplyOrder is a simple case of changing the API endpoint URL – all the parameters, values and authentication remain the same, and you’ll now have
the ability to request a range of new Sectigo products with simple, minor changes to the API call.

Maybe it doesn't even matter...I'm not sure. As long as they keep !AutoApplySSL in place I guess it's all good (unless a need arises to do something that !AutoApplySSL can't do).

Do you have any thoughts?

in .gitignore, line 1

.idea is user-specific and should not appear in a project .gitignore. Consider adding it to the user global .gitignore instead.

.idea
composer.phar

vendor/
report/

Posted from SensioLabsInsight

When the autoReplaceSSL method processes the response from the API, it assumes certificateID is present in that response. According to the docs for the !AutoReplaceSSL end point, however, this property is only present when the request sends a param called showCertificateID with a value of Y.

Without that parameter present, the library causes an Undefined Index warning. In Laravel, these warnings are set up to throw exceptions. So although it's only a warning, it is problematic in a large set of environments.

This API wrapper should either check that showCertificateID was set and equal to Y in the request before attempting to access certificateID in the response, OR it should force showCertificateID into the request with a value of Y so that the response property is always present.

My workaround for this right now is to manually add showCertificateID to the params I give to autoReplaceSSL.

The doc-block for the Util::__construct method implies that the $imapAdapter and $imapHelper params are optional and could be nulled or left out:

     * @param ImapAdapter|null          $imapAdapter
     * @param ImapHelper|null           $imapHelper

Attempting to instanciate Util without those params, however, throws an exception because they aren't defaulted to null in the function defintion:

public function __construct(CommunicationAdapter $communicationAdapter, ImapAdapter $imapAdapter, ImapHelper $imapHelper)

Is this something that could be changed? Like this:

public function __construct(CommunicationAdapter $communicationAdapter, ImapAdapter $imapAdapter = null, ImapHelper $imapHelper = null)

I can easily provide instances of the two objects:

new Comodo\Util(
    new Comodo\CommunicationAdapter(
        new Comodo\Model\Account(
            config('api.user'),
            config('api.password')
        )
    ),
    /*
        * We don't make use of the following two params, but the
        * library forgot to make them optional (even though the doc
        * block for the constructor makes it seem so)
        */
    new Comodo\ImapAdapter,
    new Comodo\ImapHelper
);

...but they're useless in my situation.

Thanks for considering.
jim

The readme mentions this repository is open source, but you don't have a license, so normal copyright applies: https://help.github.com/articles/open-source-licensing/

This project's packagist listing is not auto updated so the latest release, 2.4.0, cannot be installed via composer.