Bug 825886: When moving bugs from one product to another, I should be able to keep a security bug private across groups that I'm not a member of

10 files changed, 112 insertions, 111 deletions

diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index 383a865ef..613aefdc9 100644
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -1699,14 +1699,6 @@ sub _check_groups {
                                      : $params->{product};
     my %add_groups;
 
-    # BMO: Allow extension to add groups before the
-    # real checks are done.
-    Bugzilla::Hook::process('bug_check_groups', {
-        product     => $product,
-        group_names => $group_names, 
-        add_groups  => \%add_groups 
-    });
-
     # In email or WebServices, when the "groups" item actually 
     # isn't specified, then just add the default groups.
     if (!defined $group_names) {
@@ -1725,12 +1717,9 @@ sub _check_groups {
         foreach my $name (@$group_names) {
             my $group = Bugzilla::Group->check_no_disclose({ %args, name => $name });
 
-            # BMO: Do not check group_is_settable if the group is 
-            # already added, such as from the extension hook. group_is_settable
-            # will reject any group the user is not currently in.
-            if (!$add_groups{$group->id} 
-                && !$product->group_is_settable($group)) 
-            {
+            # BMO : allow bugs to be always placed into some groups
+            if (!$product->group_always_settable($group)
+                && !$product->group_is_settable($group)) {
                 ThrowUserError('group_restriction_not_allowed', { %args, name => $name });
             }
             $add_groups{$group->id} = $group;
diff --git a/extensions/BMO/Extension.pm b/extensions/BMO/Extension.pm
index 798bca82a..af5ae526e 100644
--- a/extensions/BMO/Extension.pm
+++ b/extensions/BMO/Extension.pm
@@ -25,22 +25,23 @@ package Bugzilla::Extension::BMO;
 use strict;
 use base qw(Bugzilla::Extension);
 
-use Bugzilla::Field;
 use Bugzilla::Constants;
-use Bugzilla::Status;
+use Bugzilla::Error;
+use Bugzilla::Field;
+use Bugzilla::Group;
+use Bugzilla::Mailer;
 use Bugzilla::Product;
+use Bugzilla::Status;
+use Bugzilla::Token;
 use Bugzilla::User;
 use Bugzilla::User::Setting;
-use Bugzilla::Util qw(html_quote trick_taint trim datetime_from detaint_natural);
-use Bugzilla::Token;
-use Bugzilla::Error;
-use Bugzilla::Mailer;
 use Bugzilla::Util;
+use Bugzilla::Util qw(html_quote trick_taint trim datetime_from detaint_natural);
 
-use Scalar::Util qw(blessed);
 use Date::Parse;
 use DateTime;
 use Encode qw(find_encoding encode_utf8);
+use Scalar::Util qw(blessed);
 use Sys::Syslog qw(:DEFAULT setlogsock);
 
 use Bugzilla::Extension::BMO::Constants;
@@ -55,6 +56,10 @@ our $VERSION = '0.1';
 
 BEGIN {
     *Bugzilla::Bug::last_closed_date = \&_last_closed_date;
+    *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;
+    *Bugzilla::check_default_product_security_group = \&_check_default_product_security_group;
 }
 
 sub template_before_process {
@@ -116,9 +121,6 @@ sub template_before_process {
         # Purpose: for pretty product chooser
         $vars->{'format'} = Bugzilla->cgi->param('format');
 
-        # Data needed for "this is a security bug" checkbox
-        $vars->{'sec_groups'} = \%product_sec_groups;
-
         if ($format eq 'doc.html.tmpl') {
             my $versions = Bugzilla::Product->new({ name => 'Core' })->versions;
             $vars->{'versions'} = [ reverse @$versions ];
@@ -568,25 +570,6 @@ sub bug_format_comment {
     });
 }
 
-# Purpose: make it always possible to file bugs in certain groups.
-sub bug_check_groups {
-    my ($self, $args) = @_;
-    my $group_names = $args->{'group_names'};
-    my $add_groups = $args->{'add_groups'};
-
-    return unless $group_names;
-    $group_names = ref $group_names 
-                   ? $group_names 
-                   : [ map { trim($_) } split(',', $group_names) ];
-
-    foreach my $name (@$group_names) {
-        if (exists $always_fileable_group{$name}) {
-            my $group = new Bugzilla::Group({ name => $name }) or next;
-            $add_groups->{$group->id} = $group;
-        }
-    }
-}
-
 # Purpose: generically handle generating pretty blocking/status "flags" from
 # custom field names.
 sub quicksearch_map {
@@ -1138,4 +1121,40 @@ sub query_database {
     }
 }
 
+# you can always file bugs into a product's default security group, as well as
+# into any of the groups in @always_fileable_groups
+sub _group_always_settable {
+    my ($self, $group) = @_;
+    return
+        $group->name eq $self->default_security_group
+        || ((grep { $_ eq $group->name } @always_fileable_groups) ? 1 : 0);
+}
+
+sub _default_security_group {
+    my ($self) = @_;
+    return exists $product_sec_groups{$self->name}
+        ? $product_sec_groups{$self->name}
+        : $product_sec_groups{_default};
+}
+
+sub _default_security_group_obj {
+    my ($self) = @_;
+    return unless my $group_name = $self->default_security_group;
+    return Bugzilla::Group->new({ name => $group_name, cache => 1 })
+}
+
+# called from the verify version, component, and group page.
+# if we're making a group invalid, stuff the default group into the cgi param
+# to make it checked by default.
+sub _check_default_product_security_group {
+    my ($self, $product, $invalid_groups, $optional_group_controls) = @_;
+    return unless my $group = $product->default_security_group_obj;
+    if (@$invalid_groups) {
+        my $cgi = Bugzilla->cgi;
+        my @groups = $cgi->param('groups');
+        push @groups, $group->name unless grep { $_ eq $group->name } @groups;
+        $cgi->param('groups', @groups);
+    }
+}
+
 __PACKAGE__->NAME;
diff --git a/extensions/BMO/lib/Data.pm b/extensions/BMO/lib/Data.pm
index 2b4ccb729..9a5ca0799 100644
--- a/extensions/BMO/lib/Data.pm
+++ b/extensions/BMO/lib/Data.pm
@@ -35,7 +35,7 @@ our @EXPORT = qw( $cf_visible_in_products
                   $status_trusted_wanters
                   $status_trusted_setters
                   $other_setters
-                  %always_fileable_group
+                  @always_fileable_groups
                   %group_auto_cc
                   %product_sec_groups
                   %create_bug_formats );
@@ -368,34 +368,35 @@ our $other_setters = {
     'cf_colo_site'  => ['infra', 'build'],
 };
 
-# Groups in which you can always file a bug, whoever you are.
-our %always_fileable_group = (
-    'addons-security'                   => 1,
-    'bugzilla-security'                 => 1,
-    'client-services-security'          => 1,
-    'consulting'                        => 1,
-    'core-security'                     => 1,
-    'finance'                           => 1,
-    'infra'                             => 1,
-    'infrasec'                          => 1,
-    'l20n-security'                     => 1,
-    'marketing-private'                 => 1,
-    'mozilla-confidential'              => 1,
-    'mozilla-corporation-confidential'  => 1,
-    'mozilla-foundation-confidential'   => 1,
-    'mozilla-engagement'                => 1,
-    'mozilla-messaging-confidential'    => 1,
-    'partner-confidential'              => 1,
-    'payments-confidential'             => 1,
-    'tamarin-security'                  => 1,
-    'websites-security'                 => 1,
-    'webtools-security'                 => 1,
-    'winqual-data'                      => 1,
+# Groups in which you can always file a bug, regardless of product or user.
+our @always_fileable_groups = qw(
+    addons-security
+    bugzilla-security
+    client-services-security
+    consulting
+    core-security
+    finance
+    infra
+    infrasec
+    l20n-security
+    marketing-private
+    mozilla-confidential
+    mozilla-corporation-confidential
+    mozilla-foundation-confidential
+    mozilla-engagement
+    mozilla-messaging-confidential
+    partner-confidential
+    payments-confidential
+    tamarin-security
+    websites-security
+    webtools-security
+    winqual-data
 );
 
 # Mapping of products to their security bits
 our %product_sec_groups = (
     "addons.mozilla.org"           => 'client-services-security',
+    "Air Mozilla"                  => 'mozilla-corporation-confidential',
     "Android Background Services"  => 'mozilla-services-security',
     "AUS"                          => 'client-services-security',
     "Bugzilla"                     => 'bugzilla-security',
@@ -450,9 +451,6 @@ our %group_auto_cc = (
     },
 );
 
-# Default security groups for products should always been fileable
-map { $always_fileable_group{$_} = 1 } values %product_sec_groups;
-
 # Force create-bug template by product
 # Users in 'include' group will be fored into using the form.
 our %create_bug_formats = (
diff --git a/extensions/BMO/template/en/default/bug/create/create-bootgecko-partner.html.tmpl b/extensions/BMO/template/en/default/bug/create/create-bootgecko-partner.html.tmpl
index cdfd90174..6e2778de0 100644
--- a/extensions/BMO/template/en/default/bug/create/create-bootgecko-partner.html.tmpl
+++ b/extensions/BMO/template/en/default/bug/create/create-bootgecko-partner.html.tmpl
@@ -210,9 +210,8 @@
 <tr>
   <th>Security:</th>
   <td>
-    [% sec_group = sec_groups.${product.name} || sec_groups._default %]
-    <input type="checkbox" name="groups"
-           value="[% sec_group FILTER html %]" id="group_[% sec_group FILTER html %]"
+    <input type="checkbox" name="groups" id="default_security_group"
+           value="[% product.default_security_group FILTER html %]"
       [% FOREACH g = group %]
         [% IF g.name == name %]
           [% ' checked="checked"' IF g.checked %]
@@ -220,8 +219,8 @@
         [% END %]
       [% END %]
     >
-    <label for="group_[% sec_group FILTER html %]">
-      Many users could be harmed by this security problem: 
+    <label for="default_security_group">
+      Many users could be harmed by this security problem:
       it should be kept hidden from the public until it is resolved.
     </label>
   </td>
diff --git a/extensions/BMO/template/en/default/bug/create/create-mdn.html.tmpl b/extensions/BMO/template/en/default/bug/create/create-mdn.html.tmpl
index cd3466fe7..f79363c99 100644
--- a/extensions/BMO/template/en/default/bug/create/create-mdn.html.tmpl
+++ b/extensions/BMO/template/en/default/bug/create/create-mdn.html.tmpl
@@ -252,11 +252,13 @@ strong.required:before {
     </p>
 
     <p id="detail_secure">
-      [% sec_group = sec_groups.${product.name} || sec_groups._default %]
-      <input type="checkbox" name="groups" id="groups" value="[% sec_group FILTER html %]">
-      <label for="groups"><strong>This <span id="secure_type">report</span> is about a problem 
-        that is putting users at risk. It should be kept hidden from the public until it is 
-        resolved.</strong></label>
+      <input type="checkbox" name="groups" id="groups"
+             value="[% product.default_security_group FILTER html %]">
+      <label for="groups">
+        <strong>This <span id="secure_type">report</span> is about a problem
+        that is putting users at risk. It should be kept hidden from the public
+        until it is resolved.</strong>
+      </label>
     </p>
 
     <input type="submit" id="commit" value="Submit"></td>
diff --git a/extensions/BMO/template/en/default/bug/create/create-mozpr.html.tmpl b/extensions/BMO/template/en/default/bug/create/create-mozpr.html.tmpl
index ad8216b47..cc4a8641d 100644
--- a/extensions/BMO/template/en/default/bug/create/create-mozpr.html.tmpl
+++ b/extensions/BMO/template/en/default/bug/create/create-mozpr.html.tmpl
@@ -565,7 +565,14 @@ TUI_hide_default('expert_fields');
 </tbody>
 
 <tbody class="expert_fields">
-  [% IF product.groups_available.size %]
+  [%# exclude the default security from from the groups_available %]
+  [%# list, as it will be added by the BMO extension %]
+  [% groups_available = [] %]
+  [% FOREACH group (product.groups_available) %]
+    [% NEXT IF group.name == product.default_security_group %]
+    [% groups_available.push(group) %]
+  [% END %]
+  [% IF groups_available.size %]
   <tr>
     <th>&nbsp;</th>
     <td colspan="3">
@@ -583,7 +590,7 @@ TUI_hide_default('expert_fields');
 
       <!-- Checkboxes -->
       <input type="hidden" name="defined_groups" value="1">
-      [% FOREACH group = product.groups_available %]
+      [% FOREACH group = groups_available %]
         <input type="checkbox" id="group_[% group.id FILTER html %]"
                name="groups" value="[% group.name FILTER html %]"
                [% ' checked="checked"' IF default.groups.contains(group.name)
diff --git a/extensions/BMO/template/en/default/hook/bug/create/create-form.html.tmpl b/extensions/BMO/template/en/default/hook/bug/create/create-form.html.tmpl
index cbe921c76..3fdec2990 100644
--- a/extensions/BMO/template/en/default/hook/bug/create/create-form.html.tmpl
+++ b/extensions/BMO/template/en/default/hook/bug/create/create-form.html.tmpl
@@ -1,9 +1,8 @@
   <tr>
     <th>Security:</th>
     <td colspan="3">
-      [% sec_group = sec_groups.${product.name} || sec_groups._default %]
       [% PROCESS group_checkbox
-                 name = sec_group
+                 name = product.default_security_group
                  desc = "Many users could be harmed by this security problem: " _
                         "it should be kept hidden from the public until it is resolved."
       %]
diff --git a/extensions/BMO/template/en/default/hook/bug/create/create-guided-form.html.tmpl b/extensions/BMO/template/en/default/hook/bug/create/create-guided-form.html.tmpl
deleted file mode 100644
index a0fff4175..000000000
--- a/extensions/BMO/template/en/default/hook/bug/create/create-guided-form.html.tmpl
+++ /dev/null
@@ -1,22 +0,0 @@
-  <tr bgcolor="[% tablecolour FILTER html %]">
-    <td valign="middle" align="right">
-      <b>Security</b>
-    </td>
-    <td valign="top">
-      <p>
-        [% sec_group = sec_groups.${product.name} || sec_groups._default %]
-
-        <input type="checkbox" name="groups" 
-               id="groups" value="[% sec_group FILTER none %]"
-          [% FOREACH g = group %]
-            [% IF g.name == sec_group %]
-              [% " checked=\"checked\"" IF g.checked %]
-            [% END %]
-          [% END %]
-        >
-        <label for="groups">
-        Many users could be harmed by this security problem: it should be kept
-        hidden from the public until it is resolved.</label>
-      </p>
-    </td>
-  </tr>
diff --git a/template/en/default/bug/create/create.html.tmpl b/template/en/default/bug/create/create.html.tmpl
index 7f2087467..786fab551 100644
--- a/template/en/default/bug/create/create.html.tmpl
+++ b/template/en/default/bug/create/create.html.tmpl
@@ -766,7 +766,14 @@ TUI_hide_default('attachment_text_field');
 [% END %]
 
 <tbody class="expert_fields">
-  [% IF product.groups_available.size %]
+  [%# BMO - exclude the default security from from the groups_available %]
+  [%# list, as it will be added by the BMO extension %]
+  [% groups_available = [] %]
+  [% FOREACH group (product.groups_available) %]
+    [% NEXT IF group.name == product.default_security_group %]
+    [% groups_available.push(group) %]
+  [% END %]
+  [% IF groups_available.size %]
   <tr>
     <th>&nbsp;</th>
     <td colspan="3">
@@ -782,7 +789,7 @@ TUI_hide_default('attachment_text_field');
 
       <!-- Checkboxes -->
       <input type="hidden" name="defined_groups" value="1">
-      [% FOREACH group = product.groups_available %]
+      [% FOREACH group = groups_available %]
         <input type="checkbox" id="group_[% group.id FILTER html %]"
                name="groups" value="[% group.name FILTER html %]"
                [% ' checked="checked"' IF default.groups.contains(group.name)
diff --git a/template/en/default/bug/process/verify-new-product.html.tmpl b/template/en/default/bug/process/verify-new-product.html.tmpl
index c02c26470..1d2e8689f 100644
--- a/template/en/default/bug/process/verify-new-product.html.tmpl
+++ b/template/en/default/bug/process/verify-new-product.html.tmpl
@@ -120,9 +120,9 @@
   
   [% IF old_groups.size %]
     <p>These groups are not legal for the '[% product.name FILTER html %]'
-    product or you are not allowed to restrict [% terms.bugs %] to these groups.
-    [%+ terms.Bugs %] will no longer be restricted to these groups and may become
-    public if no other group applies:<br>
+    product or you are not allowed to restrict [% terms.bugs %] to these groups.<br>
+    <b>[%+ terms.Bugs %] will no longer be restricted to these groups and may become
+    public if no other group applies:</b><br>
     [% FOREACH group = old_groups %]
       <input type="checkbox" id="group_[% group.id FILTER html %]"
              name="groups" disabled="disabled" value="[% group.name FILTER html %]">
@@ -150,6 +150,9 @@
     [% END %]
   [% END %]
 
+  [%# BMO - check the default product sec-group to avoid accidental removal of all groups %]
+  [% CALL Bugzilla.check_default_product_security_group(product, old_groups, optional_groups) %]
+
   [% IF optional_groups.size %]
     <p>These groups are optional. You can decide to restrict [% terms.bugs %] to
     one or more of the following groups:<br>