PHP User Login Failure Handling

Question

On each page in an application I have a check to see whether a user is logged in. I recently realized my script was not structured well and made some changes. I am wondering if this new method implements the correct order of operations for a user that is not logged in.

<?php
ob_start();
session_start();

if ($_SESSION['loggedin'] !== true) {
    $_SESSION['messages'][] = '<li>User Not Logged In</li>';
    session_write_close();
    ob_end_clean();
    header('Location: login.php');
    exit;
}
else {
    // execute page
}
?>

Prior to this script, the ob_start() call was below the login check section and therefore was causing redirect issues given that session_start() produces its own headers.

I am also interested in knowing whether the script provides adequate security for a login check.

You should not need `ob_start();` at the top unless the script is outputting something to the browser before the redirect. Are you including this script in your pages and does the including script send output to the browser before this script is called? — jeroen, Jun 29 '13 at 20:21
@jeroen `session_start()` outputs to the browser by default, so AFAIK I need the `ob_start()` to make sure the `header()` call gets executed. Other than the session this script at the very start. [Explanation of Session Output and Redirects](http://stackoverflow.com/questions/8028957/headers-already-sent-by-php/8028987#8028987) — The Thirsty Ape, Jun 29 '13 at 20:47
No, you cannot start the session if the headers already have been sent. That does not mean that `session_start` is sending the headers. — jeroen, Jun 30 '13 at 01:24

score 1 · Accepted Answer · answered Jun 29 '13 at 20:27

1

This part of code is complete and secure but you have to mention few things for more security you need to regenerate the session ID with session_regenerate_id after putting valuable data like in 'loggedin' on session.

And I think it is better to put the IF part on a function and omit the else it helps your code be simpler. and Also you can remove the following lines:

session_write_close();
ob_end_clean();

answered Jun 29 '13 at 20:27

Ghasrfakhri

130
10

function makes sense. `session_regenerate_id` is included in the login script. The `ob_end_clean()` was added to prevent output of any data to unauthorized users. Why would I remove the other? – The Thirsty Ape Jun 29 '13 at 20:44
1

You have to check for authenticate before doing anything. If check first you don't have any thing to hide. – Ghasrfakhri Jun 30 '13 at 05:20

PHP User Login Failure Handling

1 Answers1