Solved

Is my php for loop correctly accessing user login inputs?

Posted on 2012-04-05
12
189 Views
Last Modified: 2012-04-13
My webpage here has a form, whose action is a file called process_login.php (code below). I want to make sure that the for-loop below is comparing the user inputs on the login form to the username/password in the users492831.txt file, and then create a session if they match. Once a session is created, the header() function is used to direct the user to the pictures.php site, but that doesn't seem to be happening when I try to log in. Does anyone know what I'm doing wrong?

<?php 
	$fh = fopen("users492831.txt", 'r') or die("can't open file");
	$wordArray = explode(';', implode(';', array_map('trim', file('users492831.txt')))); //trim removes /n at end of each line
	
	$username = $_POST["username"];
	$password = $_POST["password"];
	$exists = false;
	
	for($i=0;$i<count($wordArray);$i++)
		{
		if($wordArray[$i] == trim($username) && $wordArray[$i+1] == trim($password))
			{$exists = true;}
		}
	fclose($fh);
	
	if($exists == true){ //save cookie if login works
		session_start();
		$myUsername = $_SESSION["username"];
		$myPassword = md5($_SESSION["password"]); //md5 is an encrypting function
		$loggedin = true;
        header("Location: pictures.php");
	}	
	else{
        header("Location: login.php");		
	}
?>

Open in new window

0
Comment
Question by:shampouya
  • 6
  • 5
12 Comments
 
LVL 108

Expert Comment

by:Ray Paseur
Comment Utility
At line 18, the $_SESSION array may or may not contain any data.  This would depend on whether the session had been set on a previous request.
$myUsername = $_SESSION["username"]; // WHAT IS IN THE SESSION?  WHO KNOWS?
0
 
LVL 108

Expert Comment

by:Ray Paseur
Comment Utility
Also, it seems that the script sets the value of $myUsername to be equal to $_SESSION["username"] and then does not use the variable $myUsername (or $myPassword or $loggedin).   There is no extra credit for setting a value in an unused variable ;-)
0
 

Author Comment

by:shampouya
Comment Utility
Ok I just deleted lines 18 and 19, you're right they we're unused. Here is my new code for the process_login.php. Does anything look wrong with the for loop and the subsequent if statement?

<? ob_start(); ?>

<?php 
	$fh = fopen("users492831.txt", 'r') or die("can't open file");
	$wordArray = explode(';', implode(';', array_map('trim', file('users492831.txt')))); //trim removes /n at end of each line
	
	$username = $_POST["username"];
	$password = $_POST["password"];
	$exists = false;
	$loggedin = false;
	
	for($i=0;$i<count($wordArray);$i++)
		{
		if($wordArray[$i] == trim($username) && $wordArray[$i+1] == trim($password))
			{$exists = true;}
		}
	fclose($fh);
	
	if($exists == true){ //save cookie if login works
		session_start();
		$_SESSION["loggedin"] = true;
		$loggedin = true;
        header("Location: pictures.php");
	}	
	else{
        header("Location: login.php");		
	}
?>
<? ob_flush(); ?>

Open in new window

0
 
LVL 108

Expert Comment

by:Ray Paseur
Comment Utility
Well, you can delete line 22 in this script.  It's another unused variable.  Not sure at all where output buffering comes into play.  And I cannot readily understand the compound statement on line 5.  As a general rule, experienced programmers stay away from things like that because they are hard to understand, debug or modify.

Let's try this... Please post a few lines from users492831.txt so I can see what the data looks like.  Then I will show you what I might do with the script.  (In truth, I would use a data base table for PHP client authentication, following the guidelines in this article).
http://www.experts-exchange.com/Web_Development/Web_Languages-Standards/PHP/A_2391.html
0
 

Author Comment

by:shampouya
Comment Utility
Sure, here is the text file: users492831.txt. The first word is the username and the second word is the password. And I'm trying to make a basic login page without using mysql because we haven't gone over mysql yet in class.
0
 
LVL 108

Expert Comment

by:Ray Paseur
Comment Utility
OK, it looks to me like you will have one user name and one password on each line.  Both fields are terminated by a semi-colon.  Is that right?
0
IT, Stop Being Called Into Every Meeting

Highfive is so simple that setting up every meeting room takes just minutes and every employee will be able to start or join a call from any room with ease. Never be called into a meeting just to get it started again. This is how video conferencing should work!

 

Author Comment

by:shampouya
Comment Utility
exactly
0
 
LVL 108

Accepted Solution

by:
Ray Paseur earned 400 total points
Comment Utility
Warning: This is untested code and only intended to be a teaching example.  You might use file() instead of file_get_contents() and explode().  I do it that way because of old habits.

