different user levels PHP

Question

I've made a login script which is working (see code below)

error_reporting(E_ALL);
ini_set('display_errors', 1);
session_start();
$db = mysqli_connect("localhost", "root", "ovlovw8", "reparatie");
if (isset($_POST['submit'])) {
    $username = mysqli_real_escape_string($db, $_POST['username']);
    $password = mysqli_real_escape_string($db, $_POST['password']);
    $password = md5($password);
    $sql = "SELECT * FROM users WHERE username='$username' AND   password='$password'";
    $result = mysqli_query($db, $sql);

    if (mysqli_num_rows($result) == 1) {
        $_SESSION['message'] = "je bent ingelogd";
        $_SESSION['username'] = $username;
        header("location: overzicht.php");
    } else {
        $_SESSION['message'] = "verkeerde inlog gegevens";
    }
}

but now i want to add multiple user levels and i've made a script for that aswell but it doesnt work.

error_reporting(E_ALL);
ini_set('display_errors', 1);
session_start();
$db = mysqli_connect("localhost", "root", "ovlovw8", "reparatie");
if (isset($_POST['submit'])) {
    $username = mysqli_real_escape_string($db, $_POST['username']);
    $password = mysqli_real_escape_string($db, $_POST['password']);
    $password = md5($password);
    $sql = "SELECT * FROM users WHERE username='$username' AND    password='$password'";
    $result = mysqli_query($db, $sql);

    if (mysqli_num_rows($result) == 1) {
        switch ($role) {
            case '1':
                $_SESSION['message'] = "je bent ingelogd";
                $_SESSION['username'] = $username;
                $redirect = 'repairs.php';
                break;
            case '2':
                $_SESSION['message'] = "je bent ingelogd";
                $_SESSION['username'] = $username;
                $redirect = 'lloverzicht.php';
                break;
            case '3':
                $_SESSION['message'] = "je bent ingelogd";
                $_SESSION['username'] = $username;
                $redirect = 'overzicht.php';
                break;
        }
        header('Location: '.$redirect);
    }
}

its basically the same code but it doesnt redirect to the next page.

I hope someone has a suggestion or knows what im doing wrong.

