0

The file code is going to check if the field is empty then run the code. The main problem is that when the text box is filled, the "authentication" doesn't work anymore.

So basically, the system gonna check if the text box is filled (both for the Username and Password), when the text box is not filled out, an error will show up "Please fill all the fields" and it's not gonna let the user login if the text box isn't filled out. The problem now is when the text boxes is properly filled (or "filled"), the authentication system isn't working anymore.

More Information:
The authentication system is using SHA256 as the "encryptor" or the "hash code". When I don't enter the code for "checking the fields if empty", the code actually works, but when I put it on the file, everything seems messed up now.

The HTML form code:

<form action="<?=$_SERVER['PHP_SELF']?>" method="post">
    Username: <input type="text" name="username" /><br />
    Password: <input type="password" name="password" /><br />
    Remember me: <input type="checkbox" name="remember" /><br />
    <input type="submit" name="submit" value="Login" />
    <a href="register.php">Register</a>
</form>

The PHP code:

   <?php
$username = $_POST['username'];
$mainpass = $_POST['password'];
if ($_SERVER['REQUEST_METHOD'] == 'POST') {
    if (empty($_POST['username']) || empty($_POST['password'])) {
        echo "Please fill all the fields!";
    }
} else {
?>
<?php
    if (isset($_POST['submit'])) {
        $username = $_POST['username'];
        $mainpass = $_POST['password'];
        $password = hash('sha256', $mainpass);

        // processing remember me option and setting cookie with long expiry date
        if (isset($_POST['remember'])) {
            session_set_cookie_params('604800'); //one week (value in seconds)
            session_regenerate_id(true);
        }

        $mysqli = new mysqli(localhost, root, "", loginsecure);
        # check connection
        if ($mysqli->connect_errno) {
            echo "<p>MySQL error no {$mysqli->connect_errno} : {$mysqli->connect_error}</p>";
            exit();
        }

        $sql    = "SELECT * from users WHERE username LIKE '{$username}' AND password LIKE '{$password}' LIMIT 1";
        $result = $mysqli->query($sql);

        if ($result->num_rows != 1) {
            echo "<p><b>Error:</b> Account doesn't exists! <a href=\"register.php\">Register here!</a></p>";
        } else {
            // Authenticated, set session variables
            $user                 = $result->fetch_array();
            $_SESSION['user_id']  = $user['id'];
            $_SESSION['username'] = $user['username'];

            // update status to online
            $timestamp = time();
            $sql       = "UPDATE users SET status={$timestamp} WHERE id={$_SESSION['user_id']}";
            $result    = $mysqli->query($sql);

            redirect_to("profile.php?id={$_SESSION['user_id']}");
            // do stuffs
        }
    }

    if (isset($_GET['msg'])) {
        echo "<p style='color:red;'>" . $_GET['msg'] . "</p>";
    }
}
?>
astroXoom
  • 39
  • 8
  • To validate if the input are empty, why don't you use JavaScript or jQuery ? PHP is server side, it will validate the input vs the database. – Marc Giroux Jul 29 '16 at 11:17
  • 1
    Actually, i'm planning to use jQuery to validate it, but as I said, "experts" can mess my code and can send "harmful" request. – astroXoom Jul 29 '16 at 11:18
  • A good read should be this: http://stackoverflow.com/questions/5004233/jquery-ajax-post-example-with-php?rq=1 that will show you an example of ajax / post. – Marc Giroux Jul 29 '16 at 11:19
  • Isn't ajax is a "client-sided" type? Someone could mess my code tho – astroXoom Jul 29 '16 at 11:21
  • 1
    Instead of using javascript to check if the input is empty you could set the `required` attribute, but you will still need to do the same check on the server. Other things to consider is using `password_hash` and `password_verify` instead of using sha256. And last this code has several sql injection vulnerabilities, always use prepared statement instead of concatenating strings to use as queries – rypskar Jul 29 '16 at 11:31

2 Answers2

0

Problem is the if statements in the else clause will never be reached except when the method is GET.

Change the if statement and remove the first assignment of $username and $password:

<?php
if ($_SERVER['REQUEST_METHOD'] == 'POST') {
    if (empty($_POST['username']) || empty($_POST['password'])) {
        echo "Please fill all the fields!";
    }
    elseif (isset($_POST['submit'])) {
        $username = $_POST['username'];
        $mainpass = $_POST['password'];
        // rest of your code...
  • Also, if you're worried about security, you should worry about sql injection. You can read more by Googling around but here's a good SO Q/A http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php –  Jul 29 '16 at 11:29
  • This is helpful, thanks for answering this :) and i'm gonna use PDO soon [the code is only using MySQLi] and gonna prevent SQLi attacks. *p.s: I'm a starter on this kind of things, i'm trying to be "simple" and "fast" as possible (also I want this thing " secured ") – astroXoom Jul 29 '16 at 11:44
  • @astroXoom that's good and I prefer pdo but I have to mention that mysqli can also be used to do parameterized queries. Good luck and happy programming! –  Jul 29 '16 at 11:46
  • @astroXoom also, see what rypskar said about [password_hash](http://php.net/password_hash) –  Jul 29 '16 at 11:47
  • Yep, i'm a "starter" on these things, and i'm only 13 y/o. So, am I going on to the "right" way? – astroXoom Jul 29 '16 at 11:48
0

what do you mean by messed up?

You should be able to do it by checking if POST username exists and if its not empty (repeat for password)

if any of these tests come back negative then output error and show the login form again.

if the tests pass continue with the processing as per normal

alex3410
  • 71
  • 6
  • When I don't entered the "check fields" code, It works fine, but when I added the code, I won't able to login – astroXoom Jul 29 '16 at 11:37