Perl Guru Forums: Perl Programming Help: DBI: Is there a better way to write this to check login information?

CGI/Perl Guide | Learning Center | Forums | Advertise | Login

Site Search: in


	MAIN INDEX	SEARCH POSTS	WHO'S ONLINE	LOG IN

Home: Perl Programming Help: DBI:
Is there a better way to write this to check login information?

hwnd
User

Mar 31, 2011, 5:59 AM

Post #1 of 15 (14370 views)

Is there a better way to write this to check login information?	Can't Post

Im wondering if there is a better or easier way to break this down for checking stored usernames and passwords in a sql db?

Code

#!/usr/bin/perl -T 
 
use strict; 
use warnings; 
use DBI;   
use CGI;  
 
my $q = new CGI; 
 
my $stored_user = $q->param('user'); 
my $stored_pass = $q->param('pass'); 
 
print $q->header(); 
 
my $dbh = DBI->connect("DBI:mysql:hwnd_news:localhost", "hwnd_hwnd", "b71lnk") 
			or die $DBI::errstr;  
   
my $sth = $dbh->prepare("select * from news_users WHERE name = ? AND pass = ?");   
$sth->execute($stored_user, $stored_pass); 
 
my ( $id, $name, $pw ); 
 
$sth->bind_columns( \$id, \$name, \$pw ); 
$sth->fetch; 
 
 if (($name eq $stored_user) && ($pw eq $stored_pass)) { 
   do something... 
 }  
 else { 
   return login error.. 
 }

miller
User

Mar 31, 2011, 8:36 AM

Post #2 of 15 (14362 views)

Re: [hwnd] Is there a better way to write this to check login information? [In reply to]	Can't Post

Well, you can save one line of typing by doing the following

Code

$sth->bind_columns(\my ($id, $name, $pw));

Also, I personally never use "SELECT * ". If you're database schema changes or is added to, it could lead to unintended errors. Additionally, the best way to self-document your code is to include the columns you're pulling explicitly in your sql statement, especially if you're intending to pull explicit columns like in your above code.

Other than that, not much to point out.

FishMonger
Veteran

Mar 31, 2011, 9:13 AM

Post #3 of 15 (14361 views)

Re: [hwnd] Is there a better way to write this to check login information? [In reply to]	Can't Post

First, it's best not to use indirect objects i.e., instead of

Code

my $q = new CGI;

do it this way

Code

my $q = CGI->new;

or even better, IMO

Code

my $cgi = CGI->new;

As for the db portion, I agree with miller on not using 'select *', and I also don't see the need to use bind_collumns.

Code

my ( $id, $name, $pw ) = $sth->fetchrow_array();

In either case, you should add a 'finish' statement after the fetch.

Code

$sth->finish;

Or add a limit clause to the select statement.

Code

my $sth = $dbh->prepare("select id, name, pass from news_users WHERE name = ? AND pass = ? LIMIT 1");

hwnd
User

Mar 31, 2011, 3:16 PM

Post #4 of 15 (14350 views)

Re: [FishMonger] Is there a better way to write this to check login information? [In reply to]	Can't Post

Thanks for the help, much appreciated. Only thing I see now is when i run the script by itself in the browser it prints out "good login" without even submitting info to the script, should I change something up and make a subroutine for this?

miller
User

Mar 31, 2011, 4:03 PM

Post #5 of 15 (14347 views)

Re: [hwnd] Is there a better way to write this to check login information? [In reply to]	Can't Post

You probably just need to add a test to see if you're actually posting the form. Something roughly like:

Code

if ($q->request_method() eq 'POST') { 
     # Validate the user 
} else { 
     # Just display the form. 
}

FishMonger
Veteran

Mar 31, 2011, 4:51 PM

Post #6 of 15 (14345 views)

Re: [miller] Is there a better way to write this to check login information? [In reply to]	Can't Post

