Dette er ikke et svar på NullReferenceException
- det arbejder vi stadig igennem i kommentarerne; dette er feedback til sikkerhedsdelene.
Det første vi kan se på er SQL-injektion; dette er meget nemt at rette - se nedenfor (bemærk, at jeg også har ryddet nogle andre ting)
// 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 tænker du måske "det er for meget kode" - helt sikkert; og der findes værktøjer til at hjælpe med det! Antag for eksempel, at 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 derefter bruge dapper og LINQ til at gøre alt det tunge løft for os:
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();
}
Dette gør alt du har (inklusive sikker åbning og lukning af forbindelsen), men den gør det rent og sikkert. Hvis metoden returnerer en null
værdi for User
, det betyder, at der ikke blev fundet noget match. Hvis en ikke-null User
forekomst returneres - den skal indeholde alle de forventede værdier blot ved at bruge navnebaserede konventioner (hvilket betyder:egenskabsnavnene og kolonnenavnene matcher).
Du vil måske bemærke, at den eneste kode, der er tilbage, er faktisk nyttig kode - det er ikke kedeligt VVS. Værktøjer som dapper er din ven; bruge dem.
Langt om længe; adgangskoder. Du bør aldrig gemme adgangskoder. Nogensinde. Ikke en eneste gang. Ikke engang krypteret. Aldrig. Du bør kun gemme hashes af adgangskoder. Det betyder, at du aldrig kan hente dem. I stedet bør du hash, hvad brugeren leverer, og sammenligne det med den allerede eksisterende hash-værdi; hvis hasherne matcher:det er et pass. Dette er et kompliceret område og vil kræve betydelige ændringer, men du bør gøre dette . Dette er vigtigt. Det, du har i øjeblikket, er usikkert.