Jump to content

How to better login script

Grinsa

By Grinsa
in PHP Coding Help

 Share

Recommended Posts

Grinsa Newbie

Grinsa

Posted
Posted

Hello everyone,

I have just finished coding a logion/register/logout script.  I am quite new to PHP (this was my first task to begin the learning process!).  The scripts now work fine and gets the job done.  It incorporates a database and has a number of checks in place.  I know that the code is probably pretty ugly however and not as efficient as it could be.  Could anyone suggest places where I could improve it or security issues with it?  I have tried to secure it against sql injection; it also ensures that no fields are blank and that the two passwords in registration are the same and I have also made username a unique field in database.  Thanks in advance for any help or guidance.

 

Here are the scripts: index.html, checklogin.php, register.php, menu.php, and logout.php

 

<html>
<body>
<table width="300" border="0" align="center" cellpadding="0" cellspacing="1" bgcolor="#CCCCCC">
<tr>
<form name="input" action="checklogin.php" method="post">
<td>
<table width="100%" border="0" cellpadding="3" cellspacing="1" bgcolor="#FFFFFF">
<tr>
<td colspan="3"><strong>Member Login </strong></td>
</tr>
<tr>
<td width="78">Username</td>
<td width="6">:</td>
<td width="294"><input name="username" type="text" id="username"></td>
</tr>
<tr>
<td>Password</td>
<td>:</td>
<td><input name="password" type="password" id="mypassword"></td>
</tr>
<tr>
<td> </td>
<td> </td>
<td><input type="submit" name="login" value="Login"></td>
</tr>
</table>
</td>
</form>
</tr>
</table>
<center>Not a member? <a href="./register.php">Register!</a></center>
</body>
</html>

 

<?php
$host="localhost";
$usr="root";
$pwd="******";
$db="*****";
$tbl_name="members";

mysql_connect($host, $usr, $pwd) or die(mysql_error());
mysql_select_db($db) or die(mysql_error());

$initialusr = $_POST['username'];
$initialpwd = $_POST['password'];
$secondusr = stripslashes($initialusr);
$secondpwd = stripslashes($initialpwd);
$pswd = mysql_real_escape_string($secondpwd);
$myusr = mysql_real_escape_string($secondusr);
$mypswd= md5($pswd);

$sql="SELECT *FROM $tbl_name WHERE username='$myusr' and password='$mypswd'";
$result=mysql_query($sql);

$count=mysql_num_rows($result);

if ($count==1) {
session_start();
$_SESSION['username'] = $myusr;
header("location:menu.php");
}
else {
echo "Incorrect Username or Password";
}
?>

 

<?php
$host="localhost";
$usr="root";
$pwd="*****";
$db="***********";
$tbl_name="members";

mysql_connect($host, $usr, $pwd) or die(mysql_error());
mysql_select_db($db) or die(mysql_error());

if (isset($_POST['register']) && $_POST['username'] && $_POST['password'] && $_POST['confirm'] && $_POST['email'] && $_POST['password'] == $_POST['confirm']) 
{ 
$pwd = mysql_real_escape_string("$_POST[password]");
$md5pwd = md5("$pwd");
$usr = mysql_real_escape_string("$_POST[username]");
$email = mysql_real_escape_string("$_POST[email]");


$query = "INSERT INTO members (username, password, email) 
VALUES('$usr', '$md5pwd', '$email')";

mysql_query($query) or die(mysql_error());
mysql_close();

echo "You have successfully registered!";
}
else{
?>

<html>
<body>
<table width="300" border="0" align="center" cellpadding="0" cellspacing="1" bgcolor="#CCCCCC">
<tr>
<form name="input" action="register.php" method="post">
<td>
<table width="100%" border="0" cellpadding="3" cellspacing="1" bgcolor="#FFFFFF">
<tr>
<td colspan="3"><strong>Register</strong></td>
</tr>
<tr>
<td width="78">Username</td>
<td width="6">:</td>
<td width="294"><input name="username" type="text" id="username"></td>
</tr>
<tr>
<td>Password</td>
<td>:</td>
<td><input name="password" type="password" id="password"></td>
</tr>
<tr>
<td>Confirm Password</td>
<td>:</td>
<td><input name="confirm" type="password" id="confirm"></td>
</tr>
<tr>
<td>Email</td>
<td>:</td>
<td><input name="email" type="text" id="email"></td>
</tr>
<tr>
<td> </td>
<td> </td>
<td><input type="submit" name="register" value="Register"></td>
</tr>
</table>
</td>
</form>
</tr>
</table>
</body>
</html>

<?php
}
?>

 

