From 74d3de5bedbc82cc640a71cf20f67a6a0ef02664 Mon Sep 17 00:00:00 2001 From: Robin Sheat Date: Tue, 14 Jun 2011 19:24:22 +1200 Subject: [PATCH] Bug 6296: allow users to be authenticated by SSL client certs Content-Type: text/plain; charset="UTF-8" This adds a new syspref: AllowPKIAuth. It can have one of three states: * None * Common Name * emailAddress If a) this is set to something that's not "None", and b) the webserver is passing SSL client cert details on to Koha, then the relevant field in the user's certificate will be matched up against the field in the database and they will be automatically logged in. This is used as a secure form of single sign-on in some organisations. The "Common Name" field is matched up against the userid, while "emailAddress" is matched against the primary email. This is an example of what might go in the Apache configuration for the virtual host: SSLVerifyClient require SSLVerifyDepth 2 SSLCACertificateFile /etc/apache2/ssl/test/ca.crt SSLOptions +StdEnvVars The last line ensures that the required details are passed to Koha. Conflicts: installer/data/mysql/sysprefs.sql installer/data/mysql/updatedatabase.pl --- C4/Auth.pm | 283 +++++++++++--------- C4/Members.pm | 32 +++- acqui/finishreceive.pl | 4 +- catalogue/updateitem.pl | 4 +- etc/koha-httpd.conf | 16 ++ installer/data/mysql/sysprefs.sql | 1 + installer/data/mysql/updatedatabase.pl | 9 + .../prog/en/modules/admin/preferences/admin.pref | 11 +- members/setstatus.pl | 2 +- reserve/placerequest.pl | 2 +- serials/reorder_members.pl | 3 +- serials/subscription-detail.pl | 3 +- 12 files changed, 239 insertions(+), 131 deletions(-) diff --git a/C4/Auth.pm b/C4/Auth.pm index 38bbd5e..da2017e 100644 --- a/C4/Auth.pm +++ b/C4/Auth.pm @@ -145,7 +145,22 @@ sub get_template_and_user { my $borrowernumber; my $insecure = C4::Context->preference('insecure'); - if ($user or $insecure) { + if ( $user or $insecure ) { + + # It's possible for $user to be the borrowernumber if they don't have a + # userid defined (and are logging in through some other method, such as + # SSL certs against an email address) + $borrowernumber = getborrowernumber($user) if defined($user); + if ( !defined($borrowernumber) && defined($user) ) { + my $borrower = GetMember( borrowernumber => $user ); + if ($borrower) { + $borrowernumber = $user; + + # A bit of a hack, but I don't know there's a nicer way to do + # it. + $user = $borrower->{firstname} . ' ' . $borrower->{surname}; + } + } # user info $template->param( loggedinusername => $user ); @@ -717,9 +732,9 @@ sub checkauth { } unless ($userid || $sessionID) { #we initiate a session prior to checking for a username to allow for anonymous sessions... - my $session = get_session("") or die "Auth ERROR: Cannot get_session()"; + my $session = get_session("") or die "Auth ERROR: Cannot get_session()"; my $sessionID = $session->id; - C4::Context->_new_userenv($sessionID); + C4::Context->_new_userenv($sessionID); $cookie = $query->cookie(CGISESSID => $sessionID); $userid = $query->param('userid'); if (($cas && $query->param('ticket')) || $userid) { @@ -730,7 +745,29 @@ sub checkauth { ( $return, $cardnumber, $retuserid ) = checkpw( $dbh, $userid, $password, $query ); $userid = $retuserid; $info{'invalidCasLogin'} = 1 unless ($return); - } else { + } elsif (($pki_field eq 'Common Name' && $ENV{'SSL_CLIENT_S_DN_CN'}) || + ($pki_field eq 'emailAddress' && $ENV{'SSL_CLIENT_S_DN_Email'})) { + my $value; + if ($pki_field eq 'Common Name') { + $value = $ENV{'SSL_CLIENT_S_DN_CN'}; + } elsif ($pki_field eq 'emailAddress') { + $value = $ENV{'SSL_CLIENT_S_DN_Email'}; + # If we're looking up the email, there's a chance that the person + # doesn't have a userid. So if there is none, we pass along the + # borrower number, and the bits of code that need to know the user + # ID will have to be smart enough to handle that. + my @users_info = GetBorrowersWithEmail($value); + if (@users_info) { + # First the userid, then the borrowernum + $value = $users_info[0][1] || $users_info[0][0]; + } else { + undef $value; + } + } + # 0 for no user, 1 for normal, 2 for demo user. + $return = $value ? 1 : 0; + $userid = $value; + } else { my $retuserid; ( $return, $cardnumber, $retuserid ) = checkpw( $dbh, $userid, $password, $query ); $userid = $retuserid if ($retuserid ne ''); @@ -745,126 +782,132 @@ sub checkauth { C4::Context->_unset_userenv($sessionID); } - my ($borrowernumber, $firstname, $surname, $userflags, - $branchcode, $branchname, $branchprinter, $emailaddress); - - if ( $return == 1 ) { - my $select = " - SELECT borrowernumber, firstname, surname, flags, borrowers.branchcode, - branches.branchname as branchname, - branches.branchprinter as branchprinter, - email - FROM borrowers - LEFT JOIN branches on borrowers.branchcode=branches.branchcode - "; - my $sth = $dbh->prepare("$select where userid=?"); - $sth->execute($userid); - unless ($sth->rows) { - $debug and print STDERR "AUTH_1: no rows for userid='$userid'\n"; - $sth = $dbh->prepare("$select where cardnumber=?"); - $sth->execute($cardnumber); - - unless ($sth->rows) { - $debug and print STDERR "AUTH_2a: no rows for cardnumber='$cardnumber'\n"; - $sth->execute($userid); - unless ($sth->rows) { - $debug and print STDERR "AUTH_2b: no rows for userid='$userid' AS cardnumber\n"; - } - } - } - if ($sth->rows) { - ($borrowernumber, $firstname, $surname, $userflags, - $branchcode, $branchname, $branchprinter, $emailaddress) = $sth->fetchrow; - $debug and print STDERR "AUTH_3 results: " . - "$cardnumber,$borrowernumber,$userid,$firstname,$surname,$userflags,$branchcode,$emailaddress\n"; - } else { - print STDERR "AUTH_3: no results for userid='$userid', cardnumber='$cardnumber'.\n"; - } + my ($borrowernumber, $firstname, $surname, $userflags, + $branchcode, $branchname, $branchprinter, $emailaddress); + + if ( $return == 1 ) { + my $select = " + SELECT borrowernumber, firstname, surname, flags, borrowers.branchcode, + branches.branchname as branchname, + branches.branchprinter as branchprinter, + email + FROM borrowers + LEFT JOIN branches on borrowers.branchcode=branches.branchcode + "; + my $sth = $dbh->prepare("$select where userid=?"); + $sth->execute($userid); + unless ( $sth->rows ) { + $debug + and print STDERR + "AUTH_1: no rows for userid='$userid'\n"; + $sth = $dbh->prepare("$select where cardnumber=?"); + $sth->execute($cardnumber); + + unless ( $sth->rows ) { + $debug + and print STDERR + "AUTH_2a: no rows for cardnumber='$cardnumber'\n"; + $sth->execute($userid); + unless ( $sth->rows ) { + $debug + and print STDERR +"AUTH_2b: no rows for userid='$userid' AS cardnumber\n"; + } + } + } + if ($sth->rows) { + ($borrowernumber, $firstname, $surname, $userflags, + $branchcode, $branchname, $branchprinter, $emailaddress) = $sth->fetchrow; + $debug and print STDERR "AUTH_3 results: " . + "$cardnumber,$borrowernumber,$userid,$firstname,$surname,$userflags,$branchcode,$emailaddress\n"; + } else { + print STDERR "AUTH_3: no results for userid='$userid', cardnumber='$cardnumber'.\n"; + } # launch a sequence to check if we have a ip for the branch, i # if we have one we replace the branchcode of the userenv by the branch bound in the ip. - my $ip = $ENV{'REMOTE_ADDR'}; - # if they specify at login, use that - if ($query->param('branch')) { - $branchcode = $query->param('branch'); - $branchname = GetBranchName($branchcode); - } - my $branches = GetBranches(); - if (C4::Context->boolean_preference('IndependantBranches') && C4::Context->boolean_preference('Autolocation')){ - # we have to check they are coming from the right ip range - my $domain = $branches->{$branchcode}->{'branchip'}; - if ($ip !~ /^$domain/){ - $loggedin=0; - $info{'wrongip'} = 1; - } - } - - my @branchesloop; - foreach my $br ( keys %$branches ) { - # now we work with the treatment of ip - my $domain = $branches->{$br}->{'branchip'}; - if ( $domain && $ip =~ /^$domain/ ) { - $branchcode = $branches->{$br}->{'branchcode'}; - - # new op dev : add the branchprinter and branchname in the cookie - $branchprinter = $branches->{$br}->{'branchprinter'}; - $branchname = $branches->{$br}->{'branchname'}; - } - } - $session->param('number',$borrowernumber); - $session->param('id',$userid); - $session->param('cardnumber',$cardnumber); - $session->param('firstname',$firstname); - $session->param('surname',$surname); - $session->param('branch',$branchcode); - $session->param('branchname',$branchname); - $session->param('flags',$userflags); - $session->param('emailaddress',$emailaddress); - $session->param('ip',$session->remote_addr()); - $session->param('lasttime',time()); - $debug and printf STDERR "AUTH_4: (%s)\t%s %s - %s\n", map {$session->param($_)} qw(cardnumber firstname surname branch) ; - } - elsif ( $return == 2 ) { - #We suppose the user is the superlibrarian - $borrowernumber = 0; - $session->param('number',0); - $session->param('id',C4::Context->config('user')); - $session->param('cardnumber',C4::Context->config('user')); - $session->param('firstname',C4::Context->config('user')); - $session->param('surname',C4::Context->config('user')); - $session->param('branch','NO_LIBRARY_SET'); - $session->param('branchname','NO_LIBRARY_SET'); - $session->param('flags',1); - $session->param('emailaddress', C4::Context->preference('KohaAdminEmailAddress')); - $session->param('ip',$session->remote_addr()); - $session->param('lasttime',time()); - } - C4::Context::set_userenv( - $session->param('number'), $session->param('id'), - $session->param('cardnumber'), $session->param('firstname'), - $session->param('surname'), $session->param('branch'), - $session->param('branchname'), $session->param('flags'), - $session->param('emailaddress'), $session->param('branchprinter') - ); - - # Grab borrower's shelves and public shelves and add them to the session - # $row_count determines how many records are returned from the db query - # and the number of lists to be displayed of each type in the 'Lists' button drop down - my $row_count = 10; # FIXME:This probably should be a syspref - my ($total, $totshelves, $barshelves, $pubshelves); - ($barshelves, $totshelves) = C4::VirtualShelves::GetRecentShelves(1, $row_count, $borrowernumber); - $total->{'bartotal'} = $totshelves; - ($pubshelves, $totshelves) = C4::VirtualShelves::GetRecentShelves(2, $row_count, undef); - $total->{'pubtotal'} = $totshelves; - $session->param('barshelves', $barshelves); - $session->param('pubshelves', $pubshelves); - $session->param('totshelves', $total); - - C4::Context::set_shelves_userenv('bar',$barshelves); - C4::Context::set_shelves_userenv('pub',$pubshelves); - C4::Context::set_shelves_userenv('tot',$total); - } + my $ip = $ENV{'REMOTE_ADDR'}; + # if they specify at login, use that + if ($query->param('branch')) { + $branchcode = $query->param('branch'); + $branchname = GetBranchName($branchcode); + } + my $branches = GetBranches(); + if (C4::Context->boolean_preference('IndependantBranches') && C4::Context->boolean_preference('Autolocation')){ + # we have to check they are coming from the right ip range + my $domain = $branches->{$branchcode}->{'branchip'}; + if ($ip !~ /^$domain/){ + $loggedin=0; + $info{'wrongip'} = 1; + } + } + + my @branchesloop; + foreach my $br ( keys %$branches ) { + # now we work with the treatment of ip + my $domain = $branches->{$br}->{'branchip'}; + if ( $domain && $ip =~ /^$domain/ ) { + $branchcode = $branches->{$br}->{'branchcode'}; + + # new op dev : add the branchprinter and branchname in the cookie + $branchprinter = $branches->{$br}->{'branchprinter'}; + $branchname = $branches->{$br}->{'branchname'}; + } + } + $session->param('number',$borrowernumber); + $session->param('id',$userid); + $session->param('cardnumber',$cardnumber); + $session->param('firstname',$firstname); + $session->param('surname',$surname); + $session->param('branch',$branchcode); + $session->param('branchname',$branchname); + $session->param('flags',$userflags); + $session->param('emailaddress',$emailaddress); + $session->param('ip',$session->remote_addr()); + $session->param('lasttime',time()); + $debug and printf STDERR "AUTH_4: (%s)\t%s %s - %s\n", map {$session->param($_)} qw(cardnumber firstname surname branch) ; + } + elsif ( $return == 2 ) { + #We suppose the user is the superlibrarian + $borrowernumber = 0; + $session->param('number',0); + $session->param('id',C4::Context->config('user')); + $session->param('cardnumber',C4::Context->config('user')); + $session->param('firstname',C4::Context->config('user')); + $session->param('surname',C4::Context->config('user')); + $session->param('branch','NO_LIBRARY_SET'); + $session->param('branchname','NO_LIBRARY_SET'); + $session->param('flags',1); + $session->param('emailaddress', C4::Context->preference('KohaAdminEmailAddress')); + $session->param('ip',$session->remote_addr()); + $session->param('lasttime',time()); + } + C4::Context::set_userenv( + $session->param('number'), $session->param('id'), + $session->param('cardnumber'), $session->param('firstname'), + $session->param('surname'), $session->param('branch'), + $session->param('branchname'), $session->param('flags'), + $session->param('emailaddress'), $session->param('branchprinter') + ); + + # Grab borrower's shelves and public shelves and add them to the session + # $row_count determines how many records are returned from the db query + # and the number of lists to be displayed of each type in the 'Lists' button drop down + my $row_count = 10; # FIXME:This probably should be a syspref + my ($total, $totshelves, $barshelves, $pubshelves); + ($barshelves, $totshelves) = C4::VirtualShelves::GetRecentShelves(1, $row_count, $borrowernumber); + $total->{'bartotal'} = $totshelves; + ($pubshelves, $totshelves) = C4::VirtualShelves::GetRecentShelves(2, $row_count, undef); + $total->{'pubtotal'} = $totshelves; + $session->param('barshelves', $barshelves); + $session->param('pubshelves', $pubshelves); + $session->param('totshelves', $total); + + C4::Context::set_shelves_userenv('bar',$barshelves); + C4::Context::set_shelves_userenv('pub',$pubshelves); + C4::Context::set_shelves_userenv('tot',$total); + } else { if ($userid) { $info{'invalid_username_or_password'} = 1; diff --git a/C4/Members.pm b/C4/Members.pm index 7def77e..70d441c 100644 --- a/C4/Members.pm +++ b/C4/Members.pm @@ -93,8 +93,8 @@ BEGIN { &DeleteMessage &GetMessages &GetMessagesCount - &IssueSlip + GetBorrowersWithEmail ); #Modify data @@ -2308,6 +2308,36 @@ sub IssueSlip { ); } +=head2 GetBorrowersWithEmail + + ([$borrnum,$userid], ...) = GetBorrowersWithEmail('me@example.com'); + +This gets a list of users and their basic details from their email address. +As it's possible for multiple user to have the same email address, it provides +you with all of them. If there is no userid for the user, there will be an +C there. An empty list will be returned if there are no matches. + +=cut + +sub GetBorrowersWithEmail { + my $email = shift; + + my $dbh = C4::Context->dbh; + + my $query = "SELECT borrowernumber, userid FROM borrowers WHERE email=?"; + my $sth=$dbh->prepare($query); + $sth->execute($email); + my @result = (); + while (my $ref = $sth->fetch) { + push @result, $ref; + } + die "Failure searching for borrowers by email address: $sth->errstr" if $sth->err; + return @result; +} + + +END { } # module clean-up code here (global destructor) + 1; __END__ diff --git a/acqui/finishreceive.pl b/acqui/finishreceive.pl index 90a85fe..d09a40a 100755 --- a/acqui/finishreceive.pl +++ b/acqui/finishreceive.pl @@ -34,7 +34,9 @@ use List::MoreUtils qw/any/; my $input=new CGI; my $flagsrequired = {acquisition => 'order_receive'}; -my ($loggedinuser, $cookie, $sessionID) = checkauth($input, 0, $flagsrequired, 'intranet'); + +checkauth($input, 0, $flagsrequired, 'intranet'); + my $user=$input->remote_user; my $biblionumber = $input->param('biblionumber'); my $biblioitemnumber=$input->param('biblioitemnumber'); diff --git a/catalogue/updateitem.pl b/catalogue/updateitem.pl index 7f93ee1..abc809d 100755 --- a/catalogue/updateitem.pl +++ b/catalogue/updateitem.pl @@ -30,7 +30,7 @@ use C4::Reserves; my $cgi= new CGI; -my ($loggedinuser, $cookie, $sessionID) = checkauth($cgi, 0, {circulate => 'circulate_remaining_permissions'}, 'intranet'); +checkauth($cgi, 0, {circulate => 'circulate_remaining_permissions'}, 'intranet'); my $biblionumber=$cgi->param('biblionumber'); my $itemnumber=$cgi->param('itemnumber'); @@ -56,7 +56,7 @@ for ($damaged,$itemlost,$wthdrawn) { # modify MARC item if input differs from items table. my $item_changes = {}; if (defined $itemnotes) { # i.e., itemnotes parameter passed from form - my ($loggedinuser, $cookie, $sessionID) = checkauth($cgi, 0, {editcatalogue => 'edit_items'}, 'intranet'); + checkauth($cgi, 0, {editcatalogue => 'edit_items'}, 'intranet'); if ((not defined $item_data_hashref->{'itemnotes'}) or $itemnotes ne $item_data_hashref->{'itemnotes'}) { $item_changes->{'itemnotes'} = $itemnotes; } diff --git a/etc/koha-httpd.conf b/etc/koha-httpd.conf index 2b986ac..39cfeeb 100644 --- a/etc/koha-httpd.conf +++ b/etc/koha-httpd.conf @@ -20,6 +20,14 @@ SetEnv MEMCACHED_SERVERS "__MEMCACHED_SERVERS__" SetEnv MEMCACHED_NAMESPACE "__MEMCACHED_NAMESPACE__" +# If your Apache is configured to use SSL, activating these will allow you +# to use client-side certificates to authenticate users. See the 'AllowPKIAuth' +# system preference. +# SSLVerifyClient require +# SSLVerifyDepth 2 +# SSLCACertificateFile /etc/apache2/ssl/test/ca.crt +# SSLOptions +StdEnvVars + mod_gzip_on yes mod_gzip_dechunk yes @@ -119,6 +127,14 @@ ErrorDocument 404 /cgi-bin/koha/errors/404.pl ErrorDocument 500 /cgi-bin/koha/errors/500.pl +# If your Apache is configured to use SSL, activating these will allow you +# to use client-side certificates to authenticate users. See the 'AllowPKIAuth' +# system preference. +# SSLVerifyClient require +# SSLVerifyDepth 2 +# SSLCACertificateFile /etc/apache2/ssl/test/ca.crt +# SSLOptions +StdEnvVars + mod_gzip_on yes mod_gzip_dechunk yes diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql index d16ff81..8f95e16 100644 --- a/installer/data/mysql/sysprefs.sql +++ b/installer/data/mysql/sysprefs.sql @@ -348,3 +348,4 @@ INSERT INTO systempreferences (variable,value,explanation,options,type) VALUES(' INSERT INTO systempreferences (variable,value,explanation,options,type) VALUES('CatalogModuleRelink',0,'If OFF the linker will never replace the authids that are set in the cataloging module.',NULL,'YesNo'); INSERT INTO systempreferences (variable,value,options,explanation,type) VALUES ('ExpireReservesMaxPickUpDelay', '0', '', 'Enabling this allows holds to expire automatically if they have not been picked by within the time period specified in ReservesMaxPickUpDelay', 'YesNo'); INSERT INTO systempreferences` (variable,value,options,explanation,type) VALUES ('ExpireReservesMaxPickUpDelayCharge', '0', NULL , 'If ExpireReservesMaxPickUpDelay is enabled, and this field has a non-zero value, than a borrower whose waiting hold has expired will be charged this amount.', 'free') +INSERT INTO systempreferences (variable,value,explanation,options,type) VALUES ('AllowPKIAuth','None','Use the field from a client-side SSL certificate to look a user in the Koha database','None|Common Name|emailAddress','Choice'); diff --git a/installer/data/mysql/updatedatabase.pl b/installer/data/mysql/updatedatabase.pl index b844560..df3181d 100755 --- a/installer/data/mysql/updatedatabase.pl +++ b/installer/data/mysql/updatedatabase.pl @@ -4892,6 +4892,15 @@ if ( C4::Context->preference("Version") < TransformToNum($DBversion) ) { SetVersion($DBversion); } +$DBversion = "3.07.00.XXX"; +if ( C4::Context->preference("Version") < TransformToNum($DBversion) ) { + $dbh->do(qq{ + INSERT INTO `systempreferences` (variable,value,explanation,options,type) VALUES('AllowPKIAuth',0,'This allows the user to authenticate via client side certificates',NULL,'YesNo'); + }); + print "Upgrade to $DBversion done (Bug 6296 New System preference AllowPKIAuth)\n"; + SetVersion($DBversion); +} + =head1 FUNCTIONS =head2 DropAllForeignKeys($table) diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/admin.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/admin.pref index f026c7e..04a3f44 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/admin.pref +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/admin.pref @@ -94,4 +94,13 @@ Administration: - of CAS when logging out of Koha. - - The CAS Authentication Server can be found at - - pref: casServerUrl + - pref: casServerUrl + - + - Use + - pref: AllowPKIAuth + default: None + choices: + None: "no" + Common Name: the Common Name (checked against userid) + emailAddress: the emailAddress + - field for SSL client certificate authentication diff --git a/members/setstatus.pl b/members/setstatus.pl index a45a331..31a62e6 100755 --- a/members/setstatus.pl +++ b/members/setstatus.pl @@ -36,7 +36,7 @@ my $input = new CGI; my $flagsrequired; $flagsrequired->{borrowers}=1; -my ($loggedinuser, $cookie, $sessionID) = checkauth($input, 0, $flagsrequired); +checkauth($input, 0, $flagsrequired); my $destination = $input->param("destination") || ''; my $cardnumber = $input->param("cardnumber"); diff --git a/reserve/placerequest.pl b/reserve/placerequest.pl index f3e79b3..3fe459c 100755 --- a/reserve/placerequest.pl +++ b/reserve/placerequest.pl @@ -35,7 +35,7 @@ use C4::Auth qw/checkauth/; my $input = CGI->new(); -my ($user, $cookie, $sesion_id, $flags) = checkauth($input, 0, { reserveforothers => 'place_holds' }, 'intranet'); +checkauth($input, 0, { reserveforothers => 'place_holds' }, 'intranet'); my @bibitems=$input->param('biblioitem'); # FIXME I think reqbib does not exist anymore, it's used in line 82, to AddReserve of contraint type 'o' diff --git a/serials/reorder_members.pl b/serials/reorder_members.pl index 28175fb..8b64fc7 100755 --- a/serials/reorder_members.pl +++ b/serials/reorder_members.pl @@ -29,8 +29,7 @@ my $subscriptionid = $query->param('subscriptionid'); my $routingid = $query->param('routingid'); my $rank = $query->param('rank'); -my ( $user, $cookie, $sesion_id, $flags ) = - checkauth( $query, 0, { serials => 1 }, 'intranet' ); +checkauth( $query, 0, { serials => 1 }, 'intranet' ); reorder_members( $subscriptionid, $routingid, $rank ); diff --git a/serials/subscription-detail.pl b/serials/subscription-detail.pl index afe633e..f6d7884 100755 --- a/serials/subscription-detail.pl +++ b/serials/subscription-detail.pl @@ -95,8 +95,7 @@ if ($op eq 'del') { } my $hasRouting = check_routing($subscriptionid); -my ($user, $sessionID, $flags); -($user, $cookie, $sessionID, $flags) +(undef, $cookie, undef, undef) = checkauth($query, 0, {catalogue => 1}, "intranet"); # COMMENT hdl : IMHO, we should think about passing more and more data hash to template->param rather than duplicating code a new coding Guideline ? -- 1.7.2.5