Subject: Re: bug#72358: 29.4; oauth2.el improvements

From: Robert Pluim <[email-masked]>
Date: Tue, 30 Jul 2024 09:46:29 +0200

>>>>> On Mon, 29 Jul 2024 14:25:01 -0700, Xiyue Deng <[email-masked]> 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.

    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 🙂

    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)

    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).

    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?

    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).

Robert
--