Patch for bug 265898: edit*.cgi files should all use ThrowUserError(); patch by Frédéric Buclin <LpSolit@gmail.com>, r=vladd, a=myk.

12 files changed, 122 insertions, 119 deletions

diff --git a/chart.cgi b/chart.cgi
index 4bab17701..8b0d3971e 100755
--- a/chart.cgi
+++ b/chart.cgi
@@ -84,9 +84,10 @@ if ($action eq "search") {
 
 Bugzilla->login(LOGIN_REQUIRED);
 
-UserInGroup(Param("chartgroup")) 
-    || ThrowUserError("authorization_failure", 
-                     {action => "use this feature"});
+UserInGroup(Param("chartgroup"))
+  || ThrowUserError("auth_failure", {group  => Param("chartgroup"),
+                                     action => "use",
+                                     object => "charts"});
 
 # Only admins may create public queries
 UserInGroup('admin') || $cgi->delete('public');
diff --git a/editclassifications.cgi b/editclassifications.cgi
index c1186f792..777e76f75 100755
--- a/editclassifications.cgi
+++ b/editclassifications.cgi
@@ -78,7 +78,11 @@ Bugzilla->login(LOGIN_REQUIRED);
 
 print $cgi->header();
 
-ThrowUserError("auth_cant_edit_classifications") unless UserInGroup("editclassifications");
+UserInGroup("editclassifications")
+  || ThrowUserError("auth_failure", {group  => "editclassifications",
+                                     action => "edit",
+                                     object => "classifications"});
+
 ThrowUserError("auth_classification_not_enabled") unless Param("useclassification");
 
 #
diff --git a/editcomponents.cgi b/editcomponents.cgi
index e2e471f34..7c1ac96ca 100755
--- a/editcomponents.cgi
+++ b/editcomponents.cgi
@@ -117,11 +117,10 @@ Bugzilla->login(LOGIN_REQUIRED);
 
 print Bugzilla->cgi->header();
 
-unless (UserInGroup("editcomponents")) {
-    ThrowUserError('auth_cant_edit_components');    
-    exit;
-}
-
+UserInGroup("editcomponents")
+  || ThrowUserError("auth_failure", {group  => "editcomponents",
+                                     action => "edit",
+                                     object => "components"});
 
 #
 # often used variables
diff --git a/editflagtypes.cgi b/editflagtypes.cgi
index 4875b4f19..48074863a 100755
--- a/editflagtypes.cgi
+++ b/editflagtypes.cgi
@@ -42,8 +42,9 @@ use vars qw( $template $vars );
 # Make sure the user is logged in and is an administrator.
 Bugzilla->login(LOGIN_REQUIRED);
 UserInGroup("editcomponents")
-  || ThrowUserError("authorization_failure", 
-                    { action => "administer flag types" });
+  || ThrowUserError("auth_failure", {group  => "editcomponents",
+                                     action => "edit",
+                                     object => "flagtypes"});
 
 # Suppress "used only once" warnings.
 use vars qw(@legal_product @legal_components %components);
diff --git a/editgroups.cgi b/editgroups.cgi
index bc22d518e..3eca512f9 100755
--- a/editgroups.cgi
+++ b/editgroups.cgi
@@ -40,7 +40,10 @@ Bugzilla->login(LOGIN_REQUIRED);
 
 print Bugzilla->cgi->header();
 
-ThrowUserError("auth_cant_edit_groups") unless UserInGroup("creategroups");
+UserInGroup("creategroups")
+  || ThrowUserError("auth_failure", {group  => "creategroups",
+                                     action => "edit",
+                                     object => "groups"});
 
 my $action = trim($cgi->param('action') || '');
 
diff --git a/editkeywords.cgi b/editkeywords.cgi
index d46476dfa..9c835e8b7 100755
--- a/editkeywords.cgi
+++ b/editkeywords.cgi
@@ -58,11 +58,10 @@ Bugzilla->login(LOGIN_REQUIRED);
 
 print Bugzilla->cgi->header();
 
-unless (UserInGroup("editkeywords")) {
-    ThrowUserError("keyword_access_denied");
-    exit;
-}
-
+UserInGroup("editkeywords")
+  || ThrowUserError("auth_failure", {group  => "editkeywords",
+                                     action => "edit",
+                                     object => "keywords"});
 
 my $action  = trim($cgi->param('action')  || '');
 $vars->{'action'} = $action;
diff --git a/editmilestones.cgi b/editmilestones.cgi
index ad07b2d61..160385104 100755
--- a/editmilestones.cgi
+++ b/editmilestones.cgi
@@ -125,11 +125,10 @@ Bugzilla->login(LOGIN_REQUIRED);
 
 print Bugzilla->cgi->header();
 
-unless (UserInGroup("editcomponents")) {
-    ThrowUserError('auth_cant_edit_milestones');    
-    exit;
-}
-
+UserInGroup("editcomponents")
+  || ThrowUserError("auth_failure", {group  => "editcomponents",
+                                     action => "edit",
+                                     object => "milestones"});
 
 #
 # often used variables
diff --git a/editparams.cgi b/editparams.cgi
index 8ffd76a08..5d7ff9178 100755
--- a/editparams.cgi
+++ b/editparams.cgi
@@ -34,14 +34,10 @@ Bugzilla->login(LOGIN_REQUIRED);
 
 print Bugzilla->cgi->header();
 
-if (!UserInGroup("tweakparams")) {
-    print "<H1>Sorry, you aren't a member of the 'tweakparams' group.</H1>\n";
-    print "And so, you aren't allowed to edit the parameters.\n";
-    PutFooter();
-    exit;
-}
-
-
+UserInGroup("tweakparams")
+  || ThrowUserError("auth_failure", {group  => "tweakparams",
+                                     action => "modify",
+                                     object => "parameters"});
 
 PutHeader("Edit parameters");
 
diff --git a/editproducts.cgi b/editproducts.cgi
index e29fd975d..8fe1a5ec5 100755
--- a/editproducts.cgi
+++ b/editproducts.cgi
@@ -244,15 +244,10 @@ Bugzilla->login(LOGIN_REQUIRED);
 
 print Bugzilla->cgi->header();
 
-unless (UserInGroup("editcomponents")) {
-    PutHeader("Not allowed");
-    print "Sorry, you aren't a member of the 'editcomponents' group.\n";
-    print "And so, you aren't allowed to add, modify or delete products.\n";
-    PutTrailer();
-    exit;
-}
-
-
+UserInGroup("editcomponents")
+  || ThrowUserError("auth_failure", {group  => "editcomponents",
+                                     action => "edit",
+                                     object => "products"});
 
 #
 # often used variables
diff --git a/editusers.cgi b/editusers.cgi
index 7f3eef3a9..a70e3fcf2 100755
--- a/editusers.cgi
+++ b/editusers.cgi
@@ -243,17 +243,12 @@ print Bugzilla->cgi->header();
 
 $editall = UserInGroup("editusers");
 
-if (!$editall) {
-    if (!Bugzilla->user->can_bless) {
-        PutHeader("Not allowed");
-        print "Sorry, you aren't a member of the 'editusers' group, and you\n";
-        print "don't have permissions to put people in or out of any group.\n";
-        print "And so, you aren't allowed to add, modify or delete users.\n";
-        PutTrailer();
-        exit;
-    }
-}
-
+$editall
+  || Bugzilla->user->can_bless
+  || ThrowUserError("auth_failure", {group  => "editusers",
+                                     reason => "cant_bless",
+                                     action => "edit",
+                                     object => "users"});
 
 
 #
@@ -385,13 +380,10 @@ if ($action eq 'list') {
 #
 
 if ($action eq 'add') {
+    $editall || ThrowUserError("auth_failure", {group  => "editusers",
+                                                action => "add",
+                                                object => "users"});
     PutHeader("Add user");
-    if (!$editall) {
-        print "Sorry, you don't have permissions to add new users.";
-        PutTrailer();
-        exit;
-    }
-
     print "<FORM METHOD=POST ACTION=editusers.cgi>\n";
     print "<TABLE BORDER=0 CELLPADDING=4 CELLSPACING=0><TR>\n";
 
@@ -415,13 +407,9 @@ if ($action eq 'add') {
 #
 
 if ($action eq 'new') {
-    PutHeader("Adding new user");
-
-    if (!$editall) {
-        print "Sorry, you don't have permissions to add new users.";
-        PutTrailer();
-        exit;
-    }
+    $editall || ThrowUserError("auth_failure", {group  => "editusers",
+                                                action => "add",
+                                                object => "users"});
 
     # Cleanups and valididy checks
     my $realname = trim($::FORM{realname} || '');
@@ -432,6 +420,7 @@ if ($action eq 'new') {
     my $disabledtext = trim($::FORM{disabledtext} || '');
     my $emailregexp = Param("emailregexp");
 
+    PutHeader("Adding new user");
     unless ($user) {
         print "You must enter a name for the new user. Please press\n";
         print "<b>Back</b> and try again.\n";
@@ -494,17 +483,10 @@ if ($action eq 'new') {
 #
 
 if ($action eq 'del') {
-    PutHeader("Delete user $user");
-    if (!$candelete) {
-        print "Sorry, deleting users isn't allowed.";
-        PutTrailer();
-        exit;
-    }
-    if (!$editall) {
-        print "Sorry, you don't have permissions to delete users.";
-        PutTrailer();
-        exit;
-    }
+    $candelete || ThrowUserError("users_deletion_disabled");
+    $editall || ThrowUserError("auth_failure", {group  => "editusers",
+                                                action => "delete",
+                                                object => "users"});
     CheckUser($user);
 
     # display some data about the user
@@ -514,6 +496,7 @@ if ($action eq 'del') {
       FetchSQLData();
     $realname = ($realname ? html_quote($realname) : "<FONT COLOR=\"red\">missing</FONT>");
     
+    PutHeader("Delete user $user");
     print "<TABLE BORDER=1 CELLPADDING=4 CELLSPACING=0>\n";
     print "<TR BGCOLOR=\"#6666FF\">\n";
     print "  <TH VALIGN=\"top\" ALIGN=\"left\">Part</TH>\n";
@@ -628,17 +611,10 @@ if ($action eq 'del') {
 #
 
 if ($action eq 'delete') {
-    PutHeader("Deleting user");
-    if (!$candelete) {
-        print "Sorry, deleting users isn't allowed.";
-        PutTrailer();
-        exit;
-    }
-    if (!$editall) {
-        print "Sorry, you don't have permissions to delete users.";
-        PutTrailer();
-        exit;
-    }
+    $candelete || ThrowUserError("users_deletion_disabled");
+    $editall || ThrowUserError("auth_failure", {group  => "editusers",
+                                                action => "delete",
+                                                object => "users"});
     CheckUser($user);
 
     SendSQL("SELECT userid
@@ -651,8 +627,9 @@ if ($action eq 'delete') {
              WHERE login_name=" . SqlQuote($user));
     SendSQL("DELETE FROM user_group_map
              WHERE user_id=" . $userid);
-    print "User deleted.<BR>\n";
 
+    PutHeader("Deleting user");
+    print "User deleted.<BR>\n";
     PutTrailer($localtrailer);
     exit;
 }
diff --git a/editwhines.cgi b/editwhines.cgi
index 5610f7eaa..8c7c269e8 100755
--- a/editwhines.cgi
+++ b/editwhines.cgi
@@ -72,7 +72,10 @@ my $sth; # database statement handle
 my $events = get_events($userid);
 
 # First see if this user may use whines
-ThrowUserError('whine_access_denied') unless (UserInGroup('bz_canusewhines'));
+UserInGroup("bz_canusewhines")
+  || ThrowUserError("auth_failure", {group  => "bz_canusewhines",
+                                     action => "schedule",
+                                     object => "reports"});
 
 # May this user send mail to other users?
 my $can_mail_others = UserInGroup('bz_canusewhineatothers');
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index d2852e80c..aa4a43286 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -17,6 +17,7 @@
   # Rights Reserved.
   #
   # Contributor(s): Gervase Markham <gerv@gerv.net>
+  #                 Frédéric Buclin <LpSolit@netscape.net>
   #%]
 
 [%# INTERFACE:
@@ -96,14 +97,60 @@
     account creation. Please contact an administrator to get a new account
     created.
 
-  [% ELSIF error == "auth_cant_edit_groups" %]
-    [% title = "Not authorized to edit groups" %]
-    Sorry, you aren't a member of the 'creategroups' group. And so,
-    you aren't allowed to edit the groups.
+  [% ELSIF error == "auth_failure" %]
+    [% title = "Authorization Required" %]
+    Sorry,
+    [% IF group %]
+      you aren't a member of the '[% group FILTER html %]' group,
+    [% END %]
+
+    [% IF reason %]
+      [% IF group %] and [% END %]
+      [% IF reason == "cant_bless" %]
+        you don't have permissions to put people in or out of any group,
+      [% END %]
+    [% END %]
+
+    and so you aren't allowed to
+    [% IF action == "add" %]
+      add new
+    [% ELSIF action == "modify" %]
+      modify
+    [% ELSIF action == "delete" %]
+      delete
+    [% ELSIF action == "edit" %]
+      add, modify or delete
+    [% ELSIF action == "schedule" %]
+      schedule
+    [% ELSIF action == "use" %]
+      use
+    [% END %]
 
-  [% ELSIF error == "authorization_failure" %]
-    [% title = "Authorization Failed" %]
-    You are not allowed to [% action FILTER html %].
+    [% IF object == "charts" %]
+      the "New Charts" feature
+    [% ELSIF object == "classifications" %]
+      classifications
+    [% ELSIF object == "components" %]
+      components
+    [% ELSIF object == "flagtypes" %]
+      flag types
+    [% ELSIF object == "groups" %]
+      groups
+    [% ELSIF object == "keywords" %]
+      keywords
+    [% ELSIF object == "milestones" %]
+      milestones
+    [% ELSIF object == "parameters" %]
+      parameters
+    [% ELSIF object == "products" %]
+      products
+    [% ELSIF object == "reports" %]
+      whine reports
+    [% ELSIF object == "users" %]
+      users
+    [% ELSIF object == "versions" %]
+      versions
+    [% END %].
 
   [% ELSIF error == "attachment_access_denied" %]
     [% title = "Access Denied" %]
@@ -146,11 +193,6 @@
     [% title = "Classification Not Enabled" %]
     Sorry, classification is not enabled.
 
-  [% ELSIF error == "auth_cant_edit_classifications" %]
-    [% title = "Access Denied" %]
-    Sorry, you aren't a member of the 'editclassifications' group, and so
-    you aren't allowed to add, modify or delete classifications.
-
   [% ELSIF error == "classification_not_specified" %]
     [% title = "You Must Supply A Classification Name" %]
     You must enter a classification name.
@@ -176,16 +218,6 @@
      Sorry, but you can not delete the default classification,
      '[% name FILTER html %]'.
 
-  [% ELSIF error == "auth_cant_edit_components" %]
-    [% title = "Access Denied" %]
-    Sorry, you aren't a member of the 'editcomponents' group, and so
-    you aren't allowed to add, modify or delete components.
-
-  [% ELSIF error == "auth_cant_edit_milestones" %]
-    [% title = "Access Denied" %]
-    Sorry, you aren't a member of the 'editcomponents' group, and so
-    you aren't allowed to add, modify or delete milestones.
-
   [% ELSIF error == "auth_cant_edit_versions" %]
     [% title = "Access Denied" %]
     Sorry, you aren't a member of the 'editcomponents' group, and so
@@ -555,11 +587,6 @@
     [% title = "Invalid Username Or Password" %]
     The username or password you entered is not valid.
 
-  [% ELSIF error == "keyword_access_denied" %]
-    [% title = "Access Denied" %]
-    Sorry, you aren't a member of the 'editkeywords' group, and so
-    you aren't allowed to add, modify or delete keywords.
-
   [% ELSIF error == "keyword_already_exists" %]
     [% title = "Keyword Already Exists" %]
     A keyword with the name [% name FILTER html %] already exists.
@@ -958,6 +985,10 @@
     The version '[% version FILTER html %]' for product 
     '[% product FILTER html %]' does not exist.
 
+  [% ELSIF error == "users_deletion_disabled" %]
+    [% title = "Deletion not activated" %]
+    Sorry, the deletion of user accounts is not allowed.
+
   [% ELSIF error == "votes_must_be_nonnegative" %]
     [% title = "Votes Must Be Non-negative" %]
     Only use non-negative numbers for your [% terms.bug %] votes.
@@ -979,11 +1010,6 @@
     Value is out of range for field 
     <em>[% field_descs.$field FILTER html %]</em>.
 
-  [% ELSIF error == "whine_access_denied" %]
-    [% title = "Access Denied" %]
-    Sorry, you aren't a member of the 'bz_canusewhines' group, and so
-    you aren't allowed to schedule whine reports.
-
   [% ELSIF error == "zero_length_file" %]
     [% title = "File Is Empty" %]
     The file you are trying to attach is empty!