Login form problems - making me go bald!

I’m scratching my head over this one, the typical way I write this is not working and I am at a loss as to why. I’m building a user login form, and building the query based off of the input values. When I mess with this line I lose all variables.

These two formats I end up with NOTHING displayed as far as my 3 variables are concerned:

$SQLGetUser = "SELECT user_id, username FROM user WHERE username='" . $loginuser . "' AND password='" . $loginpassword . "'";
$SQLGetUser = "SELECT user_id, username FROM user WHERE username='$loginuser' AND password='$loginpassword'";

But changing it to THIS

$SQLGetUser = "SELECT user_id, username FROM user WHERE username=$loginuser AND password=$loginpassword";

All the variables get set as I expect, but my SQL is not good because I lose my quote marks around my 2 values for the MySQL database so the query fails.

Even if I put in a line with the exact SQL Query it ends up making all the variables report empty. Maybe it is in my redirecting after the form reloads?

Here is the entire snippet of login code:


if(isset($_POST['submit']))
  {
    $loginuser = $_POST['loginuser'];
    $loginpassword = md5($_POST['loginpassword']);
    if((!empty($loginuser)) or (!empty($loginpassword)))
    {
      $SQLGetUser = "SELECT user_id, username FROM user WHERE username='" . $loginuser . "' AND password='" . $loginpassword . "'";
      $result = mysqli_query($link, $SQLGetUser);
      if($result)
      {
        session_start();
        while($row = mysqli_fetch_assoc($result))
        {
          $_SESSION['user_id'] = $row['user_id'];
          $user_id = $row['user_id'];
        }
        header('Location: index.php?user_id=' . $user_id);
        exit();
      } else {
          $errornotice = "Unable to log you in.";
      }
    }
  }

Any guidance will be GREATLU appreciated as I have been staring at this code for over a day now and, while I KNOW it is some stupid thing I am not thinking through correctly, I can’t seem to get my head wrapped into it so I can see where I’ve gone wrong. The strange thing is I have this almost exact same code on another page, and it pulls the values back from the database just as I expect.

Thanks in advance for any suggestions!

Greg

Just saw one problem and change my if statement near beginning to && since if BOTH are not empty I want the code to execute. Now the password does get passed through, but username still doesn’t and SQL statement is also empty it appears.

Greg

OK, got that part working, but still not logging me in, though it is reporting back the proper user info, so I know the database is working and my session is being set, so not sure why this code:


  if(!isset($_SESSION['user_id']))
  {
    header('Location: login.php');
    exit();
  }

redirects me. That session value IS set, so it should skip right over this. Unless I am not thinking clearly.

And just for the record, here is how my previous code ended up and is now functional:


if(isset($_POST['submit']))
  {
    $loginuser = $_POST['loginuser'];
    $loginpassword = md5($_POST['loginpassword']);
    $SQLGetUser = "SELECT user_id, username FROM user WHERE username='" . $loginuser . "' AND password='" . $loginpassword . "'";
    $result = mysqli_query($link, $SQLGetUser);
    if(!mysqli_num_rows($result))
    {
      $errornotice = "Unable to log you in.";
    } else {
      while($row = mysqli_fetch_assoc($result))
      {
        $_SESSION['user_id'] = $row['user_id'];
        $user_id = $row['user_id'];
      }
      header('Location: index.php');
      exit();
    }
  }

And just for the record, I KNOW I am being redundant in my $user_id variable, I did that just for testing and haven’t cleaned it back out of here yet.

Howdy Greg,

I have a few possibilities for you to investigate with your redirect issue. The first and most obvious question is are you sure you called session_start() prior to your check here:


if(!isset($_SESSION['user_id']))
{
    header('Location: login.php');
    exit();
}

It’s an easy thing to miss sometimes. Another thought that comes to mind is that its possible that the shutdown session handler isn’t doing it’s job prior to your header redirect. You could test that theory out by modifying the code right before your header redirect:


if(isset($_POST['submit']))
  {
    $loginuser = $_POST['loginuser'];
    $loginpassword = md5($_POST['loginpassword']);
    $SQLGetUser = "SELECT user_id, username FROM user WHERE username='" . $loginuser . "' AND password='" . $loginpassword . "'";
    $result = mysqli_query($link, $SQLGetUser);
    if(!mysqli_num_rows($result))
    {
      $errornotice = "Unable to log you in.";
    } else {
      while($row = mysqli_fetch_assoc($result))
      {
        $_SESSION['user_id'] = $row['user_id'];
        $user_id = $row['user_id'];
      }
      session_write_close();  /* Force the session handler to wrap up */
      header('Location: index.php');
      exit();
    }
  }

Adding the session_write_close will force the handler to wrap things up prior to your redirect. Somewhat unrelated to your problem, but you should also have a go at securing your queries. As it is you’re wide open to SQL injection.

