-4

1st post here so forgive me if i made any errors when creating the post. I have the following code in which i want to redirect users when they login based on their user_type. I have the following code but i'm quite unsure how to do that.

    <?php
    session_start();

   if(isset($_SESSION['user_type']) == "S") {
    header("Location: /main/studenthome.php");
      }

else if(isset($_SESSION['user_type']) == "A") {
    header("Location: /main/staffhome.php");
}


include_once 'dbconnect.php';

//check if form is submitted
if (isset($_POST['login'])) {

    $username = mysqli_real_escape_string($con, $_POST['username']);
    $password = mysqli_real_escape_string($con, $_POST['password']);
    $result = mysqli_query($con, "SELECT * FROM users WHERE username = '" . $username. "' and password = '" . md5($password) . "'");

    if ($row = mysqli_fetch_array($result)) {
        $_SESSION['usr_id'] = $row['usrId'];
        $_SESSION['usr_name'] = $row['username'];
        $_SESSION['user_type'] = $row["usertype"];
        header("Location: /main/studenthome.php");
    }

    } else {
        echo  "<div class= 'col-md-4 col-md-offset-4 well' >
        Incorrect <font color = '#de615e'> Username </font> or <font color = '#de615e'> Password </font>. Please try again
        </div>";
    }
}
?>

Thanks for your time

grh0ul
  • 15
  • 1
  • 5
  • what type of users u have? – Masivuye Cokile Mar 03 '17 at 13:22
  • the type of users for the time is S and A for the time being. – grh0ul Mar 03 '17 at 14:37
  • you have an answer below – Masivuye Cokile Mar 03 '17 at 14:44
  • **Never store plain text passwords!** 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 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 03 '17 at 14:54
  • [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)*** Learn about [prepared](http://en.wikipedia.org/wiki/Prepared_statement) statements for [MySQLi](http://php.net/manual/en/mysqli.quickstart.prepared-statements.php). Even [escaping the string](http://stackoverflow.com/questions/5741187/sql-injection-that-gets-around-mysql-real-escape-string) is not safe! [Don't believe it?](http://stackoverflow.com/q/38297105/1011527) – Jay Blanchard Mar 03 '17 at 14:54
  • Do not deploy his code in production - it is totally unsafe. – Jay Blanchard Mar 03 '17 at 14:55
  • 1
    [Think (twice) before posting an answer for this question](http://meta.stackoverflow.com/q/344703/), it may change your mind. – Jay Blanchard Mar 03 '17 at 14:56
  • You really shouldn't use [MD5 password hashes](http://security.stackexchange.com/questions/19906/is-md5-considered-insecure) – Jay Blanchard Mar 03 '17 at 15:10

1 Answers1

1

I have added "die" statements, to kill the code after "Location" redirects. This is because the user can essentially ignore the redirect, and it leaves open potential mistakes if you have some executable code below

The essential thing about usertypes and what they should redirect to, is in the switch statement

    if(isset($_SESSION['user_type']) && $_SESSION['user_type'] == "S") {
        header("Location: /main/studenthome.php");
        die();
    }

    else if(isset($_SESSION['user_type']) && $_SESSION['user_type'] == "A") {
        header("Location: /main/staffhome.php");
        die();
    }


    include_once 'dbconnect.php';

    //check if form is submitted
    if (isset($_POST['login'])) {

        $username = mysqli_real_escape_string($con, $_POST['username']);
        $password = mysqli_real_escape_string($con, $_POST['password']);
        $result = mysqli_query($con, "SELECT * FROM users WHERE username = '" . $username. "' and password = '" . md5($password) . "'");

        if ($row = mysqli_fetch_array($result)) {
            $_SESSION['usr_id'] = $row['usrId'];
            $_SESSION['usr_name'] = $row['username'];
            $_SESSION['user_type'] = $row["usertype"];
            switch ($_SESSION['user_type']) {
                case 'Student':
                    header("Location: /main/studenthome.php");
                    break;
                case 'Teacher':
                    header("Location: /main/anotherplace.php");
                    break;
            }
            die();
        } else {
             echo  "<div class= 'col-md-4 col-md-offset-4 well' >
                Incorrect <font color = '#de615e'> Username </font> or <font color = '#de615e'> Password </font>. Please try again
                </div>";
        }

    } 
?>
user1898027
  • 330
  • 2
  • 13
  • Thanks for the help, this code seems to work but the last else statement in the end now just displays the error without pressing the login button. – grh0ul Mar 03 '17 at 14:38
  • Ahh yeah sorry, switched the location of the last else statement. I have corrected the code now, so it is in the right place – user1898027 Mar 03 '17 at 14:43
  • This code `if(isset($_SESSION['user_type']) == "S")` is totally wrong. You wouldn't test if something is set when you're setting it. ***Fatal error: Cannot use isset() on the result of an expression (you can use "null !== expression" instead)*** – Jay Blanchard Mar 03 '17 at 15:00
  • Thought the same as well but didn't get any results when changed the position. While this is fixed now when i loggin with a usertype of 'Student' i still get redirected in the location which is for the 'Teacher' case, this seems really strange. – grh0ul Mar 03 '17 at 15:01
  • Yes you are totally right, I have corrected it now. The error occurred because I pasted the code from OP and ignored the if-statement because it seemed irrellevant – user1898027 Mar 03 '17 at 15:02
  • @JayBlanchard i use that code to check if there's a user_type set already, if there's one that means that the user is already signed in so it redirects the user to the correct page. Correct me if that's wrong – grh0ul Mar 03 '17 at 15:04
  • @S.Mak so what about those users that they type is not "s" when they come to login page, it looks like they will need to login again – Masivuye Cokile Mar 03 '17 at 15:06
  • It is wrong @S.Mak, Those statement should throw an error. – Jay Blanchard Mar 03 '17 at 15:08
  • @MasivuyeCokile there's going to be a check for each user – grh0ul Mar 03 '17 at 15:08
  • ok, cool, so what if your system grows and u have 100 use types u gonna have checker for each? – Masivuye Cokile Mar 03 '17 at 15:09
  • Other than that @S.Mak please heed the warnings in the comments. This code is nowhere near safe enough to put into production as it leaves you wide open for attack on a number of levels. – Jay Blanchard Mar 03 '17 at 15:12
  • @MasivuyeCokile the system has already a determined number of users. – grh0ul Mar 03 '17 at 15:15
  • @user1898027 i pasted the code but still redirects in the page meant for Teachers – grh0ul Mar 03 '17 at 15:21
  • That sounds strange. Are you sure the $_SESSION values are cleared out once the user logs of(maybe try incognito)? And did you correct the Switch statement, to include "S" and "A" instead of "Student" and "Teacher" , and the correct redirect links? – user1898027 Mar 03 '17 at 15:36
  • @S.Mak ahh sorry, forgot to add "break;" statements in the switch. This is most probably the cause of this redirect issue. I fixed it in above code – user1898027 Mar 03 '17 at 15:49