Social network login and registration forms

Question

I am building a social network with PHP, HTML and CSS and a bit of JS. I want to know how secure my login and register code is.

Register html:

    <input id="uname" type="text" onblur="this.value=removeSpaces(this.value);" name="username" required placeholder="Username..."><p/>
        <br>
    <input id="pww" type="password" onblur="this.value=removeSpaces(this.value);" name="pw" required placeholder="Password..."><p />
        <br>
    <input id="cpw" type="password" onblur="this.value=removeSpaces(this.value);" name="pw2" required placeholder="Confirm Password..."><p />
        <br>

    <button type="submit" id="bt1" name="Register">Register</button>

</form>

Login html:

    <input type="text" id="luname" onblur="this.value=removeSpaces(this.value);" required name="username" placeholder="Username...">
    
    <input type="password" id="lpw" onblur="this.value=removeSpaces(this.value);" required name="pw" placeholder="Password..." >

    <button id="bt2" type="submit" name="signin">Login</button>

</form>

Register code:

<?php

error_reporting(-1); // reports errors

// check if form is submitted
if ( $_SERVER['REQUEST_METHOD'] != 'POST' || ! isset($_POST['Register'])) {
    
    // looks like a hack, send to index.php
    header('Location: index.php');
    die();
}
    
require 'config/connect.php';

//Initializing variable. "" When you want to append stuff later. 0 When you want to add numbers later
$username = ''; 
$pw = ''; 
$pw2 = ''; 

$_SESSION['username'] = $username; // stores username in session variable

$success = array(); // holds success messages
$errors = []; // holds error messages

if ($pw !== $pw2) {

    //$errors[] = "The passwords do not match.";

}

if (!$errors) {
    //An SQL statement template is created and sent to the database
    $stmt = $conn->prepare("SELECT * FROM users WHERE username=?");
    // This function binds the parameters to the SQL query and tells the database what the parameters are.
    $stmt->bind_param("s", $_POST['username']);
    // the database executes the statement.
    $stmt->execute();
    // Associative arrays are arrays that use named keys that you assign to them.
    $row = $stmt->get_result()->fetch_assoc();

    if ($row && $row['username'] == $_POST['username']) {

        //$errors[] = "<p id='exists'>Username exists</p>";

        $_SESSION['error'] = '<b><p style="color: #000000; font-size: 25px; top: 34%;right: 30%;position: absolute;">Username exists</p></b>';
        header('Location: index.php');
    }
}

if (!$errors) {

    $pw = password_hash($pw, PASSWORD_BCRYPT, array('cost' => 14));

    $stmt = $conn->prepare("INSERT INTO users (username, pw) VALUES(?, ?)");
    $stmt->bind_param("ss", $_POST['username'], $pw );
    $stmt->execute();

    $_SESSION["username"] = $_POST['username'];
    header('Location: profile.php');
    die();

} else {
    
    // The foreach construct provides an easy way to iterate over arrays. 
    foreach ($errors as $error) {
        echo "$error <br /> \n";
    }
    echo '<a href="index.php" id="exists">Try again</a><br />';
}
?>

Login code:

<?php

error_reporting(-1); // reports errors

require 'config/connect.php';

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

//check if form is submitted
//if ( $_SERVER['REQUEST_METHOD'] != 'POST' || ! isset($_POST['signin'])) {

    // looks like a hack, send to index.php
    //header('Location: index.php');
    //die();
//}

if (empty($_POST["username"])) {

    //echo 'Fill in username to sign in. <a href= index.php>Try again</a><br />';
    //die();

    $_SESSION['error'] = '<b><p style="color: #000000; font-size: 25px; top: 15%;right: 38%;position: absolute;">Username cannot be empty</p></b>';
    header('Location: index.php');
}

if (empty($_POST["pw"])) {

    // echo 'Fill in password to sign in. <a href= index.php>Try again</a><br />';
    // die();

    $_SESSION['error'] = '<b><p style="color: #000000; font-size: 25px; top: 15%;right: 38%;position: absolute;">Password cannot be empty</p></b>';
    header('Location: index.php');
}

$sql = "SELECT pw FROM users WHERE username = ?";
$stmt = mysqli_prepare($conn, $sql);

if ( !$stmt ) {

    echo mysqli_error($conn);
    die();
}

$stmt->bind_param('s', $_POST['username']);

if ( !$stmt->execute() ) {

    echo mysqli_error($conn);
    die();
}
// we found a row with that username, 
// now we need to check the password is correct

// get the password from the row
$stmt->bind_result($hashed_pwd); // Binds variables to a prepared statement for result storage
$stmt->fetch(); // Fetch results from a prepared statement into the bound variables

if ( password_verify($_POST['pw'], $hashed_pwd) ) {

    // password verified
    $_SESSION["username"] = $_POST['username'];
    header('Location: profile.php');

} else {
//echo 'Incorrect username or Password. <a href= index.php>Try again</a><br />';

    $_SESSION['error'] = '<b><p style="color: #fff; font-size: 25px; top: 15%;right: 30%;position: absolute;">Incorrect username or Password.</p></b>';
    header('Location: index.php');
}
?>

Answer 1

• Allowing spaces in usernames and passwords represents absolutely no security risk -- so let your users do it if they like. Well, if you don't want usernames to have spaces, that's a personal choice. In fact, by silently removing spaces, you stand the chance of irritating your users because they will struggle to log in if the saved username or password doesn't match what they [thought they] typed.

• I don't see any reason to save a blank username string to the $_SESSION array. If you are going to save the username to the SESSION, then only added it after it is validated.

• Because your script can only produce a single error message, then it could be argued that employing an array to contain this data is an overly complex data structure. Unless you plan on extending the volume of error messages to be displayed to the user, just save the variable as a string.

• Never display mysqli_error() messages in production code. You never want to give bad-actors any "useful" information.

• I don't recommend saving html in your SESSION array. Save the lean text and maybe save meta data on the text (was it a successful or failed outcome). If html markup & styling is necessary, that should be generated by the view/template and css should be in an external style sheet.

• if ( $_SERVER['REQUEST_METHOD'] != 'POST' || ! isset($_POST['Register'])) { is more simply written as if (!isset($_POST['Register'])) {.

• The check if $row['username'] == $_POST['username'] is silly. Of course, the two values will be identical -- you just pulled the qualifying match from the database! Instead, just return the COUNT() from your query, this way you will always have a result set and you can just check if the result set's lone column value is truthy/falsey to determine if the username is already taken.