Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
ThisIsJustARandomGuy opened this issue Oct 5, 2021 · 0 comments

Comments

@ThisIsJustARandomGuy
Copy link

$sCustomerId = isset( $this->oUserConfig->CUSTOMER_ID ) ? trim($this->oUserConfig->CUSTOMER_ID) : null;

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant