2
\$\begingroup\$

I have followed a mysqli tutorial to create a login with cookies and session with remember me authentication in pdo.

in tutorial class had a new db_connection, I tried to inject my pdo connection in to class, same as in here: https://phpdelusions.net/pdo and read some answers in stackowerflow by Your Common Sence and dharman answers for injecting pdo connection in to class.

I was getting errors on it, so I changed it to global $pdo;.

I need your reviews and your reccomendations on my login script, especially on sessionCheck.php security and corrections. I am not a perofessional coder yet :) but trying hard and learning very fast, so please dont yell on me, and if possible show your recommendations with examples, Thanks

Here is my login handler class which is doing all queries simple:

LoginHandler.php

class LoginHandler {

    function runBaseQuery($sql, $args) {
        global $pdo;
        if (!$args){
             return $pdo->query($sql);
        }
        $stmt = $pdo->prepare($sql);
        $stmt->execute($args);

        if ($stmt->rowCount() > 0) {
            while($row = $stmt->fetch()) {
                $resultset[] = $row;
            }
        }

        if(!empty($resultset)) {
            return $resultset;
        }
    }

    function bindQueryParams($sql, $args){
        global $pdo;
        if (!$args){
             return $pdo->query($sql);
        }
        $stmt = $pdo->prepare($sql);
        $stmt->execute($args);
        return $stmt;
    }
}

Auth.php

require_once "../db.php";
require "LoginHandler.php";
class Auth {
    function getMemberByUsername($username) {
        global $pdo;
        $Lhandle = new LoginHandler();
        $sql = "Select * from users where username = ?";
        $result = $Lhandle->runBaseQuery($sql,  array($username));
        return $result;
    }

    function getTokenByUsername($username,$expired) {
        global $pdo;
        $Lhandle = new LoginHandler();
        $sql = "Select * from Auth_tokens where username = ? and is_expired = ?";
        $result = $Lhandle->runBaseQuery($sql, array($username, $expired));
        return $result;
    }

    function markAsExpired($tokenId) {
        global $pdo;
        $Lhandle = new LoginHandler();
        $sql = "UPDATE Auth_tokens SET is_expired = ? WHERE id = ?";
        $expired = 1;
        $result = $Lhandle->bindQueryParams($sql, array($expired, $tokenId));
        return $result;
    }

    function insertToken($username, $random_password_hash, $random_selector_hash, $expiry_date) {
        global $pdo;
        $Lhandle = new LoginHandler();
        $sql = "INSERT INTO Auth_tokens (username, password_hash, selector_hash, expiry_date) values (?, ?, ?, ?)";
        $result = $Lhandle->bindQueryParams($sql, array($username, $random_password_hash, $random_selector_hash, $expiry_date));
        return $result;
    }   
}

Util.php

class Util {
    public function getToken($length)
    {
        $token = "";
        $codeAlphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";
        $codeAlphabet .= "abcdefghijklmnopqrstuvwxyz";
        $codeAlphabet .= "0123456789";
        $max = strlen($codeAlphabet) - 1;
        for ($i = 0; $i < $length; $i ++) {
            $token .= $codeAlphabet[$this->cryptoRandSecure(0, $max)];
        }
        return $token;
    }

    public function cryptoRandSecure($min, $max)
    {
        $range = $max - $min;
        if ($range < 1) {
            return $min; // not so random...
        }
        $log = ceil(log($range, 2));
        $bytes = (int) ($log / 8) + 1; // length in bytes
        $bits = (int) $log + 1; // length in bits
        $filter = (int) (1 << $bits) - 1; // set all lower bits to 1
        do {
            $rnd = hexdec(bin2hex(openssl_random_pseudo_bytes($bytes)));
            $rnd = $rnd & $filter; // discard irrelevant bits
        } while ($rnd >= $range);
        return $min + $rnd;
    }

    public function redirect($url) {
        header("Location:" . $url);
        exit();
    }

