1

i am building a login page using HTML and JSP. But every time i get the error "username incorrect" which should be shown when the username does not match with the table is SQL server. Here is the code for login form page:

<%@ page language="java" contentType="text/html; charset=UTF-8"
pageEncoding="UTF-8"%>
<!DOCTYPE html>
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
<title>Expense System</title>
<link rel="stylesheet" href="style.css" type="text/css">
</head>

<body>

<div class=form>

<form name = login method = post action = "login1.jsp">
Username : <input name = user type = text placeholder = username> <br><br>
Password : <input name = pass type = password placeholder = password><br><br>

<input type = submit value = "Submit">
<input type = button value = "Register">

</form>
</div>
</body>
</html>

Below is the code for login1.jsp:

<%@ page language="java" contentType="text/html; charse=UTF-8"
pageEncoding="UTF-8" import="java.sql.*"%>

<% Class.forName("com.microsoft.sqlserver.jdbc.SQLServerDriver"); %>

<!DOCTYPE html>
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
<title>login check</title>
</head>
<body>

<% String connectionUrl = "jdbc:sqlserver://localhost:1433;" +
       "databaseName=signin;integratedSecurity=true;";
    Connection con = DriverManager.getConnection(connectionUrl);
    String uname = new String("");
    String upass = new String("");
    ResultSet resultset;
    Statement statement = con.createStatement();
    statement.executeQuery("select username, password from signintable");

    resultset = statement.getResultSet();

    while(resultset.next()){
        uname = resultset.getString("username");
        upass = resultset.getString("password");
    }

if(!request.getParameter("user").equals("")){
if(uname.equals(request.getParameter("user"))){
 if(upass.equals(request.getParameter("pass"))) {%>
 <jsp:forward page="welcome.html"></jsp:forward>

<% }
else {
    out.println("pass incorrect");
}
}
 else {
    out.println("username incorrect");
}
}
else { out.println("user not found!");
}
%>

</body>
</html>
Animesh Porwal
  • 557
  • 1
  • 10
  • 21
  • Bringing back your entire table and looping through it in the application every time someone logs in is not a good idea! You should investigate parameterised queries with a `WHERE` condition to just select the specific row of interest if it exists and hashing passwords too. – Martin Smith Dec 21 '11 at 20:11
  • The use of statements like `String uname = new String("");` and `String upass = new String("");` in Java is always avoidable because you're creating new objects rather than interning those strings which is a bad practice. Just use `String uname=""` and `String upass = ""`. In this case, you're pooling those string objects. – Lion Dec 21 '11 at 20:15
  • @MartinSmith i used WHERE condition but i was able to run my program successfully. But what i want to do is first check username and if it is correct then check password. How can o do this using WHERE condition. Thanks. – Animesh Porwal Dec 21 '11 at 20:22
  • @Lion Thanks for you suggestion. I'll keep this in mind. – Animesh Porwal Dec 21 '11 at 20:23
  • 1
    You should not show whether the username or password is incorrect. This makes for a hacker very easy to guess valid usernames and/or passwords by bruteforcing. You should only show whether the *combination* is valid or not. – BalusC Dec 21 '11 at 20:24

2 Answers2

2

You're hauling the entire database table into Java's memory and assigning the value of every single row to the same variable. Those variables end up holding the values of the last row of the table.

This is not right. You need to select exactly the row you need. Change your SQL query to something like as follows:

PreparedStatement statement = con.prepareStatement("select id from signintable where username=? and password=?");
statement.setString(1, request.getParameter("user"));
statement.setString(2, request.getParameter("pass"));
resultSet = statement.executeQuery();

if (resultset.next()) {
    // Valid login!
} else {
    // Invalid login!
}

Unrelated to the concrete problem, writing Java code inside a JSP file is a poor practice. I suggest you to work on that as well. Learn how to use servlets.

Community
  • 1
  • 1
BalusC
  • 1,082,665
  • 372
  • 3,610
  • 3,555
  • Thanks for your suggestion. But i am damn curious to know about the problem in my code. Can you please tell me what is the problem? – Animesh Porwal Dec 21 '11 at 20:42
  • That's explained in the 1st paragraph. Imagine that the DB returns more than 1 rows. Inside your `while` loop you're getting the username/password from every individual row and assigning to a variable which is declared outside the loop. On every loop iteration, the previously assigned value is **overridden** with the value of the current row. After the loop, the variables hold the values of the *last* row. You're making the comparisons on the values of the *last* row. – BalusC Dec 21 '11 at 21:00
-1

If you want a detailed view, with your code, just write

if(!request.getParameter("user").equals("")) {
    if(uname.equals(request.getParameter("user"))) {
        if(upass.equals(request.getParameter("pass"))) {
            out.println("found the user name ")
        }
    }
} else {
    out.println("din't find it ");
}

You will observe that after repeated "din't find it " phrase there will be "found the user name ".

Oriol
  • 274,082
  • 63
  • 437
  • 513