After just a quick look through your class, I have identified a pretty big security issue you’ll want to address. If the login attempt is not successful, you are returning to the user the reason the attempt failed. In the AJAX response, the reason will be the second element of the result array, while if standard POST, you add the reason as a GET parameter on the redirect. This could indicate to a potential hacker if they’ve identified a valid username. Ideally, if the attempt is unsuccessful, the only thing the user will know is just that. You should not output anything beyond the success status for the attempt.
The first thing I noticed in your code was the lack of comments. Even though this is just a personal project right now, if you do plan to implement this at some point in the future, the comments will be a big help. It’s my opinion that for dynamic languages like PHP, you should at least provide a comment for every method of a class that details the purpose of the function, expected types of it’s arguments (when you can’t type-hint, e.g. scalar values) and the return data type.
Additionally, you may want to look in to how to implement the idea of Gateway classes:
http://martinfowler.com/eaaCatalog/tableDataGateway.html
A Gateway class abstracts the retrieval of data into and out of your persistence mechanism away from your application logic. A Gateway class would remove the need for your UserServices class to understand the database schema related to the Users and Attempted_Logins tables. An example UsersGateway class might look like this:
<?php
class UsersGateway {
private $pdo;
public function __construct(PDO $pdo) {
$this->pdo = $pdo;
}
public function selectUserByUsername($username) {
// Query and return details from the database related to the provided username
}
public function updateAuthKey($username) {
// Query to update the authkey value for the provided username
}
}
The dependencies within your UserServices class then update. Instead of needing a PDO object, your class now has dependencies of the UserGateway and AttemptedLoginsGateway.
The value of converting your code to using Gateways will not be apparent at first, but when you can re-use these Gateway classes (everywhere you need access to user information, for example) the benefit of implementing Gateways is easy to see. BONUS: if you move to using some other persistence mechanism (NoSQL or other), you can be assured that the only classes you’ll have to modify are named *Gateway.
Finally, this is also how relying on another service to provide your cookie functionality can be helpful (as mentioned by @s_molinari). The action of setting a cookie is a persistence activity (albeit on the client side), so can thus benefit from using a Gateway mechanism.
One more quick note: blocking access to your application by IP address is almost always ONLY going to inhibit valid users. The IP Address is very easy to spoof, and if you’re worried about a million hits per minute, you’d need to be worried about thousands of IP addresses. A more appropriate security mechanism would probably be locking out attempted usernames.
.
.