Login Auth not working right

[GhostElite_89]

I'm making a login/sign up page and the following pieces are not working together properly. When I set up the login page following a guide, it had me direct input the structure and I added a user (password is encrypted). When I log in with that password/username, it passes authentication.php perfectly. When I use my signup form (signup.php is simply called by a button on an HTML), it fails saying "Incorrect Password!". I'd say it's failing because of encryption but it passes with my old login that is encrypted so I'm thoroughly lost.

Authentication.php

<?php
session_start();
// Change this to your connection info.
$DATABASE_HOST = 'localhost';
$DATABASE_USER = 'root';
$DATABASE_PASS = 'test';
$DATABASE_NAME = 'login';
// Try and connect using the info above.
$con = mysqli_connect($DATABASE_HOST, $DATABASE_USER, $DATABASE_PASS, $DATABASE_NAME);
if ( mysqli_connect_errno() ) {
	// If there is an error with the connection, stop the script and display the error.
	die ('Failed to connect to MySQL: ' . mysqli_connect_error());
}
// Now we check if the data from the login form was submitted, isset() will check if the data exists.
if ( !isset($_POST['username'], $_POST['password']) ) {
	// Could not get the data that should have been sent.
	die ('Please fill both the username and password field!');
}
// Prepare our SQL, preparing the SQL statement will prevent SQL injection.
if ($stmt = $con->prepare('SELECT id, password FROM accounts WHERE username = ?')) {
	// Bind parameters (s = string, i = int, b = blob, etc), in our case the username is a string so we use "s"
	$stmt->bind_param('s', $_POST['username']);
	$stmt->execute();
	// Store the result so we can check if the account exists in the database.
	$stmt->store_result();
if ($stmt->num_rows > 0) {
	$stmt->bind_result($id, $password);
	$stmt->fetch();
	// Account exists, now we verify the password.
	// Note: remember to use password_hash in your registration file to store the hashed passwords.
	if (password_verify ($_POST['password'], $password)) {
		// Verification success! User has loggedin!
		// Create sessions so we know the user is logged in, they basically act like cookies but remember the data on the server.
		session_regenerate_id();
		$_SESSION['loggedin'] = TRUE;
		$_SESSION['name'] = $_POST['username'];
		$_SESSION['id'] = $id;
		header('Location: dashboard.php');
	} else {
		echo 'Incorrect password!';
	}
} else {
	echo 'Incorrect username!';
}
	$stmt->close();
}
?>

 

Signup.php

<?php
 
// get database connection
include_once '../config/database.php';
 
// instantiate user object
include_once '../objects/user.php';
 
$database = new Database();
$db = $database->getConnection();
 
$user = new User($db);
 
// set user property values
$user->username = $_POST['uname'];
$user->password = base64_encode($_POST['password']);
$user->created = date('Y-m-d H:i:s');
 
// create the user
if($user->signup()){
    $user_arr=array(
        "status" => true,
        "message" => "Successfully Signup!",
        "id" => $user->id,
        "username" => $user->username
    );
}
else{
    $user_arr=array(
        "status" => false,
        "message" => "Username already exists!"
    );
}
print_r(json_encode($user_arr));
?>

 

login.php

<?php
// include database and object files
include_once '../config/database.php';
include_once '../objects/user.php';
 
// get database connection
$database = new Database();
$db = $database->getConnection();
 
// prepare user object
$user = new User($db);
// set ID property of user to be edited
$user->username = isset($_GET['username']) ? $_GET['username'] : die();
$user->password = base64_encode(isset($_GET['password']) ? $_GET['password'] : die());
// read the details of user to be edited
$stmt = $user->login();
if($stmt->rowCount() > 0){
    // get retrieved row
    $row = $stmt->fetch(PDO::FETCH_ASSOC);
    // create array
    $user_arr=array(
        "status" => true,
        "message" => "Successfully Login!",
        "id" => $row['id'],
        "username" => $row['username']
    );
}
else{
    $user_arr=array(
        "status" => false,
        "message" => "Invalid Username or Password!",
    );
}
// make it json format
// print_r(json_encode($user_arr));
if (in_array("Successfully Login!", $user_arr)) { 

	header('Location: ../../dashboard.html');
}
?>

 

[requinix]

$user->password = base64_encode($_POST['password']);

$user->password = base64_encode(isset($_GET['password']) ? $_GET['password'] : die());

