User not being deleted on link click

Question

I have a users table and I want to be able to delete a user when a link is clicked. $user_name is set in a session. Here is the link:

<?php echo "<a href='delete_user.php?id=".$user_name."' onclick=\"return confirm('Are you sure?')\">Delete Account</a>" ?>

Here is the code on delete_user.php:

<?php
session_start();
session_destroy();
require "connection.php";
?>

<?php

if($_GET['id'] != ""){

$user_name = $_GET['id'];

$sql = "DELETE FROM users WHERE user_name='{$user_name}'";

$result = mysqli_query($connection, $sql);

header('Location: register.php');

}

?>

<?php include "footer.php";?>

I don't understand why it's not deleting the user from the database when this code is executed?

Answer 1

There's no clear reason as to why your code is not working. However, you mentioned being new to PHP, so picking up good practices with your code could (1) help solve the issue at hand, (2) make your code more efficient, and easier to debug.

I recommend you use mysqli in the object-oriented manner, it requires less code, and usually easier to follow.

Making the connection is simple:

<?php
$host = 'localhost';
$user = 'USERNAME';
$pass = 'PASS';
$data = 'DATABASE';

$mysqli = new mysqli($host, $user, $pass, $data);
// catch errors for help in troubleshooting
if ($mysqli->errno)
{
    echo 'Error: ' . $mysqli->connect_error;
    exit;
}
?>

Creating a safe environment for your server keep in mind these things:

1. Do not trust user input (ever!)
2. Do not perform direct queries into your database.
3. When developing, break your code into steps so you can easily troubleshoot each part.

With those three simple things in mind, create a delete file.

<?php
if (isset($_GET['id'])
{
    // never trust any user input
    $id = urlencode($_GET['id']);

    $table = 'users';
    // set a LIMIT of 1 record for the query
    $sql = "DELETE FROM " . $table . " WHERE user_name = ? LIMIT 1";

    // to run your code create a prepared statement
    if ($stmt = $mysqli->prepare( $sql ))
    {
        // create the bind param
        $stmt->bind_param('s', $id);
        $stmt->execute();
        $message = array(
            'is_error' => 'success',
            'message' => 'Success: ' . $stmt->affected_rows . ' were updated.'
        );
        $stmt->close();
    }
    else
    {
        $message = array(
            'is_error' => 'danger',
            'message' => 'Error: There was a problem with your query'
        );
    }
}
else
{
    echo 'No user id is set...';
}

The code will help you set the query, and delete the user based on their user_name... Which I am not sure that is the best solution, unless user_name is set to be an unique field on your MySQL database.

Answer 2

Firstly this is a horrible way to do this, you are prone to SQL Injections and also using GET literally just tags the query to the end of the URL which is easily obtainable by a potential hacker or ANY user as a matter of fact. Use POST instead with a bit of jQuery magic, I would also recommend using Ajax so that you don't get redirected to php file and it will just run. As it is not anyone can access that URL and delete users so I recommend using PHP SESSIONS so that only people from your site can delete users. Also simply passing the id to the PHP file is very insecure as ANYONE could simply create a link to your php file on their site and delete users.

Therefore try this to fix your code (with added security):

PLEASE NOTE: I am aware that this may not be the best way nor the worst but it is a fairly secure method that works well.

Your main page, index.php:

<?php
session_start();
// Create a new random CSRF token.
if (! isset($_SESSION['csrf_token'])) {
    $_SESSION['csrf_token'] = base64_encode(openssl_random_pseudo_bytes(32));
}
// Check a POST is valid.
if (isset($_POST['csrf_token']) && $_POST['csrf_token'] === $_SESSION['csrf_token']) {
    // POST data is valid.
}
?>
...
<form id="delete_user_form" action="delete_user.php" method="post">
    <input type="hidden" name="user_id" value="<?php echo $user_name; ?>" />
    <input type="hidden" name="csrf_token" value="<?php echo $_SESSION['csrf_token']; ?>" />
    <input type="submit" value="Delete User" />
</form>

In your .js file (make sure you have jQuery linked):

window.csrf = { csrf_token: $("input[name= csrf_token]").val() };
$.ajaxSetup({
    data: window.csrf
});
$("#delete_user_form").submit(function(event) {
    event.preventDefault(); //Stops the form from submitting
    // CSRF token is now automatically merged in AJAX request data.
    $.post('delete_user.php', { user_id: $("input[name=user_id]").val() }, function(data) {
        //When it it's complete this is run
        console.log(data); //With this you can create a success or error message element
    });
});

Now for your delete_user.php file, this should fix the errors:

<?php
session_start();
require "connection.php";

// Checks if csrf_token is valid
if (isset($_POST['csrf_token']) && $_POST['csrf_token'] === $_SESSION['csrf_token']) {
    if(isset($_POST['user_id']) && $_POST['user_id'] != ""){

        $user_name = $_POST['user_id'];

        $sql = "DELETE FROM users WHERE user_name = '$user_name' LIMIT 1"; //LIMIT 1 only allows 1 record to be deleted

        if ($conn->query($sql) === TRUE) {
            echo "Record deleted successfully"; //You get this in your javascript output data variable
        } else {
            echo "Error deleting record: " . $conn->error; //You get this in your javascript output data variable
        }

        $conn->close();

    }
}
?>

I don't know what your connection.php contains so this is what I'd put in it:

$servername = "localhost";
$username = "username";
$password = "password";
$dbname = "myDB";

// Create connection
$conn = new mysqli($servername, $username, $password, $dbname);
// Check connection
if ($conn->connect_error) {
    die("Connection failed: " . $conn->connect_error);
}