0

Now this one always bugs me!

I'm dealing with a lot of checks that result in redirection dependant on certain results.

Example:

1) If session exists and users lands on login.php they are pointed to index.php.

2) When landing on any page after loggining in, if their account isn't complete they're redirected to account-wizard.php to complete their profile.

3) When they land on a page it checks if they're authorised for that particular content. If not they are redirected to an upgrade page.

The list goes on...

I HATE USING <meta http-equiv="refresh" content="0; url=http://example.com/">!!! It's slow, and only redirects after loading all of the page, even when I use exit(); after.

I can't use header('Location: a-place.php'); because headers have already been called and with lots of them you end up with redirect errors and endless loops.

My PHP programming is VERY far from good but I'm sick of having poor code when it comes to these redirects.

What do you do!??! Thanks in advance for any support.

Edited by mmcdonald

5
Contributors
23
Replies
73
Views
4 Years
Discussion Span
Last Post by Atli
Featured Replies
  • 1
    Atli 182   4 Years Ago

    > I can't use header('Location: a-place.php'); because headers have already been called and with lots of them you end up with redirect errors and endless loops. Honestly, that strongly indicates poor programming practices are being used. The very first thing you should be doing on any page is making sure … Read More

  • 2
    diafol 3,720   4 Years Ago

    >How can I improve this? :) Use parametized queries. Nice to see mysqli though. Create the query with '?' as placeholders instead of your variables, then bind them using an array: $stmt = $mysqli->prepare("INSERT INTO CountryLanguage VALUES (?, ?, ?, ?)"); $stmt->bind_param('sssd', $code, $language, $official, $percent); That's from the manual … Read More

  • 1
    diafol 3,720   4 Years Ago

    Another problem is your query - no error handling, so any errors from this will prevent header() from executing. Have a look here: http://www.daniweb.com/web-development/php/code/434480/using-phpmysqli-with-error-checking Read More

0

I can't use header('Location: a-place.php'); because headers have already been called

This should be the way. Perhaps you have a header, but no exit. You can also store the redirect in a variable while checking, and redirect at the end of the checks. Perhaps you should show some code.

0

This is one example. It says if user isn't on the account wizard then check that they have completed their first name and surname. If first name or surname is blank in their profile they are pushed to the account wizard.

if (basename($_SERVER['PHP_SELF']) != "account-wizard.php"){

if ($userInfo['fn'] == null || $userInfo['ln'] == null || $userInfo['authemail'] == null){
    header('Location: account-wizard.php');
    }   
}

This checks if a session exists regarding the users login, if not then they are redirected to login.php. It only runs if they are not on login.php.

    if (basename($_SERVER['PHP_SELF']) != "login.php"){
        if (!isset($_SESSION['auth'])){
            header('Location: login.php');
            exit();
            }
        }

There are many more... Is there not a better way?

0

I like the idea of storing redirects as variables but I think that's only better for larger checks in one go, right? But lets see if anyone has any advice on the snippets above. Thanks peeps.

1

I can't use header('Location: a-place.php'); because headers have already been called and with lots of them you end up with redirect errors and endless loops.

Honestly, that strongly indicates poor programming practices are being used. The very first thing you should be doing on any page is making sure the user has proper access to it. Before anything else is done there. That should be happening long before PHP starts sending content, thus closing the headers.

More abstractly, for most programming in any language, you want to try to split your application into at least three layers: business logic, data interaction, and finally output generation. That last part should always be last, and this kind of access chekcing should be managed in the business logic layer. - This type of structure is commonly achieved in PHP using the popular MVC pattern. It's well worth looking into.

However, if for some unfathomable reason you are unable to set things up so that you can do this access/redirect checking before PHP outputs anything, you can utilize PHP's Output Buffering functions to force PHP to hold of on actually locking the headers until you are ready (or even until the script execution ends), thus allowing you to set headers at any time, regardless of what else is happening.

Votes + Comments
Very useful! Thanks for your advice.
0

@Atli,

Yeah I know my PHP is poor, very poor. I've purchased a book and booked a course, it's a personal goal to get much better.

I will start looking into MVC and I'll also revise output buffering.

Thanks for your advice!

M.

0

What about form submissions? That's actually where the worst of it all occurs.

Lets take a login form for example.

I have the form submission code at the bottom of the page, and after checking against the DB and creating the session it redirects the user to the members area.

