When username is given in smb URI, its processed in initial_authentication().
While prompting for authentication details in prompt_authentication() function,
if the URI has "user_name" field, then authentication dialog will not ask for
username and domain. User will be asked for password only. (user_name field of
URI contains both username and domain, which are processed in
initial_authentication()).

 if (!actx->uri || ((GnomeVFSToplevelURI*)actx->uri)->user_name == NULL) {
   in_args.flags |=    
      GNOME_VFS_MODULE_CALLBACK_FULL_AUTHENTICATION_NEED_USERNAME |            
                  
      GNOME_VFS_MODULE_CALLBACK_FULL_AUTHENTICATION_NEED_DOMAIN;
 }

Similarly, if only domain is given in uri (smb://domain;@machine), then also,
username is not prompted for. 

I will submit a patch which takes care of all the situations. It does the following:

a) While processing URI in initial_authentication(), check if username or domain
is present and set appropriate flags. In prompt_authentication(), after checking
for the availability of username or domain name from URI, prompt for the
remaining details from user. I added two boolean variables to SmbAuthContext
struct for this purpose. I feel this is clean way of knowing if username or
domain is part of URI, at a later point.

b) Once above change is made, its required to change the way user details are
read from user_cache in lookup_user_cache(). If URI has both username and
domain, then code will not look into user cache. Else, it will look.

Created attachment 39708 [details] [review]
Proposed patch

Nielsen, ping :)

I have two points.

1. Why the change in lookup_user_cache? The change from || to && means that
it'll only fail when *both* the username and the domain don't match. You explain
it above, but I don't understand.

2. Instead of adding booleans to the auth context, could you add a
'callback_flags' member, which then would contain the necessary *_NEED_* flags
(parsed and set in inital_authentication).

Narayana: I had some query on this part of the patch,

+	} else if (!actx->uri_has_user) {
+		in_args.flags |= GNOME_VFS_MODULE_CALLBACK_FULL_AUTHENTICATION_NEED_USERNAME;
+	} else if (!actx->uri_has_domain) {
+		in_args.flags |= GNOME_VFS_MODULE_CALLBACK_FULL_AUTHENTICATION_NEED_DOMAIN;

When actx->uri_has_user and actx->uri_has_domain, both would be false, then
in_args.flags would not get the
GNOME_VFS_MODULE_CALLBACK_FULL_AUTHENTICATION_NEED_DOMAIN flag.

Could it be written like this instead,

	} else {
                if (!actx->uri_has_user)
		       in_args.flags |=
GNOME_VFS_MODULE_CALLBACK_FULL_AUTHENTICATION_NEED_USERNAME;
	        if (!actx->uri_has_domain)
		       in_args.flags |= GNOME_VFS_MODULE_CALLBACK_FULL_AUTHENTICATION_NEED_DOMAIN;
        }

Nielsen,
1. The change to lookup_user_cache() is needed because, "with_user" flag will be
true if username or domain name or both are present in smb uri. Say "with_user"
is true because, only the user name is present in uri. The condition will
succeed and function returns FALSE (that means there is no domain name filled
and checks in its caller fail). So, if "with_user" is true, then we must return
FALSE only when both "user_name" and "domain_name" match with the values in
cache. Else, lets get them from cache.

2. I made the changes suggested by introducing a flag. Attached patch has the
suggested changes. 

Kaushal: 
If both uri_has_name and uri_has_domain are FALSE, the user_name field of smb
uri will be NULL and the 'if' part of code will be executed (i.e both username
and domain will be asked). So, its fine. Note that user_name field of smb uri is
for both user name and domain name in the form "domain;user".

Created attachment 46052 [details] [review]
modified patch

Created attachment 46062 [details] [review]
Cleaned up patch further

I've cleaned up the patch further. About the lookup_user_cache code...

The whole point of that check in lookup_user_cache is that if *either* the user
or domain is specified (ie: with_user), and *either* don't match what's in the
cache (ie: the !string_compares) then we can't use the cache, and we'll need to
prompt. 

In other words what's in the cache has a different user name or domain, and
doesn't match what we're being asked to do in the URI. So we can't use the
cache, and need to prompt for the missing information. 

Does that make sense? Or am I missing something.

The change to lookup_user_cache() is required in this case: 
When username is there in smb:// uri, user details are cached when a server is
authenticated. Now, accessing a share under the server, would prompt for
authentication once again (instead of using the cache from user_cache), if
condition is "||". This is true in case the server is part of a windows domain
and domain name is required for authentication to succeed. I will attach debug
output with the condition having "&&" and "||", for better understanding of the
issue and why the condition should be "&&".

The patch modified by you works fine except for the change in
lookup_user_cache(). If the condition is modified to "&&", the patch can be
committed. I have tested it.

Created attachment 46082 [details]
debug output of user cache access when username is given in uri

Okay, I understand the problem now. But simply hacking lookup_user_cache to
return the right result for this particular problem causes other use cases to fail. 

A better approach would be that when a user but no domain is specified, in
initial_authentication we could do a lookup and figure out an appropriate domain
for accessing that server. 

Or another approach would be to have lookup_user_cache do appropriate checks
against either the just the user or both the user name and domain, depending on
what GNOME_VFS_MODULE_CALLBACK_FULL_AUTHENTICATION_NEED_* flags are present.

Created attachment 46295 [details] [review]
Patch with proposed changes

Does this work for you?

Yes, the patch works fine.

Thanks.

Should we get this in then if it works?

Committed. BTW, thanks for figuring this out.

2005-05-16  Nate Nielsen  <nielsen@memberwebs.com>

	* smb-method.c 
	(lookup_user_cache, initial_authentication, prompt_authentication): 
	Fix problems with prompting for authentication twice. 
	Patch by Narayana Pattipati. Fixes bug #172695.