Esta no es una respuesta a la NullReferenceException - todavía estamos trabajando en eso en los comentarios; esto es retroalimentación para las partes de seguridad.
Lo primero que podemos mirar es la inyección de SQL; esto es muy fácil de arreglar - ver más abajo (nota que también he arreglado algunas otras cosas)
// note: return could be "bool" or some kind of strongly-typed User object
// but I'm not going to change that here
public string[] GetValidUser(string dbUsername, string dbPassword)
{
// no need for the table to be a parameter; the other two should
// be treated as SQL parameters
string query = @"
SELECT id,email,password FROM tbl_user
WHERE [email protected] AND [email protected]";
string[] resultArray = new string[3];
// note: it isn't clear what you expect to happen if the connection
// doesn't open...
if (this.OpenConnection())
{
try // try+finally ensures that we always close what we open
{
using(MySqlCommand cmd = new MySqlCommand(query, connection))
{
cmd.Parameters.AddWithValue("email", dbUserName);
// I'll talk about this one later...
cmd.Parameters.AddWithValue("password", dbPassword);
using(MySqlDataReader dataReader = cmd.ExecuteReader())
{
if (dataReader.Read()) // no need for "while"
// since only 1 row expected
{
// it would be nice to replace this with some kind of User
// object with named properties to return, but...
resultArray[0] = dataReader.GetInt32(0).ToString();
resultArray[1] = dataReader.GetString(1);
resultArray[2] = dataReader.GetString(2);
if(dataReader.Read())
{ // that smells of trouble!
throw new InvalidOperationException(
"Unexpected duplicate user record!");
}
}
}
}
}
finally
{
this.CloseConnection();
}
}
return resultArray;
}
Ahora, podrías estar pensando "eso es demasiado código" - seguro; ¡y existen herramientas para ayudar con eso! Por ejemplo, supongamos que hicimos:
public class User {
public int Id {get;set;}
public string Email {get;set;}
public string Password {get;set;} // I'll talk about this later
}
Entonces podemos usar dapper y LINQ para hacer todo el trabajo pesado por nosotros:
public User GetValidUser(string email, string password) {
return connection.Query<User>(@"
SELECT id,email,password FROM tbl_user
WHERE [email protected] AND [email protected]",
new {email, password} // the parameters - names are implicit
).SingleOrDefault();
}
Esto hace todo tiene (incluyendo abrir y cerrar la conexión de forma segura), pero lo hace de forma limpia y segura. Si el método devuelve un null valor para el User , significa que no se encontró ninguna coincidencia. Si un User no nulo se devuelve la instancia:debe contener todos los valores esperados simplemente usando convenciones basadas en nombres (es decir:los nombres de las propiedades y los nombres de las columnas coinciden).
Puede notar que el único código que queda es código realmente útil - No es una plomería aburrida. Herramientas como dapper son tus amigas; úsalos.
Finalmente; contraseñas Nunca debe almacenar contraseñas. Alguna vez. Ni una sola vez. Ni siquiera encriptado. Nunca. Deberías solo almacenar hashes de contraseñas. Esto significa que nunca podrá recuperarlos. En su lugar, debe codificar lo que proporciona el usuario y compararlo con el valor cifrado preexistente; si los hashes coinciden:eso es un pase. Esta es un área complicada y requerirá cambios significativos, pero debe hacer esto . Esto es importante. Lo que tienes en este momento es inseguro.
