Help me with Login System

[Apostle]

Hi all, after thinking for sometimes, I thought it will be great opportunity to learn if I will start from scratch and build my own register/login system. Here is the thread that I will be posting the progress and I hope you guys will help me.

The code below is what I have so far. Just put two scripts in the same directory and that is! I hope you will help me
Thanks!
class.php
[CODE=php]
<?php
//php login sytem
class LoginRegister{
function __construct(){
}

function displogin($stat us){
if ($status == "login"){
// post login page
$enc = base64_encode(' login');
$html = <<<LOGIN
<form action = $_SERVER[PHP_SELF]?do=$enc, method = POST>
<p>Username: <input type=text name = username /></p>
<p>Password: <input type=password name = password /></p>
<input type=submit value=Login />
</form>
LOGIN;
echo $html;
}//end if

else if ($status == "register") {
//post register page
$enc = base64_encode(' register');
$html = <<<LOGIN
<form action = $_SERVER[PHP_SELF]?do=$enc, method = POST>
<p>Username: <input type=text name = username /></p>
<p>Password: <input type=password name = password /></p>
<input type=submit value=Register />
</form>
LOGIN;
echo $html;
}// end elese if


}

function auth($username, $password){
$sql = "SELECT * FROM users WHERE username = '$username' AND password = '$password' ";
$res = mysql_query($sq l) or die(mysql_error ());
if (mysql_num_rows ($res)==1){
echo "sucessful logged in as ". $username;
}//end if
else{
echo "<p style = 'color:red; font-weight:bold;'>U sername or password not correct.
<br /> New? Register!</p>";
$this->displogin('reg ister');
}// end else
}


function checkempty($use rname, $password, $mode){
if (empty($usernam e) or empty($password )){
echo "<p style = 'color:red; font-weight:bold;'>E mpty Values are not allowed</p>";
$this->displogin('log in');
}//end if
else{
//do checking
switch($mode){
case 'login':
$this->auth($username , $password);
case 'register':
$this->adduser($usern ame, $password);
default:
echo "<p style = 'color:red; font-weight:bold;'>W rong Values are not allowed</p>";
$this->displogin('log in');
}//end switch
}//end else
}

function login($uname, $passwd){
//username
$username = stripslashes($u name);
$username = mysql_real_esca pe_string($unam e);
//passsword
$password = stripslashes($p asswd);
$password = mysql_real_esca pe_string($pass wd);
//check for empty variables
$this->checkempty($us ername, $password, 'login');
}

function register($uname , $passwd){
//username
$username = stripslashes($u name);
$username = mysql_real_esca pe_string($unam e);
//passsword
$password = stripslashes($p asswd);
$password = mysql_real_esca pe_string($pass wd);
//check for empty variables
$this->checkempty($us ername, $password, 'register');
}

function adduser($userna me, $password){
$sql = "INSERT INTO users(username, password) VALUES('$userna me', '$password')";
//redirect to login page
echo "<p style = 'color:green; font-weight:bold;'>T hanks for registering. You can now login</p>";
$this->displogin('log in');
mysql_query($sq l) or die(mysql_error ());
}

}//end class
?>
[/CODE]

index.php
[CODE=php]
<?php
require "class.php" ;
$obj = new LoginRegister() ;
$conn = mysql_connect(" localhost", "root", "") or die(mysql_error ());
mysql_select_db ("admin", $conn)or die(mysql_error ());
if ((isset($_GET['do']))){
if (($_GET['do'])==(base64_enco de('login'))){
$obj->login($_POST['username'], $_POST['password']);
}//end middle first if
else if(($_GET['do'])== (base64_encode( 'register'))){
$obj->register($_POS T['username'], $_POST['password']);
}
else{
echo "<p style = 'color:red; font-weight:bold;'>P lease Login</p>";
$obj->displogin('log in');
//debug
echo base64_encode(' login').'<br />';
echo $_GET['do'];
}//end else middle

}//end last if
else{
echo "<p style = 'color:green; font-weight:bold;'>P lease Login</p>";
$obj->displogin('log in');
}//end else
?>
[/CODE]

[TheServant]

Sure. Let us know if you have a question. This section of Bytes is really for people who need some help with something specific. As much as we'd like to read through your code and impart some wisdom and knowledge in layout, syntax and method, proof reading code is not really in the job description. If you get an error, or something is not working as it should, post relavent code and all error messages and a full explanation, so we don't have to spend half our day looking through irrelevant code trying to find an unidentified problem.

If you're wanting to write a tutorial, write it in PHP insights.

[Apostle]

this is newbie start writting the script. So IWhat I wanted is criticism and suggestion. I want to end up with full secure login system. That is my intention and I believe it is in Job descriptin ;)

Sorry for being vague and welcome for help :)

[Dormilich]

knowledge has its price… either money (if you hire someone) or effort (to learn it yourself).

[TheServant]

knowledge has its price… either money (if you hire someone) or effort (to learn it yourself).

True. Apostle, you need to try and improve you script and come to us when you're stuck on something. Type in PHP login script, or login tutorial in Google and you'll have plenty of places to get the basics. Always start with the basics.

[dlite922]

this is newbie start writting the script. So IWhat I wanted is criticism and suggestion. I want to end up with full secure login system. That is my intention and I believe it is in Job descriptin ;)

Sorry for being vague and welcome for help :)

You need some major help!

What you had is not even a class. Here's what real class looks like:

