Closed Bug 921529 Opened 9 years ago Closed 9 years ago

[email] Make ActiveSync autodiscover auth failure non-fatal so that we still perform an ISPDB lookup


(Firefox OS Graveyard :: Gaia::E-Mail, defect)

Not set


(Not tracked)



(Reporter: asuth, Assigned: asuth)




(1 file)

On bug 914520 and with some discussion on bug 874348, we encountered a glitch with the server infrastructure as it relates to autoconfig. does implement ActiveSync autodiscover, but use of ActiveSync is a premium service while IMAP is free.  The ActiveSync autodiscover causes us to abort the autoconfig process before we get to looking up the ISPDB entry and so autoconfig fails (needelessly).

This bug is about fixing the general problem by making the failure non-fatal but hanging onto the error code if we don't get a more interesting failure involving IMAP.  For the specific problem we are addressing it by adding a local autoconfig entry to gaia in bug 914520 which can be easily uplifted to v1.1 without risk.
See Also: → 914520
Oh, and I am making us do this for both 401 (bad-user-or-pass) and 403 (not-authorized) failures.  We are getting a 403 in the case, which seems correct on the part of the server, but I'm not sure we can expect all servers out there to be as well behaved, so I think doing the same thing for a 401 failure is reasonable.  In terms of a risk profile, there isn't much added risk since everthing after our termination point from a 401 involves first talking to the mozilla-run ISPDB server via https.  All MITM/spoofing attacks would occur prior to that stage.
If we make 401 non-fatal, what happens when the actual problem is that the user entered their password wrong? We shouldn't make that flow worse just to help with a purely-hypothetical issue.
I'm flexible on the 401 front.  The basic implementation idea is that if we see a 'soft' failure like this, we save off the error code and keep it around unless we get a better result.  The case breakout goes like this, I think:

If we get a 401 on autodiscover but we successfully create an IMAP account, we're good there.

If we get a 401 on autodiscover but get a no-config-info, that's clear we should return 'bad-user-or-pass' since it suggests that the right answer/only answer we have is ActiveSync.

If we get a 401 on autodiscover but then the ISP database tells us to try IMAP and we get an error other than bad-user-or-pass, that's the weird but still somewhat obvious situation.  Error codes there could be[1]: 'bad-security', 'unresponsive-server', 'server-problem', 'server-maintenance'.  All of these are things that are not actually user-actionable, so we might as well just report the 'bad-user-or-pass' from ActiveSync.  It's possible that's not user actionable either, but I don't see any way to deal with that better other than overhauling our autoconfig process to be more like Thunderbird's where we have an intermediary stage where we tell them what we want to try and what it looks like it's possible to try.

The overhead of doing this for 401 is that we might take a few extra seconds to tell the user about the 'bad-user-or-pass' error for ActiveSync servers *in the cases where we don't have locally hosted XML files*.  This won't affect

1: It's conceivable there's a pathological gmail situation where we could actually get 'needs-app-pass' or 'imap-disabled' because a user using google-apps-for-domains has setup their own fake autodiscover server that references gmail's quasi-deprecated ActiveSync support.  In that case I think we can argue that the domain owner realllly wants ActiveSync and falling back to 'bad-user-or-pass' and ignoring 'needs-app-pass'/'imap-disabled' are the right way to go.
Comment on attachment 811349 [details]
Pointer to Github pull request:

This does both 401 and 403 and there's unit tests for the fallback logic I documented.  If you want I can drop it back to just 403, but I think the rationale I provided makes as much sense as things can make when ActiveSync is involved ;)
Attachment #811349 - Flags: review?(squibblyflabbetydoo)
Comment on attachment 811349 [details]
Pointer to Github pull request:

I'm a bit out of the loop on testing, but this looks good assuming the tests pass on your end (modulo my comment on GitHub/IRC). r+!
Attachment #811349 - Flags: review?(squibblyflabbetydoo) → review+
You need to log in before you can comment on or make changes to this bug.