[D7] Mailgun Email Validator [#2441453]

Synopsis

This modules allow you to validated your Email widget field module, Webform email field and Custom textfield with mailgun parameter through Mailgun Email Validator, which is much more than regex validation. Mailgun validator is based on real-world data + RFC spec to ensure more accurate validation.

Configuration

After enabling this module, please check admin/config/services/mailgun. Here you can insert your Mailgun api credential, which will include public-key.

Check for Open SSL extension enabled.

Configure Email field widget :

Install Email field Module.
Create cck field for email using email widget.
While creating the field, Mailgun email validator enable checkbox will be provided within Field setting form.
Enabling Mailgun email validator will validate email widget field,if invalid email is entered Mailgun will provide you with suggested email address right below your email field.

Configure Webform email field :

Install Webform Module.
Create webform and add email field.
While creating the field, Mailgun email validator enable checkbox will be provided within webform component setting form.
Enabling Mailgun email validator will validate email widget field,if invalid email is entered Mailgun will provide you with suggested email address right below your email field.

Usage

To enable a form to validate textfield for email, simply set #mailgun_email in the form array to TRUE.

  $form['custom_email'] = array(
    '#title' => t('Email Address'),
    '#type' => 'textfield',
    '#description' => t('Custom Mailgun Email Validator Textfield.'),
    '#mailgun_email' => TRUE,
  );

Example can be find at /mailgun_email_validator_example url after installation.

Maintainer

This project is originally written by Sandeep Kumbhatil along with co-maintainer Avinash Shukla

Project Page

This is the project page link for Mailgun Email Validator

Clone Repository

git clone --branch 7.x-1.x http://git.drupal.org/sandbox/sandykumbhatil/2428499.git mailgun_email_validator

The Project Application Review Link

https://www.drupal.org/node/2442161#comment-9667211
https://www.drupal.org/node/2411257#comment-9560251
https://www.drupal.org/node/2442161#comment-9687731
https://www.drupal.org/node/2452353#comment-9728409
https://www.drupal.org/node/2442161#comment-9732139
https://www.drupal.org/node/2454221#comment-9740271

Comment	File	Size	Author
#5	notice.JPG	110.56 KB	cmak

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

sandeep.kumbhatil CreditAttribution: sandeep.kumbhatil commented 26 February 2015 at 10:27

Status:

Needs work

» Needs review

Comment #2

PA robot CreditAttribution: PA robot commented 26 February 2015 at 11:23

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

Comment #3

sandeep.kumbhatil CreditAttribution: sandeep.kumbhatil commented 26 February 2015 at 16:22

Issue summary:

View changes

Comment #4

sandeep.kumbhatil CreditAttribution: sandeep.kumbhatil commented 27 February 2015 at 05:51

Issue summary:

View changes

Comment #5

cmak CreditAttribution: cmak commented 27 February 2015 at 11:11

File	Size
notice.JPG	110.56 KB

Please find my review below:

Automated Review

No issues found on http://pareview.sh/

Manual Review

Individual user account

Yes: Follows.

No duplication

Yes: Does not cause.

Master Branch

Yes: Follows.

Licensing

Yes: Follows.

3rd party assets/code

Yes: Follows.

README.txt/README.md

Yes: Follows.

Code long/complex enough for review

Yes: Follows.

Secure code

Yes: Meets the security requirements.

Coding style & Drupal API usage

Just a recommendation : Should fix a Notice thrown while adding a new E-Mail field. Refer attached image file.
Just a recommendation : Very few inline comments.

The starred items (*) are fairly big issues and warrant going back to Needs Work. Items marked with a plus sign (+) are important and should be addressed before a stable project release. The rest of the comments in the code walkthrough are recommendations.

If added, please don't remove the security tag, we keep that for statistics and to show examples of security problems.

This review uses the Project Application Review Template.

Comment #6

sandeep.kumbhatil CreditAttribution: sandeep.kumbhatil commented 28 February 2015 at 14:05

Issue summary:

View changes

Comment #7

sandeep.kumbhatil CreditAttribution: sandeep.kumbhatil commented 28 February 2015 at 14:06

Hi Makarand,

Thanks for reviewing the project, I have resolved issues you have mentioned.

Thank a lot.

Comment #8

vingborg CreditAttribution: vingborg commented 1 March 2015 at 01:29

Status:

Needs review

» Reviewed & tested by the community

The module seems to follow all normal conventions, as pointed out in #5.

There are no serious issues, but a few minor ones:

1) In the install module, setting "webform_allowed_tags" is a bit brutish. What if it's already set? It should read the existing values, if any, and add to that.

2) In the function "mailgun_email_validator_validate_email_field", it could theoretically call the HTTP service a lot of times, incurring long response times for the user. This is probably rare, but it *is* an issue. Does the Mailgun validator have a bulk validation option? If so, that might be more appropriate.

3) The three validate callback functions in the module files are very similar. Maybe a little refactoring? Just a recommendation.

