My login script...sketchy? please comment

Sneef

Ok, so my brother has already told me that my coding is hard to read... so no need to point that out unless you have some constructive pointers to give me 🙂

I guess my first question is ...Will this work well? Will it allow the user to stay logged in while changing page(on same site) ?

function sign_in()
	{
	echo "<form action='".$_SERVER['php_self']."' method='post'>";
	if ($_SESSION['user']):
		echo "You are currently signed in, ".$_SESSION['user'].".";
	elseif ($_POST['sign_in_name']):
		$users_row = mysql_query("SELECT * FROM users WHERE name='".$_POST['sign_in_name']."'");
		while ($users_row_object = mysql_fetch_object($users_row))
			{
			if ($_POST['sign_in_name'] and $_POST['sign_in_password'] == $users_row_object->password):
				echo "You are currently signed in, ".$users_row_object->name.".";
				$_SESSION['users_group'] = $users_row_object->group;
				$_SESSION['user'] = $users_row_object->name;
			endif;
			}
	else:
		echo "<p>You are not signed in.</p>
			  <input name='sign_in_name' type='text' width='12' /><br />
			  <input name='sign_in_password' type='password' width='12' /><br />
			  <input name='sign_in' type='submit' value='Sign in' />";
	endif;
	echo "</form>";
	}

PS: I'm not ready to implement the Logout part yet.

bradgrafelman

First of all, the array indeces are case-sensitive. 'php_self' does NOT equal 'PHP_SELF'. Thus, you need to change this:

$_SERVER['php_self']

to this:

$_SERVER['PHP_SELF']

in your script. That's the first obvious error I caught, there may be other errors lurking about...

EDIT: Also, you're assigning values to session variables... did you ever use [man]session_start/man to initiate the session?

Sneef

EDIT: Also, you're assigning values to session variables... did you ever use [man]session_start/man to initiate the session?

Yes, I did. It's in my connection to mysql script. Will these $_SESSION variables stay set until the browser is closed or session destroyed? or do I have to do something else to my script to have the session variables pass through the pages ?

AcousticJames

I believe (and I could be wrong) that a session lasts until the browser window is closed or after a "timeout" period of inactivity which is set (again, I think), in your php.ini file.

Please someone correct me if I'm wrong. I'm working on memory and haven't researched this answer.

James

justsomeone

Be very careful when using sessions as your record of whether or not a visitor is logged in. There is not a unique relationship between users and sessions.

For example, if you have a couple of people on a corprate (or school, or even internet cafe) network accessing your site, they may well be coming from behind the same proxy server. In this case, your server won't be able to tell one from the other.

You could end up in the situation where one user on a school/office/cafe network logs in as normal, and anyone else who logs on from the same network is assumed to be the same person and so get to be logged in, without ever authenticating themselves. I leave it up to you to work out the implications of that!

Sneef

I understand the risk, I just don't know how to prevent it besides giving a unique SID based on the (verified)username of the user.

justsomeone

The best answer for you depends entirely on what you're doing and just how secure you need to be.

Session vars - as you have discussed

session id's in the get/post vars - makes things more visible, but allows for multiple sessions form the same IP.

Cookies are slightly less visible than sesion id's but are still easy to hijack.

A combination of all of the above might be best?

Sneef

Would a redundant include (with a conditional hidden form containing the $_POST['SID']) be a secure and least visible solution?

I'm just afraid that even a hidden form can be looked at.

thorpe

i dont think justsomeone has it right (of course i may be wrong), session dont rely on ip's, so i dont see how someone from behind the same proxie could be confused with another user.

they rely on cookies. so the user would need to be at the same machine.

of course there is allot more to it... but.

from the manual;

A visitor accessing your web site is assigned an unique id, the so-called session id. This is either stored in a cookie on the user side or is propagated in the URL.

justsomeone

I stand corrected. It's been a while since I looked into session variables. thorpe is exactly right. Please ignore my previous contribution! 🙂

Sneef

So I should keep working with session variables. I don't want to use the url to pass the sid and i don't need the users to be "remembered" each time they visit, just be remembered between pages during the same visit. So, session variables would be the way to go I figure?