FIX: Add better and more strict invite validators (#18399)

* FIX: Add validator for email xor domain

* FIX: Add validator for max_redemptions_allowed

* FIX: Add validator for redemption_count

app/models/invite.rb

@@ -31,8 +31,10 @@ class Invite < ActiveRecord::Base
   validates_presence_of :invited_by_id
   validates :email, email: true, allow_blank: true
   validate :ensure_max_redemptions_allowed
+  validate :valid_redemption_count
   validate :valid_domain, if: :will_save_change_to_domain?
   validate :user_doesnt_already_exist, if: :will_save_change_to_email?
+  validate :email_xor_domain
 
   before_create do
     self.invite_key ||= SecureRandom.base58(10)
@@ -66,6 +68,12 @@ class Invite < ActiveRecord::Base
     end
   end
 
+  def email_xor_domain
+    if email.present? && domain.present?
+      errors.add(:base, I18n.t('invite.email_xor_domain'))
+    end
+  end
+
   def is_invite_link?
     email.blank?
   end
@@ -253,12 +261,20 @@ class Invite < ActiveRecord::Base
       limit = invited_by&.staff? ? SiteSetting.invite_link_max_redemptions_limit
                                  : SiteSetting.invite_link_max_redemptions_limit_users
 
-      if !self.max_redemptions_allowed.between?(1, limit)
+      if self.email.present? && self.max_redemptions_allowed != 1
+        errors.add(:max_redemptions_allowed, I18n.t("invite.max_redemptions_allowed_one"))
+      elsif !self.max_redemptions_allowed.between?(1, limit)
         errors.add(:max_redemptions_allowed, I18n.t("invite_link.max_redemptions_limit", max_limit: limit))
       end
     end
   end
 
+  def valid_redemption_count
+    if self.redemption_count > self.max_redemptions_allowed
+      errors.add(:redemption_count, I18n.t("invite.redemption_count_less_than_max", max_redemptions_allowed: self.max_redemptions_allowed))
+    end
+  end
+
   def valid_domain
     return if self.domain.blank?
 

config/locales/server.en.yml

@@ -264,6 +264,9 @@ en:
       invalid_access: "You are not permitted to view the requested resource."
     requires_groups: "Invite was not saved because the specified topic is inaccessible. Add one of the following groups: %{groups}."
     domain_not_allowed: "Your email cannot be used to redeem this invite."
+    max_redemptions_allowed_one: "for email invites should be 1."
+    redemption_count_less_than_max: "should be less than %{max_redemptions_allowed}."
+    email_xor_domain: "Email and domain fields are not allowed at the same time"
 
   bulk_invite:
     file_should_be_csv: "The uploaded file should be of csv format."

spec/models/invite_spec.rb

@@ -41,12 +41,33 @@ RSpec.describe Invite do
     end
 
     it 'allows only valid domains' do
-      invite = Fabricate.build(:invite, domain: 'example', invited_by: user)
+      invite = Fabricate.build(:invite, email: nil, domain: 'example', invited_by: user)
       expect(invite).not_to be_valid
 
-      invite = Fabricate.build(:invite, domain: 'example.com', invited_by: user)
+      invite = Fabricate.build(:invite, email: nil, domain: 'example.com', invited_by: user)
       expect(invite).to be_valid
     end
+
+    it 'allows only email or only domain to be present' do
+      invite = Fabricate.build(:invite, email: nil, invited_by: user)
+      expect(invite).to be_valid
+
+      invite = Fabricate.build(:invite, email: nil, domain: 'example.com', invited_by: user)
+      expect(invite).to be_valid
+
+      invite = Fabricate.build(:invite, email: 'test@example.com', invited_by: user)
+      expect(invite).to be_valid
+
+      invite = Fabricate.build(:invite, email: 'test@example.com', domain: 'example.com', invited_by: user)
+      expect(invite).not_to be_valid
+      expect(invite.errors.full_messages).to include(I18n.t('invite.email_xor_domain'))
+    end
+
+    it 'checks if redemption_count is less or equal than max_redemptions_allowed' do
+      invite = Fabricate.build(:invite, redemption_count: 2, max_redemptions_allowed: 1, invited_by: user)
+      expect(invite).not_to be_valid
+      expect(invite.errors.full_messages.first).to include(I18n.t('invite.redemption_count_less_than_max', max_redemptions_allowed: 1))
+    end
   end
 
   describe 'before_save' do
@@ -355,7 +376,7 @@ RSpec.describe Invite do
     fab!(:inviter) { Fabricate(:user) }
 
     fab!(:pending_invite) { Fabricate(:invite, invited_by: inviter, email: 'pending@example.com') }
-    fab!(:pending_link_invite) { Fabricate(:invite, invited_by: inviter, max_redemptions_allowed: 5) }
+    fab!(:pending_link_invite) { Fabricate(:invite, invited_by: inviter, email: nil, max_redemptions_allowed: 5) }
     fab!(:pending_invite_from_another_user) { Fabricate(:invite) }
 
     fab!(:expired_invite) { Fabricate(:invite, invited_by: inviter, email: 'expired@example.com', expires_at: 1.day.ago) }