これはNullReferenceException
への回答ではありません -コメントではまだ作業中です。これはセキュリティパーツへのフィードバックです。
最初に確認できるのはSQLインジェクションです。これは非常に簡単に修正できます。以下を参照してください(他のことも整理しました)
// 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;
}
さて、あなたは「それはコードが多すぎる」と思っているかもしれません-確かに;そしてそれを助けるためのツールが存在します!たとえば、次のようにしたとします。
public class User {
public int Id {get;set;}
public string Email {get;set;}
public string Password {get;set;} // I'll talk about this later
}
その後、dapper を使用できます。 とLINQは、私たちのためにすべての面倒な作業を行います:
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();
}
これはすべて あなたは(接続を安全に開閉することを含む)持っていますが、それはきれいにそして安全にそれをします。メソッドがnull
を返す場合 User
の値 、一致するものが見つからなかったことを意味します。 null以外のUser
インスタンスが返されます。名前ベースの規則を使用するだけで、期待されるすべての値が含まれている必要があります(つまり、プロパティ名と列名が一致します)。
残っているコードは実際に役立つコードだけであることに気付くかもしれません。 -それは退屈な配管ではありません。 dapperのようなツールはあなたの友達です;それらを使用してください。
ついに;パスワード。パスワードは絶対に保存しないでください。これまで。一度もありません。暗号化さえされていません。一度もない。 のみ ハッシュを保存する パスワードの。これは、それらを取得できないことを意味します。代わりに、ユーザーが提供するものをハッシュして、既存のハッシュ値と比較する必要があります。ハッシュが一致する場合:それはパスです。これは複雑な領域であり、大幅な変更が必要になりますが、これを行う必要があります 。これは重要。現在持っているものは安全ではありません。