Reputation: 55
I'm trying to build my first program and I've run into a few problems with SQL.
I have a database with customers and employees (among other things) and I'm trying to set up a basic login screen. Users log in with their EmployeeID (primary key)
This part deals with logging in for the first time.
It checks the database for a user with that ID and grabs the password. If the password is not empty / null, it shows a message box and stops there.
If the password is empty/null it is meant to confirm and then update the new password.
This is where my understanding ends. Debugging shows the error is definitely in the SQL update, but I can't see any errors...
Help?
Thanks.
Code as follows:
Private Sub NewPass()
Dim PassCheck As String
GetDatabase()
'Check if there is a password
SQLString = "SELECT EmployeeID, Password, Deleted FROM Employee "
SQLString += "WHERE EmployeeID= " & UserID
Try
connection.Open() ' Open Connection
If ConnectionState.Open.ToString = "Open" Then
command = New System.Data.OleDb.OleDbCommand(SQLString, connection)
datareader = command.ExecuteReader()
If datareader.HasRows Then
If datareader.Item("Deleted") = False Then
'Password check
outputString = datareader.Item("Password").ToString
Else
outputString = ""
End If
End If 'recordset has rows
datareader.Close()
End If
Catch ex As Exception 'Display Error message
MessageBox.Show("Error accessing database", "Error", MessageBoxButtons.OK, MessageBoxIcon.Error)
End Try
connection.Close()
PassCheck = outputString
If String.IsNullOrEmpty(PassCheck) = True Then
GetDatabase()
SQLString = "UPDATE Employee SET "
SQLString += "Password = '" & Password & "' "
SQLString += "WHERE EmployeeID = " & UserID
'Try
connection.Open()
If ConnectionState.Open.ToString = "Open" Then
'Update Password
command = New System.Data.OleDb.OleDbCommand(SQLString, connection)
command.ExecuteNonQuery()
MainMenuForm.Show()
Me.Hide()
Else
outputString = ""
MessageBox.Show("Error opening connection")
End If
connection.Close()
'Catch ex As Exception
'End Try
Else
MessageBox.Show("This Account has a password", "Error", MessageBoxButtons.OK, MessageBoxIcon.Error)
FirstTimeButton.Show()
FirstTimeBool = False
End If
End Sub
GetDatabase()
Private Sub GetDatabase()
'Locates the Database
Try
ConnectionString = "Provider=Microsoft.ACE.OLEDB.12.0;Data "
ConnectionString += "Source=" & System.Windows.Forms.Application.StartupPath & "\PharmacyDatabase.accdb "
connection = New System.Data.OleDb.OleDbConnection(ConnectionString)
Catch ex As Exception 'No Database Found
MessageBox.Show("No Database Found in \Debug", "Error", MessageBoxButtons.OK, MessageBoxIcon.Error)
End Try
End Sub
Upvotes: 2
Views: 1468
Reputation: 416149
I hate to be the bearer of bad news, but there's just so much wrong there, including multiple serious security issues and a potential denial of service flaw. Try the code below. You'll need the BCrypt library available here:
http://derekslager.com/blog/posts/2007/10/bcrypt-dotnet-strong-password-hashing-for-dotnet-and-mono.ashx
First, a new GetDatabase() function:
Private Function GetDatabase() As OleDbConnection
Return New System.Data.OleDbOleDbConnection(string.Format("Provider=Microsoft.ACE.OLEDB.12.0;Data Source={0}\PharmacyDatabase.accdb", System.Windows.Forms.Application.StartupPath))
End Function
It's counter-intuitive, but thanks to connection pooling it really is better in .Net to create a new connection each time.
Then, the main function:
Private Sub NewPass()
Dim sql As String = "UPDATE Employee SET Password = ? WHERE EmployeeID= ? AND Password IS NULL"
Dim resultCount As Integer
Using cn As System.Data.OleDb.OleDbCommand = GetDatabase(), _
command As New System.Data.OleDb.OleDbCommand(sql, cn)
command.Parameters.Add("Password", OleDbType.Char, 60).Value = BCrypt.HashPassword(password, BCrypt.GenerateSalt(12))
command.Parameters.Add("UserID", OleDbType.Integer).Value = UserId
cn.Open()
resultCount = command.ExecuteNonQuery()
End Using
If resultCount <> 1 Then
'If a password was already set (password field not null), resultCount will be zero
MessageBox.Show("This Account does not exist or already has a password", "Error", MessageBoxButtons.OK, MessageBoxIcon.Error)
FirstTimeButton.Show()
FirstTimeBool = False
Else
MainMenuForm.Show()
Me.Hide()
End If
End Sub
There are a few things in that code that are completely missing, and so I want to explain where they went. The first is that there is no connection.Close()
. This is completely handled by the Using block, and handled better because the Using block ensures the connection is closed even if an exception is thrown.
Next up is the Try/Catch block. My feeling here is that this is the wrong level of code to check for errors. The wonderful thing about exceptions is that they climb up the stack. It's usually better to save your try/catch for code nearer the UI, as it's normally in a better position to know how to respond. So code that talks to the database directly should just let the exception happen, and the business or presentation layer can decide what to do with it.
The final missing piece is the connection state check. Connection state checks are really only needed either in an exception handler if you are trying to recover from an error or in the case where you have long-lived connections that you want to just leave open. As I've already said, I don't believe this level of code is the right place for error handling and .Net does better with short lived connections, so we can skip this entirely.
Upvotes: 5
Reputation: 12495
You have the following in your code as part of the UPDATE
statement:
SQLString += "Password = '" & Password & "' "
However, I don't see where you have a variable named Password
. I see a PassCheck
, but no Password
.
Upvotes: 0