Add global Spree::Config.default_email_regexp #4022

cesartalves · 2021-04-09T19:09:54Z

This change adds a :default_email_regexp preference to be used by Spree::EmailValidator and also solidus_auth_devise, allowing the regex to be fully and easily customizable. With this introduction it will be possible to keep the Devise.email_regexp in sync with solidus's email validator regexp.

Description

Checklist:

I have followed Pull Request guidelines
I have added a detailed description into each commit message
I have updated Guides and README accordingly to this change (if needed)
I have added tests to cover this change (if needed)
I have attached screenshots to this PR for visual changes (if needed)

softr8 · 2021-04-09T21:12:32Z

core/lib/spree/app_configuration.rb

@@ -145,6 +145,10 @@ class AppConfiguration < Preferences::Configuration
    #   @return [String] Two-letter ISO code of a {Spree::Country} to assumed as the country of an unidentified customer (default: "US")
    preference :default_country_iso, :string, default: 'US'

+    # @!attribute [rw] default_email_regexp
+    #   @return [Regexp] Regex to be used in email validations, for example in Spree::EmailValidator
+    preference :default_email_regexp, :regexp, default: /\A([^@\.]|[^@\.]([^@\s]*)[^@\.])@([^@\s]+\.)+[^@\s]+\z/


Might be worth trying with Ruby's built in URI::MailTo::EMAIL_REGEXP

This is just a copy of the regex in Spree::EmailValidator. That regex you mentioned is significantly different from it and I think changing it might have a potential impact for unaware upgraders.

I think it might be worth considering an update of the regex because the current one has a couple of bugs.

At this link you can see and play with different combinations (be aware of the usage of ^ and $ instead of \A and \z, it was just to use multiline matches), but looks like that the current regex allows the following:

test @example.com: single whitespace before the @

test@example.com: leading whitespace, although this is a smaller issue since Devise by default auto strips leading and trailing whitespaces on email fields. The Spree::User class in solidus_auth_devise is immune from this bug because of the auto stripping.

Since this would enter a deprecation cycle, would it be ok updating the regex to something like this? Resulting in this example.

/\A([^@\s.]|[^@\s.]([^@\s]*)[^@\s.])@([^@\s]+\.)+[^@\s]+\z/

@cesartalves , @kennyadsl , @softr8 thoughts?

I see no problem in updating the regex. We can let users know that it's gonna happen on a release note. Besides, shouldn't break a lot of validations save few edge cases (e.g., white spaces as you mentioned).

core/lib/spree/core/validators/email.rb

spaghetticode · 2021-05-07T09:39:14Z

core/lib/spree/core/validators/email.rb

+    include ActiveSupport::Deprecation::DeprecatedConstantAccessor
+
+    SPREE_EMAIL_REGEXP = Spree::Config.default_email_regexp
+    deprecate_constant 'EMAIL_REGEXP', "#{name}::SPREE_EMAIL_REGEXP"


If I am understanding this correctly, we're introducing another constant, SPREE_EMAIL_REGEXP, for the sake of being able to deprecate EMAIL_REGEXP, cause ActiveSupport's version of deprecate_constant requires a replacement constant.

If that's the case, we can avoid this by simply using ruby's Module.deprecate_constant from stdlib which is happy with just deprecating the constant, without replacement.

Also, if we're switching to Module.deprecate_constant, since we cannot customize the deprecation message, I guess it would be nice to add a comment making super-clear that the replacement for the constant is Spree::Config.default_email_regexp... I guess that devs checking this file after seeing the deprecation may be a bit confused.

waiting-for-dev · 2021-11-08T08:27:07Z

Hey! Any particular reason why we're closing it? We could handle the change in value thanks to the new Solidus update process. I.e.:

preference :default_email_regexp, :regexp, default: by_version(/\A([^@\.]|[^@\.]([^@\s]*)[^@\.])@([^@\s]+\.)+[^@\s]+\z/, "3.2.alpha" => URI::MailTo::EMAIL_REGEXP)

Reopening to keep visibility on it.

