From bbd091bd25c907f6f18fed0b8ccc351429261956 Mon Sep 17 00:00:00 2001 From: Byron Jones Date: Tue, 6 Oct 2015 00:29:02 +0800 Subject: Bug 1164063 - show a warning near the attachments table for sec-high/sec-crit bugs without sec-approval? on patches --- Bugzilla/Bug.pm | 6 +++ extensions/BMO/Extension.pm | 51 +++++++++++++++++++++- .../hook/attachment/list-warnings.html.tmpl | 30 +++++++++---- .../hook/bug_modal/attachments-warnings.html.tmpl | 24 +++++++--- extensions/BMO/web/styles/bug_modal.css | 8 +++- extensions/BMO/web/styles/edit_bug.css | 8 +++- 6 files changed, 108 insertions(+), 19 deletions(-) diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm index 312039306..73dc98963 100644 --- a/Bugzilla/Bug.pm +++ b/Bugzilla/Bug.pm @@ -3621,6 +3621,12 @@ sub keyword_objects { return $self->{'keyword_objects'}; } +sub has_keyword { + my ($self, $keyword) = @_; + $keyword = lc($keyword); + return any { lc($_->name) eq $keyword } @{ $self->keyword_objects }; +} + sub comments { my ($self, $params) = @_; return [] if $self->{'error'}; diff --git a/extensions/BMO/Extension.pm b/extensions/BMO/Extension.pm index f4e16d416..6938eb790 100644 --- a/extensions/BMO/Extension.pm +++ b/extensions/BMO/Extension.pm @@ -46,7 +46,7 @@ use DateTime; use Email::MIME::ContentType qw(parse_content_type); use Encode qw(find_encoding encode_utf8); use File::MimeInfo::Magic; -use List::MoreUtils qw(natatime any); +use List::MoreUtils qw(natatime any last_value); use List::Util qw(first); use Scalar::Util qw(blessed); use Sys::Syslog qw(:DEFAULT); @@ -67,6 +67,7 @@ BEGIN { *Bugzilla::Bug::reporters_hw_os = \&_bug_reporters_hw_os; *Bugzilla::Bug::is_unassigned = \&_bug_is_unassigned; *Bugzilla::Bug::has_current_patch = \&_bug_has_current_patch; + *Bugzilla::Bug::missing_sec_approval = \&_bug_missing_sec_approval; *Bugzilla::Product::default_security_group = \&_default_security_group; *Bugzilla::Product::default_security_group_obj = \&_default_security_group_obj; *Bugzilla::Product::group_always_settable = \&_group_always_settable; @@ -821,6 +822,54 @@ sub _bug_has_current_patch { return 0; } +sub _bug_missing_sec_approval { + my ($self) = @_; + # see https://wiki.mozilla.org/Security/Bug_Approval_Process for the rules + + # no need to alert once a bug is closed + return 0 if $self->resolution; + + # only bugs with sec-high or sec-critical keywords need sec-approval + return 0 unless $self->has_keyword('sec-high') || $self->has_keyword('sec-critical'); + + # look for patches with sec-approval set to any value + foreach my $attachment (@{ $self->attachments }) { + next if $attachment->isobsolete || !$attachment->ispatch; + foreach my $flag (@{ $attachment->flags }) { + # only one patch needs sec-approval + return 0 if $flag->name eq 'sec-approval'; + } + } + + # tracking flags + require Bugzilla::Extension::TrackingFlags::Flag; + my $flags = Bugzilla::Extension::TrackingFlags::Flag->match({ + product => $self->product, + component => $self->component, + bug_id => $self->id, + is_active => 1, + WHERE => { + 'name like ?' => 'cf_status_firefox%', + }, + }); + # set flags are added after the sql query, filter those out + $flags = [ grep { $_->name =~ /^cf_status_firefox/ } @$flags ]; + return 0 unless @$flags; + + my $nightly = last_value { $_->name !~ /_esr\d+$/ } @$flags; + my $set = 0; + foreach my $flag (@$flags) { + my $value = $flag->bug_flag($self->id)->value; + next if $value eq '---'; + $set++; + # sec-approval is required if any of the current status-firefox + # tracking flags that aren't the latest are set to 'affected' + return 1 if $flag->name ne $nightly->name && $value eq 'affected'; + } + # sec-approval is required if no tracking flags are set + return $set == 0; +} + sub _product_default_platform_id { $_[0]->{default_platform_id} } sub _product_default_op_sys_id { $_[0]->{default_op_sys_id} } diff --git a/extensions/BMO/template/en/default/hook/attachment/list-warnings.html.tmpl b/extensions/BMO/template/en/default/hook/attachment/list-warnings.html.tmpl index bc4480084..693051996 100644 --- a/extensions/BMO/template/en/default/hook/attachment/list-warnings.html.tmpl +++ b/extensions/BMO/template/en/default/hook/attachment/list-warnings.html.tmpl @@ -7,12 +7,26 @@ #%] [% - RETURN UNLESS user.in_group('editbugs'); - RETURN UNLESS bug.attachments.size && bug.is_unassigned && bug.has_current_patch; + RETURN UNLESS user.in_group('editbugs') || bug.assigned_to.id == user.id; + RETURN UNLESS bug.attachments.size && bug.has_current_patch; %] - - - - Unassigned [% terms.bug %] with patches attached - - + +[% IF bug.is_unassigned %] + + + + Unassigned [% terms.bug %] with patches attached + + +[% END %] + +[% IF bug.missing_sec_approval %] + + + + + sec-approval required on patches before landing + + + +[% END %] diff --git a/extensions/BMO/template/en/default/hook/bug_modal/attachments-warnings.html.tmpl b/extensions/BMO/template/en/default/hook/bug_modal/attachments-warnings.html.tmpl index b00eabc1f..5242f47b4 100644 --- a/extensions/BMO/template/en/default/hook/bug_modal/attachments-warnings.html.tmpl +++ b/extensions/BMO/template/en/default/hook/bug_modal/attachments-warnings.html.tmpl @@ -7,10 +7,22 @@ #%] [% - RETURN UNLESS user.in_group('editbugs'); - RETURN UNLESS bug.attachments.size && bug.is_unassigned && bug.has_current_patch; + RETURN UNLESS user.in_group('editbugs') || bug.assigned_to.id == user.id; + RETURN UNLESS bug.attachments.size && bug.has_current_patch; %] -
- - Unassigned [% terms.bug %] with patches attached -
+ +[% IF bug.is_unassigned %] +
+ + Unassigned [% terms.bug %] with patches attached +
+[% END %] + +[% IF bug.missing_sec_approval %] +
+ + + sec-approval required on patches before landing + +
+[% END %] diff --git a/extensions/BMO/web/styles/bug_modal.css b/extensions/BMO/web/styles/bug_modal.css index 3de7bde8b..8d14ed11a 100644 --- a/extensions/BMO/web/styles/bug_modal.css +++ b/extensions/BMO/web/styles/bug_modal.css @@ -13,10 +13,14 @@ border-radius: 4px; } -#unassigned_with_patches { +.attachment-warning { padding-left: 4px; } -#unassigned_with_patches img { +.attachment-warning img { vertical-align: sub; } + +#sec-approval-warning a { + color: #b70000; +} diff --git a/extensions/BMO/web/styles/edit_bug.css b/extensions/BMO/web/styles/edit_bug.css index 7da05a126..fa4403177 100644 --- a/extensions/BMO/web/styles/edit_bug.css +++ b/extensions/BMO/web/styles/edit_bug.css @@ -40,10 +40,14 @@ input#cf_rank { width: 3em; } -#unassigned_with_patches { +.attachment-warning { font-weight: normal; } -#unassigned_with_patches img { +.attachment-warning img { vertical-align: sub; } + +#sec-approval-warning a { + color: #b70000; +} -- cgit v1.2.3-24-g4f1b