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:

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?

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
—
2011-05-25T01:01:13Z —
#5

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
—
2011-05-25T15:17:22Z —
#6

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.

Also, you have no sanitizing on your input, which would be the next step.

morrie
—
2011-05-25T15:48:40Z —
#8

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
—
2011-05-25T16:00:31Z —
#9

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
—
2011-05-25T16:58:57Z —
#10

morrie said:

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.

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
—
2011-05-25T17:28:53Z —
#11

morrie said:

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:

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.

StarLion
—
2011-05-25T17:49:20Z —
#12

Cute_Tink said:

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
—
2011-05-25T17:54:03Z —
#13

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

morrie
—
2011-05-25T17:57:01Z —
#14

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

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
—
2011-05-25T18:08:31Z —
#15

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['PHPSELF']?> not working.. try making it <?php echo $SERVER['PHPSELF']; ?> ... your server might have short tags disabled.

Cute_Tink
—
2011-05-25T18:08:40Z —
#16

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

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

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
—
2011-05-25T18:27:38Z —
#20

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: