mysqli Logout not updating Database

[Moorcam]

Hi guys,

I have the following logout code, which works just fine, as in it logs the user out and kills the session etc. However, there is one part that is not working and that is updating the database to change the is_logged_in to set to 0 rather than 1, which is set upon login.

<?php
session_start(); // Start the session

// Include database connection
require_once('includes/config.php');

// Check if user is logged in
if (!empty($_SESSION['user_id'])) {
    logoutUser($conn, $_SESSION['user_id']);
} else {
    redirectToLogin();
}

// Close the database connection
$conn->close();

/**
 * Logs out the user by updating their login status and destroying the session.
 *
 * @param mysqli $conn The database connection.
 * @param int $user_id The ID of the user to log out.
 */
function logoutUser($conn, $user_id) {
    // Prepare statement to update user login status
    $stmt = $conn->prepare("UPDATE users SET is_logged_in = ? WHERE user_id = ?");
    $is_logged_in = '0'; // Set user status to logged out
    $stmt->bind_param("si", $is_logged_in, $user_id);

    // Execute the statement
    if ($stmt->execute()) {
        // Destroy the session
        session_unset();
        session_destroy();
        redirectToLogin();
    } else {
        echo "Error updating user status: " . $stmt->error;
    }

    // Close the statement
    $stmt->close();
}

/**
 * Redirects the user to the login page.
 */
function redirectToLogin() {
    header("Location: login.php");
    exit();
}
?>

If anyone can help that would be great.

Thanks

[Barand]

Shouldn't $user_id be $_SESSION['user_id'] in your bind_param() call? Do you have error reporting on?

Why don't you just ...

UPDATE users SET is_logged_in = 0 WHERE user_id = ?

... instead of creating another variable to bind?

[Moorcam]

Hi mate,

Thanks for the reply.

Have changed it to the following as you mentioned. Still the same:

<?php
session_start(); // Start the session

// Include database connection
require_once('includes/config.php');

// Check if user is logged in
if (!empty($_SESSION['user_id'])) {
    logoutUser($conn, $_SESSION['user_id']);
} else {
    redirectToLogin();
}

// Close the database connection
$conn->close();

/**
 * Logs out the user by updating their login status and destroying the session.
 *
 * @param mysqli $conn The database connection.
 * @param int $user_id The ID of the user to log out.
 */
function logoutUser($conn, $user_id) {
    // Prepare statement to update user login status
    $stmt = $conn->prepare("UPDATE users SET is_logged_in = 0 WHERE user_id = ?");
    $stmt->bind_param("i", $_SESSION['user_id']);

    // Execute the statement
    if ($stmt->execute()) {
        // Destroy the session
        session_unset();
        session_destroy();
        redirectToLogin();
    } else {
        echo "Error updating user status: " . $stmt->error;
    }

    // Close the statement
    $stmt->close();
}

/**
 * Redirects the user to the login page.
 */
function redirectToLogin() {
    header("Location: login.php");
    exit();
}
?>

I was binding because I just find it safer to do that when inserting or updating in particular.

[Barand]

Have told mysql to report errors? ...

mysqli_report(MYSQLI_REPORT_ERROR|MYSQLI_REPORT_STRICT);

just before you connect to your DB

[Moorcam]

I have that set globally in config.php

// CHECK FOR ERRORS
ini_set('display_errors', 1);
ini_set('display_startup_errors', 1);
error_reporting(E_ALL);

 

[Barand]

What you have there won't report mysql errors, only php

[Moorcam]

Barand,

Remember when I joined here first and I made it clear that I am an old Irish fart?

Well, today defines exactly what I meant by that.

I was updating the table "users" and should have been updating the table "staff".

// Prepare the SQL statement to update the user's login status
$stmt = $conn->prepare("UPDATE staff SET is_logged_in = 0 WHERE user_id = ?");
$stmt->bind_param("i", $userId);

// Execute the statement
if ($stmt->execute()) {
        // Destroy the session
        session_unset();
        session_destroy();
        redirectToLogin();
    } else {
        echo "Error updating user status: " . $stmt->error;
    }

    // Close the statement
    $stmt->close();
}

I will now go to bed as it is 12:53 on Sunday morning here. Maybe I just need sleep thing?

Good night

[mac_gyver]

