Performing OOP style in Login System

Question

I know my question is easy to anyone. Just learning this PHP almost one month. I tried to perform my login system using OOP style. Where I need to login as a default user where the username and password is admin. When I tried to login it's saying object not found.

So here my code below.

Table:

CREATE TABLE loginmodule
(
loginId INT PRIMARY KEY AUTO_INCREMENT,
loginUsername VARCHAR(50),
loginPassword VARCHAR(50)
)

Here is my login script.

loginMe.php

<?php
require_once('../connection/connection.php');
require_once('../connection/loginCRUD.php');
require_once('../process/createProcess.php');
?>


<!doctype html>
<html>
<head>
   <title>Login Frame</title>
</head>
<body>
     <div id = "container">
     <h1>Login</h1>
     <form action = "post" action = "../process/createProcess.php">

    <div class = "form-field">
        <input type = "text" id = "username" name = "loginUsername" placeholder = "Enter Username">
    </div>

    <div class = "form-field">
        <input type = "password" id = "password" name = "loginPassword" placeholder = "Enter Password">
    </div>

    <div class = "form-field">
        <input type = "submit" id = "submit" name = "submit" value = "Login">
    </div>
    </form>
    </div><!--- end container --->
   </body>
   </html>

So I set aside my CRUD in another file. loginCRUD.php

<?php
error_reporting(0);
class CRUD
{
 public function readLogin($dbusername,$dbpassword)
 {
    global $myDatabase;
    $result = $myDatabase->query("SELECT * FROM loginmodule WHERE loginUsername = '$dbusername' AND loginPassword = '$dbpassword'");

    if($result->num_rows > 0)
    {
        $row = $result->fetch_assoc();
        return $row;
    }
}
}
?>

Last where I set aside also my process where my validation happens. createProcess.php

<?php

require_once('../connection/connection.php');
require_once('../connection/loginCRUD.php');

session_start();

$dbusername = $_POST['loginUsername']; //Get the value from textfield.
$dbpassword = $_POST['loginPassword'];

if(!empty($dbusername) && !empty($dbpassword))
{
    if($loginUsername == $dbusername && $loginPassword == $dbpassword)
    {
        $create = loginCRUD::readLogin($dbusername,$dbusername);
        echo "You are logged in!";
        @$_SESSION['loginUsername'] = $loginUsername;
    }
}

?>

Guide me if I missed something. If there's a shortcut style than this let me know :)

Can you update the question with the full error message please? It will be easier to help you. — olibiaz, May 09 '16 at 17:46
This statement, `if($loginUsername == $dbusername && $loginPassword == $dbpassword){ ...`, from where did you get `$loginUsername` and `$loginPassword`? — Rajdeep Paul, May 09 '16 at 17:47
You should use parameterized queries as well with prepared statements. This is open to SQL injections as is. http://php.net/manual/en/mysqli.quickstart.prepared-statements.php — chris85, May 09 '16 at 17:48
If your passwords are plain text that also is a separate issue you should correct. http://security.blogoverflow.com/2011/11/why-passwords-should-be-hashed/ — chris85, May 09 '16 at 17:50
@chris85 I didn't notice the type. Just trying to learn this also will move on to Prepared Statements after this :) — Francisunoxx, May 09 '16 at 17:50
Also this statement, `$create = loginCRUD::readLogin($dbusername,$dbusername);` is wrong. You have to create an instance of class `CRUD` to call it's method `readLogin()`. — Rajdeep Paul, May 09 '16 at 17:55
@MiaLegaspi Create an object of class `CRUD` and call it's method `readLogin()`, like this: `(new CRUD)->readLogin($dbusername,$dbpassword);` — Rajdeep Paul, May 09 '16 at 18:03
@RajdeepPaul Sorry I forgot an object. Already create an instance of class but still not working. — Francisunoxx, May 09 '16 at 18:06
@MiaLegaspi I've given an answer below. Hopefully this will resolve your issue. — Rajdeep Paul, May 09 '16 at 18:44

score 3 · Accepted Answer · edited May 23 '17 at 12:16

There are several errors in your code, such as:

There are two action attribute in your form tag.

<form action = "post" action = "../process/createProcess.php">
        ^                ^

It should be,

<form method="post" action="../process/createProcess.php">

On createProcess.php page, look at the following lines,

1) if($loginUsername == $dbusername && $loginPassword == $dbpassword)
            ^                                 ^

There are no variables named $loginUsername and $loginPassword

2) $create = loginCRUD::readLogin($dbusername,$dbusername);
                                       ^            ^ 
                                 both the arguments are same

You're calling readLogin() method in a wrong way. You should first create an instance of class CRUD and then call it's instance method readLogin(), like this way:

   (new CRUD)->readLogin($dbusername,$dbpassword);

3) $_SESSION['loginUsername'] = $loginUsername;

As I said, there is no variable named $loginUsername. It should be,

$_SESSION['loginUsername'] = $dbusername;

Always start session at your very top of your PHP script, right after the opening PHP tag, like this:
```
<?php 
    session_start();
    // your code
```
Your query is susceptible to SQL injection. Use prepared statements for mysqli to prevent any kind SQL injection. And this is how you can prevent SQL injection in PHP.
Never store password as a plain readable text, always perform salted password hashing on raw password before inserting it into the table.
Suggestion: Don't use global in your code. Why Globals are evil?

So your code should be like this:

CRUD class:

class CRUD{
    public function readLogin($dbusername,$dbpassword){
        global $myDatabase;
        $statement = $myDatabase->prepare("SELECT * FROM loginmodule WHERE loginUsername = ? AND loginPassword = ? LIMIT 1");
        $statement->bind_param("ss", $dbusername, $dbpassword);
        if($statement->execute()){
            $result = $statement->get_result();
            if($result->num_rows){
                $row = $result->fetch_assoc();
                return $row;
            }else{
                return false;
            }
        }else{
            return false;
        }
    }
}

createProcess.php

if(isset($_POST['submit'])){
    $dbusername = $_POST['loginUsername'];
    $dbpassword = $_POST['loginPassword'];

    if(!empty($dbusername) && !empty($dbpassword)){
        if((new CRUD)->readLogin($dbusername,$dbpassword)){
            echo "You are logged in!";
            $_SESSION['loginUsername'] = $dbusername;

            // redirect the user to the home page
        }else{
            echo "Incorrect username and/or password";
        }
    }
}

HTML

<div id = "container">
    <h1>Login</h1>
    <form method = "post" action = "../process/createProcess.php">
        <div class = "form-field">
            <input type = "text" id = "username" name = "loginUsername" placeholder = "Enter Username">
        </div>

        <div class = "form-field">
            <input type = "password" id = "password" name = "loginPassword" placeholder = "Enter Password">
        </div>

        <div class = "form-field">
            <input type = "submit" id = "submit" name = "submit" value = "Login">
        </div>
    </form>
</div>

This was the best explanation that I encounter. Last question what is the purpose of `"ss"` in `$statement->bind_param("ss", $dbusername, $dbpassword);` — Francisunoxx, May 10 '16 at 04:40
@MiaLegaspi `s` specifies the type for the corresponding bind variable. Here `"ss"` means both `$dbusername` and `$dbpassword` is of type `string`. Here's [the documentation](http://php.net/manual/en/mysqli-stmt.bind-param.php). — Rajdeep Paul, May 10 '16 at 07:55

Performing OOP style in Login System

1 Answers1