The denial of service vulnerability could be exploited in the checkout process for guest orders, as it's the only place where that regexp is used. To this point, registered users' emails (for instance, through solidus_auth_devise) don't validate against that regular expression. The previous regular expression had `([^@\s]+\.)+` bit on it. It was susceptible to exponential backtracking with a substring like `a.a.`. I.e.: ``` irb(main)> Spree::EmailValidator::EMAIL_REGEXP.match "a@a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.@" processing time: 54.293660s => nil ``` We take the occasion to remove the crafted regexp in use altogether. Instead, we use `URI::MailTo::REGEXP`, which follows https://html.spec.whatwg.org/multipage/input.html#valid-e-mail-address. It's improbable to find old emails that become invalid in a system, but, as a safeguard, we provide a task `solidus:check_orders_with_invalid_emails` to print information for orders that don't fulfill the new requirement. The issue has been present since 7af4b93. I.e., since version 1.0. The regexp had been copied from Devise's one, but they updated it in heartcombo/devise@830d3e8 References: GHSA-qxmr-qxh6-2cc9 https://en.wikipedia.org/wiki/ReDoS https://snyk.io/blog/redos-and-catastrophic-backtracking/ References #4022

waiting-for-dev · 2022-06-14T10:05:22Z

Hey @cesartalves, would you be willing to retake it?

We should:

Rebase from master
Update the regular expression to use URI::MailTo::REGEXP (we updated it to get rid of security vulnerabilities, see ff33a8b).
Address @spaghetticode's concern.

It'll also close #4341.

This change adds a `:default_email_regexp` preference to be used by Spree::EmailValidator, allowing the regex to be fully and easily customizable.

waiting-for-dev · 2022-06-23T12:46:47Z

Hey, I rebased it myself and implemented @spaghetticode's suggestion. It should be ready for review.

spaghetticode

thank you @cesartalves and @waiting-for-dev !

The denial of service vulnerability could be exploited in the checkout process for guest orders, as it's the only place where that regexp is used. To this point, registered users' emails (for instance, through solidus_auth_devise) don't validate against that regular expression. The previous regular expression had `([^@\s]+\.)+` bit on it. It was susceptible to exponential backtracking with a substring like `a.a.`. I.e.: ``` irb(main)> Spree::EmailValidator::EMAIL_REGEXP.match "a@a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.@" processing time: 54.293660s => nil ``` We take the occasion to remove the crafted regexp in use altogether. Instead, we use `URI::MailTo::REGEXP`, which follows https://html.spec.whatwg.org/multipage/input.html#valid-e-mail-address. It's improbable to find old emails that become invalid in a system, but, as a safeguard, we provide a task `solidus:check_orders_with_invalid_emails` to print information for orders that don't fulfill the new requirement. The issue has been present since 7af4b93. I.e., since version 1.0. The regexp had been copied from Devise's one, but they updated it in heartcombo/devise@830d3e8 References: GHSA-qxmr-qxh6-2cc9 https://en.wikipedia.org/wiki/ReDoS https://snyk.io/blog/redos-and-catastrophic-backtracking/ References solidusio#4022

Please, use Spree::Config.default_email_regexp now. Ref solidusio#4022

softr8 reviewed Apr 9, 2021

View reviewed changes

kennyadsl reviewed Apr 12, 2021

View reviewed changes

core/lib/spree/core/validators/email.rb Outdated Show resolved Hide resolved

cesartalves force-pushed the create-email-regexp-configuration branch from 6b44873 to 8b43092 Compare April 19, 2021 12:47

cesartalves requested a review from kennyadsl April 19, 2021 15:07

cesartalves marked this pull request as ready for review April 20, 2021 19:07

spaghetticode reviewed May 7, 2021

View reviewed changes

cesartalves closed this Oct 22, 2021

waiting-for-dev reopened this Nov 8, 2021

Add global Spree::Config.default_email_regexp

5e451b5

This change adds a `:default_email_regexp` preference to be used by Spree::EmailValidator, allowing the regex to be fully and easily customizable.

waiting-for-dev force-pushed the create-email-regexp-configuration branch from 8b43092 to 5e451b5 Compare June 23, 2022 11:51

waiting-for-dev requested a review from a team June 23, 2022 12:47

spaghetticode approved these changes Jun 23, 2022

View reviewed changes

jarednorman approved these changes Jun 24, 2022

View reviewed changes

