FIX: delete duplicate invites earlier in the process

approved
heisentest
#1

FIX: delete duplicate invites earlier in the process

There was a race condition when 2 invites existed for 1 user where in some cases data from both invites would be used for the redeem. Depending on DB ordering.

Fix is to delete duplicate invites earlier in the process prior to redeem_from_email being called.

diff --git a/app/models/invite.rb b/app/models/invite.rb
index fdd0f9b..0f23e58 100644
--- a/app/models/invite.rb
+++ b/app/models/invite.rb
@@ -56,7 +56,9 @@ class Invite < ActiveRecord::Base
   end
 
   def redeem(username: nil, name: nil, password: nil, user_custom_fields: nil, ip_address: nil)
-    InviteRedeemer.new(self, username, name, password, user_custom_fields, ip_address).redeem unless expired? || destroyed? || !link_valid?
+    if !expired? && !destroyed? && link_valid?
+      InviteRedeemer.new(self, username, name, password, user_custom_fields, ip_address).redeem
+    end
   end
 
   def self.invite_by_email(email, invited_by, topic = nil, group_ids = nil, custom_message = nil)
diff --git a/app/models/invite_redeemer.rb b/app/models/invite_redeemer.rb
index 97cd194..d41662b 100644
--- a/app/models/invite_redeemer.rb
+++ b/app/models/invite_redeemer.rb
@@ -80,7 +80,6 @@ InviteRedeemer = Struct.new(:invite, :username, :name, :password, :user_custom_f
     add_user_to_groups
     send_welcome_message
     notify_invitee
-    delete_duplicate_invites
   end
 
   def invite_was_redeemed?
@@ -89,9 +88,15 @@ InviteRedeemer = Struct.new(:invite, :username, :name, :password, :user_custom_f
   end
 
   def mark_invite_redeemed
-    Invite.where('id = ? AND redeemed_at IS NULL AND created_at >= ?',
+    count = Invite.where('id = ? AND redeemed_at IS NULL AND created_at >= ?',
                  invite.id, SiteSetting.invite_expiry_days.days.ago)
       .update_all('redeemed_at = CURRENT_TIMESTAMP')
+
+    if count == 1
+      delete_duplicate_invites
+    end
+
+    count
   end
 
   def get_invited_user
diff --git a/spec/models/invite_spec.rb b/spec/models/invite_spec.rb
index eaecfd5..8f1136e 100644
--- a/spec/models/invite_spec.rb
+++ b/spec/models/invite_spec.rb
@@ -206,7 +206,7 @@ describe Invite do
     end
 
     it 'wont redeem an expired invite' do
-      SiteSetting.expects(:invite_expiry_days).returns(10)
+      SiteSetting.invite_expiry_days = 10
       invite.update_column(:created_at, 20.days.ago)
       expect(invite.redeem).to be_blank
     end

GitHub sha: d643294c

1 Like
#2

FYI @techAPJ … very interesting heisentest relies on inconsistent ordering of:

Invite.find_by(email: ...some...email...)
2 Likes
Approved #3