Page 2 of 2

Re: How to better login script

Posted: Sat Jan 08, 2011 10:03 am
by Grinsa
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

Posted: Sun Jan 09, 2011 9:03 am
by MichaelR
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:

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!";
   }

 }
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) < 8 || strlen($value) > 32)  {
      $errorMsg[] = "Your password must be between 8 and 32 characters long!";
    }

 }

Re: How to better login script

Posted: Sun Jan 09, 2011 11:18 am
by Grinsa
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>&nbsp;</td>
<td>&nbsp;</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';
  }
 }
?>

Re: How to better login script

Posted: Sun Jan 09, 2011 1:53 pm
by greyhoundcode
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

Posted: Mon Jan 10, 2011 5:52 am
by MichaelR
Grinsa, remember to add a semi-colon (;) after "global $errorMsg".

Re: How to better login script

Posted: Mon Jan 10, 2011 12:25 pm
by Grinsa
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?

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!";
  }
 }
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

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");
	}
This is the page that the user is sent to: 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
	}
?>
The message only reads "Welcome !" when I would like for it to include the name.
This is logout.php

Code: Select all

<?php
session_start();
session_regenerate_id();

$_SESSION['login'] = array();
header("location:index.html")
?>
Thanks for all the help! it's almost complete now and i've learned a fair bit along the way!

Re: How to better login script

Posted: Mon Jan 10, 2011 1:58 pm
by greyhoundcode
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:
Could it be that you need to replace $_SESSION['username'] with $_SESSION['login']['username']?

Re: How to better login script

Posted: Mon Jan 10, 2011 3:24 pm
by social_experiment
Grinsa 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?
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 ;)

Re: How to better login script

Posted: Mon Jan 10, 2011 4:11 pm
by Grinsa
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 ;)
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?
greyhoundcode wrote:Could it be that you need to replace $_SESSION['username'] with $_SESSION['login']['username']?
That worked! Thanks.

Re: How to better login script

Posted: Tue Jan 11, 2011 12:19 am
by social_experiment
Grinsa 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?
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 function ;) ). Users are fickle things, you have to spoonfeed them.