Bug 408671 - Unable to set the "security sensitive" group on existing RHEL-5 bugzilla bugs
Unable to set the "security sensitive" group on existing RHEL-5 bugzilla bugs
Status: CLOSED NEXTRELEASE
Product: Bugzilla
Classification: Community
Component: Bugzilla General (Show other bugs)
devel
All Linux
high Severity high (vote)
: ---
: ---
Assigned To: Noura El hawary
: Reopened
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-12-03 09:32 EST by Stephen Tweedie
Modified: 2013-06-24 00:19 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-12-15 19:51:29 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
v1 to enable assigning bug to security group for non security group members (2.46 KB, patch)
2008-12-08 01:41 EST, Noura El hawary
dkl: review-
Details | Diff
Sample patch on allowing othercontrol groups in show_bug.cgi to be set (v1) (3.33 KB, patch)
2008-12-08 16:24 EST, David Lawrence
no flags Details | Diff
v3 for security bugs update (5.84 KB, patch)
2008-12-09 01:46 EST, Noura El hawary
dkl: review-
Details | Diff
v4 for adding code to update bug security data (6.72 KB, patch)
2008-12-09 19:20 EST, Noura El hawary
dkl: review+
Details | Diff
Patch to check that the security group is being newly added before updating cc and keywords (1000 bytes, patch)
2008-12-14 22:59 EST, David Lawrence
nelhawar: review+
Details | Diff

  None (edit)
Description Stephen Tweedie 2007-12-03 09:32:17 EST
Description of problem:
I cannot set the "security sensitive" group on an existing RHEL-5 BZ bug,
despite being told by Mark Cox that this is still the preferred way to mark bugs
which requiring security attention.

How reproducible:
100%

Steps to Reproduce:
1. Open bug 406881, while logged onto BZ
  
Actual results:
No sign of the "security sensitive" checkbox.

Expected results:
Should be able to set the security-sensitive box.

Additional info:
Bug is against product RHEL-5, version 5.1.  Creating a new bug _does_ allow me
to set the "security sensitive" group.
Comment 1 Kevin Baker 2008-11-21 12:55:07 EST
Hi,

is this still an issue with bugzilla 3.2? Looking under "Restrict Group Visibility:" -> "Current Groups: (edit)"...clicking the edit link reveals the security group checkbox for me. YMMV
Comment 2 Tomas Hoger 2008-11-21 13:06:12 EST
Kevin, have you checked with BZ account with less privileges?  E.g. with non-RH account, there's no group selection at all.  Given that BZ should not allow anyone with access to the bug to uncheck security group, I think that group checkbox is only visible to members of security and admin BZ groups.
Comment 3 David Lawrence 2008-11-30 18:38:57 EST
This is a bug and should be considered for fixing. Bugzilla is not recognizing that the "Security" group is set to be shown even for non-members in the edit bug template. The new bug template does this properly so we can try to replicate it's behaviour for the fix.
Comment 4 Noura El hawary 2008-12-08 01:41:05 EST
Created attachment 326085 [details]
v1 to enable assigning bug to security group for non security group members

The attached patch will view the security group in the group section of the bug report at the bug edit time even if the user is not a member of that group, and it will enable them to associate a bug to that group, but will not enable them to remove that group membership from a bug, so non members of security group can add bugs to the security group but they can not remove bugs from the security groups.

Noura
Comment 5 Noura El hawary 2008-12-08 01:41:42 EST
please note that the patch is at bz-web2 for your testing.
Comment 6 David Lawrence 2008-12-08 16:18:15 EST
Comment on attachment 326085 [details]
v1 to enable assigning bug to security group for non security group members

I personally think we should keep this feature generic to all 'othercontrol' type groups. Currently upstream code only allows for 'othercontrol' groups to be altered when a product is being changed or a new bug is created.
I think we should allow any of those groups to be checked if they are not currently checked, but then not allowed unchecked except by real members. 

A different issue that this feature needs to address is the we still need to add 'security-responce@redhat.com' and 'Security' keyword if the security group is checked on a current bug. So maybe have that code in Bugzilla::Bug::update() that checks for all groups that are set, then looks at keywords and cc list to make sure the proper data is present.

Dave
Comment 7 David Lawrence 2008-12-08 16:24:45 EST
Created attachment 326193 [details]
Sample patch on allowing othercontrol groups in show_bug.cgi to be set (v1)

Hey Noura. Here is a quick patch using some of what you did except to make it generic to all 'othercontrol' style groups instead of specific to security group. I commented out the part in add_group() that blocks othercontrol group changes except for product changes since we would not be able to do that anymore. The code in remove_group() can stay since we would only allow actual group members to uncheck the othercontrol groups. Also I don't have any code for adding the security cc and keyword data to the bug if the group is checked so that would still need to be added. What do you think or do you think we should just stick to looking for group id 71?

Dave
Comment 8 Noura El hawary 2008-12-09 01:46:51 EST
Created attachment 326272 [details]
v3 for security bugs update

Hey Dave, You patch looks good and works perfect for me, also i have added to it code that will update cc list and keyword with security information when the security group gets selected.

