3

I am currently writing a small application to keep track of monetary ins and outs, something just to improve my general C# skills. For my Login Screen currently I have the following Code

    private void Login_Load(object sender, EventArgs e)
    {
        // TODO: This line of code loads data into the 'spendingInsAndOutsDataSet.Users' table. You can move, or remove it, as needed.
        this.usersTableAdapter.Fill(this.spendingInsAndOutsDataSet.Users);
    }

    private void button1_Click(object sender, EventArgs e)
    {
        string userNameText = userName.Text;
        string passwordText = password.Text;

        foreach (DataRow row in spendingInsAndOutsDataSet.Users)
        {
            if (row.ItemArray[4].Equals(userNameText) && row.ItemArray[5].Equals(passwordText))
            {
                MessageBox.Show("Login Successful");

                MainGUI newForm = new MainGUI();
                this.Visible = false;
                newForm.Show();
                break;
            }
            else
            {
                userName.Text = String.Empty;
                password.Text = String.Empty;
                MessageBox.Show("Login Failed");
                break;
            }
        }
    }

What I am looking to do when the Login is Sucessful is to write the MachineName of the current PC to a field in Users table in my SQL Database. That way when I come to start creating records I can quickly find my UsersId (which is a foreign key in my Transactions table).

I know you can get the Active Machine Name using the System.Enviroments path but Im unsure exactly how to go about writing the update. I know how to do it using a SqlCommand but I am wondering if there is a simpler way using the DataRows I have used within the ForEach loop.

Thanks in advance, any questions let me know.

James

jeastham1993
  • 330
  • 1
  • 9
  • 20
  • 3
    use `SqlCommand`, don't try to make it harder on yourself, you are trying to go down the **worse** path, not the *better* path – Jason Oct 08 '13 at 20:57
  • Thanks for the response Jason. I have currently set the Application up to use a SqlCommand, although if anybody does still know a way to access the specific data row from the DataSet I would greatly appreciate it. – jeastham1993 Oct 08 '13 at 21:30
  • You would be better off asking the database if the results exist versus bringing it all back. Separate the functions out you are trying to achieve. 1. At least hash the passwords when you store them in the database 2. Create a SQL command that takes username / password (hashed) as parameters and get the ID row back 3. If the ID row is null - user wasn't found - if it is a valid integer - then it was – tsells Oct 09 '13 at 00:20

2 Answers2

2

In your foreach loop, set the MachineName of the current PC on relevant row then at the end of the method call:

this.usersTableAdapter.Update(this.spendingInsAndOutsDataSet.Users);

This will update the database with the machine name

However looking at your code there are a few additional comments to make I'd like to add to improve what you have:

You are loading the entire data table and then checking it for the username and password. Really you query for the user ID in the database, load that single row and check the password. If you have many users, your current implementation will create a lot of network traffic.

Instead of:

foreach (DataRow row in spendingInsAndOutsDataSet.Users)

Consider using something like:

foreach (SpendingInsAndOutsDataSet.UsersRow row in spendingInsAndOutsDataSet.Users)

i.e. the strongly typed version of the data row object. This means you can use:

row.Username.Equals(userNameText) 

instead of

row.ItemArray[4].Equals(userNameText) 

Also if you are anticipating that this will be used over a network, you should look to encrypt the passwords.

JustinHui
  • 677
  • 5
  • 17
2

Assuming it is an Access database (If not then make the necessary changes):

Use an Adapter to fill a table with your results. Then compare the row columns with the information provided by the user. Don't forget to use parameters to avoid injections that may potentially ruin your database or expose your user's information to a hacker.

DataTable dt = new DataTable();
String sql = "SELECT * FROM users WHERE user = @user and password=@password"
OleDbConnection connection = getAccessConnection();
OleDbDataAdapter da = new OleDbDataAdapter(sql, connection);
da.SelectCommand.Parameters.Add("@user", OleDbType.VarChar).Value = userNameText;
da.SelectCommand.Parameters.Add("@password", OleDbType.VarChar).Value = password.Text;
try
{
   connection.Open();
   da.Fill(dt);
   connection.Close();
}
catch(OleDbException ex)
{
   connection.Close();
   MessageBox.Show(ex.ToString());
}

if(dt.Rows.Count == 1)
    return true; //username && password matches
else if(dt.Rows.Count == 0)
    return false; // does not match

You could also use AddWithValue for your parameters.

da.SelectCommand.Parameters.AddWithValue("@user", userNameText);

getAccessConnection() is a predefined OleDbConnection function that has the connection to the database setup and creates a new instance of the connection for you (that I have created for myself).

public OleDbConnection getAccessConnection()
{
    this.connection = new OleDbConnection();
    this.connection.ConnectionString = @"Provider=Microsoft.ACE.OLEDB.12.0;Data Source=" 
            + Classified.SOURCE + ";Jet OLEDB:Database Password=" 
            + Classified.PASS + ";";
    return this.connection;
}

It is preferred to create classes for all of these functions for developers who may join in on the project. Also read up on C# test driven development as well.

Also it looks like your loop will break even if that one record fails, only allowing it to go to it's first record.

Onto creating your own data set and filling it with queried tables is also useful. Here is a brief example:

DataSet ds = new DataSet();
ds.Tables.Add(dt, "userSearchedTable");
ds.Tables["userSearchedTable"].Rows[0][1].ToString();

Then you can declare a specific data table within the set when ever you need to.

Andrew Grinder
  • 575
  • 5
  • 21
  • Superb response, thanks for all the info. Another quick question if I may how secure is the code I have written. I appreciate I need to encrypt the password column but are there any obvious security holes? – jeastham1993 Oct 09 '13 at 05:53
  • http://stackoverflow.com/questions/1678555/password-encryption-decryption-code-in-net – Andrew Grinder Oct 10 '13 at 17:54
  • You can also add in the `@password` parameter in the sql query itself. Then if the datatable is empty, then you know the password is incorrect. I will update my answer to this. – Andrew Grinder Jan 26 '15 at 18:43