you should only have one user/authentication database table. a staff member is a user with specific permissions/privileges (and you should query on each page request to get the user's current permissions.) what you currently have either duplicates user id's and/or has a extra code/queries in it.

some other points, most of which simplifies what you are doing -

1. because logoutUser() is a function, you WOULD use the $user_id input parameter in the code, like it was written.
2. you apparently have two different functions, since the last code is using $userId as the input parameter.
3. you should be using php8+, where the default setting is to use exceptions for database errors.
4. when using exceptions, if the code continues past any statement that can throw an exception (connection, query, exec, prepare, and execute), you know that no error occurred and you don't need conditional logic to confirm so.
5. you should never unconditionally output raw database errors onto a live web page, where it gives hackers useful feedback. when using exceptions, simply do nothing in your code for most errors and let php catch and handle the exception, where php will use its error related settings to control what happens with the actual error information (uncaught database exceptions will 'automatically' get displayed/logged by php.)
6. a session can hold things other than the user's id. the logout code should only unset that specific session variable.
7. the redirect you preform upon successful completion of post method form processing code MUST be to the exact same URL of the current page to cause a get request for that page. this will prevent the browser from trying to resubmit the form data should that page get browsed back to or reloaded. for a logout this doesn't matter, but for something like registration/login, if you browse back to the form's action page, you can use the browser's developer tools to look at the data that the browser resubmits and see what the username/password is.
8. there's no need to close database connections in your code since php destroys all resources when you script ends. in fact the close statement you have shown is never reached since your current logic exits in all cases before ever reaching it.
9. you cannot set display_startup_errors in your code because php has already started by the time your code runs.

 

[Moorcam]

7 hours ago, mac_gyver said:

you should only have one user/authentication database table. a staff member is a user with specific permissions/privileges (and you should query on each page request to get the user's current permissions.) what you currently have either duplicates user id's and/or has a extra code/queries in it.

some other points, most of which simplifies what you are doing -

1. because logoutUser() is a function, you WOULD use the $user_id input parameter in the code, like it was written.
2. you apparently have two different functions, since the last code is using $userId as the input parameter.
3. you should be using php8+, where the default setting is to use exceptions for database errors.
4. when using exceptions, if the code continues past any statement that can throw an exception (connection, query, exec, prepare, and execute), you know that no error occurred and you don't need conditional logic to confirm so.
5. you should never unconditionally output raw database errors onto a live web page, where it gives hackers useful feedback. when using exceptions, simply do nothing in your code for most errors and let php catch and handle the exception, where php will use its error related settings to control what happens with the actual error information (uncaught database exceptions will 'automatically' get displayed/logged by php.)
6. a session can hold things other than the user's id. the logout code should only unset that specific session variable.
7. the redirect you preform upon successful completion of post method form processing code MUST be to the exact same URL of the current page to cause a get request for that page. this will prevent the browser from trying to resubmit the form data should that page get browsed back to or reloaded. for a logout this doesn't matter, but for something like registration/login, if you browse back to the form's action page, you can use the browser's developer tools to look at the data that the browser resubmits and see what the username/password is.
8. there's no need to close database connections in your code since php destroys all resources when you script ends. in fact the close statement you have shown is never reached since your current logic exits in all cases before ever reaching it.
9. you cannot set display_startup_errors in your code because php has already started by the time your code runs.

 

Thank you kind sir.

I will look more into all this.

The reason there are two separate logins is because there are two separate panels. One for staff and one for members. I know I could do something like:

if($_SESSION['role'] == 'admin'){
header("location: index.php");
}elseif($_SESSION['role'] = ='member'){
header("location: members.php");
}

Thanks for the input. Always appreciated.

[maxxd]

Each login is still dealing with a user of some role. You shouldn't be concerned with role at login, you should be concerned with role at display time. In other words everybody logs in via the same form and when the user accesses a page, what's displayed is dependent upon the role. So if there's an admin section of the menu, that's only shown to admins. And if there are pages that only mods or admin supposed to see, the user role is checked and the redirect happens at that point instead of at the login.

[Moorcam]

1 hour ago, maxxd said:

Each login is still dealing with a user of some role. You shouldn't be concerned with role at login, you should be concerned with role at display time. In other words everybody logs in via the same form and when the user accesses a page, what's displayed is dependent upon the role. So if there's an admin section of the menu, that's only shown to admins. And if there are pages that only mods or admin supposed to see, the user role is checked and the redirect happens at that point instead of at the login.

Thank you.

You are all right. I will get onto this. Not hard to do. Still a bit of a learning curve with prepared statements etc. but getting there