1927998 - Better, unified OAuth handling

Status: RESOLVED FIXED

(Thunderbird :: General, enhancement)

Description

[Geoff Lankow (:darktrojan)]

We have multiple consumers of OAuth2 and they all do things in different ways. I'm not really happy with any of them and I'm especially not happy that there's not one unified piece of code that handles it all. This is a medium-term project to fix that.

• Mail protocols use the OAuth2Module which handles token saving/retrieval and interaction with the async authentication system.
• CardDAV generally does the same thing, but not initial discovery and set-up of address books. That handles saving tokens itself.
• CalDAV does it very differently, there is a subclass of OAuth2 for each supported provider and they handle token saving/retrieval through the calendar's system for that. There's also multiple historic formats for storing CalDAV OAuth tokens.

The first steps here will be to reform OAuth2Module and OAuth2Providers so that they handle multiple scopes better. The code was originally intended for only a single scope but we hacked multiple scopes into it and it doesn't work very well.

Then we'll change CardDAV and CalDAV to use the same code so that any further changes only need to be made once. I've filed bug 1927971 to find out more about CalDAV's tokens in the wild, so we'll need to wait until we have some useful information from that.

Finally I think we can overhaul the whole system and create a better interface for getting what we need. But that's a fair way off yet.

Comment 1

[Geoff Lankow (:darktrojan)]

Attachment: Bug 1927998 - Stop storing OAuth2 details in preferences. r=leftmostcat,mkmelin,babolivier

Comment 2

[Geoff Lankow (:darktrojan)]

Attachment: Bug 1927998 - Store OAuth2 scopes separately. r=leftmostcat,mkmelin,babolivier

I've changed getHostnameDetails to take an optional second argument, and return a list of
required scopes (based on the new argument) as well as a list of all scopes. This way we can
better determine if we have all of the scopes we need to complete the current action.

Comment 3

[Geoff Lankow (:darktrojan)]

Attachment: Bug 1927998 - Update the last-updated time of OAuth tokens. r=leftmostcat,mkmelin

… and only update it when it actually changed.

Comment 4

[Geoff Lankow (:darktrojan)]

Attachment: Bug 1927998 - Change CardDAV to use OAuth2Module. r=leftmostcat,mkmelin

Comment 5

[Geoff Lankow (:darktrojan)]

Attachment: Bug 1927998 - Move CalDAV to OAuth2Module. r=leftmostcat,mkmelin

This removes the last traces of the Lightning oAuth client for Google calendars. Some users will
have to reauthenticate but it's a very small fraction of users.

Comment 6

[Toby Pilling [:tobyp]]

Will this allow us to delete tokens when we no longer need them? It would be great if we could update our privacy policy to include more reassuring wording on that matter...

Comment 7

[Geoff Lankow (:darktrojan)]

Attachment: Bug 1927998 - Make type argument of getHostnameDetails non-optional. r=#thunderbird-back-end-reviewers

Comment 8

[Geoff Lankow (:darktrojan)]

Attachment: Bug 1927998 - Make getHostnameDetails return an object instead of an array. r=#thunderbird-back-end-reviewers

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/9d6b0dd35077
Stop storing OAuth2 details in preferences. r=leftmostcat,mkmelin,babolivier
https://hg.mozilla.org/comm-central/rev/8c594cb3017a
Store OAuth2 scopes separately. r=leftmostcat,mkmelin,babolivier
https://hg.mozilla.org/comm-central/rev/425daaa18ed2
Update the last-updated time of OAuth tokens. r=leftmostcat,mkmelin
https://hg.mozilla.org/comm-central/rev/a45216aaccc7
Change CardDAV to use OAuth2Module. r=leftmostcat,mkmelin
https://hg.mozilla.org/comm-central/rev/3efacfab89ff
Move CalDAV to OAuth2Module. r=leftmostcat,mkmelin

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/1e668ff6b0f3
Make type argument of getHostnameDetails non-optional. r=leftmostcat,mkmelin
https://hg.mozilla.org/comm-central/rev/183005fe50a9
Make getHostnameDetails return an object instead of an array. r=leftmostcat

Comment 11

[Geoff Lankow (:darktrojan)]

Closing this. There are still things I want to do but I'll open a new bug for them.

Release Note Request (optional, but appreciated)
[Why is this notable]: This has potential to break things for users.
[Suggested wording]: OAuth2 authentication overhauled. CardDAV address books and CalDAV calendars now use the same OAuth2 code as mail. Provider information is no longer stored in preferences.
[Links (documentation, blog post, etc)]:

Comment 12

[Corey Bryant]

Release note added to 135.0b1. Thanks! I did drop the first sentence to shorten it up to fit on one line.