fix: Integrate Login System with Database

[sergioceron]

Automated fix by CoderOps.

Swarm: SwarmX3
Task: Integrate Login System with Database

[Copilot]

Pull request overview

This PR aims to integrate the login/user management flow more tightly with the database layer by explicitly passing the ezSQL $db handle into UsuarioDao operations and adding SQL escaping in a few user-lookup paths.

Changes:

• Updated UsuarioManager to pass $GLOBALS['db'] into UsuarioDao calls and to escape aliases in query strings.
• Updated UsuarioDao method signatures to accept a $db parameter for some operations and adjusted some query calls (get_row, DELETE, etc.).
• Updated action_login.php to escape the submitted username before querying and to null-check the fetched user row.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 10 comments.

File	Description
services/UsuarioManager.php	Passes $GLOBALS['db'] into DAO calls; updates alias queries to use $db->escape().
dao/UsuarioDao.php	Changes DAO method signatures to accept $db; modifies several SQL queries and row-fetching calls.
action_login.php	Escapes userName in SQL and adds a null-check before comparing password hash.

Comments suppressed due to low confidence (2)

dao/UsuarioDao.php:24

• save() builds an INSERT by interpolating Usuario fields directly into SQL without escaping/parameterization. Because these fields can contain quotes, this is SQL-injection prone and can break queries. Use $db->escape() (or parameterized queries) for every string value before embedding it in SQL.

			$sql  = "insert into usuarios(usuario_nombre, usuario_apellidos, usuario_correo, usuario_telefono, usuario_direccion, usuario_nacimiento, usuario_alias, usuario_password, usuario_tipo) ";
		  	$sql .= "values('$usuario->getNombre()','$usuario->getApellidos()' ";
		 	$sql .= ",'$usuario->getCorreo()','$usuario->getTelefono()'";
			$sql .= ",'$usuario->getDireccion()','$usuario->getFechaNacimiento()'";
		 	$sql .= ",'$usuario->getAlias()','$usuario->getPassword()'";
		  	$sql .= ",'$usuario->getTipo()')";
			
			$db->query($sql);

dao/UsuarioDao.php:44

• update() interpolates Usuario fields (and usuario_id) into SQL without escaping/parameterization. This is vulnerable to SQL injection and can also break updates when values contain quotes. Escape all values (including the id, ideally by casting to int) or switch to parameterized queries.

			$sql  = "update usuarios set";
		  	$sql .= "       usuario_nombre = '$usuario->getNombre()',";
		 	$sql .= "       usuario_apellidos = '$usuario->getApellidos()',";
			$sql .= "       usuario_correo = '$usuario->getCorreo()',";
			$sql .= "       usuario_telefono = '$usuario->getTelefono()',";
			$sql .= "       usuario_direccion = '$usuario->getDireccion()',";
			$sql .= "       usuario_nacimiento = '$usuario->getFechaNacimiento()',";
			$sql .= "       usuario_alias = '$usuario->getAlias()',";
			$sql .= "       usuario_password = '$usuario->getPassword()',";
			$sql .= "       usuario_tipo = '$usuario->getTipo()'";
			$sql .= " where usuario_id=$usuario->getId()";
			
			$db->query($sql);

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

UsuarioDao implements Dao, but save($usuario, $db) / update($usuario, $db) (and other methods updated to require $db) no longer match the Dao interface signatures. In PHP this will raise a fatal compatibility error when the class is loaded, and also breaks existing internal calls (e.g., persist()/findAll()/deleteAll() still call save()/update()/findByQuery() without passing $db). Align method signatures across the interface + all DAO methods (or stop implementing Dao) and update call sites consistently (e.g., pass $db through persist/findAll/deleteAll and use self::save/self::update).

[Copilot]

delete() concatenates usuario_id without quoting and relies on $db->escape(). Since escape() won’t make non-quoted numeric expressions safe (e.g., 1 OR 1=1 contains no quotes), this can still be injectable if getId() is not strictly numeric. Cast the id to an integer (or otherwise validate it) before concatenating.

