Add protections against spams and a configuration page. #19
Conversation
Trololo Add protections against spams and a configuration page.
contactform.php
Outdated
|
|
||
| $message = $this->trans( | ||
| 'For even more security on your website forms, consult our Security & Access modules category on the %s', | ||
| array($this->getSecurityMarketPlaceLink()), |
There was a problem hiding this comment.
Don't you want to add placeholders like in the core? Like
'%link%' => array($this->getSecurityMarketPlaceLink()),
contactform.php
Outdated
|
|
||
| if (!($from = trim(Tools::getValue('from'))) || !Validate::isEmail($from)) { | ||
| $this->context->controller->errors[] = $this->trans('Invalid email address.', array(), 'Shop.Notifications.Error'); | ||
| } elseif (preg_match('#^[ ]*$#', $message)) { | ||
| } elseif (!$message) { |
There was a problem hiding this comment.
It seems a message filled with blanks could pass now, should not we test that with a trim() and empty()?
contactform.php
Outdated
| { | ||
|
|
||
| $message = $this->trans( | ||
| 'For even more security on your website forms, consult our Security & Access modules category on the %s', |
There was a problem hiding this comment.
Maybe change "website" to "shop's"?
ping @LouiseBonnard @colinegin
contactform.php
Outdated
| array( | ||
| 'type' => 'switch', | ||
| 'label' => $this->trans('Send confirmation email to your customers', array(), 'Modules.Contactform.Admin'), | ||
| 'desc' => $this->trans('Click Yes and your customers will receive a confirmation email without the message content', array(), 'Modules.Contactform.Admin'), |
There was a problem hiding this comment.
I think this wording could be improved to better guide the merchants:
Choose Yes and your customers will receive a generic confirmation email including a tracking number after their message is sent. Note: to discourage spam, the content of their message won't be included in the email.
What do you think @LouiseBonnard @colinegin ?
contactform.php
Outdated
| $lang = new Language((int) Configuration::get('PS_LANG_DEFAULT')); | ||
| $helper->default_form_language = $lang->id; | ||
| $helper->submit_action = 'update-configuration'; | ||
| $helper->currentIndex = $this->context->link->getAdminLink('AdminModules', false).'&configure=' |
There was a problem hiding this comment.
You should use parse_url() to make sure that there's a query string in the URL returned by $this->context->link->getAdminLink('AdminModules', false), then http_build_query() to rebuild the query string with your arguments.
contactform.php
Outdated
| } elseif ($url !== '' || empty($serverToken) | ||
| || $clientToken !== $serverToken || $clientTokenTTL < time() | ||
| ) { | ||
| $this->context->controller->errors[] = $this->trans('An error occurred while sending the message.', array(), 'Modules.Contactform.Shop'); |
There was a problem hiding this comment.
How about adding a "Please try again"?
contactform.php
Outdated
| @@ -176,14 +375,17 @@ public function sendMessage() | |||
| $this->context->controller->errors[] = $this->trans('An error occurred during the file-upload process.', array(), 'Modules.Contactform.Shop'); | |||
| } elseif (!empty($file_attachment['name']) && !in_array(Tools::strtolower(substr($file_attachment['name'], -4)), $extension) && !in_array(Tools::strtolower(substr($file_attachment['name'], -5)), $extension)) { | |||
| $this->context->controller->errors[] = $this->trans('Bad file extension', array(), 'Modules.Contactform.Shop'); | |||
| } elseif ($url !== '' || empty($serverToken) | |||
| || $clientToken !== $serverToken || $clientTokenTTL < time() | |||
There was a problem hiding this comment.
Either one line for all statements or one statement per line please
Fix issues, code refacto Fix issues, code refacto Fix issues, code refacto Fix issues Fix issues, code refacto Fix blank line Remove extra space Update translation
|
Thank you @alegout |
Add protections against spams and a configuration page.
Ticket: BOOM-4288
QA: Needs to be tested with 1.7.2 and 1.7.3. Wordings will have to stay in English until 1.7.4