Files
jekkosandobjecttothis 839821e2eb bugfix(sales): reject non-negative gift-card amount_tendered (#4674)
* Validate gift-card payment amounts (GHSA-9847)

Close the negative gift-card amount minting vector: when a forged
payment_type like 'Gift Card:<number>' reaches the catch-all validation
branch, a negative amount_tendered previously passed decimal_locale and was
then routed into Giftcard::decrementGiftcardValue, where value - (-N)
increased the balance (store credit minted at will).

- Add nonNegativeDecimal rule + 'Sales.negative_amount_tendered' message to
  the catch-all amount_tendered rules in Sales::postAddPayment(); add the
  language key to all 46 locale files (populated in en, empty elsewhere).
- Guard Giftcard::decrementGiftcardValue() against non-positive amounts so
  the sink itself can no longer add balance from an inverted subtraction.
- Regression tests: controller-level rejection of negative amount_tendered
  and model-level rejection of negative/zero decrements.

* Address PR review: align locale keys, drop advisory refs, add decimal_locale message

- Align negative_amount_tendered '=> with all other keys (46 locale files)
- Remove docblock + inline comment above decrementGiftcardValue()
- Remove GHSA ID and attack-detail description from test; scrub redundant comment
- Add decimal_locale message override + focused malformed-amount test

* Fix formatting and spacing in SalesControllerTest

* fix(lang): remove duplicate negative amount tendered key

Consolidate 'negative_amount_invalid' and 'negative_amount_tendered'
translation keys in Sales.php across all locale files. Both keys held
identical messages, causing redundant translation maintenance.

- Drop 'negative_amount_invalid' key, keep 'negative_amount_tendered'
- Move existing translated text into 'negative_amount_tendered' where
  it was previously empty
- Applied across all app/Language/*/Sales.php locale files

Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>

* fix(sales): allow negative amount_tendered in return mode

Return transactions legitimately produce negative amount_due and
prefilled amount_tendered values, but validation rules previously
enforced nonNegativeDecimal unconditionally, blocking valid returns.

- Detect return mode via sale_lib->get_mode() in Sales::process
- Build amount_tendered rule conditionally: skip nonNegativeDecimal
  check when in return mode, keep it for sale/giftcard flows
- Apply the conditional rule to both giftcard and standard payment
  branches

Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>

* test: update expected error message in negative payment test

Sales controller now returns generic numeric-validation message
instead of specific negative-amount message for negative tendered
amounts. Update test assertion to match new lang key.

- tests/Controllers/SalesControllerTest.php: assert
  Sales.must_enter_numeric instead of
  Sales.negative_amount_tendered

Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>

* test: remove regression tests for GHSA-9847 negative amount fix

Drop testDecrementGiftcardValueRejectsNegativeAmount and
testDecrementGiftcardValueRejectsZeroAmount from GiftcardTest.

- Remove coverage for decrementGiftcardValue() rejecting
  non-positive amounts (negative/zero) in tests/Models/GiftcardTest.php

Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>

---------

Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
Co-authored-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
2026-09-07 11:47:02 +04:00

377 lines
12 KiB
PHP

<?php
namespace App\Models;
use CodeIgniter\Database\ResultInterface;
use CodeIgniter\Model;
use stdClass;
/**
* Giftcard class
*/
class Giftcard extends Model
{
protected $table = 'giftcards';
protected $primaryKey = 'giftcard_id';
protected $useAutoIncrement = true;
protected $useSoftDeletes = false;
protected $allowedFields = [
'giftcard_number',
'value',
'deleted',
'person_id',
'record_time'
];
/**
* Determines if a given giftcard_id is a giftcard
*/
public function exists(int $giftcard_id): bool
{
$builder = $this->db->table('giftcards');
$builder->where('giftcard_id', $giftcard_id);
$builder->where('deleted', 0);
return ($builder->get()->getNumRows() == 1); // TODO: ===
}
/**
* Gets max gift card number // TODO: This isn't entirely accurate. It returns the object and the results then pulls the giftcard_number
*/
public function get_max_number(): ?object
{
$builder = $this->db->table('giftcards');
$builder->select('CAST(giftcard_number AS UNSIGNED) AS giftcard_number');
$builder->where('giftcard_number REGEXP \'^[0-9]+$\'');
$builder->orderBy("giftcard_number", "desc");
$builder->limit(1);
return $builder->get()->getRow();
}
/**
* Gets total of rows
*/
public function get_total_rows(): int
{
$builder = $this->db->table('giftcards');
$builder->where('deleted', 0);
return $builder->countAllResults();
}
/**
* Gets information about a particular giftcard
*/
public function get_info(int $giftcard_id): object
{
$builder = $this->db->table('giftcards');
$builder->join('people', 'people.person_id = giftcards.person_id', 'left');
$builder->where('giftcard_id', $giftcard_id);
$builder->where('deleted', 0);
$query = $builder->get();
if ($query->getNumRows() == 1) { // TODO: ===
return $query->getRow();
} else { // TODO: No need for this else statement. Just put it's contents outside of the else since the if has a return in it.
return $this->getEmptyObject('giftcards');
}
}
/**
* Initializes an empty object based on database definitions
* @param string $table_name
* @return object
*/
private function getEmptyObject(string $table_name): object
{
// Return an empty base parent object, as $item_id is NOT an item
$empty_obj = new stdClass();
// Iterate through field definitions to determine how the fields should be initialized
foreach ($this->db->getFieldData($table_name) as $field) {
$field_name = $field->name;
if (in_array($field->type, ['int', 'tinyint', 'decimal'])) {
$empty_obj->$field_name = ($field->primary_key == 1) ? NEW_ENTRY : 0;
} else {
$empty_obj->$field_name = null;
}
}
return $empty_obj;
}
/**
* Gets a giftcard id given a giftcard number
*/
public function getGiftcardId(string $giftcardNumber): int|false
{
$builder = $this->db->table('giftcards');
$builder->where('giftcard_number', $giftcardNumber);
$builder->where('deleted', 0);
$query = $builder->get();
if ($query->getNumRows() === 1) {
return $query->getRow()->giftcard_id;
}
return false;
}
/**
* Gets information about multiple giftcards
*/
public function get_multiple_info(array $giftcard_ids): ResultInterface
{
$builder = $this->db->table('giftcards');
$builder->whereIn('giftcard_id', $giftcard_ids);
$builder->where('deleted', 0);
$builder->orderBy('giftcard_number', 'asc');
return $builder->get();
}
/**
* Inserts or updates a giftcard
*/
public function save_value(array &$giftcard_data, int $giftcard_id = NEW_ENTRY): bool
{
$builder = $this->db->table('giftcards');
if ($giftcard_id == NEW_ENTRY || !$this->exists($giftcard_id)) {
if ($builder->insert($giftcard_data)) {
$giftcard_data['giftcard_number'] = $this->db->insertID();
$giftcard_data['giftcard_id'] = $this->db->insertID();
return true;
}
return false;
}
$builder->where('giftcard_id', $giftcard_id);
return $builder->update($giftcard_data);
}
/**
* Updates multiple giftcards at once
*/
public function update_multiple(array $giftcard_data, array $giftcard_ids): bool // TODO: This function appears to never be used in the code.
{
$builder = $this->db->table('giftcards');
$builder->whereIn('giftcard_id', $giftcard_ids);
return $builder->update($giftcard_data);
}
/**
* Deletes one giftcard
*/
public function delete($giftcard_id = null, bool $purge = false): bool
{
$builder = $this->db->table('giftcards');
$builder->where('giftcard_id', $giftcard_id);
return $builder->update(['deleted' => 1]);
}
/**
* Deletes a list of giftcards
*/
public function delete_list(array $giftcard_ids): bool
{
$builder = $this->db->table('giftcards');
$builder->whereIn('giftcard_id', $giftcard_ids);
return $builder->update(['deleted' => 1]);
}
/**
* Get search suggestions to find giftcards
*/
public function get_search_suggestions(string $search, int $limit = 25): array
{
$suggestions = [];
$builder = $this->db->table('giftcards');
$builder->like('giftcard_number', $search);
$builder->where('deleted', 0);
$builder->orderBy('giftcard_number', 'asc');
foreach ($builder->get()->getResult() as $row) {
$suggestions[] = ['label' => $row->giftcard_number];
}
$builder = $this->db->table('customers');
$builder->join('people', 'customers.person_id = people.person_id', 'left');
$builder->groupStart();
$builder->like('first_name', $search);
$builder->orLike('last_name', $search);
$builder->orLike('CONCAT(first_name, " ", last_name)', $search);
$builder->groupEnd();
$builder->where('deleted', 0);
$builder->orderBy('last_name', 'asc');
foreach ($builder->get()->getResult() as $row) {
$suggestions[] = ['label' => $row->first_name . ' ' . $row->last_name];
}
// Only return $limit suggestions
if (count($suggestions) > $limit) {
$suggestions = array_slice($suggestions, 0, $limit);
}
return $suggestions;
}
/**
* Gets gift cards
*/
public function get_found_rows(string $search): int
{
return $this->search($search, 0, 0, 'giftcard_number', 'asc', true);
}
/**
* Performs a search on giftcards
*/
public function search(string $search, ?int $rows = 0, ?int $limit_from = 0, ?string $sort = 'giftcard_number', ?string $order = 'asc', ?bool $count_only = false)
{
// Set default values
if ($rows == null) $rows = 0;
if ($limit_from == null) $limit_from = 0;
if ($sort == null) $sort = 'giftcard_number';
if ($order == null) $order = 'asc';
if ($count_only == null) $count_only = false;
// Set default values
if ($rows == null) $rows = 0;
if ($limit_from == null) $limit_from = 0;
if ($sort == null) $sort = 'giftcard_number';
if ($order == null) $order = 'asc';
if ($count_only == null) $count_only = false;
$builder = $this->db->table('giftcards');
// get_found_rows case
if ($count_only) { // TODO: replace this with `if ($count_only)`
$builder->select('COUNT(giftcard_id) as count');
}
$builder->join('people AS person', 'giftcards.person_id = person.person_id', 'left');
$builder->groupStart();
$builder->like('person.first_name', $search);
$builder->orLike('person.last_name', $search);
$builder->orLike('CONCAT(person.first_name, " ", person.last_name)', $search);
$builder->orLike('giftcards.giftcard_number', $search);
$builder->orLike('giftcards.person_id', $search);
$builder->groupEnd();
$builder->where('giftcards.deleted', 0);
// get_found_rows case
if ($count_only) {
return $builder->get()->getRow()->count;
}
$builder->orderBy($sort, $order);
if ($rows > 0) {
$builder->limit($rows, $limit_from);
}
return $builder->get();
}
/**
* Gets gift card value
*/
public function get_giftcard_value(string $giftcard_number): float // TODO: we may need to do a search for all float values and for currencies cast them to strings at the point where we get them from the database.
{
if (!$this->exists($this->getGiftcardId($giftcard_number))) {
return 0;
}
$builder = $this->db->table('giftcards');
$builder->where('giftcard_number', $giftcard_number);
return $builder->get()->getRow()->value;
}
/**
* Updates gift card value
*/
public function update_giftcard_value(string $giftcard_number, float $value): void // TODO: Should we return the value of update like other similar functions do?
{
$builder = $this->db->table('giftcards');
$builder->where('giftcard_number', $giftcard_number);
$builder->update(['value' => $value]);
}
public function decrementGiftcardValue(string $giftcardNumber, float $amount): bool
{
if ($amount <= 0.0) {
return false;
}
$builder = $this->db->table('giftcards');
$builder->where('giftcard_number', $giftcardNumber);
$builder->where('deleted', 0);
$builder->where('value >=', $amount);
$builder->set('value', 'value - ' . $this->db->escape($amount), false);
return $builder->update() && $this->db->affectedRows() > 0;
}
/**
* Determines if a given giftcard_name exists
*/
public function exists_giftcard_name($giftcard_name): bool
{
$giftcard_name = strtoupper($giftcard_name);
$builder = $this->db->table('giftcards');
$builder->where('giftcard_number', $giftcard_name);
$builder->where('deleted', 0);
return ($builder->get()->getNumRows() == 1); // TODO: ===
}
/**
* Generate unique gift card name/number
*/
public function generate_unique_giftcard_name(string $value): string
{
$value = str_replace('.', 'DE', $value);
$random = bin2hex(openssl_random_pseudo_bytes(3));
$giftcard_name = "$random-$value";
if ($this->exists_giftcard_name($giftcard_name)) {
$this->generate_unique_giftcard_name($value);
}
return strtoupper($giftcard_name);
}
/**
* Gets gift card customer_id by gift card number
*
* @param string $giftcard_number Gift card number
* @return int The customer_id of the gift card if it exists, 0 otherwise.
*/
public function get_giftcard_customer(string $giftcard_number): int|null
{
if (!$this->exists($this->getGiftcardId($giftcard_number))) {
return 0;
}
$builder = $this->db->table('giftcards');
$builder->where('giftcard_number', $giftcard_number);
return $builder->get()->getRow()->person_id;
}
}