Username keeps displaying after logout

Hey guys! I finally got my login system work on most pages of my website. However, there is one page on the website called advisory (advisory.php). It’s basically supposed to be a forum. I wanted the ‘name’ input there to display username when the user is logged in or display ‘Anonymous’ when the user is logged out. However, I tried to set a function

function getUsername() {
return isset($_SESSION[‘username’]) ? $_SESSION[‘username’] : ‘Anonymous’;
}

and then simply put .getUsername(). in the ’ name’ input in the form (advisory.php). But this didn’t work out. So, I temporarily put .($_SESSION[‘username’]). in the ‘name’ field.
But it turns out once the user is logged in, the name in comments is displayed as username (as it’s supposed to be) but when the user logs out, the name keeps displaying there with each new comment, as if the user was still logged in.
Do you guys please have any idea how can I fix this? Many thanks.

advisory.php :

<?php
session_start();

date_default_timezone_set('Europe/Berlin');

include "advisory-sql.php";

include "advisory-comments.php";
 // some html stuff //
    echo "<form method='POST' action='".setComments($conn)."'>
        <input type='hidden' name='name' value='".($_SESSION['username'])."'>
        <input type='hidden' name='date' value='".date('Y-m-d | H:i:s')."'>
        <textarea name='message' cols='30' rows='10'></textarea><br>
        <input type='submit' name='submitComment' value='Submit'><br>
    </form>";

    getComments($conn);
?>

advisory-comments.php:

<?php
function setComments($conn) {
if (isset($_POST['submitComment'])) { $name = $_POST['name']; 
$date = $_POST['date'];
$message = $_POST['message']; 

$_SESSION['name'] = $name;

$sql = "INSERT INTO advisory (name, date, message) VALUES ('$name', '$date', '$message')";


$result = $conn->query($sql); 

}

}

function getComments($conn) {
$sql = "SELECT * FROM advisory ORDER BY id DESC LIMIT 5";
$result = $conn->query($sql);
while ($row = mysqli_fetch_assoc($result)) {

    // if(!isset($_SESSION['username']) || empty($_SESSION['username'])) {
    //     echo "$_SESSION['name']";
    // } else {
    //     echo "$_SESSION['username']";
    // }


    echo "<div class='comment_box'><p>";
        echo $row['name']."<br>";
        echo $row['date']."<br>";
        echo nl2br($row['message']);
    echo"</p></div>";
}

}

function getUsername() {
return isset($_SESSION['username']) ? $_SESSION['username'] : 'Anonymous';

}

?>

login.php:

<?php 
include('server.php') 
?>
<div class="form-inputs-page">
            <input type="text" name="username" placeholder="username"><br>
        
            <input type="password" name="password" placeholder="password"><br>

            <div class="forget-page">    
                <a href="forgot.php">Forgot password?</a>
            </div><br> 

            <button type="submit" class="sbmt_log" name="login_user">Odeslat</button>
        </div>
        
    <div class="createacc-page">
        <a href="register.php">
            Click here to register!
        </a>
    </div>

server.php:

<?php

session_start();

$name = "";
$username = "";
$email    = "";
$errors = array();
$_SESSION['success'] = "";

$db = mysqli_connect('localhost', 'root', '', 'registration');

if (isset($_POST['reg_user'])) {

$name = mysqli_real_escape_string($db, $_POST['name']);
$username = mysqli_real_escape_string($db, $_POST['username']);
$email = mysqli_real_escape_string($db, $_POST['email']);
$password_1 = mysqli_real_escape_string($db, $_POST['password_1']);
$password_2 = mysqli_real_escape_string($db, $_POST['password_2']);

if (empty($name)) { array_push($errors, "Your name is required"); }
if (empty($username)) { array_push($errors, "Username is required"); }
if (empty($email)) { array_push($errors, "Email is required"); }
if (empty($password_1)) { array_push($errors, "Password is required"); }

if ($password_1 != $password_2) {
    array_push($errors, "The two passwords do not match");
}

if (count($errors) == 0) {
     
    $password = md5($password_1);
    
    $query = "INSERT INTO users (`name`, `username`, `email`, `password`)
              VALUES('$name', '$username', '$email', '$password')";
    
    mysqli_query($db, $query);

    $_SESSION['username'] = $username;
    
    $_SESSION['success'] = "You have been successfully registered";
    
    header('location: welcome.php');

    exit();
}

}

                // USER LOGIN //
                