Here's what I mean:

   <form class="form-vertical login-form" action="<?php $_SERVER['PHP_SELF']; ?>" method="post" >
        <h3 class="form-title">Login to your account</h3>
        <div class="alert alert-error hide">
            <button class="close" data-dismiss="alert"></button>
            <span>Email and password required.</span>
        </div>
        <div class="control-group">
            <!--ie8, ie9 does not support html5 placeholder, so we just show field title for that-->
            <label class="control-label visible-ie8 visible-ie9">Email</label>
            <div class="controls">
                <div class="input-icon left">
                    <i class="icon-envelope"></i>
                    <input class="m-wrap placeholder-no-fix" type="text" placeholder="Email" name="email" maxlength="50"/>
                </div>
            </div>
        </div>
        <div class="control-group">
            <label class="control-label visible-ie8 visible-ie9">Password</label>
            <div class="controls">
                <div class="input-icon left">
                    <i class="icon-lock"></i>
                    <input class="m-wrap placeholder-no-fix" type="password" placeholder="Password" name="password" maxlength="50"/>
                </div>
            </div>
        </div>
    </form>

    //THE PROCESSING SNIPPET:

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

        $email = $_POST['email'];      
        $password = md5($_POST['password']);

        if ($result = mysqli_query($mysqli, "SELECT id FROM users WHERE authemail = '$email' AND authpass = '$password'")) {

            $row_cnt = mysqli_num_rows($result);

                    if ($row_cnt != '1'){
                        $_SESSION['login_state'] = "f";
                        echo'<meta http-equiv="refresh" content="0">';
                        exit();
                    }else{
                        unset($_SESSION['login_state']); 
                        $_SESSION['auth'] = $email;
                        header('Location: index.php');
                    }

            mysqli_free_result($result);
        }
        mysqli_close($mysqli);
    }  

Now that header redirect to index.php often causes issues. Any ideas? I was always told to put the form processing code below the form (at the bottom of the page). Please advise :)

Edited by mmcdonald

0

I was always told to put the form processing code below the form

For what reason?

BTW, your login is open to injection attacks.

Edited by pritaeas

0

I don't know why anyone would advice you to do it like that, but it goes against the "standard" wisdom. Like I said above, the output generation is generally the last step, not the first. It just makes sense, don't you think, to perform the logic before you generate the output; to think before you speak. That way you can generate the output based on the logic, rather than just blurting out something that may well not make any sense, and then trying to correct it later once the logic catches up. - Like in your code, echoing a meta refresh header to relocate the user after a successfull login, who will still get a flash of the now unecessary form before that happens.

Sending the HTML before processing the form only really achieves one thing: locking the headers. You gain absolutely nothing.

0

Sending the HTML before processing the form only really achieves one thing: locking the headers. You gain absolutely nothing.

I know this. That's why I am here asking - to learn :) I'll change it all now. Thank you!

BTW, your login is open to injection attacks.

How can I improve this? :) Using prepared statements? :

$stmt = $dbConnection->prepare('SELECT * FROM employees WHERE name = ?');
$stmt->bind_param('s', $name);

$stmt->execute();

?

Edited by mmcdonald

2

How can I improve this? :)

Use parametized queries. Nice to see mysqli though. Create the query with '?' as placeholders instead of your variables, then bind them using an array:

$stmt = $mysqli->prepare("INSERT INTO CountryLanguage VALUES (?, ?, ?, ?)");
$stmt->bind_param('sssd', $code, $language, $official, $percent);

That's from the manual - give it a go, you'll find useful stuff in it.

Anyway, 'sssd' refers to string,string,string,double - 'typing' the variables

Regarding headers, with the rest here, there really shouldn't be a situation where you get that error. Any output from your php (including error messages) or html (including whitespace) prior to the header with mess it up. So, do yourself a favour and store any output into variables if need be as opposed to echoing them in situ. That means you can place your header after them without fear of error. I don't think that swotting up on output buffers is going to help you sort out your way of coding. WRT MVC - it's well worth learning about it, but IMO, unless you have a 'big enough project', it's probably not worth it. Just like OOP - some sites must have it or you end up with spaghetti code, while others can just rely on simple procedural code as OOPing it would just bring in needless complexity and bloat.

Edited by diafol

Votes + Comments
Awesome :)
0

Cheers Diafol that makes sense! Is that the same story with count and update queries? You just but the WHERE clause after the '?' placeholders closing bracket?

0

Sorry to be a pain but would you mind rewriinge the following snippet using prepared statements in the simplest possible form? I'll use it to ensure I fully understand the method and then I'll apply it to the rest of my queries. I learn best by viewing a working example and then breaking it down and understanding it.

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

        $email = $_POST['email'];      
        $password = md5($_POST['password']);

        if ($result = mysqli_query($mysqli, "SELECT id FROM users WHERE authemail = '$email' AND authpass = '$password'")) {

            $row_cnt = mysqli_num_rows($result);

                    if ($row_cnt != '1'){
                        $_SESSION['status'] = 3;
                        header('Location: login.php');
                    }else{
                        $_SESSION['auth'] = $email;
                        header('Location: index.php');
                    }

            mysqli_free_result($result);
        }
        mysqli_close($mysqli);
    }  

