I'm hoping someone would be able to identify if my code is prone to SQL injections, and just overall see if there is anything that could be done better, faster or more efficiently
Im very new to OOP and prepared statements so there might be some glaringly obvious problems but just let me know!
auth_class.php
require 'db.php';
session_start(); ///needs an open session to set the logged in value
class User {
public $username = null;
public $password = null;
public $name = null;
public $email = null;
public $ftp = null;
public $connection = null;
function __construct(){
$this->connection = connect_db(); ///this is the basic database connection
}
public function passwordValidator($password){
if (strlen($password) < 5) {
$errors[] = "Password too short!";
}
if (!preg_match("#[0-9]+#", $password)) {
$errors[] = "Password must include at least one number!";
}
if (!preg_match("#[a-zA-Z]+#", $password)) {
$errors[] = "Password must include at least one letter!";
}
if(empty($errors)){
$this->password = password_hash($password, PASSWORD_DEFAULT);
return;
}else{
return $errors;//this function checks the password and returns an array of errors it is checked on the other end
}
}
public function storeFormValues($username, $name, $email) {
//This needs to check the values below to confirm that there is somthing there! Although they are set as required inputs
if(empty($username)){
$errors[] = "please Enter a username!";
}else{
if (!preg_match("/^[a-zA-Z ]*$/",$username)) {
$errors[] = "Only letters and white space allowed";
}else{
$this->username = $username;
}
}
if(empty($name)){
$errors[] = "please Enter a name!";
}else{
if (!preg_match("/^[a-zA-Z ]*$/",$name)) {
$errors[] = "Only letters and white space allowed";
}else{
$this->name = $name;
}
}
if(empty($email)){
$errors[] = "please Enter a email!";
}else{
if (!filter_var($email, FILTER_VALIDATE_EMAIL)) {
$errors[] = "Invalid email format";
}else{
$this->email = $email;
$this->ftp = $email;
}
}
if(empty($errors)){
return; //no errors all stored fine
}else{
return $errors;//this function checks the password and returns an array of errors it is checked on the other end
}
}
public function logout() {
session_destroy();
header("Location: ../login.php"); ///destroys all session data and redirects to login
}
public function userLogin($email, $password) {
try{
$sql = "SELECT * FROM accounts where account_email = ?";
$stmt = $this->connection->prepare($sql);
$stmt->execute(array($email));
if ($stmt->rowCount() > 0){
$output = $stmt->fetch();
$hash = $output['account_password'];
if (password_verify($password, $hash)) {
header_remove();
header('Location: ./home.php');
$_SESSION['loggedIn'] = 1;
$_SESSION['username'] = $output['account_username'];
$_SESSION['id'] = $output['account_id'];
$_SESSION['name'] = $output['account_name'];
$_SESSION['email'] = $output['account_email'];
return; //user logged in
} else {
$_SESSION['loggedIn'] = 0;
$error[] = "Wrong Password!";
return $error; //user password didnt match
}
}else{
$error[] = "User Not Found!"; ///no user found
return $error;
}
}catch (PDOException $e){
//$e->getMessage();
$error[] = "There is an error Contact [email protected]";
return $error;
}
}
public function userRegister() {
//below is the basic code to add a user to the database
try{
$sql = "INSERT INTO accounts (account_name, account_password, account_username, account_email, ftp_user) VALUES (?,?,?,?,?)";
$stmt= $this->connection->prepare($sql);
$stmt->execute([$this->name, $this->password, $this->username, $this->email, $this->ftp]);
return;
} catch (PDOException $e) {
//$error[] = "DataBase Error: The could not be added.<br>".$e->getMessage();
$error[] = "We already have an account with that username!";
return $error;
}
}
public function changeUserData() {
//below is the basic code to add a user to the database
try{
$sql = "UPDATE accounts SET account_name=?, account_password=?, account_username=?, account_email =? ";
$stmt= $this->connection->prepare($sql);
$stmt->execute([$this->name, $this->password, $this->username, $this->email]);
return;
} catch (PDOException $e) {
$error[] = "DataBase Error: ERROR.<br>".$e->getMessage();
//$error[] = "We already have an account with that username!";
return $error;
}
}
}
db.php
function connect_db()
{
try
{
$servername = "localhost";
$username = "root";
$password = "";
try {
$conn = new PDO("mysql:host=$servername;dbname=main_database", $username, $password);
// set the PDO error mode to exception
$conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
//echo "Connected successfully";
}
catch(PDOException $e)
{
echo "Connection failed: " . $e->getMessage();
}
}
catch (PDOException $e)
{
// Proccess error
echo 'Cannot connect to database: ' . $e->getMessage();
}
return $conn;
}
And this is the section at the top of my login page.
include("./php/classes/auth_class.php");
if(isset($_SESSION['loggedIn']) && $_SESSION['loggedIn'] === 1) {
header('Location: ./home.php');
}
$notification = NULL;
if(isset($_POST['submitlogin'])){
$instance = new User();
$loginValidate = $instance->userlogin($_POST['email'], $_POST['password']);
if(empty($loginValidate)){
//Login successfull
}else{
//print_r($loginValidate);
//login unsucessfull
$notification = $loginValidate;
}
}
if(isset($_POST['submitregister'])){
$instance = new User();
$passwordValidate = $instance->passwordValidator($_POST['upass']);
if(empty($passwordValidate)){
$formValueValidator = $instance->storeFormValues($_POST['uname'],$_POST['fullname'], $_POST['uemail']);
if(empty($formValueValidator)){
$registerValidate = $instance->userRegister();
if(empty($registerValidate)){
///user registed fine
$notification = "Registration complete please login!";
}else{
//print_r($registerValidate); ///prints errors if found with register
$notification = $registerValidate;
}
}else{
///unable to store form values
//print_r($formValueValidator);
$notification = $formValueValidator;
}
}else{
//print_r($passwordValidate); //unable to validate password
$notification = $passwordValidate;
}
}
Thanks in advance! Cheers, Greg
User
class you havepasswordValidator()
andstoreFormValues()
- one is like a noun / name, the other like a verb / action. Instead,validatePassword()
would make sense withstoreFormValues()
. \$\endgroup\$