From e5ee36ba43a80554aceb04d6b6c4504c6d21e3fb Mon Sep 17 00:00:00 2001 From: dklawren Date: Fri, 11 Aug 2017 16:02:16 -0400 Subject: Bug 1382225 - Missing code from PhabBugz extension such as http basic auth support and other minor improvements --- extensions/PhabBugz/lib/Constants.pm | 25 +++++++++++ extensions/PhabBugz/lib/Util.pm | 78 ++++++++++++++++++++++++----------- extensions/PhabBugz/lib/WebService.pm | 20 +++++++-- 3 files changed, 95 insertions(+), 28 deletions(-) create mode 100644 extensions/PhabBugz/lib/Constants.pm (limited to 'extensions/PhabBugz/lib') diff --git a/extensions/PhabBugz/lib/Constants.pm b/extensions/PhabBugz/lib/Constants.pm new file mode 100644 index 000000000..f7485e8c4 --- /dev/null +++ b/extensions/PhabBugz/lib/Constants.pm @@ -0,0 +1,25 @@ +# 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::Extension::PhabBugz::Constants; + +use 5.10.1; +use strict; +use warnings; + +use base qw(Exporter); +our @EXPORT = qw( + PHAB_AUTOMATION_USER + PHAB_ATTACHMENT_PATTERN + PHAB_CONTENT_TYPE +); + +use constant PHAB_ATTACHMENT_PATTERN => qr/^phabricator-D(\d+)/; +use constant PHAB_AUTOMATION_USER => 'phab-bot@bmo.tld'; +use constant PHAB_CONTENT_TYPE => 'text/x-phabricator-request'; + +1; diff --git a/extensions/PhabBugz/lib/Util.pm b/extensions/PhabBugz/lib/Util.pm index b12669c9d..57dc1d7d0 100644 --- a/extensions/PhabBugz/lib/Util.pm +++ b/extensions/PhabBugz/lib/Util.pm @@ -12,9 +12,11 @@ use strict; use warnings; use Bugzilla::Error; +use Bugzilla::Util qw(trim); +use Bugzilla::Extension::PhabBugz::Constants; -use Data::Dumper; -use JSON qw(encode_json decode_json); +use JSON::XS qw(encode_json decode_json); +use List::Util qw(first); use LWP::UserAgent; use base qw(Exporter); @@ -30,6 +32,7 @@ our @EXPORT = qw( get_project_phid get_revisions_by_ids intersect + is_attachment_phab_revision make_revision_private make_revision_public request @@ -55,28 +58,47 @@ sub get_revisions_by_ids { } sub create_revision_attachment { - my ($bug, $revision_id, $revision_title) = @_; + my ( $bug, $revision_id, $revision_title ) = @_; + + my $phab_base_uri = Bugzilla->params->{phabricator_base_uri}; + ThrowUserError('invalid_phabricator_uri') unless $phab_base_uri; + + my $revision_uri = $phab_base_uri . "D" . $revision_id; + + # Check for previous attachment with same revision id. + # If one matches then return it instead. This is fine as + # BMO does not contain actual diff content. + my @review_attachments = grep { is_attachment_phab_revision($_) } @{ $bug->attachments }; + my $review_attachment = first { trim($_->data) eq $revision_uri } @review_attachments; + return $review_attachment if defined $review_attachment; + + # No attachment is present, so we can now create new one + my $is_shadow_db = Bugzilla->is_shadow_db; + Bugzilla->switch_to_main_db if $is_shadow_db; my $dbh = Bugzilla->dbh; $dbh->bz_start_transaction; my ($timestamp) = $dbh->selectrow_array("SELECT NOW()"); - my $attachment = Bugzilla::Attachment->create({ - bug => $bug, - creation_ts => $timestamp, - data => 'http://phabricator.test/D' . $revision_id, - description => $revision_title, - filename => 'phabricator-D' . $revision_id . '-url.txt', - ispatch => 0, - isprivate => 0, - mimetype => 'text/x-phabricator-request', - }); + my $attachment = Bugzilla::Attachment->create( + { + bug => $bug, + creation_ts => $timestamp, + data => $revision_uri, + description => $revision_title, + filename => 'phabricator-D' . $revision_id . '-url.txt', + ispatch => 0, + isprivate => 0, + mimetype => PHAB_CONTENT_TYPE, + } + ); $bug->update($timestamp); $attachment->update($timestamp); $dbh->bz_commit_transaction; + Bugzilla->switch_to_shadow_db if $is_shadow_db; return $attachment; } @@ -92,7 +114,7 @@ sub get_bug_role_phids { my @bug_users = ( $bug->reporter ); push(@bug_users, $bug->assigned_to) - if !$bug->assigned_to->email !~ /^nobody\@mozilla\.org$/; + if $bug->assigned_to->email !~ /^nobody\@mozilla\.org$/; push(@bug_users, $bug->qa_contact) if $bug->qa_contact; push(@bug_users, @{ $bug->cc_users }) if @{ $bug->cc_users }; @@ -271,12 +293,19 @@ sub get_members_by_bmo_id { return \@phab_ids; } +sub is_attachment_phab_revision { + my ($attachment, $include_obsolete) = @_; + return ($attachment->contenttype eq PHAB_CONTENT_TYPE + && ($include_obsolete || !$attachment->isobsolete) + && $attachment->attacher->login eq 'phab-bot@bmo.tld') ? 1 : 0; +} + sub request { my ($method, $data) = @_; my $request_cache = Bugzilla->request_cache; my $params = Bugzilla->params; - my $ua = $request_cache->{phabricato_ua}; + my $ua = $request_cache->{phabricator_ua}; unless ($ua) { $ua = $request_cache->{phabricator_ua} = LWP::UserAgent->new(timeout => 10); if ($params->{proxy_url}) { @@ -285,10 +314,10 @@ sub request { $ua->default_header('Content-Type' => 'application/x-www-form-urlencoded'); } - my $phab_api_key = $params->{phabricator_api_key}; + my $phab_api_key = $params->{phabricator_api_key}; + ThrowUserError('invalid_phabricator_api_key') unless $phab_api_key; my $phab_base_uri = $params->{phabricator_base_uri}; ThrowUserError('invalid_phabricator_uri') unless $phab_base_uri; - ThrowUserError('invalid_phabricator_api_key') unless $phab_api_key; my $full_uri = $phab_base_uri . '/api/' . $method; @@ -297,13 +326,14 @@ sub request { my $response = $ua->post($full_uri, { params => encode_json($data) }); ThrowCodeError('phabricator_api_error', { reason => $response->message }) - if $response->is_error; - - my $result = decode_json($response->content); - if ($result->{error_code}) { - ThrowCodeError('phabricator_api_error', - { code => $result->{error_code}, - reason => $result->{error_info} }); + if $response->is_error; + + my $result; + my $result_ok = eval { $result = decode_json( $response->content); 1 }; + if ( !$result_ok ) { + ThrowCodeError( + 'phabricator_api_error', + { reason => 'JSON decode failure' } ); } return $result; diff --git a/extensions/PhabBugz/lib/WebService.pm b/extensions/PhabBugz/lib/WebService.pm index 11439ba70..0d88114f6 100644 --- a/extensions/PhabBugz/lib/WebService.pm +++ b/extensions/PhabBugz/lib/WebService.pm @@ -16,6 +16,7 @@ use base qw(Bugzilla::WebService); use Bugzilla::Attachment; use Bugzilla::Bug; use Bugzilla::BugMail; +use Bugzilla::Constants; use Bugzilla::Error; use Bugzilla::Extension::Push::Util qw(is_public); use Bugzilla::User; @@ -34,7 +35,7 @@ use Bugzilla::Extension::PhabBugz::Util qw( request ); -use Data::Dumper; +use MIME::Base64 qw(decode_base64); use constant PUBLIC_METHODS => qw( revision @@ -43,12 +44,23 @@ use constant PUBLIC_METHODS => qw( sub revision { my ($self, $params) = @_; + # Phabricator only supports sending credentials via HTTP Basic Auth + # so we exploit that function to pass in an API key as the password + # of basic auth. BMO does not support basic auth but does support + # use of API keys. + my $http_auth = Bugzilla->cgi->http('Authorization'); + $http_auth =~ s/^Basic\s+//; + $http_auth = decode_base64($http_auth); + my ($login, $api_key) = split(':', $http_auth); + $params->{'Bugzilla_login'} = $login; + $params->{'Bugzilla_api_key'} = $api_key; + + my $user = Bugzilla->login(LOGIN_REQUIRED); + unless (defined $params->{revision} && detaint_natural($params->{revision})) { ThrowCodeError('param_required', { param => 'revision' }) } - my $user = Bugzilla->set_user(Bugzilla::User->new({ name => 'conduit@mozilla.bugs' })); - # Obtain more information about the revision from Phabricator my $revision_id = $params->{revision}; my @revisions = get_revisions_by_ids([$revision_id]); @@ -65,7 +77,7 @@ sub revision { if (is_public($bug)) { $result = make_revision_public($revision_id); } - # Else bug is private + # else bug is private else { my $phab_sync_groups = Bugzilla->params->{phabricator_sync_groups} || ThrowUserError('invalid_phabricator_sync_groups'); -- cgit v1.2.3-24-g4f1b