Update with pdo

PHP
1

I have

		$sql = "SELECT email, role FROM users WHERE email = :user AND pass = :pass AND operational_status = 1";
		//echo $sql;
		$result = $pdo->prepare($sql);
		$result -> execute($loginData);
		
		if($result->rowCount() > 0){

			while($row = $result->fetch()){    

			 $_SESSION['email'] = $row['email'];
			 $_SESSION['user_role'] = $row['role'];
			 $_SESSION['id'] = session_id();
			 
			 $sql = 'UPDATE users SET session_id = '.$_SESSION['id'].' WHERE email = '.$_SESSION['email'];
			 $pdo -> query($sql);
			 }
		} else {
			$message = "<div class='alert alert-danger alert-dismissible fade show' role='alert'><strong>Credentials incorrect!</strong>, please try again.";
			$message .= '<button type="button" class="close" data-dismiss="alert" aria-label="Close"><span aria-hidden="true">&times;</span></button>';
			$message .= "</div>.";
		}

and I get
Fatal error : Uncaught PDOException: SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near ‘@gmail.com’ at line 1 in C:\xampp\htdocs\SST\index.php:34 Stack trace: #0 C:\xampp\htdocs\SST\index.php(34): PDO->query(‘UPDATE users SE…’) #1 {main} thrown in C:\xampp\htdocs\SST\index.php on line 34

Did I not set up the update query right?

2

Neither of these is right

This is a good start but doesn’t actually pass the parameters to the query

$sql = SELECT email, role FROM users WHERE email = :user AND pass = :pass AND operational_status = 1;|

/echo $sql;|
$result = $pdo->prepare($sql);|
$result -> execute($loginData);|

This is very, very bad. You should never string build a SQL statement like this.

$sql = 'UPDATE users SET session_id = '.$_SESSION['id'].' WHERE email = '.$_SESSION['email'];
$pdo -> query($sql);

It should be using exec, not query. And preparing the statement to help mitigate SQL injection.

3

To be fair, when it was a simple string, query was valid :stuck_out_tongue: It’s just that it shouldnt be a simple string.

The strict answer to the problem the OP’s original query has is that his strings arent encapsulated in quotation marks.

The proper answer is to execute a parameterized query, rather than string building.

4

In the 1st one, how do I get at the parameters if not by including the array in execute(), should I do

$user = $_POST['Email'];
$pass = $_POST['Password'];

$result = $pdo->prepare(sql);
//add the parameters
$result->execute(['user' => $user , 'pass' => $pass]); 
..
5

While we are at it (and this does not relate to the problem).
This smells of poor password management. Either an unhashed password or a password hashed with an outdated method.

I’ll also question the logic here:-

It probably ‘works’ but that query should only ever fetch a single result, so there should be no while or count.
You simply fetch() and there either is, or is not a result.

if($row = $result->fetch()){
    // update session
}

And no need to fetch the email, since you already have that information.

But anyway, the update should be more like:-

$update = $pdo->prepare( "UPDATE users SET session_id = ? WHERE email = ?");
$update->execute([$_SESSION['id'], $_SESSION['email']]);

Assuming there is no better unique key than email to identify the user, such as an AI ID.

3 Likes
6

ok then

$sql = "SELECT role FROM users WHERE email = ? AND pass = ? AND operational_status = 1";
//echo $sql;
$user = $_POST['Email'];
$pass = $_POST['Password'];

$result = $pdo->prepare(sql);
//add the parameters
$result->execute($user, $pass); 
		
if($row = $result->fetch()){    
   $_SESSION['email']  = $user;
   $_SESSION['user_role'] = $row['role'];
   $_SESSION['id'] = session_id();
			 
   $update = $pdo->prepare( "UPDATE users SET session_id = ? WHERE email = ?");
   $update->execute([$_SESSION['id'], $_SESSION['email']]);
} else {
   $message = "<div class='alert alert-danger alert-dismissible fade show' role='alert'><strong>Credentials incorrect!</strong>, please try again.";
   $message .= '<button type="button" class="close" data-dismiss="alert" aria-label="Close"><span aria-hidden="true">&times;</span></button>';
   $message .= "</div>.";
}

The positional placeholders is good since I only have 2, ok?
I put the UNIQUE keyword in the users table to prevent duplicates so wouldn’t it act as a identifier?

CREATE TABLE users (
   user_id TINYINT UNSIGNED NOT NULL AUTO_INCREMENT,
   name VARCHAR(50) NOT NULL,
   email VARCHAR(50) NOT NULL,   
   pass VARCHAR(50) NOT NULL,
   session_id VARCHAR(20),
   role ENUM ('Administrator','Surveyer','Moderator'),
   created TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
   created_by VARCHAR(50) DEFAULT 'test@industechnology.com',
   updated TIMESTAMP,
   updated_by VARCHAR(50),
   operational_status BOOLEAN DEFAULT 1,
   UNIQUE KEY (email),
   PRIMARY KEY ( user_id )
);
7

This needs to be an array.

$result->execute([$user, $pass]);
9

It may work in this context. But in general it’s best to use a fixed unique identifier such as your user_id becuse it is unlikely to ever change.
In another context it may be clearer why it’s better.
Imagine you have some articles on a site and allow user comments. You store the comments in a table. The table may have columns:-

comment_id | article_id | user | comment | datetime

Here user is a foreign key to identify who made a comment. If you store my email as the ID Eg: sama74@gmail.com that may work for a while.
But then I go to my user profile and change my email address to sama74@hotmail.com, your comments table no longer works to identify me as the commenter.
If however you ID me as user_id = 1612 then there is a good chance that isn’t ever going to change.
So while email may be enforced as unique, it is not necessarily a contant that may be reiled upon.

2 Likes