    public function clearAuthCookie() {
        if (isset($_COOKIE["member_login"])) {
            setcookie("member_login", "");
        }
        if (isset($_COOKIE["random_password"])) {
            setcookie("random_password", "");
        }
        if (isset($_COOKIE["random_selector"])) {
            setcookie("random_selector", "");
        }
    }
}

sessionCheck.php

require_once "Auth.php";
require_once "Util.php";

$auth = new Auth();
$util = new Util();

// Get Current date, time
$current_time = time();
$current_date = date("Y-m-d H:i:s", $current_time);

// Set Cookie expiration for 1 month
$cookie_expiration_time = $current_time + (30 * 24 * 60 * 60);  // for 1 month

$isLoggedIn = false;

// Check if loggedin session and redirect if session exists
if(!empty($_SESSION["uid"])) {
    $isLoggedIn = true;
}
// Check if loggedin session exists
else if(!empty($_COOKIE["member_login"]) && !empty($_COOKIE["random_password"]) && !empty($_COOKIE["random_selector"])) {
    // Initiate auth token verification directive to false
    $isPasswordVerified = false;
    $isSelectorVerified = false;
    $isExpiryDateVerified = false;

    // Get token for username
    $userToken = $auth->getTokenByUsername($_COOKIE["member_login"],0);

    // Validate random password cookie with database
    if(password_verify($_COOKIE["random_password"], $userToken[0]["password_hash"])) {
        $isPasswordVerified = true;
    }

    // Validate random selector cookie with database
    if(password_verify($_COOKIE["random_selector"], $userToken[0]["selector_hash"])) {
        $isSelectorVerified = true;
    }

    // check cookie expiration by date
    if($userToken[0]["expiry_date"] >= $current_date) {
        $isExpiryDareVerified = true;
    }

    // Redirect if all cookie based validation retuens true
    // Else, mark the token as expired and clear cookies
    if(!empty($userToken[0]["id"]) && $isPasswordVerified && $isSelectorVerified && $isExpiryDareVerified) {
        $isLoggedIn = true;
    } else {
        if(!empty($userToken[0]["id"])) {
            $auth->markAsExpired($userToken[0]["id"]);
        }
        // clear cookies
        $util->clearAuthCookie();
    }
}

And finally login.php

require_once "../db.php";
require_once "../includes/functions.php";

require_once "Auth.php";
require_once "Util.php";

$auth = new Auth();
$Lhandle = new LoginHandler();
$util = new Util();

require_once "sessionCheck.php";

if ($isLoggedIn) {
    $util->redirect("dashboard.php");
}

if (isset($_POST["login"])) {
    $isAuthenticated = false;

    $kulladi = test_input($_POST["username"]);
    $sifre = test_input($_POST["password"]);
    $furl = test_input(base64_decode($_POST["refurl"]));

    // Validate the username.   
    if (empty($kulladi)) {
        $message[] = 'Please provide a username.';
    } elseif (!preg_match('/^[a-zA-Z0-9]*$/', $kulladi)) {
        $message[] = 'The username contains invalid characters.';
    } else {
        $username = test_input($kulladi);
    }

    // Validate the password.
    if (empty($sifre)) {
        $message[] = 'Empty password field.';
    } else {
        $password = test_input($sifre);
    }

    // Validate if referer url set.
    if (isset($_POST['refurl']) && !empty($_POST['refurl'])) {
        $refurl = $furl;
    } else {
        $refurl = 'dashboard.php';
    } 

    if(empty($message)){
        if($user = $auth->getMemberByUsername($username)){
            if(password_verify($password, $user[0]["password"])) {
            $isAuthenticated = true;
                $_SESSION["uid"] = $user[0]["uid"];
                $_SESSION["username"] = $user[0]["username"];

                // Set Auth Cookies if 'Remember Me' checked
                if (!empty($_POST["remember"])) {
                    setcookie("member_login", $username, $cookie_expiration_time);

                    $random_password = $util->getToken(16);
                    setcookie("random_password", $random_password, $cookie_expiration_time);

                    $random_selector = $util->getToken(32);
                    setcookie("random_selector", $random_selector, $cookie_expiration_time);

                    $random_password_hash = password_hash($random_password, PASSWORD_DEFAULT);
                    $random_selector_hash = password_hash($random_selector, PASSWORD_DEFAULT);

                    $expiry_date = date("Y-m-d H:i:s", $cookie_expiration_time);

                    // mark existing token as expired
                    $userToken = $auth->getTokenByUsername($username, 0);
                    if (!empty($userToken[0]["id"])) {
                        $auth->markAsExpired($userToken[0]["id"]);
                    }
                    // Insert new token
                    $auth->insertToken($username, $random_password_hash, $random_selector_hash, $expiry_date);
                } else {
                    $util->clearAuthCookie();
                }
                $util->redirect($refurl);
            }else{
                $message[] = "invalid user password";
            }
        }else{
            $message[] = "invalid user logins";
        }
    }
}

