I am trying to do what I think should be quite simple, which is to restrict user by “account type”. I have setup a system based on Kevin Yank’s “Manage Users with PHP Sessions” tutorial. I have added a field in my working database table, called “usertype” and added it to the registration page (data gets inserted into database, verified by checking MySQLAdmin).
I might add that I am using “enum” and pre-defined values (a.b.c) for my usertypes. Here is my sql:
CREATE TABLE `user` (
`ID` int(11) NOT NULL auto_increment,
`userid` varchar(100) NOT NULL,
`password` char(16) NOT NULL,
`fullname` varchar(100) NOT NULL,
`email` varchar(100) NOT NULL,
`notes` text,
`usertype` enum('a','b','c') NOT NULL default 'b',
PRIMARY KEY (`ID`),
UNIQUE KEY `userid` (`userid`)
) ENGINE=MyISAM AUTO_INCREMENT=2 DEFAULT CHARSET=latin1 AUTO_INCREMENT=2 ;
So, on my restricted-access pages, I need to check if they are the proper user type, or let them know what the problem is. Here is my PHP:
$sql = mysql_query("SELECT usertype FROM user WHERE id='$userid'");
while($row = mysql_fetch_array($sql)){
$usertype = $row["usertype"];
}
if ($usertype == "a") {
$userOptions = "You get options for Normal User";
} else if ($usertype == "b") {
$userOptions = "You get options for Expert User";
} else {
$userOptions = "You get options for Super User";
}
?>
My user is registered as an “a” user, but I get the output: “You get options for a Super User” every time. I have tried to echo usertype in my HTML, but get no output (null). So, somehow, my retrieval script is not doing the job. Any help or suggestions?
I even thought to try it this way, which should be sufficient (and elegantly simple):
<?php include "accesscontrol.php";
include_once "db.php";
session_start();
if ($_SESSION['usertype'] != "b") {
echo "You are not the proper user type to view this page";
die();
}
?>
but it still doesn’t work. What am I missing? What should I try?
Thanks, Tink,
I thought I was ‘request[ing] the record based on the unique ID’. If not, how would that be done?
(Also, I would welcome your opinion on my second attempt)…which way do you think is best?
Threw me a bit on your query, because you have a userid column and your variable is $userid. Your unique ID will work as id, but you have it set up expecting a string. You shouldn’t surround your INT with single quotes.
$sql = mysql_query(“SELECT usertype FROM user WHERE id=$userid”);
And make sure it is an integer before putting it in the query.
The session method would also work, provided that you properly set up your session array. At some point, you are going to have to request the usertype from the database and set it before you check it.
Just as a note, you should put session_start(); at the top of the page, right after <?php, just in case you try to access the session in one of your includes at some point.
I am confused; I thought you could either get it from a stored session variable, OR retrieve it from the database. It seems that since my system is already tracking username and password, can’t it just “schlep” the usertype with along with those? And then I can just ask for it when I want to restrict that area.
Here is the protected page that I have been working with:
<?php
session_start();
include "accesscontrol.php";
include_once "db.php";
$sql = mysql_query("SELECT usertype FROM user WHERE id=$userid");
$usertype = mysql_query($sql);
echo $usertype;
if ($usertype !="b") {
echo "You are not the proper user type to view this page";
die();
}
?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<title> Member Area</title>
</head>
<body>
<h1 id="memberIndex">Welcome B Member!</h1>
</body>
</html>
and my accesscontrol.php (with important values changed):
<?php // accesscontrol.php
session_start();
include_once 'common.php';
include_once 'db.php';
$uid = isset($_POST['uid']) ? $_POST['uid'] : $_SESSION['uid'];
$pwd = isset($_POST['pwd']) ? $_POST['pwd'] : $_SESSION['pwd'];
$usertype = isset($_POST['usertype']) ? $_POST['usertype'] : $_SESSION['usertype'];
if(!isset($uid)) {
?>
<!DOCTYPE html PUBLIC "-//W3C/DTD XHTML 1.0 Transitional//EN"
"http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<title> Please Log In for Access </title>
<meta http-equiv="Content-Type"
content="text/html; charset=iso-8859-1" />
</head>
<body>
<h1> Login Required </h1>
<p>You must log in to access this area of the site. If you are
not a registered user, <a href="signup.php">click here</a>
to sign up for instant access!</p>
<p><form method="post" action="<?=$_SERVER['PHP_SELF']?>">
User ID: <input type="text" name="uid" size="8" /><br />
Password: <input type="password" name="pwd" SIZE="8" /><br />
<input type="submit" value="Log in" />
</form></p>
</body>
</html>
<?php
exit;
}
$_SESSION['uid'] = $uid;
$_SESSION['pwd'] = $pwd;
$_SESSION['usertype'] = $usertype;
dbConnect("my_db");
$sql = "SELECT * FROM user WHERE userid = '$uid' AND password = PASSWORD('$pwd')";
$result = mysql_query($sql);
if (!$result) {
error('A database error occurred while checking your '.
'login details.\\\
If this error persists, please '.
'contact webmaster.');
}
if (mysql_num_rows($result) == 0) {
unset($_SESSION['uid']);
unset($_SESSION['pwd']);
unset($_SESSION['usertype']);
?>
<!DOCTYPE html PUBLIC "-//W3C/DTD XHTML 1.0 Transitional//EN"
"http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<title> Access Denied </title>
<meta http-equiv="Content-Type"
content="text/html; charset=iso-8859-1" />
</head>
<body>
<h1> Access Denied </h1>
<p>Your user ID or password is incorrect, or you are not a
registered user on this site. To try logging in again, click
<a href="<?=$_SERVER['PHP_SELF']?>">here</a>. To register for instant
access, click <a href="signup.php">here</a>.</p>
</body>
</html>
<?php
exit;
}
$username = mysql_result($result,0,'fullname');
?>
What happens if neither $_POST or $_SESSION are set? (Hint: It would throw an E_WARNING for Undefined Index)
Why not…
<?php // accesscontrol.php
session_start();
include_once 'common.php';
include_once 'db.php';
dbConnect("my_db"); //Assuming i'm gonna be using this elsewhere
$uid = isset($_POST['uid']) { //Handle Login Attempt
$sql = "SELECT username,userid,usertype FROM user WHERE userid = '$uid' AND password = PASSWORD('$pwd')";
//PS: It's highly recommended you DONT use MySQL's Password function to store passwords!
$result = mysql_query($sql);
if (!$result) {
error('A database error occurred while checking your '.
'login details.\\\
If this error persists, please '.
'contact webmaster.');
}
if (mysql_num_rows($result) == 0) { //Should only reach here if credentials bad.
?>
<!DOCTYPE html PUBLIC "-//W3C/DTD XHTML 1.0 Transitional//EN"
"http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<title> Access Denied </title>
<meta http-equiv="Content-Type"
content="text/html; charset=iso-8859-1" />
</head>
<body>
<h1> Access Denied </h1>
<p>Your user ID or password is incorrect, or you are not a
registered user on this site. To try logging in again, click
<a href="<?=$_SERVER['PHP_SELF']?>">here</a>. To register for instant
access, click <a href="signup.php">here</a>.</p>
</body>
</html>
<?php
die();
} else {
list($_SESSION['username'],$_SESSION['userid'],$_SESSION['usertype']) = mysql_fetch_row($result);
}
//Login Attempt Handled. Now, if you like shorthand vars....
$uid = isset($_SESSION['uid']) ? $_SESSION['uid'] : '';
$username = isset($_SESSION['username']) ? $_SESSION['username'] : '';
$usertype = isset($_SESSION['usertype']) ? $_SESSION['usertype'] : '';
//Dont Store password in Session!
if(empty($uid) {
?>
<!DOCTYPE html PUBLIC "-//W3C/DTD XHTML 1.0 Transitional//EN"
"http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<title> Please Log In for Access </title>
<meta http-equiv="Content-Type"
content="text/html; charset=iso-8859-1" />
</head>
<body>
<h1> Login Required </h1>
<p>You must log in to access this area of the site. If you are
not a registered user, <a href="signup.php">click here</a>
to sign up for instant access!</p>
<p><form method="post" action="<?=$_SERVER['PHP_SELF']?>">
User ID: <input type="text" name="uid" size="8" /><br />
Password: <input type="password" name="pwd" SIZE="8" /><br />
<input type="submit" value="Log in" />
</form></p>
</body>
</html>
<?php
die();
}
?>
Also, you have no sanitizing on your input, which would be the next step.
Would it be easier just to use integers, instead of enum: a,b,c? I don’t see the advantage to either, other than integers seem much easier. Tink, it sounds like you are suggesting that I put the usertype in the database as an integer? (why wouldn’t it work the way I have it, with enum)?
Thanks a lot for your input. The code is closely adapted from Kevin Yank’s tutorial, here on Sitepoint (I did question earlier what might’ve changed in 5 years since that was written). In it, he stores password as PASSWORD in the SQL, and actually recommends it. What is the argument against it? What else should I be doing instead?
You said:
What happens if neither $_POST or $_SESSION are set? (Hint: It would throw an E_WARNING for Undefined Index)
but this is a ternary conditional. If the uid and pwd are already set, then it lets you in. Haven’t’ had any E_Warning errors…
So, I will try your suggestions (yes, sanitizing forms inputs is my last step: suggestions?) and see if I can get this to work.
(Maybe someone could nudge ol’ KY for an update to that tutorial??)
As far as the PASSWORD function, MySQL are the ones who say not to use it to store your own passwords (They suggest MD5() and SHA1() as alternatives, and then point out that these have been hacked, so recommend SHA2). Manual Link
You said:
but this is a ternary conditional. If the uid and pwd are already set, then it lets you in. Haven’t’ had any E_Warning errors…
Lets break it down a bit. I’m going to rewrite it in full-form instead of ternary, so i can comment a bit.
if( isset($_POST['uid']) ) { //We check if POST uid exists. No errors here.
$uid = $_POST['uid']; //It existed; great. No problem.
} else { // It didnt exist. Okay, so instead...
$uid = $_SESSION['uid'];
//But.. if $_SESSION['uid'] hasnt been set yet, this statement throws an E_WARNING because you're trying to retrieve a nonexistant index.
}
Now, it’s possible that $_SESSION[‘uid’] is being created in common.php or db.php without me seeing it; if they are, then this statement changes.
So, I will try your suggestions (yes, sanitizing forms inputs is my last step: suggestions?) and see if I can get this to work.
Sanitization is brought up just about every 2 days on this forum, might try a forum search first.
(Maybe someone could nudge ol’ KY for an update to that tutorial??)
I stand on the shoulders of giants - I do not poke them in the ear, lest I fall.
I don’t think changing the standard to integers would be easier at this point. You would have the same advantage as integers using an enum - you can check to make sure it is one of the few allowed possibilities and throw a default if it doesn’t match:
$allowed = array( 'a', 'b', 'c' );
if ( !in_array( $usertype, allowed ) ) {
$usertype = 'c'; // or whatever your default is
}
I thought you could either get it from a stored session variable, OR retrieve it from the database. It seems that since my system is already tracking username and password, can’t it just “schlep” the usertype with along with those?
Yes you can. However, what I meant before is that you have to make sure that you do add it into the session at some point, probably at login when you store the other information in the session.
I also want to echo Starlion on the check. Perhaps at this point, you would either have the $_SESSION or $_POST arrays storing the values you are looking for, but what if, in the future, you pass the value through $_GET? You will then see the error he is talking about. Or, what if you have an bug that it doesn’t get set in any of those arrays? Then you will have an error and you might be tracking it down for hours. Easier in the long run to do something like this:
Then your script just references $uid and regardless of what happens, the variable is set up, which prevents warnings from php trying to find something that doesn’t exist and all you have to figure out why $uid is coming back empty. Also, it would be easier to add in another check against an additional array that you might be passing the uid through.
OK, I understand what you are saying here, but the case of uid not being set is specifically handled in the code:
if(!isset($uid)) {
(This is unchanged from Kevin’s script).
So, Starlion, you aren’t even using the password variable for authentication? Just the user name and user type? (In my system user name and user id are same). If using a password isn’t recommended, then why have one? Doesn’t whatever you replace it with become just as vulnerable? If storing a password in a session is not recommended, then why use sessions for access control? (What would be the alternative?)
I have tried your approach to the code, and it gives me "Access denied, without a means by which to log in (for some reason the
<a href="<?=$_SERVER['PHP_SELF']?>"
does not work (no form). Probably because the accesscontrol is used as an include, and somehow I am already on my target URL (this part is weird). In the of uid not being set { the form should be displayed.
By the way, the ‘db.php’ file is the connect script, and the ‘common.php’ is the error handler (a single .js function).
[SIZE=“1”]disclaimer: no offense was intended to anyone, especially not Kevin Yank. I did not by any means intend to “poke” anyone anywhere. (nudge was my term - realizing current Sitepoint CTO might not have time for such things).
My point remains: The tutorial (authored originally in 2003?) was upadated at some point, maybe it’s time again?[/SIZE]
I dont -store- the password anywhere - once the user has logged in, either successfully or not, i dont need his password anymore - he’s already authenticated himself to me, I dont need to authenticate him again until his session expires (at which point the password saved in session will be gone anyway)
As far as the <?=$_SERVER[‘PHP_SELF’]?> not working… try making it <?php echo $_SERVER[‘PHP_SELF’]; ?> … your server might have short tags disabled.
I feel that we have been side-tracked from the original task: successfully tracking and checking the usertype variable. I think that the looping, multi-purpose layout of this page takes care of any case where uid is not set, because you will be directed to the form in every other case. The page reloads itself, so that’s why checking _POST or _SESSION makes so much sense. It seems like a close loop to me. I guess I am just not seeing this as a problem, since I have tested and it work just fine, I just can’t seem to customize it with my own variable.
$_SESSION['uid'] = $uid;
$_SESSION['pwd'] = $pwd;
$_SESSION['usertype'] = $usertype;
dbConnect("my_db");
$sql = "SELECT * FROM user WHERE userid = '$uid' AND password = PASSWORD('$pwd')";
$result = mysql_query($sql);
if (!$result) {
error('A database error occurred while checking your '.
'login details.\\\
If this error persists, please '.
'contact webmaster.');
}
I think this is a little out of order. You wouldn’t want to store the values in the session until you have checked them by the database. After you have checked them through the database, then you should be safe storing the usertype and uid in the session:
dbConnect("my_db");
$sql = "SELECT * FROM user WHERE userid = '$uid' AND password = '" . md5($pwd) . "'";
$result = mysql_query($sql);
if (!$result) {
error('A database error occurred while checking your '.
'login details.\\\
If this error persists, please '.
'contact webmaster.');
}
else {
$row = mysql_fetch_assoc( $result );
$_SESSION['uid'] = $uid;
$_SESSION['usertype'] = $row['usertype'];
}
Here you use the password for authentication, then you can don’t need it again until the user needs to log in again, so I dropped it from the session group.
Wow. Yeah. Okay, I’m going to do what I said I shouldn’t.
That tutorial needs updating.
I see why he did things in the order he did - he’s trying to strictly follow his diagram. Translating it literally into code leads to the confused around-your-head-to-your-elbow sequence, but for learning purposes it will work. Sort of. I guess.
The IE Submit Bug is lurking around Kevin’s script there waiting for that to go wrong. ( isset($_POST[‘submittok’]) )
Rudy would smack my wrist if i did that query (SELECT * and then use mysql_result to pull a single cell)
That said; if you want to follow this code strictly, then the code would simply be to place:
$usertype = mysql_result($result,0,'usertype');
at the bottom of the page, right below the $username = line.