Closed
Bug 1231642
Opened 8 years ago
Closed 8 years ago
Log in to Mail.Ru (IMAP/SMTP) using OAuth
Categories
(MailNews Core :: Networking, enhancement)
Tracking
(thunderbird45+ fixed, thunderbird46 fixed)
VERIFIED
FIXED
Thunderbird 46.0
People
(Reporter: monolithed, Assigned: rkent)
References
(Blocks 2 open bugs)
Details
(Keywords: feature, user-doc-needed)
Attachments
(1 file)
2.04 KB,
patch
|
rkent
:
review+
rkent
:
approval-comm-aurora+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/47.0.2526.73 Safari/537.36
Reporter | ||
Updated•8 years ago
|
Severity: normal → enhancement
Keywords: feature,
user-doc-needed
OS: Unspecified → All
Hardware: Unspecified → All
Reporter | ||
Comment 1•8 years ago
|
||
1. Add configuration to the `comm-central` (comm-central/mailnews/base/util/OAuth2Providers.jsm) > diff --git a/mailnews/base/util/OAuth2Providers.jsm b/mailnews/base/util/OAuth2Providers.jsm > --- a/mailnews/base/util/OAuth2Providers.jsm > +++ b/mailnews/base/util/OAuth2Providers.jsm > @@ -10,32 +10,41 @@ var EXPORTED_SYMBOLS = ["OAuth2Providers > var {classes: Cc, interfaces: Ci, results: Cr, utils: Cu} = Components; > > // map of hostnames to [issuer, scope] > var kHostnames = new Map([ > ["imap.googlemail.com", ["accounts.google.com", "https://mail.google.com/"]], > ["smtp.googlemail.com", ["accounts.google.com", "https://mail.google.com/"]], > ["imap.gmail.com", ["accounts.google.com", "https://mail.google.com/"]], > ["smtp.gmail.com", ["accounts.google.com", "https://mail.google.com/"]], > + > + ["imap.mail.ru", ["o2.mail.ru", "mail.imap"]], > + ["smtp.mail.ru", ["o2.mail.ru", "mail.imap"]], > ]); > > > var kIssuers = new Map ([ > ["accounts.google.com", [ > '406964657835-aq8lmia8j95dhl1a2bvharmfk3t1hgqj.apps.googleusercontent.com', > 'kSmqreRr0qwBWJgbf5Y-PjSU', > 'https://accounts.google.com/o/oauth2/auth', > 'https://www.googleapis.com/oauth2/v3/token' > ]], > + ["o2.mail.ru", [ > + 'thunderbird', > + 'I0dCAXrcaNFujaaY', > + 'https://o2.mail.ru/login', > + 'https://o2.mail.ru/token' > + ]], > ]); 2. Add ISP configuration to the Mozilla autoconfig > Index: trunk/mail.ru > =================================================================== > --- trunk/mail.ru (revision 148636) > +++ trunk/mail.ru (working copy) > @@ -13,6 +13,7 @@ > <port>993</port> > <socketType>SSL</socketType> > <username>%EMAILADDRESS%</username> > + <authentication>OAuth2</authentication> > <authentication>password-cleartext</authentication> > </incomingServer> > <incomingServer type="imap"> > @@ -41,6 +42,7 @@ > <port>465</port> > <socketType>SSL</socketType> > <username>%EMAILADDRESS%</username> > + <authentication>OAuth2</authentication> > <authentication>password-cleartext</authentication> > </outgoingServer> > <outgoingServer type="smtp">
Assignee | ||
Comment 2•8 years ago
|
||
We hated hard-coding those Google OAuth settings, but at the time there was no easy alternative. I suppose we are going to have to face the situation with other providers. What is the current status of OAuth with Mail.ru? Specifically, is it required or otherwise strongly encouraged? What is the current user experience without OAuth? IF we were to support Mail.ru we would need to create a new app key and secret that used our thunderbird-accounts email address, not whatever you have here (which I assume you created yourself).
Reporter | ||
Comment 3•8 years ago
|
||
@Kent James > What is the current status of OAuth with Mail.ru? We already use OAuth as our primary protocol and it is strongly recommended to our clients. Please see our documentation https://cloud.mail.ru/public/Mk6L/MtC8EcgBL > What is the current user experience without OAuth? login / password > IF we were to support Mail.ru, we would need to create a new app key and secret that used our thunderbird-accounts email address, not whatever you have here (which I assume you created yourself). Nope. We created these keys specifically for this application.
Assignee | ||
Comment 4•8 years ago
|
||
OK some questions. 1) Your language implies that you are officially representing Mail.ru here. Is that true? If so, how can I verify that? 2) Have you tried the changes that you suggest in a custom build of Thunderbird, and confirmed that it works?
Assignee | ||
Updated•8 years ago
|
Status: UNCONFIRMED → NEW
Component: Untriaged → Networking
Ever confirmed: true
Product: Thunderbird → MailNews Core
Reporter | ||
Comment 5•8 years ago
|
||
> 1) Your language implies that you are officially representing Mail.ru here. Is that true? If so, how can I verify that? It's true. We can continue our communication via a.abashkin@corp.mail.ru if you see fit. > 2) Have you tried the changes that you suggest in a custom build of Thunderbird, and confirmed that it works? Yes, of course we have a custom build of Thunderbird. We had to make some changes in our OAuth-implementation. I will try to test and report you on next week.
Assignee | ||
Comment 6•8 years ago
|
||
OK at least from my perspective, this all sounds reasonable. But the whole OAuth implementation that we did was a little controversial internally, as there are differences of opinions about whether OAuth is a good thing to be promoting. Certainly the hard-coding of support for only GMail was not optimal but done to be pragmatic. There were valid criticisms that doing that would force other providers who offer OAuth to also require hard-coding, as we are seeing here with your request. Still, we have no intention of offering preferential support to GMail, so we should reasonably accept requests from Mail.ru to support their OAuth implementation. Given the current implementation, adding support for Mail.ru in the code seems reasonable. Timing wise, we are getting close to our next major release, which is TB 45 due in March 2016. But actually the first code freeze for that occurs next week. But there is nothing in the proposed changes here that would be difficult to uplift during the aurora and beta cycles, but still ideally we would get this done within six weeks so that the Mail.ru OAuth support is included in the betas for TB 45 that we will start to ship around the end of January. The changes in the core code should ideally be posted here as a patch rather than described. Could you do that, and ask for my review? For the changes to the ISP database, that requires a separate bug, but I don't see any obstacles to that moving forward quickly once the core code changes are available. I would also want to test this implementation. Can I just go to Mail.ru and create an account that would allow me to do that, or do you need to provide an account with appropriate credentials? Thanks for working to support Thunderbird better with Mail.ru, we do appreciate that.
tracking-thunderbird45:
--- → +
Reporter | ||
Comment 7•8 years ago
|
||
Attachment #8697982 -
Flags: review?(rkent)
Reporter | ||
Comment 8•8 years ago
|
||
> The changes in the core code should ideally be posted here as a patch rather than described. Could you do that, and ask for my review?? Sure, but we had to make some changes in our OAuth-implementation to suppress "state" parameter. We have to make the release by the end of this week. I'll let you know when it will be ready. > I would also want to test this implementation. Can I just go to Mail.ru and create an account that would allow me to do that, or do you need to provide an account with appropriate credentials? Test credentials: test-thunderbird-oauth@mail.ru / m0zillA Or you can create an account at https://e.mail.ru/signup?lang=en_US Thanks!
Reporter | ||
Comment 9•8 years ago
|
||
@Kent James, we are ready. Could you review?
Comment 10•8 years ago
|
||
I'm mildly disappointed that we've had to hardcode credentials for gmail, and I worry about having to add more for any other site. I don't like the precedence that this is setting. I would much rather that sites implement RFC 7591 and RFC 7628 (that's OAuth dynamic client registration and the SASL OAUTHBEARER).
Assignee | ||
Comment 11•8 years ago
|
||
(In reply to Joshua Cranmer [:jcranmer] from comment #10) > I'm mildly disappointed that we've had to hardcode credentials for gmail, > and I worry about having to add more for any other site. I don't like the > precedence that this is setting. > > I would much rather that sites implement RFC 7591 and RFC 7628 (that's OAuth > dynamic client registration and the SASL OAUTHBEARER). Yes I thought you might be skeptical, and my comment 6 was largely directed at that issue. One request per year is not much of a burden, particularly when the provider has done the majority of the work. I really think that for now we need to be pragmatic. We have no dynamic client registration to test against, so we do not really support that ourselves. It is unreasonable to preferentially support gmail over mail.ru when mail.ru has a similar request. If and when a provider supports dynamic site registration and we implement that support, then we can start demanding of mail providers that they support it if they want OAuth2 to be included in Thunderbird for their domain. We might at some point also put some restriction on the size of provider we will hardcode, but Mail.ru would reasonably meet any requirements of sufficient scale. And for the record, the support of GMail Oauth2 has gone quite smoothly with this implementation as far as I can tell.
Assignee | ||
Comment 12•8 years ago
|
||
I can confirm that email operations are successful using the provided changes, including both IMAP as well as SMTP, when the configuration is manually switched to OAuth2. You'll have to admin, Joshua, that consolidating the credentials in a single file like that makes the required changed to support a new provider dead easy from our perspective! Alexander, the login screen displays in English, but the next page is in Russian (which I can read by the way). I don't know how important support of non-Russian readers is for you, but that is less than optimal. Is there any way to fix that? Also, you mentioned that there were some changes on your server required for this to work. Is there anything that we could do to resolve whatever that issue was, so that we don't have this issue in the future?
Reporter | ||
Comment 13•8 years ago
|
||
> Alexander, the login screen displays in English, but the next page is in Russian (which I can read by the way). I don't know how important support of non-Russian readers is for you, but that is less than optimal. Is there any way to fix that? The reason why you are getting Russian language after username\password check is because Russian is the language of your mailbox (you may change it in mailbox settings). Language logics is following: 1. check GET lang parameter and use it 2. if there is no lang parameter use mailbox language settings(your case) 3. if none of the first two then use browsers language > Also, you mentioned that there were some changes on your server required for this to work. Is there anything that we could do to resolve whatever that issue was, so that we don't have this issue in the future? Nope, everything was resolved. Thank you for your help.
Assignee | ||
Comment 14•8 years ago
|
||
Joshua, I intend to r+ this and ultimately push it for TB 45 unless you want to object. But be prepared for some strong counter-objections from me.
Flags: needinfo?(Pidgeot18)
Comment 15•8 years ago
|
||
(In reply to Kent James (:rkent) from comment #14) > Joshua, I intend to r+ this and ultimately push it for TB 45 unless you want > to object. But be prepared for some strong counter-objections from me. I'm fine with this going in, but on one major caveat: I desperately want to see this static list disappear, and the way to make this disappear is dynamic client registration. This has already achieved RFC status (see the RFCs reach above). Once I can see servers implementing this, I'm planning on ripping the list out, perhaps even if not all the providers have implemented dynamic client registration yet. So, to Abashkin Alexander, if you're working with the mail.ru team, could you perchance help convince them to implement dynamic client registration? I would be more than happy to prototype a client implementation of the technology if that poses a significant stumbling block to implementing it on the server side.
Flags: needinfo?(Pidgeot18)
Assignee | ||
Comment 16•8 years ago
|
||
Comment on attachment 8697982 [details] [diff] [review] 1231642.patch Review of attachment 8697982 [details] [diff] [review]: ----------------------------------------------------------------- As discussed, I'll approve and push this.
Attachment #8697982 -
Flags: review?(rkent) → review+
Assignee | ||
Comment 17•8 years ago
|
||
https://hg.mozilla.org/comm-central/rev/625d871a9669 Alexander, could you answer Joshua's question in comment 15 about whether you would be willing to work with him on a dynamic registration prototype?
Assignee: nobody → duthen
Status: NEW → RESOLVED
Closed: 8 years ago
Flags: needinfo?(duthen)
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 46.0
Version: 45 → 46
Reporter | ||
Comment 18•8 years ago
|
||
(In reply to Joshua Cranmer [:jcranmer] from comment #15) > > So, to Abashkin Alexander, if you're working with the mail.ru team, could > you perchance help convince them to implement dynamic client registration? I > would be more than happy to prototype a client implementation of the > technology if that poses a significant stumbling block to implementing it on > the server side. (In reply to Kent James (:rkent) from comment #17) > https://hg.mozilla.org/comm-central/rev/625d871a9669 > > Alexander, could you answer Joshua's question in comment 15 about whether > you would be willing to work with him on a dynamic registration prototype? I apologize for the late reply, I had a vacation. Yes, we have such plans. When? I can't say right now.
Flags: needinfo?(duthen)
Assignee | ||
Comment 19•8 years ago
|
||
Comment on attachment 8697982 [details] [diff] [review] 1231642.patch [Triage Comment] This is a very simple change to support OAuth for Mail.ru, and should be available in TB45.
Attachment #8697982 -
Flags: approval-comm-aurora+
Comment 20•8 years ago
|
||
Aurora: https://hg.mozilla.org/releases/comm-aurora/rev/51c54450105d
status-thunderbird45:
--- → fixed
status-thunderbird46:
--- → fixed
Assignee | ||
Comment 21•8 years ago
|
||
Alexander, there is now a beta out that supports this. The autoconfig database though does not currently specify this (see https://autoconfig.thunderbird.net/v1.1/mail.ru) Are you planning on filing a bug there to add the OAuth fields to this? Or is there an existing bug?
Flags: needinfo?(monolithed)
Reporter | ||
Comment 22•8 years ago
|
||
Kent, As far as I understand, we have to specify ISP configuration after the current release. Please see my patch https://bug1253196.bmoattachments.org/attachment.cgi?id=8726130
Flags: needinfo?(monolithed)
Reporter | ||
Updated•8 years ago
|
Flags: needinfo?(rkent)
Comment 23•8 years ago
|
||
Raising the discussion here instead of on bug 1253196 for changing the ISPDB: Should/can mail.ru self-host the OAuth-declaring autoconfig file (as described at https://developer.mozilla.org/en-US/docs/Mozilla/Thunderbird/Autoconfiguration#Configuration_server_at_ISP)? I'm not a great enforcer of the advocacy aspects of the ISPDB, but this seems like a point at which a decision should be made as to whether we should be requiring any other OAuth-adopters to stand up their own self-hosted autoconfig entries. (Even if we also have to or might as well end up updating the ISPDB entry as well if they're already hosted in the ISPDB and because of bug 1253147 we can't actually remove the domain.)
Reporter | ||
Comment 24•8 years ago
|
||
Andrew, I see no problem to host autoconfig file at Mail.ru. Could you provide a usage example?
Flags: needinfo?(bugmail)
Comment 25•8 years ago
|
||
See https://developer.mozilla.org/en-US/docs/Mozilla/Thunderbird/Autoconfiguration#Configuration_server_at_ISP In short, you'd want to stand up the revised ISPDB entry at one of the following: - http://autoconfig.mail.ru/mail/config-v1.1.xml - http://mail.ru/.well-known/autoconfig/mail/config-v1.1.xml Ideally you would also to host with https (using a valid certificate) as well so that things don't break when Thunderbird's autoconfig process becomes secure.
Flags: needinfo?(bugmail)
Reporter | ||
Comment 26•8 years ago
|
||
Andrew, it seems we need more information. We have a wide variety of own and delegated hostnames. How does the ISPDB resolve hostnames (a split of mx records or emails)?
Flags: needinfo?(bugmail)
Comment 27•8 years ago
|
||
Thunderbird first tries with the mail domain the user enters including self-hosted and ISPDB-hosted. If that's not working out, it asks the ISPDB server to perform an MX lookup. Then it checks the ISPDB for that MX-looked-up domain. Unfortunately, it doesn't check for self-hosting on the MX-looked-up domain at this time. (But it's not a huge change for Thunderbird to do that, just no one is actively improving Thunderbird's autoconfig and the most recent would-be contributor seems to have given up.) If you have other domains that have MX entries pointing at mail.ru (via any MX with the mail.ru domain suffix), then we'll need the ISPDB entry updated to mirror what you are self-hosting. This would be the ideal path, I think.
Flags: needinfo?(bugmail)
Assignee | ||
Comment 28•8 years ago
|
||
Alexander, it seems to me like you are getting what you need from Andrew, so clearing the Need Info
Flags: needinfo?(rkent)
Reporter | ||
Comment 29•8 years ago
|
||
Andrew, Kent, thanks for your help! We are planning to provide the autoconfig file next week.
Reporter | ||
Comment 30•8 years ago
|
||
Here it is https://autoconfig.mail.ru/mail/config-v1.1.xml
Comment 31•8 years ago
|
||
The self-hosted file does not include the oauth line you proposed in bug 1253196. Is this an oversight? Specifically, the diff against what the ISPDB hosts right now is just: $ svn diff Index: mail.ru =================================================================== --- mail.ru (revision 150392) +++ mail.ru (working copy) @@ -1,4 +1,4 @@ -<?xml version="1.0" encoding="UTF-8"?> +<?xml version="1.0"?> <clientConfig version="1.1"> <emailProvider id="mail.ru"> <domain>mail.ru</domain>
Reporter | ||
Comment 32•8 years ago
|
||
It's true, we are planning to include these lines after 45 release.
Flags: needinfo?(bugmail)
Comment 33•8 years ago
|
||
Okay, then please needinfo me on bug 1253196 when you do that and I'll be sure to update the ISPDB at the same time.
Flags: needinfo?(bugmail)
Reporter | ||
Updated•8 years ago
|
Assignee: monolithed → rkent
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•