bug#72358: 29.4; oauth2.el improvements

[Xiyue Deng]

Robert Pluim <rpluim@gmail.com> writes:

>>>>>> On Mon, 29 Jul 2024 14:25:01 -0700, Xiyue Deng <manphiz@gmail.com> said:
>
>     Xiyue> Hi,
>     Xiyue> I have been trying out using oauth2.el to enable OAuth2-based
>     Xiyue> authentication for email service providers and had some success for
>     Xiyue> Gmail.  During this process, I have made a few changes to 
> oauth2.el that
>     Xiyue> enables it to use with Gmail OAuth2 as well as some usability and
>     Xiyue> debugging improvements, which I'm sharing below.
>
> Thank you for this. This support is becoming more necessary as time
> goes on. I even wonder if we should bring oauth2.el into emacs instead
> of it being a package.
>

I would also love that to happen.  In fact, I have another addon
inspired by auth-source-xoauth2 and makes use of oauth2.el and
auth-source backend, though it requires advising the
auth-source-search-backends and kind of awkward.  I'll post that through
another bug report for comments after this one gets submitted.

>     Xiyue> This is a series of five patches, which are attached.
>
>     Xiyue> The first patch shows the authentication URL in the minibuffer 
> window
>     Xiyue> alongside the prompt accepting the authorization code.  This helps 
> when
>     Xiyue> a user has multiple accounts from the same provider but is logged 
> into a
>     Xiyue> different account than the one that the user is trying to set up.  
> If
>     Xiyue> the user use the link (or through `browse-url') it will use the 
> active
>     Xiyue> account instead of the one intended.  By showing the URL in the
>     Xiyue> minibuffer, the user can choose other ways to get the 
> authorization code
>     Xiyue> (e.g. using another browser, using private/encognito mode, etc.)
>
> OK. This fixes one of my irritations with oauth2.el 🙂
>

Glad to hear I'm not the only one.

>     Xiyue> The second patch adds the parameters `access_type=offline' and
>     Xiyue> `prompt=consent' to the authorization URL, which is required for 
> Gmail
>     Xiyue> OAuth2 to get the refresh token.  Without these 2 parameters, Gmail
>     Xiyue> response will only contain the access token which expires in one 
> hour.
>     Xiyue> They should also be compatible with other OAuth2 authentication 
> process.
>     Xiyue> (Though I am currently having trouble to get outlook.com to work
>     Xiyue> regardless of these parameters, which I'll ask in a separate 
> thread.)
>
>     Xiyue> Note that the second patch depends on the first patch as they 
> modify the same
>     Xiyue> part of the code.
>
> OK. Iʼm assuming oauth2.el can use the refresh token next time it
> needs to authorize? (Iʼve been avoiding actually using oauth2.el in
> anger, since app passwords still work)
>

Yes, with a valid refresh token you can login without any manual steps.
Unless the refresh_token itself is expired, in which case you'll have to
re-authorize.

>     Xiyue> The third patch encodes the parameters for requesting refreshing 
> access
>     Xiyue> token, which is recommended because the client secret and other
>     Xiyue> parameters may contain characters that may break parameter parsing.
>
> OK
>
>     Xiyue> The fourth patch may need a bit of background: oauth2.el 
> (optionally)
>     Xiyue> uses plstore to save authentication data for future reuse, and the
>     Xiyue> plstore id for an account is computed using a combination of 
> `auth-url',
>     Xiyue> `token-url', and `scope'.  However, this combination of data 
> doesn't
>     Xiyue> guarantee uniqueness for accounts for a same provider, e.g. for 
> Gmail,
>     Xiyue> the three parameters are the same for different accounts, and hence
>     Xiyue> storing a second account information will override the first one.
>
>     Xiyue> This fourth patch adds `client-id' to the calculation of plstore 
> id to
>     Xiyue> ensure its uniqueness.  This may cause a few concerns:
>
>     Xiyue> - This will invalidate all existing entries and a user will have 
> to redo
>     Xiyue>   the authorization process again to get a new refresh token.  
> However,
>     Xiyue>   I think it's more important to ensure that oauth2.el works 
> correctly
>     Xiyue>   for multiple accounts of the same provider, or a user may suffer 
> from
>     Xiyue>   confusion when adding a new account invalidates a previous 
> account.
>
> I donʼt think thatʼs too big a concern. 'modern' authentication flows
> regularly re-prompt, so this will not be too surprising (although
> maybe call it out in the packageʼs NEWS or README).
>

I have added a NEWS file in patch 6 documenting this for 0.17 (which
should be the version of the next release.)

>     Xiyue> - Adding `client-id' to the calculation of plstore id may provoke
>     Xiyue>   suspicion of leaking it as the hash calculation uses md5.  In 
> most
>     Xiyue>   cases, requesting a refresh token requires both `client-id' and
>     Xiyue>   `client-secret', so without including the latter it should be 
> safe.
>     Xiyue>   There are cases when requesting only the access token may work 
> with
>     Xiyue>   `client-id' along.  Still, I think this should not be a big 
> concern as
>     Xiyue>   the data is combined with `auth-url', `token-url', and `scope' 
> which
>     Xiyue>   provides sufficient salt.  Alternatively, we can also choose to 
> use a
>     Xiyue>   more secure hash function, e.g. SHA2 or better, given that 
> existing
>     Xiyue>   entries will be invalidated anyway.
>
> If the existing entries are going to become invalid anyway, you might
> as well take the opportunity to move away from md5 at the same
> time. git picked SHA-256, but that was a while ago, so maybe SHA-512?
>

Makes sense.  I have updated the hash function to use SHA-512 in patch 5.

>     Xiyue> The fifth patch adds debug messages when doing a URL query which 
> records
>     Xiyue> the request URL, the request data, and the response data, and 
> provide a
>     Xiyue> custom variable to enable this.  This provides a way to help 
> debugging
>     Xiyue> the requests, and I find it handy when testing oauth2 against 
> different
>     Xiyue> providers.
>
> OK (although perhaps make it a defvar rather than a defcustom, to
> avoid people accidentally enabling it).
>

Done also in patch 5.

> Robert

-- 
Xiyue Deng

0001-Show-full-authentication-URL-to-let-user-choose-how-.patch
Description: Text Data

0002-Add-parameters-required-by-Google-OAuth2-to-get-refr.patch
Description: Text Data

0003-Encode-parameters-when-requesting-access.patch
Description: Text Data

0004-Support-storing-data-for-multiple-accounts-of-the-sa.patch
Description: Text Data

0005-Add-debug-messages-and-provide-a-switch-variable-for.patch
Description: Text Data

0006-Add-NEWS-file-to-document-the-changes-to-plstore-id-.patch
Description: Text Data