Simple PHP Multi User login, different pages depending on user - what's wrong here?

Question

I was looking for a simple PHP script + form that routed users to specific URLs based on the username and password entered.

Code at the top of the page:

<?php 
session_start();
$data=array("user1"=>array("url"=>"file1.php","password"=>"pass1"),
"user2"=>array("url"=>"file2.php","password"=>"pass2"));

if(isset($_POST['username']) && isset($_POST['password'])) {
    if($data[$_POST['username']]['password'] == $_POST['password']) {
        $_SESSION['username'] = $_POST['username'] . " " . $_POST['password'];
        header('Location: ' . $data[$_POST['username']]['url']);

        login('<p class="alert">Incorrect username or password.</p> ');
    }
} else {
    login();
}
?><?php
function login($response='Please enter your username and password.') {
?>

This is in the body:

<p><?=$response?></p>
<form action="" method="post">
    <table width="400" border="0" cellspacing="0" cellpadding="4">
        <tr>
            <td width="90"><label class="loginform" for="username">Username:</label></td>
            <td width="294"><input name="username" type="text" /></td>
        </tr>
        <tr>
            <td><label for="password">Password:</label></td>
            <td><input name="password" type="password"></td>
        </tr>
        <tr>
            <td>&nbsp;</td>
            <td><input type="submit" value="Login" /></td>
        </tr>
    </table>
</form>
<?php } ?>

This code works properly, but if a user enters an incorrect password, it redirects to a blank page (specifically, it only loads the page after the last:

<?php } ?>

I understand this code is not written well at all, so that is the first problem. I'd really love to see a well-written version (just to learn and study it). But if that's not possible, how would I correct this code so an incorrect password doesn't break the page?

It's odd, because if you enter nothing (just hit Submit), it says "Incorrect username or password", and if you just enter an incorrect username with no password, same thing. But with a username + wrong password, or only wrong password, it goes to the blank page.

Thanks in advance!

CodeCaster · Accepted Answer · 2011-11-22T10:49:30.153

Another lesson why indentation is important. Have a look:

if(isset($_POST['username']) && isset($_POST['password'])) 
{
    if($data[$_POST['username']]['password'] == $_POST['password']) 
    {
        $_SESSION['username'] = $_POST['username'] . " " . $_POST['password'];
        header('Location: ' . $data[$_POST['username']]['url']);

        login('<p class="alert">Incorrect username or password.</p> ');
    }
} else {
    login();
}

Now you see. What happens if username and password are POST'ed, but not correct? Nothing, it'll exit both if-blocks.

This will do:

if(isset($_POST['username']) && isset($_POST['password'])) 
{
    if($data[$_POST['username']]['password'] == $_POST['password']) 
    {
        $_SESSION['username'] = $_POST['username'] . " " . $_POST['password'];
        header('Location: ' . $data[$_POST['username']]['url']);
        exit();
    }
    else
    {
        login('<p class="alert">Incorrect username or password.</p> ');
    }
} else {
    login();
}

Or, even better imho:

$warning = '';

if(isset($_POST['username']) && isset($_POST['password'])) 
{
    if($data[$_POST['username']]['password'] == $_POST['password']) 
    {
        $_SESSION['username'] = $_POST['username'] . " " . $_POST['password'];
        header('Location: ' . $data[$_POST['username']]['url']);
        exit();
    }
    else
    {
        $warning = '<p class="alert">Incorrect username or password.</p> ';
    }
}

login($warning);

And I hope I don't have to tell you that this is no protection whatsoever, unless you compare $_SESSION['username'] on every page after logging in. :-)

Thank you for your help. It now the reverse is true, if you enter an incorrect username, it blanks the page. Re: protection, it will route them to another PW-protected page. — user1059445, Nov 22 '11 at 10:55
@user1059445 Then [enable error reporting](http://stackoverflow.com/questions/74847/php-error-reporting-best-setting-for-development-e-strict). You might output data before the call to `header()`, and then the redirect doesn't take place. — CodeCaster, Nov 22 '11 at 11:02
Warning: Cannot modify header information - headers already sent by (output started at /home/website/public_html/login/index.php:8) in /home/website/public_html/login/index.php on line 11 — user1059445, Nov 22 '11 at 11:10
My apologizes, I'm sure it's abundantly clear that I lack PHP knowledge. I simply moved the to the top, now it doesn't blank out the page. It would have been nice to have an error message, but now at least it works. — user1059445, Nov 22 '11 at 11:41
@user1059445 that error is easily fixed, it's because you output something prior to calling `header()`. Put this code on the top of your file and it will work. — CodeCaster, Nov 22 '11 at 11:51

score 0 · Answer 2 · answered Nov 22 '11 at 10:43

Find the difference below :)

Tip: try to indent your brackets, then the mistake would be obvious in the first place.

original:

if(isset($_POST['username']) && isset($_POST['password'])) {
if($data[$_POST['username']]['password'] == $_POST['password']) {
$_SESSION['username'] = $_POST['username'] . " " . $_POST['password'];
header('Location: ' . $data[$_POST['username']]['url']);

login('<p class="alert">Incorrect username or password.</p> ');

}
} else {
login();
}

corrected:

if(isset($_POST['username']) && isset($_POST['password'])) {
if($data[$_POST['username']]['password'] == $_POST['password']) {
$_SESSION['username'] = $_POST['username'] . " " . $_POST['password'];
header('Location: ' . $data[$_POST['username']]['url']);

} else {
login('<p class="alert">Incorrect username or password.</p> ');
}
} else {
login();
}

Corrected version update: This seemed to switch the problem. Now user blanks the page. — user1059445, Nov 22 '11 at 10:46

Simple PHP Multi User login, different pages depending on user - what's wrong here?

2 Answers2