SECURITY: Require groups to be given when inviting to a restricted category. (#6715)

app/assets/javascripts/discourse/controllers/invite.js.es6

@@ -289,7 +289,7 @@ export default Ember.Controller.extend(ModalFunctionality, {
 
       model.setProperties({ saving: true, error: false });
 
-      const onerror = function(e) {
+      const onerror = e => {
         if (e.jqXHR.responseJSON && e.jqXHR.responseJSON.errors) {
           self.set("errorMessage", e.jqXHR.responseJSON.errors[0]);
         } else {

app/controllers/topics_controller.rb

@@ -563,7 +563,7 @@ class TopicsController < ApplicationController
       ))
     end
 
-    guardian.ensure_can_invite_to!(topic, groups)
+    guardian.ensure_can_invite_to!(topic)
     group_ids = groups.map(&:id)
 
     begin
@@ -576,7 +576,25 @@ class TopicsController < ApplicationController
           render json: success_json
         end
       else
-        render json: failed_json, status: 422
+        json = failed_json
+
+        unless topic.private_message?
+          group_names = topic.category
+            .visible_group_names(current_user)
+            .where(automatic: false)
+            .pluck(:name)
+            .join(", ")
+
+          if group_names.present?
+            json.merge!(errors: [
+              I18n.t("topic_invite.failed_to_invite",
+                group_names: group_names
+              )
+            ])
+          end
+        end
+
+        render json: json, status: 422
       end
     rescue Topic::UserExists => e
       render json: { errors: [e.message] }, status: 422

app/models/category.rb

@@ -478,6 +478,10 @@ class Category < ActiveRecord::Base
     self.name_lower = name.downcase if self.name
   end
 
+  def visible_group_names(user)
+    self.groups.visible_groups(user)
+  end
+
   def secure_group_ids
     if self.read_restricted?
       groups.pluck("groups.id")

app/models/invite.rb

@@ -57,19 +57,6 @@ class Invite < ActiveRecord::Base
     InviteRedeemer.new(self, username, name, password, user_custom_fields).redeem unless expired? || destroyed? || !link_valid?
   end
 
-  def self.extend_permissions(topic, user, invited_by)
-    if topic.private_message?
-      topic.grant_permission_to_user(user.email)
-    elsif topic.category && topic.category.groups.any?
-      if Guardian.new(invited_by).can_invite_via_email?(topic)
-        (topic.category.groups - user.groups).each do |group|
-          group.add(user)
-          GroupActionLogger.new(Discourse.system_user, group).log_add_user_to_group(user)
-        end
-      end
-    end
-  end
-
   def self.invite_by_email(email, invited_by, topic = nil, group_ids = nil, custom_message = nil)
     create_invite_by_email(email, invited_by,
       topic: topic,
@@ -103,8 +90,11 @@ class Invite < ActiveRecord::Base
     lower_email = Email.downcase(email)
 
     if user = find_user_by_email(lower_email)
-      extend_permissions(topic, user, invited_by) if topic
-      raise UserExists.new I18n.t("invite.user_exists", email: lower_email, username: user.username, base_path: Discourse.base_path)
+      raise UserExists.new(I18n.t("invite.user_exists",
+        email: lower_email,
+        username: user.username,
+        base_path: Discourse.base_path
+      ))
     end
 
     invite = Invite.with_deleted
@@ -134,14 +124,10 @@ class Invite < ActiveRecord::Base
 
     if group_ids.present?
       group_ids = group_ids - invite.invited_groups.pluck(:group_id)
+
       group_ids.each do |group_id|
         invite.invited_groups.create!(group_id: group_id)
       end
-    else
-      if topic && topic.category && Guardian.new(invited_by).can_invite_to?(topic)
-        group_ids = topic.category.groups.where(automatic: false).pluck(:id) - invite.invited_groups.pluck(:group_id)
-        group_ids.each { |group_id| invite.invited_groups.create!(group_id: group_id) }
-      end
     end
 
     Jobs.enqueue(:invite_email, invite_id: invite.id) if send_email

app/models/topic.rb

@@ -839,62 +839,29 @@ class Topic < ActiveRecord::Base
   def invite(invited_by, username_or_email, group_ids = nil, custom_message = nil)
     target_user = User.find_by_username_or_email(username_or_email)
     guardian = Guardian.new(invited_by)
+    is_email = username_or_email =~ /^.+@.+$/
 
-    if target_user && topic_allowed_users.where(user_id: target_user.id).exists?
-      raise UserExists.new(I18n.t("topic_invite.user_exists"))
-    end
-
-    return true if target_user && invite_existing_muted?(target_user, invited_by)
-
-    if private_message? && target_user && !guardian.can_send_private_message?(target_user)
-      raise UserExists.new(I18n.t("activerecord.errors.models.topic.attributes.base.cant_send_pm"))
-    end
-
-    if target_user && private_message? && topic_allowed_users.create!(user_id: target_user.id)
-      rate_limit_topic_invitation(invited_by)
-      add_small_action(invited_by, "invited_user", target_user.username)
-
-      create_invite_notification!(
-        target_user,
-        Notification.types[:invited_to_private_message],
-        invited_by.username
-      )
-
-      true
-    elsif username_or_email =~ /^.+@.+$/ && guardian.can_invite_via_email?(self)
-
-      if target_user
-        rate_limit_topic_invitation(invited_by)
-        Invite.extend_permissions(self, target_user, invited_by)
-
-        create_invite_notification!(
-          target_user,
-          Notification.types[:invited_to_topic],
-          invited_by.username
-        )
-      else
-        invite_by_email(invited_by, username_or_email, group_ids, custom_message)
+    if target_user
+      if topic_allowed_users.exists?(user_id: target_user.id)
+        raise UserExists.new(I18n.t("topic_invite.user_exists"))
       end
 
-      true
-    elsif target_user &&
-          rate_limit_topic_invitation(invited_by) &&
-          topic_allowed_users.create!(user_id: target_user.id)
+      if invite_existing_muted?(target_user, invited_by)
+        return true
+      end
 
-      create_invite_notification!(
-        target_user,
-        Notification.types[:invited_to_topic],
-        invited_by.username
+      if private_message?
+        !!invite_to_private_message(invited_by, target_user, guardian)
+      else
+        !!invite_to_topic(invited_by, target_user, group_ids, guardian)
+      end
+    elsif is_email && guardian.can_invite_via_email?(self)
+      !!Invite.invite_by_email(
+        username_or_email, invited_by, self, group_ids, custom_message
       )
-
-      true
     end
   end
 
-  def invite_by_email(invited_by, email, group_ids = nil, custom_message = nil)
-    Invite.invite_by_email(email, invited_by, self, group_ids, custom_message)
-  end
-
   def invite_existing_muted?(target_user, invited_by)
     if invited_by.id &&
        MutedUser.where(user_id: target_user.id, muted_user_id: invited_by.id)
@@ -1397,6 +1364,55 @@ class Topic < ActiveRecord::Base
 
   private
 
+  def invite_to_private_message(invited_by, target_user, guardian)
+    if !guardian.can_send_private_message?(target_user)
+      raise UserExists.new(I18n.t(
+        "activerecord.errors.models.topic.attributes.base.cant_send_pm"
+      ))
+    end
+
+    Topic.transaction do
+      rate_limit_topic_invitation(invited_by)
+      topic_allowed_users.create!(user_id: target_user.id)
+      add_small_action(invited_by, "invited_user", target_user.username)
+
+      create_invite_notification!(
+        target_user,
+        Notification.types[:invited_to_private_message],
+        invited_by.username
+      )
+    end
+  end
+
+  def invite_to_topic(invited_by, target_user, group_ids, guardian)
+    Topic.transaction do
+      rate_limit_topic_invitation(invited_by)
+
+      if group_ids
+        (
+          self.category.groups.where(id: group_ids).where(automatic: false) -
+          target_user.groups.where(automatic: false)
+        ).each do |group|
+          if guardian.can_edit_group?(group)
+            group.add(target_user)
+
+            GroupActionLogger
+              .new(invited_by, group)
+              .log_add_user_to_group(target_user)
+          end
+        end
+      end
+
+      if Guardian.new(target_user).can_see_topic?(self)
+        create_invite_notification!(
+          target_user,
+          Notification.types[:invited_to_topic],
+          invited_by.username
+        )
+      end
+    end
+  end
+
   def update_category_topic_count_by(num)
     if category_id.present?
       Category.where(['id = ?', category_id]).update_all("topic_count = topic_count " + (num > 0 ? '+' : '') + "#{num}")

config/locales/server.en.yml

@@ -190,6 +190,7 @@ en:
     error: "There was an error uploading that file. Please try again later."
 
   topic_invite:
+    failed_to_invite: "The user cannot be invited into this topic without a group membership in either one of the following groups: %{group_names}." 
     user_exists: "Sorry, that user has already been invited. You may only invite a user to a topic once."
 
   backup:

lib/guardian.rb

@@ -289,19 +289,25 @@ class Guardian
 
   def can_invite_to?(object, groups = nil)
     return false unless authenticated?
-    return true if is_admin?
+    is_topic = object.is_a?(Topic)
+    return true if is_admin? && !is_topic
     return false if (SiteSetting.max_invites_per_day.to_i == 0 && !is_staff?)
     return false unless can_see?(object)
     return false if groups.present?
 
-    if object.is_a?(Topic) && object.private_message?
-      return false unless SiteSetting.enable_personal_messages?
-      return false if object.reached_recipients_limit? && !is_staff?
-    end
+    if is_topic
+      if object.private_message?
+        return true if is_admin?
+        return false unless SiteSetting.enable_personal_messages?
+        return false if object.reached_recipients_limit? && !is_staff?
+      end
 
-    if object.is_a?(Topic) && object.category
-      if object.category.groups.any?
-        return true if object.category.groups.all? { |g| can_edit_group?(g) }
+      if (category = object.category) && category.read_restricted
+        if (groups = category.groups&.where(automatic: false))&.any?
+          return groups.any? { |g| can_edit_group?(g) } ? true : false
+        else
+          return false
+        end
       end
     end
 

spec/components/guardian_spec.rb

@@ -555,8 +555,8 @@ describe Guardian do
         expect(Guardian.new(user).can_invite_to?(private_topic)).to be_falsey
       end
 
-      it 'returns true for admin on private topic' do
-        expect(Guardian.new(admin).can_invite_to?(private_topic)).to be_truthy
+      it 'returns false for admin on private topic' do
+        expect(Guardian.new(admin).can_invite_to?(private_topic)).to be(false)
       end
 
       it 'returns true for a group owner' do
@@ -567,6 +567,49 @@ describe Guardian do
         SiteSetting.enable_personal_messages = false
         expect(Guardian.new(trust_level_2).can_invite_to?(topic)).to be_truthy
       end
+
+      describe 'for a private category for automatic and non-automatic group' do
+        let(:automatic_group) { Fabricate(:group, automatic: true) }
+        let(:group) { Fabricate(:group) }
+
+        let(:category) do
+          Fabricate(:category, read_restricted: true).tap do |category|
+            category.groups << automatic_group
+            category.groups << group
+          end
+        end
+
+        let(:topic) { Fabricate(:topic, category: category) }
+
+        it 'should return true for an admin user' do
+          expect(Guardian.new(admin).can_invite_to?(topic)).to eq(true)
+        end
+
+        it 'should return true for a group owner' do
+          expect(Guardian.new(group_owner).can_invite_to?(topic)).to eq(true)
+        end
+
+        it 'should return false for a normal user' do
+          expect(Guardian.new(user).can_invite_to?(topic)).to eq(false)
+        end
+      end
+
+      describe 'for a private category for automatic groups' do
+        let(:group) { Fabricate(:group, automatic: true) }
+
+        let(:category) do
+          Fabricate(:private_category, group: group, read_restricted: true)
+        end
+
+        let(:group_owner) { Fabricate(:user).tap { |user| group.add_owner(user) } }
+        let(:topic) { Fabricate(:topic, category: category) }
+
+        it 'should return false for all type of users' do
+          expect(Guardian.new(admin).can_invite_to?(topic)).to eq(false)
+          expect(Guardian.new(group_owner).can_invite_to?(topic)).to eq(false)
+          expect(Guardian.new(user).can_invite_to?(topic)).to eq(false)
+        end
+      end
     end
 
     describe "private messages" do

spec/models/invite_spec.rb

@@ -59,37 +59,38 @@ describe Invite do
 
       context 'email' do
         it 'enqueues a job to email the invite' do
-          Jobs.expects(:enqueue).with(:invite_email, has_key(:invite_id))
-          topic.invite_by_email(inviter, iceking)
+          expect do
+            Invite.invite_by_email(iceking, inviter, topic)
+          end.to change { Jobs::InviteEmail.jobs.size }
         end
       end
 
       context 'destroyed' do
         it "can invite the same user after their invite was destroyed" do
-          invite = topic.invite_by_email(inviter, iceking)
-          invite.destroy
-          invite = topic.invite_by_email(inviter, iceking)
+          Invite.invite_by_email(iceking, inviter, topic).destroy!
+          invite = Invite.invite_by_email(iceking, inviter, topic)
           expect(invite).to be_present
         end
       end
 
       context 'after created' do
-        before do
-          @invite = topic.invite_by_email(inviter, iceking)
-        end
+        let(:invite) { Invite.invite_by_email(iceking, inviter, topic) }
 
         it 'belongs to the topic' do
-          expect(topic.invites).to eq([@invite])
-          expect(@invite.topics).to eq([topic])
+          expect(topic.invites).to eq([invite])
+          expect(invite.topics).to eq([topic])
         end
 
         context 'when added by another user' do
           let(:coding_horror) { Fabricate(:coding_horror) }
-          let(:new_invite) { topic.invite_by_email(coding_horror, iceking) }
+
+          let(:new_invite) do
+            Invite.invite_by_email(iceking, coding_horror, topic)
+          end
 
           it 'returns a different invite' do
-            expect(new_invite).not_to eq(@invite)
-            expect(new_invite.invite_key).not_to eq(@invite.invite_key)
+            expect(new_invite).not_to eq(invite)
+            expect(new_invite.invite_key).not_to eq(invite.invite_key)
             expect(new_invite.topics).to eq([topic])
           end
 
@@ -97,24 +98,36 @@ describe Invite do
 
         context 'when adding a duplicate' do
           it 'returns the original invite' do
-            expect(topic.invite_by_email(inviter, 'iceking@adventuretime.ooo')).to eq(@invite)
-            expect(topic.invite_by_email(inviter, 'iceking@ADVENTURETIME.ooo')).to eq(@invite)
-            expect(topic.invite_by_email(inviter, 'ICEKING@adventuretime.ooo')).to eq(@invite)
+            %w{
+              iceking@adventuretime.ooo
+              iceking@ADVENTURETIME.ooo
+              ICEKING@adventuretime.ooo
+            }.each do |email|
+              expect(Invite.invite_by_email(
+                email, inviter, topic
+              )).to eq(invite)
+            end
           end
 
           it 'updates timestamp of existing invite' do
-            @invite.created_at = 10.days.ago
-            @invite.save
-            resend_invite = topic.invite_by_email(inviter, 'iceking@adventuretime.ooo')
+            invite.update!(created_at: 10.days.ago)
+
+            resend_invite = Invite.invite_by_email(
+              'iceking@adventuretime.ooo', inviter, topic
+            )
+
             expect(resend_invite.created_at).to be_within(1.minute).of(Time.zone.now)
           end
 
           it 'returns a new invite if the other has expired' do
             SiteSetting.invite_expiry_days = 1
-            @invite.created_at = 2.days.ago
-            @invite.save
-            new_invite = topic.invite_by_email(inviter, 'iceking@adventuretime.ooo')
-            expect(new_invite).not_to eq(@invite)
+            invite.update!(created_at: 2.days.ago)
+
+            new_invite = Invite.invite_by_email(
+              'iceking@adventuretime.ooo', inviter, topic
+            )
+
+            expect(new_invite).not_to eq(invite)
             expect(new_invite).not_to be_expired
           end
         end
@@ -123,64 +136,24 @@ describe Invite do
           let!(:another_topic) { Fabricate(:topic, user: topic.user) }
 
           it 'should be the same invite' do
-            @new_invite = another_topic.invite_by_email(inviter, iceking)
-            expect(@new_invite).to eq(@invite)
-            expect(another_topic.invites).to eq([@invite])
-            expect(@invite.topics).to match_array([topic, another_topic])
+            new_invite = Invite.invite_by_email(iceking, inviter, another_topic)
+            expect(new_invite).to eq(invite)
+            expect(another_topic.invites).to eq([invite])
+            expect(invite.topics).to match_array([topic, another_topic])
           end
-
         end
       end
     end
   end
 
-  context 'to a group-private topic' do
-    let(:group) { Fabricate(:group) }
-    let(:private_category)  { Fabricate(:private_category, group: group) }
-    let(:group_private_topic) { Fabricate(:topic, category: private_category) }
-    let(:inviter) { group_private_topic.user }
-
-    before do
-      group.add_owner(inviter)
-      @invite = group_private_topic.invite_by_email(inviter, iceking)
-    end
-
-    it 'should add the groups to the invite' do
-      expect(@invite.groups).to eq([group])
-    end
-
-    context 'when duplicated' do
-      it 'should not duplicate the groups' do
-        expect(group_private_topic.invite_by_email(inviter, iceking)).to eq(@invite)
-        expect(@invite.groups).to eq([group])
-      end
-    end
-
-    it 'verifies that inviter is authorized to invite user to a topic' do
-      tl2_user = Fabricate(:user, trust_level: 2)
-
-      invite = group_private_topic.invite_by_email(tl2_user, 'foo@bar.com')
-      expect(invite.groups.count).to eq(0)
-    end
-
-    context 'automatic groups' do
-      it 'should not add invited user to automatic groups' do
-        group.update!(automatic: true)
-        expect(group_private_topic.invite_by_email(Fabricate(:admin), iceking).groups.count).to eq(0)
-      end
-    end
-  end
-
   context 'an existing user' do
     let(:topic) { Fabricate(:topic, category_id: nil, archetype: 'private_message') }
     let(:coding_horror) { Fabricate(:coding_horror) }
 
     it "works" do
-      # doesn't create an invite
-      expect { topic.invite_by_email(topic.user, coding_horror.email) }.to raise_error(Invite::UserExists)
-
-      # gives the user permission to access the topic
-      expect(topic.allowed_users.include?(coding_horror)).to eq(true)
+      expect do
+        Invite.invite_by_email(coding_horror.email, topic.user, topic)
+      end.to raise_error(Invite::UserExists)
     end
 
   end
@@ -353,7 +326,6 @@ describe Invite do
         it 'adds the user to the topic_users of the first topic' do
           expect(another_topic.invite(another_tl2_user, user.username)).to be_truthy # invited via username
           expect(topic.allowed_users.include?(user)).to eq(true)
-          expect(another_topic.allowed_users.include?(user)).to eq(true)
         end
       end
     end

spec/models/topic_spec.rb

@@ -608,35 +608,85 @@ describe Topic do
     end
 
     describe 'public topic' do
-      def expect_the_right_notification_to_be_created
+      def expect_the_right_notification_to_be_created(inviter, invitee)
         notification = Notification.last
 
         expect(notification.notification_type)
           .to eq(Notification.types[:invited_to_topic])
 
-        expect(notification.user).to eq(another_user)
+        expect(notification.user).to eq(invitee)
         expect(notification.topic).to eq(topic)
 
         notification_data = JSON.parse(notification.data)
 
         expect(notification_data["topic_title"]).to eq(topic.title)
-        expect(notification_data["display_username"]).to eq(user.username)
+        expect(notification_data["display_username"]).to eq(inviter.username)
       end
 
       describe 'by username' do
         it 'should invite user into a topic' do
           topic.invite(user, another_user.username)
-
-          expect(topic.reload.allowed_users.last).to eq(another_user)
-          expect_the_right_notification_to_be_created
+          expect_the_right_notification_to_be_created(user, another_user)
         end
       end
 
       describe 'by email' do
         it 'should be able to invite a user' do
           expect(topic.invite(user, another_user.email)).to eq(true)
-          expect(topic.reload.allowed_users.last).to eq(another_user)
-          expect_the_right_notification_to_be_created
+          expect_the_right_notification_to_be_created(user, another_user)
+        end
+
+        describe 'when topic belongs to a private category' do
+          let(:group) { Fabricate(:group) }
+
+          let(:category) do
+            Fabricate(:category, groups: [group]).tap do |category|
+              category.set_permissions(group => :full)
+              category.save!
+            end
+          end
+
+          let(:topic) { Fabricate(:topic, category: category) }
+          let(:inviter) { Fabricate(:user).tap { |user| group.add_owner(user) } }
+          let(:invitee) { Fabricate(:user) }
+
+          describe 'as a group owner' do
+            it 'should be able to invite a user' do
+              expect do
+                expect(topic.invite(inviter, invitee.email, [group.id]))
+                  .to eq(true)
+              end.to change { Notification.count } &
+                     change { GroupHistory.count }
+
+              expect_the_right_notification_to_be_created(inviter, invitee)
+
+              group_history = GroupHistory.last
+
+              expect(group_history.acting_user).to eq(inviter)
+              expect(group_history.target_user).to eq(invitee)
+
+              expect(group_history.action).to eq(
+                GroupHistory.actions[:add_user_to_group]
+              )
+            end
+
+            describe 'when group ids are not given' do
+              it 'should not invite the user' do
+                expect do
+                  expect(topic.invite(inviter, invitee.email)).to eq(false)
+                end.to_not change { Notification.count }
+              end
+            end
+          end
+
+          describe 'as a normal user' do
+            it 'should not be able to invite a user' do
+              expect do
+                expect(topic.invite(Fabricate(:user), invitee.email, [group.id]))
+                  .to eq(false)
+              end.to_not change { Notification.count }
+            end
+          end
         end
 
         context "for a muted topic" do
@@ -2009,34 +2059,6 @@ describe Topic do
     expect(topic.save).to eq(true)
   end
 
-  context 'invite by group manager' do
-    let(:group_manager) { Fabricate(:user) }
-    let(:group) { Fabricate(:group).tap { |g| g.add_owner(group_manager) } }
-    let(:private_category)  { Fabricate(:private_category, group: group) }
-    let(:group_private_topic) { Fabricate(:topic, category: private_category, user: group_manager) }
-
-    context 'to an email' do
-      let(:randolph) { 'randolph@duke.ooo' }
-
-      it "should attach group to the invite" do
-        group_private_topic.invite(group_manager, randolph)
-        expect(Invite.last.groups).to eq([group])
-      end
-    end
-
-    # should work for an existing user - give access, send notification
-    context 'to an existing user' do
-      let(:walter) { Fabricate(:walter_white) }
-
-      it "should add user to the group" do
-        expect(Guardian.new(walter).can_see?(group_private_topic)).to be_falsey
-        group_private_topic.invite(group_manager, walter.email)
-        expect(walter.groups).to include(group)
-        expect(Guardian.new(walter).can_see?(group_private_topic)).to be_truthy
-      end
-    end
-  end
-
   it "Correctly sets #message_archived?" do
     topic = Fabricate(:private_message_topic)
     user = topic.user

spec/requests/invites_controller_spec.rb

@@ -215,9 +215,13 @@ describe InvitesController do
 
     context 'with a deleted invite' do
       let(:topic) { Fabricate(:topic) }
-      let(:invite) { topic.invite_by_email(topic.user, "iceking@adventuretime.ooo") }
+
+      let(:invite) do
+        Invite.invite_by_email("iceking@adventuretime.ooo", topic.user, topic)
+      end
+
       before do
-        invite.destroy
+        invite.destroy!
       end
 
       it "redirects to the root" do
@@ -233,7 +237,9 @@ describe InvitesController do
 
     context 'with a valid invite id' do
       let(:topic) { Fabricate(:topic) }
-      let(:invite) { topic.invite_by_email(topic.user, "iceking@adventuretime.ooo") }
+      let(:invite) do
+        Invite.invite_by_email("iceking@adventuretime.ooo", topic.user, topic)
+      end
 
       it 'redeems the invite' do
         put "/invites/show/#{invite.invite_key}.json"
@@ -323,7 +329,11 @@ describe InvitesController do
 
     context 'new registrations are disabled' do
       let(:topic) { Fabricate(:topic) }
-      let(:invite) { topic.invite_by_email(topic.user, "iceking@adventuretime.ooo") }
+
+      let(:invite) do
+        Invite.invite_by_email("iceking@adventuretime.ooo", topic.user, topic)
+      end
+
       before { SiteSetting.allow_new_registrations = false }
 
       it "doesn't redeem the invite" do
@@ -338,7 +348,11 @@ describe InvitesController do
 
     context 'user is already logged in' do
       let(:topic) { Fabricate(:topic) }
-      let(:invite) { topic.invite_by_email(topic.user, "iceking@adventuretime.ooo") }
+
+      let(:invite) do
+        Invite.invite_by_email("iceking@adventuretime.ooo", topic.user, topic)
+      end
+
       let!(:user) { sign_in(Fabricate(:user)) }
 
       it "doesn't redeem the invite" do

spec/requests/topics_controller_spec.rb

@@ -2071,14 +2071,46 @@ RSpec.describe TopicsController do
         let(:recipient) { 'jake@adventuretime.ooo' }
 
         it "should attach group to the invite" do
-
           post "/t/#{group_private_topic.id}/invite.json", params: {
-            user: recipient
+            user: recipient,
+            group_ids: "#{group.id},123"
           }
 
           expect(response.status).to eq(200)
           expect(Invite.find_by(email: recipient).groups).to eq([group])
         end
+
+        describe 'when group is available to automatic groups only' do
+          before do
+            group.update!(automatic: true)
+          end
+
+          it 'should return the right response' do
+            post "/t/#{group_private_topic.id}/invite.json", params: {
+              user: Fabricate(:user)
+            }
+
+            expect(response.status).to eq(403)
+          end
+        end
+
+        describe 'when user is not part of the required group' do
+          it 'should return the right response' do
+            post "/t/#{group_private_topic.id}/invite.json", params: {
+              user: Fabricate(:user)
+            }
+
+            expect(response.status).to eq(422)
+
+            response_body = JSON.parse(response.body)
+
+            expect(response_body["errors"]).to eq([
+              I18n.t("topic_invite.failed_to_invite",
+                group_names: group.name
+              )
+            ])
+          end
+        end
       end
 
       describe 'when topic id is invalid' do