Got stuck in password change code

Question

I make this code for change password with verification. The problem is, when I clicked jbutton to change password, it works and successfully changed the password on database and show the jOptionpane Information message as well.

But after this steps, error message functioned jOptionpane is continuously showing. I try to find where the code was wrong. but couldn't yet.

 private void jBtn_UpdateActionPerformed(java.awt.event.ActionEvent evt) { 

        String user_id = txt_UserID.getText();
        String cur_pass = txt_CurrentPassword.getText();
        String new_pass = txt_NewPassword.getText();

    try {
            Connection c = DBConnection.dbconmethod();
            Statement s = c.createStatement();
            ResultSet rs = s.executeQuery("SELECT * from tch_data");

       while(rs.next()) {
                  String userid = rs.getString("user_id");
                  String pass = rs.getString("password");

            if(user_id.equals(userid) && cur_pass.equals(pass)) {

                  Statement s1 = c.createStatement();
                  s1.executeUpdate("UPDATE tch_data SET password='"+new_pass+"' WHERE user_id='"+user_id+"'");
                  JOptionPane.showMessageDialog(new view.AdminPrivacy(), "Password Succesfully Changed!", null, JOptionPane.INFORMATION_MESSAGE);

            }else {

                  JOptionPane.showMessageDialog(new view.AdminPrivacy(), "Error : Invalid Data.", "Error Message", JOptionPane.ERROR_MESSAGE);

            }   
        }

    } catch (Exception e) {
       e.printStackTrace();       
    }   
 }  

Answer 1

I fully agree with Erwin Bolwidt's answer, and it is IMHO the correct answer.

Since you also asked why you end up with messagedialogues --> because you're loading all users!!!

and your if-else block is wrong. if you're only changing/checking the password for one single user!

// make sure that it's the correct user 
if(user_id.equals(userid)) {
    // check if password was changed successfully
    if(cur_pass.equals(pass)) {
        // successful password change
    } else {
        // something went wrong with the password change
    }
} else {
    // this else is just to help you see your mistake
    // in your code you raised the error here!
}

Answer 2

You're retrieving all the rows in the database, for all the users, with your SQL query

Statement s = c.createStatement();
ResultSet rs = s.executeQuery("SELECT * from tch_data");

Of course your username and password do not match all the rows (since you'll be seeing all the users in your database in your loop) so you always get an error message for each row except for the one that has your user in it.

You should change the query to only return the row for the user that you're changing the password for. However that requires you to use a PreparedStatement. (If you simply used the user_id in the query for a regular Statement without escaping, you'd make yourself subject to SQL injection attacks. Note that also applies to the place where you update the password - you should also use a PreparedStatement for that, otherwise you'll be in for a nasty surprise when somebody changes his password to '; DROP TABLE tch_data; SELECT * FROM tch_data 'foobar or something along those lines)

So you should replace the above two lines with these 3 lines:

PreparedStatement st = c.prepareStatement("SELECT * from tch_data WHERE user_id = ?");
st.setString(1, user_id);
ResultSet rs = st.executeQuery();

Note that you've also forgotten to close your ResultSet, Statement and Connection. You should close all of them (but most importantly the Connection), otherwise you're leaking them and your application will quickly run out of resources.

Answer 3

Once the Password change has been successfully made, you should break out of the while loop using the break statement. The else statement within the while loop should also be removed and placed outside of the loop. A boolean flag should be set so as to determine whether or not a successful password change has taken place and that condition checked outside the loop, for example:

try {
    Connection c = DBConnection.dbconmethod();
    Statement s = c.createStatement();
    ResultSet rs = s.executeQuery("SELECT * from tch_data");
    boolean success = false;    // **** Added Code ****

    while(rs.next() && success == false) {
          String userid = rs.getString("user_id");
          String pass = rs.getString("password");

          if(user_id.equals(userid) && cur_pass.equals(pass)) {
              Statement s1 = c.createStatement();
              s1.executeUpdate("UPDATE tch_data SET password='"+new_pass+"' WHERE user_id='"+user_id+"'");
              success = true;   // **** Added Code ****
              JOptionPane.showMessageDialog(new view.AdminPrivacy(), "Password Succesfully Changed!", null, JOptionPane.INFORMATION_MESSAGE);
              break;  // **** Added Code ****
         }
     }
     // **** Added Code ****
     if (!success) {
         JOptionPane.showMessageDialog(new view.AdminPrivacy(), "Error : Invalid Data.", "Error Message", JOptionPane.ERROR_MESSAGE);
     }   
} catch (Exception e) { e.printStackTrace(); }