Jump to content


Check out our Community Blogs

Register and join over 40,000 other developers!


Recent Status Updates

View All Updates

Photo
- - - - -

User auth and ID questions

authentication

  • Please log in to reply
16 replies to this topic

#13 vedran

vedran

    CC Newcomer

  • Just Joined
  • PipPip
  • 18 posts

Posted 19 August 2008 - 06:38 AM

Look at your login.php file, particularly the mysql query. I'm not sure if this is it, but I never call php functions inside queries. I doubt this is it, but it could be the problem. Also try explicitly defining * in the query to return only row_id since you only require that value.

As for logging out, I don't think you want $_SESSION = array(); in your code, I've never used that and don't know why its there.

If you are still having problems, I will attempt to run it and look more closely at it.


I've tried removing $_SESSION, but somehow scr*wed it even more...
What do you mean by php functions inside queries? Do u mean md5 hash?

I'd really appreceate if you could run it, and really sorry for noobish questions and problems :)

Also the DB is

CREATE TABLE `omeragic_rebus`.`rebus_users` (
`id` MEDIUMINT( 8 ) UNSIGNED NOT NULL ,
`username` VARCHAR( 255 ) NOT NULL ,
`password` VARCHAR( 40 ) NOT NULL ,
`email` VARCHAR( 100 ) NOT NULL ,
`group_id` MEDIUMINT( 8 ) UNSIGNED NOT NULL
)

  • 0

#14 morefood2001

morefood2001

    CC Leader

  • Expert Member
  • PipPipPipPipPipPipPip
  • 1011 posts

Posted 19 August 2008 - 08:12 AM

The reason why your login doesn't work is because you are never submitting a variable by the name of login for $_GET to actually get. Removing that line worked fine. If you want to keep this, perhaps add ?logout=true to the end of logout.php.

The reason why your login doesn't work is because there is ALWAYS a result unless the database doesn't exist. Therefore you needed an additional if/else clause inside the main else. I also changed the md5 to be up in the real escape.

So here is my code, customized to fit my live testing database (you will need to change tables back). Also I did not fix the error outputting system, but it doesn't work correctly.

index.php

<?php
error_reporting(E_ALL | E_STRICT);
ini_set('display_errors', True);

session_start();
?>

<html>
<head>
<title>My login</title>
</head>
<body>
<div></div>
<?php if (isset($_SESSION['username'])) { ?>
You are now logged in <br />
<a href="logout.php">Logout</a>
<?php } else { ?>
<form action="login.php" method="post">
username: <input name="username" type="text" />
password: <input name="password" type="password" />
<input type="submit" />
</form>
<?php } ?>
<!-- Output Error -->
<?php if (in_array('error',$_SESSION)) echo $_SESSION['error']; unset($_SESSION['error']); ?>
</body>
</html>


login.php
<?php
session_start();

$db_host = 'localhost';
$db_user = 'matthous_test';
$db_pass = '*********';
$db_db = 'matthous_test';