All in all, nothing serious. It could work out of the box.

Comment #9

sandeep.kumbhatil CreditAttribution: sandeep.kumbhatil commented 2 March 2015 at 05:09

Hi @vingborg,

Thanks for review. Following are my views on your suggestions,

1. Yes I have worked on this added my html tags to "webform_allowed_tags" variable keeping already added tags untouched.

2. As for bulk validation, unfortunately Mailgun does not allow bulk validation. As per the statement on mailgun validation page.

It is not intended to be used for bulk email list scrubbing and we reserve the right to disable your account if we see it being used as such.

Refer : https://documentation.mailgun.com/api-email-validation.html

3. Keeping these functions separate for code readability, I'll try to find some way to reduce the code there.

Comment #10

vingborg CreditAttribution: vingborg commented 2 March 2015 at 10:34

Hi sandeep

No problem.

As for 1 and 2: that settles it :-)

As for 3: That's my own taste. It is not a problem for the module as such.

Comment #11

sandeep.kumbhatil CreditAttribution: sandeep.kumbhatil commented 2 March 2015 at 12:32

Issue summary:

View changes

Comment #12

nashkrammer CreditAttribution: nashkrammer commented 5 March 2015 at 07:48

Issue summary:	View changes
Issue tags:		+PAreview: review bonus

Comment #13

klausi

he/him||they/them

German

🇦🇹 Vienna

CreditAttribution: klausi commented 7 March 2015 at 10:07

Status:	Reviewed & tested by the community	» Needs work
Issue tags:	-PAreview: single application approval, -PAreview: review bonus	+PAreview: security

manual review:

mailgun_email_validator.info: why do you need to add your CSS to every single page request? Shouldn't you only add it with your form element or where you actually need it? Please add a comment.
mailgun_email_validator_form_alter(): instead of mixing the logic for different form IDs here you should use hook_form_FORM_ID_alter() to target the individual forms.
mailgun_email_validator_validate_email_field(): using LANGUAGE_NONE here will break for multilingual sites. You should get the field language, probably form the current entity and use that.
mailgun_email_validator_validate_email_field(): this vulnerable to XSS exploits if the response in $response['did_you_mean'] contains malicious text. External services need to be treated as untrusted user provided input and you need to sanitize that accordingly. Make sure to read https://www.drupal.org/node/28984 again. Same in mailgun_email_validator_validate_webform_field(). And please don't remove the security tag, we keep that for statistics and to show examples of security problems.

Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

I think this project is long enough to be eligible for the git vetted user role, so removing the single project promote tag.

Comment #14

sandeep.kumbhatil CreditAttribution: sandeep.kumbhatil commented 10 March 2015 at 06:23

Status:

Needs work

» Needs review

Hi Klausi,

Thank you for the review. I have worked on the issues you have mentioned. Following are my comments for the same.

1. Removed CSS from the info file and added to the #attached of the form so it will load CSS only when Mailgun needs it.
2. I have separated the form_alters with hook_form_FORM_ID_alter() as suggested now the code is more readable.
3. Removed LANGUAGE_NONE and replaced it with entity language.
4. Thank you for marking this issue. The API response is now passed through filter_xss and all response variables are passed through check_plain in order to sanitize the response output.

I am adding this to needs review and maintained the security tag as mentioned.

Thank You.

Comment #15

Shashwat Purav CreditAttribution: Shashwat Purav commented 13 March 2015 at 11:33

Automated Review

