<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/100873/">http://git.reviewboard.kde.org/r/100873/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 18th, 2011, 11:22 p.m., <b>David Faure</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Hmm, this whole block is inside "if (failedAuth>0)", so this means kio_ftp tried to log in, but without using the cached credentials?
Ah, probably because it tries anonymous first. Which kind of sucks on sites where you have recorded a password into kwallet...
So I'm not objecting this patch, feel free to commit, but I actually think a better fix would be "if cached credentials, then use them for the first attempt" (like kio_sftp does, I think). And then the code you touched (handling of failed auth) doesn't have to change.</pre>
</blockquote>
<p>On March 19th, 2011, 4:01 a.m., <b>Dawit Alemayehu</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Ah yes, you are right. What I did is completely wrong! Dunno what I was thinking, but I guess I saw an openPasswordDialog without cacheCachedAuthentication and thought... Well you get the point. Anyhow, I will change the code as you suggested here because it is exactly what I meant to accomplish with my patch.
Another thing that would even be more cool, and not just to this ioslave, is if cacheCachedAuthentication prompts the user to select which credential to use when there are mutiple stored for a given site. I guess I just added another item to my own TODO list! :)</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Actually I take it back. The original patch I posted with some minor modification is the correct patch.
1. The majority use case of the ftp ioslave is to access public ftp sites with anonymous login and as such the ioslave should always be optimized for that use case. To me it should not waste time looking up for cached credentials.
2. The user can circumvent the default optimization above by simply including a username when entering the ftp url. In that case, kio_ftp will never attempt the anonymous login. Instead it should lookup for any cached password or prompt and prompt the end user when it could not find one.
So in general here is what would/should happen:
1.) If the user types a ftp url without a username and the ftp server allows anonymous login, login anonymously.
2.) If the user types a ftp url without a username and the ftp server does not allows anonymous login, prompt the user.
3.) If the user types a ftp url with a username and the ftp server either allows or does not allow anonymous login, use cached authentication if available ; prompt the user otherwise.
Other rare scenarios such as what to do in the presence of multiple logins for a site when the url is specified without a username, is something each individual ioslave should not have to deal with IMHO. Instead as I suggested in my prior response, it should be handled at the kpasswordserver level when checkCachedAuthentication is invoked. Perhaps by prompting the user to choose one from a list of cached usernames ?? Anyhow, if there are no objections to this approach, I will repost the original patch with few modifications to take care of additional login related bugs.
</pre>
<br />
<p>- Dawit</p>
<br />
<p>On March 19th, 2011, 4:37 a.m., Dawit Alemayehu wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.orgrb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for kdelibs.</div>
<div>By Dawit Alemayehu.</div>
<p style="color: grey;"><i>Updated March 19, 2011, 4:37 a.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">The attached patch forces kio_ftp to first look up for cached password before showing the user a prompt to enter password information. This addresses the issue mentioned in 143488 and makes kio_ftp's behavior consistent with the other ioslaves.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Login to an ftp site, save the password and visit the same site again. No prompt.
</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=124675">124675</a>,
<a href="http://bugs.kde.org/show_bug.cgi?id=143488">143488</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>kioslave/ftp/ftp.cpp <span style="color: grey">(95c4450)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/100873/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>