vb.net connection to access database

Question

I am trying to migrate my Access db to a vb.net winform (.net framework). My exposure with this software is extremely limited and thus I am tackling each thing in part. My aim is to eventually convert the data to a SQL server before anyone does interject about using and access database (One thing at a time). Anyway I have connected by datasource, created a basic login form and have refactored the code I ported from my access database. This is what I have...

Public intLogonAttempts As Long
    Dim connection As New OleDbConnection(My.Settings.DataConnectionString)

    Private Sub Btn_Login_Click(sender As Object, e As EventArgs) Handles Btn_Login.Click



        Dim RetVal As Integer

        ' Check all information required has been inputted
        If Txt_Username.Text = "" Then
            MsgBox("Please enter a username.", vbOKOnly, "Required Data")
            Txt_Username.Select()
            Exit Sub
        End If

        If Txt_Password.Text = "" Then
            MsgBox("Please enter a password.", vbOKOnly, "Required Data")
            Txt_Password.Select()
            Exit Sub
        End If

        'If connection.State = ConnectionState.Closed Then
        '    connection.Open()
        'End If

        Dim cmd As New OleDbCommand("SELECT * FROM User where [username] = ? AND [Password] = ?", connection)
        cmd.Parameters.AddWithValue("@p1", Txt_Username.Text)
        cmd.Parameters.AddWithValue("@p1", Txt_Password.Text)
        connection.Open()
        RetVal = CInt(cmd.ExecuteScalar())

        If (RetVal = 1) Then
            ' Login successful
            MsgBox("Successful login")

            My.Settings.savedUsername = Txt_Username.Text
            My.Settings.savedPassword = Txt_Password.Text
            My.Settings.Save()
        Else
            ' Login unsuccessful
            intLogonAttempts = intLogonAttempts + 1

            If intLogonAttempts >= 3 Then
                MsgBox("You do not have access to this database. Please contact admin.", vbCritical, "Restricted Access!")
                Application.Exit()
                Exit Sub
            Else
                MsgBox("Password Invalid. Please Try Again", vbOKOnly, "Invalid Entry!")
                Txt_Password.Select()
            End If
        End If
    End Sub

However when my code hits "RetVal = CInt(cmd.ExecuteScalar())" the following error is returned;

System.Data.OleDb.OleDbException: 'Syntax error in FROM clause.'

What am I getting wrong here? I thought it was due to the sql statement but I am pretty sure that I have this correct?

Answer 1

I have divided the code into separate methods to make each method doing only a single job.

The GetUserCount method is completely disconnected from the user interface. A user could be logging in from the web or a phone app and the same method could be used.

Use Using...End Using blocks for your database objects. This ensures that they are closed and disposed even if there is an error. I don't think you ever closed your connection.

Notice that the Select command is getting Count(*). In your code you are just getting * so, with .ExecuteScalar you would get the first column of the first row of your result set. Not what you need.

I used parameter names just for readability so, we can see that the parameters are added to the parameters collection in the same order that they are appear in the Select statement.

Use the .Add method for parameters. See http://www.dbdelta.com/addwithvalue-is-evil/ and https://blogs.msmvps.com/jcoehoorn/blog/2014/05/12/can-we-stop-using-addwithvalue-already/ and another one: https://dba.stackexchange.com/questions/195937/addwithvalue-performance-and-plan-cache-implications Here is another https://andrevdm.blogspot.com/2010/12/parameterised-queriesdont-use.html I had to guess at the datatype and size so, check you database for the actual values.

You should not be storing plain text passwords. That is a separate topic that you need to research.

Private intLogonAttempts As Integer 'You don't need a Long to hold 3


Private Sub Btn_Login_Click(sender As Object, e As EventArgs) Handles Btn_Login.Click
    Dim RetVal As Integer
    If ValidateInput() Then
        RetVal = GetUserCount(Txt_Username.Text, Txt_Password.Text)
    End If

    If (RetVal = 1) Then
        ' Login successful
        MsgBox("Successful login")
        'Why are you storing this information?
        My.Settings.savedUsername = Txt_Username.Text
        My.Settings.savedPassword = Txt_Password.Text
        My.Settings.Save()
    Else
        ' Login unsuccessful
        intLogonAttempts += 1

        If intLogonAttempts >= 3 Then
            MsgBox("You do not have access to this database. Please contact admin.", vbCritical, "Restricted Access!")
            Application.Exit() 'Of course there is nothing to stop the user from restarting the app and trying 3 more times
            Exit Sub
        Else
            'Don't tell the user what is wrong; that would help unauthorized users
            'It could be the user name or the password
            MsgBox("Please Try Again", vbOKOnly, "Invalid Entry!")
        End If
    End If
End Sub

Private Function ValidateInput() As Boolean
    If Txt_Username.Text = "" Then
        MsgBox("Please enter a username.", vbOKOnly, "Required Data")
        Txt_Username.Select()
        Return False
    End If

    If Txt_Password.Text = "" Then
        MsgBox("Please enter a password.", vbOKOnly, "Required Data")
        Txt_Password.Select()
        Return False
    End If
    Return True
End Function

Private Function GetUserCount(UName As String, UPass As String) As Integer
    'All the database code is here
    Dim RetVal As Integer
    Using connection As New OleDbConnection(My.Settings.DataConnectionString),
            cmd As New OleDbCommand("SELECT Count(*) FROM [User] where [username] = @Name AND [Password] = @PWord;", connection)
        cmd.Parameters.Add("@Name", OleDbType.VarChar, 100).Value = UName
        cmd.Parameters.Add("@PWord", OleDbType.VarChar, 100).Value = UPass
        connection.Open()
        RetVal = CInt(cmd.ExecuteScalar())
    End Using
    Return RetVal
End Function

Answer 2

I forgot the [] around the table name user...

As to not waste this post however, would people be able to suggest if this is perhaps the best way to perform the login or are there better solutions out there?