-2

I have this PHP login script that SHOULD be taking the entered username & password, checking it against a value in MySQL (with the password encrypted via SHA1) and then redirecting the user to the "dash.php" if login is successful or printing an error if not. However whenever I submit the form, it just reloads the login.php... Did I make a stupid error somewhere or am I missing something? Sorry about the huge post!

login.php (containing form):

//Form Action
<?php

error_reporting(E_ALL);
ini_set('display_errors','1');

if ($_SERVER['REQUEST_METHOD'] == 'POST') {

    require ('scripts/mysqli_connect.php');

    require ('scripts/login_functions.php');


    list ($check, $data) = check_login($dbc, $_POST['username'], $_POST['password']);

    if($check) {

        redirect_user('dash.php');
    } else {
        $errors = $data;
    }

    mysqli_close($dbc);
}

?>

// Website HTML

//Form
<form class="contact-form" method="post" action="login.php">
                    <div class="col-sm-5 col-sm-offset-1">
                        <div class="form-group">
                            <label>Username: </label>
                            <input type="text" name="username" id="username" size="15" class="form-control" required="required" placeholder="username">
                        </div>
                        <div class="form-group">
                            <label>Password: </label>
                            <input type="password" name="password" id="password" size="15" class="form-control" required="required" placeholder="password">
                        </div>                        
                        <div class="form-group">
                            <input type="submit" name="submit" value="Login" />
                        </div>
                    </div>
                </form> 

login_functions.php:

<?php

function redirect_user ($page = '../login.php') {
    $url = "http://" . $_SERVER['HTTP_HOST'] . dirname($_SERVER['PHP_SELF']);

    $url = rtrim($url, '/\\');

    $url .= '/' . $page;

    //Redirect User

    header("Location: $url");
    exit(); //Quit the script.

}


function check_login($dbc, $username = '', $password = '') {
    $errors = array();

    if(empty($username)) {
        $errors[] = 'You forgot to enter your username.';
    } else {
        $u = mysqli_real_escape_string($dbc, trim($username));
    }

    if(empty($password)) {
        $errors[] = 'you forgot to enter your passord.';
    } else {
        $p = mysqli_real_escape_string($dbc, trim($password));
    }

    if (empty($errors)) {

        $q = "SELECT username, password FROM users WHERE username='$u' AND password=sha1('$p')";
        $r = @mysqli_query ($dbc, $q);


        //Check Results

        if(mysqli_num_rows($r) == 1) {

            $row = mysqli_fetch_array ($r, MYSQLI_ASSOC);

            return array(true, $row);
        } else {
            $errors[] = 'The username/password combination is incorrect.';
        }
    }

}


?>
Jay Blanchard
  • 34,243
  • 16
  • 77
  • 119
htmlboss
  • 119
  • 1
  • 4
  • 11
  • 2
    [Your script is at risk for SQL Injection.](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php). You should [use the proper methods to hash passwords with PHP](http://jayblanchard.net/proper_password_hashing_with_PHP.html). – Jay Blanchard Jun 10 '15 at 21:59
  • Do you print something before the redirection? if headers are already sent you can't redirect – oscargilfc Jun 10 '15 at 22:01
  • @JayBlanchard OP is escaping all values in the query. Yes, they should be using bind parameters, but this isn't vulnerable as written. – Jonathan Kuhn Jun 10 '15 at 22:03
  • Just a heads up, encryption and hashing are two different subjects. – castis Jun 10 '15 at 22:05
  • I see you are not returning anything from the function `check_login` in the case of an error. Try to do a `var_dump` of the `$check` and `$data` variables before you redirect. Also, what happens in your `dash.php`? You might be redirecting to `dash.php` properly which is redirecting back. – Jonathan Kuhn Jun 10 '15 at 22:09
  • For the moment, dash.php is empty for debugging purposes. – htmlboss Jun 10 '15 at 22:14
  • firstly, is the password column long enough to accomodate the hash? that's usually the first thing I ask. I've seen this happen often and a few days ago actually. Once that's been ruled out, well... it'd be on to the next step; error checking on PHP and DB. `@` is an error suppressor, so get rid of that for now in `$r = @mysqli_query ($dbc, $q);` while changing it to `$r = mysqli_query ($dbc, $q); or die(mysqli_error($dbc))` and error reporting http://php.net/manual/en/function.error-reporting.php and make sure you choose to "display" for **all** files, and not just one. – Funk Forty Niner Jun 10 '15 at 22:19
  • another thing I noticed... `action="login.php"` yet you state the other file is called login_functions.php - so, which one is it? – Funk Forty Niner Jun 10 '15 at 22:21
  • see my answer Fred. we never get the glory of login_functions to run – Drew Jun 10 '15 at 22:21
  • One of those FIIK @DrewPierce OP's no longer commenting, unless they've been hit by the "comments restriction" policy. Time will tell I guess. – Funk Forty Niner Jun 10 '15 at 22:29
  • @Fred-ii- , a drive-by question, or a focused gaze upon ice cream – Drew Jun 10 '15 at 22:31
  • Personally, I would scrap (pardon the expression) this entire project and use PDO with prepared statements and `password_hash()`. sha1 isn't really the safest method to use nowadays. Google "sha1 security" and you'll see what I mean. – Funk Forty Niner Jun 10 '15 at 22:32
  • @DrewPierce *haha!* - Now you did it; I've got me a craving for a glazed cappuccino sundae. – Funk Forty Niner Jun 10 '15 at 22:33
  • it's a subtle way us lowly point people use to get the likes of you and Gordon outta here for a bit – Drew Jun 10 '15 at 22:36

1 Answers1

1

You are not returning you errors:

    return array(true, $row);
} else {
    $errors[] = 'The username/password combination is incorrect.';
    $return array(false, $errors);
}

And you are not displaying your errors:

// Website HTML

<?php if ($errors):?>    
    <?php echo '<p>' . implode('</p><p>', $errors) . '<p>';?>
<?php endif;?>
//Form
<form class="contact-form" method="post" action="login.php">
Steve
  • 20,703
  • 5
  • 41
  • 67
  • Helpful note from Steve. check for errors first. You can go to your Developer tools on your browser, click on Network section where you'd be able to see your calls and response data. – qwerty_igor Jun 10 '15 at 22:10