Also, mysqli_query will only return false if the query fails, not if it returns zero rows.
Use SELECT COUNT(*) AS count FROM … and check whether count is zero.
$check = mysqli_query($link, "SELECT username, password FROM members WHERE username = '$username' AND password = '$password'");
$result = mysqli_num_rows($check);
if ($result != 1)
and it seems to work… is that acceptable code though? I had trouble figuring out how to implement COUNT
going to bump this instead of making a new thread, just wondering if someone can tell me if my code is ok.
I added some remember-me and md5 stuff, and it all seems to work fine, but not sure if I am setting sessions/cookies correctly - if someone could confirm that I am doing it right or wrong that would be great. Here is my index.php code:
I have no idea what you have in your safe() function, but whole approach is wrong.
There is 2 rules that must be followed unconditionally.
No “safe” function must be applied to the password.
Any string literal that goes to the query, must be prepared using mysql_real_escape_string function()
Another most important issue.
You should align your code properly.
Code indention were invented not for beauty!
But to let programmer understand what’s going on.
Once I look into your code, I cant.
Next. Each location header must be followed by exit statement, to stop further execution.
Less important issue.
Why do you double your code?
Programming itself stands for eliminating repeated code fragments.
session_start() needs to be at the beginning of the srcipt or $_SESSION won’t exist, and you won’t have access to $_SESSION[‘username’]
The script doesn’t stop because you used header(‘Location: someurl’) it continues to execute, so it is a good idea to exit after a redirect.
You have $result = 1 in your conditions, this will always evaluate to true as you are simply assigning 1 to $result.
With remember me, you need to set a token in the cookie and in the database (similar to a password check), and check these are equal, otherwise someone can just send a cookie with anybody’s username in it and get access to their account.
thank you both. I have cleaned it up a bit, and it works great.
added exit to all headers and cleaned up my doubled code. also, ‘safe’ = mysqli_real_escape_string. but I don’t need to apply that to $_POST[‘password’]?
only thing I’m confused about is this:
“With remember me, you need to set a token in the cookie and in the database (similar to a password check), and check these are equal, otherwise someone can just send a cookie with anybody’s username in it and get access to their account.”
my remember me is working now, but I guess I still need to do this.
do I just add a column to my users table and adjust that column according to whether or not they have selected remember me? I guess I also need to modify my setcookie somehow? currently it is like this:
but I don’t need to apply that to $_POST[‘password’]?
No.
It has nothing to do with $_POST[‘password’]
It is used to escape some characters in the string that goes to the query. It is $mix that goes to the query. So function should be applied to the $mix variable.
o I just add a column to my users table and adjust that column according to whether or not they have selected remember me?
Most likely.
Put there a string which is similar to your $mix. Say, md5($username.$password . ‘myremembersalt’);
and put the same into cookie.
or would it be safe to do without the database insert, just put a md5’d token in the cookie instead of a simple $username would be ok?
also, since I’d be md5ing username+password+mysalt, if the username and/or password exceeds 32 chars does that matter, will md5 always produce a 32 char string regardless of length of the string?
just md5 token without a database is not an option.
Because you cannot create a query to check.
So, you can either put this token into database for the further checking, or set another cookie, with user id, for the same purpose