<?php
session_start();
if (!isset($_SESSION['username'])){
header("location:index.html");
}
else {
?>
<html>
<body>
<?php 
$username = $_SESSION['username'];
echo "Welcome " . $username . " !";
?>
<br />
<a href = logout.php>Log out</a>
</body>
</html>
<?php
}
?>

 

<?php
session_start();
session_destroy();
header("location:index.html")
?>

 

Link to comment
https://forums.phpfreaks.com/topic/223624-how-to-better-login-script/
Share on other sites
revraz Member

revraz

Posted
Posted

My advice...

 

First, indent your code to make it easier to read.

Second, don't sanitize the password, once you hash it, sql injection can't happen.  When you sanitize it, you are actually changing the password by removing certain characters if they are being used.

Third, try to put session_start() at the top of each script that you use sessions, this will avoid output to the screen before your session is started.

Zurev Newbie

Zurev

Posted
Posted

My advice...

 

First, indent your code to make it easier to read.

Second, don't sanitize the password, once you hash it, sql injection can't happen.  When you sanitize it, you are actually changing the password by removing certain characters if they are being used.

Third, try to put session_start() at the top of each script that you use sessions, this will avoid output to the screen before your session is started.

 

On top of that, you are doing a check of if (thispostfield, and thisone, and thisone), meaning you aren't doing ANYTHING until all fields are filled, which is fine, but your user has no way of knowing if they did something wrong or not.

 

Just check if the submit button is set, and for the rest, validate them, make sure they aren't empty, and are in the right format, and if they aren't, add an error message fitting for that scenario to an array of errors. THEN do nothing if that array has any items, and use a foreach loop to loop through the array and spit out the error messages so they know.

 

By doing this you can let the user know the field is empty, and you can put checks in place for valid email addresses and other valid formats. Also, since you're a beginner it's fine for now, but there are A LOT better ways to secure passwords rather than md5'ing it and sticking it in the database if you want to look into them.

Grinsa Newbie

Grinsa

Posted
  • Author
Posted

Thank you for the critique.  I will look into more advanced ways of securing the password.  Like you said, I don't think it's quite as important at this time while I am still learning php basics so md5 will do for now.

 

I have done my best to indent my code like I think it is supposed to be, but sorry if it is messy.  I changed quite a bit in the register file to incorporate the error messages (I used if,else,else rather than an array with for loops, which I looked into but was not able to get working).  I also made a file database.php where I included the database pw, root, etc. and the connect command to keep from including it in every file.  I also removed the functions that "sanitized" the pw both here and in the checklogin.php file.  How does this look?

 

<?php
include('database.php');

if (!isset($_POST['register']))

{
?>

<html>
<body>
<table width="300" border="0" align="center" cellpadding="0" cellspacing="1" bgcolor="#CCCCCC">
<tr>
<form name="input" action="register.php" method="post">
<td>
<table width="100%" border="0" cellpadding="3" cellspacing="1" bgcolor="#FFFFFF">
<tr>
<td colspan="3"><strong>Register</strong></td>
</tr>
<tr>
<td width="78">Username</td>
<td width="6">:</td>
<td width="294"><input name="username" type="text" id="username"></td>
</tr>
<tr>
<td>Password</td>
<td>:</td>
<td><input name="password" type="password" id="password"></td>
</tr>
<tr>
<td>Confirm Password</td>
<td>:</td>
<td><input name="confirm" type="password" id="confirm"></td>
</tr>
<tr>
<td>Email</td>
<td>:</td>
<td><input name="email" type="text" id="email"></td>
</tr>
<tr>
<td> </td>
<td> </td>
<td><input type="submit" name="register" value="Register"></td>
</tr>
</table>
</td>
</form>
</tr>
</table>
</body>
</html>

<?php
}

elseif (empty($_POST['username']) || empty($_POST['password']) || empty($_POST['confirm']) || empty($_POST['email']))

{
echo "One or more fields missing";
}

elseif ($_POST['password'] != $_POST['confirm'])

{
echo "Your passwords do not match";
}

elseif (strlen($_POST['password']) < 8 || strlen($_POST['password']) > 32)

{
echo "Your password must be between 8 and 32 alphanumeric characters long";
}

elseif (strlen($_POST['username']) < 6 || strlen($_POST['username']) > 16)

{
echo "Your username must be between 6 and 16 characters long";
}

else 
{
$pwd = md5("$_POST[password]");
$usr = mysql_real_escape_string("$_POST[username]");
$email = mysql_real_escape_string("$_POST[email]");


$query = "INSERT INTO members (username, password, email) 
VALUES('$usr', '$pwd', '$email')";

mysql_query($query) or die(mysql_error());
mysql_close();

echo "You have successfully registered!";
}
?>

This thread is more than a year old. Please don't revive it unless you have something important to add.

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
 Share
Go to topic listing
×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.