Many thanks!

Edited by mmcdonald

0

Is that the same story with count and update queries?

Have you read the php.net documentation of 'mysqli prepare'? Ok last freebie...

 UPDATE table SET field1 = ?, field2 = ? WHERE field20 = ?
0

Cheers :) I've got that locked in the book of tricks!

Here is my attempt, I'm troubleshooting now:

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

        $email = $_POST['email'];      
        $password = md5($_POST['password']);

        $stmt = $mysqli->prepare("SELECT id FROM users WHERE authemail = '?' AND authpass = '?'");
        $stmt->bind_param('ss', $email, $password);
        $stmt->execute();
        $stmt->store_result();

        $rows = $stmt->num_rows;

                    if ($rows != '1'){
                        $_SESSION['login_state'] = "f";
                        header('Location: login.php');
                        exit();
                    }else{
                        unset($_SESSION['login_state']); 
                        $_SESSION['auth'] = $email;
                        header('Location: index.php');
                }
            mysqli_close($mysqli);
        }

Edited by mmcdonald

0

Got the bugger working! :D

SO this is now safe from injections?

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

        $email = $_POST['email'];      
        $password = md5($_POST['password']);

        $stmt = $mysqli->prepare("SELECT `id` FROM `users` WHERE `authemail` = ? AND `authpass` = ?");
        $stmt->bind_param('ss', $email, $password);
        $stmt->execute();
        $stmt->store_result();

        $rows = $stmt->num_rows;

                    if ($rows != 1){
                        $_SESSION['login_state'] = "f";
                        header('Location: login.php');
                        exit();
                    }else{
                        unset($_SESSION['login_state']); 
                        $_SESSION['auth'] = $email;
                        header('Location: index.php');
                    }

            mysqli_free_result($stmt);
            mysqli_close($mysqli);
        }

Edited by mmcdonald

1

Here is my take on that issue. You've got the parametrized statement part down already, it seems, but I've included a few other useful tips in there.

<?php
// Never just check for submit buttons, or "check" values.
// Make sure the data you need exists, not some random value.
if(isset($_POST['email'], $_POST["password"])){
    // Using SHA1 because, honestly, I can't bring myself to use MD5
    // for password hashing. Not even in a trivial example.
    // I strongly suggest you switch, and that you look into salting!
    $passwordHash = hash("sha1", $_POST["password"]);

    // Just put ? in the query where the values should be placed.
    $sql = "SELECT id FROM users WHERE authemail = ? AND authpass = ?";
    $stmt = $mysqli->prepare($sql);

    // Then provide values for the question marks, in order.
    // Note that it's OK to put the email directly from the POST
    // here, because parametrized queries bypass SQL injection
    // completely. And there is no point copying it into a local
    // variable.
    $stmt->bind_param("ss", $_POST["email"], $passwordHash);

    // Execute it, making sure it returns TRUE. Otherwise it failed.
    if ($stmt->execute()) {
        // Use the num_rows to check how many rows were returned.
        if ($stmt->num_rows == 1) {
            unset($_SESSION['login_state']); 
            $_SESSION['auth'] = $email;
            header('Location: index.php');
        }
        else {
            $_SESSION['login_state'] = "f";

            // Then display the form again. If this is executed
            // before the form is displayed, there is no need
            // to refresh the page. Just display the form.
        }
    }
    else {
        // Log the error .
        trigger_error("Failed to check login: " . $stmt->error, E_USER_ERROR);
    }
}  
Votes + Comments
A great help! I have my working login script (see above) but I'm getting rid of the variables now and looking through the documentation for SHA and salting. I'm going to keep this snippet aside for revision every now and then. Very helpful!
0

Thanks! I had the script working above your response but your example has provided a lot of useful information. I've heard of salting before but never gave it time! Same goes with SHA. Working on them now.

+1

M.

1

Actually, although I used SHA1 there, it's little better than MD5 these days. It's better, no doubt, but it's still pretty weak. I'd recommend looking into something even stronger, like SHA512 or Whirlpool. - Good thing is that, with the hash and hash_hmac functions, it's just as easy to create those kind of hashes as it is to do MD5 and SHA1. Just have to change the first input parameter.

