Login Script (another~)

Coding Critique is the place to post source code for peer review by other members of DevNetwork. Any kind of code can be posted. Code posted does not have to be limited to PHP. All members are invited to contribute constructive criticism with the goal of improving the code. Posted code should include some background information about it and what areas you specifically would like help with.

Popular code excerpts may be moved to "Code Snippets" by the moderators.

Moderator: General Moderators

Post Reply
matt.lo
Forum Newbie
Posts: 2
Joined: Sun Mar 08, 2009 8:41 pm

Login Script (another~)

Post by matt.lo »

Code: Select all

 
<?php
/*
matt lo
 
simple login check
 
*/
 
class login {
    protected $_error = NULL; // error set
    protected $_goto = NULL; // goto page ex: index.php or ?id=2
    
    // construct, what is the class going to do
    public function __construct($goto) { // if login accepted, go to what page (same for logout)
        // REQUIRED HIDDEN FIELD!!! handlecheck, its to identify the login form
        if(!isset($_POST['handlecheck'])) {
            throw new Exception("handlecheck not detected in the post form");
        }
        
        // create a unique token to login, i will make this random per visit in the future
        if($_POST['handlecheck'] != md5('Lg51')) {
            throw new Exception("handlecheck token does not match");
        }
        $this->_goto = $goto;
        
    }
 
    // check login fields
    public function check_login() {
        // sleep, bot filter
        sleep(5); // 5 seconds
        
        // additional room for custom fields....
        if($_POST['email'] == '' || $_POST['password'] == '') {
            $this->_error = "Email and/or password are invalid.";
        }
        
        // this checks if email/password right
        $q = mysql_query("SELECT * FROM `site_users` WHERE `email` = '".$_POST['email']."' AND `password` = '".md5($_POST['password'])."' LIMIT 0,1");      
        
        // if theres a match, everything is good
        if(mysql_num_rows($q) == 0) {
            $this->_error = "Email and/or password are invalid.";
        }
        
        if($this->boolcheck() !== false) {
            // row data from id
            $row = mysql_fetch_assoc($q);
            //if remember me is set in form (checkbox)
            if(isset($_POST['rem'])) {
                if($_POST['rem'] == '1') {
                    // set a token for unique id
                    $token = md5(uniqid(rand(), true));
                    // set a remember me cookie, expires every 2 weeks
                    setcookie('529_rem_me_100', $token, time()*3600*24*14);
                    // save token in database
                    mysql_query("UPDATE `site_users` SET `rem_token` = '".$token."' WHERE `id` = '".$row['id']."'");
                    if(mysql_error()) {
                        throw new Exception(mysql_error());
                    }
                }
            }
            $_SESSION['id'] = $row['id'];
            header("location: ".$this->_goto);
        } else {// log attempt
            // insert attempt data into sql
            mysql_query("INSERT INTO `ip_login_log` (`ip`, `emailattempt`, `datestamp`) VALUES ('".$_SERVER['REMOTE_ADDR']."', '".$_POST['email']."', '".date("Ymd")."')");
            if(mysql_error()) {// if sql error
                throw new Exception(mysql_error());
            }
        }
    }
    
    
    public function boolcheck() { // boolean return if any errors happened
        if($this->_error == NULL) {
            return true;
        } else {
            return false;
        }
    }
    
    // if errors, return the response
    public function error_response() {
        return $this->_error;
    }
    
    public function logout() {
        // delete cookie
        setcookie('529_rem_me_100', '-', time()-3600);
        unset($_SESSION['id']);
        header("location: ".$this->_goto);
    }
 
}
 
?>
I havnt made a sample yet but i think its self explanatory
before coding, do if(isset($_POST['sdafsd'])) {
$l = new login('p.php');
$l->check_login();
if($l->boolcheck() === false) {
echo $l->error_response();
}
}
crazycoders
Forum Contributor
Posts: 260
Joined: Tue Oct 28, 2008 7:48 am
Location: Montreal, Qc, Canada

Re: Login Script (another~)

Post by crazycoders »

Aint it a bit complex? You are doing a class that forces a user to use a specific form setting and values. I don't really like that because you are giving 3 hats to your class. A class must be something that is used for one specific task... Login in a user is a multiple task that involves (in this case): querying the credentials, validating the form, validating the user.

Further more, although i have not inspected your code, i don't like the example, it's not intuitive... The concept of a class is the make something simpler and more intuitive. If it's more complex or requires reading to understand something as easy to do as to login, then you have failed that concept...

Rules of programming:
1. Don't reinvent the wheel, produce something new that does something different than others do...
2. Don't make things more complex, programming is used to simplify things and make everything more productive...

Sorry about my comment, but i think that's what it's worth!
User avatar
jayshields
DevNet Resident
Posts: 1912
Joined: Mon Aug 22, 2005 12:11 pm
Location: Leeds/Manchester, England

Re: Login Script (another~)

Post by jayshields »

You're passing $_POST values directly into a MySQL query...
Post Reply