Fix for bug 236678: Clean up access to COOKIE global. Murder the last

remaining places in the tree where COOKIE is used; includes a rather
thorough cleanup of Bugzilla::Bug->user and a minor doc update. r=joel, a=justdave.

diff --git a/Bugzilla.pm b/Bugzilla.pm
index 0818fb1d5665feac3dc1f90e1eb2e80d65bb96d5..56a8d615cc91d7192966bcaabc5b0d0d7c18ab75 100644 (file)
--- a/Bugzilla.pm
+++ b/Bugzilla.pm
@@ -87,13 +87,10 @@ sub logout_user_by_id {
 # hack that invalidates credentials for a single request
 sub logout_request {
     undef $_user;
+    # XXX clean this up eventually
     $::userid = 0;
-    # XXX clean these up eventually
-    delete $::COOKIE{"Bugzilla_login"};
-    # NB - Can't delete from $cgi->cookie, so the logincookie data will
-    # remain there; it's only used in Bugzilla::Auth::CGI->logout anyway
-    # People shouldn't rely on the cookie param for the username
-    # - use Bugzilla->user instead!
+    # We can't delete from $cgi->cookie, so logincookie data will remain
+    # there. Don't rely on it: use Bugzilla->user->login instead!
 }
 
 my $_dbh;

diff --git a/Bugzilla/Auth/README b/Bugzilla/Auth/README
index c765d4971dc65ef8ff97d86ba773e06bed7c3069..e573e2c0b156ae969d5f62a906bd00d0951b5fd3 100644 (file)
--- a/Bugzilla/Auth/README
+++ b/Bugzilla/Auth/README
@@ -130,9 +130,3 @@ Bugzilla.pm
     a reorganization which moves CGI-specific code to a subdirectory,
     Bugzilla.pm should go with it.
 
-$::COOKIE
-
-    There are still instances of use of $::COOKIE to obtain Logincookie
-    information; these should be removed as well.
-
-

diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index fa759ddb7cf7e86b4c39cea4bb6144d3b49895c9..53b8bd19357212b36243d533a3c88bebdde1676b 100755 (executable)
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -387,32 +387,32 @@ sub user {
     my $self = shift;
     return $self->{'user'} if exists $self->{'user'};
 
-    $self->{'user'} = {};
-
-    my $movers = Param("movers");
-    $movers =~ s/\s?,\s?/|/g;
-    $movers =~ s/@/\@/g;
-    $self->{'user'}->{'canmove'} = Param("move-enabled") 
-      && (defined $::COOKIE{"Bugzilla_login"}) 
-        && ($::COOKIE{"Bugzilla_login"} =~ /$movers/);
-
-    # In the below, if the person hasn't logged in ($::userid == 0), then
-    # we treat them as if they can do anything.  That's because we don't
-    # know why they haven't logged in; it may just be because they don't
-    # use cookies.  Display everything as if they have all the permissions
-    # in the world; their permissions will get checked when they log in
-    # and actually try to make the change.
-    $self->{'user'}->{'canedit'} = $::userid == 0
-                                   || $::userid == $self->{'reporter'}{'id'}
-                                   || (Param('useqacontact') && $self->{'qa_contact'} && $::userid == $self->{'qa_contact'}{'id'})
-                                   || $::userid == $self->{'assigned_to'}{'id'}
-                                   || &::UserInGroup("editbugs");
-    $self->{'user'}->{'canconfirm'} = $::userid == 0
-                                   || ($self->{'qa_contact'} && $::userid == $self->{'qa_contact'}{'id'})
-                                   || $::userid == $self->{'assigned_to'}{'id'}
-                                   || &::UserInGroup("editbugs")
-                                   || &::UserInGroup("canconfirm");
-
+    use Bugzilla;
+
+    my @movers = map { trim $_ } split(",", Param("movers"));
+    my $canmove = Param("move-enabled") && Bugzilla->user && 
+                  (lsearch(\@movers, Bugzilla->user->login) != -1);
+
+    # In the below, if the person hasn't logged in, then we treat them
+    # as if they can do anything.  That's because we don't know why they
+    # haven't logged in; it may just be because they don't use cookies.
+    # Display everything as if they have all the permissions in the
+    # world; their permissions will get checked when they log in and
+    # actually try to make the change.
+    my $privileged = (!Bugzilla->user)
+                     || Bugzilla->user->in_group("editbugs")
+                     || Bugzilla->user->id == $self->{'assigned_to'}{'id'}
+                     || (Param('useqacontact') && $self->{'qa_contact'} &&
+                         Bugzilla->user->id == $self->{'qa_contact'}{'id'});
+    my $isreporter = Bugzilla->user && 
+                     Bugzilla->user->id == $self->{'reporter'}{'id'};
+
+    my $canedit = $privileged || $isreporter;
+    my $canconfirm = $privileged || Bugzilla->user->in_group("canconfirm");
+
+    $self->{'user'} = {canmove    => $canmove, 
+                       canconfirm => $canconfirm, 
+                       canedit    => $canedit,};
     return $self->{'user'};
 }
 

diff --git a/CGI.pl b/CGI.pl
index 94e727f0896a742c661054e2c65ea0a2c735e9b7..bb271cf9490165092ff775e3af40c969cd1c447f 100644 (file)
--- a/CGI.pl
+++ b/CGI.pl
@@ -421,10 +421,6 @@ foreach my $name ($::cgi->param()) {
 
 $::buffer = $::cgi->query_string();
 
-foreach my $name ($::cgi->cookie()) {
-    $::COOKIE{$name} = $::cgi->cookie($name);
-}
-
 # This could be needed in any CGI, so we set it here.
 $vars->{'help'} = $::cgi->param('help') ? 1 : 0;