Esta não é uma resposta para o
NullReferenceException - ainda estamos trabalhando nisso nos comentários; este é um feedback para as partes de segurança. A primeira coisa que podemos observar é a injeção de SQL; isso é muito fácil de corrigir - veja abaixo (observe que também arrumei algumas outras coisas)
// 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;
}
Agora, você pode estar pensando "isso é muito código" - claro; e existem ferramentas para ajudar com isso! Por exemplo, suponha que fizemos:
public class User {
public int Id {get;set;}
public string Email {get;set;}
public string Password {get;set;} // I'll talk about this later
}
Podemos então usar dapper e LINQ para fazer todo o trabalho pesado para nós:
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();
}
Isso faz tudo você tem (incluindo abrir e fechar a conexão com segurança), mas o faz de forma limpa e segura. Se o método retornar um
null valor para o User , significa que nenhuma correspondência foi encontrada. Se um User não nulo instância é retornada - ela deve conter todos os valores esperados usando apenas convenções baseadas em nome (ou seja:os nomes das propriedades e os nomes das colunas correspondem). Você pode notar que o único código que resta é um código realmente útil - não é encanamento chato. Ferramentas como dapper são seus amigos; usa-os.
Finalmente; senhas. Você nunca deve armazenar senhas. Sempre. Nem uma vez. Nem mesmo criptografado. Nunca. Você deve somente armazenar hashes de senhas. Isso significa que você nunca poderá recuperá-los. Em vez disso, você deve fazer um hash do que o usuário fornece e compará-lo com o valor de hash pré-existente; se os hashes corresponderem:isso é um passe. Esta é uma área complicada e exigirá mudanças significativas, mas você deve fazer isso . Isso é importante. O que você tem no momento é inseguro.