Also, if you are using PHP 5.5, you can take advantage of the new password_hash function. It'll create a very strong hash; a Bcrypt hash, with no effort on your part. - Even if you are not using 5.5 yet, there are other ways to make those functions available.

Votes + Comments
Insightful, which is very helpful when you're in the middle of a major learnign curve. Thanks!
0

you can also try implementing this. I have this script on my bench tester and I have been trying to hack into it for 3 weeks now, but without success. I think I made an overstatement here. It can be hack, there is no iron clad script.. eveyrthing can be hacked, but the hacker will need a super dooper fast computer to deliver the library within a fraction of a second.

The script will generate a derivative hash of a password only the script can validate if it is matched or not. Every time a password needs to be validated, the script generates a different derivative hash of the password, so for example "xaksdlljfll4seiolldld" is the stored hash for "password".. when the user type the password in the password form input, the script will hash it differently and not exactly the same as the stored password in the database.. the comparator can be something different like "awollisjd3cloepppwfeqwep". This approach provide a secure account information for the members, because even the site owner and database administrator will not be able to decypher it. This reminds me of a well known corporation who are willing to give me a two years of unlimited starbucks coffee supplies just to give them the unhashed password of their employees, but I would never never do that, even for 10 million dollars...moral values should kept us well grounded (for me ), regardless of how truly good we think we are.

Edited by veedeoo: info added

0

Yea, the PBKDF2 algorithm is also a good one. Much like bcrypt, it can be configured to take more time to generate a hash, thus avoiding the eventual death the "traditional" hashing methods faced as the processors got faster. Their failing was always that they were made to be fast, whereas PBKDF2 and bcrypt are made to be expensive to compute.

It's worth mentioning, though, that PBKDF2 is considered weaker that bcrypt; less suited for password hashing. The documentation for the new hash_pbkdf2 function (new in PHP 5.5) even makes a note of this, and recommends bcrypt instead for passwords.

when the user type the password in the password form input, the script will hash it differently and not exactly the same as the stored password in the database.

That doesn't make sense. How would it compare the stored hash and the hash made for the password you are comparing it to if the hashing method is different? You'd have to hash the input using the same exact method and parameters in order to get the same results.

0

Hi Atli,

I could honestly guess the responsible for comparison is the function slow_equals.

We can test its derivative hash function by doing a simple loop..

 for($x = 0; $x <= 20; $x++){

    echo $x.'. '.create_hash('password').'<br/>';

  }

the above codes provided hashes as shown below.. ( these are the possible derivatives for the the term 'passwords', when x is less than or equal to 20).

0. sha256:1000:egGeuoIEBpbH6tDFhnkFYx2smgOFYkxU:RxaIy45ly9fybHO2X6TbXGDzQQctNE/I
1. sha256:1000:MlqY39AXlcFPwhQQDOKv3dzyvsOskYuw:58KlEHhK/UEJiexe0/P/IZ92TenCG3d3
2. sha256:1000:qw7Kv3q+72sPKEl2usiMq3tLTXNH2XKx:5Xpw7xGaFVmZtCfpEFSgU6ZptTKJMQGa
3. sha256:1000:UgwkgqLkV+InVwP32XkMuhvHPlKlEcoC:VztVwrs5cJUUtEu9UcSTOW8bqjbxlazH
4. sha256:1000:noLwBZj+thmsgEWoCv1pP+DEVq3TOmyO:05qToWmwNrvdVMnD7v7CpuOznabkHBB0
5. sha256:1000:k+XKOwKwP95OCY9k9oFn6R4WIjAvS7Gg:AhAQdodA/d7CLQut1MNanMPhv9roKziX
6. sha256:1000:iKFYD2GuZavyXtPj1/Ke/fseDVD0UA4z:P9E8R7BgW8viot1WlobpdJ+UQ5Ejv7Ew
7. sha256:1000:WeuHltpve02t5bg1hNwxkiLJV7nWNGSK:/edJ0FrcbtjumDpi+4LrA3jbWX5fi9En
8. sha256:1000:rO/68tjYKN6culo7ZEnBkU/aItjIiRQ/:pRkiLv/iEVIARww5veu1JReHMxv+JGea
9. sha256:1000:I3GFNanvi5I6v4niYz3wTwvfAXcyXMfW:ZABb/t3DtrbNDNrbntXFJMfJWXLNuTmr
10. sha256:1000:w6Sgz/I5x3KDsaoB4VJpiu0UjL8M4JO/:UQ/tJMS8M6iwLzt1TkMy8xu5G/QTgZGV
11. sha256:1000:DbVrSMEfxTGUBSr88YA2fWK88YzQK3G9:ofadE4SD8XJ5T6h2Q0Y9TGBfhfgFKUDx
12. sha256:1000:4uWplmwCS877XN/GlxyX/6XPgkv34aXX:MBVAMXwM3ZMDcyufKKf2bx+IXh7J/hH0
13. sha256:1000:BO5JiAjHDpOyV1+tMUPn5RpUp+kcjQeP:WXmIglL8BqQU+OSe7kYw9UymFGiT9nGm
14. sha256:1000:R9NZYlhH1wEN8W/jINnPGt/x1dGJxb1a:/SHvWfRxKZnTxS17INkllew5/7OGKsXH
15. sha256:1000:p0BlIKU/T+gI5zf7olE14+a76ZfD9bV+:RA7nAzFcrIRexd2AB137LblJXTIzFsve
16. sha256:1000:lp/vQhgmAQSiUs52363uJPGpSkq19c+/:1Ao5ZeXKb0cMdpgE7iwV37RuppcWxJAG
17. sha256:1000:+rLC0YTXcB/UZ6kKmObvHcpOLMMXD+MO:HzpYb1pcnmDWdkruRazSIi9E5UCjyzOf
18. sha256:1000:LyXIOL3aqbuSqAbx59AD7jjjmuqJLyYT:yrTQ7Y85OD4O9Mkq4p1kr/K5fIn4zu/i
19. sha256:1000:71R3rhRhr/QIrATOS8M8nkGffkqbByaL:F0F3YxLlfkGCf7FuTw7uli/mvQnY/XJe
20. sha256:1000:Y98l+4uToOp6hwdMAWCWEHpcvHAAc9U2:v/80HmIIC6WRGm7OOxR+WL/HEOgEE2TP

