Re: [Evolution-hackers] Camel Authentication Changes
On Thu, 2011-10-20 at 12:50 -0400, Matthew Barnes wrote:
>
> I think we could achieve this with multiple calls to
> camel_session_authenticate_sync() with a different mechanism name each
> time ("kerberos", "ntlm", "login"), and making sure the session logic
> knows when to loop and when to bail out if an auth attempt fails.
>
> It already kinda does that by looking up the CamelServiceAuthType struct
> associated with the mechanism name (if one was given) and checking its
> 'need_password' flag. If FALSE, it does ONE authentication attempt and
> bails on failure (instead of looping). After which you could possibly
> call camel_session_authenticate_sync() again with a fallback mechanism
> name.
>
> Problem is when a mechanism name is NOT given, as is the case for most
> HTTP-based backends, or when 'need_password' is TRUE, it assumes you'll
> *will* need a password to authenticate and prompts if one is not found
> in the keyring (which I understand now to be a bad assumption). That's
> the part that needs tweaking, for now.
Yeah, the provider could just call camel_session_authenticate_sync() as
and when it actually *needs* a password (which looks fairly much to be
what imapx *is* doing, so I'm not really sure why it's demanding my
password unnecessarily).
The issue is the complexity. What happens is that the provider calls out
to camel_session_authenticate_sync(), which then calls back to the
provider to give it the information it wanted... but it provides it in a
*synchronous* method and waits for a result, so we can't just hand off
the password with a GAsyncResult to the code which called
camel_session_authenticate_sync() in the first place; we have to do
nasty things with threads and avoid deadlock while we make our
provider's ->authenticate_sync() actually wait for the completion of the
function which *called* it.
Forget EWS for now; let's just look at it in imapx. This bit in
imapx_create_new_connection(), for example:
/* XXX As part of the connect operation the CamelIMAPXServer will
* have to call camel_session_authenticate_sync(), but it has
* no way to pass itself through in that call so the service
* knows which CamelIMAPXServer is trying to authenticate.
*
* IMAPX is the only provider that does multiple connections
* like this, so I didn't want to pollute the CamelSession and
* CamelService authentication APIs with an extra argument.
* Instead we do this little hack so the service knows which
* CamelIMAPXServer to act on in its authenticate_sync() method.
*
* Because we're holding the CAMEL_SERVICE_REC_CONNECT_LOCK
* (and our own CON_LOCK for that matter) we should not have
* multiple IMAPX connections trying to authenticate at once,
* so this should be thread-safe.
*/
imapx_store->authenticating_server = g_object_ref (conn);
success = camel_imapx_server_connect (conn, cancellable, error);
g_object_unref (imapx_store->authenticating_server);
Then in imapx_reconnect() you call camel_session_authenticate_sync()
which ends up calling *back* to imapx's imapx_authenticate_sync()
method... which uses that nastily-stashed 'server' object... which was
RIGHT THERE in the imapx_reconnect() function. We could have just
returned the password to imapx_reconnect() and all would have been well.
I'm unlikely to get much done before I disappear to Prague on Saturday,
and I'm on baby duty tomorrow because she's unwell and can't go to
nursery, but I'll try to put together a patch which sorts this out.
Maybe during next week.
--
dwmw2
___
evolution-hackers mailing list
evolution-hackers@gnome.org
To change your list options or unsubscribe, visit ...
http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] Camel Authentication Changes
On Thu, 2011-10-20 at 01:14 +0100, David Woodhouse wrote:
> This looks like a good idea, and cleaning up the various ways that the
> providers were behaving differently in that authentication loop (and
> calling directly out to the GUI) is a good thing.
>
> I looked at those recently when implementing the ->try_empty_password()
> SASL methods for automatic NTLM auth, and I take my hat off to you for
> finding a way through the mess.
>
> Having said that, I do think we need to adjust this approach to deal
> with authentication as a *reactive* thing, not *proactive*. It *used* to
> be reactive, but you changed it as a side-effect of your improvements.
>
> The point is that we authenticate ourselves *if* and *when* the server
> asks us to. We cannot know in advance whether we're going to need a
> password or not.
We discussed this at length on IRC last night and I'm still digesting
all the implications of auth cases I hadn't considered. I'm hoping
that, at least for the time being, we can get by with the current API
with only some tweaks to the authentication loop logic.
Down the road I'm perfectly happy to revisit this, and even break APIs
if needed to do it right, but right now Red Hat is pressuring me to get
this account rewrite branch of mine finished as quickly as possible. So
for now I really have to move on from this.
> That's fine at the moment, but the fact that we have to explicitly
> choose *one* mechanism in advance ought to be considered a bug. In the
> future, we ought to simply try *any* of the methods that the server
> offers today, in some order of preference. So we could try Kerberos,
> fall back to NTLM, etc. These changes seem to have made that harder to
> fix.
I think we could achieve this with multiple calls to
camel_session_authenticate_sync() with a different mechanism name each
time ("kerberos", "ntlm", "login"), and making sure the session logic
knows when to loop and when to bail out if an auth attempt fails.
It already kinda does that by looking up the CamelServiceAuthType struct
associated with the mechanism name (if one was given) and checking its
'need_password' flag. If FALSE, it does ONE authentication attempt and
bails on failure (instead of looping). After which you could possibly
call camel_session_authenticate_sync() again with a fallback mechanism
name.
Problem is when a mechanism name is NOT given, as is the case for most
HTTP-based backends, or when 'need_password' is TRUE, it assumes you'll
*will* need a password to authenticate and prompts if one is not found
in the keyring (which I understand now to be a bad assumption). That's
the part that needs tweaking, for now.
> It looks like it'll always ask for a password for HTTP-based backends,
> but those have similar options; they may authenticate with Kerberos, or
> with automatic-NTLM, or with cookies... or they *may* actually decide
> that they need a password today.
Right. I've made similar changes to the addressbook and calendar
backends on my branch, more of which are HTTP based, and what I did
there was first try to run some initial inconsequential operation ("list
calendars" or some such) with no auth header at all, and if that fails
with a "401 Unauthorized" *then* initiate authentication in a similar
manner to camel_session_authenticate_sync().
I realize that doesn't cover all the use cases you were throwing at me,
particularly the server password suddenly changing in the middle of a
session, but perhaps that approach, in combination with the tweaking I
mentioned above, would address the premature password prompt in EWS for
the time being?
___
evolution-hackers mailing list
evolution-hackers@gnome.org
To change your list options or unsubscribe, visit ...
http://mail.gnome.org/mailman/listinfo/evolution-hackers
