Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -199,3 +199,4 @@
- Added feature to group card payment methods into unified "Card" payment method
- Fixed issue when newly enabled payment methods did not appear in checkout because default "all countries/currencies" restriction was not created on save
- Fixed issue when payment method country/currency dropdowns showed "0" instead of indicating that all countries/currencies are allowed
- BO : Added validation for Merchant Emails field (frontend + backend) to prevent saving invalid addresses
31 changes: 31 additions & 0 deletions controllers/admin/AdminSaferPayOfficialSettingsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,17 @@ public function ajaxProcessSaveCredentials()
}
}

$testMerchantEmails = $this->getStringValue($data, 'testMerchantEmails');
$liveMerchantEmails = $this->getStringValue($data, 'liveMerchantEmails');
$invalidEmail = $this->findInvalidEmail($testMerchantEmails) ?: $this->findInvalidEmail($liveMerchantEmails);
if ($invalidEmail !== null) {
$this->ajaxResponse(false, sprintf(
$this->module->l('Invalid merchant email address: %s', self::FILE_NAME),
$invalidEmail
));
return;
}
Comment on lines +182 to +191

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The email validation logic should be moved before the API credentials validation (lines 166-180). Validating local input data first is more efficient as it avoids unnecessary and potentially slow remote API calls if the merchant emails are already invalid.


// Credentials validated — now save
$configuration->set(SaferPayConfig::TEST_MODE, $isTestMode ? 1 : 0);

Expand Down Expand Up @@ -792,4 +803,24 @@ private function getIntValue($data, $key)
{
return isset($data[$key]) ? (int) $data[$key] : 0;
}

/**
* Returns the first invalid email in a comma-separated list, or null if all are valid.
*/
private function findInvalidEmail($emails)
{
if ($emails === '') {
return null;
}
foreach (explode(',', $emails) as $email) {
$email = trim($email);
if ($email === '') {
continue;
}
if (!\Validate::isEmail($email)) {
return $email;
}
}
return null;
}
}
1 change: 1 addition & 0 deletions src/Service/SettingsTranslationService.php
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ private function getApiCredentialsTranslations()
'merchantEmails' => $this->module->l('Merchant Emails', self::FILE_NAME),
'enterMerchantEmails' => $this->module->l('Enter merchant email addresses (comma-separated)', self::FILE_NAME),
'separateEmails' => $this->module->l('These email addresses receive payment notification emails directly from SaferPay. Separate multiple email addresses with commas.', self::FILE_NAME),
'invalidMerchantEmails' => $this->module->l('Invalid email address', self::FILE_NAME),
'saferpayFields' => $this->module->l('Saferpay Fields', self::FILE_NAME),
'saferpayFieldsDescription' => $this->module->l('Configure Saferpay Fields for inline payment form integration.', self::FILE_NAME),
'fieldAccessTokenInfo' => $this->module->l('Saferpay Field Access Token can be found in Saferpay Backoffice, navigate to', self::FILE_NAME),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,13 @@ export function ApiCredentials() {
const hasCredentials = username.length > 0 && password.length > 0
const hasBusinessLicense = isTest ? settings.testHasBusinessLicense : settings.liveHasBusinessLicense

const EMAIL_RE = /^[^\s@]+@[^\s@]+\.[^\s@]+$/
const invalidEmails = merchantEmails
.split(',')
.map(e => e.trim())
.filter(e => e.length > 0 && !EMAIL_RE.test(e))
const merchantEmailsInvalid = invalidEmails.length > 0
Comment on lines +41 to +45

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The merchantEmailsInvalid flag currently only validates the emails for the active environment (Test or Live). However, the backend validates both fields upon saving. This can lead to a confusing UX where the 'Save' button is enabled but the save operation fails due to invalid data in the hidden tab. Consider validating both testMerchantEmails and liveMerchantEmails to determine the disabled state of the Save button, while keeping the inline error message specific to the current tab.


const setField = (field: string, value: string | boolean) => {
updateSettings({ [`${prefix}${field.charAt(0).toUpperCase() + field.slice(1)}`]: value } as Record<string, string | boolean>)
}
Expand Down Expand Up @@ -242,7 +249,14 @@ export function ApiCredentials() {
placeholder={t('enterMerchantEmails')}
value={merchantEmails}
onChange={(e) => setField('merchantEmails', e.target.value)}
aria-invalid={merchantEmailsInvalid}
className={merchantEmailsInvalid ? 'sp-border-destructive focus-visible:sp-ring-destructive' : ''}
/>
{merchantEmailsInvalid && (
<p className="sp-text-xs sp-text-destructive">
{t('invalidMerchantEmails')}: {invalidEmails.join(', ')}
</p>
)}
<p className="sp-text-xs sp-text-muted-foreground">
{t('separateEmails')}
</p>
Expand Down Expand Up @@ -342,7 +356,7 @@ export function ApiCredentials() {
<Button
className="sp-min-w-[120px]"
onClick={saveCredentials}
disabled={saving || !hasCredentials || credentialStatus === 'invalid' || credentialStatus === 'checking'}
disabled={saving || !hasCredentials || credentialStatus === 'invalid' || credentialStatus === 'checking' || merchantEmailsInvalid}
aria-label={saving ? t('saving') : undefined}
>
{saving ? <Loader2 className="sp-h-4 sp-w-4 sp-animate-spin" /> : t('saveChanges')}
Expand Down
Loading