Login system, PDO and me - Part 2

Question

This is a follow up question to: Login system, PDO and me

Is this considered secure for a login script?

if (filter_input(INPUT_POST, "login", FILTER_UNSAFE_RAW)) {
    $usercookie = 'Admin-'.uniqid(mt_rand(), true);

    $lastlogin = date('d / m / y - H:ia');
    $session = uniqid(mt_rand(), true);
    $active = 1;

    $username = filter_input(INPUT_POST, "username", FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW);
    $password = filter_input(INPUT_POST, "password", FILTER_UNSAFE_RAW);

    $query = 'SELECT password FROM wcx_admin WHERE username = :username';

    $stmt = $backend->queryIt($query);
    $stmt = $backend->bind(':username', $username);
    $stmt = $backend->execute();
    $dbpass = $backend->getColumn();

    if(password_verify($password, $dbpass)) {
        $query = 'UPDATE wcx_admin SET active = :active, lastlogin = :lastlogin, session = :session WHERE username = :username';
        $stmt = $backend->queryIt($query);
        $stmt = $backend->bind(':active', $active);
        $stmt = $backend->bind(':lastlogin', $lastlogin);
        $stmt = $backend->bind(':session', $session);
        $stmt = $backend->bind(':username', $username);
        $login = $backend->execute();
    }

    if($login) {
        $_SESSION['loggedin'] = $session;
        $_SESSION['auth'] = $session;
        setcookie( 'wcxadmin', $usercookie, time()+3600);
        header('Location: index.php');
    }
    else {
        $_SESSION['errors'] = 'Incorrect Username or Password';
        header('Location: index.php');
    }
}  

The bind function:

public function bind($param, $value, $type = null){
if (is_null($type)) {
    switch (true) {
        case is_int($value):
            $type = PDO::PARAM_INT;
            break;
        case is_bool($value):
            $type = PDO::PARAM_BOOL;
            break;
        case is_null($value):
            $type = PDO::PARAM_NULL;
            break;
        default:
            $type = PDO::PARAM_STR;
    }
}
$this->stmt->bindValue($param, $value, $type);
}

Error output:

if(isset($_SESSION['errors'])) {
    echo '<div class="loginerror">'.$_SESSION['errors'].'</div>';
    $_SESSION['errors'] = '';
}

Answer 1

You're probably sick of having me answer your questions, but I couldn't help myself on this one!

Basically, yes this is safe. However, you never specified what you wanted to protect against, so I'm assuming you mean from just a normal person who might try a couple tricks on you!

But nothing is perfect, and I'll see what I can do to point that out...

• First off, I don't understand your first line.

  filter_input(INPUT_POST, "login", FILTER_UNSAFE_RAW)
  

  Are you checking to see if a "login" was sent? Your filter (FILTER_UNSAFE_RAW) doesn't do anything to the string if you don't specify other filters:

  Do nothing, optionally strip or encode special characters.

  So I'm assuming it's just a check to see if the correct POST is there? If that's the case, what's wrong with using $_POST['login']?

• I think your $usercookie is considered to be a security aspect, no? Well according to the docs:

  This function does not create random nor unpredictable strings. This function must not be used for security purposes.

  And from the mt_rand() page...

  This function does not generate cryptographically secure values, and should not be used for cryptographic purposes.

  And then a little bit further...

  If you need a cryptographically secure value, consider using openssl_random_pseudo_bytes() instead.

  I think we all know what needs to be done here. (Get rid of of two functions you're using!)

• Why are you not handling timestamps in $lastlogin instead of the database? The database has functionality meant for this. I believe you're using MySQL.
• $session has the same security issue $usercookie had.
• I would put all of these variables inside the first if.
• For $username, I hope when someone registers, you alert them of the invalid characters that might be in their name. If you automatically strip them, they'll be very confused when the go to log in!
• FILTER_UNSAFE_RAW does nothing to the $password. What's it point again?

• I'm going to make an adjustment here:

      $login = $backend->execute();
  }
  
  if($login) {
      $_SESSION['loggedin'] = $session;
      $_SESSION['auth'] = $session;
      setcookie( 'wcxadmin', $usercookie, time()+3600);
      header('Location: index.php');
  }
  else {
      $_SESSION['errors'] = 'Incorrect Username or Password';
      header('Location: index.php');
  }
  

  And change all this to...

      if($backend->execute()) {
          // Sessions, cookie, and headers
      }
  }
  $_SESSION['errors'] = 'Incorrect Username or Password';
  header('Location: index.php');
  

  I think that makes the code more concise.

• Will $_SESSION['loggedin'] ever be different than $_SESSION['auth']? You set them equal to each other, which I think could be avoided.
• Your bind function looks fine, clean too.
• Error checking seems fine too!

Good improvement though :)