PATCH: Fix most of the login issues with the FTP ioslave...

Review Request #101173 - Created April 22, 2011 and submitted

Dawit Alemayehu
99686, 124675, 143488, 258888
The attached patch addresses most of the FTP login related problems and is a replacement for the previous review request Here are all the changes in this patch:

- Show the "Remember password" checkbox even after the failure of the first login attempt. [Bug:258888]
- Always check for cached password before trying to login anonymously unless the "TryAnonymousLoginFirst"
  flag was set in kio_ftprc. [Bug: 99686, 143488, 124675]
- Avoid sending the "anonymous" username so it will not be used in the key used to store the password in kwallet.
- When a url contains a username, but the user chooses to login with a different username in the password dialog, 
  then use redirection to update the client of the change.
- Store password information in persistent storage if and only if the user checked the "Remember password" checkbox.
- Attempt to login with incorrect username and validate the "Remember password" is actually shown again.
- Corrected the username information from the password dialog to ensure the client is updated properly about the password change.
- Clicked on the "Remember password" to store password in persistent storage and retry logging into the same server at a later point.
Dawit Alemayehu
Dawit Alemayehu
David Faure
Dawit Alemayehu
Review request changed

Change Summary:

Removed the m_bUserNameChanged flag and replaced it with a parameter in ftpLogin.


Revision 4 (+86 -34)

Show changes

David Faure

kioslave/ftp/ftp.cpp (Diff revision 4)
Sorry, I still have a question.

After a username change, setHost is going to be called, right? So it will disconnect kio_ftp from the host?

In that case, what's the point in doing the redirection code in this method - I thought the point was to do it after connected(), but that can't be it.
So it still seems to me that the redirection code could be done inside of ftpLogin, which would simplify the code quite a lot (no bool *, in particular).

But I guess I'm missing something; maybe setHost isn't called, after a redirection, so the app simply reuses the already-connected slave? In that case I don't see a better solution, indeed.
  1. #1. After the user name changed redirection, setHost will be called. But why do you think kio_ftp will be disconnected from the server ? If that is because of the closeConnection() call in setHost, then rest assured that it will not be called. Why ? Because "m_user" has already been updated to the new username and since nothing else will change, the if statement in setHost will never be true.
    #2. We want the redirection to be the same as any other redirection. The only difference being we only want to update the client with the new user name. As per response in #1, the ioslave already has updated its local copy of the user name (m_user) ; so eveything else is exactly the same as the other redirection.
    If you attempt to move the redirection to ftpLogin, you have to throw away your successful connection to the server, because you have to return false, i.e. not logged in to ftpOpenConnection.
David Faure
OK, I understand now. Thanks!
Commit Hook
This review has been submitted with commit 3ab6c7f05f113353435e10b6d6fd0fdbbd495855 by Dawit Alemayehu.