Personally, I prefer (in most cases) not to test for POST or GET, but instead test for expected params.

To hwnd, as to the question regarding subroutines, I'd defiantly write several subroutines and based on the user input (or lack of), Id execute the appropriate subroutine. In most cases I use a dispatch table for this purpose.
http://en.wikipedia.org/wiki/Dispatch_table

hwnd
User

Mar 31, 2011, 7:26 PM

Post #7 of 15 (14335 views)

Re: [FishMonger] Is there a better way to write this to check login information? [In reply to]	Can't Post

Possbily could you break this down how to implement this into how i would use this for this kind of job?

Here is my current updated code:

Code

#!/usr/bin/perl -T 
 
use strict; 
use warnings; 
use DBI;   
use CGI;  
 
my $q = CGI->new; 
 
my $stored_user = $q->param('user'); 
my $stored_pass = $q->param('pass'); 
 
print $q->header; 
 
my $dbh = DBI->connect("DBI:mysql:hwnd_news:localhost", "hwnd_hwnd", "b71lnk") 
			or die $DBI::errstr;  
   
my $sth = $dbh->prepare("select id, name, pass from news_users WHERE name = ? AND pass = ? LIMIT 1"); 
$sth->execute($stored_user, $stored_pass); 
 
my ( $id, $name, $pw ) = $sth->fetchrow_array(); 
 
 if (($name eq $stored_user) && ($pw eq $stored_pass)) { 
   print "good login\n"; 
 }  
 else { 
   print "bad login\n"; 
 }

(This post was edited by hwnd on Apr 1, 2011, 4:21 AM)

FishMonger
Veteran

Apr 1, 2011, 9:29 AM

Post #8 of 15 (14306 views)

Re: [hwnd] Is there a better way to write this to check login information? [In reply to]	Can't Post

This is a rough example. In actual production code I'd have more checks and balances.

Code

#!/usr/bin/perl -T 
 
use strict; 
use warnings FATAL => 'all'; 
use DBI; 
use CGI; 
use CGI::Carp qw( fatalsToBrowser ); 
 
my $cgi = CGI->new; 
 
print $cgi->header, 
      $cgi->start_html('Site Login'); 
 
display_login_form() and exit unless $cgi->param('login'); 
 
my $authorized = authenticate($cgi->param('user'), $cgi->param('pass')); 
 
if ( $authorized ) { 
    print "good login\n"; 
} 
else { 
    print "bad login\n"; 
} 
 
# ***************************************** 
 
sub display_login_form { 
    # code that generates the login form 
} 
 
sub authenticate { 
   
    my $user = shift; 
    my $pass = shift; 
     
    return 0 unless $user && $pass; 
     
    my $dbh = DBI->connect("DBI:mysql:hwnd_news:localhost", "hwnd_hwnd", "b71lnk") 
            or die $DBI::errstr; 
     
    my $sth = $dbh->prepare( 
                            "SELECT id, name, pass from news_users 
                            WHERE name = ? AND pass = ? 
                            LIMIT 1" 
                            ); 
    $sth->execute($user, $pass); 
     
    my ( $id, $name, $pw ) = $sth->fetchrow_array(); 
    $dbh->disconnect; 
     
    return ($name eq $user and $pw eq $pass) ? 1 : 0; 
}

(This post was edited by FishMonger on Apr 1, 2011, 9:29 AM)

hwnd
User

Apr 1, 2011, 1:49 PM

Post #9 of 15 (14299 views)

Re: [FishMonger] Is there a better way to write this to check login information? [In reply to]	Can't Post

This is great, thank you for the help and examples of implementing and doing this. But in this line, what is this calling and is it from the query line, login.pl?login

Code

... unless $cgi->param('login');

I'm asking because i made a simple from in the display_login sub routine and called the script by itself and with ?login and everytime its returning the display form and not a result

(This post was edited by hwnd on Apr 1, 2011, 2:10 PM)

