CVE-2022-39385: Incorrect Authorization (CWE-863)
value: Discourse is the an open source discussion platform. In some rare cases users redeeming an invitation can be added as a participant to several private message topics that they should not be added to. They are not notified of this, it happens transparently in the background. This issue has been resolved in commit `a414520742` and will be included in future releases. Users are advised to upgrade. Users are also advised to set `SiteSetting.max_invites_per_day` to 0 until the patch is installed.
CWE description: The product performs an authorization check when an actor attempts to access a resource or perform an action, but it does not correctly perform the check. This allows attackers to bypass intended access restrictions.
CVSS v3 base score: 6.5
Published: 2022-11-14T21:15Z
Repository: https://github.com/discourse/discourse
Language: Ruby
Files changed: app/controllers/session_controller.rb, app/models/invite.rb, app/models/invite_redeemer.rb, db/migrate/20221103051248_remove_invalid_topic_allowed_users_from_invites.rb, spec/models/invite_redeemer_spec.rb, spec/requests/invites_controller_spec.rb
Commit date: 2022-11-14 12:02:06 +1000
Commit message: SECURITY: Prevent email from being nil in InviteRedeemer (#19004)

This commit adds some protections in InviteRedeemer to ensure that email
can never be nil, which could cause issues with inviting the invited
person to private topics since there was an incorrect inner join.

If the email is nil and the invite is scoped to an email, we just use
that invite.email unconditionally.  If a redeeming_user (an existing
user) is passed in when redeeming an email, we use their email to
override the passed in email.  Otherwise we just use the passed in
email.  We now raise an error after all this if the email is still nil.
This commit also adds some tests to catch the private topic fix, and
some general improvements and comments around the invite code.

This commit also includes a migration to delete TopicAllowedUser records
for users who were mistakenly added to topics as part of the invite
redemption process.
Diff: diff --git a/app/controllers/session_controller.rb b/app/controllers/session_controller.rb
index d05ba4351f6c..490f4db0ed7b 100644
--- a/app/controllers/session_controller.rb
+++ b/app/controllers/session_controller.rb
@@ -758,11 +758,11 @@ class SessionController < ApplicationController
     if invite.blank?
       raise Invite::ValidationFailed.new(I18n.t("invite.not_found", base_url: Discourse.base_url))
     end
 
     if invite.redeemable?
-      if !invite.is_invite_link? && sso.email != invite.email
+      if invite.is_email_invite? && sso.email != invite.email
         raise Invite::ValidationFailed.new(I18n.t("invite.not_matching_email"))
       end
     elsif invite.expired?
       raise Invite::ValidationFailed.new(I18n.t('invite.expired', base_url: Discourse.base_url))
     elsif invite.redeemed?
diff --git a/app/models/invite.rb b/app/models/invite.rb
index 917b486f294e..191070018575 100644
--- a/app/models/invite.rb
+++ b/app/models/invite.rb
@@ -72,12 +72,20 @@ class Invite < ActiveRecord::Base
     if email.present? && domain.present?
       errors.add(:base, I18n.t('invite.email_xor_domain'))
     end
   end
 
+  # Even if a domain is specified on the invite, it still counts as
+  # an invite link.
   def is_invite_link?
-    email.blank?
+    self.email.blank?
+  end
+
+  # Email invites have specific behaviour and it's easier to visually
+  # parse is_email_invite? than !is_invite_link?
+  def is_email_invite?
+    self.email.present?
   end
 
   def redeemable?
     !redeemed? && !expired? && !deleted_at? && !destroyed? && link_valid?
   end
@@ -199,12 +207,10 @@ class Invite < ActiveRecord::Base
     email_token: nil,
     redeeming_user: nil
   )
     return if !redeemable?
 