We examine each line of the users file one at a time.  The logic asks if the posted username matches.  If that test is satisfied, we ask if the password matches.  It both tests are satisfied, the script redirects the client browser with the header() command.  If we exhaust the list of users and have not found the double match on user name and password, we fall out the end of the foreach() loop and redirect to login.  To show we are logged in, we put the user name in the $_SESSION array; this conveys a little more meaning than just TRUE.

Make sense?  Questions?  All the best, ~Ray
<?php // RAY_temp_shampouya.php
error_reporting(E_ALL);

// ALWAYS START THE SESSION UNCONDITIONALLY ON EVERY PAGE
session_start();

// THIS SIMULATES READING THE FILE INTO A STRING VARIABLE
// $url = 'users492831.txt';
// $str = file_get_contents($url);
$str = 'prebek;wow;';

// BREAK THE STRING INTO AN ARRAY
$arr = explode(PHP_EOL, $str);

// PROCESS EACH OF THE ARRAY ELEMENTS INTO A USER NAME AND PASSWORD
foreach ($arr as $udata)
{
    // PUTS USER IN ARRAY[0] AND PWD IN ARRAY[1]
    $x = explode(';', $udata);

    // IF USER NAME MATCHES
    if ($x[0] == $_POST['username'])
    {
        // IF PASSWORD ALSO MATCHES
        if ($x[1] == $_POST['password'])
        {
            // SET THE USER NAME IN THE SESSION
            $_SESSION["loggedin"] = $x[0];

            // REDIRECT
            header("Location: pictures.php");
            exit;
        }
    }
}
// IF WE GET HERE, THERE WAS NO MATCH FOR THE LOGIN
header("Location: login.php");
exit();

Open in new window

0
 
LVL 29

Assisted Solution

by:Olaf Doschke
Olaf Doschke earned 100 total points
Comment Utility
If I were you I'd rather not store the clear text password in the users492831.txt, but md5 of it.

General login logic: User enters username and password, you compute the md5 of the entered password and compare it to the stored md5 value in the users line to check for the correct login.

Even if users492831.txt is a file only readable for your php script, there's always the chance the whole web server get's compromised or by some attack a memory dump shows the array during login, whatever.

Users possibly use their passwords on many sites (bad practice of them of course) and that might include banking or anything else valuable. So treat your users passwords like money, protect them. It doesn't matter if they ought to pay more attention to their own safety, make your contribution to their safety anyway.

Otherwise there is not much of a difference in doing this with a mysql database, you'd apply the same logic. Additional safety would be to even only transfer the password encrypted by https instead of http (meaning via an ssl encrypted connection).

In regard of the rest of your code, you'll figure it out with the help of others, I'm quite confident, so I won't repeat any advice youa lready got in that respect.

Bye, Olaf.
0
 

Author Comment

by:shampouya
Comment Utility
Thanks for the tips Olaf, I will implement them. Ray, I simply added session_start(); to the start of each php file and it did the trick. Now I am able to log in with "prebek" and "wow". What exactly does session_start(); do, does it simply look for cookies and gain access to my username and password variables? I'm wondering why that made my code work.
0
 

Author Closing Comment

by:shampouya
Comment Utility
thanks
0
 
LVL 108

Expert Comment

by:Ray Paseur
Comment Utility
@shampouya: You're in luck!  All of the PHP functions are documented in the online man pages.  When you have a question like What exactly does session_start(); do? you can get an answer at the click of a mouse. (Note the pattern of this URL.  You can put the function name into the URL or just use the search box at the top right of the pages on http://php.net

See http://php.net/manual/en/function.session-start.php for the explanation you need.
0

Featured Post

6 Surprising Benefits of Threat Intelligence

All sorts of threat intelligence is available on the web. Intelligence you can learn from, and use to anticipate and prepare for future attacks.

Join & Write a Comment

Suggested Solutions

This article discusses how to create an extensible mechanism for linked drop downs.
Since pre-biblical times, humans have sought ways to keep secrets, and share the secrets selectively.  This article explores the ways PHP can be used to hide and encrypt information.
The viewer will learn the benefit of using external CSS files and the relationship between class and ID selectors. Create your external css file by saving it as style.css then set up your style tags: (CODE) Reference the nav tag and set your prop…
The viewer will learn how to create a basic form using some HTML5 and PHP for later processing. Set up your basic HTML file. Open your form tag and set the method and action attributes.: (CODE) Set up your first few inputs one for the name and …

763 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

12 Experts available now in Live!

Get 1:1 Help Now