Hope that helps you. Have a good night!

Thank you for the suggestions (and I was cleaning up code to prevent SQL injection but stripped it out in case I may have written it incorrectly and it was causing the problem). Below is the complete code for both pages, the login.php where the user inputs their credentials and the index.php where they get directed back to. I want index.php to push them to login if they aren’t properly logged in and that is where this problem is arising. As you will see in the index.php page, I echo’d out the $_SESSION[‘user_id’] value that I test for at the top. If I comment out my if test in the first lines, then, of course, the page loads fine, AND it shows both a session_id value, as well as the proper user_id value as returned from the database. Whichever user I log in as, the user_id is correct. So everything seems to be working fine, except for the test to see if the $_SESSION[‘user_id’] value is set. Very puzzling (to me at least).

Unfortunately neither of your suggestions worked, I did add some code to check if the session is started, and if not, start it just to make sure. But even before I did that the code would echo out the session variable I was setting properly. And the code for session_write_close(); didn’t seem to make any difference either.

Any critique on the rest of the code is always welcome as well as I won’t learn what I am doing wrong unless someone points it out. With that said, some things I am aware are not at all complete (such as correcting for SQL injection attacks) but this is still development code and I typically don’t worry about some of that stuff until later. However, if some think that is a mistake and I should be implementing it from the start I’d be happy to hear that as well. I think I’ve cleared out anything in this code that would give away any pertinent info, but if someone sees something I overlooked let me know so I can correct it.

As it is written here, the code all works perfectly, except for the fact that index.php redirects ALL the time, unless I comment out the IF test in the first few lines.

Greg

CODE FOR login.php

<?php
  include_once('../includes/helpers.inc.php'); // Custom functions (such as to make cleaning up input simpler)
  include_once('../includes/dbconnect.inc.php'); // Pretty self explanatory (-:
  if(isset($_POST['submit']))
  {
    $loginuser = $_POST['loginuser'];
    $loginpassword = md5($_POST['loginpassword']);
    $SQLGetUser = "SELECT user_id, username FROM user WHERE username='" . $loginuser . "' AND password='" . $loginpassword . "'";
    $result = mysqli_query($link, $SQLGetUser);
    if(!mysqli_num_rows($result))
    {
      $errornotice = "Unable to log you in.";
    } else {
      while($row = mysqli_fetch_assoc($result))
      {
        if (!session_id()){
          session_start();
        }
        $_SESSION['user_id'] = $row['user_id'];
        $user_id = $row['user_id'];
      }
      session_write_close();
      header('Location: index.php');
      exit();
    }
  }
  include_once('../includes/header.inc.php');
  include_once('../includes/navigation.inc.php');
?>
  <div class="mainbody">
    <div class="columncontainer2">
      <div class="columncontainer1">
        <div class="leftcolumn">
          <div class="topicheader">
            Administration Access
          </div> <!-- topicheader -->
          <h2 class="headercontent">Login</h2>
          <div class="errorpost"><?php echo $errornotice; ?><br /><?php echo $dberrornotice; ?></div>
          <div class="bodycontent">
            <div id="loginform">
            <form action="login.php" method="post">
              <p>
                <label for="username">Username: </label><input id="username" type="text" name="loginuser" />
                <label for="password">Password: </label><input id="password" type="password" name="loginpassword" />
              </p>
              <p>Username Entered: <?php echo $loginuser; ?></p> // These lines I put in for testing, and all show the values I expect
              <p>Password Entered: <?php echo $loginpassword; ?></p> // The one difference is when a proper set of credentials are entered
              <p>SQL: <?php echo $SQLGetUser; ?></p> // And IF query in index.php is active, then it returns to this page with all variables
              <p>SESSION USER ID: <?php echo $_SESSION['user_id']; ?></p> // wiped out (except the session one of course)
              <p>User_ID: <?php echo $user_id; ?></p>
            <input type="submit" name="submit" value="Log-In" />
            </form>
          </div>
        </div>
      </div>
      <?php
        include('../includes/rightcolumn.inc.php');
      ?>
    </div> <!-- columncontainer1 -->
  </div> <!-- columncontainer2 -->
</div><!-- mainbody -->
<?php
include('../includes/footer.inc.php');
?>

CODE FOR index.php


<?php
  // If I comment out this IF then the page loads as expected
  if(!isset($_SESSION['user_id']))
  {
    header('Location: login.php');
    exit();
  }
  include_once('../includes/helpers.inc.php');
  include_once('../includes/dbconnect.inc.php');
  include_once('../includes/header.inc.php');
  include_once('../includes/navigation.inc.php');
  // Post Record Count
  $post_count = $link->query("SELECT * FROM posts");
  $comment_count = $link->query("SELECT * FROM comments");
