SitePoint Sponsor

User Tag List

Results 1 to 2 of 2
  1. #1
    SitePoint Member
    Join Date
    Jul 2006
    Location
    Dublin & Dundalk
    Posts
    9
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    Please critique my user management script

    Hi guys,

    I've written a user management script for my site. It works perfectly and I'm just wondering could you all have a look and tell me if there's any gaping holes, security worries, crappy scripting or any thoughts that cross your mind while looking at it. Thanks.

    This is my first go at OOP aswell, so any pointers there so I don't develop bad habits would be much appreciated.

    First off here is the SQL for the user's db. (For Reference)
    Code:
    CREATE TABLE `users` (
      `user_id` int(7) unsigned zerofill NOT NULL auto_increment,
      `username` varchar(20) default NULL,
      `psword` blob NOT NULL,
      `email` varchar(255) NOT NULL,
      PRIMARY KEY  (`user_id`),
      UNIQUE KEY `email` (`email`),
      UNIQUE KEY `username` (`username`)
    ) ENGINE=InnoDB;
    The idea of the script is that you can login/out from any page by just POSTing the login data to the page with a hidden field 'login' or logout by GETing the field logout.

    It uses cookies to keep a user logged-in and if they log-out it remembers their email address.

    Here is the class:
    PHP Code:
    <?php
    class User {
        var 
    $loginfailed FALSE;
        var 
    $errmsg;
        var 
    $userdata;
        var 
    $remember FALSE;

        function 
    User () {
        
    # Can call login/out from any page and it will log them in and continue to display the page
        
    if ($_POST['login'] == 1$this->login($_POST['username'], $_POST['password'], $_POST['remember']);
        elseif (
    $_GET['logout'] == 1$this->logout();
        
    # Logs in unlogged-in users with cookies
        
    elseif (!isset($_SESSION['loggedin']) && isset($_COOKIE['my_site_name']['user_id'], $_COOKIE['my_site_name']['hash'])) { // loggedin is not set yet or is false
            
    $this->eatCookie();
            }
        }

        function 
    login ($username$password$remember) {
            
    $this->remember $remember;
            
    $result mysql_query("SELECT user_id FROM users WHERE username = '$username' OR email = '$username';");
            if (
    mysql_num_rows($result) !== 1) {
                
    // No username like this exists, show login form again
                
    $this->loginfailed TRUE;
                
    $this->errmsg 'Sorry, Your Username Doesn\'t Exist';
                return 
    FALSE;
                }
            else {
                
    $user_id mysql_result($result0);
                
    $this->userdata mysql_fetch_object(mysql_query("SELECT user_id, email, psword FROM users WHERE psword = PASSWORD('$password')"));
                if (!
    is_object($this->userdata)) {
                    
    // Wrong password
                    
    $this->loginfailed TRUE;
                    
    $this->errmsg 'Sorry, Wrong Password';
                    return 
    FALSE;
                    }
                else {
                    if (
    $this->doLogin()) return TRUE;
                    }
                }
            }

            function 
    doLogin () {
                
    // set as logged in
                
    $_SESSION['loggedin'] = TRUE;
                
    $_SESSION['user_id'] = $this->userdata->user_id;
                
    // set the cookies
                
    if ($this->setCookies()) return TRUE;
                }

                function 
    setCookies () {
                    
    // set cookie to remember the username/email address anyway
                    
    setcookie('my_site_name[email]'$this->userdata->emailmktime()+(86400*30), '/'''FALSE);
                    if (
    $this->remember) {
                        
    $cookie md5($this->userdata->email $this->userdata->psword);
                        if (
    setcookie('my_site_name[user_id]'$this->userdata->user_idmktime()+(86400*30), '/'''FALSE) &&
                            
    setcookie('my_site_name[hash]'$cookiemktime()+(86400*30), '/'''FALSE)) {
                            return 
    TRUE;
                            }
                        }
                    }

        function 
    logout() {
            unset(
    $_SESSION['loggedin']);
            unset(
    $_SESSION['user_id']);
            
    // Delete cookie by setting value to nothing
            
    $this->clearCookies();
            }

            function 
    clearCookies () {
                
    // clears 'remember my pass' cookies and leaves e-mail/username one
                
    if (setcookie('my_site_name[user_id]'''mktime()+(86400*30), '/'''FALSE) &&
                    
    setcookie('my_site_name[hash]'''mktime()+(86400*30), '/'''FALSE)) {
                    return 
    TRUE;
                    }
                }

        function 
    eatCookie () {
            
    // Gets data to make hash using uid in cookie
            
    $this->userdata mysql_fetch_object(mysql_query("SELECT user_id, psword, email FROM users WHERE user_id = {$_COOKIE['my_site_name']['user_id']};"));
            
    // makes hash
            
    $hash md5($this->userdata->email $this->userdata->psword);
            if (
    $hash == $_COOKIE['my_site_name']['hash']) {
                
    $this->remember TRUE;
                if (
    $this->doLogin()) {
                    return 
    TRUE;
                    };
                }
            }

        function 
    showLoginForm () {
            if (
    $this->loginfailed) echo "<h6 class='error'>$this->errmsg</h6>"?>
            <form action="<?php echo $_SERVER['PHP_SELF']; ?>" method="post" enctype="multipart/form-data" name="loginform">
            <input name="login" type="hidden" value="1" />
            <input name="username" type="text" value="<?php if (!$this->loginfailed) echo $_COOKIE['my_site_name']['email']; ?>" />
            <input name="password" type="password" />
            <input name="remember" type="checkbox" value="TRUE" checked />
            <input name="" type="submit" />
            </form> <?php
            
    }

        }
    ?>
    It would be used in a page as such:
    PHP Code:
    <?php
    require_once('userclass.php');
    session_start();
    connect(); // script to connect to MySQL db
    $user = new User(); ?>

    <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
    <html xmlns="http://www.w3.org/1999/xhtml">
    <head>
    <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1" />
    <title>Log-In</title>
    </head>
    <body>
    <h1>User Login</h1>

    <?php $user->showLoginForm(); ?>

    </body>
    </html>
    Please give me your thoughts on this. I was trying to use this script (click for its &#100;&#105;&#103;&#103; page (why does it have '&#100;&#105;&#103;&#103;' as a bad word??)) but you can see from the bad comments themselves that its no good.

    Thanks in advance for any input, and if you think its decent feel free to use it if you credit me (Patrick Murphy - www.spudmurphydesign.com). If the commenting is a bit sparse please let me know where you're caught up and I'll help out.

    Thanks guys.

  2. #2
    SitePoint Guru
    Join Date
    May 2005
    Location
    Finland
    Posts
    608
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    As so many others before you, you have got all of your layers (Model, View and Controller or domain logic (database), display logic and decision making based on requests) mixed in a single class. That's a procedural way of doing things - quite understandable, if this is your first attempt at OOP. See here and here for a few similar discussions.


Bookmarks

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •