Hello and welcome to our community! Is this your first visit?
Register
Enjoy an ad free experience by logging in. Not a member yet? Register.
Page 1 of 2 12 LastLast
Results 1 to 15 of 17
  1. #1
    Regular Coder
    Join Date
    Sep 2010
    Posts
    331
    Thanks
    9
    Thanked 6 Times in 6 Posts

    Anything wrong with my OOP?

    Just starting to learn OOP and first off I have to say I can (finally) see why this is more useful than procedural. Anyway, took this script off of a site, made some very small edits (still more to make). But I was just wondering if anything is wrong (bad practices, code leading to errors, etc)? And how would I make it so the database is checked during/after registration for users with the same username (i.e no two people with the same username/password).

    Classes:
    PHP Code:
    <?php
    include "/includes/database.php";
    class 
    Users {
    private 
    $username;
    private 
    $password;
    private 
    $email;
    private 
    $salt "Zo4rU5Z1YyKJAASY0PT6EUg7BBYdlEhPaNLuxAwU8lqu1ElzHv0Ri7EM6irpx5w";
    //Get user information
    public function __construct($data = array()) {
    if (isset(
    $data['username'])) {
    $this->username mysqli_real_escape_string($data['username']);
    }
    if (isset(
    $data['password'])) {
    $this->password mysqli_real_escape_string($data['password']);
    }
    if (isset(
    $data['email'])) {
    $this->email mysqli_real_escape_string($data['email']);
    }
    public function 
    storeFormValues($params) {
    $this->__construct($params);
    }
    }
    public function 
    userLogin() {
    //success variable will be used to return if the login was successful
    $sucess false;
    try {
        
    //create our pdo object
        
    $con = new PDO(DB_DSNDB_USERNAMEDB_PASSWORD);
        
    //set how pdo will handle errors
        
    $con->setAttribute(PDO::ATTR_ERRMODEPDO::ERRMODE_EXCEPTION);
        
    //this would be our query
        
    $sql "SELECT * FROM `users` WHERE `username` = :username AND `password` = :password LIMIT 1";
        
    //prepare the statements
        
    $stmt $con->prepare($sql);
        
    //give value to named parameter :username
        
    $stmt->bindValue("username"$this->usernamePDO::PARAM_STR);
        
    //give value to named parameter :password
        
    $stmt->bindValue("password"hash("sha256"$this->password $this->salt), PDO PARAM_STR);
        
    $stmt->execute();
        
    $valid $stmt->fetchColumn(); //Set $_SESSION variables
        
    if ($valid) {
            
    $success true;
            
    $_SESSION['loggedin'] = 1;
            
    $mem $stmt->fetchColumn();
            
    $_SESSION['username'] = $mem['username'];
            
    $_SESSION['userid'] = $mem['id'];
            
    $_SESSION['level'] = $mem['level'];
            
    $ip = ($_SERVER['HTTP_X_FORWARDED_FOR']) ? $_SERVER['HTTP_X_FORWARDED_FOR'] : $_SERVER['REMOTE_ADDR'];
        }
        
    $con null;
        return 
    $success;
        } catch (
    PDOException $e) {
        echo 
    $e->getMessage();
        return 
    $success;
        }
        
    }
    public function 
    register() {
    $correct false;
    try {
        
    $con = new PDO(DB_DSNDB_USERNAMEDB_PASSWORD);
        
    $con->setAttribute(PDO::ATTR_ERRMODEPDO::ERRMODE_EXCEPTION);
        
    $sql "INSERT INTO `users`(username, password) VALUES(:username, :password)";
        
        
    $stmt $con->prepare($sql);
        
    $stmt->bindValue("username"$this->usernamePDO::PARAM_STR);
        
    $stmt->bindValue("password"hash("sha256"$this->password $this->salt), PDO::PARAM_STR);
        
    $stmt->execute();
        return 
    "<p>Registration was successful - </p> <a href='#login'>You may now login</a>";
        } catch (
    PDOException $e) {
            return 
    $e->getMessage();
        }
            
    }
    }
    ?>
    Form:
    PHP Code:
    <form method="POST" action="">
    <fieldset>
    <legend>Registration</legend>
    <input type="text" name="username" placeholder="Username" required />
    <input type="password" name="password" placeholder="Password" required />
    <input type="password" name="cpw" placeholder="password" required /> 
    <input type="email" name="email" placeholder="Email Address" required />

    <input type="hidden" name="submitted" value="1" />
    <input type="submit" name="submit" value="Register" />
    </fieldset>
    </form>
    <?php
    $usr 
    = new Users//create new instance of the class Users
    $usr->storeFormValues($_POST['username'], $_POST['password'], $_POST['email']); //store form values
    //if the entered password is match with the confirm password then register him
    if ($_POST['password'] == $_POST['cpw']) {
        echo 
    $usr->register($_POST); 
    } else {
    //if not then say that the user must enter the same password to the confirm box
    echo "<p>Password and Confirm Password fields do not match</p>";
    }
    ?>
    Coding is a challenge, get used to it
    Always remember to debug
    Try the guess & check method
    Break it down into simple steps

  • #2
    God Emperor Fou-Lu's Avatar
    Join Date
    Sep 2002
    Location
    Saskatoon, Saskatchewan
    Posts
    16,994
    Thanks
    4
    Thanked 2,662 Times in 2,631 Posts
    This is very bad practice:
    Code:
    if (isset($data['username'])) {
    $this->username = mysqli_real_escape_string($data['username']);
    }
    You shouldn't be storing information in an escaped format. That should be used only during write to a mysql database. You are using PDO though, so you shouldn't be doing anything with MySQLi if that's the intent. Binding doesn't require, and nor should it be given an escaped string as it will corrupt the original value of the string.

    This makes no sense:
    PHP Code:
    public function storeFormValues($params) {
    $this->__construct($params);

    I don't see a point of calling a constructor on an existing instance of an object. If you need to modify something, do so at the property level; __construct shouldn't be explicitly invoked and should only be used when instantiating a parent constructor or when implicitly called by the new keyword.

  • #3
    Regular Coder
    Join Date
    Sep 2010
    Posts
    331
    Thanks
    9
    Thanked 6 Times in 6 Posts
    Quote Originally Posted by Fou-Lu View Post
    This is very bad practice:
    Code:
    if (isset($data['username'])) {
    $this->username = mysqli_real_escape_string($data['username']);
    }
    You shouldn't be storing information in an escaped format. That should be used only during write to a mysql database. You are using PDO though, so you shouldn't be doing anything with MySQLi if that's the intent. Binding doesn't require, and nor should it be given an escaped string as it will corrupt the original value of the string.

    This makes no sense:
    PHP Code:
    public function storeFormValues($params) {
    $this->__construct($params);

    I don't see a point of calling a constructor on an existing instance of an object. If you need to modify something, do so at the property level; __construct shouldn't be explicitly invoked and should only be used when instantiating a parent constructor or when implicitly called by the new keyword.
    Ah, was wondering about using mysqli AND PDO together.
    So, just:
    PHP Code:
    if (isset($data['username'])) {
    $this->username $data['username'];

    then?
    As for the constructor part, I was actually wondering about that as well when I was looking at the script. I assumed it was sending the values to the storeFormValue(), which then sent them to the constructor, which as you said would be smarter/better/whatever you wanna say to send them straight to the constructor. Methods are class functions and properties are class variables, correct? Or can everything within a class be considered a method?
    Coding is a challenge, get used to it
    Always remember to debug
    Try the guess & check method
    Break it down into simple steps

  • #4
    God Emperor Fou-Lu's Avatar
    Join Date
    Sep 2002
    Location
    Saskatoon, Saskatchewan
    Posts
    16,994
    Thanks
    4
    Thanked 2,662 Times in 2,631 Posts
    Only functions are methods.
    The constructor chain appears to me to be a (lazy at that) short cut. On a side note, doing it the other way around completely would be acceptable solution that doesn't require internally constructing new objects. The constructor can certainly call any method it wants, so if you have an overwriting method, chain the constructor to that instead of the other way around.

    Yeah, PDO and MySQLi are not compatible. Since you need to have MySQLi established to make use of a mysql_real_escape_string anyway, you should choose which to use. But more importantly properties should never be stored in an escaped format, regardless of if you intend to use a prepared statement bound variable or if you intend to use a simple string with escaped input data. Escaping is specific for the db, not for the data in question. It is completely legal for me to have a string with \' in the string, but by escaping it you have now corrupted it onto \\\' instead.

  • #5
    Regular Coder
    Join Date
    Sep 2010
    Posts
    331
    Thanks
    9
    Thanked 6 Times in 6 Posts
    updated code:
    registration:
    PHP Code:
    <?php
    session_start
    ();
    include_once 
    "/includes/database.php";
    include 
    "/includes/classes.php";
    ?>
    <form method="POST" action="">
    <fieldset>
    <legend>Registration</legend>
    <input type="text" name="username" placeholder="Username" required />
    <input type="password" name="password" placeholder="Password" required />
    <input type="password" name="cpw" placeholder="password" required /> 
    <input type="email" name="email" placeholder="Email Address" required />

    <input type="hidden" name="submitted" value="1" />
    <input type="submit" name="submit" value="Register" />
    </fieldset>
    </form>
    <?php
    $usr 
    = new Users($_POST['username'], $_POST['password'], $_POST['email']); //create new instance of the class Users
    //if the entered password is match with the confirm password then register him
    if ($_POST['password'] == $_POST['cpw']) {
        echo 
    $usr->register($_POST['username'], $_POST['password'], $_POST['email']); 
    } else {
    //if not then say that the user must enter the same password to the confirm box
    echo "<p>Password and Confirm Password fields do not match</p>";
    }
    ?>
    </div>
    </div>
    </body>
    </html>
    classes:
    PHP Code:
    <?php
    include "/includes/database.php";
    class 
    Users {
    private 
    $username;
    private 
    $password;
    private 
    $email;
    private 
    $salt "Zo4rU5Z1YyKJAASY0PT6EUg7BBYdlEhPaNLuxAwU8lqu1ElzHv0Ri7EM6irpx5w";
    //Get user information
    public function __construct($data = array()) {
    if (isset(
    $data['username'])) {
    $this->username $data['username'];
    }
    if (isset(
    $data['password'])) {
    $this->password $data['password'];
    }
    if (isset(
    $data['email'])) {
    $this->email $data['email'];
    }
    }
    public function 
    userLogin() {
    //success variable will be used to return if the login was successful
    $sucess false;
    try {
        
    //create our pdo object
        
    $con = new PDO(DB_DSNDB_USERNAMEDB_PASSWORD);
        
    //set how pdo will handle errors
        
    $con->setAttribute(PDO::ATTR_ERRMODEPDO::ERRMODE_EXCEPTION);
        
    //this would be our query
        
    $sql "SELECT * FROM `users` WHERE `username` = :username AND `password` = :password LIMIT 1";
        
    //prepare the statements
        
    $stmt $con->prepare($sql);
        
    //give value to named parameter :username
        
    $stmt->bindValue("username"$this->usernamePDO::PARAM_STR);
        
    //give value to named parameter :password
        
    $stmt->bindValue("password"hash("sha256"$this->password $this->salt), PDO PARAM_STR);
        
    $stmt->execute();
        
    $valid $stmt->fetchColumn(); //Set $_SESSION variables
        
    if ($valid) {
            
    $success true;
            
    $_SESSION['loggedin'] = 1;
            
    $mem $stmt->fetchColumn();
            
    $_SESSION['username'] = $mem['username'];
            
    $_SESSION['userid'] = $mem['id'];
            
    $_SESSION['level'] = $mem['level'];
            
    $ip = ($_SERVER['HTTP_X_FORWARDED_FOR']) ? $_SERVER['HTTP_X_FORWARDED_FOR'] : $_SERVER['REMOTE_ADDR'];
        }
        
    $con null;
        return 
    $success;
        } catch (
    PDOException $e) {
        echo 
    $e->getMessage();
        return 
    $success;
        }
        
    }
    public function 
    register() {
    $correct false;
    try {
        
    $con = new PDO(DB_DSNDB_USERNAMEDB_PASSWORD);
        
    $con->setAttribute(PDO::ATTR_ERRMODEPDO::ERRMODE_EXCEPTION);
        
    $sql "INSERT INTO `users`(username, password, email) VALUES(:username, :password, :email)";
        
        
    $stmt $con->prepare($sql);
        
    $stmt->bindValue("username"$this->usernamePDO::PARAM_STR);
        
    $stmt->bindValue("password"hash("sha256"$this->password $this->salt), PDO::PARAM_STR);
        
    $stmt->bindValue("email"$this->emailPDO::PARAM_STR);
        
    $stmt->execute();
        return 
    "<p>Registration was successful - </p> <a href='#login'>You may now login</a>";
        } catch (
    PDOException $e) {
            return 
    $e->getMessage();
        }
            
    }
    }
    ?>
    Coding is a challenge, get used to it
    Always remember to debug
    Try the guess & check method
    Break it down into simple steps

  • #6
    Senior Coder Dormilich's Avatar
    Join Date
    Jan 2010
    Location
    Behind the Wall
    Posts
    3,444
    Thanks
    13
    Thanked 361 Times in 357 Posts
    your if() condition in the userLogin() method won’t work out. your result set contains only one row that is processed by the first call to fetchColumn(). hence the second call will return false (no more rows to fetch from) and all the array accesses will cause a warning.

    string PDOStatement::fetchColumn ([ int $column_number = 0 ] )

    Returns a single column from the next row of a result set or FALSE if there are no more rows.
    The computer is always right. The computer is always right. The computer is always right. Take it from someone who has programmed for over ten years: not once has the computational mechanism of the machine malfunctioned.
    André Behrens, NY Times Software Developer

  • #7
    Regular Coder
    Join Date
    Sep 2010
    Posts
    331
    Thanks
    9
    Thanked 6 Times in 6 Posts
    Quote Originally Posted by Dormilich View Post
    your if() condition in the userLogin() method won’t work out. your result set contains only one row that is processed by the first call to fetchColumn(). hence the second call will return false (no more rows to fetch from) and all the array accesses will cause a warning.
    so, something like this? or would it just be better to use $valid (even though it is oddly named in this case)
    PHP Code:
    $mem $stmt->fetchColumn(1); 
    and I finally found the original script: http://forum.codecall.net/topic/6977...#axzz2CKPnnQCz
    Coding is a challenge, get used to it
    Always remember to debug
    Try the guess & check method
    Break it down into simple steps

  • #8
    Senior Coder Dormilich's Avatar
    Join Date
    Jan 2010
    Location
    Behind the Wall
    Posts
    3,444
    Thanks
    13
    Thanked 361 Times in 357 Posts
    Quote Originally Posted by elitis View Post
    or would it just be better to use $valid (even though it is oddly named in this case)
    $valid only gives you the content of the first field. you would need a complete ->fetch() to get all data.

    in that original code, all that $valid is used for is to check, whether there are data returned. that these data are superfluous in the concept is a mistake often made. (the well-known problem of the SQL wildcard character *).
    The computer is always right. The computer is always right. The computer is always right. Take it from someone who has programmed for over ten years: not once has the computational mechanism of the machine malfunctioned.
    André Behrens, NY Times Software Developer

  • #9
    Regular Coder
    Join Date
    Sep 2010
    Posts
    331
    Thanks
    9
    Thanked 6 Times in 6 Posts
    Quote Originally Posted by Dormilich View Post
    $valid only gives you the content of the first field. you would need a complete ->fetch() to get all data.

    in that original code, all that $valid is used for is to check, whether there are data returned. that these data are superfluous in the concept is a mistake often made. (the well-known problem of the SQL wildcard character *).
    Ah, should have known that based on ->fetchColumn(). Well, while I was researching fetch(), I stumbled upon fetchAll(). So, my question is what is the difference? Would you say they are interchangeable?
    Coding is a challenge, get used to it
    Always remember to debug
    Try the guess & check method
    Break it down into simple steps

  • #10
    Senior Coder Dormilich's Avatar
    Join Date
    Jan 2010
    Location
    Behind the Wall
    Posts
    3,444
    Thanks
    13
    Thanked 361 Times in 357 Posts
    of course not. ->fetch() gives you the content of one row, ->fetchAll() the content of all rows (which at times could make a very huge array).
    The computer is always right. The computer is always right. The computer is always right. Take it from someone who has programmed for over ten years: not once has the computational mechanism of the machine malfunctioned.
    André Behrens, NY Times Software Developer

  • #11
    Regular Coder
    Join Date
    Sep 2010
    Posts
    331
    Thanks
    9
    Thanked 6 Times in 6 Posts
    Having problems with the $username and $email variables. In the database, only the first letter of each variable's value is displayed. Also, when attempting to log in, I always receive my "Invalid username/password" error message even when the values are correct.

    classes:
    PHP Code:
    <?php
    include "config.php";
    class 
    Users {
    private 
    $username;
    private 
    $password;
    private 
    $email;
    private 
    $salt "Zo4rU5Z1YyKJAASY0PT6EUg7BBYdlEhPaNLuxAwU8lqu1ElzHv0Ri7EM6irpx5w";
    private 
    $ip;
    //Get user information
    public function __construct($data = array()) {
    if (isset(
    $data['username'])) {
    $this->username $data['username'];
    }
    if (isset(
    $data['password'])) {
    $this->password $data['password'];
    }
    if (isset(
    $data['email'])) {
    $this->email $data['email'];
    }
    }
    public function 
    userLogin() {
    //success variable will be used to return if the login was successful
    $sucess false;
    try {
        
    //create our pdo object
        
    $con = new PDO(DB_DSNDB_USERNAMEDB_PASSWORD);
        
    //set how pdo will handle errors
        
    $con->setAttribute(PDO::ATTR_ERRMODEPDO::ERRMODE_EXCEPTION);
        
    //this would be our query
        
    $sql "SELECT * FROM `users` WHERE `username` = :username AND `password` = :password LIMIT 1";
        
    //prepare the statements
        
    $stmt $con->prepare($sql);
        
    //give value to named parameter :username
        
    $stmt->bindValue("username"$this->usernamePDO::PARAM_STR);
        
    //give value to named parameter :password
        
    $stmt->bindValue("password"hash("sha256"$this->password $this->salt), PDO::PARAM_STR);
        
    $stmt->execute();
        
    $valid $stmt->fetchColumn(); //Check data returned & set $_SESSION variables
        
    if ($valid) {
            
    $success true;
            
    $_SESSION['loggedin'] = 1;
            
    $mem $stmt->fetch();
            
    $_SESSION['username'] = $mem['username'];
            
    $_SESSION['userid'] = $mem['id'];
            
    $_SESSION['level'] = $mem['level'];
            
    $ip = ($_SERVER['HTTP_X_FORWARDED_FOR']) ? $_SERVER['HTTP_X_FORWARDED_FOR'] : $_SERVER['REMOTE_ADDR'];
        }
        
    $con null;
        return 
    $success;
        } catch (
    PDOException $e) {
        echo 
    $e->getMessage();
        return 
    $success;
        }    
    }
    public function 
    register() {
    $correct false;
    try {
        
    $con = new PDO(DB_DSNDB_USERNAMEDB_PASSWORD);
        
    $con->setAttribute(PDO::ATTR_ERRMODEPDO::ERRMODE_EXCEPTION);
        
    $checkUsrs "SELECT * FROM `users` WHERE `username`= :username";
        
    $usrsChecked $con->prepare($checkUsrs);
        
    $usrsChecked->bindValue("username"$this->usernamePDO::PARAM_STR);
        
    $usrsChecked->execute(); 
        
    $valid $usrsChecked->fetchColumn();
        if (!
    $valid) {
        
    $sql "INSERT INTO `users`(ip, username, password, email) VALUES(:ip, :username, :password, :email)";
        
    $stmt $con->prepare($sql);
        
    $stmt->bindValue("ip"$this->ipPDO::PARAM_STR);
        
    $stmt->bindValue("username"$this->usernamePDO::PARAM_STR);
        
    $stmt->bindValue("password"hash("sha256"$this->password $this->salt), PDO::PARAM_STR);
        
    $stmt->bindValue("email"$this->emailPDO::PARAM_STR);
        
    $stmt->execute();
        return 
    "<p>Registration was successful - </p> <a href='#login'>You may now login</a>";
        }
        else echo 
    "<p>Username already taken</p>";
        } catch (
    PDOException $e) {
            return 
    $e->getMessage();
        }
            
    }
    function 
    getIp() {
        
    $this->ip $_SERVER['REMOTE_ADDR'];
        if (!empty(
    $_SERVER['HTTP_CLIENT_IP'])) {
            
    $this->ip $_SERVER['HTTP_CLIENT_IP'];
        } elseif (!empty(
    $_SERVER['HTTP_X_FORWARDED_FOR'])) {
            
    $this->ip $_SERVER['HTTP_X_FORWARDED_FOR'];
        }
        return 
    $this->ip;
    }
    }
    ?>
    Login form script:
    PHP Code:
    <?php
        $usr 
    = new Users($_POST['username'], $_POST['password']);
        if (
    $_POST['submitted'] == or isset($_POST['submit']))
        {
        if (
    $usr->userLogin()) {
            echo 
    "Welcome ," $_SESSION['username']; 
            } else {
            echo 
    "<p style='color:red;'>Invalid Username/Password</p>";
            }
        }
        
    ?>
    register page script:
    PHP Code:
    <?php
    $usr 
    = new Users($_POST['username'], $_POST['password'], $_POST['email']); //create new instance of the class Users
    if (isset($_POST['submit']) or $_POST['submitted'] == 1)
    {
    $usrIP $usr->getIp();
    echo 
    $usr->register($_POST['username'], $_POST['password'], $_POST['email'], $usrIP); 
    }
    ?>
    Coding is a challenge, get used to it
    Always remember to debug
    Try the guess & check method
    Break it down into simple steps

  • #12
    Senior Coder Dormilich's Avatar
    Join Date
    Jan 2010
    Location
    Behind the Wall
    Posts
    3,444
    Thanks
    13
    Thanked 361 Times in 357 Posts
    second code block: Users constructor expects an array ...

    first code block:
    - $mem = $stmt->fetch(); no more data to fetch, returning false.
    - username already exists in the class
    - SQL, no need of a LIMIT 1 if you have proper DB constraints (e.g. unique username)

    third code block: Users->register() does not accept parameters
    The computer is always right. The computer is always right. The computer is always right. Take it from someone who has programmed for over ten years: not once has the computational mechanism of the machine malfunctioned.
    André Behrens, NY Times Software Developer

  • #13
    Regular Coder
    Join Date
    Sep 2010
    Posts
    331
    Thanks
    9
    Thanked 6 Times in 6 Posts
    Thanks Dormilich, fixed everything but now ran into another (presumably) smaller problem. It isn't echoing the $username property. I'd assume this is because, to paraphrase, the $mem property is returning false due to lack of data. But how exactly would I fix that? I've tried fetchAll(), and fetch(1), but to no avail.

    classes:
    PHP Code:
    <?php
    include "config.php";
    class 
    Users {
    private 
    $username;
    private 
    $password;
    private 
    $email;
    private 
    $salt "Zo4rU5Z1YyKJAASY0PT6EUg7BBYdlEhPaNLuxAwU8lqu1ElzHv0Ri7EM6irpx5w";
    private 
    $ip;
    //Get user information
    public function __construct($data = array()) {
    if (isset(
    $data['username'])) {
    $this->username $data['username'];
    }
    if (isset(
    $data['password'])) {
    $this->password $data['password'];
    }
    if (isset(
    $data['email'])) {
    $this->email $data['email'];
    }
    $this->ip $_SERVER['REMOTE_ADDR'];
        if (!empty(
    $_SERVER['HTTP_CLIENT_IP'])) {
            
    $this->ip $_SERVER['HTTP_CLIENT_IP'];
        } elseif (!empty(
    $_SERVER['HTTP_X_FORWARDED_FOR'])) {
            
    $this->ip $_SERVER['HTTP_X_FORWARDED_FOR'];
        }
    }
    public function 
    userLogin() {
    //success variable will be used to return if the login was successful
    $sucess false;
    try {
        
    //create our pdo object
        
    $con = new PDO(DB_DSNDB_USERNAMEDB_PASSWORD);
        
    //set how pdo will handle errors
        
    $con->setAttribute(PDO::ATTR_ERRMODEPDO::ERRMODE_EXCEPTION);
        
    //this would be our query
        
    $sql "SELECT * FROM `users` WHERE `username` = :username AND `password` = :password";
        
    //prepare the statements
        
    $stmt $con->prepare($sql);
        
    //give value to named parameter :username
        
    $stmt->bindValue("username"$this->usernamePDO::PARAM_STR);
        
    //give value to named parameter :password
        
    $stmt->bindValue("password"hash("sha256"$this->password $this->salt), PDO::PARAM_STR);
        
    $stmt->execute();
        
    $valid $stmt->fetchColumn(); //Check data returned & set $_SESSION variables
        
    if ($valid) {
            
    $success true;
            
    $_SESSION['loggedin'] = 1;
            
    $mem $stmt->fetch(1);
            
    $_SESSION['username'] = $mem['username'];
            
    $_SESSION['userid'] = $mem['id'];
            
    $_SESSION['level'] = $mem['level'];
            
    $userIp = ($_SERVER['HTTP_X_FORWARDED_FOR']) ? $_SERVER['HTTP_X_FORWARDED_FOR'] : $_SERVER['REMOTE_ADDR'];
        }
        
    $con null;
        return 
    $success;
        } catch (
    PDOException $e) {
        echo 
    $e->getMessage();
        return 
    $success;
        }    
    }
    public function 
    register() {
    $correct false;
    try {
        
    $con = new PDO(DB_DSNDB_USERNAMEDB_PASSWORD);
        
    $con->setAttribute(PDO::ATTR_ERRMODEPDO::ERRMODE_EXCEPTION);
        
    $checkUsrs "SELECT * FROM `users` WHERE `username`= :username";
        
    $usrsChecked $con->prepare($checkUsrs);
        
    $usrsChecked->bindValue("username"$this->usernamePDO::PARAM_STR);
        
    $usrsChecked->execute(); 
        
    $valid $usrsChecked->fetchColumn();
        if (!
    $valid) {
        
    $sql "INSERT INTO `users`(ip, username, password, email) VALUES(:ip, :username, :password, :email)";
        
    $stmt $con->prepare($sql);
        
    $stmt->bindValue("ip"$this->ipPDO::PARAM_STR);
        
    $stmt->bindValue("username"$this->usernamePDO::PARAM_STR);
        
    $stmt->bindValue("password"hash("sha256"$this->password $this->salt), PDO::PARAM_STR);
        
    $stmt->bindValue("email"$this->emailPDO::PARAM_STR);
        
    $stmt->execute();
        return 
    "<p>Registration was successful - </p> <a href='#login'>You may now login</a>";
        }
        else echo 
    "<p>Username already taken</p>";
        } catch (
    PDOException $e) {
            return 
    $e->getMessage();
        }
            
    }
    }
    ?>
    login script:
    PHP Code:
        <?php
        $usr 
    = new Users($_POST);
        if (!isset(
    $_POST['submit'])) {
        echo 
    "<form method='POST' action=''>
                    <input type='text' style='position:relative;top:10px;right:25.5%;float:right;border-radius:5px;padding-left:5px;height:20px;' placeholder='Username' name='username' />
                    <input type='password' style='position:relative;top:10px;right:-6%;float:right;border-radius:5px;padding-left:5px;height:20px;' placeholder='password' name='password' />
                    <input type='submit' style='position:relative;top:12px;right:-28.5%;float:right;border-radius:5px;' name='submit' value='Sign In' />
            <input type='hidden' name='submitted' value='1' />
            </form>
            <a href='#' style='text-decoration:none;float:right;position:relative;top:35px;right:-10%;'>Forgot Username?</a>
            <a href='#' style='text-decoration:none;float:right;position:relative;top:35px;right:-37.5%;'>Forgot Password?</a>"
    ; } else {
            if (
    $usr->userLogin()) {
                if (isset(
    $_SESSION['loggedin']) && $_SESSION['loggedin'] == 1
                    echo 
    "<p>Welcome ," $_SESSION['username'] . '</p>'; } 
                    else { echo 
    "<p style='color:red;'>Invalid Username/Password</p>";    
                    echo 
    "<form method='POST' action=''>
                    <input type='text' style='position:relative;top:10px;right:25.5%;float:right;border-radius:5px;padding-left:5px;height:20px;' placeholder='Username' name='username' />
                    <input type='password' style='position:relative;top:10px;right:-6%;float:right;border-radius:5px;padding-left:5px;height:20px;' placeholder='password' name='password' />
                    <input type='submit' style='position:relative;top:12px;right:-28.5%;float:right;border-radius:5px;' name='submit' value='Sign In' />
            <input type='hidden' name='submitted' value='1' />
            </form>
            <a href='#' style='text-decoration:none;float:right;position:relative;top:35px;right:-10%;'>Forgot Username?</a>
            <a href='#' style='text-decoration:none;float:right;position:relative;top:35px;right:-37.5%;'>Forgot Password?</a>"
    ;
            }
            }
        
    ?>
    Side Note: It is allowing me to log in with no trouble, so I'd assume $_SESSION['loggedin'] is set and is set to 1, although whenever I exit that tab and return I must log in again which makes me assume something (by which I mean what you previously stated) is going on with the $_SESSION variables in that if statement
    Last edited by elitis; 11-21-2012 at 01:45 AM.
    Coding is a challenge, get used to it
    Always remember to debug
    Try the guess & check method
    Break it down into simple steps

  • #14
    Senior Coder Dormilich's Avatar
    Join Date
    Jan 2010
    Location
    Behind the Wall
    Posts
    3,444
    Thanks
    13
    Thanked 361 Times in 357 Posts
    the problem is that by calling $valid = $stmt->fetchColumn(); you get the first field’s value and then move to the next result row (that doesn’t exist). hence if you want to get the data out of the original row, you need to change that line. bear in mind that the original code was never interested in DB data in the first place.
    The computer is always right. The computer is always right. The computer is always right. Take it from someone who has programmed for over ten years: not once has the computational mechanism of the machine malfunctioned.
    André Behrens, NY Times Software Developer

  • #15
    Regular Coder
    Join Date
    Sep 2010
    Posts
    331
    Thanks
    9
    Thanked 6 Times in 6 Posts
    Quote Originally Posted by Dormilich View Post
    the problem is that by calling $valid = $stmt->fetchColumn(); you get the first field’s value and then move to the next result row (that doesn’t exist). hence if you want to get the data out of the original row, you need to change that line. bear in mind that the original code was never interested in DB data in the first place.
    ugh, almost there...It is displaying the username now, but when the tab is exited and then reopened you are required to log in again. I'd assume it was this line: if (!isset($_POST['submit'])) {
    updated login method:
    PHP Code:
    public function userLogin() {
    //success variable will be used to return if the login was successful
    $sucess false;
    try {
        
    //create our pdo object
        
    $con = new PDO(DB_DSNDB_USERNAMEDB_PASSWORD);
        
    //set how pdo will handle errors
        
    $con->setAttribute(PDO::ATTR_ERRMODEPDO::ERRMODE_EXCEPTION);
        
    //this would be our query
        
    $sql "SELECT * FROM `users` WHERE `username` = :username AND `password` = :password";
        
    //prepare the statements
        
    $stmt $con->prepare($sql);
        
    //give value to named parameter :username
        
    $stmt->bindValue("username"$this->usernamePDO::PARAM_STR);
        
    //give value to named parameter :password
        
    $stmt->bindValue("password"hash("sha256"$this->password $this->salt), PDO::PARAM_STR);
        
    $stmt->execute();
        
    $vMem $stmt->fetch(); //Check data returned & set $_SESSION variables
        
    if ($vMem) {
            
    $success true;
            
    $_SESSION['loggedin'] = 1;
            
    $_SESSION['username'] = $vMem['username'];
            
    $_SESSION['userid'] = $vMem['id'];
            
    $_SESSION['level'] = $vMem['level'];
            
    $userIp = ($_SERVER['HTTP_X_FORWARDED_FOR']) ? $_SERVER['HTTP_X_FORWARDED_FOR'] : $_SERVER['REMOTE_ADDR'];
        }
        
    $con null;
        return 
    $success;
        } catch (
    PDOException $e) {
        echo 
    $e->getMessage();
        return 
    $success;
        }    

    login script:
    PHP Code:
    <?php
        $usr 
    = new Users($_POST);
        if (!isset(
    $_POST['submit'])) {
        echo 
    "<form method='POST' action=''>
                    <input type='text' style='position:relative;top:10px;right:25.5%;float:right;border-radius:5px;padding-left:5px;height:20px;' placeholder='Username' name='username' />
                    <input type='password' style='position:relative;top:10px;right:-6%;float:right;border-radius:5px;padding-left:5px;height:20px;' placeholder='password' name='password' />
                    <input type='submit' style='position:relative;top:12px;right:-28.5%;float:right;border-radius:5px;' name='submit' value='Sign In' />
            <input type='hidden' name='submitted' value='1' />
            </form>
            <a href='#' style='text-decoration:none;float:right;position:relative;top:35px;right:-10%;'>Forgot Username?</a>
            <a href='#' style='text-decoration:none;float:right;position:relative;top:35px;right:-37.5%;'>Forgot Password?</a>"
    ; } else {
            if (
    $usr->userLogin()) {
                if (isset(
    $_SESSION['loggedin']) && $_SESSION['loggedin'] == 1
                    echo 
    "<p>Welcome ," $_SESSION['username'] . '</p>'; } 
                    else { echo 
    "<p style='color:red;'>Invalid Username/Password</p>";    
                    echo 
    "<form method='POST' action=''>
                    <input type='text' style='position:relative;top:10px;right:25.5%;float:right;border-radius:5px;padding-left:5px;height:20px;' placeholder='Username' name='username' />
                    <input type='password' style='position:relative;top:10px;right:-6%;float:right;border-radius:5px;padding-left:5px;height:20px;' placeholder='password' name='password' />
                    <input type='submit' style='position:relative;top:12px;right:-28.5%;float:right;border-radius:5px;' name='submit' value='Sign In' />
            <input type='hidden' name='submitted' value='1' />
            </form>
            <a href='#' style='text-decoration:none;float:right;position:relative;top:35px;right:-10%;'>Forgot Username?</a>
            <a href='#' style='text-decoration:none;float:right;position:relative;top:35px;right:-37.5%;'>Forgot Password?</a>"
    ;
            }
            }
        
    ?>
    Coding is a challenge, get used to it
    Always remember to debug
    Try the guess & check method
    Break it down into simple steps


  •  
    Page 1 of 2 12 LastLast

    Posting Permissions

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