if (isset($_POST['login_user'])) {
 
$username = mysqli_real_escape_string($db, $_POST['username']);
$password = mysqli_real_escape_string($db, $_POST['password']);

if (empty($username)) {
    array_push($errors, "Username is required");
}

if (empty($password)) {
    array_push($errors, "Password is required");
}

if (count($errors) == 0) {

    $password = md5($password);
    
    
    $query = "SELECT * FROM users WHERE username=
            '$username' AND password='$password'";
    $results = mysqli_query($db, $query);

    if (mysqli_num_rows($results) == 1) {
         
        $_SESSION['username'] = $username;
         
        $_SESSION['success'] = "You have logged in!";
         
        header('location: welcome.php');

        exit();

    } else {
         
        array_push($errors, "Username or password incorrect");
    }
}

}

 ?>

Dont take the name from a form field.

If the username is in the session, use it; otherwise, use Anon.

Just to be sure I understand, you want the logged in user to see who created the comment, otherwise it’s supposed to show Anon? If so, this

 echo "<div class='comment_box'><p>";
        echo $row['name']."<br>";
        echo $row['date']."<br>";
        echo nl2br($row['message']);
    echo"</p></div>";

needs to be

        echo "<div class='comment_box'><p>";
        if (isset($_SESSION['username']) {
          echo $row['name']."<br>";
        } else {
          echo "Anon<br>";
        }
        echo $row['date']."<br>";
        echo nl2br($row['message']);
        echo"</p></div>";
1 Like

Wouldn’t that make… all comments appear to be from the logged in user, rather than the person who… yaknow… posted it?

:shifty:

Why would it do that? It’s checking for the existence of the session variable but using the db object to display…

Ohhhh i see.

The OP seems to be talking about the input though? I guess we need a better description of the expectation.

I can see the confusion OP’s first paragraph says one thing, second something else. :person_shrugging:

But the person recorded as posting it, isn’t necessarily the person who posted it. :grimacing:

There were 10 good points made there, how many did you take notice of?

1 Like

There’s a lot of things I could say about this approach you have.

This approach doesn’t make a lot of sense to me tbh. If you’re already logged in, why are you passing the value into a form for the user to submit only to send that exact value to the next screen? Shouldn’t your next screen be able to be smart enough to know if the user is logged in? This is why I dislike the use of “hidden” fields. It just makes no sense.

This is definitely a no-no. Use the default password_hash(); function instead. To verify if a password is valid, use password_verify() against both the user inputted password and the password stored in the database. When doing this, you should only need to do your WHERE clause on the email or username. Not on the password since you can’t select the hashed password because it’s already hashed and what you’re going to be running it against will be a unhashed raw password.

2 Likes

Thank you! Actually, I want everyone to see the author of the comment, logged in or not… but I want the author to be either username (logged in) or Anonymous (not logged in)

This is exactly the point.
The user name should never be coming from a form input in the first place.
In processing the form after submission, don’t be looking for the user in $_POST, take it from $_SESSION there.
Though when storing who posted with the comment, use their ID, not the name, as it’s the one solid identifier that won’t change.

This is not to mention all the other unspeakable archaic badness that exists in the code above.

1 Like

I see you are defining $_SESSION['username'] within the setComments() function. It should not be set there. When the user LOGS IN is where $_SESSION['username'] should be set and when a user LOGS OUT, $_SESSION['username'] should be unset.

unset($_SESSION['username']);

Then the getUsername() function should works as it should.

Note: You’ve probably already been told about using plain passwords.

1 Like

Honestly, I personally wouldn’t store any values in “hidden” fields or any type of fields. The only exception would be to store the values a user had previously typed in. For instance, if they’re putting in a title to a blog article and had submitted that blog article, but got redirected back due to an error. I would say that’s a good reason to put $_SESSION values into a text field. Otherwise anything else doesn’t really make much sense. If you wanted to pass someone’s user ID, why can’t you do that on the page the user is accessing on the PHP side? For instance, if you know the user is logged in and you got the username, why can’t you get the user ID using that username from either the session or from the database on the page you’re trying to access? Why are we passing values that you can easily get and passing them from one page to another to another to another to another? It’s redundant work IMO. The whole concept needs to be re-thought out since you can reduce a lot of bloat from just good logic.


Also, a note to take is that anything on the webpage can be altered and submitted. So say for instance you’re logged in as “Sam Smith” with a user ID of 123. If someone malicious came by and changed the text field to “John Parker” with a user ID of 124 and that user actually exists, now you’re submitting a comment as “John Parker” and when the real John Parker comes, he’s going to say “Hey, I didn’t submit this comment though.” Now you have a security hole you just opened up.

This is why I don’t think login systems are something beginners want to try and tackle. You got to think about every malicious intent and try to combat it. That’s why I never trust anything on the webpage because it can be altered since that’s all client side. Anyone can add random text fields that aren’t even supposed to be there to see what breaks your application.

1 Like

Thanks a lot to all of you guys for your help and advices. I went with this:

function getComments($conn) {
$sql = "SELECT * FROM advisory ORDER BY comment_id DESC LIMIT 5";
$result = $conn->query($sql);
while ($row = mysqli_fetch_assoc($result)) {
    echo "<div class='comment_box'><p>";
    if (isset($_SESSION['username'])) { 
        echo $_SESSION['username']."<br>";
        } else { 
            echo $row['name']."<br>";
        }
        echo $row['date']."<br>";
        echo nl2br($row['message']);
    echo"</p></div>";
    }
}

edited forum form this way:

  echo "<form method='POST' action='".setComments($conn)."'>
        <input type='hidden' name='date' value='".date('Y-m-d | H:i:s')."'>
        <input type='text' name='name' value='' placeholder='Your name'><br>
        <textarea name='message' cols='30' rows='10'></textarea><br>
        <input type='submit' name='submitComment' value='Submit'><br>
    </form>";

    getComments($conn);

it actually works except once another user logs in, all the comments appear to be from the logged in user and once the user logs out, the comments posted from the user who was logged in have empty space instead of the user’s name.

So, I tried to solve this by creating a function that would actually post the username according to the user’s id… I came up with this:

 <?php

 include 'server.php';
 $id="";

 $sql = "SELECT id, username FROM users WHERE id=
 '$id' AND username='$username'";

 $results=mysqli_query($db,$sql);

 if (isset($id)) { 
 echo $_SESSION['username']."<br>";
 } else {
 echo $row['name']."<br>";
 }
 ?>

it partly works but when I put it into function, so I could use it in the form, it stops working at all.
Any help here, please? Thank you again!

That’s exactly what I’m saying. When I say store the ID, I’m taking about in the table that stores comments (advisory), not the form, there is no need for it in the form at all.

id   |   user_id  |  datetime   |   message

The User ID can easily be taken from the $_SESSION when processing the form. It has no place in $_POST data.

In that case you would use the $_POST data to fill the inputs with what was initially submitted to save the user starting over with the form and correct only the input with an error. There would be no redirect on error, but the form presented again after processing fails due to user error.

1 Like

Show us how you “put it into function”, please. In what way does it “partly work”? Why does your query need to have a WHERE clause for both the id and the username? Surely the id is enough to identify the user.

1 Like

These 2 fields can honestly be processed when you’re inserting into your comments table. I don’t really understand why you’re putting them in “hidden” fields or allowing the user to make modifications to the inputs. If you’re using the text field for anonymous users, that poses a security concern. This can allow bots to spam your comments and even try to insert malicious content like SQL injection. Seeing how you’re not properly using prepared statements, this will definitely introduce SQL injection without a doubt.

1 Like

The OP was already given all this, now repeated information, in a previous thread, including not passing values through hidden fields due to the unsecure nature of doing so and the fact that putting a php function call in the form’s action='".setComments($conn)."' attribute doesn’t call that function when the form is submitted.

Soldier on.

1 Like

I did already mention the points you made in the other topic, most of which have been totally ignored.

Again as pointed out, there is no need for this, it’s only more redundant code that make more work for the server and opens you up to data tampering. Like the user data in the session, you can get the current time any time, as in after form submission. Or better still have your database column set to default to the current time, so you don’t even need to think about it.

Though on an side note, about storing current times in forms. I do record the time a user loads a form and store it in the session (not an input). I then get the time of submission, then compare the two for the difference. It’s a very effective way of trapping spam bots.
And you will get spam if you are letting unlogged users comment.

2 Likes

This topic was automatically closed 91 days after the last reply. New replies are no longer allowed.