Bug 443570

I did the original conversion of the cookies code when we did the PostgreSQL to
MySQL conversion last august. I can take this task if noone objects.

Dave

Hi Dave,
     Thanks a lot. :)
     
    "The hwcert cannot save the cookies in the session cycle of browser on the
new schema" is not caused by "Added the new field ipaddr".
I have spent one night for debug the cookies part codes, and found out the
reason is that the field "logincookies.cookie"'s Type is "varchar(16)" in the
new 3.2 schema while  the field "logincookies.cookie"'s Type is "varchar(40)" in
the old 2.18 schema. In the 2.18's cookie codes,the token is a string which have
40 chars and are generate by func:generate_unique_id (), it is like
("4811fe1c7da89ad71b1bd0799580678352a75a42"). When we turn to the new 3.2 schema
with the 2.18's cookies codes, the 40 char string will be cut to 16 chars and
save into db logincookies.cookie. it is like ("4811fe1c7da89ad7").So they
doesn't match and the authenticate will fail.

Best Regards!
Nicho

Created attachment 303544 [details]
Patch to convert auth system to use 3.2 style cookies (v1)

Attaching a patch that allows hwcert code to use 3.2 style cookies.

Basically I took the older stock 2.18 Bugzilla/Auth.pm,
Bugzilla/Auth/Cookie.pm, and Auth/CGI.pm and put them in place of the current
modules. I then converted the code to use the
Bugzilla::Token::GenerateUniqueToken to create a key that is used in
logincookies.cookie. Also I added code in Bugzilla/Auth.pm that checks for
X_FORWARDED_FOR which allows us to see the clients real ipaddress for
logincookies.ipaddr. I have test this on bugdev and login seems to work
properly. I did have to drop the code in Bugzilla/User.pm that looks for
profiles.refreshed_when in order to test my changes. I think that column is
being worked on in a separate bug.

Please review
Dave

Created attachment 303648 [details]
Patch to convert auth system to use 3.2 style cookies (v2)

New patch that fixes the issue where logincookie entries were not being deleted
properly on logout. Also fixed a few other errors with removing cookies, etc.

Please review.
Dave

 This patch works well for me. thanks.

Created attachment 303787 [details]
For patching operation

< Index: Bugzilla/User.pm
< ===================================================================
< RCS file: /cvs/qa/hwcert/Bugzilla/User.pm,v
< retrieving revision 1.1.1.1
< diff -u -r1.1.1.1 User.pm
< --- Bugzilla/User.pm	16 Nov 2004 16:42:36 -0000	1.1.1.1
< +++ Bugzilla/User.pm	24 Apr 2008 15:31:37 -0000
< @@ -100,27 +100,6 @@
<
<      bless ($self, $class);
<
< -    # Now update any old group information if needed
< -    my $result = $dbh->selectrow_array(q{SELECT 1
< -					      FROM profiles, groups
< -					     WHERE userid=?
< -					       AND profiles.refreshed_when <=
< -						     groups.last_changed},
< -					  undef,
< -					  $id);
< -
< -    if ($result) {
< -	   my $is_main_db;
< -	   unless ($is_main_db = Bugzilla->dbwritesallowed()) {
< -	       Bugzilla->switch_to_main_db();
< -	   }
< -	   # FIXME Throws error due to unique constraint differences in current
db
< -	   #$self->derive_groups($tables_locked_for_derive_groups);
< -	   unless ($is_main_db) {
< -	       Bugzilla->switch_to_shadow_db();
< -	   }
< -    }
< -
<      return $self;
<  }
<
< @@ -321,14 +300,7 @@
<	       }
<	   }
<      }
< -
< -    $dbh->do(q{UPDATE profiles
< -		     SET refreshed_when = ?
< -		   WHERE userid=?},
< -		undef,
< -		$time,
< -		$id);
< -
< +
<      Bugzilla::DB::UnlockTables() unless $already_locked;
<  }


As all the bug's patch will be patched into one code tree.So above codes have
been moved to bug 443561 and bug 443566's patch.

Best Regards!
Nicho

Created attachment 310913 [details]
patch merge change.

Because previous patch (303787) is diffed before adding extra log for cookies
issue into our catalog. So after our pushing that into live site. we need to do
this patch again.  I have tested this patch well. and deploy all the patches of
"the match bz 3.2 db series bugs/feature" on the site:
http://bugdev.devel.redhat.com/hwcert-xisun2/ and test well.

Best Regards!
Nicho

Nicho, so is this patch moved to the other bugs or only updated here or...?

Dear Rob,
    This patch(310913) is only updated here. because the original patch (303787)
was worked out and have passed the code review by dkl before adding extra log
for cookies issue #bug431701, after we have fixed #bug431701 and check the
change into cvs and push into live site, we need update the orignal patch
(303787), So this patch(310913) is the update version for this bug/match 3.2
db's logincookies tables' change.

Hope nicho has explained that clearly :)

Best Regards
nicho

are both patches needed or should "Patch to convert auth system to use 3.2 style
cookies (v2)" be obsoleted?

np, can you have eng-ops review the correct patch then, and if accepted and
tested well commit it?

The patch's review was passed by Noura, and she advice me to ask Dave to
double-check this patch.

dkl,reference
    Pls help me to review the patch (attach_id: 310913), and Comment #9
explained the reason why we redo this as a new patch (310913) and need u to help
to review it again.

The test instance is http://bugdev.devel.redhat.com/hwcert-xisun2/ . 

Thank you very much in advance. 

Nicho.

Checked into cvs.

tested and run good