waiting-for-dev merged commit d65a081 into solidusio:master Jun 27, 2022

kennyadsl added a commit to nebulab/solidus that referenced this pull request Mar 28, 2023

Remove deprecated EmailValidator::EMAIL_REGEXP

c018dca

Please, use Spree::Config.default_email_regexp now. Ref solidusio#4022

kennyadsl added a commit to nebulab/solidus that referenced this pull request Mar 28, 2023

Remove deprecated EmailValidator::EMAIL_REGEXP

7f173b2

Please, use Spree::Config.default_email_regexp now. Ref solidusio#4022

kennyadsl added a commit to nebulab/solidus that referenced this pull request Mar 28, 2023

Remove deprecated EmailValidator::EMAIL_REGEXP

ec8a3ee

Please, use Spree::Config.default_email_regexp now. Ref solidusio#4022

kennyadsl added a commit to nebulab/solidus that referenced this pull request Mar 28, 2023

Remove deprecated EmailValidator::EMAIL_REGEXP

ed07baa

Please, use Spree::Config.default_email_regexp now. Ref solidusio#4022

kennyadsl added a commit to nebulab/solidus that referenced this pull request Apr 12, 2023

Remove deprecated EmailValidator::EMAIL_REGEXP

7038528

Please, use Spree::Config.default_email_regexp now. Ref solidusio#4022

kennyadsl added a commit to nebulab/solidus that referenced this pull request Apr 14, 2023

Remove deprecated EmailValidator::EMAIL_REGEXP

9ad985e

Please, use Spree::Config.default_email_regexp now. Ref solidusio#4022

kennyadsl added a commit to nebulab/solidus that referenced this pull request Apr 18, 2023

Remove deprecated EmailValidator::EMAIL_REGEXP

95bae85

Please, use Spree::Config.default_email_regexp now. Ref solidusio#4022

kennyadsl added a commit to nebulab/solidus that referenced this pull request Apr 18, 2023

Remove deprecated EmailValidator::EMAIL_REGEXP

7aa7cc2

Please, use Spree::Config.default_email_regexp now. Ref solidusio#4022

kennyadsl added a commit to nebulab/solidus that referenced this pull request Apr 19, 2023

Remove deprecated EmailValidator::EMAIL_REGEXP

f836f61

Please, use Spree::Config.default_email_regexp now. Ref solidusio#4022

kennyadsl added a commit to nebulab/solidus that referenced this pull request Apr 24, 2023

Remove deprecated EmailValidator::EMAIL_REGEXP

e2923f6

Please, use Spree::Config.default_email_regexp now. Ref solidusio#4022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add global Spree::Config.default_email_regexp #4022

Add global Spree::Config.default_email_regexp #4022

cesartalves commented Apr 9, 2021 •

edited

Loading

softr8 Apr 9, 2021

cesartalves Apr 12, 2021

nirebu May 4, 2021

cesartalves May 4, 2021

spaghetticode May 7, 2021 •

edited

Loading

waiting-for-dev commented Nov 8, 2021

waiting-for-dev commented Jun 14, 2022 •

edited

Loading

waiting-for-dev commented Jun 23, 2022 •

edited

Loading

spaghetticode left a comment

Add global Spree::Config.default_email_regexp #4022

Add global Spree::Config.default_email_regexp #4022

Conversation

cesartalves commented Apr 9, 2021 • edited Loading

softr8 Apr 9, 2021

Choose a reason for hiding this comment

cesartalves Apr 12, 2021

Choose a reason for hiding this comment

nirebu May 4, 2021

Choose a reason for hiding this comment

cesartalves May 4, 2021

Choose a reason for hiding this comment

spaghetticode May 7, 2021 • edited Loading

Choose a reason for hiding this comment

waiting-for-dev commented Nov 8, 2021

waiting-for-dev commented Jun 14, 2022 • edited Loading

waiting-for-dev commented Jun 23, 2022 • edited Loading

spaghetticode left a comment

Choose a reason for hiding this comment

cesartalves commented Apr 9, 2021 •

edited

Loading

spaghetticode May 7, 2021 •

edited

Loading

waiting-for-dev commented Jun 14, 2022 •

edited

Loading

waiting-for-dev commented Jun 23, 2022 •

edited

Loading