From 693580bcf06964bb80003aff98cd0d4af90463df Mon Sep 17 00:00:00 2001 From: dklawren Date: Wed, 26 Feb 2020 14:32:54 -0500 Subject: [PATCH] Bug 1612290 - Provide self-service UI for users to reactivate their account after being disabled due to bouncing --- Bugzilla/App.pm | 10 +- Bugzilla/App/BouncedEmails.pm | 63 ++++++++ Bugzilla/App/SES.pm | 43 +++++- Bugzilla/Constants.pm | 6 + Bugzilla/User.pm | 42 ++++- docker-compose.test.yml | 4 + docker-compose.yml | 2 + editusers.cgi | 1 + t/bmo/bounced-emails.t | 143 ++++++++++++++++++ .../account/email/bounced-emails.html.tmpl | 61 ++++++++ .../en/default/admin/users/userdata.html.tmpl | 10 ++ template/en/default/global/header.html.tmpl | 8 + template/en/default/global/messages.html.tmpl | 2 + 13 files changed, 383 insertions(+), 12 deletions(-) create mode 100644 Bugzilla/App/BouncedEmails.pm create mode 100644 t/bmo/bounced-emails.t create mode 100644 template/en/default/account/email/bounced-emails.html.tmpl diff --git a/Bugzilla/App.pm b/Bugzilla/App.pm index 5990a6854..5699f6203 100644 --- a/Bugzilla/App.pm +++ b/Bugzilla/App.pm @@ -21,11 +21,12 @@ use Bugzilla::Constants qw(bz_locations MAX_STS_AGE); use Bugzilla::Extension (); use Bugzilla::Install::Requirements (); use Bugzilla::Logging; +use Bugzilla::App::API; +use Bugzilla::App::BouncedEmails; use Bugzilla::App::CGI; +use Bugzilla::App::Main; use Bugzilla::App::OAuth2::Clients; use Bugzilla::App::SES; -use Bugzilla::App::Main; -use Bugzilla::App::API; use Bugzilla::App::Static; use Mojo::Loader qw( find_modules ); use Module::Runtime qw( require_module ); @@ -192,11 +193,12 @@ sub setup_routes { my ($self) = @_; my $r = $self->routes; + Bugzilla::App::API->setup_routes($r); + Bugzilla::App::BouncedEmails->setup_routes($r); Bugzilla::App::CGI->setup_routes($r); Bugzilla::App::Main->setup_routes($r); - Bugzilla::App::API->setup_routes($r); - Bugzilla::App::SES->setup_routes($r); Bugzilla::App::OAuth2::Clients->setup_routes($r); + Bugzilla::App::SES->setup_routes($r); $r->static_file('/__lbheartbeat__'); $r->static_file( diff --git a/Bugzilla/App/BouncedEmails.pm b/Bugzilla/App/BouncedEmails.pm new file mode 100644 index 000000000..7765b5b1a --- /dev/null +++ b/Bugzilla/App/BouncedEmails.pm @@ -0,0 +1,63 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. +# +# This Source Code Form is "Incompatible With Secondary Licenses", as +# defined by the Mozilla Public License, v. 2.0. + +package Bugzilla::App::BouncedEmails; + +use 5.10.1; +use Mojo::Base qw( Mojolicious::Controller ); + +use Bugzilla::Constants; +use Bugzilla::Error; +use Bugzilla::Token; + +sub setup_routes { + my ($class, $r) = @_; + $r->any('/bounced_emails/:userid')->to('BouncedEmails#view'); +} + +sub view { + my ($self) = @_; + my $user = $self->bugzilla->login(LOGIN_REQUIRED); + my $other_user = Bugzilla::User->check({id => $self->param('userid')}); + + unless ($user->in_group('editusers') + || $user->in_group('disableusers') + || $user->id == $other_user->id) + { + ThrowUserError('auth_failure', + {reason => "not_visible", action => "modify", object => "user"}); + } + + if ( $self->param('enable_email') + && $user->id == $other_user->id + && $other_user->email_disabled) + { + my $token = $self->param('token'); + check_token_data($token, 'bounced_emails'); + + $other_user->set_email_enabled(1); + $other_user->update(); + + return $self->redirect_to('/home'); + } + + my $token = issue_session_token('bounced_emails'); + $self->stash( + { + bounce_max => BOUNCE_COUNT_MAX, + user => $user, + other_user => $other_user, + token => $token + } + ); + return $self->render( + template => 'account/email/bounced-emails', + handler => 'bugzilla' + ); +} + +1; diff --git a/Bugzilla/App/SES.pm b/Bugzilla/App/SES.pm index 037d64534..ffebdbe9f 100644 --- a/Bugzilla/App/SES.pm +++ b/Bugzilla/App/SES.pm @@ -10,7 +10,7 @@ package Bugzilla::App::SES; use 5.10.1; use Mojo::Base qw( Mojolicious::Controller ); -use Bugzilla::Constants qw(ERROR_MODE_DIE); +use Bugzilla::Constants qw(BOUNCE_COUNT_MAX ERROR_MODE_DIE); use Bugzilla::Logging; use Bugzilla::Mailer qw(MessageToMTA); use Bugzilla::User (); @@ -182,7 +182,7 @@ sub _process_bounce { # disable each account that is bouncing foreach my $recipient (@{$notification->{bounce}->{bouncedRecipients}}) { my $address = $recipient->{emailAddress}; - my $reason = sprintf '(%s) %s', $recipient->{action} // 'error', + my $reason = sprintf '(%s) %s', $recipient->{action} // 'error', $recipient->{diagnosticCode} // 'unknown'; my $user = Bugzilla::User->new({name => $address, cache => 1}); @@ -199,16 +199,45 @@ sub _process_bounce { mta => $notification->{bounce}->{reportingMTA} // 'unknown', reason => $reason, }; - my $disable_text; + my $bounce_message; $template->process('admin/users/bounce-disabled.txt.tmpl', - $vars, \$disable_text) + $vars, \$bounce_message) || die $template->error(); - $user->set_disabledtext($disable_text); + # Increment bounce count for user + my $bounce_count = $user->bounce_count + 1; + + # If user has not had a bounce in less than 30 days, set the bounce count to 1 instead + my $dbh = Bugzilla->dbh; + my ($has_recent_bounce) = $dbh->selectrow_array( + "SELECT 1 FROM audit_log WHERE object_id = ? AND class = 'Bugzilla::User' AND field = 'bounce_message' AND (" + . $dbh->sql_date_math('at_time', '+', 30, 'DAY') + . ") > NOW()", + undef, $user->id + ); + $bounce_count = 1 if !$has_recent_bounce; + $user->set_disable_mail(1); + $user->set_bounce_count($bounce_count); + + # if we hit the max amount, go ahead and disabled the account + # and an admin will need to reactivate the account. + if ($bounce_count == BOUNCE_COUNT_MAX) { + $user->set_disabledtext($bounce_message); + } + $user->update(); + + # Do this outside of Object.pm as we do not want to + # store the messages anywhere else. + $dbh->do( + "INSERT INTO audit_log (user_id, class, object_id, field, added, at_time) + VALUES (?, 'Bugzilla::User', ?, 'bounce_message', ?, LOCALTIMESTAMP(0))", + undef, $user->id, $user->id, $bounce_message + ); + Bugzilla->audit( - "bounce for <$address> disabled userid-" . $user->id . ": $reason"); + "bounce for <$address> disabled email for userid-" . $user->id . ": $reason"); } } @@ -224,7 +253,7 @@ sub _process_complaint { state $check = compile(Self, ComplaintNotification); my ($self, $notification) = $check->(@_); my $template = Bugzilla->template_inner(); - my $json = JSON::MaybeXS->new(pretty => 1, utf8 => 1, canonical => 1,); + my $json = JSON::MaybeXS->new(pretty => 1, utf8 => 1, canonical => 1,); foreach my $recipient (@{$notification->{complaint}->{complainedRecipients}}) { my $reason = $notification->{complaint}->{complaintFeedbackType} // 'unknown'; diff --git a/Bugzilla/Constants.pm b/Bugzilla/Constants.pm index 53223421d..6b1f0e58a 100644 --- a/Bugzilla/Constants.pm +++ b/Bugzilla/Constants.pm @@ -203,6 +203,8 @@ use Memoize; EMAIL_LIMIT_EXCEPTION JOB_QUEUE_VIEW_MAX_JOBS + + BOUNCE_COUNT_MAX ); @Bugzilla::Constants::EXPORT_OK = qw(contenttypes); @@ -666,6 +668,10 @@ use constant EMAIL_LIMIT_EXCEPTION => "email_limit_exceeded\n"; # (view_job_queue.cgi). use constant JOB_QUEUE_VIEW_MAX_JOBS => 2500; +# Maximum number of times an email can bounce for an account +# before the account is completely disabled. +use constant BOUNCE_COUNT_MAX => 5; + sub bz_locations { # Force memoize() to re-compute data per project, to avoid diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm index aa0ad0901..911c39164 100644 --- a/Bugzilla/User.pm +++ b/Bugzilla/User.pm @@ -85,7 +85,8 @@ sub DB_COLUMNS { 'profiles.password_change_reason', 'profiles.mfa', 'profiles.mfa_required_date', - 'profiles.nickname' + 'profiles.nickname', + 'profiles.bounce_count' ), ; } @@ -106,6 +107,7 @@ use constant VALIDATORS => { password_change_required => \&Bugzilla::Object::check_boolean, password_change_reason => \&_check_password_change_reason, mfa => \&_check_mfa, + bounce_count => \&_check_numeric, }; sub UPDATE_COLUMNS { @@ -122,6 +124,7 @@ sub UPDATE_COLUMNS { mfa mfa_required_date nickname + bounce_count ); push(@cols, 'cryptpassword') if exists $self->{cryptpassword}; return @cols; @@ -473,6 +476,16 @@ sub _check_mfa { return ''; } +sub _check_numeric { + my ($self, $value) = (@_); + if ($value !~ /^[0-9]+$/) { + ThrowCodeError('param_must_be_numeric', + {param => $value, function => 'Bugzilla::User::_check_numeric'}); + return "must be a numeric value"; + } + return $value; +} + ################################################################################ # Mutators ################################################################################ @@ -2753,6 +2766,33 @@ sub account_ip_login_failures { return $self->{account_ip_login_failures}; } +################# +# Email Bounces # +################# + +sub bounce_count { $_[0]->{bounce_count}; } + +sub bounce_messages { + my ($self) = @_; + my $dbh = Bugzilla->dbh; + my $bounce_count = $self->bounce_count; + return $self->{bounce_messages} ||= $dbh->selectall_arrayref( + "SELECT " . $dbh->sql_date_format('at_time', '%Y-%m-%d %H:%i') . " + AS bounce_when, added AS bounce_message FROM audit_log + WHERE object_id = ? AND class = 'Bugzilla::User' AND field = 'bounce_message' + ORDER BY at_time DESC LIMIT $bounce_count", + {Slice => {}}, + $self->id + ); +} + +sub set_bounce_count { + my ($self, $count) = @_; + $self->set('bounce_count', $count); + $self->{bounce_count} = $count; +} + + ############### # Subroutines # ############### diff --git a/docker-compose.test.yml b/docker-compose.test.yml index 4bae22506..1fb065589 100644 --- a/docker-compose.test.yml +++ b/docker-compose.test.yml @@ -19,6 +19,8 @@ services: - BMO_db_user=bugs - BMO_memcached_namespace=bugzilla - BMO_memcached_servers=memcached:11211 + - BMO_ses_username=ses@mozilla.bugs + - BMO_ses_password=password123456789! - BMO_urlbase=AUTOMATIC - BUGZILLA_ALLOW_INSECURE_HTTP=1 - BZ_ANSWERS_FILE=/app/conf/checksetup_answers.txt @@ -42,6 +44,8 @@ services: bmo.db: image: mozillabteam/bmo-mysql:5.7 + tmpfs: + - /tmp logging: driver: "none" environment: diff --git a/docker-compose.yml b/docker-compose.yml index 36ab67d02..2bfedb895 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -24,6 +24,8 @@ services: - BMO_db_user=bugs - BMO_memcached_namespace=bugzilla - BMO_memcached_servers=memcached:11211 + - BMO_ses_username=ses@mozilla.bugs + - BMO_ses_password=password123456789! - BMO_urlbase=http://bmo.test/ - BUGZILLA_ALLOW_INSECURE_HTTP=1 - BZ_ANSWERS_FILE=/app/conf/checksetup_answers.txt diff --git a/editusers.cgi b/editusers.cgi index 278393c6f..00cfb4bb4 100755 --- a/editusers.cgi +++ b/editusers.cgi @@ -285,6 +285,7 @@ elsif ($action eq 'update') { $otherUser->set_name($cgi->param('name')); $otherUser->set_disabledtext($cgi->param('disabledtext')); $otherUser->set_disable_mail($cgi->param('disable_mail')); + $otherUser->set_bounce_count(0) if $cgi->param('reset_bounce'); } $changes = $otherUser->update(); diff --git a/t/bmo/bounced-emails.t b/t/bmo/bounced-emails.t new file mode 100644 index 000000000..b61e066a4 --- /dev/null +++ b/t/bmo/bounced-emails.t @@ -0,0 +1,143 @@ +#!/usr/bin/env perl +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. +# +# This Source Code Form is "Incompatible With Secondary Licenses", as +# defined by the Mozilla Public License, v. 2.0. +use strict; +use warnings; +use 5.10.1; +use lib qw( . lib local/lib/perl5 ); + +BEGIN { + $ENV{LOG4PERL_CONFIG_FILE} = 'log4perl-t.conf'; + $ENV{BUGZILLA_DISABLE_HOSTAGE} = 1; +} + +use Mojo::URL; +use Mojo::UserAgent; +use Test2::V0; +use Test::Selenium::Remote::Driver; + +my $ADMIN_LOGIN = $ENV{BZ_TEST_ADMIN} // 'admin@mozilla.bugs'; +my $ADMIN_PW_OLD = $ENV{BZ_TEST_ADMIN_PASS} // 'Te6Oovohch'; +my $SES_USERNAME = $ENV{BMO_ses_username} // 'ses@mozilla.bugs'; +my $SES_PASSWORD = $ENV{BMO_ses_password} // 'password123456789!'; + +my @require_env = qw( + BZ_BASE_URL + BZ_TEST_NEWBIE + BZ_TEST_NEWBIE_PASS + TWD_HOST + TWD_PORT +); + +my @missing_env = grep { !exists $ENV{$_} } @require_env; +BAIL_OUT("Missing env: @missing_env") if @missing_env; + +my $sel = Test::Selenium::Remote::Driver->new( + base_url => $ENV{BZ_BASE_URL}, + browser => 'firefox', + version => '', + javascript => 1 +); + +my $ua = Mojo::UserAgent->new; +$ua->on( + start => sub { + my ($ua, $tx) = @_; + $tx->req->headers->header('X-Amz-SNS-Message-Type' => 'Notification'); + } +); + +my $ses_data = ; +my $ses_url = Mojo::URL->new($ENV{BZ_BASE_URL} . 'ses/index.cgi') + ->userinfo("$SES_USERNAME:$SES_PASSWORD"); + +# First bounce +my $result = $ua->post($ses_url => $ses_data)->result; +ok($result->is_success, 'Posting first bounce was successful'); + +# Allow user to reset their email +$sel->set_implicit_wait_timeout(600); +login_ok($sel, $ENV{BZ_TEST_NEWBIE}, $ENV{BZ_TEST_NEWBIE_PASS}); +$sel->body_text_contains('Change notification emails have been disabled', + 'Email disabled warning is displayed'); +$sel->click_element_ok('//a[@id="bounced_emails_link"]'); +sleep(2); +$sel->title_is('Bounced Emails'); +$sel->click_element_ok('//input[@id="enable_email"]'); +submit($sel, '//input[@value="Submit"]'); +sleep(2); +$sel->title_is('Bugzilla Main Page'); +$sel->body_text_lacks( + 'Change notification emails have been disabled', + 'Email disabled warning is no longer displayed' +); +logout_ok($sel); + +# Bounce 4 more times causing account to be locked +$result = $ua->post($ses_url => $ses_data)->result; +ok($result->is_success, 'Posting third bounce was successful'); +$result = $ua->post($ses_url => $ses_data)->result; +ok($result->is_success, 'Posting fourth bounce was successful'); +$result = $ua->post($ses_url => $ses_data)->result; +ok($result->is_success, 'Posting fifth bounce was successful'); +$result = $ua->post($ses_url => $ses_data)->result; +ok($result->is_success, 'Posting fifth bounce was successful'); + +# User should not be able to login again +login($sel, $ENV{BZ_TEST_NEWBIE}, $ENV{BZ_TEST_NEWBIE_PASS}); +$sel->title_is('Account Disabled'); +$sel->body_text_contains( + 'Your Bugzilla account has been disabled due to issues delivering emails to your address.', + 'Account disabled message is displayed' +); + +done_testing; + +sub submit { + my ($sel, $xpath) = @_; + $sel->find_element($xpath, 'xpath')->click_ok('Submit OK'); +} + +sub click_and_type { + my ($sel, $name, $text) = @_; + + eval { + my $el + = $sel->find_element(qq{//*[\@id="bugzilla-body"]//input[\@name="$name"]}, + 'xpath'); + $el->click(); + $sel->send_keys_to_active_element($text); + pass("found $name and typed $text"); + }; + if ($@) { + fail("failed to find $name"); + } +} + +sub login { + my ($sel, $login, $password) = @_; + $sel->get_ok("/login"); + $sel->title_is("Log in to Bugzilla"); + click_and_type($sel, 'Bugzilla_login', $login); + click_and_type($sel, 'Bugzilla_password', $password); + submit($sel, '//input[@id="log_in"]'); +} + +sub login_ok { + my ($sel) = @_; + login(@_); + $sel->title_is('Bugzilla Main Page'); +} + +sub logout_ok { + my ($sel) = @_; + $sel->get_ok('/index.cgi?logout=1'); + $sel->title_is("Logged Out"); +} + +__DATA__ +{"Type":"Notification","Message":"{\"eventType\":\"Bounce\",\"bounce\":{\"bounceType\":\"Permanent\",\"bounceSubType\":\"General\",\"bouncedRecipients\":[{\"emailAddress\":\"newbie@mozilla.example\",\"action\":\"failed\",\"status\":\"5.1.1\",\"diagnosticCode\":\"smtp;5505.1.1userunknown\"}],\"timestamp\":\"2017-08-05T00:41:02.669Z\",\"feedbackId\":\"01000157c44f053b-61b59c11-9236-11e6-8f96-7be8aexample-000000\",\"reportingMTA\":\"dsn;mta.example.com\"},\"mail\":{\"timestamp\":\"2017-08-05T00:40:02.012Z\",\"source\":\"BugzillaDaemon\",\"sourceArn\":\"arn:aws:ses:us-east-1:123456789012:identity/bugzilla@mozilla.bugs\",\"sendingAccountId\":\"123456789012\",\"messageId\":\"EXAMPLE7c191be45-e9aedb9a-02f9-4d12-a87d-dd0099a07f8a-000000\",\"destination\":[\"newbie@mozilla.example\"],\"headersTruncated\":false,\"headers\":[{\"name\":\"From\",\"value\":\"BugzillaDaemon\"},{\"name\":\"To\",\"value\":\"newbie@mozilla.example\"},{\"name\":\"Subject\",\"value\":\"MessagesentfromAmazonSES\"},{\"name\":\"MIME-Version\",\"value\":\"1.0\"},{\"name\":\"Content-Type\",\"value\":\"multipart/alternative;boundary=\"}],\"commonHeaders\":{\"from\":[\"BugzillaDaemon\"],\"to\":[\"newbie@mozilla.example\"],\"messageId\":\"EXAMPLE7c191be45-e9aedb9a-02f9-4d12-a87d-dd0099a07f8a-000000\",\"subject\":\"MessagesentfromAmazonSES\"},\"tags\":{\"ses:configuration-set\":[\"ConfigSet\"],\"ses:source-ip\":[\"192.0.2.0\"],\"ses:from-domain\":[\"example.com\"],\"ses:caller-identity\":[\"ses_user\"]}}}"} diff --git a/template/en/default/account/email/bounced-emails.html.tmpl b/template/en/default/account/email/bounced-emails.html.tmpl new file mode 100644 index 000000000..ba0b74928 --- /dev/null +++ b/template/en/default/account/email/bounced-emails.html.tmpl @@ -0,0 +1,61 @@ +[%# This Source Code Form is subject to the terms of the Mozilla Public + # License, v. 2.0. If a copy of the MPL was not distributed with this + # file, You can obtain one at http://mozilla.org/MPL/2.0/. + # + # This Source Code Form is "Incompatible With Secondary Licenses", as + # defined by the Mozilla Public License, v. 2.0. + #%] + +[% PROCESS global/header.html.tmpl + title = "Bounced Emails" + style_urls = [ "skins/standard/describecomponents.css" ] +%] + +

[% title FILTER html %]

+ +[% IF user.id == other_user.id AND other_user.bounce_count AND other_user.email_disabled %] +

+ Due to issues delivering email to your account, we have temporarily disabled email notifications + being sent to it. If you feel like the issue has been resolved, you may reactivate email delivery + below. After a maximum of [% bounce_max FILTER html %] bounces, we will disable logging in to your + account and you will need to contact an administrator to reactivate it.

+ +
+ + + + + + +
I have resolved the issue and would like email delivery for my account to be reactivated.
+
+ +
+[% END %] + +[% IF (user.id == other_user.id OR user.in_group('editusers') OR user.in_group('disableusers')) + AND other_user.bounce_count %] +

History

+ +
+ [% FOREACH bounce = other_user.bounce_messages %] + [% IF loop.first %] + Current + [% ELSIF loop.count == 2 %] + Older + [% END %] +
+
+

[% bounce.bounce_when FILTER time FILTER html %]

+
+
+

[% bounce.bounce_message FILTER html %]

+
+
+ [% END %] +
+[% ELSE %] +No bounced email messages have been recorded. +[% END %] + +[% PROCESS global/footer.html.tmpl %] diff --git a/template/en/default/admin/users/userdata.html.tmpl b/template/en/default/admin/users/userdata.html.tmpl index 30396f22d..0abca2baf 100644 --- a/template/en/default/admin/users/userdata.html.tmpl +++ b/template/en/default/admin/users/userdata.html.tmpl @@ -108,6 +108,16 @@ + [% IF otheruser.bounce_count %] + + + + + ( + [% otheruser.bounce_count FILTER html %]) + + + [% END %] diff --git a/template/en/default/global/header.html.tmpl b/template/en/default/global/header.html.tmpl index 9d60d34a7..fa324d426 100644 --- a/template/en/default/global/header.html.tmpl +++ b/template/en/default/global/header.html.tmpl @@ -438,6 +438,14 @@

[% header FILTER none %]

[% END %] +[%# Show banner for users who have email disabled due to bounces %] +[% IF user.bounce_count AND user.email_disabled %] +
+ Change notification emails have been disabled for your account due to issues delivering to your address. + View recent errors and reactivate email. +
+[% END %] + [% IF message %]
[% message %]
[% END %] diff --git a/template/en/default/global/messages.html.tmpl b/template/en/default/global/messages.html.tmpl index 6c94548ae..ce0b31cac 100644 --- a/template/en/default/global/messages.html.tmpl +++ b/template/en/default/global/messages.html.tmpl @@ -74,6 +74,8 @@ [% ELSE %] [% terms.Bug %]mail has been enabled. [% END %] + [% ELSIF field == 'bounce_count' %] + Bounced email count has been reset. [% ELSIF field == 'password_change_required' %] The user [% otheruser.password_change_required ? "must" : "no longer needs to" %] update their password. [% ELSIF field == 'password_change_reason' %] -- 2.47.3