Ce n'est pas une réponse à la NullReferenceException - nous y travaillons toujours dans les commentaires ; c'est un retour pour les parties sécurité.
La première chose que nous pouvons regarder est l'injection SQL; c'est très facile à corriger - voir ci-dessous (notez que j'ai aussi rangé d'autres choses)
// 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;
}
Maintenant, vous pensez peut-être "c'est trop de code" - bien sûr ; et des outils existent pour vous aider ! Par exemple, supposons que nous ayons :
public class User {
public int Id {get;set;}
public string Email {get;set;}
public string Password {get;set;} // I'll talk about this later
}
Nous pouvons alors utiliser dapper et LINQ pour faire tout le gros du travail pour nous :
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();
}
Cela fait tout vous avez (y compris l'ouverture et la fermeture en toute sécurité de la connexion), mais il le fait proprement et en toute sécurité. Si sa méthode renvoie un null valeur pour l'User , cela signifie qu'aucune correspondance n'a été trouvée. Si un User non nul instance est renvoyée - elle doit contenir toutes les valeurs attendues en utilisant uniquement les conventions basées sur les noms (ce qui signifie :les noms de propriété et les noms de colonne correspondent).
Vous remarquerez peut-être que le seul code qui reste est le code réellement utile - ce n'est pas de la plomberie ennuyeuse. Des outils comme dapper sont vos amis; utilisez-les.
Pour terminer; mots de passe. Vous ne devez jamais stocker de mots de passe. Déjà. Pas même une fois. Même pas crypté. Jamais. Vous devriez seulement stocker les hachages de mots de passe. Cela signifie que vous ne pourrez jamais les récupérer. Au lieu de cela, vous devez hacher ce que l'utilisateur fournit et le comparer à la valeur hachée préexistante ; si les hachages correspondent :c'est une réussite. Il s'agit d'un domaine compliqué qui nécessitera des changements importants, mais vous devriez le faire . C'est important. Ce que vous avez pour le moment n'est pas sûr.
