new login system with smarty + login class

Discussions of secure PHP coding. Security in software is important, so don't be afraid to ask. And when answering: be anal. Nitpick. No security vulnerability is too small.

Moderator: General Moderators

Post Reply
staar2
Forum Commoner
Posts: 83
Joined: Fri Apr 06, 2007 2:57 am

new login system with smarty + login class

Post by staar2 »

I haven't done ever login system so this is my first try. If you see some better style or something which can be make differently please tell. Comment code and let me make this code better.

index.php

Code: Select all

 
<?php
<?php
require_once('config.php');
require_once('Login.class.php');
$login = Login::getInstance();
 
if ($login->isLoggedIn()) {
    if ($_GET['page'] == 'logout') {
        $login->logout();
        $sm->assign('member', false);
        header('Location: login.php');
    }
    $sm->assign('auth', $_SESSION['auth']);
    $sm->assign('sid', session_id());
    $sm->assign('member', $login->isLoggedIn());
}
 
$sm->display('index.tpl');
?>
?>
 
index.tpl

Code: Select all

 
{capture assign=content}
<b>Hello world, {$name}</b>
{if $member == true}
    <a href="{$SCRIPT_NAME}?page=logout">Log out</a>
    <p>Hello you are logged in here</p>
    <p>SID: {$sid}</p>
    <p>Auth; {$auth}</p>
{/if}
{/capture}
{include file="default.tpl"}
 
login.php

Code: Select all

 
<?php
require_once 'config.php';
require_once 'Login.class.php';
 
if (isset($_POST['login'])) {
    //make here check if right then log in
    //SELECT id, password, username FROM users WHERE username = 'Jurka' AND password = MD5(123456);
    $login = Login::getInstance();
    $login->setDatabase($db);
    
    if ($login->checkLogin($_POST['username'], $_POST['password'])) {
        $sm->assign('message', 'Logged in');
        header('Location: index.php');
    } else {
        $sm->assign('message', 'Loging in failed!');
    }
}
 
$sm->display('login/login.tpl');
?>
 
login.tpl

Code: Select all

 
{capture assign=content}
{if isset($message)}
    <p>{$message}</p>
{/if}
<form action="{$SCRIPT_NAME}" method="post">
<p>Username: <input type="text" name="username" value="" /></p>
<p>Password: <input type="password" name="password" value="" /></p>
<input type="submit" name="login" value="Log in" />
</form>
{/capture}
{include file="default.tpl"}
 
Login.class.php

Code: Select all

 
<?php
<?php
 
class Login {
    
    private $_db;
    static private $_instance = null;
    
    private function __construct() {
        session_start();
    }
    
    static public function getInstance() {
        if (self::$_instance == null) {
            self::$_instance = new Login();
        }
        
        return self::$_instance;
        
    }
    
    public function setDatabase(PDO $database) {
        $this->_db = $database;
    }
    
    public function checkLogin($username, $password) {
        $sql = "SELECT COUNT(*) 
                FROM `users` 
                WHERE (`username` = :user 
                AND `password` = :pass)
                GROUP BY `username`";
        
        $stmt = $this->_db->prepare($sql);
        $stmt->bindParam(":user", $username);
        $stmt->bindParam(":pass", hash('sha256', $password + PASSWORD_SALT));
        $stmt->execute();
        
        $count = $stmt->fetch();
        
        if ($count[0] == 1) {
            session_regenerate_id(true);
            $_SESSION['username'] = $username;
            $_SESSION['auth'] = md5('auth');
            return true;
        } else {
            return false;
        }
    }
    
    public function logout() {
        session_destroy();
    }
    
    public function isLoggedIn() {
        if (isset($_SESSION['username']) && $_SESSION['auth'] === md5('auth')) {
            return true;
        } else {
            return false;
        }
    }
}
?>
 
config.php

Code: Select all

 
<?php
define('SMARTY', 'smarty/libs/Smarty.class.php');
define('PASSWORD_SALT', 'ThiS Is salt');
 
require_once(SMARTY);
 
$sm = new Smarty();
 
$sm->compile_dir    = 'templates_c/';
$sm->template_dir   = 'templates/';
$sm->debugging      = false;
 
$db = new PDO('mysql:host=localhost;dbname=andmed', 'root', '');
?>
 
Last edited by staar2 on Wed Feb 20, 2008 10:55 am, edited 1 time in total.
User avatar
Sekka
Forum Commoner
Posts: 91
Joined: Mon Feb 18, 2008 10:25 am
Location: Huddersfield, West Yorkshire, UK

Re: new login system with smarty + login class

Post by Sekka »

Personally, I think you are trying to recreate something that has most probably been done before, and better.

I know you will want to make your own, because I am the same, but at least download someone else's and see what they do. Some of the things I have noticed about your methods I would look up and correct,

* You need to escape the variables passed into your SQL code. Your site is wide open to SQL injection at the moment. I think you are using MySQLi, so at least look up preparing statements and data binding. This will solve your problem.

* You are storing passwords in your database using MD5 encryption. This method is old and can be cracked. I suggest you use the new encryptions like SHA512. You implement these using the hash() function.

* You also need to implement password salting for extra security. Basically, each user's password is given a unique key. Before the password is encrypted, the key is added to the end. This ensures that 2 people with the same password will have different encrypted passwords.

* You are storing data in the session to keep them logged in. This is fair enough, but at least encrypt the data so it can't be manipulated externally.

The only reason I point these out is because I researched logins and sessions the other day and found all this out.

Hope these at least give you a head start!
staar2
Forum Commoner
Posts: 83
Joined: Fri Apr 06, 2007 2:57 am

Re: new login system with smarty + login class

Post by staar2 »

Sekka wrote:Personally, I think you are trying to recreate something that has most probably been done before, and better.

I know you will want to make your own, because I am the same, but at least download someone else's and see what they do. Some of the things I have noticed about your methods I would look up and correct,

* You need to escape the variables passed into your SQL code. Your site is wide open to SQL injection at the moment. I think you are using MySQLi, so at least look up preparing statements and data binding. This will solve your problem.

* You are storing passwords in your database using MD5 encryption. This method is old and can be cracked. I suggest you use the new encryptions like SHA512. You implement these using the hash() function.

* You also need to implement password salting for extra security. Basically, each user's password is given a unique key. Before the password is encrypted, the key is added to the end. This ensures that 2 people with the same password will have different encrypted passwords.

* You are storing data in the session to keep them logged in. This is fair enough, but at least encrypt the data so it can't be manipulated externally.

The only reason I point these out is because I researched logins and sessions the other day and found all this out.

Hope these at least give you a head start!
Ty i was waiting for someone who comment the code.
Encryption with salt i going to added, but isn't SHA1 with salt enough?
I'll update code later. :drunk:
staar2
Forum Commoner
Posts: 83
Joined: Fri Apr 06, 2007 2:57 am

Re: new login system with smarty + login class

Post by staar2 »

modified my code and added use of hash function now use sha256 encryption, also encrypted Session data, added the PDO use of prepared statements.
What needs to be done is registration.
Post Reply