Skip to content

fix: Integrate Login System with Database #46

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sergioceron wants to merge 3 commits into
base: master
Choose a base branch
from
Open

fix: Integrate Login System with Database #46

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d7803df
fix: Integrate Login System with Database
sergioceron Apr 11, 2026
a12ad5c
fix: Integrate Login System with Database
sergioceron Apr 11, 2026
2931273
fix: Integrate Login System with Database
sergioceron Apr 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 49 additions & 49 deletions action_login.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,50 +1,50 @@
<?php
/**********************************************************************
* Author : Sergio Ceron Figueroa (sxceron@laciudadx.com)
* Alias : sxceron
* Web : http://www.dotrow.info
* Name : jShop v1.0
* Desc : Valida un usuario para iniciar sesion
* userName : obligatorio (nombre de usuario)
* userPassword: obligatorio (password del usuario)
*
*
**********************************************************************/
// Include file headers
include_once "./includes/validator.php";
include_once "./includes/settings.php";
include_once "./includes/db.php";
include_once "./includes/security.php";
$_validator = new Validator();
$_validator->setMethod( "POST" );
$_validator->setVars( array("userName:required", "userPassword:required") );
if( $_validator->validate() ){
$values = $_validator->getValues();
$user = $db->get_row( "select * from usuarios where usuario_alias='".$values["userName"]."'" );
if( $user->usuario_password == md5($values["userPassword"]) ){
$_SESSION[ 'user_id' ] = $user->usuario_id;
$_SESSION[ 'user_alias' ] = $user->usuario_alias;
$_SESSION[ 'user_role' ] = $user->usuario_tipo;
$db->query( "update usuarios set usuario_ultimoacceso='".date("y/m/d")."' where usuario_id=".$user->usuario_id );
if( $user->usuario_tipo == 2 ){
header( 'Location: ./admin_ponencias.php');
}else if( $user->usuario_tipo == 3 ){
header( 'Location: ./evaluate_ponencias.php');
}else{
header( 'Location: ./adminpanel.php');
}
}else{
header( 'Location: ./login.php?id='.base64_encode( "2" ) );
}
}else{
for( $err="", $i = 0; $i < count($e = $_validator->getErrors()); $i++ ){
$err = $err.";".$e[$i]["field"];
}
header( 'Location: ./login.php?id='.base64_encode( "0" ).'&tk='.base64_encode($err) );
}
<?php
/**********************************************************************
* Author : Sergio Ceron Figueroa (sxceron@laciudadx.com)
* Alias : sxceron
* Web : http://www.dotrow.info
* Name : jShop v1.0
* Desc : Valida un usuario para iniciar sesion
* userName : obligatorio (nombre de usuario)
* userPassword: obligatorio (password del usuario)
*
*
**********************************************************************/
// Include file headers
include_once "./includes/validator.php";
include_once "./includes/settings.php";
include_once "./includes/db.php";
include_once "./includes/security.php";

$_validator = new Validator();
$_validator->setMethod( "POST" );
$_validator->setVars( array("userName:required", "userPassword:required") );

if( $_validator->validate() ){
$values = $_validator->getValues();

$user = $db->get_row( "SELECT * FROM usuarios WHERE usuario_alias='".$db->escape($values["userName"])."'" );
if( $user && $user->usuario_password === md5($values["userPassword"]) ){
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
$_SESSION[ 'user_id' ] = $user->usuario_id;
$_SESSION[ 'user_alias' ] = $user->usuario_alias;
$_SESSION[ 'user_role' ] = $user->usuario_tipo;
$db->query( "update usuarios set usuario_ultimoacceso='".date("y/m/d")."' where usuario_id=".$user->usuario_id );
if( $user->usuario_tipo == 2 ){
header( 'Location: ./admin_ponencias.php');
}else if( $user->usuario_tipo == 3 ){
header( 'Location: ./evaluate_ponencias.php');
}else{
header( 'Location: ./adminpanel.php');
}
}else{
header( 'Location: ./login.php?id='.base64_encode( "2" ) );
}
Comment on lines +32 to +41
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.

}else{
for( $err="", $i = 0; $i < count($e = $_validator->getErrors()); $i++ ){
$err = $err.";".$e[$i]["field"];
}
header( 'Location: ./login.php?id='.base64_encode( "0" ).'&tk='.base64_encode($err) );
}

