diff --git a/app/assets/javascripts/discourse/controllers/invite.js.es6 b/app/assets/javascripts/discourse/controllers/invite.js.es6 index e9293ea8059..428b04b4f71 100644 --- a/app/assets/javascripts/discourse/controllers/invite.js.es6 +++ b/app/assets/javascripts/discourse/controllers/invite.js.es6 @@ -27,19 +27,22 @@ export default Ember.Controller.extend(ModalFunctionality, { return Discourse.User.currentProp("admin"); }.property(), - disabled: function() { - if (this.get('model.saving')) return true; - if (Ember.isEmpty(this.get('emailOrUsername'))) return true; - const emailOrUsername = this.get('emailOrUsername').trim(); + @computed('isAdmin', 'emailOrUsername', 'invitingToTopic', 'isPrivateTopic', 'model.groupNames', 'model.saving', 'model.details.can_invite_to') + disabled(isAdmin, emailOrUsername, invitingToTopic, isPrivateTopic, groupNames, saving, can_invite_to) { + if (saving) return true; + if (Ember.isEmpty(emailOrUsername)) return true; + const emailTrimmed = emailOrUsername.trim(); + // when inviting to forum, email must be valid - if (!this.get('invitingToTopic') && !emailValid(emailOrUsername)) return true; + if (!invitingToTopic && !emailValid(emailTrimmed)) return true; // normal users (not admin) can't invite users to private topic via email - if (!this.get('isAdmin') && this.get('isPrivateTopic') && emailValid(emailOrUsername)) return true; + if (!isAdmin && isPrivateTopic && emailValid(emailTrimmed)) return true; // when inviting to private topic via email, group name must be specified - if (this.get('isPrivateTopic') && Ember.isEmpty(this.get('model.groupNames')) && emailValid(emailOrUsername)) return true; - if (this.get('model.details.can_invite_to')) return false; + if (isPrivateTopic && Ember.isEmpty(groupNames) && emailValid(emailTrimmed)) return true; + + if (can_invite_to) return false; return false; - }.property('isAdmin', 'emailOrUsername', 'invitingToTopic', 'isPrivateTopic', 'model.groupNames', 'model.saving'), + }, disabledCopyLink: function() { if (this.get('hasCustomMessage')) return true; @@ -65,9 +68,10 @@ export default Ember.Controller.extend(ModalFunctionality, { return this.get('model') !== this.currentUser; }.property('model'), - showCopyInviteButton: function() { - return (!Discourse.SiteSettings.enable_sso && !this.get('isMessage')); - }.property('isMessage'), + @computed('isMessage', 'model.details.can_invite_via_email') + showCopyInviteButton(isMessage, can_invite_via_email) { + return (can_invite_via_email && !isMessage); + }, topicId: Ember.computed.alias('model.id'), @@ -83,32 +87,38 @@ export default Ember.Controller.extend(ModalFunctionality, { }.property('invitingToTopic'), // Show Groups? (add invited user to private group) - showGroups: function() { - return this.get('isAdmin') && (emailValid(this.get('emailOrUsername')) || this.get('isPrivateTopic') || !this.get('invitingToTopic')) && !Discourse.SiteSettings.enable_sso && Discourse.SiteSettings.enable_local_logins && !this.get('isMessage'); - }.property('isAdmin', 'emailOrUsername', 'isPrivateTopic', 'isMessage', 'invitingToTopic'), + @computed('isAdmin', 'emailOrUsername', 'isPrivateTopic', 'isMessage', 'invitingToTopic', 'model.details.can_invite_via_email') + showGroups(isAdmin, emailOrUsername, isPrivateTopic, isMessage, invitingToTopic, can_invite_via_email) { + return isAdmin && + can_invite_via_email && + !isMessage && + (emailValid(emailOrUsername) || isPrivateTopic || !invitingToTopic); + }, - showCustomMessage: function() { - return (this.get('model') === this.currentUser || emailValid(this.get('emailOrUsername'))); - }.property('emailOrUsername'), + @computed('emailOrUsername') + showCustomMessage(emailOrUsername) { + return (this.get('model') === this.currentUser || emailValid(emailOrUsername)); + }, // Instructional text for the modal. - inviteInstructions: function() { - if (Discourse.SiteSettings.enable_sso || !Discourse.SiteSettings.enable_local_logins) { - // inviting existing user when SSO enabled + @computed('isMessage', 'invitingToTopic', 'emailOrUsername', 'isPrivateTopic', 'isAdmin', 'model.details.can_invite_via_email') + inviteInstructions(isMessage, invitingToTopic, emailOrUsername, isPrivateTopic, isAdmin, can_invite_via_email) { + if (!can_invite_via_email) { + // can't invite via email, only existing users return I18n.t('topic.invite_reply.sso_enabled'); - } else if (this.get('isMessage')) { + } else if (isMessage) { // inviting to a message return I18n.t('topic.invite_private.email_or_username'); - } else if (this.get('invitingToTopic')) { + } else if (invitingToTopic) { // inviting to a private/public topic - if (this.get('isPrivateTopic') && !this.get('isAdmin')) { + if (isPrivateTopic && !isAdmin) { // inviting to a private topic and is not admin return I18n.t('topic.invite_reply.to_username'); } else { // when inviting to a topic, display instructions based on provided entity - if (Ember.isEmpty(this.get('emailOrUsername'))) { + if (Ember.isEmpty(emailOrUsername)) { return I18n.t('topic.invite_reply.to_topic_blank'); - } else if (emailValid(this.get('emailOrUsername'))) { + } else if (emailValid(emailOrUsername)) { this.set("inviteIcon", "envelope"); return I18n.t('topic.invite_reply.to_topic_email'); } else { @@ -120,7 +130,7 @@ export default Ember.Controller.extend(ModalFunctionality, { // inviting to forum return I18n.t('topic.invite_reply.to_forum'); } - }.property('isMessage', 'invitingToTopic', 'emailOrUsername'), + }, showGroupsClass: function() { return this.get('isPrivateTopic') ? 'required' : 'optional'; @@ -147,11 +157,12 @@ export default Ember.Controller.extend(ModalFunctionality, { return this.get('isMessage') ? I18n.t('topic.invite_private.error') : I18n.t('topic.invite_reply.error'); }.property('isMessage'), - placeholderKey: function() { - return (Discourse.SiteSettings.enable_sso || !Discourse.SiteSettings.enable_local_logins) ? - 'topic.invite_reply.username_placeholder' : - 'topic.invite_private.email_or_username_placeholder'; - }.property(), + @computed('model.details.can_invite_via_email') + placeholderKey(can_invite_via_email) { + return (can_invite_via_email) ? + 'topic.invite_private.email_or_username_placeholder' : + 'topic.invite_reply.username_placeholder'; + }, customMessagePlaceholder: function() { return I18n.t('invite.custom_message_placeholder'); diff --git a/app/models/invite.rb b/app/models/invite.rb index 6d20f14ed1e..e3e16c8ff49 100644 --- a/app/models/invite.rb +++ b/app/models/invite.rb @@ -59,7 +59,7 @@ class Invite < ActiveRecord::Base 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_to?(topic) && !SiteSetting.enable_sso + 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) diff --git a/app/models/topic.rb b/app/models/topic.rb index 1e8078051fd..9b85fc4f291 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -744,7 +744,7 @@ SQL end end - if username_or_email =~ /^.+@.+$/ && !SiteSetting.enable_sso && SiteSetting.enable_local_logins + if username_or_email =~ /^.+@.+$/ && Guardian.new(invited_by).can_invite_via_email?(self) # rate limit topic invite RateLimiter.new(invited_by, "topic-invitations-per-day", SiteSetting.max_topic_invitations_per_day, 1.day.to_i).performed! diff --git a/app/serializers/topic_view_serializer.rb b/app/serializers/topic_view_serializer.rb index 780d2b80c37..586eb35baad 100644 --- a/app/serializers/topic_view_serializer.rb +++ b/app/serializers/topic_view_serializer.rb @@ -115,6 +115,7 @@ class TopicViewSerializer < ApplicationSerializer result[:can_recover] = true if scope.can_recover_topic?(object.topic) result[:can_remove_allowed_users] = true if scope.can_remove_allowed_users?(object.topic) result[:can_invite_to] = true if scope.can_invite_to?(object.topic) + result[:can_invite_via_email] = true if scope.can_invite_via_email?(object.topic) result[:can_create_post] = true if scope.can_create?(Post, object.topic) result[:can_reply_as_new_topic] = true if scope.can_reply_as_new_topic?(object.topic) result[:can_flag_topic] = actions_summary.any? { |a| a[:can_act] } diff --git a/lib/guardian.rb b/lib/guardian.rb index bd678fd811b..9f479154909 100644 --- a/lib/guardian.rb +++ b/lib/guardian.rb @@ -229,7 +229,6 @@ class Guardian def can_invite_to?(object, group_ids=nil) return false if ! authenticated? - return false unless (!SiteSetting.must_approve_users? || is_staff?) return true if is_admin? return false if (SiteSetting.max_invites_per_day.to_i == 0 && !is_staff?) return false if ! can_see?(object) @@ -244,6 +243,11 @@ class Guardian user.has_trust_level?(TrustLevel[2]) end + def can_invite_via_email?(object) + return false unless can_invite_to?(object) + !SiteSetting.enable_sso && SiteSetting.enable_local_logins && (!SiteSetting.must_approve_users? || is_staff?) + end + def can_bulk_invite_to_forum?(user) user.admin? end diff --git a/spec/components/guardian_spec.rb b/spec/components/guardian_spec.rb index 3bd576bcc43..e0ce2a29a4a 100644 --- a/spec/components/guardian_spec.rb +++ b/spec/components/guardian_spec.rb @@ -341,16 +341,6 @@ describe Guardian do expect(Guardian.new(moderator).can_invite_to?(topic)).to be_truthy end - it 'returns true when the site requires approving users and is mod' do - SiteSetting.expects(:must_approve_users?).returns(true) - expect(Guardian.new(moderator).can_invite_to?(topic)).to be_truthy - end - - it 'returns false when the site requires approving users and is regular' do - SiteSetting.expects(:must_approve_users?).returns(true) - expect(Guardian.new(coding_horror).can_invite_to?(topic)).to be_falsey - end - it 'returns false for normal user on private topic' do expect(Guardian.new(user).can_invite_to?(private_topic)).to be_falsey end @@ -364,6 +354,38 @@ describe Guardian do end end + describe 'can_invite_via_email?' do + it 'returns true for all (tl2 and above) users when sso is disabled, local logins are enabled, user approval is not required' do + expect(Guardian.new(trust_level_2).can_invite_via_email?(topic)).to be_truthy + expect(Guardian.new(moderator).can_invite_via_email?(topic)).to be_truthy + expect(Guardian.new(admin).can_invite_via_email?(topic)).to be_truthy + end + + it 'returns false for all users when sso is enabled' do + SiteSetting.enable_sso = true + + expect(Guardian.new(trust_level_2).can_invite_via_email?(topic)).to be_falsey + expect(Guardian.new(moderator).can_invite_via_email?(topic)).to be_falsey + expect(Guardian.new(admin).can_invite_via_email?(topic)).to be_falsey + end + + it 'returns false for all users when local logins are disabled' do + SiteSetting.enable_local_logins = false + + expect(Guardian.new(trust_level_2).can_invite_via_email?(topic)).to be_falsey + expect(Guardian.new(moderator).can_invite_via_email?(topic)).to be_falsey + expect(Guardian.new(admin).can_invite_via_email?(topic)).to be_falsey + end + + it 'returns correct valuse when user approval is required' do + SiteSetting.must_approve_users = true + + expect(Guardian.new(trust_level_2).can_invite_via_email?(topic)).to be_falsey + expect(Guardian.new(moderator).can_invite_via_email?(topic)).to be_truthy + expect(Guardian.new(admin).can_invite_via_email?(topic)).to be_truthy + end + end + describe 'can_see?' do it 'returns false with a nil object' do diff --git a/spec/mailers/invite_mailer_spec.rb b/spec/mailers/invite_mailer_spec.rb index ff61f39987b..3dc8a007f11 100644 --- a/spec/mailers/invite_mailer_spec.rb +++ b/spec/mailers/invite_mailer_spec.rb @@ -73,7 +73,8 @@ describe InviteMailer do context "invite to topic" do - let(:topic) { Fabricate(:topic, excerpt: "Topic invite support is now available in Discourse!") } + let(:trust_level_2) { build(:user, trust_level: 2) } + let(:topic) { Fabricate(:topic, excerpt: "Topic invite support is now available in Discourse!", user: trust_level_2) } let(:invite) { topic.invite(topic.user, 'name@example.com') } context "default invite message" do diff --git a/spec/models/invite_spec.rb b/spec/models/invite_spec.rb index 17702be6b9e..fab03714dd6 100644 --- a/spec/models/invite_spec.rb +++ b/spec/models/invite_spec.rb @@ -330,25 +330,24 @@ describe Invite do end context 'invited to topics' do - let!(:topic) { Fabricate(:private_message_topic) } + let(:tl2_user) { Fabricate(:user, trust_level: 2) } + let!(:topic) { Fabricate(:private_message_topic, user: tl2_user) } let!(:invite) { topic.invite(topic.user, 'jake@adventuretime.ooo') } context 'redeem topic invite' do - it 'adds the user to the topic_users' do user = invite.redeem topic.reload expect(topic.allowed_users.include?(user)).to eq(true) expect(Guardian.new(user).can_see?(topic)).to eq(true) end - end context 'invited by another user to the same topic' do - let(:coding_horror) { User.find_by(username: "CodingHorror") } - let!(:another_invite) { topic.invite(coding_horror, 'jake@adventuretime.ooo') } + let(:another_tl2_user) { Fabricate(:user, trust_level: 2) } + let!(:another_invite) { topic.invite(another_tl2_user, 'jake@adventuretime.ooo') } let!(:user) { invite.redeem } it 'adds the user to the topic_users' do @@ -358,25 +357,14 @@ describe Invite do end context 'invited by another user to a different topic' do - let!(:another_invite) { another_topic.invite(coding_horror, 'jake@adventuretime.ooo') } let!(:user) { invite.redeem } - - let(:coding_horror) { User.find_by(username: "CodingHorror") } - let(:another_topic) { Fabricate(:topic, category_id: nil, archetype: "private_message", user: coding_horror) } + let(:another_tl2_user) { Fabricate(:user, trust_level: 2) } + let(:another_topic) { Fabricate(:topic, user: another_tl2_user) } 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) - duplicate_invite = Invite.find_by(id: another_invite.id) - expect(duplicate_invite).to be_nil - end - - context 'if they redeem the other invite afterwards' do - - it 'wont redeem a duplicate invite' do - expect(another_invite.redeem).to be_blank - end - end end end diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index 4ae69364d25..947a88bdbce 100644 --- a/spec/models/topic_spec.rb +++ b/spec/models/topic_spec.rb @@ -502,28 +502,33 @@ describe Topic do end - it "rate limits topic invitations" do - SiteSetting.stubs(:max_topic_invitations_per_day).returns(2) - RateLimiter.stubs(:disabled?).returns(false) - RateLimiter.clear_all! + context 'rate limits' do - start = Time.now.tomorrow.beginning_of_day - freeze_time(start) + it "rate limits topic invitations" do + SiteSetting.stubs(:max_topic_invitations_per_day).returns(2) + RateLimiter.stubs(:disabled?).returns(false) + RateLimiter.clear_all! - user = Fabricate(:user) - topic = Fabricate(:topic) + start = Time.now.tomorrow.beginning_of_day + freeze_time(start) - freeze_time(start + 10.minutes) - topic.invite(topic.user, user.username) + user = Fabricate(:user) + trust_level_2 = Fabricate(:user, trust_level: 2) + topic = Fabricate(:topic, user: trust_level_2) - freeze_time(start + 20.minutes) - topic.invite(topic.user, "walter@white.com") + freeze_time(start + 10.minutes) + topic.invite(topic.user, user.username) - freeze_time(start + 30.minutes) + freeze_time(start + 20.minutes) + topic.invite(topic.user, "walter@white.com") + + freeze_time(start + 30.minutes) + + expect { + topic.invite(topic.user, "user@example.com") + }.to raise_error(RateLimiter::LimitExceeded) + end - expect { - topic.invite(topic.user, "user@example.com") - }.to raise_error(RateLimiter::LimitExceeded) end context 'bumping topics' do