Dies ist keine Antwort auf die NullReferenceException - wir arbeiten das noch in den Kommentaren durch; das ist Feedback für die Sicherheitsteile.
Das erste, was wir uns ansehen können, ist die SQL-Injection; Dies ist sehr einfach zu beheben - siehe unten (beachten Sie, dass ich auch einige andere Dinge aufgeräumt habe)
// 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;
}
Jetzt denken Sie vielleicht:"Das ist zu viel Code" - sicher; und es gibt Werkzeuge, die dabei helfen! Angenommen, wir haben Folgendes getan:
public class User {
public int Id {get;set;}
public string Email {get;set;}
public string Password {get;set;} // I'll talk about this later
}
Wir können dann dapper verwenden und LINQ, um die ganze schwere Arbeit für uns zu erledigen:
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();
}
Das macht alles Sie haben (einschließlich des sicheren Öffnens und Schließens der Verbindung), aber es macht es sauber und sicher. Wenn die Methode einen null zurückgibt Wert für den User , bedeutet dies, dass keine Übereinstimmung gefunden wurde. Wenn ein User ungleich Null ist -Instanz zurückgegeben wird - sie sollte alle erwarteten Werte enthalten, nur unter Verwendung namensbasierter Konventionen (was bedeutet:die Eigenschaftsnamen und Spaltennamen stimmen überein).
Sie werden vielleicht bemerken, dass der einzige verbleibende Code tatsächlich nützlicher Code ist - Es ist keine langweilige Klempnerarbeit. Tools wie Dapper sind dein Freund; Verwenden Sie sie.
Endlich; Passwörter. Sie sollten niemals Passwörter speichern. Je. Nicht ein einziges Mal. Nicht einmal verschlüsselt. Niemals. Sie sollten nur Hashes speichern von Passwörtern. Das bedeutet, dass Sie sie niemals abrufen können. Stattdessen sollten Sie die Angaben des Benutzers hashen und mit dem bereits vorhandenen Hash-Wert vergleichen. Wenn die Hashes übereinstimmen:Das ist ein Pass. Dies ist ein komplizierter Bereich und erfordert erhebliche Änderungen, aber Sie sollten dies tun . Das ist wichtig. Was Sie im Moment haben, ist unsicher.
