Add comprehensive email validation for blocked users

[ShashankFC]

Test 3

Summary by CodeRabbit

Release Notes

New Features

• Added email blocking mechanism for account creation that prevents specific email addresses from being used
• Enhanced email validation to track and reject previously failed emails
• Improved account creation error responses with detailed validation information

Database

• New database table added to persist blocked email configurations and tracking data

_{✏️ Tip: You can customize this high-level summary in your review settings.}

---

Replicated from ai-code-review-evaluation/discourse-coderabbit#3

[ShashankFC]

@cubic-dev-ai review this pull request

[cubic-dev-ai]

@cubic-dev-ai review this pull request

@ShashankFC I have started the AI code review. It will take a few minutes to complete.

[cubic-dev-ai]

4 issues found across 10 files

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="app/models/blocked_email.rb">

<violation number="1" location="app/models/blocked_email.rb:12">
P1: Email lookup is case-sensitive, allowing blocked emails to be bypassed by changing letter casing. Email addresses should be compared case-insensitively. Downcase the email before querying (and consider adding a `before_validation { self.email = email&.downcase }` callback to normalize on save).</violation>

<violation number="2" location="app/models/blocked_email.rb:14">
P2: Non-atomic read-modify-write on `match_count` causes a race condition under concurrent requests, and `save` unnecessarily runs validations/callbacks for a simple tracking update. Use an atomic update instead.</violation>
</file>

<file name="app/assets/javascripts/discourse/controllers/create_account_controller.js">

<violation number="1" location="app/assets/javascripts/discourse/controllers/create_account_controller.js:17">
P2: Shared mutable default on prototype: arrays/objects defined directly in `.extend()` are shared across all instances. This is a classic Ember anti-pattern. Initialize it in `init()` instead to ensure each instance gets its own array.</violation>
</file>

<file name="lib/validators/email_validator.rb">

<violation number="1" location="lib/validators/email_validator.rb:19">
P1: **Regex injection risk**: The setting value is interpolated into a regex with only dots escaped. Other regex metacharacters (e.g., `+`, `*`, `?`, `(`, `)`) will be interpreted as regex syntax, which can lead to ReDoS or unintended matching. The code also implicitly relies on raw `|` characters in the setting to serve as regex alternation, which is fragile.

Split the setting into individual domains, escape each properly with `Regexp.escape`, then join with `|` for alternation.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[cubic-dev-ai]

P1: Email lookup is case-sensitive, allowing blocked emails to be bypassed by changing letter casing. Email addresses should be compared case-insensitively. Downcase the email before querying (and consider adding a before_validation { self.email = email&.downcase } callback to normalize on save).

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At app/models/blocked_email.rb, line 12:

<comment>Email lookup is case-sensitive, allowing blocked emails to be bypassed by changing letter casing. Email addresses should be compared case-insensitively. Downcase the email before querying (and consider adding a `before_validation { self.email = email&.downcase }` callback to normalize on save).</comment>

<file context>
@@ -0,0 +1,25 @@
+  end
+
+  def self.should_block?(email)
+    record = BlockedEmail.where(email: email).first
+    if record
+      record.match_count += 1
</file context>

[cubic-dev-ai]

P1: Regex injection risk: The setting value is interpolated into a regex with only dots escaped. Other regex metacharacters (e.g., +, *, ?, (, )) will be interpreted as regex syntax, which can lead to ReDoS or unintended matching. The code also implicitly relies on raw | characters in the setting to serve as regex alternation, which is fragile.

Split the setting into individual domains, escape each properly with Regexp.escape, then join with | for alternation.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At lib/validators/email_validator.rb, line 19:

<comment>**Regex injection risk**: The setting value is interpolated into a regex with only dots escaped. Other regex metacharacters (e.g., `+`, `*`, `?`, `(`, `)`) will be interpreted as regex syntax, which can lead to ReDoS or unintended matching. The code also implicitly relies on raw `|` characters in the setting to serve as regex alternation, which is fragile.

Split the setting into individual domains, escape each properly with `Regexp.escape`, then join with `|` for alternation.</comment>

<file context>
@@ -0,0 +1,24 @@
+  end
+
+  def email_in_restriction_setting?(setting, value)
+    domains = setting.gsub('.', '\.')
+    regexp = Regexp.new("@(#{domains})", true)
+    value =~ regexp
</file context>

[cubic-dev-ai]

P2: Non-atomic read-modify-write on match_count causes a race condition under concurrent requests, and save unnecessarily runs validations/callbacks for a simple tracking update. Use an atomic update instead.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At app/models/blocked_email.rb, line 14:

<comment>Non-atomic read-modify-write on `match_count` causes a race condition under concurrent requests, and `save` unnecessarily runs validations/callbacks for a simple tracking update. Use an atomic update instead.</comment>

<file context>
@@ -0,0 +1,25 @@
+  def self.should_block?(email)
+    record = BlockedEmail.where(email: email).first
+    if record
+      record.match_count += 1
+      record.last_match_at = Time.zone.now
+      record.save
</file context>

[cubic-dev-ai]

P2: Shared mutable default on prototype: arrays/objects defined directly in .extend() are shared across all instances. This is a classic Ember anti-pattern. Initialize it in init() instead to ensure each instance gets its own array.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At app/assets/javascripts/discourse/controllers/create_account_controller.js, line 17:

<comment>Shared mutable default on prototype: arrays/objects defined directly in `.extend()` are shared across all instances. This is a classic Ember anti-pattern. Initialize it in `init()` instead to ensure each instance gets its own array.</comment>

<file context>
@@ -14,6 +14,7 @@ Discourse.CreateAccountController = Discourse.Controller.extend(Discourse.ModalF
   accountPasswordConfirm: 0,
   accountChallenge: 0,
   formSubmitted: false,
+  rejectedEmails: Em.A([]),
 
   submitDisabled: function() {
</file context>