PHP Login class

I wrote this PHP Login system and I would like to see if I have any errors/made any mistakes or if you think I can do something to improve it, like including tokenizing and improving security even more.

This script allows "Remember Me" option which uses a cookie and a session table to authorize a user. It has basic functions such as add/delete/modify user etc. There is an index page where a person can log in or register a new account. Login class which handles all the user interaction and securepage which user accesses when there is a successful log in.

index.php

```

startSession();
$login->connectToDB();
$session_id = session_id();

// If the user has a cookie set, redirect him to secure page
if($login->isAuthorized()) {
header("Location: securePage.php");
}

if($_POST['login']){

// get the data, trim the blank spacesß
$username = trim($_POST['username']);
$password = trim($_POST['password']);

//if checked, the value will be 'on'
//otherwise, it will be blank
$rememberme = $_POST['rememberme'];

// verify if the username and password are correct
// and if rememberme is set to 'on', create a cookie

if($username && $password){

// Check the login details and redirect to securePage.php
// if the password is not correct, notify the user
$login->checkLogin($username, $password, $rememberme, $session_id);

} else {

echo("Please enter a username and password");

}
}
if($_POST['create']){

// create an account
// and notify the user the account has been created
$username = trim($_POST['username']);
$password = trim($_POST['password']);
$first_name = trim($_POST['first_name']);
$last_name = trim($_POST['last_name']);
$email = trim($_POST['email']);
$login->addUser($username, $password, $first_name, $last_name, $email);

}

?>

#table {
width: 340px;
height: 450px;
margin: 0 auto;
border: 3px solid;
padding: 20px;
}

Existing Users
Us

Initial thoughts:

Lots of redundant comments, for example:

// get the data, trim the blank spaces
$username = trim($_POST['username']);

Some obvious refactoring not implemented, for example:

if($_POST['create']){
    // create an account
    // and notify the user the account has been created
    $username = trim($_POST['username']);
    $password = trim($_POST['password']);

• If you're always going to trim the data, just trim it, and never worry about it again.

• If a chunk of code warrants a comment, it likely warrants a well-named method instead.

Is your HTML really not indented? Ew :(

function addUser

Missed refactoring, or at least remove redundant comments.

function deleteUser

You're not checking if username already exists, you're just trying to delete it. All of this function's comments can go in a function-level comment.

function checkLogin

Extraneous comments. Have to go through ~20 lines of code to see what happens if no $result; probably cleaner to flip conditional and handle the shorter case first.

function setRememberMe

Extraneous comments; all obvious from code.

function isAuthorized()

Too long, too much scanning to determine functionality. Consider something like this:

// Checks user login via information in autologin cookie.
public function isAuthorized() {
  if (!isset($_COOKIE['autologin'])) {
    return false;
  } 

    $session_id = $_COOKIE['autologin'];
    $query = "SELECT * FROM sessions WHERE session_id = '" . $session_id . "'";                                         
    $result = mysql_query($query) OR die('Cannot perform query!');  
  $user_by_session = mysql_fetch_assoc($result); 

    $user_ip = $_SERVER['REMOTE_ADDR'];
    $user_agent =  $_SERVER['HTTP_USER_AGENT'];
    if (($user_by_session["user_ip"] != $user_ip) || ($user_by_session["user_agent"] != $user_agent)) {
      return false;
  }

    $query = "SELECT * FROM users WHERE username = '" . $user_by_session["user_id"] . "' LIMIT 0,5";    
    $user_entries = mysql_query($query) OR die("Cannot perform query!");
    while ($row = mysql_fetch_assoc($user_entries)) {
        $this->username   = $row['username'];
        $this->first_name = $row['first_name'];
        $this->last_name  = $row['last_name'];
        $this->password   = $row['password'];
        $this->email      = $row['email'];
        $this->session_id = $session_id;
    } 

    $_SESSION['usrData'] = $this;
    return true;
}

"Getter" functions: redundant comments.

StackExchange Code Review Q#6076, answer score: 3