?>
16 changes: 8 additions & 8 deletions dao/UsuarioDao.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ public static function deleteAll($usuarios){
}
}

public static function save($usuario){
public static function save($usuario, $db){
try{
Comment on lines +15 to 16
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
$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()' ";
Expand All @@ -27,7 +27,7 @@ public static function save($usuario){
}
}

public static function update($usuario){
public static function update($usuario, $db){
try{
$sql = "update usuarios set";
$sql .= " usuario_nombre = '$usuario->getNombre()',";
Expand Down Expand Up @@ -59,19 +59,19 @@ public static function persist($usuario){
}
}

public static function delete($usuario){
public static function delete($usuario, $db){
try {
$db->query("delete from usuarios where usuario_id=$usuario->getId()");
$db->query("DELETE FROM usuarios WHERE usuario_id=".$db->escape($usuario->getId()));
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
}catch(Exception $e){
throw new TransactionExcepion($e->getMessage(), $usuario, TransactionExcepion::DELETE_CODE, $e);
}
}

public static function findByQuery($query){
public static function findByQuery($query, $db){
$users = array();
$sql = "select * from usuarios where $query";
try{
$rows = $db->get_results( $sql );
$rows = $db->get_results($sql);
foreach( $rows as $row ){
$users[] = new Usuario($row);
}
Comment on lines 72 to 77
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$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 uses AI. Check for mistakes.
Expand All @@ -95,11 +95,11 @@ public static function findAll(){
return $users;
}

public static function findById($id){
public static function findById($id, $db){
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
$sql = "select * from usuarios where usuario_id=$id";
$usuario = new Usuario();
try{
$row = $db->get_results( $sql );
$row = $db->get_row($sql);
$usuario = new Usuario($row);
}catch(Exception $e){
Comment on lines 101 to 104
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$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.

Copilot uses AI. Check for mistakes.
throw new QueryException($e->getMessage(), $sql, 0, $e);
Expand Down
14 changes: 7 additions & 7 deletions services/UsuarioManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ class UsuarioManager{
public static function registrarAdministrador($usuario){
try{
$usuario->setTipo(UsuarioType::ADMINISTRADOR);
UsuarioDao::persist($usuario);
UsuarioDao::persist($usuario, $GLOBALS['db']);
}catch(TransactionException $te){
Comment on lines 3 to 7
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
throw $te;
}
Expand Down Expand Up @@ -47,39 +47,39 @@ public static function registrarAsistente($usuario){

public static function obtener($id){
try{
return UsuarioDao::findById($id);
return UsuarioDao::findById($id, $GLOBALS['db']);
}catch(QueryException $qe){
throw $qe;
}
}

public static function eliminar($usuario){
try{
UsuarioDao::delete($usuario);
UsuarioDao::delete($usuario, $GLOBALS['db']);
}catch(TransactionException $te){
throw $te;
}
}

public static function listar(){
try{
return UsuarioDao::findAll();
return UsuarioDao::findAll($GLOBALS['db']);
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
}catch(QueryException $qe){
throw $qe;
}
}

public static function actualizar($usuario){
try{
return UsuarioDao::persist($usuario);
return UsuarioDao::persist($usuario, $GLOBALS['db']);
}catch(TransactionException $te){
throw $te;
}
}

public static function alreadyRegistered($alias){
try{
$usuarios = UsuarioDao::findByQuery("usuario_alias='$alias'");
$usuarios = UsuarioDao::findByQuery("usuario_alias='".$GLOBALS['db']->escape($alias)."'", $GLOBALS['db']);
if( count($usuarios) > 0 ){
return true;
}
Expand All @@ -91,7 +91,7 @@ public static function alreadyRegistered($alias){

public static function checkPassword($_usuario){
try{
$usuario = UsuarioDao::findByQuery("usuario_alias='$_usuario->getAlias()'");
$usuario = UsuarioDao::findByQuery("usuario_alias='".$GLOBALS['db']->escape($_usuario->getAlias())."'", $GLOBALS['db']);
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
if( $usuario->getPassword() == $_usuario->getPassword() ){
return true;
}
Expand Down