Jump to content

Again some very needed help


Erwin007

Recommended Posts

Hi,
hate to ask but after hours of trying I can't figure it out.
Probably something incredible small...

if(isset($_POST['login'])){
        extract($_POST);
        $query   = "SELECT * FROM users WHERE `username` = '$username' AND password ='$password'";
        $result  = mysqli_query($con,$query);
        $num_row = mysqli_num_rows($result);

        if( $num_row > 0){
            $row    = mysqli_fetch_assoc($result);
            $id = $row['id'];
            $_SESSION['$username']  = $row;
            $_SESSION['user'] = $username;
            
            $admin = $row['admin'];
            
            if( $admin = 1 ){
                header("location:index_admin.php");
            }else{
                header("location:index.php");
            }    
            
        }else{
            header("location:login.php?msg=error");
        }

 

it doesn't matter if the user has admin 1 or 0 it always goes to the index_admin.php

Link to comment
Share on other sites

5 minutes ago, cyberRobot said:

Side note: you'll want to be extra cautious with values that can be tampered with by the users (i.e., $username and $password). The values could potentially break the query...or be used for SQL Injection Attacks. These issues can be avoided with Prepared Statements.
https://www.php.net/manual/en/mysqli.quickstart.prepared-statements.php

Thanks for the advice but first I need to have it working the "vulnerable" way

 

Link to comment
Share on other sites

30 minutes ago, Erwin007 said:

thanks for your reply, unfortunately it doesn't work

Is it still always going to index_admin.php?

Have you tried outputting $admin to see if it contains the expected value? Of course, you'll need to temporarily disable the header calls so you can see the output.

Also, it's always a good idea to use exit after a header call. Otherwise, the code below the header call may still execute. More information can be found here: https://www.php.net/manual/en/function.header.php

    //...

    $admin = $row['admin'];

    if( $admin == 1 ){
        header("location:index_admin.php");
        exit;
    }else{
        header("location:index.php");
        exit;
    }

}else{
    header("location:login.php?msg=error");
    exit;
}

 

Link to comment
Share on other sites

the only redirect you should have in your post method form processing code should be upon successfully logging in and it should be to the exact same URL of the current page to cause a get request for that page. if you don't do this, anyone can use the browser's developer tools network tab to see what the submitted username and password are. if you want the user to be able to go to a different page, provide navigation links, or ever better, integrate the login system with any page that needs it.

by putting the form and form processing on different pages and doing this - header("location:login.php?msg=error"), you are opening your site to a phishing attack, where someone can trick your users to enter their username/password on the phishing site, then redirect to your site and make it look like they mistyped their information.

Link to comment
Share on other sites

49 minutes ago, cyberRobot said:

Is it still always going to index_admin.php?

Have you tried outputting $admin to see if it contains the expected value? Of course, you'll need to temporarily disable the header calls so you can see the output.

Also, it's always a good idea to use exit after a header call. Otherwise, the code below the header call may still execute. More information can be found here: https://www.php.net/manual/en/function.header.php

    //...

    $admin = $row['admin'];

    if( $admin == 1 ){
        header("location:index_admin.php");
        exit;
    }else{
        header("location:index.php");
        exit;
    }

}else{
    header("location:login.php?msg=error");
    exit;
}

 

I tried your suggestion but still the same result.
In the table the admin is a varchar with length = 1, but I tried tinyint also with the same result.

Link to comment
Share on other sites

Is "admin" the column name in the database table "users"?

Have you tried outputting $row to see if it contains the expected values? For example, you could add the bottom two lines after the $row assignment.

$row = mysqli_fetch_assoc($result);

print '<pre>' . print_r($row, true) . '</pre>';  //<-- debug code
exit;  //end script to avoid header() from executing

 

  • Like 1
Link to comment
Share on other sites

Just to be clear, you implemented the fix for the bug that cyberRobot provided?

if( $admin == 1 ){

So at this point your entire question has changed!  Now you are always going to index.php rather than admin_index.php?

A few general comments, and things I noticed: 

The correct valid form of the location header is:

header("Location: index.php");

Notice "Location" not 'location' and the space between the colon and the url.

MDN is a good place to verify these types of things:  https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Location

These small details often aren't a problem, until they are, and something doesn't work right, and you can't figure out why.  

In general, you are at the point, where you need to verify your assumptions one step at a time, with debugging output, using var_dump(), die("something...), or something like what @cyberRobot provided in your last message. 

The other option is to get xdebug working, so you can step through your code.  Setting it up can be difficult for some people, depending on your development environment and IDE being used, but it is often a tremendous aid to developers who are learning.

If you want to let us know what your development environment is (os, php setup, code editor etc) we might be able to locate some resources or tutorials to help you with the setup.

This code appears to be wrong:

        $_SESSION['$username'] = $row;
        $_SESSION['user'] = $username;

 

First you have $username, which is an odd key, and makes me wonder what you are trying to do there.  It also looks like these assignments are mixed up.  I would assume what you really want is:

        $_SESSION['username'] = $username;
        $_SESSION['user'] = $row;

 

Last but not least, I would strongly urge you to start looking at how to employ functions, to make your code more manageable and DRY.  

Look at how many places you are doing a header location!  Consider this simple function:

function redirect($url): void {
    header("Location: $url");
    exit;
}

You might notice that whenever you redirect using the location header, you can not trust the browser to perform the redirect, so you should always exit() so that no additional code will be run.  Putting it in a function, helps insure you don't forget. 

With this function, your code would become:

if (isset($_POST['login'])) {
    extract($_POST);
    $query   = "SELECT * FROM users WHERE `username` = '$username' AND password ='$password'";
    $result  = mysqli_query($con,$query);
    $num_row = mysqli_num_rows($result);

    if( $num_row > 0){
        $row    = mysqli_fetch_assoc($result);
        $id = $row['id'];

        $_SESSION['user'] = $row;
        $_SESSION['username'] = $username;
            
        $admin = $row['admin'];
            
        if( $admin = 1 ){
            redirect("index_admin.php");
        } else {
            redirect("location:index.php");
        }    
            
    } else {
        redirect("login.php?msg=error");
    }

 

This is more of a best practice recommendation, but when you have code like this, that has a functional flow that ends/exits/returns, it is best practice not to employ nested if then else blocks when you can avoid them.  Your code can be rewritten this way:

if (isset($_POST['login'])) {
    extract($_POST);
    $query   = "SELECT * FROM `users` WHERE `username` = '$username' AND `password` ='$password'";
    $result  = mysqli_query($con,$query);
    $num_row = mysqli_num_rows($result);

    if ($num_row < 1) {
        redirect("login.php?msg=error");                 
    }
                     
    $row    = mysqli_fetch_assoc($result);
    $id = $row['id'];

    $_SESSION['user'] = $row;
    $_SESSION['username'] = $username;          
    $admin = $row['admin'];
            
    if ($admin = 1) {
        redirect("index_admin.php");
    }
                     
    redirect("location:index.php");

 

Link to comment
Share on other sites

This thread is more than a year old. Please don't revive it unless you have something important to add.

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.