I am currently trying to find out why my login statement works with using a username, but not email (or vice versa if switch in query)
The query only seems to accept the first value of the query, even if I've put it in brackets, it doesnt seem to register the second paramater. Running the query in phpMyAdmin works flawlessly, but breaks in the script
open to suggestions
$sql = "SELECT * FROM db_cms_users WHERE username = ? OR email = ? AND password = ?";
$stmt = $this->connect()->prepare($sql);
if(!$stmt->execute([$userID, $userID, $password])){
$stmt = null;
header("location: index.php?error=failstmt");
exit();
}
if($stmt->rowCount() == 0){
$stmt = null;
header("location: login.php?error=nouser");
exit();
}
I have tried
$sql = "SELECT * FROM db_cms_users WHERE (username = ? OR email = ?) AND password = ?";
rowCount returns true if I input a username, but false if I input a email. statements match SQL Database.
Dumped Variables
SQL: [76] SELECT * FROM db_cms_users WHERE username = ? OR email = ? AND password = ?
Sent SQL: [137] SELECT * FROM db_cms_users WHERE username = '[email protected]' OR email = '[email protected]' AND password = 'password'
Params: 3
- Key: Position #0: paramno=0 name=[0] "" is_param=1 param_type=2
- Key: Position #1: paramno=1 name=[0] "" is_param=1 param_type=2
- Key: Position #2: paramno=2 name=[0] "" is_param=1 param_type=2
Database output
Array
(
[0] => Array
(
[id] => 1
[0] => 1
[username] => test
[1] => test
[password] => $2y$10$QNKXEo3pnGPCjUMnfXlV..JJ4OFcSQJ5EVg75xOjlE7p5pL7Dqwau
[2] => $2y$10$QNKXEo3pnGPCjUMnfXlV..JJ4OFcSQJ5EVg75xOjlE7p5pL7Dqwau
[email] => test@email.com
[3] => test@email.com
[status] => 1
[4] => 1
[is_admin] => 1
[5] => 1
[registration] => 2021-11-13 12:21:28
[6] => 2021-11-13 12:21:28
)
)
File: user.class.php
protected function loginUser($userID, $password){
$sql = "SELECT password FROM db_cms_users WHERE username = ? OR email = ?";
$stmt = $this->connect()->prepare($sql);
if(!$stmt->execute([$userID, $userID])){
$stmt = null;
header("location: index.php?error=failstmt");
exit();
}
if($stmt->rowCount() == 0){
$stmt = null;
header("location: login.php?error=loginerror");
exit();
}
$hashedPwd = $stmt->fetchAll();
$checkPwd = password_verify($password, $hashedPwd[0]['password']);
if($checkPwd == false){
$stmt = null;
header("location: index.php?error=wrongpwd");
exit();
}elseif($checkPwd == true){
$sql = "SELECT * FROM db_cms_users WHERE username = ? OR email = ? AND password = ?";
$stmt = $this->connect()->prepare($sql);
if(!$stmt->execute([$userID, $userID, $password])){
#$stmt = null;
header("location: index.php?error=failstmt");
exit();
}
if($stmt->rowCount() == 0){
$stmt = null;
header("location: login.php?error=nouser");
exit();
}
$row = $stmt->fetchAll();
//make session later
//nov 13/21
session_start();
$_SESSION['username'] = $row[0]['username'];
$_SESSION['uid'] = $row[0]['id'];
return true;
}
}
File userContr.class.php
public function login($userID, $password){
$result = $this->loginUser($userID, $password);
return $result;
}
File test.php
<?php
ob_start();
session_start();
ini_set('display_errors', 1);
ini_set('display_startup_errors', 1);
error_reporting(E_ALL);
include "includes/autoloader.inc.php";
$userObj = new UserView();
$data = $userObj->showUser(1);
echo "<pre>";
print_r ($data[0]);
echo "</pre>";
$userObj = new UserContr();
if(isset($_SESSION['uid'])){
echo "<h1>Welcome back ". $_SESSION['username'] ."!";
echo "<a href='?a=logout'>Logout</a>";
if(isset($_GET['a'])){
$a = $_GET['a'];
if($a == "logout"){
$userObj->logoutUser();
exit();
}
}
}else{
if(isset($_POST['loginUser'])){
$userID = $_POST['userid'];
$password = $_POST['password'];
$result = $userObj->login($userID, $password);
if($result == true){
header("location: index.php");
exit();
}else{
echo "There was a login error";
exit();
}
}else{
echo "<h1>Login</h1>";
echo "<form method='post' action>
<input type='text' name='userid' placeholder='Username/Email'>
<input type='password' name='password' placeholder='password'><br>
<input type='submit' name='loginUser' value='Login'>
</form>";
}
}
ob_end_flush();
SQL: Dump file
-- Table structure for table `db_cms_users`
--
CREATE TABLE `db_cms_users` (
`id` int(11) NOT NULL,
`username` text NOT NULL,
`password` text NOT NULL,
`email` text NOT NULL,
`status` int(11) NOT NULL DEFAULT '0',
`is_admin` int(11) NOT NULL DEFAULT '0',
`registration` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;
--
-- Dumping data for table `db_cms_users`
--
INSERT INTO `db_cms_users` (`id`, `username`, `password`, `email`, `status`, `is_admin`, `registration`) VALUES
(1, 'test', '$2y$10$QNKXEo3pnGPCjUMnfXlV..JJ4OFcSQJ5EVg75xOjlE7p5pL7Dqwau', '[email protected]', 1, 1, '2021-11-13 20:21:28');
CodePudding user response:
Thanks for the big update. The context is very useful. It seems like what you're trying to do isn't logical or necessary.
To begin with, everything starts off well. In your loginUser()
function, you're correctly fetching the user details based on the email / username. Then you're verifying the password the right way using password_verify()
. That's all fine and sensible.
But the bit you seem to be having trouble with doesn't make a whole lot of sense. It looks like after you've verified the password, you then make another query to get the same user - except that this time you're trying to put the password into the WHERE
clause. This makes no sense because
You've already found the user with the first query and verified them, and
the raw password will never match the one in the database because the one in the database is hashed (as it should be, hence why you used
password_verify()
earlier in the code).
You really don't need that second SELECT query - what are you trying to achieve with it?
If you just change the first query to select more fields, then your problem is solved - you can put those details directly into the Session without needing to run another query:
protected function loginUser($userID, $password) {
$sql = "SELECT username, id, password FROM db_cms_users WHERE username = ? OR email = ?";
$stmt = $this->connect()->prepare($sql);
if(!$stmt->execute([$userID, $userID])) {
$stmt = null;
header("location: index.php?error=failstmt");
exit();
}
if($stmt->rowCount() == 0) {
$stmt = null;
header("location: login.php?error=loginerror");
exit();
}
$user = $stmt->fetchAll();
$checkPwd = password_verify($password, $user[0]['password']);
if($checkPwd == false) {
header("location: index.php?error=wrongpwd");
exit();
}
elseif($checkPwd == true) {
session_start();
$_SESSION['username'] = $user[0]['username'];
$_SESSION['uid'] = $user[0]['id'];
return true;
}
}
P.S. Security best practice recommends that when the credentials are not valid you don't let the user know whether it was the username or password (or both) which was the problem. For example if you disclose that the username was wrong, it indicates to a malicious party that they can discard that username and try another one, and equally if you disclose that only the password is incorrect you are indicating that they should keep attempting to crack the password for that username. You should simply state "invalid credentials" in either case, which then does not give any clues about how to narrow the search for a valid login.