Newbie creating login with sessions.

[AStrangerWCandy]

I'm trying to create a site that has logins and stores the login information in the user's session data. Basically self taught PHP from PHP for Dummies and then Learning PHP 5 from O'Reilly. Here is the code snippet from a function I've written for validating whether the information submitted in the login box exists and matches whats stored in the mysql database. Just wondering from people who know better than I if this looks like it should work (I haven't created the Tables and whatnot yet) or if I'm just headed in the wrong direction:

 

//Query the database to make sure username/password entered are valid and matchup

$sql2 = "SELECT userName FROM Users WHERE userName='$_POST[username]' AND passWord='$_POST[password]'";

$sql3 = "SELECT passWord FROM Users WHERE userName='$_POST[username]' AND passWord='$_POST[password]'";

$result1 = mysql_query($sql2);

$result2 = mysql_query($sql3);

$users = array('$result1' => '$result2');

 

//Makes sure username is valid

if (! array_key_exists($_POST['username'], $users))

{

$errors[] = 'Please enter a valid username and password.';

}

 

return $errors;

}

[Asheeown]

Welcome to PHP, first off.

Your approach is knowledgeable, however there are better ways of going forth with it.

 

First lets take a look at

  $sql2 = "SELECT userName FROM Users WHERE userName='$_POST[username]' AND passWord='$_POST[password]'";

  $sql3 = "SELECT passWord FROM Users WHERE userName='$_POST[username]' AND passWord='$_POST[password]'";

 

Very good to match the username and password, however you can minimize it to one query and just have it select a count, like so:

 

$sql = "SELECT count(*) FROM Users WHERE userName='$_POST[username]' AND passWord='$_POST[password]'";
$result = mysql_query($sql);
$Count = mysql_result($result, 0);
if($Count > 0) {
     echo "User information is correct";  // You can continue with the login process after this
} else {
     echo "User information is incorrect";
}

 

Also, when you say you are going to store the username and password in a session variable, it's not really a good idea to do this.  You can however store a "logged in" flag in a session variable to tell you that user is logged in, then you can have others, such as username, first name, etc.

[mikesta707]

for one, associative array keys (basically array keys that are names, and not numbers) have to be surrounded by single quotes. Since you surround the whole variable with single quotes, you should concatenate the sql query string with the values, like so

 

$sql2 = "SELECT userName FROM Users WHERE userName='".$_POST['username']."' AND passWord='".$_POST['password']."'";

also, you are doing two queries for no real reason. you can grap both username and password with one query

$sql2 = "SELECT userName, passWord FROM Users WHERE userName='".$_POST['username']."' AND passWord='".$_POST['password']."'";

 

im not even sure what your doing in this line

$users = array('$result1' => '$result2');

but get rid of it. its completely wrong.

 

If you want to check if the user exists on the database, you want to do a mysql_num_rows check, IE

 

if(mysql_num_rows($result1) == 1){
return true;//user successfully logged on
}
return false;//login unsuccessful

 

im also not really sure what

if (! array_key_exists($_POST['username'], $users))
      {
      $errors[] = 'Please enter a valid username and password.';
      }

is supposed to accomplish either.. that won't really check for anything useful im ny opinion.

 

However, you are on the right track! good luck with the rest of your PHP experience!

[jamesxg1]

Here would this be a bit more usable ?,

 

<?php
      function Login($lusername, $lpassword) {
                  
          $username = mysql_real_escape_string(trim(addslashes(strip_tags($lusername))));
          $password = mysql_real_escape_string(trim(addslashes(strip_tags(md5($lpassword)))));
          
          $LoginQ = "SELECT * FROM `members` WHERE username = '$username' AND password = '$password'";
          $LoginR = mysql_query($LoginQ) or die (mysql_error());


          if(mysql_num_rows($LoginR) == '1') {
           
              while($fetch =  mysql_fetch_array($LoginR)) { 
                  
                  if($fetch['sus'] == '0') {

                                  
                            $_SESSION['username'] = $username;
                            $_SESSION['id'] = $fetch['id'];

                             echo '<meta http-equiv="refresh" content="0; url=user_home.php">';
                  } else {
                              echo "Sorry but the account you are trying to access is suspended, Please try again."; 
                                 
                             }          
              }
              
          } else {
              
          echo "Sorry but the account you are trying to access is non existant, Please try again."; 
              
          }
    
    
      }
       ?>

 

Then on your form just use this.

 

<?php

include 'loginscript.php';

if(isset($_POST['submit'])) {
Login($_POST['username'], $_POST['password']);

}

?>

 

James.

[AStrangerWCandy]

TY for the responses on both, very helpful especially with the numrow tidbit and avoiding usernames in $_SESSION

[mikesta707]

meh, storing passwords in a session variable probably isn't the best idea, but it is best to store something unique about the user in a session so you know who is logged in at the time. The user only submits their username once, so you only really have one chance to store something unique about that user so you can access his information on other pages (be they an account page, forums, etc.)

 

You don't necessarily need to store the username, but a unique user_id or unique email address or something unique is usually necessary

[AStrangerWCandy]

yeah pretty sure I'll be rewriting it so that when user information in the database is created it adds a unique id the user never sees, and then stores THAT in the session to signify who is looking at the page. Would it be better to keep it seperate?

 

One part of the session says you are logged in, then a second part says who the person logged in is.

 

BTW I just have to say this forum is already phenomenal, never seen such quick and helpful responses regarding anything in my life ;P

[mikesta707]

yeah pretty sure I'll be rewriting it so that when user information in the database is created it adds a unique id the user never sees, and then stores THAT in the session to signify who is looking at the page. Would it be better to keep it seperate?

 

One part of the session says you are logged in, then a second part says who the person logged in is.

 

BTW I just have to say this forum is already phenomenal, never seen such quick and helpful responses regarding anything in my life ;P

 

Yeah you will probably want to have 2 different session variables, 1 for if they are logged in, and a second for their user id if they are logged in.

[AStrangerWCandy]

Another question that didn't seem like it needed another thread. Would it be a good security precaution to set a bunch of junk $_SESSION variables that essentially do nothing upon login and then using a random one of those as the actual login signifier as a defense against potential spoofing?

[AStrangerWCandy]

Oh and thanks again for the help guys, I've completed the login script and it works perfectly!