mirror of
https://github.com/opensourcepos/opensourcepos.git
synced 2026-07-25 22:27:05 -04:00
Fix encryption key persistence for Docker environments
The check_encryption() function now properly handles Docker/container environments where ROOTPATH/.env may be read-only or ephemeral. Changes: - Returns false when key persistence fails instead of always returning true - Removes error suppression (@) to properly detect write failures - Adds fallback to WRITEPATH/config/encryption.key for container volumes - Splits logic into separate functions for clarity and testability Fixes encryption key being lost on container restarts, which caused stored passwords to become undecryptable. GitHub-Issue: #4554 Add fallback key loading from WRITEPATH in Encryption config When encryption key is not available from .env or environment variables, the config now attempts to load from WRITEPATH/config/encryption.key. This supports Docker environments where: - .env file is read-only or ephemeral - Key was persisted to the writable volume via check_encryption() GitHub-Issue: #4554 Handle encryption unavailability gracefully in controllers Changed EncrypterInterface property to nullable and added proper error handling for cases where encryption key is not available. Changes: - Config controller: nullable encrypter property, try/catch around encryption - Email_lib: check encryption before using encrypter - Return meaningful error messages when encryption fails - Log warnings when passwords saved without encryption Users will now see clear error messages instead of unhandled exceptions when encryption key cannot be initialized. GitHub-Issue: #4554 Add encryption_failed error message to language file Added localization string for encryption failure error messages. GitHub-Issue: #4554 Add decrypt_value() and encrypt_value() helper functions Extracts the recurring decryption/encryption pattern into reusable helper functions with consistent error handling: - decrypt_value(): Safely decrypts encrypted values with try/catch - encrypt_value(): Safely encrypts values with error handling Both functions handle: - Empty/null values gracefully - Missing encryption key (logs warning) - Encryption/decryption failures (logs error, returns default) This pattern appears in 8+ locations across the codebase. GitHub-Issue: #4554 Refactor all encryption/decryption to use helper functions Replaces direct encrypter calls with decrypt_value() and encrypt_value() helpers throughout the codebase for consistent error handling: - Config controller: SMTP, SMS, Mailchimp credential encryption - Email_lib: SMTP password decryption - Sms_lib: SMS password decryption - Mailchimp_lib: API key decryption - Customers controller: Mailchimp list ID decryption Removes nullable EncrypterInterface property from Config controller as encryption is now handled via helper functions. GitHub-Issue: #4554 Address CodeRabbit feedback: validate key length, clarify encryption failure handling - loadKeyFromWritable() now validates key length >= 64 before accepting - encrypt_value() renamed param, defaults to failing encryption required - Clearer error message when credentials not saved GitHub-Issue: #4554 fix: address CodeRabbit review comments for encryption key persistence - Always mirror encryption key to both .env and WRITEPATH (Docker safety) - Guard array key access with isset() before reading in Encryption.php - Fix encrypt_value() to not treat string '0' as empty - Improve error logging for failed encryption attempts refactor: PSR-compliant naming and address objecttothis review comments - Rename functions to camelCase: checkEncryption, writeEncryptionKeyToEnv, writeEncryptionKeyToWritable, loadEncryptionKeyFromWritable, abortEncryptionConversion, removeBackup, decryptValue, encryptValue - Update all callers in Config.php, Customers.php, Migrations, Email_lib.php, Sms_lib.php, Mailchimp_lib.php - Add EncryptionException import in security_helper.php (removed FQN) - Use camelCase variables: $smtpPass, $emailConfig, $batchSaveData in affected files - Remove unnecessary inline comments (code is self-documenting) - Keep necessary docstrings for public API documentation Address remaining CodeRabbit review comments - Fix decryptValue() to use explicit null/empty check instead of empty() (handles string "0" correctly) - Guard checkEncryption() result in migration before proceeding - Check read success before writing backup restoration - Consistent DIRECTORY_SEPARATOR usage in paths GitHub-Issue: #4554
This commit is contained in:
Ollama
@@ -17,11 +17,9 @@ use App\Models\Enums\Rounding_mode;
|
||||
use App\Models\Stock_location;
|
||||
use App\Models\Tax;
|
||||
use CodeIgniter\Database\BaseConnection;
|
||||
use CodeIgniter\Encryption\EncrypterInterface;
|
||||
use CodeIgniter\HTTP\ResponseInterface;
|
||||
use Config\Database;
|
||||
use Config\OSPOS;
|
||||
use Config\Services;
|
||||
use DirectoryIterator;
|
||||
use NumberFormatter;
|
||||
use ReflectionException;
|
||||
@@ -30,7 +28,6 @@ class Config extends Secure_Controller
|
||||
{
|
||||
protected $helpers = ['security'];
|
||||
private BaseConnection $db;
|
||||
private EncrypterInterface $encrypter;
|
||||
private Barcode_lib $barcode_lib;
|
||||
private Sale_lib $sale_lib;
|
||||
private Receiving_lib $receiving_lib;
|
||||
@@ -62,13 +59,6 @@ class Config extends Secure_Controller
|
||||
$this->tax = model(Tax::class);
|
||||
$this->config = config(OSPOS::class)->settings;
|
||||
$this->db = Database::connect();
|
||||
|
||||
helper('security');
|
||||
if (check_encryption()) {
|
||||
$this->encrypter = Services::encrypter();
|
||||
} else {
|
||||
log_message('alert', 'Error preparing encryption key');
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -256,25 +246,11 @@ class Config extends Secure_Controller
|
||||
// Integrations Related fields
|
||||
$data['mailchimp'] = [];
|
||||
|
||||
if (check_encryption()) { // TODO: Hungarian notation
|
||||
if (!isset($this->encrypter)) {
|
||||
helper('security');
|
||||
$this->encrypter = Services::encrypter();
|
||||
}
|
||||
$data['mailchimp']['api_key'] = decryptValue($this->config['mailchimp_api_key'] ?? null);
|
||||
$data['mailchimp']['list_id'] = decryptValue($this->config['mailchimp_list_id'] ?? null);
|
||||
|
||||
$data['mailchimp']['api_key'] = (isset($this->config['mailchimp_api_key']) && !empty($this->config['mailchimp_api_key']))
|
||||
? $this->encrypter->decrypt($this->config['mailchimp_api_key'])
|
||||
: '';
|
||||
|
||||
$data['mailchimp']['list_id'] = (isset($this->config['mailchimp_list_id']) && !empty($this->config['mailchimp_list_id']))
|
||||
? $this->encrypter->decrypt($this->config['mailchimp_list_id'])
|
||||
: '';
|
||||
|
||||
// Remove any backup of .env created by check_encryption()
|
||||
remove_backup();
|
||||
} else {
|
||||
$data['mailchimp']['api_key'] = '';
|
||||
$data['mailchimp']['list_id'] = '';
|
||||
if (checkEncryption()) {
|
||||
removeBackup();
|
||||
}
|
||||
|
||||
$data['mailchimp']['lists'] = $this->_mailchimp();
|
||||
@@ -512,15 +488,23 @@ class Config extends Secure_Controller
|
||||
public function postSaveEmail(): ResponseInterface
|
||||
{
|
||||
$password = '';
|
||||
$passwordInput = $this->request->getPost('smtp_pass');
|
||||
|
||||
if (check_encryption() && !empty($this->request->getPost('smtp_pass'))) {
|
||||
$password = $this->encrypter->encrypt($this->request->getPost('smtp_pass'));
|
||||
if (!empty($passwordInput)) {
|
||||
$password = encryptValue($passwordInput);
|
||||
if (empty($password)) {
|
||||
log_message('error', 'SMTP password encryption failed - credentials not saved');
|
||||
|
||||
return $this->response->setJSON([
|
||||
'success' => false,
|
||||
'message' => lang('Config.encryption_failed'),
|
||||
]);
|
||||
}
|
||||
}
|
||||
|
||||
$protocol = $this->request->getPost('protocol');
|
||||
$mailpath = $this->request->getPost('mailpath');
|
||||
|
||||
// Validate mailpath: required for sendmail, optional for others but must be safe if provided
|
||||
$isMailpathRequired = ($protocol === 'sendmail');
|
||||
$isMailpathProvided = !empty($mailpath);
|
||||
$isMailpathValid = $isMailpathProvided && preg_match('/^[a-zA-Z0-9_\-\/.]+$/', $mailpath);
|
||||
@@ -528,7 +512,7 @@ class Config extends Secure_Controller
|
||||
if (($isMailpathRequired && !$isMailpathProvided) || ($isMailpathProvided && !$isMailpathValid)) {
|
||||
return $this->response->setJSON([
|
||||
'success' => false,
|
||||
'message' => lang('Config.mailpath_invalid')
|
||||
'message' => lang('Config.mailpath_invalid'),
|
||||
]);
|
||||
}
|
||||
|
||||
@@ -540,7 +524,7 @@ class Config extends Secure_Controller
|
||||
'smtp_pass' => $password,
|
||||
'smtp_port' => $this->request->getPost('smtp_port', FILTER_SANITIZE_NUMBER_INT),
|
||||
'smtp_timeout' => $this->request->getPost('smtp_timeout', FILTER_SANITIZE_NUMBER_INT),
|
||||
'smtp_crypto' => $this->request->getPost('smtp_crypto')
|
||||
'smtp_crypto' => $this->request->getPost('smtp_crypto'),
|
||||
];
|
||||
|
||||
$success = $this->appconfig->batch_save($batch_save_data);
|
||||
@@ -558,16 +542,25 @@ class Config extends Secure_Controller
|
||||
public function postSaveMessage(): ResponseInterface
|
||||
{
|
||||
$password = '';
|
||||
$passwordInput = $this->request->getPost('msg_pwd');
|
||||
|
||||
if (check_encryption() && !empty($this->request->getPost('msg_pwd'))) {
|
||||
$password = $this->encrypter->encrypt($this->request->getPost('msg_pwd'));
|
||||
if (!empty($passwordInput)) {
|
||||
$password = encryptValue($passwordInput);
|
||||
if (empty($password)) {
|
||||
log_message('error', 'SMS password encryption failed');
|
||||
|
||||
return $this->response->setJSON([
|
||||
'success' => false,
|
||||
'message' => lang('Config.encryption_failed'),
|
||||
]);
|
||||
}
|
||||
}
|
||||
|
||||
$batch_save_data = [
|
||||
'msg_msg' => $this->request->getPost('msg_msg'),
|
||||
'msg_uid' => $this->request->getPost('msg_uid'),
|
||||
'msg_pwd' => $password,
|
||||
'msg_src' => $this->request->getPost('msg_src')
|
||||
'msg_src' => $this->request->getPost('msg_src'),
|
||||
];
|
||||
|
||||
$success = $this->appconfig->batch_save($batch_save_data);
|
||||
@@ -623,24 +616,38 @@ class Config extends Secure_Controller
|
||||
*/
|
||||
public function postSaveMailchimp(): ResponseInterface
|
||||
{
|
||||
$api_key = '';
|
||||
$list_id = '';
|
||||
$apiKey = '';
|
||||
$listId = '';
|
||||
|
||||
if (check_encryption()) {
|
||||
$api_key_unencrypted = $this->request->getPost('mailchimp_api_key');
|
||||
if (!empty($api_key_unencrypted)) {
|
||||
$api_key = $this->encrypter->encrypt($api_key_unencrypted);
|
||||
}
|
||||
$apiKeyInput = $this->request->getPost('mailchimp_api_key');
|
||||
if (!empty($apiKeyInput)) {
|
||||
$apiKey = encryptValue($apiKeyInput);
|
||||
if (empty($apiKey)) {
|
||||
log_message('error', 'Mailchimp API key encryption failed');
|
||||
|
||||
$list_id_unencrypted = $this->request->getPost('mailchimp_list_id');
|
||||
if (!empty($list_id_unencrypted)) {
|
||||
$list_id = $this->encrypter->encrypt($list_id_unencrypted);
|
||||
return $this->response->setJSON([
|
||||
'success' => false,
|
||||
'message' => lang('Config.encryption_failed'),
|
||||
]);
|
||||
}
|
||||
}
|
||||
|
||||
$batch_save_data = ['mailchimp_api_key' => $api_key, 'mailchimp_list_id' => $list_id];
|
||||
$listIdInput = $this->request->getPost('mailchimp_list_id');
|
||||
if (!empty($listIdInput)) {
|
||||
$listId = encryptValue($listIdInput);
|
||||
if (empty($listId)) {
|
||||
log_message('error', 'Mailchimp list ID encryption failed');
|
||||
|
||||
$success = $this->appconfig->batch_save($batch_save_data);
|
||||
return $this->response->setJSON([
|
||||
'success' => false,
|
||||
'message' => lang('Config.encryption_failed'),
|
||||
]);
|
||||
}
|
||||
}
|
||||
|
||||
$batchSaveData = ['mailchimp_api_key' => $apiKey, 'mailchimp_list_id' => $listId];
|
||||
|
||||
$success = $this->appconfig->batch_save($batchSaveData);
|
||||
|
||||
return $this->response->setJSON(['success' => $success, 'message' => lang('Config.saved_' . ($success ? '' : 'un') . 'successfully')]);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user