From 003bcd60fa4358c80dd05c281e42b4b043e1f5f5 Mon Sep 17 00:00:00 2001 From: Laurent Destailleur Date: Fri, 8 Apr 2022 19:52:40 +0200 Subject: [PATCH] FIX Add protection on bad input of payment on multicurrency invoices --- htdocs/compta/paiement.php | 9 +++- .../compta/paiement/class/paiement.class.php | 46 +++++++++++++++++-- htdocs/langs/en_US/errors.lang | 8 +--- .../class/multicurrency.class.php | 18 ++++---- 4 files changed, 60 insertions(+), 21 deletions(-) diff --git a/htdocs/compta/paiement.php b/htdocs/compta/paiement.php index df6e32f5cf8..774495091a5 100644 --- a/htdocs/compta/paiement.php +++ b/htdocs/compta/paiement.php @@ -190,6 +190,7 @@ if (empty($reshook)) { // Check if payments in both currency if ($totalpayment > 0 && $multicurrency_totalpayment > 0) { + $langs->load("errors"); setEventMessages($langs->transnoentities('ErrorPaymentInBothCurrency'), null, 'errors'); $error++; } @@ -220,6 +221,8 @@ if (empty($reshook)) { $thirdparty->fetch($socid); } + $multicurrency_code = array(); + // Clean parameters amount if payment is for a credit note foreach ($amounts as $key => $value) { // How payment is dispatched $tmpinvoice = new Facture($db); @@ -228,6 +231,7 @@ if (empty($reshook)) { $newvalue = price2num($value, 'MT'); $amounts[$key] = - abs($newvalue); } + $multicurrency_code[$key] = $tmpinvoice->multicurrency_code; } foreach ($multicurrency_amounts as $key => $value) { // How payment is dispatched @@ -237,6 +241,7 @@ if (empty($reshook)) { $newvalue = price2num($value, 'MT'); $multicurrency_amounts[$key] = - abs($newvalue); } + $multicurrency_code[$key] = $tmpinvoice->multicurrency_code; } if (!empty($conf->banque->enabled)) { @@ -252,9 +257,11 @@ if (empty($reshook)) { $paiement->datepaye = $datepaye; $paiement->amounts = $amounts; // Array with all payments dispatching with invoice id $paiement->multicurrency_amounts = $multicurrency_amounts; // Array with all payments dispatching + $paiement->multicurrency_code = $multicurrency_code; // Array with all currency of payments dispatching $paiement->paiementid = dol_getIdFromCode($db, GETPOST('paiementcode'), 'c_paiement', 'code', 'id', 1); $paiement->num_payment = GETPOST('num_paiement', 'alpha'); $paiement->note_private = GETPOST('comment', 'alpha'); + $paiement->fk_account = GETPOST('accountid', 'int'); if (!$error) { // Create payment and update this->multicurrency_amounts if this->amounts filled or @@ -271,7 +278,7 @@ if (empty($reshook)) { if (GETPOST('type') == Facture::TYPE_CREDIT_NOTE) { $label = '(CustomerInvoicePaymentBack)'; // Refund of a credit note } - $result = $paiement->addPaymentToBank($user, 'payment', $label, GETPOST('accountid'), GETPOST('chqemetteur'), GETPOST('chqbank')); + $result = $paiement->addPaymentToBank($user, 'payment', $label, GETPOST('accountid', 'int'), GETPOST('chqemetteur'), GETPOST('chqbank')); if ($result < 0) { setEventMessages($paiement->error, $paiement->errors, 'errors'); $error++; diff --git a/htdocs/compta/paiement/class/paiement.class.php b/htdocs/compta/paiement/class/paiement.class.php index 32edc0629bd..07213fe0ce0 100644 --- a/htdocs/compta/paiement/class/paiement.class.php +++ b/htdocs/compta/paiement/class/paiement.class.php @@ -72,8 +72,9 @@ class Paiement extends CommonObject public $amount; // Total amount of payment (in the main currency) public $multicurrency_amount; // Total amount of payment (in the currency of the bank account) - public $amounts = array(); // array: invoice ID => amount for that invoice (in the main currency)> - public $multicurrency_amounts = array(); // array: invoice ID => amount for that invoice (in the invoice's currency)> + public $amounts = array(); // array: invoice ID => amount for that invoice (in the main currency) + public $multicurrency_amounts = array(); // array: invoice ID => amount for that invoice (in the invoice's currency) + public $multicurrency_code = array(); // array: invoice ID => currency code for that invoice public $pos_change = 0; // Excess received in TakePOS cash payment @@ -230,7 +231,7 @@ class Paiement extends CommonObject global $conf, $langs; $error = 0; - $way = $this->getWay(); + $way = $this->getWay(); // 'dolibarr' to use amount, 'customer' to use foreign multicurrency amount $now = dol_now(); @@ -239,16 +240,37 @@ class Paiement extends CommonObject $totalamount_converted = 0; $atleastonepaymentnotnull = 0; - if ($way == 'dolibarr') { + if ($way == 'dolibarr') { // Payments were entered into the column of main currency $amounts = &$this->amounts; $amounts_to_update = &$this->multicurrency_amounts; - } else { + } else { // Payments were entered into the column of foreign currency $amounts = &$this->multicurrency_amounts; $amounts_to_update = &$this->amounts; } + $currencyofpayment = ''; + foreach ($amounts as $key => $value) { // How payment is dispatch + if (empty($value)) { + continue; + } + // $key is id of invoice, $value is amount, $way is a 'dolibarr' if amount is in main currency, 'customer' if in foreign currency $value_converted = Multicurrency::getAmountConversionFromInvoiceRate($key, $value, $way); + // Add controls of input validity + if ($value_converted === false) { + // We failed to find the conversion for one invoice + $this->error = 'FailedToFoundTheConversionRateForInvoice'; + return -1; + } + if (empty($currencyofpayment)) { + $currencyofpayment = $this->multicurrency_code[$key]; + } + if ($currencyofpayment != $this->multicurrency_code[$key]) { + // If we have invoices with different currencies in the payment, we stop here + $this->error = 'ErrorYouTryToPayInvoicesWithDifferentCurrenciesInSamePayment'; + return -1; + } + $totalamount_converted += $value_converted; $amounts_to_update[$key] = price2num($value_converted, 'MT'); @@ -260,6 +282,19 @@ class Paiement extends CommonObject } } + if (!empty($currencyofpayment)) { + // We must check that the currency of invoices is the same than the currency of the bank + $bankaccount = new Account($this->db); + $bankaccount->fetch($this->fk_account); + $bankcurrencycode = empty($bankaccount->currency_code) ? $conf->currency : $bankaccount->currency_code; + if ($currencyofpayment != $bankcurrencycode && $currencyofpayment != $conf->currency && $bankcurrencycode != $conf->currency) { + $langs->load("errors"); + $this->error = $langs->trans('ErrorYouTryToPayInvoicesInACurrencyFromBankWithAnotherCurrency', $currencyofpayment, $bankcurrencycode); + return -1; + } + } + + $totalamount = price2num($totalamount); $totalamount_converted = price2num($totalamount_converted); @@ -305,6 +340,7 @@ class Paiement extends CommonObject if (is_numeric($amount) && $amount <> 0) { $amount = price2num($amount); $sql = "INSERT INTO ".MAIN_DB_PREFIX."paiement_facture (fk_facture, fk_paiement, amount, multicurrency_amount)"; + // TODO Add multicurrency_code and multicurrency_tx $sql .= " VALUES (".((int) $facid).", ".((int) $this->id).", ".((float) $amount).", ".((float) $this->multicurrency_amounts[$key]).")"; dol_syslog(get_class($this).'::create Amount line '.$key.' insert paiement_facture', LOG_DEBUG); diff --git a/htdocs/langs/en_US/errors.lang b/htdocs/langs/en_US/errors.lang index c117355cd44..3a7b4abeff7 100644 --- a/htdocs/langs/en_US/errors.lang +++ b/htdocs/langs/en_US/errors.lang @@ -281,6 +281,8 @@ ErrorRequestTooLarge=Error, request too large ErrorNotApproverForHoliday=You are not the approver for leave %s ErrorAttributeIsUsedIntoProduct=This attribute is used in one or more product variants ErrorAttributeValueIsUsedIntoProduct=This attribute value is used in one or more product variants +ErrorPaymentInBothCurrency=Error, all amounts must be entered in the same column +ErrorYouTryToPayInvoicesInACurrencyFromBankWithAnotherCurrency=You try to pay invoices in the currency %s from an account with the currency %s # Warnings WarningParamUploadMaxFileSizeHigherThanPostMaxSize=Your PHP parameter upload_max_filesize (%s) is higher than PHP parameter post_max_size (%s). This is not a consistent setup. @@ -314,10 +316,8 @@ WarningTheHiddenOptionIsOn=Warning, the hidden option %s is on. WarningCreateSubAccounts=Warning, you can't create directly a sub account, you must create a third party or an user and assign them an accounting code to find them in this list WarningAvailableOnlyForHTTPSServers=Available only if using HTTPS secured connection. WarningModuleXDisabledSoYouMayMissEventHere=Module %s has not been enabled. So you may miss a lot of event here. -<<<<<<< HEAD WarningPaypalPaymentNotCompatibleWithStrict=The value 'Strict' makes the online payment features not working correctly. Use 'Lax' instead. -<<<<<<< HEAD # Validate RequireValidValue = Value not valid RequireAtLeastXString = Requires at least %s character(s) @@ -338,7 +338,3 @@ BadSetupOfField = Error bad setup of field BadSetupOfFieldClassNotFoundForValidation = Error bad setup of field : Class not found for validation BadSetupOfFieldFileNotFound = Error bad setup of field : File not found for inclusion BadSetupOfFieldFetchNotCallable = Error bad setup of field : Fetch not callable on class -======= -======= ->>>>>>> branch '13.0' of git@github.com:Dolibarr/dolibarr.git ->>>>>>> branch '14.0' of git@github.com:Dolibarr/dolibarr.git diff --git a/htdocs/multicurrency/class/multicurrency.class.php b/htdocs/multicurrency/class/multicurrency.class.php index 1ebf33d3925..8b42f7cb25b 100644 --- a/htdocs/multicurrency/class/multicurrency.class.php +++ b/htdocs/multicurrency/class/multicurrency.class.php @@ -559,11 +559,11 @@ class MultiCurrency extends CommonObject /** * Get the conversion of amount with invoice rate * - * @param int $fk_facture id of facture - * @param double $amount amount to convert - * @param string $way 'dolibarr' mean the amount is in dolibarr currency - * @param string $table facture or facture_fourn - * @return double amount converted + * @param int $fk_facture id of facture + * @param double $amount amount to convert + * @param string $way 'dolibarr' mean the amount is in dolibarr currency + * @param string $table facture or facture_fourn + * @return double|boolean amount converted or false if conversion fails */ public static function getAmountConversionFromInvoiceRate($fk_facture, $amount, $way = 'dolibarr', $table = 'facture') { @@ -576,16 +576,16 @@ class MultiCurrency extends CommonObject return price2num($amount / $multicurrency_tx, 'MU'); } } else { - return $amount; + return false; } } /** * Get current invoite rate * - * @param int $fk_facture id of facture - * @param string $table facture or facture_fourn - * @return bool + * @param int $fk_facture id of facture + * @param string $table facture or facture_fourn + * @return float|bool Rate of currency or false if error */ public static function getInvoiceRate($fk_facture, $table = 'facture') {