Registration and Login System for Application

Question

I am developing a social platform and currently just completed the registration and login PHP pages. The application is an android application that authenticates against a MySQL DB. I am seeking critiques on how "safe" and correct my PHP pages are that I am authenticating against, IE: Am I storing and authenticating passwords in a safe manner. This is my first try so I am up for changes.

Registration Page

<?php
define('HOST', 'localhost');
define('USER', 'admin');
define('PASS', 'password');
define('DB', 'database');
$con = mysqli_connect(HOST, USER, PASS, DB);

$username = $_POST["username"];
$password = $_POST["password"];
$email = $_POST["email"];
$age = $_POST["age"];
$id = $_POST["id"];
$hash = password_hash($password, PASSWORD_DEFAULT);

$sql = "INSERT INTO `table` (Username,Password,Email) values
       ('$username','$hash','$email')";

if (mysqli_query($con, $sql)) {
$response["success"] = 1;
$response["message"] = "Success";
die(json_encode($response));
} else {

$response["success"] = 0;
$response["message"] = "Invalid ";
die(json_encode($response));
}
mysql_close();
php
?>

Login Auth

<?php
define('HOST', 'localhost');
define('USER', 'admin');
define('PASS', 'password');
define('DB', 'database');
mysql_connect(HOST, USER, PASS);
mysql_select_db(DB)or die;
$username = $_POST["username"];
$password = $_POST["password"];

if (!empty($_POST)) {
if (empty($_POST['username']) || empty($_POST['password'])) {
    // Create some data that will be the JSON response 
    $response["success"] = 0;
    $response["message"] = "One or both of the fields are empty .";
    die(json_encode($response));
}
$query = "SELECT * FROM `table` WHERE `Username` = '$username' LIMIT 1";
$result = mysql_query($query) or die(mysql_error());
$row = mysql_fetch_array($result);
$hash = $row["Password"];
if (password_verify($password, $hash)) {
    $response["success"] = 1;
    $response["message"] = "You have been sucessfully login";
    die(json_encode($response));
} else {

    $response["success"] = 0;
    $response["message"] = "invalid username or password ";
    die(json_encode($response));
}
} else {

$response["success"] = 0;
$response["message"] = " One or both of the fields are empty ";
die(json_encode($response));
}
mysql_close();

Any input or information to make this better/more functional is greatly appreciated.

Answer 1

Security

Yes, you are storing passwords safely. But your code is still incredibly insecure, as you are completely open to SQL injection.

You should use prepared statements, always, for all variable data.

[I also don't see any protection against CSRF; If you don't have it, you should add it, even for login forms; A user may for example force-login a victim into a fake account, and - if the victim does not pay attention - the attacker can thus see actions the victim takes on the site (which may potentially be interesting on a social platform and would definitely violate the privacy of your users)]

Misc

There are a lot of small mistakes and oddities in your code which makes it harder to read and maintain than necessary. Here's a small list, although I may have missed some.

• Your indentation is off, making your code hard to read.
• You mix mysql_ and mysqli_ functions, which will not work.
• Do not use mysql_ functions; they are deprecated and will be removed in PHP7.
• You have statements after you die (eg mysql_close();), which doesn't make sense.
• Why die? There are few reasons to do it (one would be after a redirect), but in your cases it doesn't really make sense.
• You have some duplication which you should remove (eg the One or both of the fields are empty error parts; You also have a lot more die(json_encode($response)); than you need).
• I would use a lot more functions. It makes code more readable (because logical blocks now have a name) and more reusable and testable.
• Why do you have POST values that you don't use (eg age, id)?
• You define your db info more than once, which makes your code hard to maintain. It also makes hiding your db password harder.
• You first access POST, and only afterwards ask if POST is empty or not (and you only actually check this in your login code, not your registration code).

Answer 2

I redid much of the code per information from Tim.

<?php

include 'connection.php';

$username = filter_input(INPUT_POST, "username");
$password = filter_input(INPUT_POST, "password");
$email = filter_input(INPUT_POST, "email");
$hash = password_hash($password, PASSWORD_DEFAULT);
$stmt = $connection->prepare("INSERT INTO `blazed_user_base` (Username,Password,Email) values (?,?,?)");
$stmt->bind_param("sss", $username, $password, $email);

if ($stmt->execute()) {
    $response["success"] = 1;
    $response["message"] = "Success.";
} else {
   $response["success"] = 0;
   $response["message"] = "Failure.";
}
mysqli_close($connection);
echo json_encode($response);

1. I removed unnecessary variables that were being posted that I forgot about.
2. Used solely mysqli and not a conjunction of the two.
3. I am also in the process of creating Functions for errors and validating of user input.
4. I created a separate file all together to handle all connections to the DB.
5. Used solely prepared statements in my code.

I am only adding the code for the Registration file right now as I am redoing the Login as well.

Answer 3

I am seeking critiques on how "safe" and correct my PHP pages are

Your code is completely insecure.

Problems:

1. $response array is never initialized as a variable. in strict error reporting, this code will fail. $response should be initialized as so $response=[]; before you can begin assigning keys. its a moot point though, since response is completely unnecessary. it serves no purpose whatsoever.

2. using mysqli in registration and mysql in login is a dead give away that you have no idea what you are doing. for your users sake i hope you aren't storing any customer data or accepting payments. you should consider actually learning the language before you go slapping together apps and releasing them into the wild.

3. not using prepared statements. you can either learn to do it manually or use an library like RedBean that handles it for you.

4. closing php tags ?> are not necessary, and i'm pretty sure that your tag is wrong php?>. its just ?>.

5. mysql_close() never gets reached, because you are using die to return the message.

6. don't use die to output responses.

here's a bare minimum version of registration that is secure in that it won't get SQL injected. you still need to perform validations about the input, such as insuring the email and username are available, but thats an exercise i leave to you. you also need to take this lesson and apply it to the login.

<?php

    define('HOST', 'localhost');
    define('USER', 'admin');
    define('PASS', 'password');
    define('DB', 'database');


    try
    {

        $db = new mysqli(HOST, USER, PASS, DB);


        $required = ['username','password','age'];


        if(!isset($_POST) || empty($_POST)){
            throw new exception(''); //you could maybe add messages here if you want. 
        }

        foreach($required as $k){
            if(!isset($_POST[$k]) || empty($_POST[$k])){
                throw new  exception('');
            }
        }


        //you should probably perform some validation here, but thats up to you. 

        $username = $_POST["username"];
        $password = password_hash($_POST['password'], PASSWORD_DEFAULT);//why waste another variable?
        $email = $_POST["email"];
        $age = (int) $_POST["age"]; //this is unused?


        $stmt = $db->prepare('INSERT INTO table (Username,Password,Email,Age) values(?,?,?,?)'); //initiate our prepared statement. the ? are placeholders for our values
        $stmt->bind_param('sssi',$username,$password,$email,$age); //all 3 values are strings, so 'sss' followed by each parameter. this binds parameters to the placeholders.

        if($stmt->execute()){ //if stmt execute returns true the insert was successful
            $stmt->close(); //close the statement is a best practice. if you have more than 1 statement in a script you must definitely close the statement
            send_success();
        }else{
            throw new exception('');
        }

    }
    catch(exception $e)
    {
        send_error();
    }


    function send_success()
    {
        echo json_encode(['success'=>1,'message'=>'success']);
        die;
    }

    function send_error()
    {
        echo json_encode(['success'=>0,'message'=>'invalid']);
        die;
    }

I really hope you head my warning about diving head first into a consumer facing app. you need to spend a great deal of time learning and developing