diff --git a/app/controllers/session_controller.rb b/app/controllers/session_controller.rb index b4b8d31ae8a..e4a3effabab 100644 --- a/app/controllers/session_controller.rb +++ b/app/controllers/session_controller.rb @@ -648,7 +648,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 54a309b550d..0377dfb0e2e 100644 --- a/app/models/invite.rb +++ b/app/models/invite.rb @@ -66,8 +66,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? diff --git a/app/models/invite_redeemer.rb b/app/models/invite_redeemer.rb index fd993840d8b..056f8bdb856 100644 --- a/app/models/invite_redeemer.rb +++ b/app/models/invite_redeemer.rb @@ -1,7 +1,21 @@ # 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. InviteRedeemer = Struct.new(:invite, :email, :username, :name, :password, :user_custom_fields, :ip_address, :session, :email_token, :redeeming_user, keyword_init: true) do def redeem + ensure_email_is_present!(email) Invite.transaction do if can_redeem_invite? && mark_invite_redeemed process_invitation @@ -10,6 +24,27 @@ InviteRedeemer = Struct.new(:invite, :email, :username, :name, :password, :user_ end end + # 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 #{self.redeeming_user.present? ? "(ID: #{self.redeeming_user.id})" : "not"} present.", + ) + end + + if email.blank? && self.invite.is_email_invite? + self.email = self.invite.email + elsif self.redeeming_user.present? + self.email = self.redeeming_user.email + else + self.email = email + end + + raise Discourse::InvalidParameters if self.email.blank? + end + # extracted from User cause it is very specific to invites 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) @@ -72,7 +107,10 @@ InviteRedeemer = Struct.new(:invite, :email, :username, :name, :password, :user_ 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 @@ -83,24 +121,26 @@ InviteRedeemer = Struct.new(:invite, :email, :username, :name, :password, :user_ 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 @@ -161,9 +201,18 @@ InviteRedeemer = Struct.new(:invite, :email, :username, :name, :password, :user_ 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 @@ -185,15 +234,17 @@ InviteRedeemer = Struct.new(:invite, :email, :username, :name, :password, :user_ 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 00000000000..45066ab7841 --- /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[6.1] + 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 579aa730367..ccb27df580e 100644 --- a/spec/models/invite_redeemer_spec.rb +++ b/spec/models/invite_redeemer_spec.rb @@ -5,6 +5,85 @@ require 'rails_helper' 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.redeem }.to change { User.count } + expect(redeemer.email).to eq("blah@test.com") + 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.redeem }.not_to change { User.count } + expect(redeemer.email).to eq(redeeming_user.email) + 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.redeem }.not_to change { User.count } + expect(redeemer.email).to eq(redeeming_user.email) + 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).redeem }.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.redeem }.to change { User.count } + expect(redeemer.email).to eq("foobar@example.com") + 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.redeem }.to raise_error(ActiveRecord::RecordNotSaved, I18n.t("invite.not_matching_email")) + expect(redeemer.email).to eq(redeeming_user.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.redeem }.to raise_error(ActiveRecord::RecordNotSaved, I18n.t("invite.not_matching_email")) + expect(redeemer.email).to eq("foobar@example.com") + 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.redeem }.to change { User.count } + expect(redeemer.email).to eq("foobar@example.com") + 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') @@ -92,10 +171,10 @@ describe InviteRedeemer do Jobs.run_immediately! invite = Fabricate(:invite, - invited_by: admin, - email: 'walter.white@email.com', - emailed_status: Invite.emailed_status_types[:sent], - ) + invited_by: admin, + email: 'walter.white@email.com', + emailed_status: Invite.emailed_status_types[:sent], + ) user = InviteRedeemer.create_user_from_invite( invite: invite, @@ -121,210 +200,236 @@ describe InviteRedeemer do 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) - end + 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 + 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(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 + 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 + 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 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 "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")) - end - end - - context 'with domain' do - fab!(:invite) { Fabricate(:invite, email: nil, domain: "test.com") } - - it "raises an error if the email domain does not match the invite domain" do - redeemer = InviteRedeemer.new(invite: invite, email: "blah@somesite.com", username: username, name: name) - expect { redeemer.redeem }.to raise_error(ActiveRecord::RecordNotSaved, I18n.t("invite.domain_not_allowed")) - end - - context "when a redeeming user is passed in" do - fab!(:redeeming_user) { Fabricate(:user, email: "foo@test.com") } - - it "raises an error if the user's email domain does not match the invite domain" 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.domain_not_allowed")) + 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 - end - context 'invite_link' do - fab!(:invite_link) { Fabricate(:invite, email: nil, max_redemptions_allowed: 5, expires_at: 1.month.from_now, emailed_status: Invite.emailed_status_types[:not_required]) } - let(:invite_redeemer) { InviteRedeemer.new(invite: invite_link, email: 'foo@example.com') } - - it 'works as expected' 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 - invite_link.reload - expect(user.send_welcome_message).to eq(true) - expect(user.trust_level).to eq(SiteSetting.default_invitee_trust_level) - expect(user.active).to eq(false) - expect(invite_link.redemption_count).to eq(1) + 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 not redeem the invite if InvitedUser record already exists for email" do - user = invite_redeemer.redeem - invite_link.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 - another_invite_redeemer = InviteRedeemer.new(invite: invite_link, email: 'foo@example.com') + 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 "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 redeem the invite if InvitedUser record does not exists for email" do - user = invite_redeemer.redeem - invite_link.reload + it "should correctly update the invite redeemed_at date" do + SiteSetting.invite_expiry_days = 2 + invite.update!(created_at: 10.days.ago) - another_invite_redeemer = InviteRedeemer.new(invite: invite_link, email: 'bar@example.com') - another_user = another_invite_redeemer.redeem - expect(another_user.is_a?(User)).to eq(true) + 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 is already being used by an existing user" do - Fabricate(:user, email: 'foo@example.com') - expect { invite_redeemer.redeem }.to raise_error(ActiveRecord::RecordInvalid, /Primary email has already been taken/) + 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: 'foo@example.com') } + fab!(:redeeming_user) { Fabricate(:user, email: "foobar@example.com") } - it "does not create a new user" do - expect do - InviteRedeemer.new(invite: invite_link, redeeming_user: redeeming_user).redeem - end.not_to change { User.count } + 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 'with domain' do + fab!(:invite) { Fabricate(:invite, email: nil, domain: "test.com") } + + it "raises an error if the email domain does not match the invite domain" do + redeemer = InviteRedeemer.new(invite: invite, email: "blah@somesite.com", username: username, name: name) + expect { redeemer.redeem }.to raise_error(ActiveRecord::RecordNotSaved, I18n.t("invite.domain_not_allowed")) + end + + context "when a redeeming user is passed in" do + fab!(:redeeming_user) { Fabricate(:user, email: "foo@test.com") } + + it "raises an error if the user's email domain does not match the invite domain" 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.domain_not_allowed")) + end + end + end + + context 'invite_link' do + fab!(:invite_link) { Fabricate(:invite, email: nil, max_redemptions_allowed: 5, expires_at: 1.month.from_now, emailed_status: Invite.emailed_status_types[:not_required]) } + let(:invite_redeemer) { InviteRedeemer.new(invite: invite_link, email: 'foo@example.com') } + + it 'works as expected' do + user = invite_redeemer.redeem + invite_link.reload + + expect(user.send_welcome_message).to eq(true) + expect(user.trust_level).to eq(SiteSetting.default_invitee_trust_level) + expect(user.active).to eq(false) + expect(invite_link.redemption_count).to eq(1) + end + + it "should not redeem the invite if InvitedUser record already exists for email" do + user = invite_redeemer.redeem + invite_link.reload + + another_invite_redeemer = InviteRedeemer.new(invite: invite_link, email: 'foo@example.com') + another_user = another_invite_redeemer.redeem + expect(another_user).to eq(nil) + end + + it "should redeem the invite if InvitedUser record does not exists for email" do + user = invite_redeemer.redeem + invite_link.reload + + another_invite_redeemer = InviteRedeemer.new(invite: invite_link, email: 'bar@example.com') + another_user = another_invite_redeemer.redeem + expect(another_user.is_a?(User)).to eq(true) + end + + it "raises an error if the email is already being used by an existing user" do + Fabricate(:user, email: 'foo@example.com') + expect { invite_redeemer.redeem }.to raise_error(ActiveRecord::RecordInvalid, /Primary email has already been taken/) + end + + context "when a redeeming user is passed in" do + fab!(:redeeming_user) { Fabricate(:user, email: 'foo@example.com') } + + it "does not create a new user" do + expect do + InviteRedeemer.new(invite: invite_link, redeeming_user: redeeming_user).redeem + end.not_to change { User.count } + end + end end end end - end end diff --git a/spec/requests/invites_controller_spec.rb b/spec/requests/invites_controller_spec.rb index 358d3dfd502..d2564f68b2f 100644 --- a/spec/requests/invites_controller_spec.rb +++ b/spec/requests/invites_controller_spec.rb @@ -801,76 +801,144 @@ 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