auth0-blog / starwars-phpapp

6 stars 8 forks source link

Logout check doesn't exit #1

Open LukeKirby opened 7 years ago

LukeKirby commented 7 years ago

Hi, just looking at the example index page and after setting the redirect header, the script needs to exit straight away otherwise the rest of the page is passed to the client anyway. Its good to encourage readers best practices when handling sessions

https://github.com/auth0-blog/starwars-phpapp/blob/master/index.php#L28-L29

Needs to be session_destroy(); header("Location: /"); exit;

unicodeveloper commented 7 years ago

Hello @LukeKirby What do you mean by the rest of the page is passed to the client?

LukeKirby commented 7 years ago

The rest of the page is still processed by php (because ithasn't been told to stop yet) and sends the html to the client browser. adding the exit() call after the redirect header stops this.

In a different scenario if you had

// pseudo
If User Is Not Member {
    session_destroy();
    header("Location: /");
}

<h1>Discount Code for £200 off your next order: CAE13</h1>

The browser still sees the h1 tag because php wasn't told to exit and thus sends the rest of the html

unicodeveloper commented 7 years ago

It's okay for the rest of the page to be processed by PHP because we have PHP code in the body of the index page.

LukeKirby commented 7 years ago

I should clarify, the browser gets the whole html response, but because of the location header it will redirect straight away so the end user doesn't see the page but the browser does.

There's nothing of importance on the index page as it happens but there should be an exit() call straight after the redirect header because its good practice and also people new to php might copy and paste that code from your index page onto a secure section on their website project and find they've added in a security hole.