diff --git a/app/controllers/session_controller.rb b/app/controllers/session_controller.rb index d05ba4351f..490f4db0ed 100644 --- a/app/controllers/session_controller.rb +++ b/app/controllers/session_controller.rb @@ -760,7 +760,7 @@ class SessionController < ApplicationController 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? diff --git a/app/models/invite.rb b/app/models/invite.rb index 917b486f29..1910700185 100644 --- a/app/models/invite.rb +++ b/app/models/invite.rb @@ -74,8 +74,16 @@ class Invite < ActiveRecord::Base 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? @@ -201,8 +209,6 @@ class Invite < ActiveRecord::Base ) return if !redeemable? - email = self.email if email.blank? && !is_invite_link? - InviteRedeemer.new( invite: self, email: email, diff --git a/app/models/invite_redeemer.rb b/app/models/invite_redeemer.rb index 93496140fa..6a7ae4b22a 100644 --- a/app/models/invite_redeemer.rb +++ b/app/models/invite_redeemer.rb @@ -1,5 +1,18 @@ # 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, @@ -13,7 +26,7 @@ class InviteRedeemer :redeeming_user def initialize( - invite: nil, + invite:, email: nil, username: nil, name: nil, @@ -23,9 +36,7 @@ class InviteRedeemer session: nil, email_token: nil, redeeming_user: nil) - @invite = invite - @email = email @username = username @name = name @password = password @@ -34,6 +45,8 @@ class InviteRedeemer @session = session @email_token = email_token @redeeming_user = redeeming_user + + ensure_email_is_present!(email) end def redeem @@ -45,7 +58,29 @@ class InviteRedeemer 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 @@ -107,7 +142,10 @@ class InviteRedeemer 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 @@ -118,24 +156,26 @@ class InviteRedeemer 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 @@ -148,6 +188,10 @@ class InviteRedeemer 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) @@ -196,9 +240,18 @@ class InviteRedeemer 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 @@ -221,15 +274,17 @@ class InviteRedeemer 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") 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 0000000000..d161a9f16d --- /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 caa1470d7b..a978a688ec 100644 --- a/spec/models/invite_redeemer_spec.rb +++ b/spec/models/invite_redeemer_spec.rb @@ -3,6 +3,83 @@ 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') @@ -113,172 +190,199 @@ RSpec.describe InviteRedeemer do 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 + 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 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 + 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" + 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 - 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) + 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 - 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 + it "should correctly update the invite redeemed_at date" do + SiteSetting.invite_expiry_days = 2 + invite.update!(created_at: 10.days.ago) - 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 + inviter = invite.invited_by + inviter.admin = true + user = invite_redeemer.redeem + invite.reload - 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") } + 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 - redeeming_user.update!(email: "foo@bar.com") - redeemer = InviteRedeemer.new(invite: invite, redeeming_user: redeeming_user) + 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 + end end context 'with domain' do diff --git a/spec/requests/invites_controller_spec.rb b/spec/requests/invites_controller_spec.rb index e450296d87..8410bd66ef 100644 --- a/spec/requests/invites_controller_spec.rb +++ b/spec/requests/invites_controller_spec.rb @@ -912,76 +912,144 @@ RSpec.describe InvitesController do 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 "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 + it 'redeems the invitation and creates the invite accepted notification' 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")) + 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) + + 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 + + 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 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 + 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 "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")) + 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