[Best practice issues identified by pareview.sh / drupalcs / coder. Please don't copy/paste all of the results unless they are short. If there are a lot, then post a link to the automated review and mention that problems should be addressed.]

Manual Review

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

Yes: Does not cause duplication and/or fragmentation.

Master Branch

Yes: Follows the guidelines for master branch.

Licensing

Yes: Follows the licensing requirements.

3rd party assets/code

Yes: Follows the guidelines for 3rd party assets/code.

README.txt/README.md

Yes: Follows the guidelines for in-project documentation and/or the README Template.

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity.

Secure code

Yes: Meets the security requirements.

Coding style & Drupal API usage

[List of identified issues in no particular order. Use (*) and (+) to indicate an issue importance. Replace the text below by the issues themselves:

Please mention "Email" CCK field is required in case user is not using webforms so that the module documentation will not confuse beginners.
This module otherwise looks RTBC to me.

If added, please don't remove the security tag, we keep that for statistics and to show examples of security problems.

This review uses the Project Application Review Template.

Comment #16

sandeep.kumbhatil CreditAttribution: sandeep.kumbhatil commented 13 March 2015 at 12:22

Hi Shashwat Purav,

Thanks for the suggestion.

Since module can be used as standalone for custom forms and as a extension for Email module field widget or a Webform email field, I am not adding dependency with other modules, rather as per your suggestion, I am editing the readme for clarity on steps to use it with Email field widget module and Webform module.

Comment #17

sandeep.kumbhatil CreditAttribution: sandeep.kumbhatil commented 13 March 2015 at 12:41

Issue summary:

View changes

Comment #18

sandeep.kumbhatil CreditAttribution: sandeep.kumbhatil commented 17 March 2015 at 10:25

Issue summary:

View changes

Comment #19

VikrantR CreditAttribution: VikrantR commented 17 March 2015 at 11:23

Status:

Needs review

» Needs work

Automated Review

No issues found on http://pareview.sh/

Manual Review

Individual user account

Yes: Follows.

No duplication

Yes: Does not cause.

Master Branch

Yes: Follows.

Licensing

Yes: Follows.

3rd party assets/code

Yes: Follows.

README.txt/README.md

Yes: Follows.

Code long/complex enough for review

Yes: Follows. the guidelines for project length and complexity.

Secure code

Yes: Meets the security requirements.

Coding style & Drupal API usage

(*) As per comment #13 : function mailgun_email_validator_validate_custom_email_field() is still vulnerable to XSS exploits, Please update it.
Just a recommendation, please update git clone command to : git clone --branch 7.x-1.x http://git.drupal.org/sandbox/sandykumbhatil/2428499.git mailgun_email_validator
Please add comment #16 information in README file also, so there will be no confusion for beginners.
Please add "Open ssl extension required information" in README file

If added, please don't remove the security tag, we keep that for statistics and to show examples of security problems.

This review uses the Project Application Review Template.

Comment #20

sandeep.kumbhatil CreditAttribution: sandeep.kumbhatil commented 17 March 2015 at 11:36

Issue summary:

View changes

Comment #21

sandeep.kumbhatil CreditAttribution: sandeep.kumbhatil commented 17 March 2015 at 11:48

HI Vikrant Ramteke,

Thank you for review this project, I have resolved all the issue you have mentioned. Kindly review the same.

Thanks a lot.. :)

Comment #22

sandeep.kumbhatil CreditAttribution: sandeep.kumbhatil commented 17 March 2015 at 11:49

Issue summary:

View changes

Comment #23

sandeep.kumbhatil CreditAttribution: sandeep.kumbhatil commented 17 March 2015 at 11:50

Status:

Needs work

» Needs review

Comment #24

VikrantR CreditAttribution: VikrantR commented 17 March 2015 at 13:56

Status:

Needs review

» Reviewed & tested by the community

Thanks Sandeep,

Now this looks RTBC for me.

Comment #25

sandeep.kumbhatil CreditAttribution: sandeep.kumbhatil commented 18 March 2015 at 06:03

Issue summary:

View changes

Comment #26

sandeep.kumbhatil CreditAttribution: sandeep.kumbhatil commented 20 March 2015 at 06:02

Issue summary:	View changes
Issue tags:		+PAreview: review bonus

Comment #27

er.pushpinderrana CreditAttribution: er.pushpinderrana commented 21 March 2015 at 07:21

Assigned:

Unassigned

» er.pushpinderrana

Assigning to myself for next review.

Comment #28

er.pushpinderrana CreditAttribution: er.pushpinderrana commented 23 March 2015 at 10:14

Assigned:	er.pushpinderrana	» Unassigned
Status:	Reviewed & tested by the community	» Fixed

Automated Review

Best practice issues identified by pareview.sh / drupalcs / coder. Yes, one issue found.
Review of the 7.x-1.x branch (commit 3310e80):

FILE: ...ar/www/drupal-7-pareview/pareview_temp/mailgun_email_validator.module
---------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
---------------------------------------------------------------------------
70 | ERROR | The $text argument to l() should be enclosed within t() so
| | that it is translatable
---------------------------------------------------------------------------

Manual Review

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

Yes: Does not cause module duplication and/or fragmentation.

Master Branch

Yes: Follows the guidelines for master branch.

Licensing

Yes: Follows the licensing requirements.

3rd party assets/code

Yes: Follows the guidelines for 3rd party assets/code.

README.txt/README.md

Yes: Follows the guidelines for in-project documentation and/or the README Template.

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity.

Secure code

Yes: Meets the security requirements.

Coding style & Drupal API usage

List of identified issues in no particular order. Use (*) and (+) to indicate an issue importance. Replace the text below by the issues themselves:

mailgun_email_validator_form_webform_component_edit_form_alter(): arg() is evil, and should almost always be avoided.
mailgun_email_validator_credential_form(): do not create link markup yourself, use url() or l() instead.
(+) $response_data = filter_xss($raw_response->data);: filter_xss() is not needed here as you are using check_plain() later to escape data where actual data is printing i.e check_plain($response['did_you_mean']).
...]

But that are not critical application blockers, so...

Thanks for your contribution, Sandeep Kumbhatil!

I updated your account so you can promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and stay involved!

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thanks to the dedicated reviewer(s) as well.

Comment #29

6 April 2015 at 10:14

Status:

Fixed

» Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.