diff --git a/app/Controllers/Customers.php b/app/Controllers/Customers.php index 664800cab..df475efc2 100644 --- a/app/Controllers/Customers.php +++ b/app/Controllers/Customers.php @@ -360,7 +360,15 @@ class Customers extends Persons $consent = $data[3] == '' ? 0 : 1; if (sizeof($data) >= 16 && $consent) { - $email = strtolower($data[4]); + $email = filter_var(strtolower($data[4]), FILTER_SANITIZE_EMAIL); + + // Empty email is allowed, but if provided it must be valid + if ($email !== '' && !filter_var($email, FILTER_VALIDATE_EMAIL)) { + $failCodes[] = 'Row ' . $i . ': Invalid email format'; + $i++; + continue; + } + $personData = [ 'first_name' => $data[0], 'last_name' => $data[1], diff --git a/app/Database/Migrations/20210422000000_database_optimizations.php b/app/Database/Migrations/20210422000000_database_optimizations.php index 4c5c496ae..b8a63d449 100644 --- a/app/Database/Migrations/20210422000000_database_optimizations.php +++ b/app/Database/Migrations/20210422000000_database_optimizations.php @@ -18,12 +18,35 @@ class Migration_database_optimizations extends Migration { log_message('info', 'Migrating database optimizations.'); + helper('migration'); + + dropForeignKeyConstraints(['ospos_customers_ibfk_1'], 'customers'); + dropForeignKeyConstraints(['ospos_customers_points_ibfk_1'], 'customers_points'); + dropForeignKeyConstraints(['ospos_sales_ibfk_2'], 'sales'); + dropForeignKeyConstraints(['ospos_sales_payments_ibfk_2'], 'sales_payments'); + dropForeignKeyConstraints(['ospos_sales_ibfk_1'], 'sales'); + dropForeignKeyConstraints(['ospos_receivings_ibfk_1'], 'receivings'); + dropForeignKeyConstraints(['ospos_inventory_ibfk_2'], 'inventory'); + dropForeignKeyConstraints(['ospos_grants_ibfk_2'], 'grants'); + dropForeignKeyConstraints(['ospos_expenses_ibfk_2'], 'expenses'); + dropForeignKeyConstraints(['ospos_employees_ibfk_1'], 'employees'); + dropForeignKeyConstraints(['ospos_cash_up_ibfk_1'], 'cash_up'); + dropForeignKeyConstraints(['ospos_cash_up_ibfk_2'], 'cash_up'); + dropForeignKeyConstraints(['ospos_items_ibfk_1'], 'items'); + dropForeignKeyConstraints(['ospos_expenses_ibfk_3'], 'expenses'); + dropForeignKeyConstraints(['ospos_receivings_ibfk_2'], 'receivings'); + dropForeignKeyConstraints(['ospos_suppliers_ibfk_1'], 'suppliers'); + + createPrimaryKey('customers', 'person_id'); + createPrimaryKey('employees', 'person_id'); + createPrimaryKey('suppliers', 'person_id'); + $attribute = model(Attribute::class); $attribute->deleteOrphanedValues(); - $this->migrate_duplicate_attribute_values(DECIMAL); - $this->migrate_duplicate_attribute_values(DATE); + $this->migrateDuplicateAttributeValues(DECIMAL); + $this->migrateDuplicateAttributeValues(DATE); // Select all attributes that have data in more than one column $builder = $this->db->table('attribute_values'); @@ -36,51 +59,60 @@ class Migration_database_optimizations extends Migration $builder->where('attribute_value IS NOT NULL'); $builder->where('attribute_decimal IS NOT NULL'); $builder->groupEnd(); - $attribute_values = $builder->get(); + $attributeValues = $builder->get(); $this->db->transStart(); // Clean up Attribute values table where there is an attribute value and an attribute_date/attribute_decimal - foreach ($attribute_values->getResultArray() as $attribute_value) { + foreach ($attributeValues->getResultArray() as $attributeValue) { $builder = $this->db->table('attribute_values'); - $builder->delete(['attribute_id' => $attribute_value['attribute_id']]); + $builder->delete(['attribute_id' => $attributeValue['attribute_id']]); $builder = $this->db->table('attribute_links'); $builder->select('links.definition_id, links.item_id, links.attribute_id, defs.definition_type'); $builder->join('attribute_definitions defs', 'defs.definition_id = links.definition_id'); - $builder->where('attribute_id', $attribute_value['attribute_id']); - $attribute_links = $builder->get(); + $builder->where('attribute_id', $attributeValue['attribute_id']); + $attributeLinks = $builder->get(); - if ($attribute_links) { + if ($attributeLinks) { $builder = $this->db->table('attribute_links'); - $attribute_links = $attribute_links->getResultArray() ?: []; + $attributeLinks = $attributeLinks->getResultArray() ?: []; - foreach ($attribute_links->getResultArray() as $attribute_link) { - $builder->where('attribute_id', $attribute_link['attribute_id']); - $builder->where('item_id', $attribute_link['item_id']); + foreach ($attributeLinks as $attributeLink) { + $builder->where('attribute_id', $attributeLink['attribute_id']); + $builder->where('item_id', $attributeLink['item_id']); $builder->delete(); - switch ($attribute_link['definition_type']) { + switch ($attributeLink['definition_type']) { case DECIMAL: - $value = $attribute_value['attribute_decimal']; + $value = $attributeValue['attribute_decimal']; break; case DATE: $config = config(OSPOS::class)->settings; - $attribute_date = DateTime::createFromFormat('Y-m-d', $attribute_value['attribute_date']); - $value = $attribute_date->format($config['dateformat']); + $attributeDate = DateTime::createFromFormat('Y-m-d', (string) $attributeValue['attribute_date']); + + if ($attributeDate === false) { + log_message('warning', 'Migration 20210422000000: unparseable attribute_date "' . $attributeValue['attribute_date'] . '" for attribute_id ' . $attributeValue['attribute_id'] . ' — preserving raw value.'); + $value = (string) $attributeValue['attribute_date']; + } else { + $dateFormat = empty($config['dateformat']) ? 'Y-m-d' : $config['dateformat']; + if (empty($config['dateformat'])) { + log_message('warning', 'Migration 20210422000000: dateformat config empty, falling back to Y-m-d for attribute_id ' . $attributeValue['attribute_id'] . '.'); + } + $value = $attributeDate->format($dateFormat); + } break; default: - $value = $attribute_value['attribute_value']; + $value = $attributeValue['attribute_value']; break; } - $attribute->saveAttributeValue($value, $attribute_link['definition_id'], $attribute_link['item_id'], false, $attribute_link['definition_type']); + $attribute->saveAttributeValue($value, $attributeLink['definition_id'], $attributeLink['item_id'], false, $attributeLink['definition_type']); } } } $this->db->transComplete(); - helper('migration'); execute_script(APPPATH . 'Database/Migrations/sqlscripts/3.4.0_database_optimizations.sql'); log_message('info', 'Finished migrating database optimizations.'); } @@ -88,58 +120,57 @@ class Migration_database_optimizations extends Migration /** * Given the type of attribute, deletes any duplicates it finds in the attribute_values table and reassigns those */ - private function migrate_duplicate_attribute_values($attribute_type): void + private function migrateDuplicateAttributeValues(string $attributeType): void { // Remove duplicate attribute values needed to make attribute_decimals and attribute_dates unique $this->db->transStart(); - $column = 'attribute_' . strtolower($attribute_type); + $column = 'attribute_' . strtolower($attributeType); $builder = $this->db->table('attribute_values'); $builder->select("$column"); $builder->groupBy($column); $builder->having("COUNT($column) > 1"); - $duplicated_values = $builder->get(); + $duplicatedValues = $builder->get(); - foreach ($duplicated_values->getResultArray() as $duplicated_value) { - $subquery_builder = $this->db->table('attribute_values'); - $subquery_builder->select('attribute_id'); - $subquery_builder->where($column, $duplicated_value[$column]); - $subquery = $subquery_builder->getCompiledSelect(); + foreach ($duplicatedValues->getResultArray() as $duplicatedValue) { + $subqueryBuilder = $this->db->table('attribute_values'); + $subqueryBuilder->select('attribute_id'); + $subqueryBuilder->where($column, $duplicatedValue[$column]); + $subquery = $subqueryBuilder->getCompiledSelect(); $builder = $this->db->table('attribute_values'); $builder->select('attribute_id'); - $builder->where($column, $duplicated_value[$column]); + $builder->where($column, $duplicatedValue[$column]); $builder->where("attribute_id IN ($subquery)", null, false); - $attribute_ids_to_fix = $builder->get(); + $attributeIdsToFix = $builder->get(); - $this->reassign_duplicate_attribute_values($attribute_ids_to_fix, $duplicated_value); + $this->reassignDuplicateAttributeValues($attributeIdsToFix); } $this->db->transComplete(); } /** - * Updates the attribute_id in all attribute_link rows with duplicated attribute_ids then deletes unneeded rows from attribute_values + * Updates the attribute_id in all attribute_link rows with duplicated attributeIds then deletes unneeded rows from attributeValues * - * @param ResultInterface $attribute_ids_to_fix All attribute_ids that need to parsed - * @param array $attribute_value The attribute value in question. + * @param ResultInterface $attributeIdsToFix All attributeIds that need to parsed */ - private function reassign_duplicate_attribute_values(ResultInterface $attribute_ids_to_fix, array $attribute_value): void + private function reassignDuplicateAttributeValues(ResultInterface $attributeIdsToFix): void { - $attribute_ids = $attribute_ids_to_fix->getResultArray(); - $retain_attribute_id = $attribute_ids[0]['attribute_id']; + $attributeIds = $attributeIdsToFix->getResultArray(); + $retainAttributeId = $attributeIds[0]['attribute_id']; - foreach ($attribute_ids as $attribute_id) { + foreach ($attributeIds as $attributeId) { // Update attribute_link with the attribute_id we are keeping $builder = $this->db->table('attribute_links'); - $builder->where('attribute_id', $attribute_id['attribute_id']); - $builder->update(['attribute_id' => $retain_attribute_id]); + $builder->where('attribute_id', $attributeId['attribute_id']); + $builder->update(['attribute_id' => $retainAttributeId]); // Delete the row from attribute_values if it isn't our keeper - if ($attribute_id['attribute_id'] !== $retain_attribute_id) { + if ($attributeId['attribute_id'] !== $retainAttributeId) { $builder = $this->db->table('attribute_values'); - $builder->delete(['attribute_id' => $attribute_id['attribute_id']]); + $builder->delete(['attribute_id' => $attributeId['attribute_id']]); } } } diff --git a/app/Database/Migrations/sqlscripts/3.4.0_database_optimizations.sql b/app/Database/Migrations/sqlscripts/3.4.0_database_optimizations.sql index 50f702b95..86f3e189e 100644 --- a/app/Database/Migrations/sqlscripts/3.4.0_database_optimizations.sql +++ b/app/Database/Migrations/sqlscripts/3.4.0_database_optimizations.sql @@ -11,10 +11,6 @@ ALTER TABLE `ospos_attribute_definitions` ADD INDEX(`definition_type`); ALTER TABLE `ospos_cash_up` MODIFY `deleted` tinyint(1) DEFAULT 0 NOT NULL; #ospos_customers table -ALTER TABLE `ospos_customers` DROP FOREIGN KEY `ospos_customers_ibfk_1`; -ALTER TABLE `ospos_customers_points` DROP FOREIGN KEY `ospos_customers_points_ibfk_1`; -ALTER TABLE `ospos_sales` DROP FOREIGN KEY `ospos_sales_ibfk_2`; - ALTER TABLE `ospos_customers` MODIFY `taxable` tinyint(1) DEFAULT 1 NOT NULL; ALTER TABLE `ospos_customers` MODIFY `deleted` tinyint(1) DEFAULT 0 NOT NULL; ALTER TABLE `ospos_customers` MODIFY `discount_type` tinyint(1) DEFAULT 0 NOT NULL; @@ -31,16 +27,6 @@ ALTER TABLE `ospos_dinner_tables` MODIFY `deleted` tinyint(1) DEFAULT 0 NOT NULL ALTER TABLE `ospos_dinner_tables` ADD INDEX(`status`); #ospos_employees table -ALTER TABLE `ospos_sales_payments` DROP FOREIGN KEY `ospos_sales_payments_ibfk_2`; -ALTER TABLE `ospos_sales` DROP FOREIGN KEY `ospos_sales_ibfk_1`; -ALTER TABLE `ospos_receivings` DROP FOREIGN KEY `ospos_receivings_ibfk_1`; -ALTER TABLE `ospos_inventory` DROP FOREIGN KEY `ospos_inventory_ibfk_2`; -ALTER TABLE `ospos_grants` DROP FOREIGN KEY `ospos_grants_ibfk_2`; -ALTER TABLE `ospos_expenses` DROP FOREIGN KEY `ospos_expenses_ibfk_2`; -ALTER TABLE `ospos_employees` DROP FOREIGN KEY `ospos_employees_ibfk_1`; -ALTER TABLE `ospos_cash_up` DROP FOREIGN KEY `ospos_cash_up_ibfk_1`; -ALTER TABLE `ospos_cash_up` DROP FOREIGN KEY `ospos_cash_up_ibfk_2`; - ALTER TABLE `ospos_employees` MODIFY `deleted` tinyint(1) DEFAULT 0 NOT NULL; ALTER TABLE `ospos_employees` MODIFY `hash_version` tinyint(1) DEFAULT 2 NOT NULL; @@ -67,7 +53,6 @@ ALTER TABLE `ospos_expense_categories` ADD INDEX(`category_description`); ALTER TABLE `ospos_giftcards` MODIFY `deleted` tinyint(1) DEFAULT 0 NOT NULL; #ospos_items table -ALTER TABLE `ospos_items` DROP FOREIGN KEY `ospos_items_ibfk_1`; ALTER TABLE `ospos_items` MODIFY `deleted` tinyint(1) DEFAULT 0 NOT NULL; ALTER TABLE `ospos_items` MODIFY `stock_type` tinyint(1) DEFAULT 0 NOT NULL; ALTER TABLE `ospos_items` MODIFY `item_type` tinyint(1) DEFAULT 0 NOT NULL; @@ -112,10 +97,6 @@ ALTER TABLE `ospos_sessions` ADD INDEX(`ip_address`); ALTER TABLE `ospos_stock_locations` MODIFY `deleted` tinyint(1) DEFAULT 0 NOT NULL; #ospos_suppliers table -ALTER TABLE `ospos_expenses` DROP FOREIGN KEY `ospos_expenses_ibfk_3`; -ALTER TABLE `ospos_receivings` DROP FOREIGN KEY `ospos_receivings_ibfk_2`; -ALTER TABLE `ospos_suppliers` DROP FOREIGN KEY `ospos_suppliers_ibfk_1`; - ALTER TABLE `ospos_suppliers` MODIFY `deleted` tinyint(1) DEFAULT 0 NOT NULL; ALTER TABLE `ospos_suppliers` MODIFY `category` tinyint(1) NOT NULL; ALTER TABLE `ospos_suppliers` ADD INDEX(`category`); diff --git a/app/Helpers/dompdf_helper.php b/app/Helpers/dompdf_helper.php index 3baf0a5ea..fcd9a3c1f 100644 --- a/app/Helpers/dompdf_helper.php +++ b/app/Helpers/dompdf_helper.php @@ -5,8 +5,13 @@ */ function create_pdf(string $html, string $filename = ''): string { - // Need to enable magic quotes for the - $dompdf = new Dompdf\Dompdf(['isRemoteEnabled' => true, 'isPhpEnabled' => true]); + // Security: Disable PHP execution in PDFs to prevent RCE attacks + // Security: Disable remote file access to prevent SSRF attacks + // Only local files referenced in HTML are allowed + $dompdf = new Dompdf\Dompdf([ + 'isRemoteEnabled' => false, + 'isPhpEnabled' => false + ]); $dompdf->loadHtml(str_replace(['\n', '\r'], '', $html)); $dompdf->render(); diff --git a/app/Helpers/migration_helper.php b/app/Helpers/migration_helper.php index ef056404e..6baa9f24c 100644 --- a/app/Helpers/migration_helper.php +++ b/app/Helpers/migration_helper.php @@ -88,7 +88,7 @@ function executeScriptWithTransaction(string $path): bool } /** - * Drops provided foreign key constraints from given table. + * Drops provided foreign key constraints from a given table if the constraint exists. * This is required to successfully create the generated unique constraint. * * @param array $foreignKeys names of the foreign key constraints to drop diff --git a/app/Helpers/security_helper.php b/app/Helpers/security_helper.php index ea52be64d..5311b4ce9 100644 --- a/app/Helpers/security_helper.php +++ b/app/Helpers/security_helper.php @@ -40,17 +40,16 @@ function check_encryption(): bool $config_file = file_get_contents($config_path); - if (strpos($config_file, 'encryption.key') !== false) { - $config_file = preg_replace("/(encryption\.key.*=.*)('.*')/", "$1'$key'", $config_file); + if (preg_match('/^\s*encryption\.key\s*=/m', $config_file)) { + $config_file = preg_replace("/^(\s*encryption\.key\s*=\s*).*/m", "\$1'$key'", $config_file, 1); } else { $config_file .= "\nencryption.key = '$key'\n"; } if (!empty($old_key)) { $old_line = "# encryption.key = '$old_key' REMOVE IF UNNEEDED\r\n"; - $insertion_point = stripos($config_file, 'encryption.key'); - if ($insertion_point !== false) { - $config_file = substr_replace($config_file, $old_line, $insertion_point, 0); + if (preg_match('/^encryption\.key\s*=/m', $config_file, $matches, PREG_OFFSET_CAPTURE)) { + $config_file = substr_replace($config_file, $old_line, $matches[0][1], 0); } } diff --git a/app/Helpers/tabular_helper.php b/app/Helpers/tabular_helper.php index 50d7fb4ff..7ede9c5a8 100644 --- a/app/Helpers/tabular_helper.php +++ b/app/Helpers/tabular_helper.php @@ -226,7 +226,7 @@ function get_person_data_row(object $person): array 'people.person_id' => $person->person_id, 'last_name' => $person->last_name, 'first_name' => $person->first_name, - 'email' => empty($person->email) ? '' : mailto($person->email, $person->email), + 'email' => empty($person->email) ? '' : mailto(esc($person->email), esc($person->email)), 'phone_number' => $person->phone_number, 'messages' => empty($person->phone_number) ? '' @@ -292,7 +292,7 @@ function get_customer_data_row(object $person, object $stats): array 'people.person_id' => $person->person_id, 'last_name' => $person->last_name, 'first_name' => $person->first_name, - 'email' => empty($person->email) ? '' : mailto($person->email, $person->email), + 'email' => empty($person->email) ? '' : mailto(esc($person->email), esc($person->email)), 'phone_number' => $person->phone_number, 'total' => to_currency($stats->total), 'messages' => empty($person->phone_number) @@ -363,7 +363,7 @@ function get_supplier_data_row(object $supplier): array 'category' => $supplier->category, 'last_name' => $supplier->last_name, 'first_name' => $supplier->first_name, - 'email' => empty($supplier->email) ? '' : mailto($supplier->email, $supplier->email), + 'email' => empty($supplier->email) ? '' : mailto(esc($supplier->email), esc($supplier->email)), 'phone_number' => $supplier->phone_number, 'messages' => empty($supplier->phone_number) ? '' diff --git a/app/Models/Sale.php b/app/Models/Sale.php index 627a1a826..5e667c606 100644 --- a/app/Models/Sale.php +++ b/app/Models/Sale.php @@ -237,6 +237,9 @@ class Sale extends Model $builder->orLike('customer_p.first_name', $search); // Customer first name $builder->orLike('CONCAT(customer_p.first_name, " ", customer_p.last_name)', $search); // Customer first and last name $builder->orLike('customer.company_name', $search); // Customer company name + if (ctype_digit($search)) { + $builder->orWhere('sales.sale_id', $search); // Sale ID + } $builder->groupEnd(); } } @@ -1477,6 +1480,9 @@ class Sale extends Model $builder->orLike('CONCAT(customer_p.first_name, " ", customer_p.last_name)', $search); // Customer company name $builder->orLike('customer.company_name', $search); + if (ctype_digit($search)) { + $builder->orWhere('sales.sale_id', $search); // Sale ID + } $builder->groupEnd(); } } diff --git a/app/Views/configs/tax_config.php b/app/Views/configs/tax_config.php index 7b970e6a6..23248082c 100644 --- a/app/Views/configs/tax_config.php +++ b/app/Views/configs/tax_config.php @@ -51,10 +51,6 @@
'number', - 'step' => 'any', - 'min' => '0', - 'max' => '100', 'name' => 'default_tax_1_rate', 'id' => 'default_tax_1_rate', 'class' => 'form-control input-sm', @@ -76,10 +72,6 @@
'number', - 'step' => 'any', - 'min' => '0', - 'max' => '100', 'name' => 'default_tax_2_rate', 'id' => 'default_tax_2_rate', 'class' => 'form-control input-sm', diff --git a/tests/Controllers/CustomersCsvImportTest.php b/tests/Controllers/CustomersCsvImportTest.php new file mode 100644 index 000000000..4c071ebfc --- /dev/null +++ b/tests/Controllers/CustomersCsvImportTest.php @@ -0,0 +1,266 @@ +customer = model(Customer::class); + $this->employee = model(Employee::class); + + helper('test'); + } + + protected function tearDown(): void + { + parent::tearDown(); + } + + protected function loginAsEmployee(): void + { + $session = Services::session(); + $session->set('person_id', 1); + $session->set('menu_group', 'office'); + } + + protected function createCsvFile(array $rows): string + { + $tempFile = tempnam(sys_get_temp_dir(), 'csv_test_'); + + $handle = fopen($tempFile, 'w'); + foreach ($rows as $row) { + fputcsv($handle, $row); + } + fclose($handle); + + return $tempFile; + } + + public function testValidEmailIsAccepted(): void + { + $this->loginAsEmployee(); + + $csvContent = [ + ['First Name', 'Last Name', 'Gender', 'Consent', 'Email', 'Phone', 'Address 1', 'Address 2', 'City', 'State', 'Zip', 'Country', 'Comments', 'Company', 'Account Number', 'Discount', 'Discount Type', 'Taxable'], + ['John', 'Doe', '1', '1', 'john.doe@example.com', '555-1234', '123 Main St', '', 'Springfield', 'IL', '62701', 'US', '', '', '', '', '', ''] + ]; + + $tempFile = $this->createCsvFile($csvContent); + + $_FILES['file_path'] = [ + 'name' => 'test.csv', + 'type' => 'text/csv', + 'tmp_name' => $tempFile, + 'error' => UPLOAD_ERR_OK, + 'size' => filesize($tempFile) + ]; + + $result = $this->post('/customers/importCsvFile'); + + $result->assertOK(); + $result->assertJSONExact(['success' => true, 'message' => 'Customers imported successfully']); + + $importedCustomer = $this->customer->where('email', 'john.doe@example.com')->first(); + $this->assertNotNull($importedCustomer); + + unlink($tempFile); + } + + public function testInvalidEmailIsRejected(): void + { + $this->loginAsEmployee(); + + $csvContent = [ + ['First Name', 'Last Name', 'Gender', 'Consent', 'Email', 'Phone', 'Address 1', 'Address 2', 'City', 'State', 'Zip', 'Country', 'Comments', 'Company', 'Account Number', 'Discount', 'Discount Type', 'Taxable'], + ['John', 'Doe', '1', '1', 'not-an-email', '555-1234', '123 Main St', '', 'Springfield', 'IL', '62701', 'US', '', '', '', '', '', ''] + ]; + + $tempFile = $this->createCsvFile($csvContent); + + $_FILES['file_path'] = [ + 'name' => 'test.csv', + 'type' => 'text/csv', + 'tmp_name' => $tempFile, + 'error' => UPLOAD_ERR_OK, + 'size' => filesize($tempFile) + ]; + + $result = $this->post('/customers/importCsvFile'); + + $result->assertOK(); + + $resultBody = json_decode($result->getJSON(), true); + $this->assertFalse($resultBody['success'], 'Import should fail for invalid email'); + $this->assertStringContainsString('Row 1', $resultBody['message'], 'Error message should reference failing row'); + $this->assertStringContainsString('Invalid email format', $resultBody['message'], 'Error message should mention email validation'); + + $importedCustomer = $this->customer->where('email', 'not-an-email')->first(); + $this->assertNull($importedCustomer, 'Customer with invalid email should not be imported'); + + unlink($tempFile); + } + + public function testXssPayloadInEmailIsSanitized(): void + { + $this->loginAsEmployee(); + + $maliciousEmail = '@example.com'; + + $csvContent = [ + ['First Name', 'Last Name', 'Gender', 'Consent', 'Email', 'Phone', 'Address 1', 'Address 2', 'City', 'State', 'Zip', 'Country', 'Comments', 'Company', 'Account Number', 'Discount', 'Discount Type', 'Taxable'], + ['John', 'Doe', '1', '1', $maliciousEmail, '555-1234', '123 Main St', '', 'Springfield', 'IL', '62701', 'US', '', '', '', '', '', ''] + ]; + + $tempFile = $this->createCsvFile($csvContent); + + $_FILES['file_path'] = [ + 'name' => 'test.csv', + 'type' => 'text/csv', + 'tmp_name' => $tempFile, + 'error' => UPLOAD_ERR_OK, + 'size' => filesize($tempFile) + ]; + + $result = $this->post('/customers/importCsvFile'); + + $result->assertOK(); + + $importedCustomer = $this->customer->where('email LIKE', '%example.com')->first(); + + $this->assertNotNull($importedCustomer, 'Customer should be imported after sanitization'); + $this->assertStringNotContainsString('', $importedCustomer->email, 'Script tags should be removed'); + + unlink($tempFile); + } + + public function testMixedValidAndInvalidEmails(): void + { + $this->loginAsEmployee(); + + $csvContent = [ + ['First Name', 'Last Name', 'Gender', 'Consent', 'Email', 'Phone', 'Address 1', 'Address 2', 'City', 'State', 'Zip', 'Country', 'Comments', 'Company', 'Account Number', 'Discount', 'Discount Type', 'Taxable'], + ['Valid', 'User', '1', '1', 'valid@example.com', '555-1111', '123 Main St', '', 'City1', 'ST', '12345', 'US', '', '', '', '', '', ''], + ['Invalid', 'User', '1', '1', 'invalid-email', '555-2222', '456 Oak Ave', '', 'City2', 'ST', '23456', 'US', '', '', '', '', '', ''], + ['Another', 'Valid', '1', '1', 'another@example.com', '555-3333', '789 Pine Rd', '', 'City3', 'ST', '34567', 'US', '', '', '', '', '', ''] + ]; + + $tempFile = $this->createCsvFile($csvContent); + + $_FILES['file_path'] = [ + 'name' => 'test.csv', + 'type' => 'text/csv', + 'tmp_name' => $tempFile, + 'error' => UPLOAD_ERR_OK, + 'size' => filesize($tempFile) + ]; + + $result = $this->post('/customers/importCsvFile'); + + $result->assertOK(); + + $validCustomer1 = $this->customer->where('email', 'valid@example.com')->first(); + $this->assertNotNull($validCustomer1, 'Valid customer should be imported'); + + $validCustomer2 = $this->customer->where('email', 'another@example.com')->first(); + $this->assertNotNull($validCustomer2, 'Another valid customer should be imported'); + + $invalidCustomer = $this->customer->where('email', 'invalid-email')->first(); + $this->assertNull($invalidCustomer, 'Invalid email customer should not be imported'); + + unlink($tempFile); + } + + public function testEmailWithSpecialCharactersIsSanitized(): void + { + $this->loginAsEmployee(); + + $emailWithSpecialChars = 'test"user@example.com'; + $csvContent = [ + ['First Name', 'Last Name', 'Gender', 'Consent', 'Email', 'Phone', 'Address 1', 'Address 2', 'City', 'State', 'Zip', 'Country', 'Comments', 'Company', 'Account Number', 'Discount', 'Discount Type', 'Taxable'], + ['Test', 'User', '1', '1', $emailWithSpecialChars, '555-1234', '123 Main St', '', 'Springfield', 'IL', '62701', 'US', '', '', '', '', '', ''] + ]; + + $tempFile = $this->createCsvFile($csvContent); + + $_FILES['file_path'] = [ + 'name' => 'test.csv', + 'type' => 'text/csv', + 'tmp_name' => $tempFile, + 'error' => UPLOAD_ERR_OK, + 'size' => filesize($tempFile) + ]; + + $result = $this->post('/customers/importCsvFile'); + + $result->assertOK(); + + $importedCustomer = $this->customer->where('email LIKE', '%example.com')->first(); + + $this->assertNotNull($importedCustomer, 'Sanitized email should be imported'); + $this->assertStringNotContainsString('"', $importedCustomer->email, 'Quote characters should be sanitized'); + + unlink($tempFile); + } + + public function testEmptyEmailIsAccepted(): void + { + $this->loginAsEmployee(); + + // Empty email should be allowed - customers may not have email addresses + $csvContent = [ + ['First Name', 'Last Name', 'Gender', 'Consent', 'Email', 'Phone', 'Address 1', 'Address 2', 'City', 'State', 'Zip', 'Country', 'Comments', 'Company', 'Account Number', 'Discount', 'Discount Type', 'Taxable'], + ['John', 'Doe', '1', '1', '', '555-1234', '123 Main St', '', 'Springfield', 'IL', '62701', 'US', '', '', '', '', '', ''] + ]; + + $tempFile = $this->createCsvFile($csvContent); + + $_FILES['file_path'] = [ + 'name' => 'test.csv', + 'type' => 'text/csv', + 'tmp_name' => $tempFile, + 'error' => UPLOAD_ERR_OK, + 'size' => filesize($tempFile) + ]; + + $result = $this->post('/customers/importCsvFile'); + + $result->assertOK(); + + $resultBody = json_decode($result->getJSON(), true); + $this->assertTrue($resultBody['success'], 'Import should succeed with empty email'); + + // Find customer by name since email is empty + $importedCustomer = $this->customer->select('customers.*, people.*') + ->join('people', 'people.person_id = customers.person_id') + ->where('first_name', 'John') + ->where('last_name', 'Doe') + ->first(); + + $this->assertNotNull($importedCustomer, 'Customer with empty email should be imported'); + $this->assertEquals('', $importedCustomer->email, 'Email should be empty string'); + + unlink($tempFile); + } +} \ No newline at end of file