php sessions to authenticate user on login form

Question

I have the following code designed to begin a session and store username/password data, and if nothing is submitted, or no session data stored, redirect to a fail page.

session_start();
if(isset($_POST['username']) || isset($_POST['password'])) {
    $username = $_POST['username'];
    $password = $_POST['password'];
    $_SESSION['username'] = $username;
    $_SESSION['password'] = $password;
}

if(isset($_SESSION['username']) || isset($_SESSION['password'])){
    $navbar = "1";
    $logindisplay = "0";
    $username = $_SESSION['username'];
    $password = $_SESSION['password'];
} else {
    header('Location:http://website.com/fail.php');
}

$authed = auth($username, $password);
if( $authed == "0" ){
    header('Location:http://website.com/fail.php');
}

Its not working the way it should and is redirecting me to fail even though i submitted my info and stored it in the session. Am i doing something wrong?

NOTE the authed function worked fine before i added the session code.

Answer 1

First, don't store the password in the session. It's a bad thing. Second, don't store the username in the session until after you have authenticated.

Try the following:

<?php

session_start();

if (isset($_POST['username']) && isset($_POST['password'])) {
    $username = $_POST['username'];
    $password = $_POST['password'];
    $authed = auth($username, $password);

    if (! $authed) {
        header('Location: http://website.com/fail.php');
    } else {
        $_SESSION['username'] = $username;
    }
}

if (isset($_SESSION['username'])) {
    $navbar = 1;
    $logindisplay = 0;
} else {
    header ('Location: http://website.com/fail.php');
}

Answer 2

Just some random points, even though they may not actually pertain to the problem:

• Don't store the password in plaintext in the session. Only evaluate if the password is okay, then store loggedIn = true or something like that in the session.

• Check if the password and the username are $_POSTed, not || (or).

• Don't pass password and username back and forth between $password and $_SESSION['password']. Decide on one place to keep the data and leave it there.

• Did you check if you can store anything at all in the session? Cookies okay etc...?

To greatly simplify your code, isn't this all you need to do?

if (isset($_POST['username'] && isset($_POST['password'])) {
    if (auth($_POST['username'], $_POST['password'])) {
        $_SESSION['user'] = /* userid or name or token or something */;
        header(/* to next page */);
    } else {
        // display "User credentials incorrect", stay on login form
    }
} else {
    // optionally: display "please fill out all fields"
}

Answer 3

The solution to my specific problem above

session_start();
if(isset($_POST['username']) || isset($_POST['password'])){
$username = $_POST['username'];
$password = $_POST['password'];
$_SESSION['username'] = $username;
$_SESSION['password'] = $password;
}

if(isset($_SESSION['username']) || isset($_SESSION['password'])){
$navbar = "1";
$logindisplay = "0";
$username = $_SESSION['username'];
$password = $_SESSION['password'];
$authed = auth($username, $password);
if( $authed == "0" ){
header('Location:http://website.com/fail.php');
}
} else {
header('Location:http://website.com/fail.php');
}

Answer 4

Here are a few other things, which may or may not help you, by the way :

• Do you have error_reporting on ? (see also)
• Do you have display_errors on ?
• Is session_start the first thing you are doing in your page ? There must be nothing output before
• Are the cookies created on the client-side ?
• header Location indicates the browser it has to go to another page ; it doesn't stop the execution of the PHP script. You might want to (almost always anyway) add "exit" after it.

Answer 5

Headers are not function calls. They put a directive into the HTTP headers, and the last one to execute is the one which will be processed. So let say if you have something like this

if ($bAuthed)
{
     header("location: login.php");
}

// error case
header("location: error-login.php");

You will always be redirected to error-login.php no matter what happens. Headers are not function calls!

Answer 6

what about using this to setup session

session_start();
if(isset($_POST['username']) && isset($_POST['password'])
{
    if(auth($_POST['username'], $_POST['password']))
    {
        // auth okay, setup session
        $_SESSION['user'] = $_POST['username'];
        // redirect to required page
        header("Location: index.php");
     } else {
        // didn't auth go back to loginform
        header("Location: loginform.html");
     }
 } else {
     // username and password not given so go back to login
     header("Location: loginform.html");
 }

and at the top of each "secure" page use

session_start();
session_regenerate_id();
if(!isset($_SESSION['user']) // if there is no valid session
{
    header("Location: loginform.html");
}

this keeps a very small amount of code at the top of each page instead of running the full auth at the top of every page. To logout of the session:

session_start();
unset($_SESSION['user']);
session_destroy();
header("Location: loginform.html");