*"it doesnt work"* isn't very descriptive of your problem. Does it instead delete the user? Color the background red? Or maybe crashed with an error message? You see where I'm going with this. ;) — domsson, Mar 07 '17 at 14:12
yeah i figured that out but my problem is that it doesnt redirect but it should. — brokkosnarf, Mar 07 '17 at 14:13
Where does `$role` come from? And http://stackoverflow.com/questions/401656/secure-hash-and-salt-for-php-passwords — jeroen, Mar 07 '17 at 14:15
Your script is at risk of [SQL Injection Attack](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) Have a look at what happened to [Little Bobby Tables](http://bobby-tables.com/) Even [if you are escaping inputs, its not safe!](http://stackoverflow.com/questions/5741187/sql-injection-that-gets-around-mysql-real-escape-string) Use [prepared parameterized statements](http://php.net/manual/en/mysqli.quickstart.prepared-statements.php) — RiggsFolly, Mar 07 '17 at 14:15
When you debug this, what specifically is happening and how specifically is it failing? If it "doesn't redirect", what *does* it do? What is the server's response? What's in the error logs? Have you turned on error reporting? When you debug this, does it get to the redirect code at all? — David, Mar 07 '17 at 14:15
Please dont __roll your own__ password hashing. PHP provides [`password_hash()`](http://php.net/manual/en/function.password-hash.php) and [`password_verify()`](http://php.net/manual/en/function.password-verify.php) please use them. And here are some [good ideas about passwords](https://www.owasp.org/index.php/Password_Storage_Cheat_Sheet) If you are using a PHP version prior to 5.5 [there is a compatibility pack available here](https://github.com/ircmaxell/password_compat) — RiggsFolly, Mar 07 '17 at 14:16
Some sensible code indentation would be a good idea. It helps us read the code and more importantly it will help **you debug your code** [Take a quick look at a coding standard](http://www.php-fig.org/psr/psr-2/) for your own benefit. You may be asked to amend this code in a few weeks/months and you will thank me in the end. — RiggsFolly, Mar 07 '17 at 14:16
***You really shouldn't use [MD5 password hashes](http://security.stackexchange.com/questions/19906/is-md5-considered-insecure)*** and you really should use PHP's [built-in functions](http://jayblanchard.net/proper_password_hashing_with_PHP.html) to handle password security. Make sure 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, Mar 07 '17 at 14:17
Doing your own password hashing isn't the end of the world if you know what you are doing. I have my own hashing library making use of the blowfish algorithm. Anyway I digress, what is the output of `$role` - That's where I would start debugging. — Dan_, Mar 07 '17 at 14:17
The vast majority, as evidenced by the code they post, do not know what they are doing @JokerDan — Jay Blanchard, Mar 07 '17 at 14:18
@JokerDan are you a cryptologist? http://security.stackexchange.com/questions/18197/why-shouldnt-we-roll-our-own — ʰᵈˑ, Mar 07 '17 at 14:18
Maybe, you never know... I can see that OP is also so new to PHP that they don't need to be bombarded with so many more advanced things right now, I just hope this isn't a live site. I have done freelance work securing sites with scripts worse than OP's to be fair. — Dan_, Mar 07 '17 at 14:20
You are _quite simply_ not `fetch`ing the result of your query!! So I am guessing that `$role` should be **but is not** being populated from the result of you query — RiggsFolly, Mar 07 '17 at 14:23
@DaanSlurink in the if statement add `while($row = mysqli_fetch_assoc($result)) { $role = $row['role']; }` — Masivuye Cokile, Mar 07 '17 at 14:25
...and given they do have a column named `role` with 1, 2 or 3 as values. — Funk Forty Niner, Mar 07 '17 at 14:30
They probably do not want to do a `while` loop @MasivuyeCokile as the role should just be assigned for the one user. They should just fetch the row `$row = mysqli_fetch_assoc($result);` — Jay Blanchard, Mar 07 '17 at 14:30
@JokerDan if they are new ***and serious about progressing*** then yes, they need to be bombarded with a lot of information about the ways they could shoot themselves in the foot and possibly screw over every single one of their end users. — castis, Mar 07 '17 at 14:31
@JayBlanchard yes that will work best, there's no need to loop through — Masivuye Cokile, Mar 07 '17 at 14:33
@castis im fairly new to programming but im in college and im doing an internship currently and i havent been told anything about secure code yet. — brokkosnarf, Mar 07 '17 at 14:35
*"i havent been told anything about secure code yet"* - Do schools/colleges wait until it's too late? The things they teach in schools and what they don't teach. This is 2016, not the early 90's. A lot of water's gone under the bridge in over 30 years. — Funk Forty Niner, Mar 07 '17 at 14:42
well @Fred-ii- some schools / colleges just dont care, I finished my course 3 years back, not a single lecture talked anything about prepared statements, had to figure that by reading and practising — Masivuye Cokile, Mar 07 '17 at 14:48
If teachers and professors are not talking about security from day one, they're doing it wrong. Challenge them. They're teaching sloppy and dangerous coding practices which students will have to unlearn later. If you don't have time to do it right the first time, when will you find the time to add it later? — Jay Blanchard, Mar 07 '17 at 14:49
@JayBlanchard lol, The prof who teaches php in my class had to learn it himself before the semester started. Im just lucky im good friends with the "smart kid" because he taught me more in a couple of weeks than my teacher the whole semester. — brokkosnarf, Mar 07 '17 at 14:52
And everyone always wants to talk about how "good" the educational system is once you reach university status. It's more "scary" than "good". Most guys who have been practicing software engineering are floored at what they see when they decide to return to university for advanced degrees. But I digress. — Jay Blanchard, Mar 07 '17 at 15:00
*"The prof who teaches php in my class had to learn it himself before the semester started"* - You'll definitely be "ahead of your class" now, not to mention the teacher *lol* — Funk Forty Niner, Mar 07 '17 at 15:02
@MasivuyeCokile *"some schools / colleges just dont care"* - Or don't "know", given the OP's comment: *"The prof who teaches php in my class had to learn it himself before the semester started"* - Yipe! — Funk Forty Niner, Mar 07 '17 at 15:03
Well this hast most certainly been an enlightening thread for me and i definitely spent time researching security in code. — brokkosnarf, Mar 07 '17 at 15:04
@Fred-ii- its really bad one term he gave us some lynda videos to learn from while he was learning php — brokkosnarf, Mar 07 '17 at 15:05
@DaanSlurink So the teacher's learning as he goes; I wonder how they got hired in the first place *lol* You should tell him/her that "a lot of water has gone under the bridge sir/madam in over 30 years" as I previously stated. MD5 dates back to that. Another thing they won't tell you or don't know about is; MD5 stores a 32-length string. If/when you do move over to `password_hash()` to store hashes with, the column will need to be increased to a minimum of 60 length. The manual suggests that 255 is a good bet ;-) — Funk Forty Niner, Mar 07 '17 at 15:08
*"Well this hast most certainly been an enlightening thread for me and i definitely spent time researching security in code."* - You'll find a lot of Q&A's on Stack about everything that has been mentioned in comments and Jay's answer. Just have a search in Google also, which will have many results, and many of which leading back here on Stack. Oh, and *Welcome to Stack!* @DaanSlurink *Cheers* - Stay safe ;-) — Funk Forty Niner, Mar 07 '17 at 15:10
@Fred-ii- he originally taught java but because of savings i think they let him teach php — brokkosnarf, Mar 07 '17 at 15:10
@Fred-ii- *This is 2016, not the early 90's*. Buddy, it's 2017 ;) — ʰᵈˑ, Mar 07 '17 at 16:03
@ʰᵈˑ OOoohhh man, did I do that?! *lol* I guess I still have 2016 still running in my veins. — Funk Forty Niner, Mar 07 '17 at 16:05

score 1 · Accepted Answer · edited May 23 '17 at 12:25

First, some warnings (in accordance with this link):

Little Bobby says your script is at risk for SQL Injection Attacks. Learn about prepared statements for MySQLi. Even escaping the string is not safe! Don't believe it?

You really shouldn't use MD5 password hashes and you really should use PHP's built-in functions to handle password security. Make sure you don't escape passwords or use any other cleansing mechanism on them before hashing. Doing so changes the password and causes unnecessary additional coding.

Now to address your issue:

You need to fetch the result from your database so you can define $role:

if (mysqli_num_rows($result) == 1) {
    $row = mysqli_fetch_assoc($result); // fetch the result of your query
    $role = $row['role']; // assigned the value of $role
    switch ($role) {
        case '1':
            $_SESSION['message'] = "je bent ingelogd";
            $_SESSION['username'] = $username;
            $redirect = 'repairs.php';
            break;
        case '2':
            $_SESSION['message'] = "je bent ingelogd";
            $_SESSION['username'] = $username;
            $redirect = 'lloverzicht.php';
            break;
        case '3':
            $_SESSION['message'] = "je bent ingelogd";
            $_SESSION['username'] = $username;
            $redirect = 'overzicht.php';
            break;
    }
    header('Location: '.$redirect);
    exit();
}

Once done you need to add an exit(); after the redirect so that PHP does not attempt to process any other code you may have following the redirect.

@DaanSlurink accept the answer see here : http://meta.stackexchange.com/questions/5234/how-does-accepting-an-answer-work — Masivuye Cokile, Mar 07 '17 at 14:45

different user levels PHP

1 Answers1