Let say one and only one from the 'password' hashed derivatives above was stored in the database, and lets also assume that the user type the 'password' correctly..

We can check the match by writing a simple interface class., but should be more elaborate with strict form input validation as much as possible.

We create our simple interface class using line 17 derivative above as our stored hash..

<?php

## Warning! script below is for demonstration purposes ONLY, and should not be use to validate the actual password
## input from the user. All form inputs MUST undergo strict validation and sanitization.

## password_hash.php is the file downloaded from  https://defuse.ca/php-pbkdf2.htm
include_once('password_hash.php');


class validate_pass{

public function __construct(){}

## set public for testing
public function check_pass($db_pass,$hashed_pass){
 ## utilize the hasher function
    if(validate_password($db_pass,$hashed_pass))
            return true;
}
## can use this to validate form inputs, but not enough.. this is just an example..
public function sanitize($input){
return (filter_var($input, FILTER_SANITIZE_STRING));

}

public function generate_hash($pass_input){
    return (create_hash($pass_input));
}

}


$user_obj = new validate_pass();
## let us demonstrate the ability of the hasher to provide derivatives of a single string
echo 'Another Derivative : '. $user_obj->generate_hash('password').'<br/>';

## let us set the password from the derivatives.. this can be stored in the database, during the initial registration 
$hashed_pass = 'sha256:1000:+rLC0YTXcB/UZ6kKmObvHcpOLMMXD+MO:HzpYb1pcnmDWdkruRazSIi9E5UCjyzOf';

## password  below is from the user's input for validation purposes.
$password = 'password';

## let us validate password against the hashed pass
echo '<br/> Pass check: '.($user_obj->check_pass($password,$hashed_pass)? 'Password Match':'Invalid username or password');

Try changing the 'password' string to something else, or pick any of the 21 derivatives above and make it equal to the $hashed_pass..

Edited by veedeoo: info added

0

Ahh, I see the confusion. I was talking about matching user input to existing hashes. New hashes will of course be created based on a new salt, which will result in a different hash, even though the method is exactly the same.

If you examine one of those lines you generated, for instance:

sha256:1000:WeuHltpve02t5bg1hNwxkiLJV7nWNGSK:/edJ0FrcbtjumDpi+4LrA3jbWX5fi9En

It's split into four parts:

alagorithm:iterations:salt:hash

If you want to re-create the hash - to match it against user input, for example - then you need to extract the algorithm, iterations and salt from the existing hash and run those through the PBKDF2 algorithm with the new input. If the input is right, it will generate the existing hash again, exactly like it was. - That's what your validate_password function does.

By the way, your validate_pass class seems a little redundant there. It's really just a shallow wrapper around a few existing functions. It even hides the actual false value from validate_password, giving you null instead.

This question has already been answered. Start a new discussion instead.
Have something to contribute to this discussion? Please be thoughtful, detailed and courteous, and be sure to adhere to our posting rules.