2

I'm creating a very basic user login interface for a website I'm working on for a "members" portion of the website. I am using for $_SESSION variables to store information. ['access'] stores a boolean value as to whether or not the user is permitted access, ['invalid'] is true if the username/password don't match (for error handling), ['no_attempt'] is true if the user tries to access a restricted page without being logged in, and ['uri'] stores the URI of the page they tried to access without being logged in.

With how I have coded my pages so far, everything works, except the error message is never displayed when ['no_attempt'] is true. The following are excerpts from my pages:

login.php

session_start();
if(md5($_POST['username']) === '[username hash]' && md5($_POST['password']) === '[password hash]') {
    // allow access to members pages    
    $_SESSION['login']['allow'] = true;

    // clear error messages
    $_SESSION['login']['invalid'] = false;
    $_SESSION['login']['no_attempt'] = false;

    // if a url hasn't been stored for redirect, direct to the members home page
    if (!isset($_SESSION['login']['redirect']) || empty($_SESSION['login']['redirect'])) $_SESSION['login']['redirect'] = '/members';
    header("Location: {$_SESSION['login']['redirect']}");
}
else {
    $_SESSION['login']['invalid'] = true;
    header('Location: /');
}

The beginning of each restricted page begins with this code:

session_start();
if($_SESSION['login']['allow'] != true) {
    $_SESSION['login']['redirect'] = $_SERVER['REQUEST_URI'];
    $_SESSION['login']['no_attempt'] = true;
    header('Location: /');
}
require ('../assets/includes/site-header.php');

site-header.php
This page is called at the beginning of each page of my website. It has the title, nav bar, and stuff like that. Below is only the Member login part of the sidebar.

if(isset($_SESSION['login']['allow']) && $_SESSION['login']['allow'] == true) {
    echo "<li><a href=\"/members\" id=\"members\">Members</a></li>\n";
}
else {
    echo "<li>\n";
    echo "<div class=\"expandable\">Members <span class=\"show-hide\">+</span></div>\n";
    echo "<div class=\"nav-details\">\n";
    if (isset($_SESSION['login']['invalid']) && $_SESSION['login']['invalid'] == true) {
        echo "<div id=\"login-error\">The username or password you<br />entered was incorrect.</div>\n";                                    
    }
    elseif (isset($_SESSION['login']['no_attempt']) && $_SESSION['login']['no_attempt'] == true) {
        echo "<div id=\"login-error\">You must login before<br />accessing that page.</div>\n";
    }
    echo "<form method=\"post\" action=\"/members/login.php\" id=\"login-form\">\n";
    echo "Username:<br />\n";
    echo "<input type=\"text\" name=\"username\" /><br /><br />\n";
    echo "Password:<br />\n";
    echo "<input type=\"password\" name=\"password\" /><br /><br />\n";
    echo "<input type=\"submit\" value=\"Login\" id=\"login-button\" />\n";
    echo "</form>\n";
    echo "</div>\n";
    echo "</li>\n";
}
$_SESSION['login']['invalid'] = false;
$_SESSION['login']['no_attempt'] = false;
hakre
  • 193,403
  • 52
  • 435
  • 836
  • 1
    MD5 is a very insecure hashing algorithm, don't use it. – Robert Allan Hennigan Leahy Dec 21 '12 at 02:03
  • 2
    You don't need to include more code, at least not right now. But you need to change some code, please see: http://php.net/faq.passwords - For your session problem, you need to debug it. Verify your expectations by putting the data to a proofread: `var_dump($_SESSION);` is your friend (view source in your browser). So this requires basic troubleshooting by you first. – hakre Dec 21 '12 at 02:04
  • What would you recommend I use if not md5 (I'm not too worries about security for this site in the first place, at least in the regard.) @hakre Also, I should have mentioned that when removed the last line: $_SESSION['login']['no_attempt'] = false; on index.php, the error message appears, albeit every time I reload the page rather than only once. – Lucas Schneider Dec 21 '12 at 02:12
  • http://stackoverflow.com/questions/2235897/php-how-to-use-crypt-with-crypt-blowfish - for the session, it depends, you need to get your own feeling for it, it's different to without sessions because with session you start to share partial state between the requests. do the troubleshooting and debugging and the rest comes quasi automatically. – hakre Dec 21 '12 at 02:25
  • Do you have any idea what might be causing the problem, though? – Lucas Schneider Dec 21 '12 at 02:38
  • Why have you added these two lines at the end $_SESSION['login']['invalid'] = false; $_SESSION['login']['no_attempt'] = false; I guess this reset the session in all condition their by allowing restricted access. – Girish Sahu Jan 06 '13 at 21:58
  • That was to make sure that the error messages would only appear once. What I don't understand is why the "invalid" error message works perfectly fine but the "no_attempt" error does not. – Lucas Schneider Feb 02 '13 at 18:02

2 Answers2

0

I was finally able to fix my problem! The error was with the snippet of code I was including at the top of every restricted page. I changed:

session_start();
if($_SESSION['login']['allow'] != true) {
    $_SESSION['login']['redirect'] = $_SERVER['REQUEST_URI'];
    $_SESSION['login']['no_attempt'] = true;
    header('Location: /');
}
require ('../assets/includes/site-header.php');

to this:

session_start();
if($_SESSION['login']['allow'] != true) {
    $_SESSION['login']['redirect'] = $_SERVER['REQUEST_URI'];
    $_SESSION['login']['no_attempt'] = true;
    header('Location: /');
}
else {
    require ('../assets/includes/site-header.php');
}

I'm guessing that this has something to do with me setting $_SESSION['login']['invalid'] = false; $_SESSION['login']['no_attempt'] = false; on site-header.php. I'm still a beginner with php, but I'm thinking that maybe the php code on site-header.php was processed due to being required before the page redirected the page by header('Location: /'); If anyone knows if this is why my problem was fixed, or if it was for something else, I'd be happy to hear it!

-1

Separate out this section of your code...

   elseif (isset($_SESSION['login']['no_attempt']) && $_SESSION['login']['no_attempt'] == true) {
    echo "<div id=\"login-error\">You must login before<br />accessing that page.</div>\n";
}

to the following...

    elseif(isset($_SESSION['login']['no_attempt'])
    {
        if($_SESSION['login']['no_attempt'] === true)
          {
                echo "<div id=\"login-error\">You must login before<br />accessing that page.</div>\n";
          }
    }

see if that works. Note I used the '===' operator... this assures that it is 'identical'

Highspeed
  • 442
  • 3
  • 18