Login Script Problem - Dosn't recognize invalid user/pass

[camowel]

I've been working on the login system for my site for a while now, it was working for a while but seems to have stopped after I fixed it to get all the user's info set properly, it no longer recognizes invalid username/password combos

Here is the login function that handles the username and password from a form:

<?php
	function login($user, $pass) {
		global $db;
		$sql = "SELECT `uid`, `username` FROM `users` WHERE `username`='$user' AND `password`=SHA1('$pass')";
		$q = $db->query($sql);
		if ($q->numrows != 1) {
			$this->error_message("Invalid username or password!");
			header('Location: login.php');
		}
		$info = $q->data[0];
		$hash = SHA1($info['username']);
		setcookie('login', $hash, time() + 60*60*24*7);
		$sql_cookie_delete = "DELETE FROM `cookies` WHERE `uid` = '$info[uid]'";
		$sql_cookie = "INSERT INTO `cookies` (`uid`, `hash`) VALUES ('$info[uid]', '$hash')";
		$db->query($sql_cookie_delete);
		$db->query($sql_cookie);
	}
?>

I'm not sure why it won't work, $q->numrows contains the number of rows that the query returned, $this->error_message() sets the error message so that it can be displayed on the next page

Please help if you can this has been bothering me for a while now and I can't really work much on my site without having a solid user system running

Thanks for reading, and please ask for any more info you might need to help!

[thorpe]

this function doesn't return any result, so, my question is.... what happens when you feed it a username password combo?

[halojoy]

// to ensure, in case you get 2,3,4 from DB
if ($q->numrows < 1) {
              $this->error_message("Invalid username or password!");
              header('Location: login.php');
          }

This is one of them cases
where we like to see the php script that uses the function
( thorpe also thinks so, what I can understand)
not only a declaration of a function
so,
if you do not get this, very easy to solve, problem resolved
it is 99% beacause you didnt 'ask right' - lacking too much info about problem

the swede
🆒 :p 😕

[Weedpacket]

So if the username/password combination is invalid,

Originally posted by camowel

		if ($q->numrows != 1) {
			$this->error_message("Invalid username or password!");
			header('Location: login.php');
		}

[/B]

And then after the header() call, in the absence of any reason to do otherwise, PHP then goes on to evaluate the rest of the function.

[camowel]

This function isn't really supposed to return a result, the point is to set a cookie, and update a few mysql rows

then when the user goes to another page, they are logged in by a function at the top of that script

your right... definately didn't 'ask right' this time, this login system is the oddest i've ever written, but I'm trying to make it 'portable' so I can use it again and again 😉

Here is the script that uses it:

<?php
require_once('setup.php');

if (!isset($_POST['username']) || (!isset($_POST['password']))) {
	$_SESSION['user']->error_message("Please enter a username and password!");
	header('Location: login.php');
}

$_SESSION['user']->login(addslashes($_POST['username']), addslashes($_POST['password']));

header('Location: '.$_POST['redirect']);

?>

THe post values:
- username is the username
- password is the password
- redirect is the page the user was on before they were asked (or decided) to log in

Here is another function that may be causing the problem, I didn't really think it could be this one originally, but it may be:

<?php
	function checkcookie() {
		if (!empty($_COOKIE['login']) && $this->login != TRUE) {
			global $db;
			$hash = $_COOKIE['login'];
			$sql_uid = "SELECT `uid` FROM `cookies` WHERE `hash`='$hash'";
			$q_uid = $db->query($sql_uid);
			$uid = $q_uid->data[0]['uid'];
			$sql_user = "SELECT * FROM `users` WHERE `uid`='$uid'";
			$q_user = $db->query($sql_user);
			if ($q_user->numrows != 1) {
				$this->error_message("Error loggin you in!");
				header('Location: index.php');
			}
			$info = $q_user->data[0];
			$this->username = $info['username'];
			$this->email = $info['email'];
			$this->uid = $uid;
			$this->login = TRUE;
			$sql_access = "SELECT `access` FROM `ranks` WHERE `rid`='$info[rank]'";
			$q_access = $db->query($sql_access);
			$this->access = $q_access->data[0]['access'];
			setcookie('login', $_COOKIE['login'], time()+60*60*24*7);
		}
	}
?>

This function is called at the start of every page, it is part of my user object that I use to track users who are on the site

Just in case this might help, this is sort of the theory as to how this is supposed to work

• Username and password taken through a form

• call the login() function from the script that gets the form data

• the login function will set a cookie ('login') if the user can be logged in, the cookie is a hash (SHA1)

• the hash value is also put into mysql along with the username

• when the checkcookie() function is called, if the cookie is set, and the user is not already logged in:

• the script uses the hash value from the cookie to find the user id from mysql

• using the id, the script selects the row of user information

• using the user information from the database, the script sets some variables

• and the user is considered 'logged in'

Not sure if its working exactly like I planned (of course, its not working)

Something else, that is annoying me, is that it will only recognizes one of the accounts I try to use to login (my admin account) the other test users I have created can't seem to log in, I've got a feeling that these two problems are somehow (annoyingly) related

I tried your suggestion with using '<' instead of '!=' (halojoy) but it didn't seem to make a difference, and, it would acctually be a possible bug in the script (I think) since the username field in the database is a Primary Key, it shouldn't be possible for it to have more than one of the same username, and IF there is more than one username/password set that is exactly the same, there is most likely a problem with the way everything is working and could log the wrong user in. That is of course just the way I thought it would end up... But I'm usually wrong anyways 😉

And after the header() call (mentioned in Weeedpacket's post) there should be no reason for it to run the rest of the script, as the user is being redirected away from that page and it won't (shouldn't) run, this is one of the most confusing things 😉

Thank you very much for the replies!