And html codes in login.php

<form action="" method="post" id="frmLogin">
    <div class="error-message">
	<?php if(isset($message)) {?>
	<?php echo implode('<br/>', $message); ?>
	</div>
	<?php } else{ ?>
			<div class="success-message">
				You have successfully created your account.
				<br/>Would you like to <a href="#">login</a> now?
			</div>
	<?php } ?>
    <div class="field-group">
        <div>
            <label for="login">Username</label>
        </div>
        <div>
            <input name="username" type="text"
                value="<?php if(isset($_COOKIE["member_login"])) { echo $_COOKIE["member_login"]; } ?>"
                class="input-field">
        </div>
    </div>
    <div class="field-group">
        <div>
            <label for="password">Password</label>
        </div>
        <div>
            <input name="password" type="password"
                value="<?php if(isset($_COOKIE["password"])) { echo $_COOKIE["password"]; } ?>"
                class="input-field">
        </div>
    </div>
    <div class="field-group">
        <div>
            <input type="checkbox" name="remember" id="remember" <?php if(isset($_COOKIE["member_login"])) { ?> checked
                <?php } ?> /> <label for="remember-me">Remember me</label>
        </div>
    </div>
    <div class="field-group">
        <div>
			<?php if(isset($_SERVER['HTTP_REFERER'])){ ?>
			<input type="hidden" name="refurl" value="<?php echo base64_encode($_SERVER['HTTP_REFERER']); ?>" />
			<?php } ?>
            <input type="submit" name="login" value="Login" class="form-submit-button"></span>
        </div>
    </div>
</form>

And This is the function used for filter inputs. Note: didnt filter all inputs yet, but will.

function test_input($data) {
    $data = filter_var(trim($data), FILTER_SANITIZE_STRING);
    $data = stripslashes($data);
    $data = htmlspecialchars($data);
    $data = strip_tags($data);
    return $data;
}
\$\endgroup\$
  • \$\begingroup\$ Welcome to code review, does the code work as expected? \$\endgroup\$ – pacmaninbw Dec 30 '19 at 14:06
  • \$\begingroup\$ FYI, date()'s default second parameter is time(). \$\endgroup\$ – mickmackusa Dec 31 '19 at 1:54
  • \$\begingroup\$ @mickmackusa to find date() and time() and compare with cookie expiry time userToken[0]["expiry_date"] >= $current_date \$\endgroup\$ – Dlk Dec 31 '19 at 14:14
  • \$\begingroup\$ Ah yes, I see now that you use time twice. Something else... isset($_POST['refurl']) is not needed if followed by !empty(). \$\endgroup\$ – mickmackusa Dec 31 '19 at 15:20
  • 2
    \$\begingroup\$ For one, you don't need nor should you be using that test_input() function with those functions in there. They don't do much to prevent a potential SQL injection. A prepared statement will take care of that. \$\endgroup\$ – Funk Forty Niner Jan 1 at 21:37

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.