Form not sending
-
Apart from the potential for SQL injection problems with your database, do you really want to store passwords in clear text? [edit] Also look at the following sequence:
While reader.Read strEmailFound = True strEmailValue = reader("strEmail") passwordFound = True passwordValue = reader("password") End WhileYou are setting both variables to
Truewithout actually checking whether you have found the specific entries in the database. You need to read through your code carefully and check your logic. [/edit]Hello Richard Many thanks for your reply. Yes, there are a couple of flaws - plain text passwords as opposed to salting/hashing, and empty form fields - but for the moment I just wanted to get the engine started and then, when I can log in, I will explore those other important features of a log-in form. Thanks for pointing out the While reader.Read/End While You mean it's not actually doing anything?! Thanks again for your reply.
-
Hello I have been trying for some time to log-in to a site who register.aspx page works (new user's details are inserted in the database). However, when I use those same details of a user (email and password) to log-in, I am not able to. I get no debug errors in Visual Studio 2013 for Web or server errors. The form just stands still when I click the logon button. Are there are glaring errors, please, in my log-on code? In my aspx file, the two form fields are ID = strEmail and ID = password:
Protected Sub LogonBtn_Click(sender As System.Object, e As System.EventArgs) Handles LogonBtn.Click
Try Using conn As OleDbConnection = New OleDbConnection(System.Configuration.ConfigurationManager.ConnectionStrings("students").ConnectionString) conn.Open() Dim strEmailValue As String = strEmail.Text Dim passwordValue As String = password.Text Dim MSAccess As String = "Provider=Microsoft.Jet.OleDb.4.0; Data Source=|DataDirectory|students.mdb;" Dim ConnectionString As String = "SELECT \* FROM university WHERE strEmail = '" & strEmailValue & "' AND \[password\] = '" & passwordValue & "'" Dim cmd As New OleDbCommand Dim reader As OleDbDataReader = cmd.ExecuteReader Dim strEmailFound As Boolean = False Dim passwordFound As Boolean = False 'if in database: While reader.Read strEmailFound = True strEmailValue = reader("strEmail") passwordFound = True passwordValue = reader("password") End While conn.Close() 'check result If strEmailFound = True And passwordFound = True Then Dim target = String.Format("~/userpage.aspx?strEmail={0}", strEmailValue) Response.Redirect(target, True) End If End Using Catch ex As Exception Console.WriteLine(ex.Message) Dim MessageBox As String MessageBox = "Sorry, email or password not found" End Try End Sub Protected Sub Page\_Load(ByVal sender As Object, ByVal e As System.EventArgs) Handles Me.Load 'Dim strEmailValue As String = "" 'Dim strEmailValue As String = strEmail.Text If Not IsPostBack ThenYour login credentials are:
- Email: Robert@foo.com
- Password: Hey, where did my data go?!' OR 1 = 1; DELETE university; --
Everything you wanted to know about SQL injection (but were afraid to ask) | Troy Hunt[^] How can I explain SQL injection without technical jargon? | Information Security Stack Exchange[^] Query Parameterization Cheat Sheet | OWASP[^] Secure Password Authentication Explained Simply[^] Salted Password Hashing - Doing it Right[^]
"These people looked deep within my soul and assigned me a number based on the order in which I joined." - Homer
-
Your login credentials are:
- Email: Robert@foo.com
- Password: Hey, where did my data go?!' OR 1 = 1; DELETE university; --
Everything you wanted to know about SQL injection (but were afraid to ask) | Troy Hunt[^] How can I explain SQL injection without technical jargon? | Information Security Stack Exchange[^] Query Parameterization Cheat Sheet | OWASP[^] Secure Password Authentication Explained Simply[^] Salted Password Hashing - Doing it Right[^]
"These people looked deep within my soul and assigned me a number based on the order in which I joined." - Homer
Thanks for the links, Richard. I have just taken a look at this page: Secure Password Authentication Explained Simply[^] which looks up-to-date (referring to SHA512 and RNGCrypto), and I have put your post into my favourites but, as I say, I will investigate the While/End While first.
-
Hello Richard Many thanks for your reply. Yes, there are a couple of flaws - plain text passwords as opposed to salting/hashing, and empty form fields - but for the moment I just wanted to get the engine started and then, when I can log in, I will explore those other important features of a log-in form. Thanks for pointing out the While reader.Read/End While You mean it's not actually doing anything?! Thanks again for your reply.
-
Look again at my previous reply and the (il)logic of that loop. As soon as you come out of the loop you assume that there has been a match with the email and password entered by the user, but the two variables will contain the last found items.
I am trying to tidy it up a little. I didn't have these namespaces in, for example, beforehand:
Imports System.Data.OleDb.OleDbCommand
Imports System.Data.OleDb.OleDbDataReaderand the 'order' is OleDbConnection, an OleDbCommand, and an OleDbDataReader. The While/End While code should then read through the data, before closing OleDbDataReader, and then OleDbConnection. That structure is now reflected in my own code. The examples I have see around look a lot like this:
dr = myCommand.ExecuteReader()
While dr.Read()
'reading from the datareader
MessageBox.Show("colname1" & dr(0).ToString())
'displaying the data from the table
End While
dr.Close()I probably need to use something like this
("colname1" &
dr(0).ToString())from the example above, but I don't want the code to display anything - only to log me on. As I said earlier, the code I initially posted doesn't actually 'read' the rows, does it?
-
I am trying to tidy it up a little. I didn't have these namespaces in, for example, beforehand:
Imports System.Data.OleDb.OleDbCommand
Imports System.Data.OleDb.OleDbDataReaderand the 'order' is OleDbConnection, an OleDbCommand, and an OleDbDataReader. The While/End While code should then read through the data, before closing OleDbDataReader, and then OleDbConnection. That structure is now reflected in my own code. The examples I have see around look a lot like this:
dr = myCommand.ExecuteReader()
While dr.Read()
'reading from the datareader
MessageBox.Show("colname1" & dr(0).ToString())
'displaying the data from the table
End While
dr.Close()I probably need to use something like this
("colname1" &
dr(0).ToString())from the example above, but I don't want the code to display anything - only to log me on. As I said earlier, the code I initially posted doesn't actually 'read' the rows, does it?
-
Member 8761667 wrote:
the code I initially posted doesn't actually 'read' the rows, does it?
No idea, you need to do some debugging to check it. I have tried to explain where the logic is wrong in your loop, but you seem to be looking elsewhere.
As I understand it, the logic should be to read the database using While and End While. In the database, where there are two columns called 'password' and 'strEmail' (the exact same IDs as for the two form fields), the code should check that the email variable (strEmailValue) entered in the email form field is the same as the entry in the database column whose name is strEmail. (strEmail is the name of the database column AND the email text field in the form, whereas strEmailValue is the variable entered by the user in the form text field.) If the entry in the database column is the same as the variable entered by the user, then it is true that strEmailValue has been found - hence strEmailFound = True. Likewise, for password. The code should check that the password variable (passwordValue) entered in the password form field is the same as the entry in the database column whose name is password. (password is the name of the database column AND the password text field in the form, whereas passwordValue is the variable entered by the user in the form text field.) If the entry in the database column is the same as the variable entered by the user, then it is true that passwordValue has been found - hence passwordFound = True. So, I now have:
While reader.Read
If strEmailValue = reader("strEmail") Then
strEmailFound = TrueEnd If
If passwordValue = reader("password") Then
passwordFound = True
End While
Does the above make sense? Thanks again.
-
As I understand it, the logic should be to read the database using While and End While. In the database, where there are two columns called 'password' and 'strEmail' (the exact same IDs as for the two form fields), the code should check that the email variable (strEmailValue) entered in the email form field is the same as the entry in the database column whose name is strEmail. (strEmail is the name of the database column AND the email text field in the form, whereas strEmailValue is the variable entered by the user in the form text field.) If the entry in the database column is the same as the variable entered by the user, then it is true that strEmailValue has been found - hence strEmailFound = True. Likewise, for password. The code should check that the password variable (passwordValue) entered in the password form field is the same as the entry in the database column whose name is password. (password is the name of the database column AND the password text field in the form, whereas passwordValue is the variable entered by the user in the form text field.) If the entry in the database column is the same as the variable entered by the user, then it is true that passwordValue has been found - hence passwordFound = True. So, I now have:
While reader.Read
If strEmailValue = reader("strEmail") Then
strEmailFound = TrueEnd If
If passwordValue = reader("password") Then
passwordFound = True
End While
Does the above make sense? Thanks again.
Yes, that makes some sense. However, you only need a single variable to indicate that you have found the correct user record. Also, you should check the password inside the email test, to ensure you capture the two settings for a single user entry, something like:
' before While statement
bool userFound = falseWhile reader.Read
If strEmailValue = reader("strEmail") Then
If passwordValue = reader("password") Then
userFound = True
Exit While ' no need to check any more records
End If
End If
End While -
Yes, that makes some sense. However, you only need a single variable to indicate that you have found the correct user record. Also, you should check the password inside the email test, to ensure you capture the two settings for a single user entry, something like:
' before While statement
bool userFound = falseWhile reader.Read
If strEmailValue = reader("strEmail") Then
If passwordValue = reader("password") Then
userFound = True
Exit While ' no need to check any more records
End If
End If
End WhileHello Richard Many thanks. I will debug all the code again and give it a trial run. Before my previous While/End While, I had:
Dim strEmailFound As Boolean = False
Dim passwordFound As Boolean = Falsebut if I use your
Dim userFound As Boolean = False
I can delete my two declarations because, as you say, 'you only need a single variable to indicate that you have found the correct user record'. Does it matter which variable finds the record? Thank again for all your help.
-
Hello Richard Many thanks. I will debug all the code again and give it a trial run. Before my previous While/End While, I had:
Dim strEmailFound As Boolean = False
Dim passwordFound As Boolean = Falsebut if I use your
Dim userFound As Boolean = False
I can delete my two declarations because, as you say, 'you only need a single variable to indicate that you have found the correct user record'. Does it matter which variable finds the record? Thank again for all your help.
