Skip to content

FrontendClient should throw if malformed customerId, shopId, language or secret are passed #31

@ThisIsJustARandomGuy

Description

@ThisIsJustARandomGuy

https://github.com/qenta-cee/checkout-client-library/blob/c239fc09eeb2984cf6810a26677ded666cc5acab/src/QentaCEE/QPay/FrontendClient.php#L284

Consider the following example (observe the spaces around the secret):

// init.php
$clientId = "D32123";
$shopId = "...";
$secret = "      VERYSECRET     ";

// The constructor calls trim() on the secret here so it becomes "VERYSECRET"
$client = new FrontendClient([
        'CUSTOMER_ID' => $customerId,
        'SHOP_ID' => $shopId,
        'SECRET' => $secret,
        'LANGUAGE' => 'en',
    ]);

// Set up the client (skipped here)

// This works while it should *not*
$response = $client->initiate();

$response->hasFailed(); // This returns false but it should have failed because the $secret is wrong

// Redirect the customer. Pseudocode
redirect_to($response->getRedirectUrl());

The customer is now able to complete the payment.

// return.php
// The customer has paid and was now redirected back to the shop.
// So we get the request data
$requestData = $_REQUEST;

// And call the return factory
// Here, $secret is *not* modified using trim().
// So it stays "      VERYSECRET     " instead of "VERYSECRET" which was used to initialize the payment
$return = ReturnFactory::getInstance($requestData, $secret);

// Of course now the return cannot be validated:
$return->validate(); // This returns false even though the payment was a success!

So the customer is able to complete the payment, gets redirected back to the shop and then gets an error message.

Either ReturnFactory::getInstance($return, $secret) should also call trim() on the $secret or the FrontendClient should fail to initialize if faulty login data is passed to it (my preferred solution). Currently, the payment is initialized and the user can pay but then the validation back at the shop fails. In my opinion the payment init process should fail instead of silently modifying the provided login data and later failing when the payment is already complete. What's worse is that this applies to handling of both, the normal returns as well as IPNs.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions