Skip to content

Commit 7125406

Browse files
dklawrendklawren
authored
Bug 2002037 - JiraWebhookSync Extension: Incorrect hostname used for see_also urls added to outgoing webhook payloads
1 parent 181fcb8 commit 7125406

File tree

5 files changed

+96
-83
lines changed

5 files changed

+96
-83
lines changed

‎extensions/JiraWebhookSync/Extension.pm‎

Lines changed: 45 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ use Bugzilla::Logging;
2020
use Bugzilla::Util qw(trim);
2121

2222
use JSON::MaybeXS qw(decode_json);
23-
use List::Util qw(uniq);
23+
use List::Util qw(none uniq);
2424
use Mojo::URL;
2525
use Mojo::Util qw(dumper);
2626

@@ -35,16 +35,38 @@ sub db_schema_abstract_schema {
3535
NOTNULL => 1,
3636
REFERENCES => {TABLE => 'bugs', COLUMN => 'bug_id', DELETE => 'CASCADE',}
3737
},
38-
jira_id => {TYPE => 'VARCHAR(255)', NOTNULL => 1,},
38+
jira_url => {TYPE => 'VARCHAR(255)', NOTNULL => 1,},
3939
jira_project_key => {TYPE => 'VARCHAR(100)', NOTNULL => 1,},
4040
],
4141
INDEXES => [
42-
jira_bug_map_bug_id_idx => {FIELDS => ['bug_id', 'jira_id'], TYPE => 'UNIQUE',},
42+
jira_bug_map_bug_id_idx => {FIELDS => ['bug_id', 'jira_url'], TYPE => 'UNIQUE',},
4343
jira_bug_map_project_idx => ['jira_project_key'],
4444
],
4545
};
4646
}
4747