<?php
/**
*  This class handles interactions for user access and registration
* 
* @date 11/06/2009
* @author  Apostle 
* @file LoginRegister.class.php	
*/

class LoginRegister
{

	/**
	* The DB object used to access the database
	*/
	private $DB; 


	/**
	* Constuctor
	* 
	*/
	function __construct()
	{
		$this->DB = new DB(); 
	}
	
	/**
	* Authenticates a username and password and returns true or false depending on validity
	* 
	* @access public
	* @param mixed $username
	* @param mixed $password
	* @return bool
	*/
	public function authenticateUser($username, $password)
	{
		// initialize and clean variables
		$cleanUser = mysql_real_escape_string($username); 
		$cleanPass = mysql_real_escape_string($password); 
		
		// Run query and get results
	    $sql = "SELECT COUNT(*) AS count FROM users WHERE username = '$cleanUser' AND password = '$cleanPass' ";	    
	    $result = $this->DB->query($sql); 
	    
	    // Parse result
	    if(!empty($result)) // if not empty
	    {
		    if($result[0]['count'] == 1) { // make sure count is one and only one user with the same username and passwword.
	    		return true; 
		    }
	    }
	    	    
	    return false; 	    	    
	} 
	
	
	/**
	* Registers a new user name and password and returns true of successful and false if not. 
	* 
	* @access public
	* @param mixed $username
	* @param mixed $password
	* @return bool
	*/
	public function registerUser($username, $password)
	{
		// initialize and clean variables
		$cleanUser = mysql_real_escape_string($username); 
		$cleanPass = mysql_real_escape_string($password); 
		
		// first check if this user already exists
		if($this->checkUserExist($cleanUser))
		{
			die("Error: A user by this name already exists. You should have already run this check before and told the user before calling registerUser()"); 			
			exit(0); // make sure you exit!
		}
		else
		{
			// user doesn't exist, add him:
			$sql = "INSERT INTO users(username, password) VALUES('$cleanUser', '$cleanPass')";
			$result = $this->DB->query($sql); 
			if(empty($result)) 
			{
					die("Something went wrong. Was not able to add user"); 
			}
			
			return true;
		}
	
		return false;
	}

	
	/**
	* Checks if a user already exists, returns true if user already exists and false if no user exists with given username.
	* 
	* @access public
	* @param mixed $username
	* @return bool
	*/
	public function checkUserExist($username)
	{
		// initialize and clean variables
		$cleanUser = mysql_real_escape_string($username); 
	
		// query
	    $sql = "SELECT COUNT(*) AS count FROM users WHERE username = '$cleanUser'";
	    $result = $this->DB->query($sql); 
	    
	    // Parse result
	    if(!empty($result)) // if not empty
	    {
	    	// we dont' care about the content, if there is a result this user exists
		    return true
	    }
	    
	    return false; 	
	}	
}

All your other functions should be in a different file that use this class. I'll leave that for you to learn.

* YOUR BIGGEST MISTAKE *

You did not validate the user input before inserting them in an SQL.

Imagine if I tried to login to your used any bogus user name this for a password: hack' OR 1 = 1 LIMIT 1;

Thus your SQL would look like this when executed:

SELECT * FROM users WHERE username = 'hacker' AND password = 'hack' OR 1=1 LIMIT 1;' ";

Then your check, which says the number of results should be 1 return true because i'm sure you have at least one user name in your users table where the number 1 is always equal to 1. This is called

SQL INJECTION

Google the **** out of it. You're software is always unsecured without it.

I've done more than enough. I hope you learn PHP before you write unsafe software like this. I really REALLY hope you go read up on tutorials and practice programming and proper software testing before deploying any code.

Good luck,




Dan

[Apostle]

Thanks Dan for Postive criticism.
I completely rewrote the whole thing and will post it here. For now I it is Here
I will post it here.

The reason I want to write from the scratch is to learn new thing as I go, and I know there are many experts that can drill and expose my ignorance on something and definitely improve my skills.

So feel free to criticize me or advice me on anything (code, good coding habits et al)

Thanks for your time guys :)

[TheServant]

Writing from scratch is the best for learning, and that is what you should do. However, when you start spending time developing, you can't re-write everything (and have a life) so you will need to learn how to use and modify already tried and tested code.

Again, we're here to help when you get stuck, and generally we don't read through screens of code, but if you post snippets for specific problems, we'll mention any issues with the surrounding code no probs ;)

[Apostle]

Any recommended code that I can build upon? As per say, I'm beginner in these things and security matters alot in web apps :)

[Dormilich]

currently the best measure against SQL Injection is using Prepared Statements (implemented in PHP’s MySQLi & PDO classes)

[Apostle]

I have learned a little on MYSQLi, I will check for PDO!
If you don't mind you can provide me a link. For now, I going to google

[Dormilich]

MySQLi
PDO
_______________ __

[Apostle]

Thanks I'm going to check

[dlite922]

Learn OOP too while you're at it. Practice makes perfect. In the beginning working with already made code and reverse engineering it, modifying it, and especially improving and testing is the ultimate learning experience. That is how I learned PHP.

The reason I recommend OOP is I no longer see PHP as a scripting language and I use it for large applications.

In my opinion if someone wants to script, go learn Perl, PHP's sister. She's much much better at little scripts that make your life easier.

An advanced login script to me is an entry to a small to medium application. PHP/MySQL is a good choice for this.



Dan