Questa non è una risposta alla NullReferenceException - ci stiamo ancora lavorando nei commenti; questo è un feedback per le parti di sicurezza.
La prima cosa che possiamo guardare è SQL injection; questo è molto facile da risolvere - vedi sotto (nota che ho riordinato anche alcune altre cose)
// 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;
}
Ora, potresti pensare "è troppo codice" - certo; e gli strumenti esistono per aiutare in questo! Ad esempio, supponiamo di averlo fatto:
public class User {
public int Id {get;set;}
public string Email {get;set;}
public string Password {get;set;} // I'll talk about this later
}
Possiamo quindi utilizzare dapper e LINQ per fare tutto il lavoro pesante per noi:
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();
}
Questo fa tutto hai (incluso l'apertura e la chiusura in sicurezza della connessione), ma lo fa in modo pulito e sicuro. Se il metodo restituisce un null valore per l'User , significa che non è stata trovata alcuna corrispondenza. Se un User non nullo viene restituita l'istanza - dovrebbe contenere tutti i valori previsti usando solo le convenzioni basate sui nomi (significato:i nomi delle proprietà e i nomi delle colonne corrispondono).
Potresti notare che l'unico codice rimasto è codice effettivamente utile - non è un impianto idraulico noioso. Strumenti come dapper sono tuoi amici; usali.
Infine; Le password. Non dovresti mai memorizzare le password. Mai. Neanche una volta. Nemmeno crittografato. Mai. Dovresti solo memorizzare hash di password. Ciò significa che non potrai mai recuperarli. Invece, dovresti eseguire l'hashing di ciò che l'utente fornisce e confrontarlo con il valore hash preesistente; se gli hash corrispondono:questo è un passaggio. Questa è un'area complicata e richiederà modifiche significative, ma dovresti farlo . Questo è importante. Quello che hai in questo momento è insicuro.