No. No base64_encode. Don't do that.

$_GET['password']

That is incredibly bad. Do not put the password in the URL. That's... I don't know why this code even exists.

Make the problem go away by changing the form to use method=post. I really, really shouldn't have to say that.

[GhostElite_89]

7 minutes ago, requinix said:

$user->password = base64_encode($_POST['password']);

$user->password = base64_encode(isset($_GET['password']) ? $_GET['password'] : die());

No. No base64_encode. Don't do that.

$_GET['password']

That is incredibly bad. Do not put the password in the URL. That's... I don't know why this code even exists.

Make the problem go away by changing the form to use method=post. I really, really shouldn't have to say that.

Sorry, learner here so I'm a newbie to this all. Thus, why I'm following guides to figure things out and also reading tutorials. Any pointers on how to improve my code and make it more secure?

[GhostElite_89]

So I've switched my code to be more secure and more complex at the same time. I keep receiving a Database Query Error message which means it's failing at the query part which has me confused cause the syntax is on point I feel like... Here's what I have...

(only document that has all classes and functions:

<?php
class Account
{
	/* Class properties (variables) */
	
	/* The ID of the logged in account (or NULL if there is no logged in account) */
	private $id;
	
	/* The name of the logged in account (or NULL if there is no logged in account) */
	private $name;
	
	/* TRUE if the user is authenticated, FALSE otherwise */
	private $authenticated;
	
	
	/* Public class methods (functions) */
	
	/* Constructor */
	public function __construct()
	{
		/* Initialize the $id and $name variables to NULL */
		$this->id = NULL;
		$this->name = NULL;
		$this->authenticated = FALSE;
	}
	
	/* Destructor */
	public function __destruct()
	{
		
	}
	
	/* "Getter" function for the $id variable */
	public function getId(): ?int
	{
		return $this->id;
	}
	
	/* "Getter" function for the $name variable */
	public function getName(): ?string
	{
		return $this->name;
	}
	
	/* "Getter" function for the $authenticated variable */
	public function isAuthenticated(): bool
	{
		return $this->authenticated;
	}
	
	/* Add a new account to the system and return its ID (the account_id column of the accounts table) */
	public function addAccount(string $name, string $passwd): int
	{
		/* Global $pdo object */
		global $pdo;
		
		/* Trim the strings to remove extra spaces */
		$name = trim($name);
		$passwd = trim($passwd);
		
		/* Check if the user name is valid. If not, throw an exception */
		if (!$this->isNameValid($name))
		{
			throw new Exception('Invalid user name');
		}
		
		/* Check if the password is valid. If not, throw an exception */
		if (!$this->isPasswdValid($passwd))
		{
			throw new Exception('Invalid password');
		}
		
		/* Check if an account having the same name already exists. If it does, throw an exception */
		if (!is_null($this->getIdFromName($name)))
		{
			throw new Exception('User name not available');
		}
		
		/* Finally, add the new account */
		
		/* Insert query template */
		$query = 'INSERT INTO login.accounts (account_name, account_passwd) VALUES (:name, :passwd)';
		
		/* Password hash */
		$hash = password_hash($passwd, PASSWORD_DEFAULT);
		
		/* Values array for PDO */
		$values = array(':name' => $name, ':passwd' => $hash);
		
		/* Execute the query */
		try
		{
			$res = $pdo->prepare($query);
			$res->execute($values);
		}
		catch (PDOException $e)
		{
		   /* If there is a PDO exception, throw a standard exception */
		   throw new Exception('Database query error');
		}
		
		/* Return the new ID */
		return $pdo->lastInsertId();
	}

signup (invoked when button is pressed)

<?php
session_start();

/* Include the database connection file (remember to change the connection parameters) */
require './db_inc.php';

/* Include the Account class file */
require './account_class.php';

/* Create a new Account object */
$account = new Account();
try
{
	$newId = $account->addAccount('test', 'test');
}
catch (Exception $e)
{
	echo $e->getMessage();
	die();
}

echo 'The new account ID is ' . $newId;
?>

 

[requinix]

Unfortunately I don't really have tips or tricks to pass along. I can mention phptherightway.com as a good resource, but it isn't totally comprehensive.
For now, make sure you avoid articles and tutorials from some random person's blog and instead try to hit up the more reputable sites that perhaps you've already heard of before.

		catch (PDOException $e)
		{
		   /* If there is a PDO exception, throw a standard exception */
		   throw new Exception('Database query error');
		}

Don't do that either. You're taking a perfectly good exception and then discarding everything it has to offer and replacing it with a mere "Database query error".

Remove the whole try/catch and see what happens.

[GhostElite_89]

I took out the exception and am getting this:

Parse error: syntax error, unexpected 'public' (T_PUBLIC)

for the login function

Edited February 18, 2020 by GhostElite_89

[requinix]

Then you changed something else in that class, because it wasn't doing this before.

Fix the syntax error and try again.

[GhostElite_89]

I got it working, the class cant be called Public. Now I'm having issues with the following code not executing:

<?php
/* Include the database connection file (remember to change the connection parameters) */
require './db_inc.php';

/* Include the Account class file */
require './account_class.php';
/* Create a new Account object */
$account = new Account();

function login() {
$login = FALSE;
$username = $_POST["username"];
$password = $_POST["password"];

try
{
	
	$login = $account->login($username, $password);
	echo "Success";
}
catch (Exception $e)
{
	echo $e->getMessage();
	die();
}

if ($login)
{
	echo 'Authentication successful.';
	echo 'Account ID: ' . $account->getId() . '<br>';
	echo 'Account name: ' . $account->getName() . '<br>';
}
else
{
	echo 'Authentication failed.';
}
}
?>

here's where it gets passed:

 function login(string $name, string $passwd): bool
	{
		/* Global $pdo object */
		global $pdo;
		
		/* Trim the strings to remove extra spaces */
		$name = trim($name);
		$passwd = trim($passwd);
		
		/* Check if the user name is valid. If not, return FALSE meaning the authentication failed */
		if (!$this->isNameValid($name))
		{
			return FALSE;
			
		}
		
		/* Check if the password is valid. If not, return FALSE meaning the authentication failed */
		if (!$this->isPasswdValid($passwd))
		{
			return FALSE;
		}
		
		/* Look for the account in the db. Note: the account must be enabled (account_enabled = 1) */
		$query = 'SELECT * FROM login.accounts WHERE (account_name = :name) AND (account_enabled = 1)';
		
		/* Values array for PDO */
		$values = array(':name' => $name);
		
		/* Execute the query */
		
		
			$res = $pdo->prepare($query);
			$res->execute($values);
	
		$row = $res->fetch(PDO::FETCH_ASSOC);
		
		/* If there is a result, we must check if the password matches using password_verify() */
		if (is_array($row))
		{
			if (password_verify($passwd, $row['account_passwd']))
			{
				/* Authentication succeeded. Set the class properties (id and name) */
				$this->id = intval($row['account_id'], 10);
				$this->name = $name;
				$this->authenticated = TRUE;
				
				/* Register the current Sessions on the database */
				$this->registerLoginSession();
				
				/* Finally, Return TRUE */
				return TRUE;
			}
		}
		
		/* If we are here, it means the authentication failed: return FALSE */
		return FALSE;
	}
	
	/* A sanitization check for the account username */
	function isNameValid(string $name): bool
	{
		/* Initialize the return variable */
		$valid = TRUE;
		
		/* Example check: the length must be between 8 and 16 chars */
		$len = mb_strlen($name);
		
		if (($len < 8) || ($len > 16))
		{
			$valid = FALSE;
		}
		
		/* You can add more checks here */
		
		return $valid;
	}
	
	/* A sanitization check for the account password */
	function isPasswdValid(string $passwd): bool
	{
		/* Initialize the return variable */
		$valid = TRUE;
		
		/* Example check: the length must be between 8 and 16 chars */
		$len = mb_strlen($passwd);
		
		if (($len < 8) || ($len > 16))
		{
			$valid = FALSE;
		}
		
		/* You can add more checks here */
		
		return $valid;
	}
	
	/* A sanitization check for the account ID */
	function isIdValid(int $id): bool
	{
		/* Initialize the return variable */
		$valid = TRUE;
		
		/* Example check: the ID must be between 1 and 1000000 */
		
		if (($id < 1) || ($id > 1000000))
		{
			$valid = FALSE;
		}
		
		/* You can add more checks here */
		
		return $valid;
	}
	
	/* Login using Sessions */
	function sessionLogin(): bool
	{
		/* Global $pdo object */
		global $pdo;
		
		/* Check that the Session has been started */
		if (session_status() == PHP_SESSION_ACTIVE)
		{
			/* 
				Query template to look for the current session ID on the account_sessions table.
				The query also make sure the Session is not older than 7 days
			*/
			$query = 
			
			'SELECT * FROM login.account_sessions, login.accounts WHERE (account_sessions.session_id = :sid) ' . 
			'AND (account_sessions.login_time >= (NOW() - INTERVAL 7 DAY)) AND (account_sessions.account_id = accounts.account_id) ' . 
			'AND (accounts.account_enabled = 1)';
			
			/* Values array for PDO */
			$values = array(':sid' => session_id());
			
			/* Execute the query */
			
			
				$res = $pdo->prepare($query);
				$res->execute($values);
			

			$row = $res->fetch(PDO::FETCH_ASSOC);
			
			if (is_array($row))
			{
				/* Authentication succeeded. Set the class properties (id and name) and return TRUE*/
				$this->id = intval($row['account_id'], 10);
				$this->name = $row['account_name'];
				$this->authenticated = TRUE;
				
				return TRUE;
			}
		}
		
		/* If we are here, the authentication failed */
		return FALSE;
	}
	
	/* Logout the current user */
	function logout()
	{
		/* Global $pdo object */
		global $pdo;
		
		/* If there is no logged in user, do nothing */
		if (is_null($this->id))
		{
			return;
		}
		
		/* Reset the account-related properties */
		$this->id = NULL;
		$this->name = NULL;
		$this->authenticated = FALSE;
		
		/* If there is an open Session, remove it from the account_sessions table */
		if (session_status() == PHP_SESSION_ACTIVE)
		{
			/* Delete query */
			$query = 'DELETE FROM login.account_sessions WHERE (session_id = :sid)';
			
			/* Values array for PDO */
			$values = array(':sid' => session_id());
			
			/* Execute the query */

				$res = $pdo->prepare($query);
				$res->execute($values);

		}
	}
	
	/* Close all account Sessions except for the current one (aka: "logout from other devices") */
	function closeOtherSessions()
	{
		/* Global $pdo object */
		global $pdo;
		
		/* If there is no logged in user, do nothing */
		if (is_null($this->id))
		{
			return;
		}
		
		/* Check that a Session has been started */
		if (session_status() == PHP_SESSION_ACTIVE)
		{
			/* Delete all account Sessions with session_id different from the current one */
			$query = 'DELETE FROM login.account_sessions WHERE (session_id != :sid) AND (account_id = :account_id)';
			
			/* Values array for PDO */
			$values = array(':sid' => session_id(), ':account_id' => $this->id);
			
			/* Execute the query */

				$res = $pdo->prepare($query);
				$res->execute($values);

		}
	}
	
	/* Returns the account id having $name as name, or NULL if it's not found */
	function getIdFromName(string $name): ?int
	{
		/* Global $pdo object */
		global $pdo;
		
		/* Since this method is public, we check $name again here */
		if (!$this->isNameValid($name))
		{
			throw new Exception('Invalid user name');
		}
		
		/* Initialize the return value. If no account is found, return NULL */
		$id = NULL;
		
		/* Search the ID on the database */
		$query = 'SELECT account_id FROM login.accounts WHERE (account_name = :name)';
		$values = array(':name' => $name);

			$res = $pdo->prepare($query);
			$res->execute($values);

		
		$row = $res->fetch(PDO::FETCH_ASSOC);
		
		/* There is a result: get it's ID */
		if (is_array($row))
		{
			$id = intval($row['account_id'], 10);
		}
		
		return $id;
	}
	
	
	/* Private class methods */
	
	/* Saves the current Session ID with the account ID */
	function registerLoginSession()
	{
		/* Global $pdo object */
		global $pdo;
		
		/* Check that a Session has been started */
		if (session_status() == PHP_SESSION_ACTIVE)
		{
			/* 	Use a REPLACE statement to:
				- insert a new row with the session id, if it doesn't exist, or...
				- update the row having the session id, if it does exist.
			*/
			$query = 'REPLACE INTO login.account_sessions (session_id, account_id, login_time) VALUES (:sid, :accountId, NOW())';
			$values = array(':sid' => session_id(), ':accountId' => $this->id);
			
			/* Execute the query */

				$res = $pdo->prepare($query);
				$res->execute($values);

		}
	}

It's simply just refreshing the screen.

[requinix]

I see you have a function there (in the first file) called "login". I also see you aren't calling it there. I also can't see where that file gets run from.