313547 - When strict_isolation is set, don't allow inappropriate users on new bugs

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, defect)

Description

[Joel Peshkin]

Strict isolation enforces rules on who can be assigned or cc'd on existing bugs.  post_bug.cgi must do the same.

Comment 1

[Joel Peshkin]

Attachment: The same check as is done by process_bug

Comment 2

[Frédéric Buclin]

Comment on attachment 200570 [details] [diff] [review]
The same check as is done by process_bug

>Index: post_bug.cgi

>+if (Param("strict_isolation")) {
>+    my @roles = ('assigned_to');
>+    if (Param("useqacontact")) {
>+        push @roles, 'qa_contact';
>+    }
>+    foreach my $role (@roles) {
>+        my $role_user = Bugzilla::User->new($cgi->param($role));
>+        if (!$role_user->can_edit_product($product_id)) {
>+            ThrowUserError("invalid_user_group",

If you enter a new bug in a component which has no default QA contact, you get an error! You should only consider the QA contact if both Param('useqacontact') is on *and* a QA contact has been given.


>+        if (Param("strict_isolation")) {
>+            my $cc_user = Bugzilla::User->new($ccid);
>+            if (!$cc_user->can_edit_product($product_id)) {
>+                push (@blocked_cc, $person);

Nit: I prefer $cc_user->login than $person.


>+    my $blocked_cc = join(", ", @blocked_cc);
>+    ThrowUserError("invalid_user_group",
>+        {'user' => $blocked_cc});

The error message doesn't expect to receive a list of user:

>     User '[% user FILTER html %]' is not able to edit the

Comment 3

[Frédéric Buclin]

Looks like this bug needs to be fixed for 2.21.2. This is the last piece of the strict_isolation stuff.

Comment 4

[Marc Schumann [:Wurblzap]]

This should be part of the strict_isolation package when it is released for the first time, I'd say.

Comment 5

[Max Kanat-Alexander]

Yeah, we can't release with this bug in existence.

Comment 6

[Joel Peshkin]

Attachment: Fixed and clean version

Comment 7

[Teemu Mannermaa (:wicked)]

Comment on attachment 206264 [details] [diff] [review]
Fixed and clean version

>Index: post_bug.cgi
>===================================================================
>+    $related_users{$cgi->param('qa_contact')} = 1 if Param('useqacontact');

This still doesn't work when no QA contact is specified for the product. A blank user is added to the hash and later checked, which naturally ends up in an error message stating "User '' is not able to edit the 'TestProduct' Product .".

>Index: template/en/default/global/user-error.html.tmpl
>===================================================================
>     [%+ field_descs.product FILTER html %]

Nit: This adds a word product with capital P. True, your patch didn't add this line but this makes the error look awfull. :(

>+    [% ELSIF new %]
>+      .

Nit: This leaves a blank before the dot.

Comment 8

[Frédéric Buclin]

joel, have you time to update your patch this week?

Comment 9

[Joel Peshkin]

Attachment: v3

Comment 10

[Frédéric Buclin]

Comment on attachment 207211 [details] [diff] [review]
v3

>+    if (Param('useqacontact') && (trim($cgi->param('qa_contact')) ne '')) {
>+        $related_users{$cgi->param('qa_contact')} = 1;

$cgi->param('qa_contact') is an integer (it has been converted from a login name to a user ID earlier), so trim() doesn't make sense and comparing it to '' even less. You should only check that $cgi->param('qa_contact') is given:

if (Param('useqacontact') && $cgi->param('qa_contact'))


>+        if (!$related_user->can_edit_product($cgi->param('product_id'))) {

Nit: you could write $product_id instead of $cgi->param('product_id').


Fix at least the first point on checkin. And why not the second one too. ;) Works fine. r=LpSolit

Comment 11

[Joel Peshkin]

Checked in with LpSolit's nits fixed

Checking in post_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/post_bug.cgi,v  <--  post_bug.cgi
new revision: 1.134; previous revision: 1.133
done
Checking in template/en/default/global/user-error.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/user-error.html.tmpl,v  <--  user-error.html.tmpl
new revision: 1.145; previous revision: 1.144
done