0

I have developed a PHP login/registration script and when the user logs in, it starts a session and creates a cookie then redirects the user to dashboard.php using the following:

    //get data in variables
    $emailAddress = mysqli_real_escape_string($conn,$_POST['emailAddress']);
    $password = mysqli_real_escape_string($conn,$_POST['password']);
    $remember = isset($_POST['rememberMe']);

    if(!filter_var($emailAddress, FILTER_VALIDATE_EMAIL)) {
         $error = 'Please enter a valid email address.';
    }
    elseif(email_exists($emailAddress,$conn)) { // if email address exists
        $result = mysqli_query($conn,"SELECT password FROM users WHERE email = '$emailAddress'");
        $retrievePassword = mysqli_fetch_assoc($result);
        if(!password_verify($password,$retrievePassword['password'])) // if password does not match
        {
            $error = 'Password is incorrect.';
        } else { // if password correct, log user in
            $_SESSION['email'] = $emailAddress;
            if($remember == 'on') { // if "keep user logged in" was ticked
                setcookie("email",$emailAddress,time()+7200); // keep user logged in for 2 hours
            }
            header("location: dashboard.php");
        }
    } else { // if email does not exist
        $error = 'Email address not registered.';
    }

At the top of all of my restricted pages (such as dashboard.php) I have:

if(logged_in()) { // if user logged in, show page

and the logged_in() function is:

function logged_in(){
    if(isset($_SESSION['email']) || isset($_COOKIE['email'])) {
        return true;
    } else {
        return false;
    }
}

In the database, I have a column titled usrUserTypeNo where admins are '1' and normal users are '3'. How can I incorporate this user type in to the session so that I can determine what content is shown to the user depending on their role?

Adey
  • 3
  • 3
  • 3
    `isset($_COOKIE['email'])` — **Danger** — If you just check that a cookie with a name exists, an attacker could insert any value into their cookies and bypass your login mechanism. – Quentin Apr 24 '23 at 13:41
  • What have you tried so far? Where are you stuck? Why not place the user level into the session and read it from there? – Nico Haase Apr 24 '23 at 14:05
  • Please be warned that the query you've shared is highly vulnerable for SQL injection. Have a look at prepared statements to avoid getting hacked – Nico Haase Apr 25 '23 at 06:26
  • @NicoHaase Thanks, this is exactly the code that was taught in the tutorial I followed on Udemy for the login/registration system - which part is vulnerable? I'd like to look in to this issue. – Adey Apr 25 '23 at 15:54
  • Then the tutorial is a pretty bad one. You should **never** use string concatenation to build an SQL query, even if you escape it manually in your code – Nico Haase Apr 26 '23 at 06:25
  • @NicoHaase i'm assuming you mean the `$result = mysqli_query($conn,"SELECT password...` bit and I should use `if(mysqli_query($conn,$result)) {` instead? – Adey Apr 26 '23 at 08:39
  • Yes, that's the insecure query, and no, you should not use `$result`, but a properly prepared statement – Nico Haase Apr 26 '23 at 08:46
  • @NicoHaase OK thank you, I have looked at prepared statements and it makes perfect sense, but why would it be so important with SELECT statements, would it not just be a risk to INSERT statements? – Adey Apr 26 '23 at 09:38
  • Yes, that's important in the same way. Just have a look about how SQL injections work. Also, make **all** queries secure, and not just some of them – Nico Haase Apr 26 '23 at 09:41

1 Answers1

1

I wondered if there was an easier way to incorporate this in to the actual logged_in() function rather than having a SELECT * query before any content to then add conditional for whether usrUserTypeNo is 1 or 3.

Not really.

If you want to do one of three different things (not logged in action, normal user action, admin user action) then you have to test for those states and write logic for each one.

You could store the access level in the session instead of requesting it from the database each time.

Quentin
  • 914,110
  • 126
  • 1,211
  • 1,335
  • Thanks Quentin, I think adding the access level to the session was my initial thought but unsure how to go about it - would I need a SELECT statement as part of the login process to actually get the level first, and then do I need a second $_SESSION['level'] or can it be incorporated in to the $_SESSION['email'] somehow? – Adey Apr 24 '23 at 17:29
  • You need a SELECT statement as part of the login process to get the password hash. Get the level in the same SELECT. – Quentin Apr 24 '23 at 19:38
  • There is no benefit in trying to mangle two different pieces of data into a single array index. – Quentin Apr 24 '23 at 19:38