1

I've created a column named 'permissions' in my database table with the value 1 for administrator and 2 for the regular users. The problem is whenever I log in with a normal user, it keeps redirecting me to the admin's page.

<?php
$con=mysql_connect('localhost','root','') or die(mysql_error());
mysql_select_db('user_registration') or die("cannot select DB");


if(isset($_POST["login"])){

    if(!empty($_POST['user']) && !empty($_POST['pass'])) {

        $user = strip_tags($_POST['user']);
        $pass = strip_tags($_POST['pass']);


        $query=mysql_query("SELECT * FROM users WHERE username='".$user."' AND password='".$pass."'");
        $numrows=mysql_num_rows($query);

        $permissions= "SELECT username FROM users WHERE permissions = '1'";
        $result=mysql_query($permissions);


            if($numrows!=0) {
                while($row=mysql_fetch_assoc($query)) {
                    $dbusername=$row['username'];
                    $dbpassword=$row['password'];
                }

                if($user == $dbusername && $pass == $dbpassword) {
                    session_start();
                    $_SESSION['sess_user']=$user;
                        if (mysql_num_rows($result) == 1 ) {
                            header("Location: fullDB.php");
                        } else { 
                            header("Location: member.php");
                        }
                }

            } else {
                echo "Invalid username or password!";
            }

    } else {
        echo "All fields are required!";
    }
}
?>
Sergiu Turus
  • 27
  • 1
  • 1
  • 8
  • 2
    Please note that `mysql_*` is now deprecated as of PHP7 because of security issues. It is suggested that you switch to `mysqli_*` or `PDO` extensions. – Derek Pollard Apr 20 '16 at 15:08
  • 1
    [Little Bobby](http://bobby-tables.com/) says [your script is at risk for SQL Injection Attacks.](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php). Even [escaping the string](http://stackoverflow.com/questions/5741187/sql-injection-that-gets-around-mysql-real-escape-string) is not safe! – Jay Blanchard Apr 20 '16 at 15:10
  • 1
    Please [stop using `mysql_*` functions](http://stackoverflow.com/questions/12859942/why-shouldnt-i-use-mysql-functions-in-php). [These extensions](http://php.net/manual/en/migration70.removed-exts-sapis.php) have been removed in PHP 7. Learn about [prepared](http://en.wikipedia.org/wiki/Prepared_statement) statements for [PDO](http://php.net/manual/en/pdo.prepared-statements.php) and [MySQLi](http://php.net/manual/en/mysqli.quickstart.prepared-statements.php) and consider using PDO, [it's really pretty easy](http://jayblanchard.net/demystifying_php_pdo.html). – Jay Blanchard Apr 20 '16 at 15:10
  • 1
    Please use PHP's [built-in functions](http://jayblanchard.net/proper_password_hashing_with_PHP.html) to handle password security. If you're using a PHP version less than 5.5 you can use the `password_hash()` [compatibility pack](https://github.com/ircmaxell/password_compat). Make sure that you [don't escape passwords](http://stackoverflow.com/q/36628418/1011527) or use any other cleansing mechanism on them before hashing. Doing so *changes* the password and causes unnecessary additional coding. – Jay Blanchard Apr 20 '16 at 15:10
  • 1
    Also, your code is making 2 query calls where a single query would suffice. It is also worth mentioning that your code is HIGHLY susceptible to SQL injection – Derek Pollard Apr 20 '16 at 15:10
  • NVMIND my code is working. Thank you guys & The Arrow! – Sergiu Turus Apr 20 '16 at 16:02

2 Answers2

6

You make all users admins because you're doing the admin check totally wrong:

    $permissions= "SELECT username FROM users WHERE permissions = '1'";

This query isn't "is this user an administrator". It's "Is there an administrator user in the database". If you have at least one admin user, then ALL users will be treated as admins.

Since you've already fetched the user details with the first query, the second query is totally redundant:

$query=mysql_query("SELECT * FROM users WHERE username='".$user."' AND password='".$pass."'");

$row = mysql_fetch_assoc($query);
if ($row['permissions'] == 1) { 
  ... admin user ...
} else {
  ... regular peon ...
}

And note that you're undoubtedly vulnerable to sql injection attacks, and are using an obsolete/deprecated DB library. the mysql_*() functions should NOT be used any more.

Marc B
  • 356,200
  • 43
  • 426
  • 500
1

The problem is with your query. Let's take a look a closer look:

SELECT username FROM users WHERE permissions = '1'

What you're saying here is, give me only usernames from the table of users where the permissions attribute is equal to 1. Let's assume you have one administrator and one normal user in you table with permissions attribute equal to 1 and 2 respectively. This query will always return you one row, and you're checking if the number of rows equals 1. You're not checking the type of user, only "if there's an administrative user".

I hope this helps. Please be aware that you're code is vulnerable to SQL Injection.