GNOME Bugzilla – Bug 114572
Ftp module needs authorization callback
Last modified: 2004-12-22 21:47:04 UTC
The ftp module does not have an authorization callback so there is no way to pop up a dialog for user name and password when an application accesses a password protected ftp site using gnome-vfs.
Created attachment 17238 [details] [review] Patch provided
All this caching of authorization info should be handled at a higher level instead of being duplicated in each method. I guess a patch like this one can be used as a temporary solution. For what it's worth, it's a dupe of #44346, but I don't want to close it since it has a patch
+ *user = g_strdup (out_args.username); + *pass = g_strdup (out_args.password); +} I think you are leaking out_args.username and out_args.password here In authn_cache_add + if (g_hash_table_lookup (authn_cache, uri) != NULL) { + /* replace */ + g_hash_table_insert (authn_cache, uri, info); + } + else { + /* insert */ + g_hash_table_insert (authn_cache, + gnome_vfs_uri_dup (uri), + info); + } As I understand glib's API doc, in the "replace" case, uri will be unref'ed which is probably not desirable. Since you provided the appropriate destroy functions when you created the hash table, you can just use gnome_vfs_ref (uri); g_hash_table_insert (authn_cache, uri, info); and get rid of this if statement. + authn_cache_remove (conn->uri); + The authorization tokens in the cache should be refcounted, several ftp connections may be established with the same server at the same time. If you remove the authorization info as soon as one connection ends, the user may get unexpected authentication dialogs. That probably doesn't matter that much, since that won't happen really often, but I think that can still happen. Will the user always be asked for a login/password if he only types ftp://some.ftp.site.com in nautilus ? Imo this shouldn't happen, when the user has given no login and no password, the method should check in its cache, use the info it founds there if there is something, otherwise it should try to login as anonymous, and only asks for a password if it fails
auth caching is a problem with the current vfs implementation. There is no guarantees about the context the vfs is used in, so we can't use timeouts etc to clear caches after a while. In the future we will solve this with a gnome-vfs daemon, but for now i think caching password for the runtime of the app is fine.
*** Bug 44346 has been marked as a duplicate of this bug. ***
Laszlo, what's up with this patch? Are you planning to attach an updated version/comment on the bug when you have some time ?
Created attachment 24058 [details] [review] patch using keyring
I attached a new patch that uses the Gnome keyring. I know this can't go into 2.6 because of feature freeze, but I would still appreciate your opinion. Also it has been mentioned that first an anonymous login should be tried if that does not work then the dialog displayed. I don't think this is how it should work because it would mean that if anonymous is allowed then the user can only log in as anonymous since the dialog will never be displayed.
Some issues with this patch: If you enter nothing in the user/password it should default to the old anon user/password. There can be several connections for each uri [server and user as specified in the uri]. Each call to ftp_connection_acquire can open a new one. Once we've decided on a password for the connection we should never ask for it again, at least while one connection to the server still exists. (However ftp://ftp.foo.com and ftp://user@ftp.foo.com should now use the same password.) Also, we need to timeout the spare connections. At the moment they are kept open forever. I agree on the don't-default-to-anon. (Although teuf doesn't.)
s/should now use the same password/should not use the same password/
The only thing I tried to address here was to provide a password request dialog for ftp. Issues related to ftp_connection_acquire() and connection time-out were not planned to be addressed by this patch and they should be fixed qas a separate bug. Or am I missing something? As far as the anonymous login is concerned I don't like guessing what the user means. If we assume that no username/password in the dialog means anonymous then we either have to store this in the keyring or not. If we don't then we will be popping up dialogs forever, if we do and the user pressed it by accident then the launcher will be unusable for them and they need to create a new one (and it will be unclear for them what happened). Actually we used a more sophisticated UI for launcher creation in JDS and I will probably want to include something like that in HEAD. This had a drop-down list with protocols (ftp, http etc), a textfield for the server and path on the server, separate textfield for username, for ftp it had a tickbox called anonymous as well (we intentionally did not include a password textfield here). If the user ticked the anonymous tickbox then an anonymous launcher was created, if not then a textfield became active for the username to be provided. In a layout like this it is much easier to make sure that we have what the user intended. Also we might consider changing the password request window in gnome-keyring for ftp to mirror the above behaviour. We could have an anonymous tickbox as well that is ticked if the user created an anonymous launcher (in this case the user/password textfields could be insensitive). If the launcher is not anonymous then the tickbox is not ticked by default. This way we would definitely know what the user wants to happen.
If you don't reuse old passwords in ftp_connection_acquire() the user will have to re-enter the password each time another parallel access to the ftp site is done (unless the user used the keyring). Thats not acceptable authentication behaviour. Most users of ftp will use anonymous logins. Forcing everyone in the world to learn that you must type anonymous in the user field is a non-starter. All use of ftp servers isn't from launchers, so we can't fix everything from there. Anyway, this is how the smb backend already works, and how most ftp clients work. Of course we would have to store the anonymous name in the keyring if the user did so. I think its pretty uncommon that the user by mistake types nothing in the dialog, but selects add to keyring, and if that happens they can use the (to-be-written) keyring management ui to fix it up.
*** Bug 136014 has been marked as a duplicate of this bug. ***
*** Bug 136138 has been marked as a duplicate of this bug. ***
*** Bug 121957 has been marked as a duplicate of this bug. ***
*** Bug 140405 has been marked as a duplicate of this bug. ***
*** Bug 129215 has been marked as a duplicate of this bug. ***
*** Bug 142978 has been marked as a duplicate of this bug. ***
*** Bug 143406 has been marked as a duplicate of this bug. ***
We had a lengthy discussion about this on nautilus-list and we agreed on a dialog design and behaviour. I attached the dialog and the two patches (one for gnome-vfs and another one for libgnomeui) that implement the behaviour. Please review.
Created attachment 28597 [details] [review] gnome-vfs patch
Created attachment 28598 [details] [review] libgnomeui patch
Created attachment 28599 [details] ftp dialog
This is now in, i started from your patch, and then basically rewrote large parts of the ftp method to make it work better. Some things i noted about your patch: Different, shorter strings in the password dialog. You forgot to mark one string for translation. The dialog returned true for anon, even if anon support wasn't turned on. + + GnomeVFSModuleCallbackFullAuthenticationOutFlags out_flags; /* Reserved "padding" to avoid future breaks in ABI compatibility */ - void *reserved1; Breaks ABI on 64bit. I changed out_flags to be a gsize. + *user = g_strdup ("anonymous"); + *pass = g_strdup ("");; some ftp servers require a valid email address Don't fail the login in the keyring password didn't work. instead ask. + g_free (keyring); + return GNOME_VFS_ERROR_ACCESS_DENIED; + } Use LOGIN_FAILED instead You need to cache username/password even if the keyring isn't used, since we open new connections now and then. If there is no authentication callbacks, we should do anonymous logins. Plus some indentation issues. Additionally i added dir listing caching and made symlinks work.