-2

This is my first post.

I am trying to check with my MySQL Database to see if the user is a admin or a regular user, it always says that I am an admin. My database is:

id name email password role

I have a test user which is: id: 1 name: admin email: admin@admin.com role: 2

My code is:

<?php if (isset($_SESSION['usr_id']) || $row["role"] = 1) { ?>
<!DOCTYPE html>
<html>
<head>
<h2>You are a admin!</h2>
</head>
</html>
<?php } else { ?>
<h2>You are either not logged in, or you have no access to this page.</h2>
<?php } ?>

However, it always says that I am a admin even if my role is 2!

We__Create
  • 5
  • 1
  • 5
  • 1
    **WARNING**: Writing your own access control layer is not easy and there are many opportunities to get it severely wrong as you've so aptly demonstrated here. Please, do not write your own authentication system when any modern [development framework](http://codegeekz.com/best-php-frameworks-for-developers/) like [Laravel](http://laravel.com/) comes with a robust [authentication system](https://laravel.com/docs/5.2/authentication) built-in. – tadman May 22 '16 at 00:08
  • @tadman, maybe it's just for practice/learning :) – Chris May 22 '16 at 00:09
  • @Chris Learn from the experts by using a framework and seeing how it works, reading the source. Don't fumble around in the dark. – tadman May 22 '16 at 00:09
  • @tadman, everyone has their way of learning. Jumping into a framework right away might be overwhelming for some. This looks like a simple beginners app. I doubt think OP would use this for production, but what do I know? What kind of beginner "reads source" anyway?? – Chris May 22 '16 at 00:13
  • @Chris What is it with the PHP community that thinks it's acceptable to avoid using frameworks for as long as possible? It's not. PHP is lucky to have many *excellent* frameworks. **Use them**. Stop insisting that you have to take the hard road here. That's preposterous. Laravel has excellent documentation, is very beginner friendly, and will get you up to speed *faster* than raw PHP. – tadman May 22 '16 at 00:21
  • 1
    @tadman PHP does have many excellent frameworks indeed. But I disagree with your premise that beginners should start using a framework before they even get comfortable with the language itself. Learn the fundamentals of language itself first (e.g difference between `||` and `&&` and diff. between `=` and `==`), then learn a framework. But that's just my personal advice and opinion. – Chris May 22 '16 at 00:42
  • @Chris I'm not disagreeing that you must learn the syntax of the language, but learn it in *conjunction* with a framework that helps you out. What's happening in this question is writing an access control system: Crazy hard to get right, super easy to get so wrong it allows people to hack your site. That is the worst way to learn. Python people encourage Django, Ruby has Rails, PHP often says "Eh, just do it the old way." – tadman May 22 '16 at 00:45
  • @tadman Alright. Fair enough. – Chris May 22 '16 at 00:47

4 Answers4

5

$row["role"] = 1 this is assignment, not comparison. Go for $row["role"] == 1

michaJlS
  • 2,465
  • 1
  • 16
  • 22
  • 2
    And in addition to that, you need to replace the || on the first line by &&, otherwise `isset($_SESSION['usr_id'])` by itself will be enough to enter the `if`, but that condition should be true for just any user – Leo supports Monica Cellio May 22 '16 at 00:11
0

you're using the OR operator (||), the operator verifies compliance any of the 2 conditions are indicating in the IF statement, for this reason will display the message: "You are a admin!" when any of the 2 conditions is met. If you want the 2 conditions are met to show you that message must use the logical AND (&&) to verify the validity of the 2 conditions within your IF statement.

if (isset ($_SESSION ['usr_id']) && $row ["role"] = 1) { ... }

0

There must be 1 in session. Try again after destroy the session.

Also change the condition

FROM : <?php if (isset($_SESSION['usr_id']) || $row["role"] = 1) { ?>

TO : <?php if ((isset($_SESSION['usr_id']) && $_SESSION['usr_id'] == 1) || $row["role"] == 1) { ?>

Muhammad Azeem
  • 1,129
  • 1
  • 12
  • 16
0

Ok, it seems you are assigning the value instead of comparing it. That is the reason why you always log in as an administrator.

Just change $row["role"] = 1 by $row["role"] == 1. Furthermore, you should use the && operator instead of ||.

Finally, it seems you are learning programming for the first time, and these are typical mistakes that everyone who is programmer had at the beginning. I recommend you the book "PHP: A Beginner's Guide", by Vikram Vaswani, which is an excellent and easy guide for those who are learning to program in PHP.

Good luck.

Fabrizio Valencia
  • 19,299
  • 4
  • 16
  • 19