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

<?php
//php login sytem
class LoginRegister{
 function __construct(){
}
 
function displogin($status){
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($sql) 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;'>Username or password not correct.
        <br /> New? Register!</p>";
        $this->displogin('register');
    }// end else
}
 
 
function checkempty($username, $password, $mode){
    if (empty($username) or empty($password)){
    echo "<p style = 'color:red; font-weight:bold;'>Empty Values are not allowed</p>";
    $this->displogin('login');
    }//end if
    else{
    //do checking
    switch($mode){
        case 'login':
        $this->auth($username, $password);
        case 'register':
        $this->adduser($username, $password);
        default:
            echo "<p style = 'color:red; font-weight:bold;'>Wrong Values are not allowed</p>";
            $this->displogin('login');
        }//end switch
    }//end else
}
 
function login($uname, $passwd){
    //username
    $username = stripslashes($uname);
    $username = mysql_real_escape_string($uname);
    //passsword    
    $password = stripslashes($passwd);
    $password = mysql_real_escape_string($passwd);
    //check for empty variables
    $this->checkempty($username, $password, 'login');    
}
 
function register($uname, $passwd){
    //username
    $username = stripslashes($uname);
    $username = mysql_real_escape_string($uname);
    //passsword    
    $password = stripslashes($passwd);
    $password = mysql_real_escape_string($passwd);
    //check for empty variables
    $this->checkempty($username, $password, 'register');    
}
 
function adduser($username, $password){
    $sql = "INSERT INTO users(username, password) VALUES('$username', '$password')";
    //redirect to login page
    echo "<p style = 'color:green; font-weight:bold;'>Thanks for registering. You can now login</p>";
    $this->displogin('login');
    mysql_query($sql) or die(mysql_error());
}
 
}//end class
?>
 

index.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_encode('login'))){
    $obj->login($_POST['username'], $_POST['password']);
     }//end middle first if
     else if(($_GET['do'])== (base64_encode('register'))){
        $obj->register($_POST['username'], $_POST['password']);
     }
     else{
         echo "<p style = 'color:red; font-weight:bold;'>Please Login</p>";
        $obj->displogin('login');    
        //debug
        echo base64_encode('login').'<br />';
        echo $_GET['do'];
     }//end else middle
 
}//end last if 
else{
    echo "<p style = 'color:green; font-weight:bold;'>Please Login</p>";
    $obj->displogin('login');    
}//end else
?>
 

[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]

@Dormilich
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]

@Apostle
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)