From 6fb33c210e6917f32056dcffd00e55d0d97302bc Mon Sep 17 00:00:00 2001 From: Dylan William Hardison Date: Fri, 30 Mar 2018 16:20:49 -0400 Subject: Bug 1450343 - Make the SES handler use Bugzilla::Logging and log more details --- Bugzilla/ModPerl/BasicAuth.pm | 13 ++-- ses/index.cgi | 141 +++++++++++++++++++++++------------------- 2 files changed, 85 insertions(+), 69 deletions(-) diff --git a/Bugzilla/ModPerl/BasicAuth.pm b/Bugzilla/ModPerl/BasicAuth.pm index e93680e9d..7248a19f3 100644 --- a/Bugzilla/ModPerl/BasicAuth.pm +++ b/Bugzilla/ModPerl/BasicAuth.pm @@ -25,18 +25,22 @@ use warnings; # AUTH_VAR_NAME and AUTH_VAR_PASS are the names of variables defined in # `localconfig` which hold the authentication credentials. -use Apache2::Const -compile => qw(OK HTTP_UNAUTHORIZED); +use Apache2::Const -compile => qw(OK HTTP_UNAUTHORIZED); ## no critic (Freenode::ModPerl) +use Bugzilla::Logging; use Bugzilla (); sub handler { my $r = shift; my ($status, $password) = $r->get_basic_auth_pw; - return $status if $status != Apache2::Const::OK; + if ($status != Apache2::Const::OK) { + WARN("Got non-OK status: $status when trying to get password"); + return $status + } my $auth_var_name = $ENV{AUTH_VAR_NAME}; my $auth_var_pass = $ENV{AUTH_VAR_PASS}; unless ($auth_var_name && $auth_var_pass) { - warn "AUTH_VAR_NAME and AUTH_VAR_PASS environmental vars not set\n"; + ERROR('AUTH_VAR_NAME and AUTH_VAR_PASS environmental vars not set'); $r->note_basic_auth_failure; return Apache2::Const::HTTP_UNAUTHORIZED; } @@ -44,13 +48,14 @@ sub handler { my $auth_user = Bugzilla->localconfig->{$auth_var_name}; my $auth_pass = Bugzilla->localconfig->{$auth_var_pass}; unless ($auth_user && $auth_pass) { - warn "$auth_var_name and $auth_var_pass not configured\n"; + ERROR("$auth_var_name and $auth_var_pass not configured"); $r->note_basic_auth_failure; return Apache2::Const::HTTP_UNAUTHORIZED; } unless ($r->user eq $auth_user && $password eq $auth_pass) { $r->note_basic_auth_failure; + WARN('username and password do not match'); return Apache2::Const::HTTP_UNAUTHORIZED; } diff --git a/ses/index.cgi b/ses/index.cgi index aa5b34704..9e1632586 100755 --- a/ses/index.cgi +++ b/ses/index.cgi @@ -13,49 +13,55 @@ use warnings; use lib qw(.. ../lib ../local/lib/perl5); use Bugzilla (); +use Bugzilla::Logging; use Bugzilla::Constants qw( ERROR_MODE_DIE ); use Bugzilla::Mailer qw( MessageToMTA ); use Bugzilla::User (); use Bugzilla::Util qw( html_quote remote_ip ); -use JSON::XS qw( decode_json encode_json ); +use JSON::MaybeXS qw( decode_json encode_json ); use LWP::UserAgent (); use Try::Tiny qw( try catch ); Bugzilla->error_mode(ERROR_MODE_DIE); try { main(); -} catch { - warn "SES: Fatal error: $_\n"; - respond(500 => 'Internal Server Error'); +} +catch { + FATAL("Fatal error: $_"); + respond( 500 => 'Internal Server Error' ); }; sub main { - my $message = decode_json_wrapper(Bugzilla->cgi->param('POSTDATA')) // return; + my $message = decode_json_wrapper( Bugzilla->cgi->param('POSTDATA') ) // return; my $message_type = $ENV{HTTP_X_AMZ_SNS_MESSAGE_TYPE} // '(missing)'; - if ($message_type eq 'SubscriptionConfirmation') { + if ( $message_type eq 'SubscriptionConfirmation' ) { confirm_subscription($message); } - elsif ($message_type eq 'Notification') { - my $notification = decode_json_wrapper($message->{Message}) // return; + elsif ( $message_type eq 'Notification' ) { + my $notification = decode_json_wrapper( $message->{Message} ) // return; my $notification_type = $notification->{notificationType} // ''; - if ($notification_type eq 'Bounce') { + if ( $notification_type eq '' ) { + my $keys = join ', ', keys %$notification; + WARN("No notificationType in notification (keys: $keys)"); + } + if ( $notification_type eq 'Bounce' ) { process_bounce($notification); } - elsif ($notification_type eq 'Complaint') { + elsif ( $notification_type eq 'Complaint' ) { process_complaint($notification); } else { - warn "SES: Unsupported notification-type: $notification_type\n"; - respond(200 => 'OK'); + WARN("Unsupported notification-type: $notification_type"); + respond( 200 => 'OK' ); } } else { - warn "SES: Unsupported message-type: $message_type\n"; - respond(200 => 'OK'); + WARN("Unsupported message-type: $message_type"); + respond( 200 => 'OK' ); } } @@ -63,112 +69,117 @@ sub confirm_subscription { my ($message) = @_; my $subscribe_url = $message->{SubscribeURL}; - if (!$subscribe_url) { - warn "SES: Bad SubscriptionConfirmation request: missing SubscribeURL\n"; - respond(400 => 'Bad Request'); + if ( !$subscribe_url ) { + WARN('Bad SubscriptionConfirmation request: missing SubscribeURL'); + respond( 400 => 'Bad Request' ); return; } - my $ua = ua(); - my $res = $ua->get($message->{SubscribeURL}); - if (!$res->is_success) { - warn "SES: Bad response from SubscribeURL: " . $res->status_line . "\n"; - respond(400 => 'Bad Request'); + my $ua = ua(); + my $res = $ua->get( $message->{SubscribeURL} ); + if ( !$res->is_success ) { + WARN( 'Bad response from SubscribeURL: ' . $res->status_line ); + respond( 400 => 'Bad Request' ); return; } - respond(200 => 'OK'); + respond( 200 => 'OK' ); } sub process_bounce { my ($notification) = @_; my $type = $notification->{bounce}->{bounceType}; - # these should be infrequent and hopefully small - warn("SES: notification: " . encode_json($notification)); + if ( $type eq 'Transient' ) { - if ($type eq 'Transient') { # just log transient bounces - foreach my $recipient (@{ $notification->{bounce}->{bouncedRecipients} }) { + foreach my $recipient ( @{ $notification->{bounce}->{bouncedRecipients} } ) { my $address = $recipient->{emailAddress}; - Bugzilla->audit("SES: transient bounce for <$address>"); + Bugzilla->audit("transient bounce for <$address>"); } } - elsif ($type eq 'Permanent') { + elsif ( $type eq 'Permanent' ) { + # disable each account that is permanently bouncing - foreach my $recipient (@{ $notification->{bounce}->{bouncedRecipients} }) { + foreach my $recipient ( @{ $notification->{bounce}->{bouncedRecipients} } ) { my $address = $recipient->{emailAddress}; - my $reason = sprintf('(%s) %s', $recipient->{action} // 'error', - $recipient->{diagnosticCode} // 'unknown'); + my $reason + = sprintf( '(%s) %s', $recipient->{action} // 'error', $recipient->{diagnosticCode} // 'unknown' ); - my $user = Bugzilla::User->new({ name => $address, cache => 1 }); + my $user = Bugzilla::User->new( { name => $address, cache => 1 } ); if ($user) { + # never auto-disable admin accounts - if ($user->in_group('admin')) { - Bugzilla->audit("SES: ignoring permanent bounce for admin <$address>: $reason"); + if ( $user->in_group('admin') ) { + Bugzilla->audit("ignoring permanent bounce for admin <$address>: $reason"); } else { my $template = Bugzilla->template_inner(); - my $vars = { - mta => $notification->{bounce}->{reportingMTA} // 'unknown', + my $vars = { + mta => $notification->{bounce}->{reportingMTA} // 'unknown', reason => $reason, }; my $disable_text; - $template->process('admin/users/bounce-disabled.txt.tmpl', $vars, \$disable_text) + $template->process( 'admin/users/bounce-disabled.txt.tmpl', $vars, \$disable_text ) || die $template->error(); - $user->set_disabledtext($disable_text); - $user->set_disable_mail(1); - $user->update(); - Bugzilla->audit("SES: permanent bounce for <$address> disabled userid-" . $user->id . ": $reason"); + $user->set_disabledtext($disable_text); + $user->set_disable_mail(1); + $user->update(); + Bugzilla->audit( + "permanent bounce for <$address> disabled userid-" . $user->id . ": $reason" ); } } else { - Bugzilla->audit("SES: permanent bounce for <$address> has no user: $reason"); + Bugzilla->audit("permanent bounce for <$address> has no user: $reason"); } } } else { - warn "SES: Unsupported bounce type: $type\n"; + WARN("Unsupported bounce type: $type\n"); } - respond(200 => 'OK'); + respond( 200 => 'OK' ); } sub process_complaint { + # email notification to bugzilla admin my ($notification) = @_; - my $template = Bugzilla->template_inner(); - my $json = JSON::XS->new->pretty->utf8->canonical; + my $template = Bugzilla->template_inner(); + my $json = JSON::MaybeXS->new( + pretty => 1, + utf8 => 1, + canonical => 1, + ); - foreach my $recipient (@{ $notification->{complaint}->{complainedRecipients} }) { - my $reason = $notification->{complaint}->{complaintFeedbackType} // 'unknown'; + foreach my $recipient ( @{ $notification->{complaint}->{complainedRecipients} } ) { + my $reason = $notification->{complaint}->{complaintFeedbackType} // 'unknown'; my $address = $recipient->{emailAddress}; - Bugzilla->audit("SES: complaint for <$address> for '$reason'"); + Bugzilla->audit("complaint for <$address> for '$reason'"); my $vars = { email => $address, - user => Bugzilla::User->new({ name => $address, cache => 1 }), + user => Bugzilla::User->new( { name => $address, cache => 1 } ), reason => $reason, notification => $json->encode($notification), }; my $message; - $template->process('email/ses-complaint.txt.tmpl', $vars, \$message) + $template->process( 'email/ses-complaint.txt.tmpl', $vars, \$message ) || die $template->error(); MessageToMTA($message); } - respond(200 => 'OK'); + respond( 200 => 'OK' ); } sub respond { - my ($code, $message) = @_; - print Bugzilla->cgi->header( - -status => "$code $message", - ); + my ( $code, $message ) = @_; + print Bugzilla->cgi->header( -status => "$code $message" ); + # apache will generate non-200 response pages for us say html_quote($message) if $code == 200; } @@ -176,17 +187,17 @@ sub respond { sub decode_json_wrapper { my ($json) = @_; my $result; - if (!defined $json) { - warn 'SES: Missing JSON from ' . remote_ip() . "\n"; - respond(400 => 'Bad Request'); + if ( !defined $json ) { + WARN( 'Missing JSON from ' . remote_ip() ); + respond( 400 => 'Bad Request' ); return undef; } my $ok = try { $result = decode_json($json); } catch { - warn 'SES: Malformed JSON from ' . remote_ip() . "\n"; - respond(400 => 'Bad Request'); + WARN( 'Malformed JSON from ' . remote_ip() ); + respond( 400 => 'Bad Request' ); return undef; }; return $ok ? $result : undef; @@ -195,9 +206,9 @@ sub decode_json_wrapper { sub ua { my $ua = LWP::UserAgent->new(); $ua->timeout(10); - $ua->protocols_allowed(['http', 'https']); - if (my $proxy_url = Bugzilla->params->{'proxy_url'}) { - $ua->proxy(['http', 'https'], $proxy_url); + $ua->protocols_allowed( [ 'http', 'https' ] ); + if ( my $proxy_url = Bugzilla->params->{'proxy_url'} ) { + $ua->proxy( [ 'http', 'https' ], $proxy_url ); } else { $ua->env_proxy; -- cgit v1.2.3-24-g4f1b