-1

I have this login 'system' that has roles for 'users' and 'admin' where the user will be directed into a website with limited capabilities. And and admin where he/she will be directed to a separate web page and works fine. but I want to add another 'role'. here is my source code:

// attempt login if no errors on form
        if (count($errors) == 0) {
            $password = md5($password);

            $query = "SELECT * FROM users WHERE username='$username' AND password='$password' LIMIT 1";
            $results = mysqli_query($db, $query);

            if (mysqli_num_rows($results) == 1) { // user found
                // check if user is admin or user
                $logged_in_user = mysqli_fetch_assoc($results);
                if ($logged_in_user['user_type'] == 'admin') {

                    $_SESSION['user'] = $logged_in_user;
                    $_SESSION['success']  = "You are now logged in";
                    header('location: admin/home.php');       
                }else{
                    $_SESSION['user'] = $logged_in_user;
                    $_SESSION['success']  = "You are now logged in";

                    header('location: index.php');
                }
            }else {
                array_push($errors, "Wrong username/password combination");
            }
        }

Thank you!

  • 3
    I'd delete this code and start all over, following security practices. Don't use md5, use prepared statements – Rotimi May 21 '20 at 10:25
  • @Rotimi. noted. but this is still in development as I'm new in using PHP. but all I want to learn right now is how to add another role. – Francis Kane May 21 '20 at 10:32
  • 1
    **Warning:** You are wide open to [SQL Injections](https://stackoverflow.com/a/60496/1839439) and should use parameterized **prepared statements** instead of manually building your queries. They are provided by [PDO](https://php.net/manual/pdo.prepared-statements.php) or by [MySQLi](https://php.net/manual/mysqli.quickstart.prepared-statements.php). Never trust any kind of input! Even when your queries are executed only by trusted users, [you are still in risk of corrupting your data](http://bobby-tables.com/). [Escaping is not enough!](https://stackoverflow.com/q/5741187) – Dharman May 21 '20 at 10:38
  • 1
    **Never store passwords in clear text or using MD5/SHA1!** Only store password hashes created using PHP's [`password_hash()`](https://php.net/manual/en/function.password-hash.php), which you can then verify using [`password_verify()`](https://php.net/manual/en/function.password-verify.php). Take a look at this post: [How to use password_hash](https://stackoverflow.com/q/30279321/1839439) and learn more about [bcrypt & password hashing in PHP](https://stackoverflow.com/a/6337021/1839439) – Dharman May 21 '20 at 10:39
  • Delete this code and start over. Get a proper tutorial with prepared statements and password_hash. – Dharman May 21 '20 at 10:40

1 Answers1

0
/* Somewhere else in your lib functions. */
function checkUsername ($username)
{
    if ( strlen($username) >= 32 ) // I don't know your limits
        return false;

    // Others tests ...

    return true;
}

if (count($errors) == 0) {
    
    //$password = md5($password);

    //$query = "SELECT * FROM users WHERE username='$username' AND password='$password' LIMIT 1";
    //$results = mysqli_query($db, $query);

    /* You should not do this ! 
        1. md5() is deprecated, but if you are using it, it's still ok for learning purpose, but consider a better hash.
        2. You should not compare directly username and password in the query, prefer to retrieve data from the user,
        then test the password in PHP, it will add some extra security.
        Then use a function to filter your username, like checking the length and/or authorized characters.
        3. Use mysqli_real_escape_string(), prepared query is more reliable, automatic, ... But do the same.
        4. You should not use "SELECT * ..." form, but rather the fields you really needs.
    */

    if ( checkUsername($username) )
    {
        $query = 'SELECT * FROM users WHERE username = "'.mysqli_real_escape_string($db, $username).'" LIMIT 1;';

        $results = mysqli_query($db, $query);

        if (mysqli_num_rows($results) == 1) { // user found
            // check if user is admin or user
            $logged_in_user = mysqli_fetch_assoc($results);

            /* Check the password here. TODO: use other hash than md5() */
            if ( $logged_in_user['password'] === md5($password) )
            {
                //if ($logged_in_user['user_type'] == 'admin') {
                //  $_SESSION['user'] = $logged_in_user;
                //  $_SESSION['success']  = "You are now logged in";
                //  header('location: admin/home.php');       
                //}else{
                //  $_SESSION['user'] = $logged_in_user;
                //  $_SESSION['success']  = "You are now logged in";
                //  header('location: index.php');
                //}

                /* If you want more roles, just re-use $_SESSION['user']['user_type'] to define it. */

                $_SESSION['user'] = $logged_in_user;

                switch ($_SESSION['user']['user_type'])
                {
                    case 'admin':
                        header('location: admin/home.php');
                        break;

                    case 'my_new_role':
                        header('location: admin/new_role_index.php');
                        break;

                    // case 'whatever' : // For a other role

                    default:
                        header('location: index.php');
                        break;
                }

                $_SESSION['success'] = "You are now logged in";

                /* Always interrupt your script when you are doing a redirection, 
                 * to prevent some code below to continue the execution. */
                exit(0);
            }
            else
            {
                array_push($errors, "Wrong password !");
            }
        }
        else 
        {
            array_push($errors, "Unknown user !");
        }
    }
    else
    {
        array_push($errors, "Invalid username !");
    }
}