User Access Restriction by account type (easy help?)

**morrie** · May 24, 2011 13:17

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:

PHP Code:


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:

PHP Code:


$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?

**Cute Tink** · May 24, 2011 16:54

If you are expecting only one row, don't use a while loop.

PHP Code:


$sql = mysql_query("SELECT usertype FROM user WHERE id='$userid'"); 
$row = mysql_fetch_array($sql);
$usertype = $row["usertype"];

Just as a point, it would be better to request the record based on the unique ID, because it is a simpler comparison and it is the index of the table.

**morrie** · May 24, 2011 16:55

I even thought to try it this way, which should be sufficient (and elegantly simple):

PHP Code:


<?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?

**morrie** · May 24, 2011 17:01

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?

**Cute Tink** · May 24, 2011 18:01

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.

**morrie** · May 25, 2011 08:17

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 Code:


<?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 Code:


<?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.\\nIf 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');

?>

**StarLion** · May 25, 2011 08:43

Kind of an odd way to handle the access control.

$uid = isset($_POST['uid']) ? $_POST['uid'] : $_SESSION['uid'];

What happens if neither $_POST or $_SESSION are set? (Hint: It would throw an E_WARNING for Undefined Index)

Why not...

PHP Code:


<?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.\\nIf 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.

**morrie** · May 25, 2011 08:48

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)?

**morrie** · May 25, 2011 09:00

StarLion,

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??)

**StarLion** · May 25, 2011 09:58

Originally Posted by morrie

StarLion,

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?

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.

PHP Code:


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.

**Cute Tink** · May 25, 2011 10:28

Originally Posted by morrie

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)?

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:

PHP Code:


$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:

PHP Code:


$uid = '';

if( isset($_POST['uid']) ) { 

   $uid =  $_POST['uid']; 

} 

elseif ( isset( $_SESSION['uid'] ) ) {  

   $uid = $_SESSION['uid']; 

}

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.

**StarLion** · May 25, 2011 10:49

Originally Posted by Cute Tink

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:

Well it's not just the future.

First page load. $_SESSION is empty. $_POST is empty. Error occurs.

**Cute Tink** · May 25, 2011 10:54

True enough. I didn't look far enough down the code to realize this was the login page as well.

**morrie** · May 25, 2011 10:57

OK, I understand what you are saying here, but the case of uid not being set is specifically handled in the code:

PHP 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

PHP Code:


<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).

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?

**StarLion** · May 25, 2011 11:08

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.

**Cute Tink** · May 25, 2011 11:08

It's the PASSWORD() function that he is recommending against. You should do something like this instead:

PHP Code:


$sql = "SELECT * FROM user WHERE userid = '$uid' AND password = '" . md5($pwd) . "'";

Or you can use SHA1():
PHP: md5 - Manual
PHP: sha1 - Manual

**morrie** · May 25, 2011 11:10

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.

I guess I would like to get that to work, and then deal with any bug-fixes later.
Original code and tutorial here:
Managing Users with PHP Sessions and MySQL Article » SitePoint

**morrie** · May 25, 2011 11:14

Thanks for the password help, Tink. I am looking into that now.

**Cute Tink** · May 25, 2011 11:25

PHP Code:


$_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.\\nIf 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:

PHP Code:


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.\\nIf 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.

**StarLion** · May 25, 2011 11:27

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:

PHP Code:


$usertype = mysql_result($result,0,'usertype');

at the bottom of the page, right below the $username = line.

**morrie** · May 25, 2011 13:30

StarLion,

I tried to find about the "IE submit bug" that you mention. Is it a security risk? I have in my contract that I no longer support ie6, so am I off the hook unless it's a security bug. Strangely I didn't get much fuel querying Google for the obvious keywords (with/without quotes)..

**StarLion** · May 25, 2011 14:45

It's not a security bug - IE has the bad habit that if you hit the enter key to submit your form, rather than push the button, it wont send the button's value through the form.

**morrie** · May 26, 2011 15:00

Thanks you for that, StarLion, I will look into that. (So you are saying this is one way to get that E_WARNING, you were talking about. Is that so bad? If I were the user, I would refresh and fill out again. I don't see any way to prevent this, other than ditch Kevin's elegant multi-purpose pattern).

In the meantime, I am happy to say that I have a system that works (your easy suggestion was all that I needed).

Now, in addressing security issues, I will implement SHA2 encryption, and "sanitize" my form inputs(do you have any favorite techniques?). (I will also move my final includes to a secure path (above the document root, with privileges 644). My sessions directory is already in same location, 644).

Is there anything else that would need to be done in terms of my queries to make this thing (at least 9mm) bullet-proof?

I decided to go with:

PHP Code:


$sql = "SELECT usertype,userid,password,fullname FROM user WHERE userid = '$uid' AND password = PASSWORD('$pwd')";

for my main sql query. Password issue aside, are there any security problems with this system that might need tweaking?

**Cute Tink** · May 26, 2011 18:02

You should always design your scripts so the user wouldn't have to refresh and fill it out again. Some people won't like that and will leave your site behind.

The partial script I posted above will prevent the E_WARNING from happening.

PHP Code:


$uid = '';

if( isset($_POST['uid']) ) { 

   $uid =  $_POST['uid']; 

} 

elseif ( isset( $_SESSION['uid'] ) ) {  

   $uid = $_SESSION['uid']; 

}

This won't generate the E_WARNING.

User Tag List

Thread: User Access Restriction by account type (easy help?)

Thread Tools

Display

User Access Restriction by account type (easy help?)

Bookmarks

Bookmarks

Posting Permissions