Signature verify of response is incorrect when refunding in API 1.9

[lucasfecko]

Hey,

after upgrading to 1.9 we have noticed exceptions after our financial dept. started refunding customers. I have decided to report it as issue with potential fix for you to consider, example and how to reproduce the issue:

Let's say we have a transaction with this pay_id and we want to refund it:

$conf = new Configuration();
$client = $conf->provideClient('cz');
$client->paymentRefund('29f9ada86e6b@GB', false, '999999999');

the transaction is made up so the refund should fail, but it instead throws exception:

Result signature is incorrect. Please make sure that bank's public key in file specified in config is correct and up-to-date.  

The API in version 1.9 started returning some elements in response as null and the library considers them in case of signature verification, this is the trace log (I have replaced some sensitive stuff with ...):

2022-10-21 14:14:10  Unknown IP      Will send request to method payment/refund
2022-10-21 14:14:10  Unknown IP      URL to send request to: https://api.platebnibrana.csob.cz/api/v1.9/payment/refund
2022-10-21 14:14:10  Unknown IP      JSON payload: {"merchantId":"...","payId":"29f9ada86e6b@GB","dttm":"20221021141410","amount":"999999999","signature":"..."}
2022-10-21 14:14:10  Unknown IP      API response: {"dttm":"20221021141410","signature":"...","merchantId":null,"payId":"29f9ada86e6b@GB","resultCode":140,"resultMessage":"Payment not found","paymentStatus":null,"authCode":null,"statusDetail":null,"actions":null}
2022-10-21 14:14:10  Unknown IP      String for verifying signature: "29f9ada86e6b@GB|20221021141410|140|Payment not found||||", using key /.../csob/mips_platebnibrana.csob.cz.pub
2022-10-21 14:14:10  Unknown IP      Failed: signature is incorrect.

As we can see the library uses this for signature verify: 29f9ada86e6b@GB|20221021141410|140|Payment not found|||| and we see |||| at the end because the API responded with paymentStatus and rest as null so the signature is joined with nothing there.

If we add this to Client.php:

function verifyResponseSignature($response, $signature, $responseFieldsOrder = array()) {

        $responseWithoutSignature = $response;
        if (isset($responseWithoutSignature["signature"])) {
            unset($responseWithoutSignature["signature"]);
        }
        // new addition, remove null content from array
        if($this->getConfig()->queryApiVersion('1.9')) {
           $responseWithoutSignature = array_filter($responseWithoutSignature);
        }
        .......

The signature verification now works since null array members are removed with array_filter.

I can submit a pull request and edit also csob-client.php if you consider this a proper fix, and if not at least I let you know.

[ondrakoupil]

Hi, thanks for reporting. This should be fixed in 2a82118, can you give it a try?

[lucasfecko]

Hey, thanks, I can now confirm the issue is fixed with that commit, thank you very much!

[janbrasna]

Are the nulls actually returned from the API, or the client invents them in place for "mandatory" keys (esp. on error responses)?

I believe there should be no literal nulls returned in any eAPI JSON response — if they are, a bug should be filed at @csob…

[ondrakoupil]

@janbrasna Yes, they are in the actual response from the bank's API, but only in payment/refund method. At least in situation when the payment is not in correct refundable state and API returns an error with code 150...

{
    "dttm": "20221118091325",
    "signature": "EO/fwUico6dVL/fWENM2UD7gTtjmgtlBwYx6/F4QjyS3nb9W1NUrnqdBKEHmOhCI2y/pySNEOQKMlCb/M46phflwSWGI7mmpSiXQBx283oz7gznlbjK3VJkkUk/Zshzj+2dYbPC/+hI3ulSCZKFczFToUwHa6PpneF8UcSGcOSYXTKvf5U88Lj/ARYYxWHcFdmyI06xnBtdMEOv+h4O1144FcTjh7zl2/9Nz+w4fvGJ0ScYZf0Lnowx1NVM/O7Th2xcAamAtMfD+FUmSoVzKaaDuo2qz3k68djNUn9AYD3JZzIsvqs2IKKPW1f7ordbBWJ0TOFju69EgPPALp1aCxA==",
    "merchantId": null,
    "payId": "9ad3704eb06f@HK",
    "resultCode": 150,
    "resultMessage": "Payment not in valid state",
    "paymentStatus": 4,
    "authCode": null,
    "statusDetail": null,
    "actions": null
}

Correct string base for response signature validation that validates is this one, with nulls completely ignored.

9ad3704eb06f@HK|20221118091443|150|Payment not in valid state|7

In other methods similar (payment/reverse, payment/confirm etc), there are no nulls. It really seems as a bug...

[janbrasna]

@ondrakoupil Aight if that's the raw response then yes: it's a @csob bug. (There are undoubtedly some status fragments in the payload from other operations that don't belong there — I'm reporting it internally.)

As a rule of thumb there are no nulls anywhere in the API, and if you encounter them, you have to treat them such as the key was never even present in the variables for digest… (and that's why they are not generally accepted in requests, and should not be returned in the responses… because they mean "variable not present", thus don't include in digest for signature… and that's super cumbersome to specify… so, yes, they shouldn't be there. Thanks for debugging this!)