?>
<div class="mainbody">
  <div class="notation-spacer-bar"></div>
  <div class="columncontainer2">
    <div class="columncontainer1">
      <div class="leftcolumn">
        <div class="topicheader">
          Blog Admin
        </div> <!-- topicheader -->
        <h2 class="headercontent">Login</h2>
        <div class="bodycontent">
          <div id="admin-menu">
            <ul>
              <li><a href="#">Admin Home</a></li>
              <li><a href="#">New Post</a></li>
              <li><a href="#">Delete Post</a></li>
              <li><a href="logout.php">Log Out</a></li>
              <li><a href="#">Blog Home</a></li>
            </ul>
          </div>
          <div id="mainContent">
            // THESE ECHO OUT EXACTLY THE VALUES THEY SHOULD
            <p>SESSION ID: <?php echo $_SESSION['user_id']; ?></p>
            <p>Session_ID: <?php echo session_id(); ?></p>

            <table>
              <tr>
                <td>Total Blog Posts</td>
                <td><?php echo $post_count->num_rows; ?></td>
              </tr>
              <tr>
                <td>Total Comments</td>
                <td><?php echo $comment_count->num_rows; ?></td>
              </tr>
            </table>
          </div>
        </div>
      </div>
      <?php
      include('../../includes/inc_rightcolumn.inc');
      ?>
    </div> <!-- columncontainer1 -->
  </div> <!-- columncontainer2 -->
</div><!-- mainbody -->
<?php
include('../includes/footer.inc.php');
?>

Well, maybe this means something (I’m still running it through my head, but thought I’d post here too).

I properly log in, with the IF echo’d out, so the index page shows. I have it echoing out both the user_id session variable I set, as well as the actual session_id as shown in my code above. But when I click on the Logout link (which is simply this code at the moment):


<?php
  session_destroy();
  header('Location: index.php');
  exit();
?>

I receive these errors:
Warning: session_destroy() [function.session-destroy]: Trying to destroy uninitialized session in logout.php on line 2
Warning: Cannot modify header information - headers already sent by (output started at logout.php:2) in logout.php on line 3

Maybe that tells someone something, and I will go and Google it myself, but wanted to post this real quick so no one spends a lot of time trying to figure out where I went wrong if this ends up making it obvious (which I have a feeling it will once I do some research). I have to head out, but will post something as soon as I know and can.

Thanks for all the advice, don’t know what I would do without the Sitepoint Forums! (and books, and classes, and…)

Greg

I haven’t looked at the rest of the thread but that would probably be failing since no session has been initialized using session_start.


<?php 
  session_start();
  session_destroy(); 
  header('Location: index.php'); 
  exit(); 
?>

Than again lately I have been working with libraries that make that all the session handling stuff a blockbox so my memory might have alluded me here.

Looking at that again the approach here does not look correct. I would normally set a flag and merely unset it rather than destroying the session entirely. So that presence of that flag is what would dictate whether a user has logged in or not which can be used to than access the meta data associated with a user if they are logged in.

There is a session_start, it is inside the header.inc.php file which is called at the top of the file. (not the index.php but the login so that should initialize the session (or am I thinking incorrectly here?).

What’s more puzzling to me, is the IF test’s if there is a value in $_SESSION[‘user_id’], if it is not set (i.e. no value assigned to it) then redirect to login page because user is not logged in. However it ALWAYS redirects, and if I comment out that test, the index page does load. I put an echo statement to see if anything is held in $_SESSION[‘user_id’] and it has a value. Shouldn’t that mean that it is set?

Greg

Move the session_start() to the start of the index.php file. As the code stands you’re checking to see if their is an active session which of course there won’t be as you’ve not started any sessions so you’re re-directing the user back to the login screen.

OMFG!!! That did it! I knew it was going to be something stupid simple and just overlooked. But how did I not stub my toe on that one!?!?! Thank you. It was really starting to drive me nuts, I had thought since the session_start was called when login.php was launched that would suffice. If index.php is called first, no session_start so it loops to login.php. Login.php starts a session so it would be fine. Guess you need a session_start at the beginning of the file that utilizes session variables every time?

Guess I need to go do more reading about sessions. Should you have a session_start at the beginning of every script (just for safety’s sake?) or is that a bit of overkill?

Greg

Just speaking for myself. I call a file (initialize.php) at the beginning of every page. The first line in that file is session_start(); followed by some other things I want to have available, such as my debug settings (I comment this out after debugging everything) and connecting to the database.

That’s a great idea CSU-Bill! I already have some files with those group settings, but don’t always call it at the start of every file. I especially like placing the debug settings there rather than on and off in different files. Think I’m going to “borrow” this and implement it in this project.

Off we go, actually think I’ll get a full day of coding today. Doesn’t happen often, so glad I got such great answers so I can implement them while coding.

Greg