if (isset($_POST['username']))
{
// MySQL Connection
$db_link = mysql_connect($db_host, $db_user, $db_pass)
or die('MySQL Connection Error:'.mysql_error());
mysql_select_db($db_db)
or die('MySQL Error: Cannot select table');

$username = mysql_real_escape_string($_POST['username']);
$password = mysql_real_escape_string(md5($_POST['password']));

// MySQL Query
$result = mysql_query("SELECT * FROM matthous_users
WHERE username = '$username' AND password = '$password' ");

if(!$result) {
$_SESSION['error'] = '<span style="color: red">Database not found.</span>';
} else {
// Mysql fetch row results
$row = mysql_fetch_assoc($result);

if ($row['username'] == $username){
$_SESSION['userid'] = $row['id'];
$_SESSION['username'] = $username;
$_SESSION['error'] = 'Login successful<br> Welcome, '.$username;
}else{
$_SESSION['error'] = '<span style="color: red">Login Failed</span>';
}

}
mysql_close($db_link);
}

header('Location: ./')
?>


logout.php
<?php
session_start();

$_SESSION = array();
if ($_COOKIE[session_name()])
{
setcookie(session_name(), '', time()-42000, '/');
}
session_destroy();
header('Location: ./');

?>

  • 1

#15 John

John

    CC Mentor

  • Moderator
  • 4450 posts
  • Location:New York, NY

Posted 19 August 2008 - 10:51 AM

Look at your login.php file, particularly the mysql query. I'm not sure if this is it, but I never call php functions inside queries.

The md5 function he is calling inside his query is actually a MySQL not php. However is logic is still skewed. He is calling the mysql_real_escape_string on a password prior to creating the md5 hash. That said, if the password contains special characters, they will be escaped and render the password incorrect. However, by removing that line, the query is vulnerable to an sql injection. I would chage the code to:


	$username = mysql_real_escape_string($_POST['username']);
$password = md5($_POST['password']);

// MySQL Query
$result = mysql_query("SELECT * FROM rebus_users WHERE username = '$username' AND password = '$password' ");
The extra mysql_real_escape_string in morefood2001's post is redundant since an md5 contains no characters that needs to be escaped. Although it doesn't hurt anything by including it (other than maybe a few clock cycles).

I would like to point out something morefood2001 said. mysql_query doesnt always return a result. According to the php manual if *should* return false if there is an error with the query. In light of that, does your query return an error or an empty result set? I believe it is the latter which is why your method is not the best.


However, two of our members have written tutorials retarding the creation of a login/logoff system. Both are great examples, but I would lean toward the latter as it uses a method more similar to mine (checking if the number of rows returned by mysql_query is one).
http://forum.codecal...off-system.html
http://forum.codecal...ain-script.html
  • 0

#16 morefood2001

morefood2001

    CC Leader

  • Expert Member
  • PipPipPipPipPipPipPip
  • 1011 posts

Posted 19 August 2008 - 11:57 AM

The extra mysql_real_escape_string in morefood2001's post is redundant since an md5 contains no characters that needs to be escaped. Although it doesn't hurt anything by including it (other than maybe a few clock cycles).


We discussed that after my post, I wasn't thinking correctly on that point. It is redundant.

I would like to point out something morefood2001 said. mysql_query doesnt always return a result. According to the php manual if *should* return false if there is an error with the query. In light of that, does your query return an error or an empty result set? I believe it is the latter which is why your method is not the best.


I worded my thought incorrectly, thanks for catching that.
  • 0

#17 Orjan

Orjan

    CC Mentor

  • Moderator
  • 2918 posts
  • Location:Karlstad, Sweden
  • Programming Language:C, Java, C++, C#, PHP, JavaScript, Pascal
  • Learning:Java, C#

Posted 20 August 2008 - 02:00 PM

I have worked a bit with this and have concluded to this part to be loaded on every page, if you want the system to log users off automatically after a certain time.

the session name thing is not necesary, but if there are many homepages within the same domain, it is safer to have it so anyone else within the same domain elaborates with your cookies. it does not take much execution time, and makes the code a little bit more secure.


$sessname = 'MyTinyApp';
$session_length=3600 // one hour, time is in seconds
session_set_cookie_params($session_length);
session_name($sessname);
session_start();

// Reset the expiration time upon page load.
// Without this part, the session will limit to
// one hour no matter how much access the user do.
if (isset($_COOKIE[$sessname])) {
setcookie($sessname, $_COOKIE[$sessname], time() + $session_length, "/");
}


In my version of the system, I always create the session cookie, but I never set any variables in $_SESSION until logged in properly.

hope this part can help aswell.
  • 0





Also tagged with one or more of these keywords: authentication

Recommended from our users: Dynamic Network Monitoring from WhatsUp Gold from IPSwitch. Free Download