48+
sub install_update_db {
49+
my ($self) = @_;
50+
my $dbh = Bugzilla->dbh;
51+
52+
if (!$dbh->bz_column_info('jira_bug_map', 'jira_url')) {
53+
$dbh->bz_add_column('jira_bug_map', 'jira_url', {TYPE => 'VARCHAR(255)'});
54+
55+
my $jira_rows
56+
= $dbh->selectall_arrayref('SELECT id, jira_id FROM jira_bug_map');
57+
foreach my $row (@{$jira_rows}) {
58+
my ($id, $jira_id) = @{$row};
59+
my $jira_url = "https://mozilla-hub.atlassian.net/browse/$jira_id";
60+
$dbh->do('UPDATE jira_bug_map SET jira_url = ? WHERE id = ?', undef, $jira_url,
61+
$id);
62+
}
63+
64+
$dbh->bz_drop_column('jira_bug_map', 'jira_id');
65+
$dbh->bz_alter_column('jira_bug_map', 'jira_url',
66+
{TYPE => 'VARCHAR(255)', NOTNULL => 1});
67+
}
68+
}
69+
4870
# Adds the JiraWebhookSync configuration panel to the admin interface.
4971
# This hook allows administrators to configure Jira webhook synchronization settings.
5072
sub config_add_panels {
@@ -61,24 +83,27 @@ sub bug_start_of_update {
6183
my $new_bug = $args->{bug};
6284

6385
foreach my $see_also (@{$new_bug->see_also}) {
86+
my $see_also_url = $see_also->name;
6487

6588
# Check if this see_also URL corresponds to a Jira ticket
66-
my ($jira_id, $project_key)
67-
= Bugzilla::Extension::JiraWebhookSync::JiraBugMap->extract_jira_info(
68-
$see_also->name);
89+
my $project_key
90+
= Bugzilla::Extension::JiraWebhookSync::JiraBugMap->extract_jira_project_key(
91+
$see_also_url);
6992

70-
next unless $jira_id && $project_key;
93+
next unless $project_key;
7194

72-
INFO("Intercepting see_also for Jira ticket: $jira_id (project: $project_key)");
95+
INFO(
96+
"Intercepting see_also for Jira ticket: $see_also_url (project: $project_key)");
7397

74-
# Add the jira id and project key to the jira_bug_map table unless it already exists
98+
# Add the jira id and project key to the jira_bug_map table unless it already exists
7599
my $existing_map
76-
= Bugzilla::Extension::JiraWebhookSync::JiraBugMap->get_by_bug_id($new_bug->id);
100+
= Bugzilla::Extension::JiraWebhookSync::JiraBugMap->get_by_bug_id(
101+
$new_bug->id);
77102
if (!$existing_map) {
78103
INFO('Creating new Jira mapping for bug ' . $new_bug->id);
79104
Bugzilla::Extension::JiraWebhookSync::JiraBugMap->create({
80105
bug_id => $new_bug->id,
81-
jira_id => $jira_id,
106+
jira_url => $see_also_url,
82107
jira_project_key => $project_key,
83108
});
84109
}
@@ -112,25 +137,15 @@ sub webhook_before_send {
112137
INFO("Processing webhook for bug $bug_id to Jira host $hostname");
113138

114139
# Check if there's a Jira mapping for this bug
115-
my $jira_map
116-
= Bugzilla::Extension::JiraWebhookSync::JiraBugMap->get_by_bug_id($bug_id);
117-
118-
if ($jira_map) {
119-
120-
# Construct the Jira URL from the mapping
121-
my $jira_url = "https://$hostname/browse/" . $jira_map->jira_id;
122-
123-
INFO("Adding Jira see_also to webhook payload: $jira_url");
124-
125-
# Add the Jira URL to the see_also array in the payload
126-
if (!exists $payload->{bug}->{see_also}) {
127-
$payload->{bug}->{see_also} = [];
128-
}
129-
130-
# Only add if not already present
131-
my %existing_see_also = map { $_ => 1 } @{$payload->{bug}->{see_also}};
132-
if (!$existing_see_also{$jira_url}) {
133-
push @{$payload->{bug}->{see_also}}, $jira_url;
140+
if (my $jira_map
141+
= Bugzilla::Extension::JiraWebhookSync::JiraBugMap->get_by_bug_id($bug_id))
142+
{
143+
INFO('Adding Jira see_also to webhook payload: ' . $jira_map->jira_url);
144+
145+
# Add the Jira URL to the see_also array in the payload if not already present
146+
$payload->{bug}->{see_also} ||= [];
147+
if (none { $_ eq $jira_map->jira_url } @{$payload->{bug}->{see_also}}) {
148+
push @{$payload->{bug}->{see_also}}, $jira_map->jira_url;
134149
}
135150
}
136151

‎extensions/JiraWebhookSync/lib/JiraBugMap.pm‎

Lines changed: 35 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,14 @@ use base qw(Bugzilla::Object);
1515

1616
use Bugzilla;
1717
use Bugzilla::Bug;
18+
use Bugzilla::Constants;
1819
use Bugzilla::Error;
1920
use Bugzilla::Logging;
2021
use Bugzilla::Util qw(detaint_natural trim);
2122

2223
use JSON::MaybeXS qw(decode_json);
2324
use List::Util qw(none);
24-
use Mojo::URL;
25+
use URI;
2526
use Scalar::Util qw(blessed);
2627

2728
###############################
@@ -33,31 +34,31 @@ use constant DB_TABLE => 'jira_bug_map';
3334
use constant DB_COLUMNS => qw(
3435
id
3536
bug_id
36-
jira_id
3737
jira_project_key
38+
jira_url
3839
);
3940

4041
use constant UPDATE_COLUMNS => qw(
41-
jira_id
42+
jira_url
4243
jira_project_key
4344
);
4445

4546
use constant VALIDATORS => {
4647
bug_id => \&_check_bug_id,
47-
jira_id => \&_check_jira_id,
48+
jira_url => \&_check_jira_url,
4849
jira_project_key => \&_check_jira_project_key,
4950
};
5051

5152
use constant VALIDATOR_DEPENDENCIES => {
52-
jira_id => ['jira_project_key'],
53+
jira_url => ['jira_project_key'],
5354
};
5455

5556
###############################
5657
#### Accessors ######
5758
###############################
5859

5960
sub bug_id { return $_[0]->{bug_id}; }
60-
sub jira_id { return $_[0]->{jira_id}; }
61+
sub jira_url { return $_[0]->{jira_url}; }
6162
sub jira_project_key { return $_[0]->{jira_project_key}; }
6263

6364
sub bug {
@@ -69,7 +70,7 @@ sub bug {
6970
#### Mutators #####
7071
###############################
7172

72-
sub set_jira_id { $_[0]->set('jira_id', $_[1]); }
73+
sub set_jira_url { $_[0]->set('jira_url', $_[1]); }
7374
sub set_jira_project_key { $_[0]->set('jira_project_key', $_[1]); }
7475

7576
###############################
@@ -85,13 +86,26 @@ sub _check_bug_id {
8586
return $bug_id;
8687
}
8788

88-
sub _check_jira_id {
89-
my ($invocant, $jira_id) = @_;
89+
sub _check_jira_url {
90+
my ($invocant, $jira_url) = @_;
9091

91-
$jira_id = trim($jira_id);
92-
$jira_id || ThrowUserError('jira_id_required');
92+
$jira_url = trim($jira_url);
93+
$jira_url || ThrowUserError('jira_url_required');
9394

94-
return $jira_id;
95+
$jira_url = URI->new($jira_url);
96+
97+
if ( !$jira_url
98+
|| ($jira_url->scheme ne 'http' && $jira_url->scheme ne 'https')
99+
|| !$jira_url->authority
100+
|| length($jira_url->path) > MAX_BUG_URL_LENGTH)
101+
{
102+
ThrowUserError('jira_url_required');
103+
}
104+
105+
# always https
106+
$jira_url->scheme('https');
107+
108+
return $jira_url->as_string;
95109
}
96110

97111
sub _check_jira_project_key {
@@ -117,38 +131,28 @@ sub get_by_bug_id {
117131
});
118132
}
119133

120-
# Get mapping by jira_id
121-
sub get_by_jira_id {
122-
my ($class, $jira_id) = @_;
123-
124-
return $class->new({
125-
condition => 'jira_id = ?',
126-
values => [$jira_id],
127-
});
128-
}
129-
130-
# Extract Jira project key from a Jira URL or ID
131-
sub extract_jira_info {
134+
# Extract Jira project key from a Jira URL
135+
sub extract_jira_project_key {
132136
my ($class, $see_also) = @_;
133137
my $params = Bugzilla->params;
134138

135139
# Match pattern
136140
# https://jira.example.com/browse/PROJ-123
137-
my $url = Mojo::URL->new($see_also);
138-
my ($project_key, $jira_id);
139-
if ($url->path =~ m{^/browse/([[:upper:]]+-\d+)$}) {
140-
$jira_id = $1;
141-
($project_key) = $jira_id =~ /^([[:upper:]]+)-/;
141+
my $url = URI->new($see_also);
142+
my $project_key = undef;
143+
if ($url->path =~ m{^/browse/([[:upper:]]+)-\d+$}) {
144+
$project_key = $1;
145+
return undef if !$project_key;
142146

143147
# Return undef if project key is not in configured list
144148
if (none { $_ eq $project_key }
145149
@{decode_json($params->{jira_webhook_sync_project_keys} || '[]')})
146150
{
147-
return (undef, undef);
151+
return undef;
148152
}
149153
}
150154

151-
return ($jira_id, $project_key);
155+
return $project_key;
152156
}
153157

154158
1;

‎extensions/JiraWebhookSync/t/bmo/jira_webhook_sync.t‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ $t->get_ok('http://externalapi.test:8001/webhooks/last_payload')
117117
# bug is updated and the webhook is triggered.
118118
# Then we look again at the last payload from the webhook, which should
119119
# contain the see also value that was generated from the mapping table.
120-
my $jira_url = 'https://externalapi.test/browse/BZFF-100';
120+
my $jira_url = 'https://example.com/browse/BZFF-100';
121121
my $update = {see_also => {add => [$jira_url]}, 'priority' => 'P2'};
122122
$t->put_ok($config->{browser_url}
123123
. "/rest/bug/$bug_id_1" =>

‎extensions/JiraWebhookSync/t/sanity.t‎

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -21,31 +21,25 @@ use ok 'Bugzilla::Extension::JiraWebhookSync::JiraBugMap';
2121
local Bugzilla->params->{jira_webhook_sync_project_keys} = '["FOO","BAZ"]';
2222

2323
# Success
24-
my ($jira_id, $project_key)
25-
= Bugzilla::Extension::JiraWebhookSync::JiraBugMap->extract_jira_info(
24+
my $project_key
25+
= Bugzilla::Extension::JiraWebhookSync::JiraBugMap->extract_jira_project_key(
2626
'https://externalapi.test/browse/FOO-100');
27-
ok(
28-
$jira_id eq 'FOO-100' && $project_key eq 'FOO',
29-
'Correct Jira ID and Project Key extracted'
30-
);
31-
($jira_id, $project_key)
32-
= Bugzilla::Extension::JiraWebhookSync::JiraBugMap->extract_jira_info(
27+
ok($project_key eq 'FOO', 'Correct Jira ID and Project Key extracted');
28+
$project_key
29+
= Bugzilla::Extension::JiraWebhookSync::JiraBugMap->extract_jira_project_key(
3330
'https://externalapi.test/browse/BAZ-300');
34-
ok(
35-
$jira_id eq 'BAZ-300' && $project_key eq 'BAZ',
36-
'Correct Jira ID and Project Key extracted'
37-
);
31+
ok($project_key eq 'BAZ', 'Correct Jira ID and Project Key extracted');
3832

3933
# Failed
40-
($jira_id, $project_key)
41-
= Bugzilla::Extension::JiraWebhookSync::JiraBugMap->extract_jira_info(
34+
$project_key
35+
= Bugzilla::Extension::JiraWebhookSync::JiraBugMap->extract_jira_project_key(
4236
'https://externalapi.test/browse/FOO-100/some/more/path');
43-
ok(!defined $jira_id && !defined $project_key,
37+
ok(!defined $project_key,
4438
'No Jira ID or Project Key extracted for improperly formatted url');
45-
($jira_id, $project_key)
46-
= Bugzilla::Extension::JiraWebhookSync::JiraBugMap->extract_jira_info(
39+
$project_key
40+
= Bugzilla::Extension::JiraWebhookSync::JiraBugMap->extract_jira_project_key(
4741
'https://externalapi.test/browse/BAR-100');
48-
ok(!defined $jira_id && !defined $project_key,
42+
ok(!defined $project_key,
4943
'No Jira ID or Project Key extracted for wrong project key');
5044

5145
done_testing();

‎extensions/JiraWebhookSync/template/en/default/hook/global/user-error.html.tmpl‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@
1010
[% title = "Bug ID Required" %]
1111
A [% terms.bug %] ID is required for the Jira issue.
1212

13-
[% ELSIF error == "jira_id_required" %]
14-
[% title = "Jira ID Required" %]
15-
A Jira issue ID is required.
13+
[% ELSIF error == "jira_url_required" %]
14+
[% title = "Jira Url Required or Invalid" %]
15+
A Jira issue url was missing or is not a valid url.
1616

1717
[% ELSIF error == "jira_project_key_required" %]
1818
[% title = "Jira Project Key Required" %]

0 commit comments

Comments
 (0)