Help with reading database.

I have this code that I hacked together from someone else's example. I kind of understand how it works. I just don't know if it will and i am not in a location right now to check. I was wondering if I did this correctly first, second how can it improve and should i do something different. Basically i just want to check the password in a database. I am passing the username and password to this function from another functio

private bool authUser(string UserName, string Password)
{
string connectionString = ConfigurationSettings.AppSettings["DBConnectionString"];

SqlConnection DBConnection = new SqlConnection(connectionString);

bool result = false;

DBConnection.open()
SqlCommand checkCommand = new SqlCommand("SELECT password FROM Users WHERE userName='" + Password + "', DBConnection)
SqlDataReader checkDataReader = checkCommand.ExecuteReader();

if(checkDataReader.GetString(0) == Password)
{
result = true;
}
else
{
result = false;
}
checkDataReader.Close();
DBConnection.Close();

return result;
}

Thank you
Buddy LindseyNo. It won't work.
You're using the Password in your SQL statement, not the username. So unless the users are really silly and use their usernames as their passwords, it's going to fail.
More seriously, using string concatenation to build query strings represents one of the biggest security disasters on the planet. Never, ever do it. Just search the web for "SQL Injection attack" to see what I mean.
You should also ponder whether == with string comparisons is case sensitive or not.
You can also leak connection resources because you're not using try {} finally {} blocks or the C# using(){} wrapper.
And I'm really hoping that the password is protected in some way (by hashing or encryption), so that the password that is passed into this method is not the plain text password as entered by a user.
|||Yep that definetly won't work,
I agree with DMW, people who write code like that should be shot! :-)
At it's most basic that function may work , apart from the obvious mistake of querying the Password as the Username,
I also convert my string comparisons to .toupper or .tolower , just get rid of casing issues.
on Passwords I always enter them into the DB encrypted, and basically compare the encrypted text.
I would never do this in Code, as I would rather do this kind of operation in the Stored Proc, and let the SP deliver the result.
Inline SQL is so Passe, in a field where we must be moving with the times , this is a no no.
|||That is a little harsh of a response don't you think. Did youtake the time to consider that i may be a new developer and not reallyknow what i am doing.
Anyway, that is why I posted I wanted to dknow if i was doing it rightor not. And when i pass the password to the function it willalready be hashed.
Thanks for the help.
|||

appologies if I came across a little, harsh I did not certainly mean in it in that way,

I was trying to be funny.

I'll stick to code, because I suck at language :-)

|||I agree.
I don't recall advocating that ANY developer ever be shot.
And the reason that I spend my time on this forum is that I'm passionate about passing on what little experience I have to other developers so that they can learn from my mistakes.
So keep posting, and you'll get lot's of support and advice.
Speaking of which, I'll offer one other piece of advice: never, ever use someone else's code (especially not sample code or book code) unless you're really happy with how it works. Most samples, including book samples, don't follow best practice. If they did, the code would be harder to follow and generally much too long to fit in a book. A lot of "book code" (by which I mean conference samples, MSDN samples, my samples) is written to try and explain a learning point, not for commercial use. I know that this sounds daft, but that's the way it is.
The most common thing that is omitted is error handling code (which is critical), and the normal security reviews that would accompany real application development.|||Apology accpeted no hard feelings at all.
yeah one reason I posted was to make sure that I was doing it right. Thank for taking the time to answer.
I have the book code complete so that i can learn better ways of codingI guess i need to pull that sucker out and try to read it again.
Thanks for all the help.
sql