Noura
Comment 9 David Lawrence 2008-12-09 13:15:38 EST
Comment on attachment 326272 [details]
v3 for security bugs update

>Index: Bugzilla/Bug.pm
>+    #if (!Bugzilla->user->in_group($group->name)) {
>+    #    my $controls = $self->product_obj->group_controls->{$group->id};
>+    #    if (!$self->{_old_product_name}
>+    #        || $controls->{othercontrol} == CONTROLMAPNA)
>+    #    {
>+    #        ThrowUserError('group_change_denied',
>+    #                       { bug => $self, group_id => $group->id });
>+    #    }
>+    #}

Need to add "# REDHAT EXTENSION " tags around this with a short explanation
of why we removed the functionality.
 
>Index: process_bug.cgi
>+    # REDHAT EXTENSION START 408671
>+    # Allow extensions to perform security bug data updates
>+    Bugzilla::Hook::process( 'update_security_bug_data',
>+                             { bug => $bug } );
>+    # REDHAT EXTENSION END 408671
>+

Use the current update_bug_data instead of creating a new one. For this we will need to move the current
hook up before the $bug->update($timestamp). For example:

Index: process_bug.cgi
===================================================================
RCS file: /cvs/qa/rh_bugzilla_3/process_bug.cgi,v
retrieving revision 1.29
diff -u -r1.29 process_bug.cgi
--- process_bug.cgi 28 Nov 2008 05:20:18 -0000  1.29
+++ process_bug.cgi 9 Dec 2008 17:25:00 -0000
@@ -586,6 +586,13 @@
     $dbh->bz_start_transaction();

     my $timestamp = $dbh->selectrow_array(q{SELECT NOW()});
+
+    # REDHAT EXTENSION START 406161 408671
+    # Allow extensions to perform data updates
+    Bugzilla::Hook::process( 'update_bug_data',
+        { bug => $bug, timestamp => $timestamp } );
+    # REDHAT EXTENSION END 406161 408671
+
     my $changes = $bug->update($timestamp);

     my %notify_deps;
@@ -631,12 +638,6 @@
     # Set and update flags.
     Bugzilla::Flag->process($bug, undef, $timestamp, $vars);

-    # REDHAT EXTENSION START 406161
-    # Allow extensions to perform data updates
-    Bugzilla::Hook::process( 'update_bug_data',
-        { bug => $bug, timestamp => $timestamp } );
-    # REDHAT EXTENSION END 406161
-
     $dbh->bz_commit_transaction();

Also, I am not sure that we needed to created a new extension directory called 
'bug_security'. I think we can just move extensions/bug_security/code/update_secuity_bug_data.pl
to extensions/redhat_fields/code/update_bug_data.pl instead. I have tested the above change
on bz-web2-test and moving the update_bug_data still allows the other extensions to work
such as external_bugs.

>Index: extensions/bug_security/code/update_security_bug_data.pl
>+foreach my $group (@{$bug->product_obj->groups_valid}) {
>+    my $gid = $group->id;
>+    if ($gid == 71 && 
>+        (defined $cgi->param("bit-$gid") || 
>+         defined $cgi->param("defined_bit-$gid")
>+        ) && 
>+        ($cgi->param("bit-$gid") == 1)){
>+
>+            $bug->add_cc('security-response-team@redhat.com');
>+            $bug->modify_keywords('Security', 'add');
>+
>+        }
>+}

Nit-pick: move the { to the line below in the if {} block and line them up.

    if ($gid == 71 
        && (defined $cgi->param("bit-$gid") || 
            defined $cgi->param("defined_bit-$gid")) 
        && ($cgi->param("bit-$gid") == 1))
    {

        $bug->add_cc('security-response-team@redhat.com');
        $bug->modify_keywords('Security', 'add');

    }
Comment 12 Noura El hawary 2008-12-09 19:20:56 EST
Created attachment 326434 [details]
v4 for adding code to update bug security data

Hey Dave, I have also tried using the same extension to the one we have for external bug and it worked fine. 

Thanks,
Noura
Comment 13 David Lawrence 2008-12-10 12:29:29 EST
Comment on attachment 326434 [details]
v4 for adding code to update bug security data

Looks good Noura.
Comment 14 Noura El hawary 2008-12-10 20:17:46 EST
Thanks for the review Dave, committed to cvs should be live next bugzilla update which happens every Thursday.

Noura
Comment 17 David Lawrence 2008-12-14 22:59:51 EST
Created attachment 326901 [details]
Patch to check that the security group is being newly added before updating cc and keywords

Noura please take a look at this patch that fixes the issue thoger mentioned. This now checks to see that the security checkbox is being added to a bug that it was not previously enabled before adding the cc and keyword. This should allow
the security team to remove themselves from CC if the want.

Thanks
Dave
Comment 18 Noura El hawary 2008-12-14 23:17:44 EST
Comment on attachment 326901 [details]
Patch to check that the security group is being newly added before updating cc and keywords

looks good Dave.

Noura

Note You need to log in before you can comment on or make changes to this bug.