Simple password protection in PHP

Question

I was reading a quite interesting chapter in a "Learning PHP"-book of mine and came across a code example which I wanted to modify and use on my personal website (to protect a simple document, nothing "big", which is why I also don't encrypt the passwords).

I've used the php-sample and I just can't make it work at all.
Here it is (dont get scared by the length, it's really simple):

<?php

if ($_POST['_submit_check']) {
    if ($form_errors = validate_form()) {
        show_form($form_errors);
    } else {
        process_form();
    }
} else {
    show_form();
}

function show_form($errors = '') {
    echo '<form method="POST" action="' . $_SERVER['PHP_SELF'] . '">';

    if ($errors) {
        echo '<br>'
        echo implode('<br>', $errors);
        echo '<br>';
    }

    echo 'Username: ';
    echo '<input type="text" name="username" value="Username goes here">';
    echo '<br>'

    echo 'Password: ';
    echo '<input type="password" name="password">';
    echo '<br>'

    echo '<input type="submit" name="submit" value="Log In">';
    echo '<input type="hidden" name="_submit_check" value="1">'; //when the form is entered, this returns true and the first line of the document is good to go

    echo '</form>';
}

function validate_form() {
    $errors = array();

    $users = array('admin' => 'pass123',
                   'notsoadmin' => 'pass1234');


    if (!array_key_exists($_POST['username']) {
        $errors[] = "Please enter username and password";
    }

    $saved_password = $users[ $_POST['password'] ];
    if ($saved_password != $_POST['password']) {
        echo "Password and username don't match. Please try again";
    }

    return $errors;
}

function process_form() {
    $_SESSION['username'] = $_POST['username'];

    echo "Welcome, $_SESSION[username]";
}

?>

Before my HTML and stuff I also added this:

<?php session_start(); ?>

Clearly I've missed something... Maybe it's $form_errors right at the beginning, that causes the problem (which is "nothing happens"), it was in my book but I'm not sure why/where it comes from?

What were you expecting to see? What happened instead? (y'know, "it's not working" and "nothing happens" is kind of ... vague) — Piskvor left the building, Sep 09 '10 at 13:29
You're right, not very clear... Well, basically, what I want is, that if the form has been submitted lately, show the "Welcome" thing. If it has not been submitted, display the form. If the form is submitted with no errors, no problems. If it is submitted with errors, display the form once again with an "improvised error log" — Latze, Sep 09 '10 at 17:27

score 5 · Accepted Answer · edited May 23 '17 at 12:07

5

Shouldn't...

    $saved_password = $users[ $_POST['password'] ];
    if ($saved_password != $_POST['password']) {
        ...
    }

actually be..

    $saved_password = $users[ $_POST['username'] ];
    if ($saved_password != $_POST['password']) {
        ...
    }

i.e. you should be looking for username entry in $users not the password

By the way it's really bad practice to store raw passwords like that. Consider HASHing and SALTing them.

Check this question out for information

edited May 23 '17 at 12:07

Community

1
1

answered Sep 09 '10 at 13:38

irishbuzz

2,420
1
19
16

Not to be a stickler, but you meant hashing along with a salt, right? Say, `sha1()`. – Fanis Hatzidakis Sep 09 '10 at 19:05
@Fanis Indeed. I actually though I had mentioned both. Thanks. – irishbuzz Sep 10 '10 at 09:18

bigstylee · Answer 2 · 2010-09-09T13:43:34.957

During validation you should be a little more explict to what you are checking. Always avoid if ($variable) and instead use a function (isset/empty/etc) to check the state of the variable.

if ($_POST['_submit_check']) {
    if ($form_errors = validate_form()) { //always returns an array so will evaluate to true
        show_form($form_errors);
    } else {
        process_form();
    }
} else {
    show_form();
}

//change to

if (isset($_POST['_submit_check'])) {
    $form_errors = validate_form();
    if (!empty($form_errors)) {
        show_form($form_errors);
    } else {
        process_form();
    }
} else {
    show_form();
}

score 1 · Answer 3 · answered Sep 09 '10 at 13:39

$saved_password = $users[ $_POST['password'] ];
if ($saved_password != $_POST['password']) {
    echo "Password and username don't match. Please try again";
}

I think the above will not work because $users is an array of username => password. You need to check for the username key:

$saved_password = $users[$_POST['username']] ;

score 1 · Answer 4 · answered Sep 09 '10 at 13:55

1

Being the security freak that I am, it would also be a good idea to use hashing to protect your script from being cracked through a vulnerability in your server. Consider something like the sha1() hash; it's very fast and safe.

answered Sep 09 '10 at 13:55

mattbasta

13,492
9
47
68

Simple password protection in PHP

4 Answers4