From cd91ac3ff341d2738f58e9634a98c5f848aafa52 Mon Sep 17 00:00:00 2001 From: objec Date: Tue, 5 May 2026 15:06:04 +0400 Subject: [PATCH] Fix bugs - Add missing `MailchimpPlugin.` prefix to lang() calls. - Do not subscribe customer if consent is not true. - Escape output in tabular_helper.php - Removed testConnection() as unneeded code - Fix activity count logic - Whitelist Sort Column Headers for Plugins.php - Store encrypted API key as base64 instead of raw binary to prevent truncation - Rollback on batchSave partial failure. - Remove dead code. - Disable plugin before uninstalling it. - Fix getPluginSettings() internal key leak - Add action column to plugin headers function - Automatically add grant to all admins in case person_id 1 is not active Signed-off-by: objec --- app/Controllers/Plugins.php | 2 +- .../3.4.1_PluginConfigTableCreate.sql | 4 +- app/Helpers/tabular_helper.php | 3 +- app/Libraries/Plugins/PluginManager.php | 2 + app/Models/PluginConfig.php | 17 ++--- .../Libraries/MailchimpLibrary.php | 17 ++--- .../MailchimpPlugin/MailchimpPlugin.php | 65 ++++++++----------- 7 files changed, 51 insertions(+), 59 deletions(-) diff --git a/app/Controllers/Plugins.php b/app/Controllers/Plugins.php index 106bfb0a6..4db6216c6 100644 --- a/app/Controllers/Plugins.php +++ b/app/Controllers/Plugins.php @@ -26,7 +26,7 @@ class Plugins extends Secure_Controller $search = strtolower($this->request->getGet('search') ?? ''); $limit = (int)($this->request->getGet('limit') ?? 0); $offset = (int)($this->request->getGet('offset') ?? 0); - $sort = $this->request->getGet('sort', FILTER_SANITIZE_FULL_SPECIAL_CHARS) ?? 'name'; + $sort = $this->sanitizeSortColumn(plugin_headers(), $this->request->getGet('sort', FILTER_SANITIZE_FULL_SPECIAL_CHARS), 'name'); $order = strtolower($this->request->getGet('order', FILTER_SANITIZE_FULL_SPECIAL_CHARS) ?? 'asc'); $pluginData = $this->buildPluginDataArray(); diff --git a/app/Database/Migrations/sqlscripts/3.4.1_PluginConfigTableCreate.sql b/app/Database/Migrations/sqlscripts/3.4.1_PluginConfigTableCreate.sql index b6b635366..30a4200e3 100644 --- a/app/Database/Migrations/sqlscripts/3.4.1_PluginConfigTableCreate.sql +++ b/app/Database/Migrations/sqlscripts/3.4.1_PluginConfigTableCreate.sql @@ -12,5 +12,5 @@ INSERT IGNORE INTO `ospos_modules` (`name_lang_key`, `desc_lang_key`, `sort`, `m INSERT IGNORE INTO `ospos_permissions` (`permission_id`, `module_id`) VALUES ('plugins', 'plugins'); -INSERT IGNORE INTO `ospos_grants` (`permission_id`, `person_id`, `menu_group`) VALUES - ('plugins', 1, 'office'); +INSERT IGNORE INTO `ospos_grants` (`permission_id`, `person_id`, `menu_group`) +SELECT 'plugins', `person_id`, 'office' FROM `ospos_grants` WHERE `permission_id` = 'config'; diff --git a/app/Helpers/tabular_helper.php b/app/Helpers/tabular_helper.php index c495d6de9..4acc85223 100644 --- a/app/Helpers/tabular_helper.php +++ b/app/Helpers/tabular_helper.php @@ -940,6 +940,7 @@ function plugin_headers(): array ['description' => lang('Plugins.description')], ['version' => lang('Plugins.version'), 'escape' => false], ['status' => lang('Plugins.status'), 'escape' => false], + ['actions' => lang('Plugins.actions'), 'escape' => false], ]; } @@ -970,7 +971,7 @@ function get_plugin_data_row(array $plugin): array return [ 'plugin_id' => $pluginId, 'name' => '' . esc($plugin['name']) . '
' . esc($pluginId) . '', - 'description' => $plugin['description'], + 'description' => esc($plugin['description']), 'version' => '' . esc($plugin['version']) . '', 'status' => $statusHtml, 'edit' => $editHtml, diff --git a/app/Libraries/Plugins/PluginManager.php b/app/Libraries/Plugins/PluginManager.php index 49b0c75fb..f9511d77d 100644 --- a/app/Libraries/Plugins/PluginManager.php +++ b/app/Libraries/Plugins/PluginManager.php @@ -166,6 +166,8 @@ class PluginManager return false; } + $this->disablePlugin($pluginId); + if (!$plugin->uninstall()) { log_message('error', "Failed to uninstall plugin: {$pluginId}"); return false; diff --git a/app/Models/PluginConfig.php b/app/Models/PluginConfig.php index e7c22cc22..53493555b 100644 --- a/app/Models/PluginConfig.php +++ b/app/Models/PluginConfig.php @@ -49,7 +49,8 @@ class PluginConfig extends Model public function getPluginSettings(string $pluginId): array { $builder = $this->db->table('plugin_config'); - $builder->like('key', $pluginId . '_', 'after'); + $builder->like('key', $pluginId . '_', 'after') + ->notLike('key', $pluginId . '__', 'after'); $query = $builder->get(); $settings = []; @@ -79,17 +80,17 @@ class PluginConfig extends Model public function batchSave(array $data): bool { - $success = true; - - $this->db->transStart(); + $this->db->transBegin(); foreach ($data as $key => $value) { - $success &= $this->setValue($key, $value); + if (!$this->setValue($key, $value)) { + $this->db->transRollback(); + return false; + } } - $this->db->transComplete(); - - return $success && $this->db->transStatus(); + $this->db->transCommit(); + return true; } public function getAll(): array diff --git a/app/Plugins/MailchimpPlugin/Libraries/MailchimpLibrary.php b/app/Plugins/MailchimpPlugin/Libraries/MailchimpLibrary.php index be2b0a5a8..62821b7f7 100644 --- a/app/Plugins/MailchimpPlugin/Libraries/MailchimpLibrary.php +++ b/app/Plugins/MailchimpPlugin/Libraries/MailchimpLibrary.php @@ -303,33 +303,30 @@ class MailchimpLibrary $customerActivities = $this->getMemberActivity($listId, $customerData->email); if ($customerActivities !== false) { if (array_key_exists('activity', $customerActivities)) { + $sent = 0; $open = 0; - $unopen = 0; $click = 0; - $total = 0; $lastOpen = ''; foreach ($customerActivities['activity'] as $activity) { - if ($activity['action'] == 'sent') { - ++$unopen; - } elseif ($activity['action'] == 'open') { + if ($activity['action'] === 'sent') { + ++$sent; + } elseif ($activity['action'] === 'open') { if (empty($lastOpen)) { $lastOpen = substr($activity['timestamp'], 0, 10); } ++$open; - } elseif ($activity['action'] == 'click') { + } elseif ($activity['action'] === 'click') { if (empty($lastOpen)) { $lastOpen = substr($activity['timestamp'], 0, 10); } ++$click; } - - ++$total; } - $mailchimpData['mailchimpActivity']['total'] = $total; + $mailchimpData['mailchimpActivity']['total'] = $sent; $mailchimpData['mailchimpActivity']['open'] = $open; - $mailchimpData['mailchimpActivity']['unopen'] = $unopen; + $mailchimpData['mailchimpActivity']['unopen'] = max(0, $sent - $open); $mailchimpData['mailchimpActivity']['click'] = $click; $mailchimpData['mailchimpActivity']['last_open'] = $lastOpen; } diff --git a/app/Plugins/MailchimpPlugin/MailchimpPlugin.php b/app/Plugins/MailchimpPlugin/MailchimpPlugin.php index 5d8a22e32..b9e4d4483 100644 --- a/app/Plugins/MailchimpPlugin/MailchimpPlugin.php +++ b/app/Plugins/MailchimpPlugin/MailchimpPlugin.php @@ -21,7 +21,12 @@ class MailchimpPlugin extends BasePlugin { parent::__construct(); - $this->mailchimpLibrary = new MailchimpLibrary($this->getSettings()); + $apiKey = $this->decryptApiKey($this->getSetting('api_key', '')); + $this->mailchimpLibrary = new MailchimpLibrary([ + 'api_key' => $apiKey, + 'list_id' => $this->getSetting('list_id', ''), + 'sync_on_save' => $this->getSetting('sync_on_save', '1'), + ]); log_message('debug', 'MailchimpPlugin initialized'); } @@ -80,17 +85,7 @@ class MailchimpPlugin extends BasePlugin public function getSettings(): array { - $encryptedKey = $this->getSetting('api_key', ''); - $apiKey = ''; - - if (!empty($encryptedKey)) { - try { - $apiKey = Services::encrypter()->decrypt($encryptedKey); - } catch (\Exception $e) { - // Key stored as plaintext (pre-encryption migration) — use as-is - $apiKey = $encryptedKey; - } - } + $apiKey = $this->decryptApiKey($this->getSetting('api_key', '')); return [ 'api_key' => $apiKey, @@ -101,6 +96,22 @@ class MailchimpPlugin extends BasePlugin ]; } + private function decryptApiKey(string $encryptedKey): string + { + if (empty($encryptedKey)) { + return ''; + } + try { + $decoded = base64_decode($encryptedKey, true); + if ($decoded !== false) { + return Services::encrypter()->decrypt($decoded); + } + } catch (\Exception) { + // Legacy plaintext or old binary-encrypted key — fall through + } + return $encryptedKey; + } + private function getFormattedLists(string $apiKey): array { if (empty($apiKey)) { @@ -127,7 +138,7 @@ class MailchimpPlugin extends BasePlugin if (array_key_exists('api_key', $settings)) { $rawKey = (string)$settings['api_key']; $normalized['api_key'] = !empty($rawKey) - ? Services::encrypter()->encrypt($rawKey) + ? base64_encode(Services::encrypter()->encrypt($rawKey)) : ''; } @@ -186,7 +197,10 @@ class MailchimpPlugin extends BasePlugin log_message('debug', "Customer saved event received for ID: {$customerData['person_id']}"); - $statusInt = (int)(($postData ?? [])['status'] ?? SubscriptionStatus::SUBSCRIBED->value); + $statusInt = !empty($customerData['consent']) + ? (int)(($postData ?? [])['status'] ?? SubscriptionStatus::SUBSCRIBED->value) + : SubscriptionStatus::UNSUBSCRIBED->value; + $statusEnum = SubscriptionStatus::tryFrom($statusInt) ?? SubscriptionStatus::SUBSCRIBED; $subscriptionStatus = strtolower($statusEnum->name); @@ -214,27 +228,4 @@ class MailchimpPlugin extends BasePlugin { return $this->getSetting('sync_on_save', '1') === '1'; } - - - public function testConnection(): array - { - $apiKey = $this->getSetting('api_key'); - - if (empty($apiKey)) { - return ['success' => false, 'message' => lang('api_key_required')]; - } - - $result = $this->mailchimpLibrary->getLists(); - - if ($result && isset($result['lists'])) { - return [ - 'success' => true, - 'message' => lang('key_successfully'), - 'lists' => $result['lists'] - ]; - } - - return ['success' => false, 'message' => lang('key_unsuccessfully')]; - } - }