How to better login script
Moderator: General Moderators
Re: How to better login script
Ok, I will need to read up further on defining your own functions, but I understand what this script aims to do. Unfortunately, I still have the same problem as stated previously. I had already changed the alphanum (as well as the checkusername --> checkUsername and checkpassword --> checkPassword) to no avail. I changed this form to checkregister.php and the html form to register.php and it seems like everything is being accepted as successful registration with no checks 
Re: How to better login script
The problem I can see is with the scope of the $errorMsg variable. It's not being defined as 'global', so is_array($errorMsg), which is called in the global scope, is always going to return false. Try adding "global $errorMsg" to the functions, as shown below:
However, because you're not using a return value, this can be simplified to the following:
Code: Select all
function checkPassword($value) {
global $errorMsg;
if (strlen($value) > 7 || strlen($value) < 33) {
return $value;
}
else {
$errorMsg[] = "Your password must be between 8 and 32 characters long!";
}
}
Code: Select all
function checkPassword($value) {
global $errorMsg;
if (strlen($value) < 8 || strlen($value) > 32) {
$errorMsg[] = "Your password must be between 8 and 32 characters long!";
}
}
Re: How to better login script
Thanks for your reply. Unfortunately the script does not seem to function after that I added those lines (when I click the register button a white screen is returned). Here are the register form (html file) and the checkregister.php files:
Code: Select all
<html>
<body>
<table width="300" border="0" align="center" cellpadding="0" cellspacing="1" bgcolor="#CCCCCC">
<tr>
<form name="input" action="checkregister.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>
Code: Select all
<?php
include('database.php');
// check empty value
function checkEmpty($value) {
global $errorMsg
if ($value != '') {
$clean_val = htmlentities($value);
return $clean_val;
}
else {
$errorMsg[] = "One or more fields are missing!";
}
}
function checkUsername($value) {
global $errorMsg
if (strlen($value) < 6 || strlen($value) > 16) {
$errorMsg[] = "Your username must be between 6 and 16 characters long!";
}
}
function alphanumUsername($value) {
global $errorMsg
if (ctype_alnum($value) == false) {
$errorMsg[] = "Your username must consist of numbers and letters only!";
}
}
function confirm($value, $value1) {
global $errorMsg
if ($value != $value1) {
$errorMsg[] = "Your passwords do not match!";
}
}
function checkPassword($value) {
global $errorMsg
if (strlen($value) < 8 || strlen($value) > 32) {
$errorMsg[] = "Your password must be between 8 and 32 characters long!";
}
}
//---
checkempty($_POST['username']);
checkempty($_POST['password']);
checkempty($_POST['confirm']);
checkempty($_POST['email']);
checkUsername($_POST['username']);
alphanumUsername($_POST['username']);
confirm($_POST['password'], $_POST['confirm']);
checkPassword($_POST['password']);
// ----
if (is_array($errorMsg)) {
foreach ($errorMsg as $key) {
echo 'Error : '. $key .'<br />';
}
}
else {
$password = hash('sha512', $_POST['password']);
$username = $_POST['username'];
$email = $_POST['email'];
$query = "INSERT INTO members (username, password, email)
VALUES ('". mysql_real_escape_string($username) ."', '". $password ."',
'". mysql_real_escape_string($email) ."')";
$sql = mysql_query($query);
if ($sql) {
echo 'Registration successful';
}
else {
echo 'Registration unsuccessful';
}
}
?>
- greyhoundcode
- Forum Regular
- Posts: 613
- Joined: Mon Feb 11, 2008 4:22 am
Re: How to better login script
Nothing to do with previous comments, but something I always do after a login/logout is regenerate the session ID.
Re: How to better login script
Grinsa, remember to add a semi-colon (;) after "global $errorMsg".
Re: How to better login script
Ah yes, sorry about that. This fixed the issue and the script seems to work great. I have just two other minor comments/questions:
1) The first function sanitizes the value inputs and returns a "cleanvalue." Should this be done for the password since it is hashed anyways which eliminates the risk for sql injection?
2) I seem to have some sort of issue with my sessions. They are correctly being set I think but the script won't display the username in the welcome statement:
checklogin.php
This is the page that the user is sent to: menu.php
The message only reads "Welcome !" when I would like for it to include the name.
This is logout.php
Thanks for all the help! it's almost complete now and i've learned a fair bit along the way!
1) The first function sanitizes the value inputs and returns a "cleanvalue." Should this be done for the password since it is hashed anyways which eliminates the risk for sql injection?
Code: Select all
// check empty value
function checkEmpty($value) {
global $errorMsg;
if ($value != '') {
$clean_val = htmlentities($value);
return $clean_val;
}
else {
$errorMsg[] = "One or more fields are missing!";
}
}
checklogin.php
Code: Select all
//this is at the very beginning of script
session_start();
session_regenerate_id();
// this is later when password/username combo is verified and if correct:
if ($count==1)
{
$_SESSION['login'] = array('username' => $myusr);
header("location:menu.php");
}
Code: Select all
<?php
session_start();
session_regenerate_id();
if (!isset($_SESSION['login']['username']))
{
echo "You must be <a href='index.html'>logged in</a> to view this page!";
}
else
{
?>
<html>
<body>
<?php
$username = $_SESSION['username'];
echo "Welcome " . $username . " !";
?>
<br />
<a href = logout.php>Log out</a>
</body>
</html>
<?php
}
?>
This is logout.php
Code: Select all
<?php
session_start();
session_regenerate_id();
$_SESSION['login'] = array();
header("location:index.html")
?>
- greyhoundcode
- Forum Regular
- Posts: 613
- Joined: Mon Feb 11, 2008 4:22 am
Re: How to better login script
Could it be that you need to replace $_SESSION['username'] with $_SESSION['login']['username']?Grinsa wrote:2) I seem to have some sort of issue with my sessions. They are correctly being set I think but the script won't display the username in the welcome statement:
- social_experiment
- DevNet Master
- Posts: 2793
- Joined: Sun Feb 15, 2009 11:08 am
- Location: .za
Re: How to better login script
It depends, you are correct in saying that the password gets hashed and if additional code is included it will not match your existing password. The other part is that eliminate some additional characters from being used in a password (&, >, <). Better to be safe than sorryGrinsa wrote:The first function sanitizes the value inputs and returns a "cleanvalue." Should this be done for the password since it is hashed anyways which eliminates the risk for sql injection?
“Don’t worry if it doesn’t work right. If everything did, you’d be out of a job.” - Mosher’s Law of Software Engineering
Re: How to better login script
Yeah i suppose, but the user would not be aware of the fact that he is not able to use these characters and his password would be different than what he thought he had entered?social_experiment wrote:It depends, you are correct in saying that the password gets hashed and if additional code is included it will not match your existing password. The other part is that eliminate some additional characters from being used in a password (&, >, <). Better to be safe than sorry
That worked! Thanks.greyhoundcode wrote:Could it be that you need to replace $_SESSION['username'] with $_SESSION['login']['username']?
- social_experiment
- DevNet Master
- Posts: 2793
- Joined: Sun Feb 15, 2009 11:08 am
- Location: .za
Re: How to better login script
I doubt it would tax your system if you added a simple "Passwords can only contain alphanumeric characters" message somewhere (and then test it with a functionGrinsa wrote:Yeah i suppose, but the user would not be aware of the fact that he is not able to use these characters and his password would be different than what he thought he had entered?
“Don’t worry if it doesn’t work right. If everything did, you’d be out of a job.” - Mosher’s Law of Software Engineering