Suggested change

	$db->query("DELETE FROM usuarios WHERE usuario_id=".$db->escape($usuario->getId()));
	$usuarioId = (int) $usuario->getId();
	$db->query("DELETE FROM usuarios WHERE usuario_id=".$usuarioId);

[Copilot]

findById() builds ... where usuario_id=$id without escaping/validation. If $id can come from request parameters, this is SQL-injection prone. Cast $id to int or escape/quote appropriately before building the query.

Suggested change

	public static function findById($id, $db){
	public static function findById($id, $db){
	$id = (int)$id;

[Copilot]

UsuarioDao::persist() is being called with a $db argument here, but UsuarioDao::persist is currently defined as persist($usuario) (no $db param). This will cause an argument count error at runtime. Update UsuarioDao::persist to accept/use the db handle (and propagate it to save/update), or revert the call to the existing signature and use the established global $db pattern.

[Copilot]

UsuarioDao::findAll() is currently defined without parameters, but it’s being called with $GLOBALS['db'] here. This will raise an argument count error unless findAll is updated to accept $db (and pass it down to findByQuery).

Suggested change

	return UsuarioDao::findAll($GLOBALS['db']);
	return UsuarioDao::findAll();

[Copilot]

UsuarioDao::findByQuery() returns an array of Usuario objects, but checkPassword() treats the result as a single Usuario ($usuario->getPassword()). This will fail (array has no getPassword). Either fetch the first element from the result array (and handle the empty case), or add a DAO method that returns a single user by alias.

Suggested change

	$usuario = UsuarioDao::findByQuery("usuario_alias='".$GLOBALS['db']->escape($_usuario->getAlias())."'", $GLOBALS['db']);
	$usuarios = UsuarioDao::findByQuery("usuario_alias='".$GLOBALS['db']->escape($_usuario->getAlias())."'", $GLOBALS['db']);
	if( count($usuarios) === 0 ){
	return false;
	}
	$usuario = $usuarios[0];

[Copilot]

Password verification is still using md5(...). MD5 is not suitable for password hashing (fast and vulnerable to brute force). Migrate to password_hash() on registration/update and password_verify() on login; if you must keep legacy hashes, consider a staged migration that re-hashes on successful login.

Suggested change

	if( $user && $user->usuario_password === md5($values["userPassword"]) ){
	$isValidPassword = false;
	if( $user ){
	if( password_verify($values["userPassword"], $user->usuario_password) ){
	$isValidPassword = true;
	if( password_needs_rehash($user->usuario_password, PASSWORD_DEFAULT) ){
	$newPasswordHash = password_hash($values["userPassword"], PASSWORD_DEFAULT);
	$db->query(
	"update usuarios set usuario_password='".$db->escape($newPasswordHash)."' where usuario_id=".$user->usuario_id
	);
	}
	}else if( $user->usuario_password === md5($values["userPassword"]) ){
	$isValidPassword = true;
	$newPasswordHash = password_hash($values["userPassword"], PASSWORD_DEFAULT);
	$db->query(
	"update usuarios set usuario_password='".$db->escape($newPasswordHash)."' where usuario_id=".$user->usuario_id
	);
	}
	}
	if( $user && $isValidPassword ){

[Copilot]

After issuing header('Location: ...'), the script should exit; to prevent any further processing/output (especially if this file is included or modified later). Add exit; after each redirect path in this controller.

[Copilot]

$db->get_results() returns rows as objects by default (ezSQL), but new Usuario($row) expects an associative array (it uses $row["usuario_id"], etc.). This will cause errors when hydrating users from queries. Either request ARRAY_A output from ezSQL (and pass an array into Usuario), or update Usuario’s constructor to handle ezSQL row objects.

[Copilot]

$db->get_row($sql) returns an object by default, but Usuario’s constructor expects an associative array. As written, new Usuario($row) will fail when $row is an object. Pass ARRAY_A as the second parameter to get_row() (or adjust Usuario to accept objects) to keep DAO hydration consistent.