-    email = self.email if email.blank? && !is_invite_link?
-
     InviteRedeemer.new(
       invite: self,
       email: email,
       username: username,
       name: name,
diff --git a/app/models/invite_redeemer.rb b/app/models/invite_redeemer.rb
index 93496140fa3e..6a7ae4b22a05 100644
--- a/app/models/invite_redeemer.rb
+++ b/app/models/invite_redeemer.rb
@@ -1,7 +1,20 @@
 # frozen_string_literal: true
 
+# NOTE: There are a _lot_ of complicated rules and conditions for our
+# invite system, and the code is spread out through a lot of places.
+# Tread lightly and read carefully when modifying this code. You may
+# also want to look at:
+#
+# * InvitesController
+# * SessionController
+# * Invite model
+# * User model
+#
+# Invites that are scoped to a specific email (email IS NOT NULL on the Invite
+# model) have different rules to invites that are considered an "invite link",
+# (email IS NULL) on the Invite model.
 class InviteRedeemer
   attr_reader :invite,
     :email,
     :username,
     :name,
@@ -11,31 +24,31 @@ class InviteRedeemer
     :session,
     :email_token,
     :redeeming_user
 
   def initialize(
-    invite: nil,
+    invite:,
     email: nil,
     username: nil,
     name: nil,
     password: nil,
     user_custom_fields: nil,
     ip_address: nil,
     session: nil,
     email_token: nil,
     redeeming_user: nil)
-
     @invite = invite
-    @email = email
     @username = username
     @name = name
     @password = password
     @user_custom_fields = user_custom_fields
     @ip_address = ip_address
     @session = session
     @email_token = email_token
     @redeeming_user = redeeming_user
+
+    ensure_email_is_present!(email)
   end
 
   def redeem
     Invite.transaction do
       if can_redeem_invite? && mark_invite_redeemed
@@ -43,11 +56,33 @@ class InviteRedeemer
         invited_user
       end
     end
   end
 
-  # extracted from User cause it is very specific to invites
+  # The email must be present in some form since many of the methods
+  # for processing + redemption rely on it. If it's still nil after
+  # these checks then we have hit an edge case and should not proceed!
+  def ensure_email_is_present!(email)
+    if email.blank?
+      Rails.logger.warn(
+        "email param was blank in InviteRedeemer for invite ID #{@invite.id}. The `redeeming_user` was #{@redeeming_user.present? ? "(ID: #{@redeeming_user.id})" : "not"} present.",
+      )
+    end
+
+    if email.blank? && @invite.is_email_invite?
+      @email = @invite.email
+    elsif @redeeming_user.present?
+      @email = @redeeming_user.email
+    else
+      @email = email
+    end
+
+    raise Discourse::InvalidParameters if @email.blank?
+  end
+
+  # This will _never_ be called if there is a redeeming_user being passed
+  # in to InviteRedeemer -- see invited_user below.
   def self.create_user_from_invite(email:, invite:, username: nil, name: nil, password: nil, user_custom_fields: nil, ip_address: nil, session: nil, email_token: nil)
     if username && UsernameValidator.new(username).valid_format? && User.username_available?(username, email)
       available_username = username
     else
       available_username = UserNameSuggester.suggest(email)
@@ -105,39 +140,44 @@ class InviteRedeemer
     end
 
     user.save!
     authenticator.finish
 
-    if invite.emailed_status != Invite.emailed_status_types[:not_required] && email == invite.email && invite.email_token.present? && email_token == invite.email_token
+    if invite.emailed_status != Invite.emailed_status_types[:not_required] &&
+        email == invite.email &&
+        invite.email_token.present? &&
+        email_token == invite.email_token
       user.activate
     end
 
     User.find(user.id)
   end
 
   private
 
   def can_redeem_invite?
     return false if !invite.redeemable?
+    return false if email.blank?
 
-    # Invite has already been redeemed by anyone.
-    if !invite.is_invite_link? && InvitedUser.exists?(invite_id: invite.id)
+    # Invite scoped to email has already been redeemed by anyone.
+    if invite.is_email_invite? && InvitedUser.exists?(invite_id: invite.id)
       return false
     end
 
-    # Email will not be present if we are claiming an invite link, which
-    # does not have an email or domain scope on the invitation.
-    if email.present? || redeeming_user.present?
-      email_to_check = redeeming_user&.email || email
+    # The email will be present for either an invite link (where the user provides
+    # us the email manually) or for an invite scoped to an email, where we
+    # prefill the email and do not let the user modify it.
+    #
+    # Note that an invite link can also have a domain scope which must be checked.
+    email_to_check = redeeming_user&.email || email
 
-      if invite.email.present? && !invite.email_matches?(email_to_check)
-        raise ActiveRecord::RecordNotSaved.new(I18n.t('invite.not_matching_email'))
-      end
+    if invite.email.present? && !invite.email_matches?(email_to_check)
+      raise ActiveRecord::RecordNotSaved.new(I18n.t('invite.not_matching_email'))
+    end
 
-      if invite.domain.present? && !invite.domain_matches?(email_to_check)
-        raise ActiveRecord::RecordNotSaved.new(I18n.t('invite.domain_not_allowed'))
-      end
+    if invite.domain.present? && !invite.domain_matches?(email_to_check)
+      raise ActiveRecord::RecordNotSaved.new(I18n.t('invite.domain_not_allowed'))
     end
 
     # Anon user is trying to redeem an invitation, if an existing user already
     # redeemed it then we cannot redeem now.
     redeeming_user ||= User.where(admin: false, staged: false).find_by_email(email)
@@ -146,10 +186,14 @@ class InviteRedeemer
     end
 
     true
   end
 
+  # Note that the invited_user is returned by #redeemed, so other places
+  # (e.g. the InvitesController) can perform further actions on it, this
+  # is why things like send_welcome_message are set without being saved
+  # on the model.
   def invited_user
     return @invited_user if defined?(@invited_user)
 
     # The redeeming user is an already logged in user or a user who is
     # activating their account who is redeeming the invite,
@@ -194,13 +238,22 @@ class InviteRedeemer
 
     @invited_user_record.present?
   end
 
   def add_to_private_topics_if_invited
-    topic_ids = Topic.where(archetype: Archetype::private_message).includes(:invites).where(invites: { email: email }).pluck(:id)
+    # Should not happen because of ensure_email_is_present!, but better to cover bases.
+    return if email.blank?
+
+    topic_ids = TopicInvite.joins(:invite)
+      .joins(:topic)
+      .where("topics.archetype = ?", Archetype::private_message)
+      .where("invites.email = ?", email)
+      .pluck(:topic_id)
     topic_ids.each do |id|
-      TopicAllowedUser.create!(user_id: invited_user.id, topic_id: id) unless TopicAllowedUser.exists?(user_id: invited_user.id, topic_id: id)
+      if !TopicAllowedUser.exists?(user_id: invited_user.id, topic_id: id)
+        TopicAllowedUser.create!(user_id: invited_user.id, topic_id: id)
+      end
     end
   end
 
   def add_user_to_groups
     guardian = Guardian.new(invite.invited_by)
@@ -219,19 +272,21 @@ class InviteRedeemer
     @invited_user_record.update!(user_id: invited_user.id)
     invited_user.send_welcome_message = true
   end
 
   def notify_invitee
-    if inviter = invite.invited_by
-      inviter.notifications.create!(
-        notification_type: Notification.types[:invitee_accepted],
-        data: { display_username: invited_user.username }.to_json
-      )
-    end
+    return if invite.invited_by.blank?
+    invite.invited_by.notifications.create!(
+      notification_type: Notification.types[:invitee_accepted],
+      data: { display_username: invited_user.username }.to_json
+    )
   end
 
   def delete_duplicate_invites
+    # Should not happen because of ensure_email_is_present!, but better to cover bases.
+    return if email.blank?
+
     Invite
       .where('invites.max_redemptions_allowed = 1')
       .joins("LEFT JOIN invited_users ON invites.id = invited_users.invite_id")
       .where('invited_users.user_id IS NULL')
       .where('invites.email = ? AND invites.id != ?', email, invite.id)
diff --git a/db/migrate/20221103051248_remove_invalid_topic_allowed_users_from_invites.rb b/db/migrate/20221103051248_remove_invalid_topic_allowed_users_from_invites.rb
new file mode 100644
index 000000000000..d161a9f16d87
--- /dev/null
+++ b/db/migrate/20221103051248_remove_invalid_topic_allowed_users_from_invites.rb
@@ -0,0 +1,60 @@
+# frozen_string_literal: true
+
+class RemoveInvalidTopicAllowedUsersFromInvites < ActiveRecord::Migration[7.0]
+  def up
+    # We are getting all the topic_allowed_users records that
+    # match an invited user, which is created as part of the invite
+    # redemption flow. The original invite would _not_ have had a topic_invite
+    # record, and the user should have been added to the topic in the brief
+    # period between creation of the invited_users record and the update of
+    # that record.
+    #
+    # Having > 2 topic allowed users disqualifies messages sent only
+    # by the system or an admin to the user.
+    subquery_sql = <<~SQL
+      SELECT DISTINCT id
+      FROM (
+               SELECT tau.id, tau.user_id, COUNT(*) OVER (PARTITION BY tau.user_id)
+               FROM topic_allowed_users tau
+                    JOIN invited_users iu ON iu.user_id = tau.user_id
+                    LEFT JOIN topic_invites ti ON ti.invite_id = iu.invite_id AND tau.topic_id = ti.topic_id
+               WHERE ti.id IS NULL
+                 AND tau.created_at BETWEEN iu.created_at AND iu.updated_at
+                 AND iu.redeemed_at > '2022-10-27'
+           ) AS matching_topic_allowed_users
+      WHERE matching_topic_allowed_users.count > 2
+    SQL
+
+    # Back up the records we are going to change in case we are too
+    # brutal, and for further inspection.
+    #
+    # TODO DROP this table (topic_allowed_users_backup_nov_2022) in a later migration.
+    DB.exec(<<~SQL)
+      CREATE TABLE topic_allowed_users_backup_nov_2022
+      (
+          id       INT NOT NULL,
+          user_id  INT NOT NULL,
+          topic_id INT NOT NULL
+      );
+      INSERT INTO topic_allowed_users_backup_nov_2022(id, user_id, topic_id)
+      SELECT id, user_id, topic_id
+      FROM topic_allowed_users
+      WHERE id IN (
+                      #{subquery_sql}
+                  )
+    SQL
+
+    # Delete the invalid topic allowed users that should not be there.
+    DB.query(<<~SQL)
+      DELETE
+      FROM topic_allowed_users
+      WHERE id IN (
+                      #{subquery_sql}
+                  )
+    SQL
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end
diff --git a/spec/models/invite_redeemer_spec.rb b/spec/models/invite_redeemer_spec.rb
index caa1470d7ba0..a978a688ec3a 100644
--- a/spec/models/invite_redeemer_spec.rb
+++ b/spec/models/invite_redeemer_spec.rb
@@ -1,10 +1,87 @@
 # frozen_string_literal: true
 
 RSpec.describe InviteRedeemer do
   fab!(:admin) { Fabricate(:admin) }
 
+  describe "#initialize" do
+    fab!(:redeeming_user) { Fabricate(:user, email: "redeemer@test.com") }
+
+    context "for invite link" do
+      fab!(:invite) { Fabricate(:invite, email: nil) }
+
+      context "when an email is passed in without a redeeming user" do
+        it "uses that email for invite redemption" do
+          redeemer = described_class.new(invite: invite, email: "blah@test.com")
+          expect(redeemer.email).to eq("blah@test.com")
+          expect { redeemer.redeem }.to change { User.count }
+          expect(User.find_by_email(redeemer.email)).to be_present
+        end
+      end
+
+      context "when an email is passed in with a redeeming user" do
+        it "uses the redeeming user's email for invite redemption" do
+          redeemer = described_class.new(invite: invite, email: "blah@test.com", redeeming_user: redeeming_user)
+          expect(redeemer.email).to eq(redeeming_user.email)
+          expect { redeemer.redeem }.not_to change { User.count }
+        end
+      end
+
+      context "when an email is not passed in with a redeeming user" do
+        it "uses the redeeming user's email for invite redemption" do
+          redeemer = described_class.new(invite: invite, email: nil, redeeming_user: redeeming_user)
+          expect(redeemer.email).to eq(redeeming_user.email)
+          expect { redeemer.redeem }.not_to change { User.count }
+        end
+      end
+
+      context "when no email and no redeeming user is passed in" do
+        it "raises an error" do
+          expect { described_class.new(invite: invite, email: nil, redeeming_user: nil) }.to raise_error(Discourse::InvalidParameters)
+        end
+      end
+    end
+
+    context "for invite with email" do
+      fab!(:invite) { Fabricate(:invite, email: "foobar@example.com") }
+
+      context "when an email is passed in without a redeeming user" do
+        it "uses that email for invite redemption" do
+          redeemer = described_class.new(invite: invite, email: "foobar@example.com")
+          expect(redeemer.email).to eq("foobar@example.com")
+          expect { redeemer.redeem }.to change { User.count }
+          expect(User.find_by_email(redeemer.email)).to be_present
+        end
+      end
+
+      context "when an email is passed in with a redeeming user" do
+        it "uses the redeeming user's email for invite redemption" do
+          redeemer = described_class.new(invite: invite, email: "blah@test.com", redeeming_user: redeeming_user)
+          expect(redeemer.email).to eq(redeeming_user.email)
+          expect { redeemer.redeem }.to raise_error(ActiveRecord::RecordNotSaved, I18n.t("invite.not_matching_email"))
+        end
+      end
+
+      context "when an email is not passed in with a redeeming user" do
+        it "uses the invite email for invite redemption" do
+          redeemer = described_class.new(invite: invite, email: nil, redeeming_user: redeeming_user)
+          expect(redeemer.email).to eq("foobar@example.com")
+          expect { redeemer.redeem }.to raise_error(ActiveRecord::RecordNotSaved, I18n.t("invite.not_matching_email"))
+        end
+      end
+
+      context "when no email and no redeeming user is passed in" do
+        it "uses the invite email for invite redemption" do
+          redeemer = described_class.new(invite: invite, email: nil, redeeming_user: nil)
+          expect(redeemer.email).to eq("foobar@example.com")
+          expect { redeemer.redeem }.to change { User.count }
+          expect(User.find_by_email(redeemer.email)).to be_present
+        end
+      end
+    end
+  end
+
   describe '.create_user_from_invite' do
     it "should be created correctly" do
       invite = Fabricate(:invite, email: 'walter.white@email.com')
       user = InviteRedeemer.create_user_from_invite(invite: invite, email: invite.email, username: 'walter', name: 'Walter White')
       expect(user.username).to eq('walter')
@@ -111,175 +188,202 @@ RSpec.describe InviteRedeemer do
       expect(ReviewableUser.count).to eq(0)
     end
   end
 
   describe "#redeem" do
-    fab!(:invite) { Fabricate(:invite, email: "foobar@example.com") }
     let(:name) { 'john snow' }
     let(:username) { 'kingofthenorth' }
     let(:password) { 'know5nOthiNG' }
     let(:invite_redeemer) { InviteRedeemer.new(invite: invite, email: invite.email, username: username, name: name) }
 
-    context "when must_approve_users setting is enabled" do
-      before do
-        SiteSetting.must_approve_users = true
-      end
+    context "with email" do
+      fab!(:invite) { Fabricate(:invite, email: "foobar@example.com") }
+      context "when must_approve_users setting is enabled" do
+        before do
+          SiteSetting.must_approve_users = true
+        end
 
-      it "should redeem an invite but not approve the user when invite is created by a staff user" do
-        inviter = invite.invited_by
-        inviter.update!(admin: true)
-        user = invite_redeemer.redeem
+        it "should redeem an invite but not approve the user when invite is created by a staff user" do
+          inviter = invite.invited_by
+          inviter.update!(admin: true)
+          user = invite_redeemer.redeem
 
-        expect(user.name).to eq(name)
-        expect(user.username).to eq(username)
-        expect(user.invited_by).to eq(inviter)
-        expect(user.approved).to eq(false)
+          expect(user.name).to eq(name)
+          expect(user.username).to eq(username)
+          expect(user.invited_by).to eq(inviter)
+          expect(user.approved).to eq(false)
 
-        expect(inviter.notifications.count).to eq(1)
+          expect(inviter.notifications.count).to eq(1)
+        end
+
+        it "should redeem the invite but not approve the user when invite is created by a regular user" do
+          inviter = invite.invited_by
+          user = invite_redeemer.redeem
+
+          expect(user.name).to eq(name)
+          expect(user.username).to eq(username)
+          expect(user.invited_by).to eq(inviter)
+          expect(user.approved).to eq(false)
+
+          expect(inviter.notifications.count).to eq(1)
+        end
+
+        it "should redeem the invite and approve the user when user email is in auto_approve_email_domains setting" do
+          SiteSetting.auto_approve_email_domains = "example.com"
+          user = invite_redeemer.redeem
+
+          expect(user.name).to eq(name)
+          expect(user.username).to eq(username)
+          expect(user.approved).to eq(true)
+          expect(user.approved_by).to eq(Discourse.system_user)
+        end
       end
 
-      it "should redeem the invite but not approve the user when invite is created by a regular user" do
+      it "should redeem the invite if invited by non staff and approve if staff not required to approve" do
         inviter = invite.invited_by
         user = invite_redeemer.redeem
 
         expect(user.name).to eq(name)
         expect(user.username).to eq(username)
         expect(user.invited_by).to eq(inviter)
-        expect(user.approved).to eq(false)
-
         expect(inviter.notifications.count).to eq(1)
+        expect(user.approved).to eq(false)
       end
 
-      it "should redeem the invite and approve the user when user email is in auto_approve_email_domains setting" do
-        SiteSetting.auto_approve_email_domains = "example.com"
-        user = invite_redeemer.redeem
-
-        expect(user.name).to eq(name)
-        expect(user.username).to eq(username)
-        expect(user.approved).to eq(true)
-        expect(user.approved_by).to eq(Discourse.system_user)
+      it "should delete invite if invited_by user has been removed" do
+        invite.invited_by.destroy!
+        expect { invite.reload }.to raise_error(ActiveRecord::RecordNotFound)
       end
-    end
 
-    it "should redeem the invite if invited by non staff and approve if staff not required to approve" do
-      inviter = invite.invited_by
-      user = invite_redeemer.redeem
-
-      expect(user.name).to eq(name)
-      expect(user.username).to eq(username)
-      expect(user.invited_by).to eq(inviter)
-      expect(inviter.notifications.count).to eq(1)
-      expect(user.approved).to eq(false)
-    end
-
-    it "should delete invite if invited_by user has been removed" do
-      invite.invited_by.destroy!
-      expect { invite.reload }.to raise_error(ActiveRecord::RecordNotFound)
-    end
+      it "can set password" do
+        user = InviteRedeemer.new(invite: invite, email: invite.email, username: username, name: name, password: password).redeem
+        expect(user).to have_password
+        expect(user.confirm_password?(password)).to eq(true)
+        expect(user.approved).to eq(false)
+      end
 
-    it "can set password" do
-      user = InviteRedeemer.new(invite: invite, email: invite.email, username: username, name: name, password: password).redeem
-      expect(user).to have_password
-      expect(user.confirm_password?(password)).to eq(true)
-      expect(user.approved).to eq(false)
-    end
+      it "can set custom fields" do
+        required_field = Fabricate(:user_field)
+        optional_field = Fabricate(:user_field, required: false)
+        user_fields = {
+          required_field.id.to_s => 'value1',
+          optional_field.id.to_s => 'value2'
+        }
+        user = InviteRedeemer.new(invite: invite, email: invite.email, username: username, name: name, password: password, user_custom_fields: user_fields).redeem
+
+        expect(user).to be_present
+        expect(user.custom_fields["user_field_#{required_field.id}"]).to eq('value1')
+        expect(user.custom_fields["user_field_#{optional_field.id}"]).to eq('value2')
+      end
 
-    it "can set custom fields" do
-      required_field = Fabricate(:user_field)
-      optional_field = Fabricate(:user_field, required: false)
-      user_fields = {
-        required_field.id.to_s => 'value1',
-        optional_field.id.to_s => 'value2'
-      }
-      user = InviteRedeemer.new(invite: invite, email: invite.email, username: username, name: name, password: password, user_custom_fields: user_fields).redeem
-
-      expect(user).to be_present
-      expect(user.custom_fields["user_field_#{required_field.id}"]).to eq('value1')
-      expect(user.custom_fields["user_field_#{optional_field.id}"]).to eq('value2')
-    end
+      it "does not add user to group if inviter does not have permissions" do
+        group = Fabricate(:group, grant_trust_level: 2)
+        InvitedGroup.create(group_id: group.id, invite_id: invite.id)
+        user = InviteRedeemer.new(invite: invite, email: invite.email, username: username, name: name, password: password).redeem
 
-    it "does not add user to group if inviter does not have permissions" do
-      group = Fabricate(:group, grant_trust_level: 2)
-      InvitedGroup.create(group_id: group.id, invite_id: invite.id)
-      user = InviteRedeemer.new(invite: invite, email: invite.email, username: username, name: name, password: password).redeem
+        expect(user.group_users.count).to eq(0)
+      end
 
-      expect(user.group_users.count).to eq(0)
-    end
+      it "adds user to group" do
+        group = Fabricate(:group, grant_trust_level: 2)
+        InvitedGroup.create(group_id: group.id, invite_id: invite.id)
+        group.add_owner(invite.invited_by)
 
-    it "adds user to group" do
-      group = Fabricate(:group, grant_trust_level: 2)
-      InvitedGroup.create(group_id: group.id, invite_id: invite.id)
-      group.add_owner(invite.invited_by)
+        user = InviteRedeemer.new(invite: invite, email: invite.email, username: username, name: name, password: password).redeem
 
-      user = InviteRedeemer.new(invite: invite, email: invite.email, username: username, name: name, password: password).redeem
+        expect(user.group_users.count).to eq(4)
+        expect(user.trust_level).to eq(2)
+      end
 
-      expect(user.group_users.count).to eq(4)
-      expect(user.trust_level).to eq(2)
-    end
+      it "adds an entry to the group logs when the invited user is added to a group" do
+        group = Fabricate(:group)
+        InvitedGroup.create(group_id: group.id, invite_id: invite.id)
+        group.add_owner(invite.invited_by)
+
+        GroupHistory.destroy_all
+
+        user = InviteRedeemer.new(
+          invite: invite,
+          email: invite.email,
+          username: username,
+          name: name,
+          password: password
+        ).redeem
+
+        expect(group.reload.usernames.split(",")).to include(user.username)
+        expect(GroupHistory.exists?(
+          target_user_id: user.id,
+          acting_user: invite.invited_by.id,
+          group_id: group.id,
+          action: GroupHistory.actions[:add_user_to_group]
+        )).to eq(true)
+      end
 
-    it "adds an entry to the group logs when the invited user is added to a group" do
-      group = Fabricate(:group)
-      InvitedGroup.create(group_id: group.id, invite_id: invite.id)
-      group.add_owner(invite.invited_by)
+      it "only allows one user to be created per invite" do
+        user = invite_redeemer.redeem
+        invite.reload
 
-      GroupHistory.destroy_all
+        user.email = "john@example.com"
+        user.save!
 
-      user = InviteRedeemer.new(
-        invite: invite,
-        email: invite.email,
-        username: username,
-        name: name,
-        password: password
-      ).redeem
-
-      expect(group.reload.usernames.split(",")).to include(user.username)
-      expect(GroupHistory.exists?(
-        target_user_id: user.id,
-        acting_user: invite.invited_by.id,
-        group_id: group.id,
-        action: GroupHistory.actions[:add_user_to_group]
-      )).to eq(true)
-    end
+        another_invite_redeemer = InviteRedeemer.new(invite: invite, email: invite.email, username: username, name: name)
+        another_user = another_invite_redeemer.redeem
+        expect(another_user).to eq(nil)
+      end
 
-    it "only allows one user to be created per invite" do
-      user = invite_redeemer.redeem
-      invite.reload
+      it "should correctly update the invite redeemed_at date" do
+        SiteSetting.invite_expiry_days = 2
+        invite.update!(created_at: 10.days.ago)
 
-      user.email = "john@example.com"
-      user.save!
+        inviter = invite.invited_by
+        inviter.admin = true
+        user = invite_redeemer.redeem
+        invite.reload
 
-      another_invite_redeemer = InviteRedeemer.new(invite: invite, email: invite.email, username: username, name: name)
-      another_user = another_invite_redeemer.redeem
-      expect(another_user).to eq(nil)
-    end
+        expect(user.invited_by).to eq(inviter)
+        expect(inviter.notifications.count).to eq(1)
+        expect(invite.invited_users.first).to be_present
+      end
 
-    it "should correctly update the invite redeemed_at date" do
-      SiteSetting.invite_expiry_days = 2
-      invite.update!(created_at: 10.days.ago)
+      it "raises an error if the email does not match the invite email" do
+        redeemer = InviteRedeemer.new(invite: invite, email: "blah@test.com", username: username, name: name)
+        expect { redeemer.redeem }.to raise_error(ActiveRecord::RecordNotSaved, I18n.t("invite.not_matching_email"))
+      end
 
-      inviter = invite.invited_by
-      inviter.admin = true
-      user = invite_redeemer.redeem
-      invite.reload
+      it "adds the user to the appropriate private topic and no others" do
+        topic1 = Fabricate(:private_message_topic)
+        topic2 = Fabricate(:private_message_topic)
+        TopicInvite.create(invite: invite, topic: topic1)
+        user = InviteRedeemer.new(invite: invite, email: invite.email, username: username, name: name, password: password).redeem
+        expect(TopicAllowedUser.exists?(topic: topic1, user: user)).to eq(true)
+        expect(TopicAllowedUser.exists?(topic: topic2, user: user)).to eq(false)
+      end
 
-      expect(user.invited_by).to eq(inviter)
-      expect(inviter.notifications.count).to eq(1)
-      expect(invite.invited_users.first).to be_present
-    end
+      context "when a redeeming user is passed in" do
+        fab!(:redeeming_user) { Fabricate(:user, email: "foobar@example.com") }
 
-    it "raises an error if the email does not match the invite email" do
-      redeemer = InviteRedeemer.new(invite: invite, email: "blah@test.com", username: username, name: name)
-      expect { redeemer.redeem }.to raise_error(ActiveRecord::RecordNotSaved, I18n.t("invite.not_matching_email"))
-    end
+        it "raises an error if the email does not match the invite email" do
+          redeeming_user.update!(email: "foo@bar.com")
+          redeemer = InviteRedeemer.new(invite: invite, redeeming_user: redeeming_user)
+          expect { redeemer.redeem }.to raise_error(ActiveRecord::RecordNotSaved, I18n.t("invite.not_matching_email"))
+        end
 
-    context "when a redeeming user is passed in" do
-      fab!(:redeeming_user) { Fabricate(:user, email: "foobar@example.com") }
+        it "adds the user to the appropriate private topic and no others" do
+          topic1 = Fabricate(:private_message_topic)
+          topic2 = Fabricate(:private_message_topic)
+          TopicInvite.create(invite: invite, topic: topic1)
+          InviteRedeemer.new(invite: invite, redeeming_user: redeeming_user).redeem
+          expect(TopicAllowedUser.exists?(topic: topic1, user: redeeming_user)).to eq(true)
+          expect(TopicAllowedUser.exists?(topic: topic2, user: redeeming_user)).to eq(false)
+        end
 
-      it "raises an error if the email does not match the invite email" do
-        redeeming_user.update!(email: "foo@bar.com")
-        redeemer = InviteRedeemer.new(invite: invite, redeeming_user: redeeming_user)
-        expect { redeemer.redeem }.to raise_error(ActiveRecord::RecordNotSaved, I18n.t("invite.not_matching_email"))
+        it "does not create a topic allowed user record if the invited user is already in the topic" do
+          topic1 = Fabricate(:private_message_topic)
+          TopicInvite.create(invite: invite, topic: topic1)
+          TopicAllowedUser.create(topic: topic1, user: redeeming_user)
+          expect { InviteRedeemer.new(invite: invite, redeeming_user: redeeming_user).redeem }.not_to change { TopicAllowedUser.count }
+        end
       end
     end
 
     context 'with domain' do
       fab!(:invite) { Fabricate(:invite, email: nil, domain: "test.com") }
diff --git a/spec/requests/invites_controller_spec.rb b/spec/requests/invites_controller_spec.rb
index e450296d870b..8410bd66ef51 100644
--- a/spec/requests/invites_controller_spec.rb
+++ b/spec/requests/invites_controller_spec.rb
@@ -910,80 +910,148 @@ RSpec.describe InvitesController do
         expect(response.body).to include(I18n.t('login.new_registrations_disabled'))
       end
     end
 
     context 'when user is already logged in' do
-      fab!(:invite) { Fabricate(:invite, email: 'test@example.com') }
-      fab!(:user) { Fabricate(:user, email: 'test@example.com') }
-      fab!(:group) { Fabricate(:group) }
-
       before { sign_in(user) }
 
-      it 'redeems the invitation and creates the invite accepted notification' do
-        put "/invites/show/#{invite.invite_key}.json", params: { id: invite.invite_key }
-        expect(response.status).to eq(200)
-        expect(response.parsed_body["message"]).to eq(I18n.t("invite.existing_user_success"))
-        invite.reload
-        expect(invite.invited_users.first.user).to eq(user)
-        expect(invite.redeemed?).to be_truthy
-        expect(
-          Notification.exists?(
-            user: invite.invited_by, notification_type: Notification.types[:invitee_accepted]
-          )
-        ).to eq(true)
-      end
+      context "for an email invite" do
+        fab!(:invite) { Fabricate(:invite, email: 'test@example.com') }
+        fab!(:user) { Fabricate(:user, email: 'test@example.com') }
+        fab!(:group) { Fabricate(:group) }
 
-      it 'redirects to the first topic the user was invited to and creates the topic notification' do
-        topic = Fabricate(:topic)
-        TopicInvite.create!(invite: invite, topic: topic)
-        put "/invites/show/#{invite.invite_key}.json", params: { id: invite.invite_key }
-        expect(response.status).to eq(200)
-        expect(response.parsed_body['redirect_to']).to eq(topic.relative_url)
-        expect(Notification.where(notification_type: Notification.types[:invited_to_topic], topic: topic).count).to eq(1)
-      end
+        it 'redeems the invitation and creates the invite accepted notification' do
+          put "/invites/show/#{invite.invite_key}.json", params: { id: invite.invite_key }
+          expect(response.status).to eq(200)
+          expect(response.parsed_body["message"]).to eq(I18n.t("invite.existing_user_success"))
+          invite.reload
+          expect(invite.invited_users.first.user).to eq(user)
+          expect(invite.redeemed?).to be_truthy
+          expect(
+            Notification.exists?(
+              user: invite.invited_by, notification_type: Notification.types[:invitee_accepted]
+            )
+          ).to eq(true)
+        end
 
-      it "adds the user to the groups specified on the invite and allows them to access the secure topic" do
-        group.add_owner(invite.invited_by)
-        secured_category = Fabricate(:category)
-        secured_category.permissions = { group.name => :full }
-        secured_category.save!
+        it 'redirects to the first topic the user was invited to and creates the topic notification' do
+          topic = Fabricate(:topic)
+          TopicInvite.create!(invite: invite, topic: topic)
+          put "/invites/show/#{invite.invite_key}.json", params: { id: invite.invite_key }
+          expect(response.status).to eq(200)
+          expect(response.parsed_body['redirect_to']).to eq(topic.relative_url)
+          expect(Notification.where(notification_type: Notification.types[:invited_to_topic], topic: topic).count).to eq(1)
+        end
 
-        topic = Fabricate(:topic, category: secured_category)
-        TopicInvite.create!(invite: invite, topic: topic)
-        InvitedGroup.create!(invite: invite, group: group)
+        it "adds the user to the private topic" do
+          topic = Fabricate(:private_message_topic)
+          TopicInvite.create!(invite: invite, topic: topic)
+          put "/invites/show/#{invite.invite_key}.json", params: { id: invite.invite_key }
+          expect(response.status).to eq(200)
+          expect(response.parsed_body['redirect_to']).to eq(topic.relative_url)
+          expect(TopicAllowedUser.exists?(user: user, topic: topic)).to eq(true)
+        end
 
-        put "/invites/show/#{invite.invite_key}.json", params: { id: invite.invite_key }
-        expect(response.status).to eq(200)
-        expect(response.parsed_body["message"]).to eq(I18n.t("invite.existing_user_success"))
-        expect(response.parsed_body['redirect_to']).to eq(topic.relative_url)
-        invite.reload
-        expect(invite.redeemed?).to be_truthy
-        expect(user.reload.groups).to include(group)
-        expect(Notification.where(notification_type: Notification.types[:invited_to_topic], topic: topic).count).to eq(1)
-      end
+        it "adds the user to the groups specified on the invite and allows them to access the secure topic" do
+          group.add_owner(invite.invited_by)
+          secured_category = Fabricate(:category)
+          secured_category.permissions = { group.name => :full }
+          secured_category.save!
+
+          topic = Fabricate(:topic, category: secured_category)
+          TopicInvite.create!(invite: invite, topic: topic)
+          InvitedGroup.create!(invite: invite, group: group)
 
-      it "does not try to log in the user automatically" do
-        expect do
           put "/invites/show/#{invite.invite_key}.json", params: { id: invite.invite_key }
-        end.not_to change { UserAuthToken.count }
-        expect(response.status).to eq(200)
-        expect(response.parsed_body["message"]).to eq(I18n.t("invite.existing_user_success"))
-      end
+          expect(response.status).to eq(200)
+          expect(response.parsed_body["message"]).to eq(I18n.t("invite.existing_user_success"))
+          expect(response.parsed_body['redirect_to']).to eq(topic.relative_url)
+          invite.reload
+          expect(invite.redeemed?).to be_truthy
+          expect(user.reload.groups).to include(group)
+          expect(Notification.where(notification_type: Notification.types[:invited_to_topic], topic: topic).count).to eq(1)
+        end
 
-      it "errors if the user's email doesn't match the invite email" do
-        user.update!(email: "blah@test.com")
-        put "/invites/show/#{invite.invite_key}.json", params: { id: invite.invite_key }
-        expect(response.status).to eq(412)
-        expect(response.parsed_body["message"]).to eq(I18n.t("invite.not_matching_email"))
+        it "does not try to log in the user automatically" do
+          expect do
+            put "/invites/show/#{invite.invite_key}.json", params: { id: invite.invite_key }
+          end.not_to change { UserAuthToken.count }
+          expect(response.status).to eq(200)
+          expect(response.parsed_body["message"]).to eq(I18n.t("invite.existing_user_success"))
+        end
+
+        it "errors if the user's email doesn't match the invite email" do
+          user.update!(email: "blah@test.com")
+          put "/invites/show/#{invite.invite_key}.json", params: { id: invite.invite_key }
+          expect(response.status).to eq(412)
+          expect(response.parsed_body["message"]).to eq(I18n.t("invite.not_matching_email"))
+        end
+
+        it "errors if the user's email domain doesn't match the invite domain" do
+          user.update!(email: "blah@test.com")
+          invite.update!(email: nil, domain: "example.com")
+          put "/invites/show/#{invite.invite_key}.json", params: { id: invite.invite_key }
+          expect(response.status).to eq(412)
+          expect(response.parsed_body["message"]).to eq(I18n.t("invite.domain_not_allowed"))
+        end
       end
 
-      it "errors if the user's email domain doesn't match the invite domain" do
-        user.update!(email: "blah@test.com")
-        invite.update!(email: nil, domain: "example.com")
-        put "/invites/show/#{invite.invite_key}.json", params: { id: invite.invite_key }
-        expect(response.status).to eq(412)
-        expect(response.parsed_body["message"]).to eq(I18n.t("invite.domain_not_allowed"))
+      context "for an invite link" do
+        fab!(:invite) { Fabricate(:invite, email: nil) }
+        fab!(:user) { Fabricate(:user, email: 'test@example.com') }
+        fab!(:group) { Fabricate(:group) }
+
+        it 'redeems the invitation and creates the invite accepted notification' do
+          put "/invites/show/#{invite.invite_key}.json", params: { id: invite.invite_key }
+          expect(response.status).to eq(200)
+          expect(response.parsed_body["message"]).to eq(I18n.t("invite.existing_user_success"))
+          invite.reload
+          expect(invite.invited_users.first.user).to eq(user)
+          expect(invite.redeemed?).to be_truthy
+          expect(
+            Notification.exists?(
+              user: invite.invited_by, notification_type: Notification.types[:invitee_accepted]
+            )
+          ).to eq(true)
+        end
+
+        it 'redirects to the first topic the user was invited to and creates the topic notification' do
+          topic = Fabricate(:topic)
+          TopicInvite.create!(invite: invite, topic: topic)
+          put "/invites/show/#{invite.invite_key}.json", params: { id: invite.invite_key }
+          expect(response.status).to eq(200)
+          expect(response.parsed_body['redirect_to']).to eq(topic.relative_url)
+          expect(Notification.where(notification_type: Notification.types[:invited_to_topic], topic: topic).count).to eq(1)
+        end
+
+        it "adds the user to the groups specified on the invite and allows them to access the secure topic" do
+          group.add_owner(invite.invited_by)
+          secured_category = Fabricate(:category)
+          secured_category.permissions = { group.name => :full }
+          secured_category.save!
+
+          topic = Fabricate(:topic, category: secured_category)
+          TopicInvite.create!(invite: invite, topic: topic)
+          InvitedGroup.create!(invite: invite, group: group)
+
+          put "/invites/show/#{invite.invite_key}.json", params: { id: invite.invite_key }
+          expect(response.status).to eq(200)
+          expect(response.parsed_body["message"]).to eq(I18n.t("invite.existing_user_success"))
+          expect(response.parsed_body['redirect_to']).to eq(topic.relative_url)
+          invite.reload
+          expect(invite.redeemed?).to be_truthy
+          expect(user.reload.groups).to include(group)
+          expect(Notification.where(notification_type: Notification.types[:invited_to_topic], topic: topic).count).to eq(1)
+        end
+
+        it "does not try to log in the user automatically" do
+          expect do
+            put "/invites/show/#{invite.invite_key}.json", params: { id: invite.invite_key }
+          end.not_to change { UserAuthToken.count }
+          expect(response.status).to eq(200)
+          expect(response.parsed_body["message"]).to eq(I18n.t("invite.existing_user_success"))
+        end
       end
     end
 
     context 'with topic invites' do
       fab!(:invite) { Fabricate(:invite, email: 'test@example.com') }

Vulnerable code:       if !invite.is_invite_link? && sso.email != invite.email
    email.blank?
    email = self.email if email.blank? && !is_invite_link?

    invite: nil,

    @email = email
  # extracted from User cause it is very specific to invites
    if invite.emailed_status != Invite.emailed_status_types[:not_required] && email == invite.email && invite.email_token.present? && email_token == invite.email_token
    # Invite has already been redeemed by anyone.
    if !invite.is_invite_link? && InvitedUser.exists?(invite_id: invite.id)
    # Email will not be present if we are claiming an invite link, which
    # does not have an email or domain scope on the invitation.
    if email.present? || redeeming_user.present?
      email_to_check = redeeming_user&.email || email
      if invite.email.present? && !invite.email_matches?(email_to_check)
        raise ActiveRecord::RecordNotSaved.new(I18n.t('invite.not_matching_email'))
      end
      if invite.domain.present? && !invite.domain_matches?(email_to_check)
        raise ActiveRecord::RecordNotSaved.new(I18n.t('invite.domain_not_allowed'))
      end
    topic_ids = Topic.where(archetype: Archetype::private_message).includes(:invites).where(invites: { email: email }).pluck(:id)
      TopicAllowedUser.create!(user_id: invited_user.id, topic_id: id) unless TopicAllowedUser.exists?(user_id: invited_user.id, topic_id: id)
    if inviter = invite.invited_by
      inviter.notifications.create!(
        notification_type: Notification.types[:invitee_accepted],
        data: { display_username: invited_user.username }.to_json
      )
    end
    fab!(:invite) { Fabricate(:invite, email: "foobar@example.com") }
    context "when must_approve_users setting is enabled" do
      before do
        SiteSetting.must_approve_users = true
      end
      it "should redeem an invite but not approve the user when invite is created by a staff user" do
        inviter = invite.invited_by
        inviter.update!(admin: true)
        user = invite_redeemer.redeem
        expect(user.name).to eq(name)
        expect(user.username).to eq(username)
        expect(user.invited_by).to eq(inviter)
        expect(user.approved).to eq(false)
        expect(inviter.notifications.count).to eq(1)
      it "should redeem the invite but not approve the user when invite is created by a regular user" do
        expect(user.approved).to eq(false)

      it "should redeem the invite and approve the user when user email is in auto_approve_email_domains setting" do
        SiteSetting.auto_approve_email_domains = "example.com"
        user = invite_redeemer.redeem

        expect(user.name).to eq(name)
        expect(user.username).to eq(username)
        expect(user.approved).to eq(true)
        expect(user.approved_by).to eq(Discourse.system_user)
    end
    it "should redeem the invite if invited by non staff and approve if staff not required to approve" do
      inviter = invite.invited_by
      user = invite_redeemer.redeem

      expect(user.name).to eq(name)
      expect(user.username).to eq(username)
      expect(user.invited_by).to eq(inviter)
      expect(inviter.notifications.count).to eq(1)
      expect(user.approved).to eq(false)
    end

    it "should delete invite if invited_by user has been removed" do
      invite.invited_by.destroy!
      expect { invite.reload }.to raise_error(ActiveRecord::RecordNotFound)
    end
    it "can set password" do
      user = InviteRedeemer.new(invite: invite, email: invite.email, username: username, name: name, password: password).redeem
      expect(user).to have_password
      expect(user.confirm_password?(password)).to eq(true)
      expect(user.approved).to eq(false)
    end
    it "can set custom fields" do
      required_field = Fabricate(:user_field)
      optional_field = Fabricate(:user_field, required: false)
      user_fields = {
        required_field.id.to_s => 'value1',
        optional_field.id.to_s => 'value2'
      }
      user = InviteRedeemer.new(invite: invite, email: invite.email, username: username, name: name, password: password, user_custom_fields: user_fields).redeem

      expect(user).to be_present
      expect(user.custom_fields["user_field_#{required_field.id}"]).to eq('value1')
      expect(user.custom_fields["user_field_#{optional_field.id}"]).to eq('value2')
    end
    it "does not add user to group if inviter does not have permissions" do
      group = Fabricate(:group, grant_trust_level: 2)
      InvitedGroup.create(group_id: group.id, invite_id: invite.id)
      user = InviteRedeemer.new(invite: invite, email: invite.email, username: username, name: name, password: password).redeem
      expect(user.group_users.count).to eq(0)
    end
    it "adds user to group" do
      group = Fabricate(:group, grant_trust_level: 2)
      InvitedGroup.create(group_id: group.id, invite_id: invite.id)
      group.add_owner(invite.invited_by)
      user = InviteRedeemer.new(invite: invite, email: invite.email, username: username, name: name, password: password).redeem
      expect(user.group_users.count).to eq(4)
      expect(user.trust_level).to eq(2)
    end
    it "adds an entry to the group logs when the invited user is added to a group" do
      group = Fabricate(:group)
      InvitedGroup.create(group_id: group.id, invite_id: invite.id)
      group.add_owner(invite.invited_by)
      GroupHistory.destroy_all
      user = InviteRedeemer.new(
        invite: invite,
        email: invite.email,
        username: username,
        name: name,
        password: password
      ).redeem

      expect(group.reload.usernames.split(",")).to include(user.username)
      expect(GroupHistory.exists?(
        target_user_id: user.id,
        acting_user: invite.invited_by.id,
        group_id: group.id,
        action: GroupHistory.actions[:add_user_to_group]
      )).to eq(true)
    end
    it "only allows one user to be created per invite" do
      user = invite_redeemer.redeem
      invite.reload
      user.email = "john@example.com"
      user.save!
      another_invite_redeemer = InviteRedeemer.new(invite: invite, email: invite.email, username: username, name: name)
      another_user = another_invite_redeemer.redeem
      expect(another_user).to eq(nil)
    end
    it "should correctly update the invite redeemed_at date" do
      SiteSetting.invite_expiry_days = 2
      invite.update!(created_at: 10.days.ago)
      inviter = invite.invited_by
      inviter.admin = true
      user = invite_redeemer.redeem
      invite.reload
      expect(user.invited_by).to eq(inviter)
      expect(inviter.notifications.count).to eq(1)
      expect(invite.invited_users.first).to be_present
    end
    it "raises an error if the email does not match the invite email" do
      redeemer = InviteRedeemer.new(invite: invite, email: "blah@test.com", username: username, name: name)
      expect { redeemer.redeem }.to raise_error(ActiveRecord::RecordNotSaved, I18n.t("invite.not_matching_email"))
    end
    context "when a redeeming user is passed in" do
      fab!(:redeeming_user) { Fabricate(:user, email: "foobar@example.com") }
      it "raises an error if the email does not match the invite email" do
        redeeming_user.update!(email: "foo@bar.com")
        redeemer = InviteRedeemer.new(invite: invite, redeeming_user: redeeming_user)
        expect { redeemer.redeem }.to raise_error(ActiveRecord::RecordNotSaved, I18n.t("invite.not_matching_email"))
      fab!(:invite) { Fabricate(:invite, email: 'test@example.com') }
      fab!(:user) { Fabricate(:user, email: 'test@example.com') }
      fab!(:group) { Fabricate(:group) }

      it 'redeems the invitation and creates the invite accepted notification' do
        put "/invites/show/#{invite.invite_key}.json", params: { id: invite.invite_key }
        expect(response.status).to eq(200)
        expect(response.parsed_body["message"]).to eq(I18n.t("invite.existing_user_success"))
        invite.reload
        expect(invite.invited_users.first.user).to eq(user)
        expect(invite.redeemed?).to be_truthy
        expect(
          Notification.exists?(
            user: invite.invited_by, notification_type: Notification.types[:invitee_accepted]
          )
        ).to eq(true)
      end
      it 'redirects to the first topic the user was invited to and creates the topic notification' do
        topic = Fabricate(:topic)
        TopicInvite.create!(invite: invite, topic: topic)
        put "/invites/show/#{invite.invite_key}.json", params: { id: invite.invite_key }
        expect(response.status).to eq(200)
        expect(response.parsed_body['redirect_to']).to eq(topic.relative_url)
        expect(Notification.where(notification_type: Notification.types[:invited_to_topic], topic: topic).count).to eq(1)
      end
      it "adds the user to the groups specified on the invite and allows them to access the secure topic" do
        group.add_owner(invite.invited_by)
        secured_category = Fabricate(:category)
        secured_category.permissions = { group.name => :full }
        secured_category.save!
        topic = Fabricate(:topic, category: secured_category)
        TopicInvite.create!(invite: invite, topic: topic)
        InvitedGroup.create!(invite: invite, group: group)
        put "/invites/show/#{invite.invite_key}.json", params: { id: invite.invite_key }
        expect(response.status).to eq(200)
        expect(response.parsed_body["message"]).to eq(I18n.t("invite.existing_user_success"))
        expect(response.parsed_body['redirect_to']).to eq(topic.relative_url)
        invite.reload
        expect(invite.redeemed?).to be_truthy
        expect(user.reload.groups).to include(group)
        expect(Notification.where(notification_type: Notification.types[:invited_to_topic], topic: topic).count).to eq(1)
      end
      it "does not try to log in the user automatically" do
        expect do
        end.not_to change { UserAuthToken.count }
        expect(response.status).to eq(200)
        expect(response.parsed_body["message"]).to eq(I18n.t("invite.existing_user_success"))
      end
      it "errors if the user's email doesn't match the invite email" do
        user.update!(email: "blah@test.com")
        put "/invites/show/#{invite.invite_key}.json", params: { id: invite.invite_key }
        expect(response.status).to eq(412)
        expect(response.parsed_body["message"]).to eq(I18n.t("invite.not_matching_email"))
      it "errors if the user's email domain doesn't match the invite domain" do
        user.update!(email: "blah@test.com")
        invite.update!(email: nil, domain: "example.com")
        put "/invites/show/#{invite.invite_key}.json", params: { id: invite.invite_key }
        expect(response.status).to eq(412)
        expect(response.parsed_body["message"]).to eq(I18n.t("invite.domain_not_allowed"))
Fixed code:       if invite.is_email_invite? && sso.email != invite.email
  # Even if a domain is specified on the invite, it still counts as
  # an invite link.
    self.email.blank?
  end

  # Email invites have specific behaviour and it's easier to visually
  # parse is_email_invite? than !is_invite_link?
  def is_email_invite?
    self.email.present?
# NOTE: There are a _lot_ of complicated rules and conditions for our
# invite system, and the code is spread out through a lot of places.
# Tread lightly and read carefully when modifying this code. You may
# also want to look at:
#
# * InvitesController
# * SessionController
# * Invite model
# * User model
#
# Invites that are scoped to a specific email (email IS NOT NULL on the Invite
# model) have different rules to invites that are considered an "invite link",
# (email IS NULL) on the Invite model.
    invite:,

    ensure_email_is_present!(email)
  # The email must be present in some form since many of the methods
  # for processing + redemption rely on it. If it's still nil after
  # these checks then we have hit an edge case and should not proceed!
  def ensure_email_is_present!(email)
    if email.blank?
      Rails.logger.warn(
        "email param was blank in InviteRedeemer for invite ID #{@invite.id}. The `redeeming_user` was #{@redeeming_user.present? ? "(ID: #{@redeeming_user.id})" : "not"} present.",
      )
    end

    if email.blank? && @invite.is_email_invite?
      @email = @invite.email
    elsif @redeeming_user.present?
      @email = @redeeming_user.email
    else
      @email = email
    end

    raise Discourse::InvalidParameters if @email.blank?
  end

  # This will _never_ be called if there is a redeeming_user being passed
  # in to InviteRedeemer -- see invited_user below.
    if invite.emailed_status != Invite.emailed_status_types[:not_required] &&
        email == invite.email &&
        invite.email_token.present? &&
        email_token == invite.email_token
    return false if email.blank?
    # Invite scoped to email has already been redeemed by anyone.
    if invite.is_email_invite? && InvitedUser.exists?(invite_id: invite.id)
    # The email will be present for either an invite link (where the user provides
    # us the email manually) or for an invite scoped to an email, where we
    # prefill the email and do not let the user modify it.
    #
    # Note that an invite link can also have a domain scope which must be checked.
    email_to_check = redeeming_user&.email || email
    if invite.email.present? && !invite.email_matches?(email_to_check)
      raise ActiveRecord::RecordNotSaved.new(I18n.t('invite.not_matching_email'))
    end
    if invite.domain.present? && !invite.domain_matches?(email_to_check)
      raise ActiveRecord::RecordNotSaved.new(I18n.t('invite.domain_not_allowed'))
  # Note that the invited_user is returned by #redeemed, so other places
  # (e.g. the InvitesController) can perform further actions on it, this
  # is why things like send_welcome_message are set without being saved
  # on the model.
    # Should not happen because of ensure_email_is_present!, but better to cover bases.
    return if email.blank?

    topic_ids = TopicInvite.joins(:invite)
      .joins(:topic)
      .where("topics.archetype = ?", Archetype::private_message)
      .where("invites.email = ?", email)
      .pluck(:topic_id)
      if !TopicAllowedUser.exists?(user_id: invited_user.id, topic_id: id)
        TopicAllowedUser.create!(user_id: invited_user.id, topic_id: id)
      end
    return if invite.invited_by.blank?
    invite.invited_by.notifications.create!(
      notification_type: Notification.types[:invitee_accepted],
      data: { display_username: invited_user.username }.to_json
    )
    # Should not happen because of ensure_email_is_present!, but better to cover bases.
    return if email.blank?

# frozen_string_literal: true

class RemoveInvalidTopicAllowedUsersFromInvites < ActiveRecord::Migration[7.0]
  def up
    # We are getting all the topic_allowed_users records that
    # match an invited user, which is created as part of the invite
    # redemption flow. The original invite would _not_ have had a topic_invite
    # record, and the user should have been added to the topic in the brief
    # period between creation of the invited_users record and the update of
    # that record.
    #
    # Having > 2 topic allowed users disqualifies messages sent only
    # by the system or an admin to the user.
    subquery_sql = <<~SQL
      SELECT DISTINCT id
      FROM (
               SELECT tau.id, tau.user_id, COUNT(*) OVER (PARTITION BY tau.user_id)
               FROM topic_allowed_users tau
                    JOIN invited_users iu ON iu.user_id = tau.user_id
                    LEFT JOIN topic_invites ti ON ti.invite_id = iu.invite_id AND tau.topic_id = ti.topic_id
               WHERE ti.id IS NULL
                 AND tau.created_at BETWEEN iu.created_at AND iu.updated_at
                 AND iu.redeemed_at > '2022-10-27'
           ) AS matching_topic_allowed_users
      WHERE matching_topic_allowed_users.count > 2
    SQL

    # Back up the records we are going to change in case we are too
    # brutal, and for further inspection.
    #
    # TODO DROP this table (topic_allowed_users_backup_nov_2022) in a later migration.
    DB.exec(<<~SQL)
      CREATE TABLE topic_allowed_users_backup_nov_2022
      (
          id       INT NOT NULL,
          user_id  INT NOT NULL,
          topic_id INT NOT NULL
      );
      INSERT INTO topic_allowed_users_backup_nov_2022(id, user_id, topic_id)
      SELECT id, user_id, topic_id
      FROM topic_allowed_users
      WHERE id IN (
                      #{subquery_sql}
                  )
    SQL

    # Delete the invalid topic allowed users that should not be there.
    DB.query(<<~SQL)
      DELETE
      FROM topic_allowed_users
      WHERE id IN (
                      #{subquery_sql}
                  )
    SQL
  end

  def down
    raise ActiveRecord::IrreversibleMigration
  end
end
  describe "#initialize" do
    fab!(:redeeming_user) { Fabricate(:user, email: "redeemer@test.com") }

    context "for invite link" do
      fab!(:invite) { Fabricate(:invite, email: nil) }

      context "when an email is passed in without a redeeming user" do
        it "uses that email for invite redemption" do
          redeemer = described_class.new(invite: invite, email: "blah@test.com")
          expect(redeemer.email).to eq("blah@test.com")
          expect { redeemer.redeem }.to change { User.count }
          expect(User.find_by_email(redeemer.email)).to be_present
        end
      end

      context "when an email is passed in with a redeeming user" do
        it "uses the redeeming user's email for invite redemption" do
          redeemer = described_class.new(invite: invite, email: "blah@test.com", redeeming_user: redeeming_user)
          expect(redeemer.email).to eq(redeeming_user.email)
          expect { redeemer.redeem }.not_to change { User.count }
        end
      end

      context "when an email is not passed in with a redeeming user" do
        it "uses the redeeming user's email for invite redemption" do
          redeemer = described_class.new(invite: invite, email: nil, redeeming_user: redeeming_user)
          expect(redeemer.email).to eq(redeeming_user.email)
          expect { redeemer.redeem }.not_to change { User.count }
        end
      end

      context "when no email and no redeeming user is passed in" do
        it "raises an error" do
          expect { described_class.new(invite: invite, email: nil, redeeming_user: nil) }.to raise_error(Discourse::InvalidParameters)
        end
      end
    end

    context "for invite with email" do
      fab!(:invite) { Fabricate(:invite, email: "foobar@example.com") }

      context "when an email is passed in without a redeeming user" do
        it "uses that email for invite redemption" do
          redeemer = described_class.new(invite: invite, email: "foobar@example.com")
          expect(redeemer.email).to eq("foobar@example.com")
          expect { redeemer.redeem }.to change { User.count }
          expect(User.find_by_email(redeemer.email)).to be_present
        end
      end

      context "when an email is passed in with a redeeming user" do
        it "uses the redeeming user's email for invite redemption" do
          redeemer = described_class.new(invite: invite, email: "blah@test.com", redeeming_user: redeeming_user)
          expect(redeemer.email).to eq(redeeming_user.email)
          expect { redeemer.redeem }.to raise_error(ActiveRecord::RecordNotSaved, I18n.t("invite.not_matching_email"))
        end
      end

      context "when an email is not passed in with a redeeming user" do
        it "uses the invite email for invite redemption" do
          redeemer = described_class.new(invite: invite, email: nil, redeeming_user: redeeming_user)
          expect(redeemer.email).to eq("foobar@example.com")
          expect { redeemer.redeem }.to raise_error(ActiveRecord::RecordNotSaved, I18n.t("invite.not_matching_email"))
        end
      end

      context "when no email and no redeeming user is passed in" do
        it "uses the invite email for invite redemption" do
          redeemer = described_class.new(invite: invite, email: nil, redeeming_user: nil)
          expect(redeemer.email).to eq("foobar@example.com")
          expect { redeemer.redeem }.to change { User.count }
          expect(User.find_by_email(redeemer.email)).to be_present
        end
      end
    end
  end

    context "with email" do
      fab!(:invite) { Fabricate(:invite, email: "foobar@example.com") }
      context "when must_approve_users setting is enabled" do
        before do
          SiteSetting.must_approve_users = true
        end
        it "should redeem an invite but not approve the user when invite is created by a staff user" do
          inviter = invite.invited_by
          inviter.update!(admin: true)
          user = invite_redeemer.redeem
          expect(user.name).to eq(name)
          expect(user.username).to eq(username)
          expect(user.invited_by).to eq(inviter)
          expect(user.approved).to eq(false)
          expect(inviter.notifications.count).to eq(1)
        end

        it "should redeem the invite but not approve the user when invite is created by a regular user" do
          inviter = invite.invited_by
          user = invite_redeemer.redeem

          expect(user.name).to eq(name)
          expect(user.username).to eq(username)
          expect(user.invited_by).to eq(inviter)
          expect(user.approved).to eq(false)

          expect(inviter.notifications.count).to eq(1)
        end

        it "should redeem the invite and approve the user when user email is in auto_approve_email_domains setting" do
          SiteSetting.auto_approve_email_domains = "example.com"
          user = invite_redeemer.redeem

          expect(user.name).to eq(name)
          expect(user.username).to eq(username)
          expect(user.approved).to eq(true)
          expect(user.approved_by).to eq(Discourse.system_user)
        end
      it "should redeem the invite if invited by non staff and approve if staff not required to approve" do
        expect(user.approved).to eq(false)
      it "should delete invite if invited_by user has been removed" do
        invite.invited_by.destroy!
        expect { invite.reload }.to raise_error(ActiveRecord::RecordNotFound)
      it "can set password" do
        user = InviteRedeemer.new(invite: invite, email: invite.email, username: username, name: name, password: password).redeem
        expect(user).to have_password
        expect(user.confirm_password?(password)).to eq(true)
        expect(user.approved).to eq(false)
      end
      it "can set custom fields" do
        required_field = Fabricate(:user_field)
        optional_field = Fabricate(:user_field, required: false)
        user_fields = {
          required_field.id.to_s => 'value1',
          optional_field.id.to_s => 'value2'
        }
        user = InviteRedeemer.new(invite: invite, email: invite.email, username: username, name: name, password: password, user_custom_fields: user_fields).redeem

        expect(user).to be_present
        expect(user.custom_fields["user_field_#{required_field.id}"]).to eq('value1')
        expect(user.custom_fields["user_field_#{optional_field.id}"]).to eq('value2')
      end
      it "does not add user to group if inviter does not have permissions" do
        group = Fabricate(:group, grant_trust_level: 2)
        InvitedGroup.create(group_id: group.id, invite_id: invite.id)
        user = InviteRedeemer.new(invite: invite, email: invite.email, username: username, name: name, password: password).redeem
        expect(user.group_users.count).to eq(0)
      end
      it "adds user to group" do
        group = Fabricate(:group, grant_trust_level: 2)
        InvitedGroup.create(group_id: group.id, invite_id: invite.id)
        group.add_owner(invite.invited_by)
        user = InviteRedeemer.new(invite: invite, email: invite.email, username: username, name: name, password: password).redeem
        expect(user.group_users.count).to eq(4)
        expect(user.trust_level).to eq(2)
      end
      it "adds an entry to the group logs when the invited user is added to a group" do
        group = Fabricate(:group)
        InvitedGroup.create(group_id: group.id, invite_id: invite.id)
        group.add_owner(invite.invited_by)

        GroupHistory.destroy_all

        user = InviteRedeemer.new(
          invite: invite,
          email: invite.email,
          username: username,
          name: name,
          password: password
        ).redeem

        expect(group.reload.usernames.split(",")).to include(user.username)
        expect(GroupHistory.exists?(
          target_user_id: user.id,
          acting_user: invite.invited_by.id,
          group_id: group.id,
          action: GroupHistory.actions[:add_user_to_group]
        )).to eq(true)
      end
      it "only allows one user to be created per invite" do
        user = invite_redeemer.redeem
        invite.reload
        user.email = "john@example.com"
        user.save!
        another_invite_redeemer = InviteRedeemer.new(invite: invite, email: invite.email, username: username, name: name)
        another_user = another_invite_redeemer.redeem
        expect(another_user).to eq(nil)
      end
      it "should correctly update the invite redeemed_at date" do
        SiteSetting.invite_expiry_days = 2
        invite.update!(created_at: 10.days.ago)
        inviter = invite.invited_by
        inviter.admin = true
        user = invite_redeemer.redeem
        invite.reload
        expect(user.invited_by).to eq(inviter)
        expect(inviter.notifications.count).to eq(1)
        expect(invite.invited_users.first).to be_present
      end
      it "raises an error if the email does not match the invite email" do
        redeemer = InviteRedeemer.new(invite: invite, email: "blah@test.com", username: username, name: name)
        expect { redeemer.redeem }.to raise_error(ActiveRecord::RecordNotSaved, I18n.t("invite.not_matching_email"))
      end
      it "adds the user to the appropriate private topic and no others" do
        topic1 = Fabricate(:private_message_topic)
        topic2 = Fabricate(:private_message_topic)
        TopicInvite.create(invite: invite, topic: topic1)
        user = InviteRedeemer.new(invite: invite, email: invite.email, username: username, name: name, password: password).redeem
        expect(TopicAllowedUser.exists?(topic: topic1, user: user)).to eq(true)
        expect(TopicAllowedUser.exists?(topic: topic2, user: user)).to eq(false)
      end
      context "when a redeeming user is passed in" do
        fab!(:redeeming_user) { Fabricate(:user, email: "foobar@example.com") }
        it "raises an error if the email does not match the invite email" do
          redeeming_user.update!(email: "foo@bar.com")
          redeemer = InviteRedeemer.new(invite: invite, redeeming_user: redeeming_user)
          expect { redeemer.redeem }.to raise_error(ActiveRecord::RecordNotSaved, I18n.t("invite.not_matching_email"))
        end
        it "adds the user to the appropriate private topic and no others" do
          topic1 = Fabricate(:private_message_topic)
          topic2 = Fabricate(:private_message_topic)
          TopicInvite.create(invite: invite, topic: topic1)
          InviteRedeemer.new(invite: invite, redeeming_user: redeeming_user).redeem
          expect(TopicAllowedUser.exists?(topic: topic1, user: redeeming_user)).to eq(true)
          expect(TopicAllowedUser.exists?(topic: topic2, user: redeeming_user)).to eq(false)
        end
        it "does not create a topic allowed user record if the invited user is already in the topic" do
          topic1 = Fabricate(:private_message_topic)
          TopicInvite.create(invite: invite, topic: topic1)
          TopicAllowedUser.create(topic: topic1, user: redeeming_user)
          expect { InviteRedeemer.new(invite: invite, redeeming_user: redeeming_user).redeem }.not_to change { TopicAllowedUser.count }
        end
      context "for an email invite" do
        fab!(:invite) { Fabricate(:invite, email: 'test@example.com') }
        fab!(:user) { Fabricate(:user, email: 'test@example.com') }
        fab!(:group) { Fabricate(:group) }
        it 'redeems the invitation and creates the invite accepted notification' do
          put "/invites/show/#{invite.invite_key}.json", params: { id: invite.invite_key }
          expect(response.status).to eq(200)
          expect(response.parsed_body["message"]).to eq(I18n.t("invite.existing_user_success"))
          invite.reload
          expect(invite.invited_users.first.user).to eq(user)
          expect(invite.redeemed?).to be_truthy
          expect(
            Notification.exists?(
              user: invite.invited_by, notification_type: Notification.types[:invitee_accepted]
            )
          ).to eq(true)
        end
        it 'redirects to the first topic the user was invited to and creates the topic notification' do
          topic = Fabricate(:topic)
          TopicInvite.create!(invite: invite, topic: topic)
          put "/invites/show/#{invite.invite_key}.json", params: { id: invite.invite_key }
          expect(response.status).to eq(200)
          expect(response.parsed_body['redirect_to']).to eq(topic.relative_url)
          expect(Notification.where(notification_type: Notification.types[:invited_to_topic], topic: topic).count).to eq(1)
        end
        it "adds the user to the private topic" do
          topic = Fabricate(:private_message_topic)
          TopicInvite.create!(invite: invite, topic: topic)
          put "/invites/show/#{invite.invite_key}.json", params: { id: invite.invite_key }
          expect(response.status).to eq(200)
          expect(response.parsed_body['redirect_to']).to eq(topic.relative_url)
          expect(TopicAllowedUser.exists?(user: user, topic: topic)).to eq(true)
        end
        it "adds the user to the groups specified on the invite and allows them to access the secure topic" do
          group.add_owner(invite.invited_by)
          secured_category = Fabricate(:category)
          secured_category.permissions = { group.name => :full }
          secured_category.save!

          topic = Fabricate(:topic, category: secured_category)
          TopicInvite.create!(invite: invite, topic: topic)
          InvitedGroup.create!(invite: invite, group: group)
          expect(response.status).to eq(200)
          expect(response.parsed_body["message"]).to eq(I18n.t("invite.existing_user_success"))
          expect(response.parsed_body['redirect_to']).to eq(topic.relative_url)
          invite.reload
          expect(invite.redeemed?).to be_truthy
          expect(user.reload.groups).to include(group)
          expect(Notification.where(notification_type: Notification.types[:invited_to_topic], topic: topic).count).to eq(1)
        end
        it "does not try to log in the user automatically" do
          expect do
            put "/invites/show/#{invite.invite_key}.json", params: { id: invite.invite_key }
          end.not_to change { UserAuthToken.count }
          expect(response.status).to eq(200)
          expect(response.parsed_body["message"]).to eq(I18n.t("invite.existing_user_success"))
        end

        it "errors if the user's email doesn't match the invite email" do
          user.update!(email: "blah@test.com")
          put "/invites/show/#{invite.invite_key}.json", params: { id: invite.invite_key }
          expect(response.status).to eq(412)
          expect(response.parsed_body["message"]).to eq(I18n.t("invite.not_matching_email"))
        end

        it "errors if the user's email domain doesn't match the invite domain" do
          user.update!(email: "blah@test.com")
          invite.update!(email: nil, domain: "example.com")
          put "/invites/show/#{invite.invite_key}.json", params: { id: invite.invite_key }
          expect(response.status).to eq(412)
          expect(response.parsed_body["message"]).to eq(I18n.t("invite.domain_not_allowed"))
        end
      context "for an invite link" do
        fab!(:invite) { Fabricate(:invite, email: nil) }
        fab!(:user) { Fabricate(:user, email: 'test@example.com') }
        fab!(:group) { Fabricate(:group) }

        it 'redeems the invitation and creates the invite accepted notification' do
          put "/invites/show/#{invite.invite_key}.json", params: { id: invite.invite_key }
          expect(response.status).to eq(200)
          expect(response.parsed_body["message"]).to eq(I18n.t("invite.existing_user_success"))
          invite.reload
          expect(invite.invited_users.first.user).to eq(user)
          expect(invite.redeemed?).to be_truthy
          expect(
            Notification.exists?(
              user: invite.invited_by, notification_type: Notification.types[:invitee_accepted]
            )
          ).to eq(true)
        end

        it 'redirects to the first topic the user was invited to and creates the topic notification' do
          topic = Fabricate(:topic)
          TopicInvite.create!(invite: invite, topic: topic)
          put "/invites/show/#{invite.invite_key}.json", params: { id: invite.invite_key }
          expect(response.status).to eq(200)
          expect(response.parsed_body['redirect_to']).to eq(topic.relative_url)
          expect(Notification.where(notification_type: Notification.types[:invited_to_topic], topic: topic).count).to eq(1)
        end

        it "adds the user to the groups specified on the invite and allows them to access the secure topic" do
          group.add_owner(invite.invited_by)
          secured_category = Fabricate(:category)
          secured_category.permissions = { group.name => :full }
          secured_category.save!

          topic = Fabricate(:topic, category: secured_category)
          TopicInvite.create!(invite: invite, topic: topic)
          InvitedGroup.create!(invite: invite, group: group)

          put "/invites/show/#{invite.invite_key}.json", params: { id: invite.invite_key }
          expect(response.status).to eq(200)
          expect(response.parsed_body["message"]).to eq(I18n.t("invite.existing_user_success"))
          expect(response.parsed_body['redirect_to']).to eq(topic.relative_url)
          invite.reload
          expect(invite.redeemed?).to be_truthy
          expect(user.reload.groups).to include(group)
          expect(Notification.where(notification_type: Notification.types[:invited_to_topic], topic: topic).count).to eq(1)
        end

        it "does not try to log in the user automatically" do
          expect do
            put "/invites/show/#{invite.invite_key}.json", params: { id: invite.invite_key }
          end.not_to change { UserAuthToken.count }
          expect(response.status).to eq(200)
          expect(response.parsed_body["message"]).to eq(I18n.t("invite.existing_user_success"))
        end