Login System

Chris Muench · 12-04-2003, 10:01 PM

Are their any major problems you notice with this code: (I have many files because this is just a start to the re write of my program) I just need to know if their are any security issues or bad coding.

login.php

PHP Code:


			
<html>

<head>



</head>



<body>

<?php

session_start();



include ("config.php");

include ("classes/db_functions.php");

include ("classes/security_functions.php");



//create two objects that are needed in this script

$dbf=new db_functions($cfg_server,$cfg_username,$cfg_password,$cfg_database);

$sec=new security_functions();



if(isset($_POST['username']) and isset($_POST['password']))

{

    $username = $_POST['username'];

    $password = $_POST['password'];

     

        if($sec->checkLogin($dbf,$username,$password))

        {

             $_SESSION ['user'] = $username;

            header ("location: index.php");

        }

        else

        {

               echo '<center>username or password are incorrect</center>';

        }

}



elseif($sec->isLoggedIn())

{

    header ("location: index.php");

}

else

{

?>

<center>

    <form name="login" action="login.php" method="POST">

        <table width="348" border="0" cellspacing="2" cellpadding="0">

            <tr>

                <td width="85">

                    Username:

                </td>

                <td>

                    <input type="text" name="username" size="24">

                </td>

            </tr>

            <tr>

                <td width="85">

                    Password:

                </td>

                <td>

                    <input type="password" name="password" size="24">

                </td>

            </tr>

        </table>

        <input type="submit" name="Submit" value="Log In"> 

    </form>

</center>



<?php

}



?>

</body>

</html>

index.php

PHP Code:


			
<?php

session_start();

include ("config.php");

include ("classes/db_functions.php");

include ("classes/security_functions.php");





$dbf=new db_functions($cfg_server,$cfg_username,$cfg_password,$cfg_database);

$sec=new security_functions();



if(!$sec->isLoggedIn())

{

    header ("location: login.php");

}



?>

db_functions.php

PHP Code:


			
<?php

class db_functions

{    

    //class variable that represents the database connection.

    var $conn;

    

    //user-defined constructor

    function db_functions($server,$username,$password,$database)

    {

        //pre: parameters must be correct in order to connect to database.

        //post: connects to database.

        

        $this->conn = mysql_connect("$server", "$username", "$password") or die("Could not connect : " . mysql_error());

        mysql_select_db("$database",$this->conn) or die("Could not select database <b>$database</b>");

        

    }

    



}



?>

security_functions.php

PHP Code:


			
<?php





class security_functions

{

    //defalt constructor

    function security_functions()

    {

    

    

    }

    

    function isLoggedIn()

    {

        if(isset($_SESSION['user']))

        {

            return true;

        }

        return false;

    }

    

    function checkLogin($dbf,$username,$password)

    {

        //pre: $dbf must be a db_functions object and $username and password must be strings

        //post: returns boolean based on if their login was succesfull.

        

        $result = mysql_query ("SELECT * FROM users WHERE username='$username' and password='$password'",$dbf->conn);    

    

        $num = @mysql_num_rows($result);

        

        if($num > 0)

        {

            return true;

        }

        

        return false;

    }







}

Thanks,

Chris Muench

planetsim · 12-05-2003, 04:31 AM

Im a bit weary about this function "isLoggedIn() "

You are only checking if the user has a session registered. You should also check that the user session matches maybe a userid or username to make sure they are not only logged in but registered.

Also you must encrypt your passwords either by md5() which just hashes or there is mcrypt not sure on others

***Weedpacket*** · 12-05-2003, 05:57 AM

Quote:

Originally posted by planetsim
Also you must encrypt your passwords either by md5() which just hashes or there is mcrypt not sure on others

There's also PHP's native support of SHA1, which happens to be what the NIST recommends, but that's a more recent addition.

Chris Muench · 12-05-2003, 07:23 AM

How do you dycrpt passwords inserted into the database (md5)

***Weedpacket*** · 12-05-2003, 08:49 AM

Quote:

Originally posted by Chris Muench
How do you dycrpt passwords inserted into the database (md5)

You can't. That's the idea.

Shrike · 12-05-2003, 01:53 PM

Then the client asks "can we have a remind password feature please?"

planetsim · 12-05-2003, 04:52 PM

And you create a random password function

PHP Code:


			
function rnd_Pass()

{

  srand(microtime() * 1000000000000);

  $pass_rnd = array('a','b','c',

    'd','e','f',

    'g','h','i',

    'j','k','l',

    'm','n','o',

    'p','q','r',

    's','t','u',

    'v','w','x',

    'y','z','A',

    'B','C','D',

    'E','F','G',

    'H','I','J',

    'K','L','M',

    'N','O','P',

    'Q','R','S',

    'T','U','V',

    'W','X','Y',

    'Z','0','1',

    '2','3','4',

    '5','6','7',

    '8','9');

  $rnd = array_rand($pass_rnd, 8);

  $password = "";

  for($i=0;$i<=7;$i++){

    $password .= $pass_rnd[$rnd[$i]];

  }

  return $password;

}

Quite a basic one there

agoe · 12-05-2003, 07:52 PM

Quote:

Originally posted by planetsim
Im a bit weary about this function "isLoggedIn() "

You are only checking if the user has a session registered. You should also check that the user session matches maybe a userid or username to make sure they are not only logged in but registered.

i disagree. if the user types the correct username and password, the session-variable is set. otherwise it is not.

consider this:

PHP Code:


			
$sql = "SELECT the, fields, you, need FROM your_table WHERE Username='".validiate_function($_POST['username'])."' AND Password='".md5($_POST["password"])."'";

    $result = mysql_query($sql) or die(mysql_error());

    

if (mysql_num_rows($result) == 1) {

        // the user is authenticated

    $_SESSION['authed'] = 1;

} else {

    echo "wrong username/password";

}

then, on top of every page that needs authentication, just check if $_SESSION['authed'] == 1.

it's as bulletproof as it gets.

Moonglobe · 12-05-2003, 08:22 PM

Quote:

PHP Code:


			
  $pass_rnd = array('a','b','c',

    'd','e','f',

    'g','h','i',

    'j','k','l',

    'm','n','o',

    'p','q','r',

    's','t','u',

    'v','w','x',

    'y','z','A',

    'B','C','D',

    'E','F','G',

    'H','I','J',

    'K','L','M',

    'N','O','P',

    'Q','R','S',

    'T','U','V',

    'W','X','Y',

    'Z','0','1',

    '2','3','4',

    '5','6','7',

    '8','9');

PHP Code:


			
$pass_rnd = array_merge(range('a','z'), range('A','Z'), range(0,9));

daveyboy · 12-15-2003, 03:39 PM

checking a single session value is fine, as long as u only use it for authentication. whenever you call session_start() your session vars will overwrite any the user has passed in. just make sure that you're checking $_SESSION['user'] not just $user.

one thing: your redirects are incorrect: the location header requires a fully qualified url, e.g. http://www.example.com/index.php

Moonglobe · 12-15-2003, 08:00 PM

Quote:

Originally posted by daveyboy
one thing: your redirects are incorrect: the location header requires a fully qualified url, e.g. http://www.example.com/index.php

in theory yes, but there's near total backward compatibiliy with HTTP/1.0 (that's a 1.1 thing)

planetsim · 12-15-2003, 08:18 PM

Quote:

Originally posted by Moonglobe

PHP Code:

$pass_rnd = array_merge(range('a','z'), range('A','Z'), range(0,9));

Suppose should update my code.. I remember writing that function like when i first learnt i thought it was the best thing i made and never went and made it better.