Detta är inte ett svar på NullReferenceException
- Vi arbetar fortfarande igenom det i kommentarerna; detta är feedback för säkerhetsdelarna.
Det första vi kan titta på är SQL-injektion; detta är väldigt lätt att fixa - se nedan (observera att jag har fixat en del andra saker också)
// 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;
}
Nu kanske du tänker "det är för mycket kod" - visst; och verktyg finns för att hjälpa till med det! Anta till exempel att vi gjorde:
public class User {
public int Id {get;set;}
public string Email {get;set;}
public string Password {get;set;} // I'll talk about this later
}
Vi kan sedan använda dapper och LINQ för att göra allt det tunga arbetet för oss:
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();
}
Detta gör allt du har (inklusive säkert att öppna och stänga anslutningen), men det gör det rent och säkert. Om metoden returnerar en null
värde för User
, det betyder att ingen matchning hittades. Om en icke-null User
instans returneras - den bör innehålla alla förväntade värden bara med namnbaserade konventioner (vilket betyder:egenskapsnamnen och kolumnnamnen matchar).
Du kanske märker att den enda koden som finns kvar är faktiskt användbar kod - Det är inte tråkigt VVS. Verktyg som dapper är din vän; använd dem.
Till sist; lösenord. Du bör aldrig lagra lösenord. Någonsin. Inte ens en gång. Inte ens krypterad. Aldrig. Du bör bara lagra hashar av lösenord. Det betyder att du aldrig kan hämta dem. Istället bör du hasha vad användaren tillhandahåller och jämföra det med det redan existerande hasha värdet; om hasharna matchar:det är ett pass. Det här är ett komplicerat område och kommer att kräva betydande förändringar, men du bör göra detta . Det här är viktigt. Det du har för tillfället är osäkert.