Login doesn't work properly

Hi,

What the function should do is log the user in and set a session ID, however, upon submitting the form, it redirects to the page where the code is on (requests/login.php) but the page is blank.

I’ve checked that:

  • session_start() is at the top of include/conn.php
  • users is a valid table in my database
  • $_POST['submit'] should work because of name='submit' in the form.

My register function works, however my login function does not. Any help is greatly appreciated.

requests/login.php:

<?php
    include("../include/conn.php");

    $username = $_POST['username'];
    $password = $_POST['password'];

      if (isset($_POST['submit'])) {
        // get password from user row
        $stmt = $conn->prepare("SELECT * FROM users WHERE username = ?");
        $stmt->bind_param('s', $username);   
        $stmt->execute();
        $getres1 = $stmt->get_result();
        // now we see if user exists
        if ($getres1->num_rows > 0) {
          while ($row = $getres1->fetch_assoc()) {
            // password check
            $db_password = $row['password'];
      
            if (password_verify($password, $db_password)) {
              $_SESSION['id'] = $row['id'];
              header("Location: ../index.php");
            }
          }
        }
      }
?>

login.php:

        <form action="requests/login.php" method="post">
            <span>Username</span> <input type="text" name="username"><br>
            <span>Password</span> <input type="password" name="password"><br><br>
            <input type="submit" name='submit' value="Log In">
        </form>

You have no logic to handle a login fail so it goes to the same page. It is blank because you have not told the code to do anything on failure.

You should also be checking the REQUEST METHOD, not hoping the name of a button is submitted in order for your code to work which will completely fail in certain cases.

Do not create variables for nothing.

The form should be on the same page, Php at the top, form below that and completely remove the form action attribute.

You are using variables before they are set. You are going to fill up your error log.

Additionally, the while loop is pointless. You should only ever get back one result, so there is nothing to loop over.

I would also suggest you use PDO.

2 Likes

There’s actually a lot of things that can go wrong with this script tbh. What happens if a user is already logged in and wants to deliberately see what they can break? Are you going to allow them to attempt to log in even though they’re already logged in? I would actually put at the top of that code to check if the user is logged in, if they are then redirect them back to the home page.

Then you’ve got this right here that can fail with ease. When the user arrives at that login page, it will already throw them errors saying username and password doesn’t exist within the $_POST array. You should actually be checking to see if it’s submitted through a POST request rather than relying on checking to see if the submit button was clicked on.

Darn! You beat me to it.

1 Like

Your code has no logic for validation or user error messages, so, of course there’s no output.

Here’s a list of practices that will help make your code secure, provide a good user experience, result in the least amount of code, and though having validation and error handling, your code will either work or it will tell you why it isn’t -

  1. Put the form processing code and the form on the same page. The form processing code goes above the start of the html document. This will let you display any user/validation error messages, when you redisplay the form, and let you repopulate the relevant form fields with their existing values so that the user doesn’t need to keep reentering data over and over.
  2. Use ‘require’ for things that your code must have for it to work. Include/require are not functions and the () around the path/filename are unnecessary typing.
  3. Put all the form processing code inside the conditional statement that has detected if the form has been submitted.
  4. Don’t copy variables to other variables for nothing. This is just a waste of typing. You should actually keep the form data as a set, in an array variable, then operate on elements in this array variable throughout the rest of the code.
  5. You should not try to detect if the submit button isset. There are cases where it won’t be. Instead, detect if a post method form has been submitted.
  6. You should trim all the data first. If you keep the form data as a set, you can do this with one single statement.
  7. Validate all the input data, storing validation error messages in an array, using the field name as the array index.
  8. After the end of all the validation logic, if there are no errors (the array holding the errors will be empty), use the form data. To display the errors, test and either loop over or implode the contents of the array holding the error messages at the appropriate location in the html document.
  9. Build any sql query statement in a php variable.
  10. List out the columns you are SELECTing in the sql query statement.
  11. This would be a good time to switch to the much simpler and better designed PDO extension.
  12. The msyqli get_result() method may not exist on any particular php build/installation, so using it results in non-portable code. Another good reason to use the PDO extension.
  13. Don’t use a loop to fetch data from a query that will match at most one row. Just fetch (and test at the same time) that one row of data.
  14. If the username isn’t found, you would add a message to the array holding the user/validation errors - ‘Sorry, the username/password is invalid.’
  15. If the username is found, you would verify if the passwords match. If the password doesn’t verify, you would setup the same message as above for the wrong username. If the password does verify, you would store the user’s id in a session variable. Since many things can have an id, the session variable should be named user_id or similar.
  16. After all of the above logic, if there are no errors, you would use a header() redirect to the exact same URL of the current page. This will cause a get request for that page so that the browser won’t attempt to resubmit the form data should the user reload or navigate away from and back to the URL.
  17. Every header() redirect needs an exit/die statement after it to stop php code execution.
4 Likes