diff --git a/app/assets/javascripts/admin/models/admin-user.js.es6 b/app/assets/javascripts/admin/models/admin-user.js.es6 index 5c0660fedd9..b5767f781a5 100644 --- a/app/assets/javascripts/admin/models/admin-user.js.es6 +++ b/app/assets/javascripts/admin/models/admin-user.js.es6 @@ -1,3 +1,4 @@ +import computed from 'ember-addons/ember-computed-decorators'; import { propertyNotEqual } from 'discourse/lib/computed'; import { popupAjaxError } from 'discourse/lib/ajax-error'; import ApiKey from 'admin/models/api-key'; @@ -6,11 +7,42 @@ import TL3Requirements from 'admin/models/tl3-requirements'; const AdminUser = Discourse.User.extend({ - customGroups: Em.computed.filter("groups", (g) => !g.automatic && Group.create(g)), - automaticGroups: Em.computed.filter("groups", (g) => g.automatic && Group.create(g)), + customGroups: Ember.computed.filter("groups", g => !g.automatic && Group.create(g)), + automaticGroups: Ember.computed.filter("groups", g => g.automatic && Group.create(g)), canViewProfile: Ember.computed.or("active", "staged"), + @computed("bounce_score", "reset_bounce_score_after") + bounceScore(bounce_score, reset_bounce_score_after) { + if (bounce_score > 0) { + return `${bounce_score} - ${moment(reset_bounce_score_after).format('LL')}`; + } else { + return bounce_score; + } + }, + + @computed("bounce_score") + bounceScoreExplanation(bounce_score) { + if (bounce_score === 0) { + return I18n.t("admin.user.bounce_score_explanation.none"); + } else if (bounce_score < Discourse.SiteSettings.bounce_score_threshold) { + return I18n.t("admin.user.bounce_score_explanation.some"); + } else { + return I18n.t("admin.user.bounce_score_explanation.threshold_reached"); + } + }, + + canResetBounceScore: Ember.computed.gt("bounce_score", 0), + + resetBounceScore() { + return Discourse.ajax(`/admin/users/${this.get("id")}/reset_bounce_score`, { + type: 'POST' + }).then(() => this.setProperties({ + "bounce_score": 0, + "reset_bounce_score_after": null + })); + }, + generateApiKey() { const self = this; return Discourse.ajax("/admin/users/" + this.get('id') + "/generate_api_key", { diff --git a/app/assets/javascripts/admin/templates/email-bounced.hbs b/app/assets/javascripts/admin/templates/email-bounced.hbs index b03e6179bec..e2df9e04bf4 100644 --- a/app/assets/javascripts/admin/templates/email-bounced.hbs +++ b/app/assets/javascripts/admin/templates/email-bounced.hbs @@ -6,7 +6,6 @@ {{i18n 'admin.email.user'}} {{i18n 'admin.email.to_address'}} {{i18n 'admin.email.email_type'}} - {{i18n 'admin.email.skipped_reason'}} @@ -15,7 +14,6 @@ {{text-field value=filter.user placeholderKey="admin.email.logs.filters.user_placeholder"}} {{text-field value=filter.address placeholderKey="admin.email.logs.filters.address_placeholder"}} {{text-field value=filter.type placeholderKey="admin.email.logs.filters.type_placeholder"}} - {{text-field value=filter.skipped_reason placeholderKey="admin.email.logs.filters.skipped_reason_placeholder"}} {{#each model as |l|}} @@ -31,16 +29,9 @@ {{l.to_address}} {{l.email_type}} - - {{#if l.post_url}} - {{l.skipped_reason}} - {{else}} - {{l.skipped_reason}} - {{/if}} - {{else}} - {{i18n 'admin.email.logs.none'}} + {{i18n 'admin.email.logs.none'}} {{/each}} diff --git a/app/assets/javascripts/admin/templates/user-index.hbs b/app/assets/javascripts/admin/templates/user-index.hbs index 663d5062f7f..6499840ac28 100644 --- a/app/assets/javascripts/admin/templates/user-index.hbs +++ b/app/assets/javascripts/admin/templates/user-index.hbs @@ -350,7 +350,20 @@
{{i18n 'admin.user.staged'}}
{{model.staged}}
-
{{i18n 'admin.user.stage_explanation'}}
+
{{i18n 'admin.user.staged_explanation'}}
+
+ +
+
{{i18n 'admin.user.bounce_score'}}
+
{{model.bounceScore}}
+
+ {{#if model.canResetBounceScore}} + + {{/if}} + {{model.bounceScoreExplanation}} +
diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index 8b10de76627..4f02e19ab27 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -23,7 +23,8 @@ class Admin::UsersController < Admin::AdminController :primary_group, :generate_api_key, :revoke_api_key, - :anonymize] + :anonymize, + :reset_bounce_score] def index users = ::AdminUserIndexQuery.new(params).find_users @@ -355,6 +356,12 @@ class Admin::UsersController < Admin::AdminController end end + def reset_bounce_score + guardian.ensure_can_reset_bounce_score!(@user) + @user.user_stat.update_columns(bounce_score: 0, reset_bounce_score_after: nil) + render json: success_json + end + private def fetch_user diff --git a/app/jobs/regular/user_email.rb b/app/jobs/regular/user_email.rb index 4a7a87d96db..8a109d233de 100644 --- a/app/jobs/regular/user_email.rb +++ b/app/jobs/regular/user_email.rb @@ -48,7 +48,24 @@ module Jobs @skip_context = { type: type, user_id: user_id, to_address: to_address, post_id: post_id } end - NOTIFICATIONS_SENT_BY_MAILING_LIST ||= Set.new %w{posted replied mentioned group_mentioned quoted} + NOTIFICATIONS_SENT_BY_MAILING_LIST ||= Set.new %w{ + posted + replied + mentioned + group_mentioned + quoted + } + + CRITICAL_EMAIL_TYPES = Set.new %i{ + account_created + admin_login + confirm_new_email + confirm_old_email + forgot_password + notify_old_email + signup + signup_after_approval + } def message_for_email(user, post, type, notification, notification_type=nil, notification_data_hash=nil, @@ -109,7 +126,7 @@ module Jobs email_args[:email_token] = email_token end - if type == 'notify_old_email' + if type == :notify_old_email email_args[:new_email] = user.email end @@ -117,7 +134,7 @@ module Jobs return skip_message(I18n.t('email_log.exceeded_emails_limit')) end - if (user.user_stat.try(:bounce_score) || 0) >= SiteSetting.bounce_score_threshold + if !CRITICAL_EMAIL_TYPES.include?(type) && user.user_stat.bounce_score >= SiteSetting.bounce_score_threshold return skip_message(I18n.t('email_log.exceeded_bounces_limit')) end diff --git a/app/jobs/scheduled/poll_mailbox.rb b/app/jobs/scheduled/poll_mailbox.rb index cd6b3226f8a..583f8e76302 100644 --- a/app/jobs/scheduled/poll_mailbox.rb +++ b/app/jobs/scheduled/poll_mailbox.rb @@ -30,7 +30,7 @@ module Jobs set_incoming_email_rejection_message( receiver.incoming_email, - I18n.t("email.incoming.errors.bounced_email_report") + I18n.t("email.incoming.errors.bounced_email_error") ) rescue Email::Receiver::AutoGeneratedEmailReplyError => e log_email_process_failure(mail_string, e) diff --git a/app/serializers/admin_detailed_user_serializer.rb b/app/serializers/admin_detailed_user_serializer.rb index ea7bfbabba9..b6a2072f97c 100644 --- a/app/serializers/admin_detailed_user_serializer.rb +++ b/app/serializers/admin_detailed_user_serializer.rb @@ -20,7 +20,9 @@ class AdminDetailedUserSerializer < AdminUserSerializer :primary_group_id, :badge_count, :warnings_received_count, - :user_fields + :user_fields, + :bounce_score, + :reset_bounce_score_after has_one :approved_by, serializer: BasicUserSerializer, embed: :objects has_one :api_key, serializer: ApiKeySerializer, embed: :objects @@ -76,4 +78,12 @@ class AdminDetailedUserSerializer < AdminUserSerializer object.user_fields.present? end + def bounce_score + object.user_stat.bounce_score + end + + def reset_bounce_score_after + object.user_stat.reset_bounce_score_after + end + end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 59b4791ade3..95200674c17 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -2593,10 +2593,18 @@ en: block_failed: 'There was a problem blocking the user.' block_confirm: 'Are you sure you want to block this user? They will not be able to create any new topics or posts.' block_accept: 'Yes, block this user' + bounce_score: "Bounce Score" + reset_bounce_score: + label: "Reset" + title: "Reset bounce score back to 0" deactivate_explanation: "A deactivated user must re-validate their email." suspended_explanation: "A suspended user can't log in." block_explanation: "A blocked user can't post or start topics." - stage_explanation: "A staged user can only post via email in specific topics." + staged_explanation: "A staged user can only post via email in specific topics." + bounce_score_explanation: + none: "No bounces were received recently from that email." + some: "Some bounces were received recently from that email." + threshold_reached: "Received too many bounces from that email." trust_level_change_failed: "There was a problem changing the user's trust level." suspend_modal_title: "Suspend User" trust_level_2_users: "Trust Level 2 Users" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 01345d0094c..e67d48fff2d 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -64,7 +64,7 @@ en: reply_user_not_matching_error: "Happens when a reply came in from a different email address the notification was sent to." topic_not_found_error: "Happens when a reply came in but the related topic has been deleted." topic_closed_error: "Happens when a reply came in but the related topic has been closed." - bounced_email_report: "Email is a bounced email report." + bounced_email_error: "Email is a bounced email report." auto_generated_email_reply: "Email contains a reply to an auto generated email." screened_email_error: "Happens when the sender's email address was already screened." diff --git a/config/routes.rb b/config/routes.rb index 31682f385ec..21459fc9c3f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -108,6 +108,7 @@ Discourse::Application.routes.draw do get "leader_requirements" => "users#tl3_requirements" get "tl3_requirements" put "anonymize" + post "reset_bounce_score" end get "users/:id.json" => 'users#show', defaults: {format: 'json'} get 'users/:id/:username' => 'users#show' diff --git a/config/site_settings.yml b/config/site_settings.yml index a8fac94afbd..937752b5f33 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -572,6 +572,7 @@ email: type: list block_auto_generated_emails: true bounce_score_threshold: + client: true default: 4 min: 1 diff --git a/lib/email/receiver.rb b/lib/email/receiver.rb index 83543fe36ae..e766554a3ce 100644 --- a/lib/email/receiver.rb +++ b/lib/email/receiver.rb @@ -156,7 +156,7 @@ module Email end def verp - @verp ||= @mail.destinations.select { |to| to[/\+verp-\h{32}@/] }.first + @verp ||= all_destinations.select { |to| to[/\+verp-\h{32}@/] }.first end def update_bounce_score(email, score) @@ -171,10 +171,8 @@ module Email user.user_stat.reset_bounce_score_after = 30.days.from_now user.user_stat.save - if user.active && user.user_stat.bounce_score >= SiteSetting.bounce_score_threshold - user.deactivate + if user.user_stat.bounce_score >= SiteSetting.bounce_score_threshold StaffActionLogger.new(Discourse.system_user).log_revoke_email(user) - EmailToken.where(email: user.email, confirmed: true).update_all(confirmed: false) end end @@ -292,16 +290,18 @@ module Email user end - def destinations - [ @mail.destinations, + def all_destinations + @all_destinations ||= [ + @mail.destinations, [@mail[:x_forwarded_to]].flatten.compact.map(&:decoded), [@mail[:delivered_to]].flatten.compact.map(&:decoded), - ].flatten - .select(&:present?) - .uniq - .lazy - .map { |d| check_address(d) } - .drop_while(&:blank?) + ].flatten.select(&:present?).uniq.lazy + end + + def destinations + all_destinations + .map { |d| check_address(d) } + .drop_while(&:blank?) end def check_address(address) diff --git a/lib/guardian/user_guardian.rb b/lib/guardian/user_guardian.rb index f67f494108e..3eca04f589b 100644 --- a/lib/guardian/user_guardian.rb +++ b/lib/guardian/user_guardian.rb @@ -51,6 +51,10 @@ module UserGuardian is_staff? && !user.nil? && !user.staff? end + def can_reset_bounce_score?(user) + user && is_staff? + end + def can_check_emails?(user) is_admin? || (is_staff? && SiteSetting.show_email_on_profile) end diff --git a/spec/components/email/receiver_spec.rb b/spec/components/email/receiver_spec.rb index b6b33fefd43..f8497dc28bd 100644 --- a/spec/components/email/receiver_spec.rb +++ b/spec/components/email/receiver_spec.rb @@ -68,7 +68,7 @@ describe Email::Receiver do let(:bounce_key) { "14b08c855160d67f2e0c2f8ef36e251e" } let(:bounce_key_2) { "b542fb5a9bacda6d28cc061d18e4eb83" } - let!(:user) { Fabricate(:user, email: "foo@bar.com", active: true) } + let!(:user) { Fabricate(:user, email: "foo@bar.com") } let!(:email_log) { Fabricate(:email_log, user: user, bounce_key: bounce_key) } let!(:email_log_2) { Fabricate(:email_log, user: user, bounce_key: bounce_key_2) } @@ -82,7 +82,6 @@ describe Email::Receiver do email_log.reload expect(email_log.bounced).to eq(true) - expect(email_log.user.active).to eq(true) expect(email_log.user.user_stat.bounce_score).to eq(1) end @@ -91,7 +90,6 @@ describe Email::Receiver do email_log.reload expect(email_log.bounced).to eq(true) - expect(email_log.user.active).to eq(true) expect(email_log.user.user_stat.bounce_score).to eq(2) Timecop.freeze(2.days.from_now) do @@ -99,7 +97,6 @@ describe Email::Receiver do email_log_2.reload expect(email_log_2.bounced).to eq(true) - expect(email_log_2.user.active).to eq(false) expect(email_log_2.user.user_stat.bounce_score).to eq(4) end end