FishMonger
Veteran

Apr 1, 2011, 2:31 PM

Post #10 of 15 (14293 views)

Re: [hwnd] Is there a better way to write this to check login information? [In reply to]	Can't Post

The example I gave made the assumption that the submit button had a name/value of 'login' and that is what I was testing for instead of the username and password.

So, if this was the initial loading of the page, then the login form would be displayed. Once the user submits the form via the "login" button, the test would see that login param and proceed with the authentication portion.

hwnd
User

Apr 1, 2011, 8:59 PM

Post #11 of 15 (14283 views)

Re: [FishMonger] Is there a better way to write this to check login information? [In reply to]	Can't Post

It's giving an error using this statement -> Software error: Use of uninitialized value in string eq at loginb.pl line 57

Code

return ($name eq $user and $pw eq $pass) ? 1 : 0;

I'm guessing the data is not what it thinks it is. I don't need to check the return values from the database because they'll only be defined if the username and password match. It won't return a record, so $id, $name and $pw will be undef if the password and username don't match. So I just check if id is defined which is the same as asking did it return a record.

Code

return (defined $id);

Also I tried this with just selected the id and it works fine.

Code

sub authenticate {  
    
    my $user = shift;  
    my $pass = shift;  
      
    return 0 unless $user && $pass;  
      
    my $dbh = DBI->connect("DBI:mysql:hwnd_news:localhost", "hwnd_hwnd", "b71lnk")  
            or die $DBI::errstr;  
      
    my $sth = $dbh->prepare(  
                            "SELECT id from news_users  
                            WHERE name = ? AND pass = ?  
                            LIMIT 1"  
                            );  
    $sth->execute($user, $pass); 
 
    my ( $id ) = $sth->fetchrow_array(); 
    $dbh->disconnect;  
       
    return ($id) ? 1 : 0 
 
}

(This post was edited by hwnd on Apr 2, 2011, 1:55 PM)

serotta1958
New User

Aug 4, 2011, 7:53 AM

Post #12 of 15 (11928 views)

Re: [FishMonger] Is there a better way to write this to check login information? [In reply to]	Can't Post

when do you call this script?
I dont understand what protects the down stream scripts from a person by passing the login and calling those scripts directly.
Is there an authentication check at top of each script?

FishMonger
Veteran

Aug 4, 2011, 8:10 AM

Post #13 of 15 (11927 views)

Re: [serotta1958] Is there a better way to write this to check login information? [In reply to]	Can't Post

You are correct. There is nothing in the posted script to prevent the user from bypassing the login.

To prevent that problem, I use server side sessions and each page checks the session to verify authentication and redirects to the login page as needed.

http://search.cpan.org/~markstos/CGI-Session-4.48/lib/CGI/Session.pm

serotta1958
New User

Aug 4, 2011, 8:42 AM

Post #14 of 15 (11925 views)

Re: [FishMonger] Is there a better way to write this to check login information? [In reply to]	Can't Post

Thanks!!
So after the sucessful login shown above you would store data on the session using
# Storing data in the session:
$session->param('f_name', 'Sherzod');

and then at the top of each downstream script you check the session using
my $f_name = $session->param('f_name');

also, why do you use your own user table and not just GRANT each individual user access in mysql security?

FishMonger
Veteran

Aug 4, 2011, 12:54 PM

Post #15 of 15 (11912 views)

Re: [serotta1958] Is there a better way to write this to check login information? [In reply to]	Can't Post

I'd store and check multiple pieces of data but the key param that I focus on is "logged-in" and use it in a boolean test at the beginning of each script.

Quote

also, why do you use your own user table and not just GRANT each individual user access in mysql security?

In the context of the question in this thread, doing that doesn't make any sense. If you have something specific in mind, please post your question in a new thread.

Search for (options)

Web Applications & Managed Hosting Powered by Gossamer Threads
Visit our Mailing List Archives