480026 - auto config verify logon needs to be able to override cert dialog

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[David :Bienvenu]

The normal imap/pop3/smtp code overrides the invalid cert dialog via the msg window attached to the url. This doesn't work for the verifyLogon call that the auto config code makes, since it doesn't take a msgWindow.  Making it take a msgWindow would cause password prompts in the case of bad passwords (which wouldn't be a bad thing IMO) but that's a somewhat separate issue. What I can do is make the verifyLogon calls return the URI that gets run, and the caller can set their own notification callbacks on the channel - in the case of autoconfig, it will set the bad cert handler override.

Comment 1

[David :Bienvenu]

My mistake; it's the transport that needs the notification callbacks set, and you can't get from the uri to the transport. The channel might be a substitute, but I can't get there either. So I think I am going to need to pass in a msg window - if we so decide, I can suppress the password prompt an other way.

Comment 2

[David :Bienvenu]

Attachment: proposed fix

this patch makes two changes - pass in an nsIMsgWindow to verifyLogon, and have verifyLogon return the nsIURI that is run.

This should allow the autoconfig code to do what it wants, though I need to figure out a way of retrying the url in the case of accepted certificate exceptions.

Comment 3

[[e-mail]]

Comment on attachment 364003 [details] [diff] [review]
proposed fix

>     rv = RunPopUrl(aServer, url);
>+    if (aURL)
>+      url.forget(aURL);
>+  }
You can only return values on success. Does RunPopUrl always succeed? (Why are you returning the URL anyway? Maybe the bad cert handler should retry the detection if the certificate is accepted?)

Comment 4

[David :Bienvenu]

(In reply to comment #3)
> (From update of attachment 364003 [details] [diff] [review])
> >     rv = RunPopUrl(aServer, url);
> >+    if (aURL)
> >+      url.forget(aURL);
> >+  }
> You can only return values on success. Does RunPopUrl always succeed?

I'll fix that.

> (Why are you returning the URL anyway? 

This gives us the flexibility to poke at the url. For example, if I add an attribute to suppress password prompts to the url, I could set it after launching the url, since the password prompting doesn't happen until later.

Maybe the bad cert handler should retry the
> detection if the certificate is accepted?)

Yes, that's what's going to happen - I can't make the backend code do it, because the bad cert handler dialog is asynchronous, which it needs to be to get a chance to retrieve the bad cert...

Comment 5

[[e-mail]]

Comment on attachment 364003 [details] [diff] [review]
proposed fix

OK, so r+sr with me with the outparam not written on failure.

Comment 6

[David :Bienvenu]

fix, fix checked in, with the rv of RunPopUrl checked before doing the forget.

Comment 7

[David :Bienvenu]

It wouldn't be this patch, which is irrelevant for 2.0 since 2.0 doesn't have automatic account discovery/config, but bug 429843 would be very relevant.