Potential bug/inconsistency in auth-source netrc vs plstore backends

Potential bug/inconsistency in auth-source netrc vs plstore backends

[Soham Gumaste]

Hello,

I am trying to patch the `oauth2.el` elpa library to better integrate
with the native auth-sources system so that functions like nnimap and
smtpmail can use it transparently.

I noticed the following inconsistency while playing with this: As
shown by [1], the netrc backend does not require :host, :user or :port
as required keys in matched results, however, the plstore backend
requires :host, :login: :port and :secret.

This is causing an issue for me as I would like to patch the oauth2.el
library to store the access token as a secret, but without any port
associated with it. The oauth2.el library already uses plstore and it
is the more convenient backend to use.

Please let me know if this is indeed a bug and the possible direction
for a bugfix patch if that is the case.

Thanks


[1]: https://git.savannah.gnu.org/cgit/emacs.git/tree/lisp/auth-source.el#n1274
[2]: https://git.savannah.gnu.org/cgit/emacs.git/tree/lisp/auth-source.el#n2112
-- 
Soham Gumaste
sohamg2@gmail.com

Re: Potential bug/inconsistency in auth-source netrc vs plstore backends

[Soham Gumaste]

Apologies for the double email but please CC me in replies as I
consume this list in digest mode.

On Thu, Nov 7, 2024 at 2:34 PM Soham Gumaste <sohamg2@gmail.com> wrote:
>
> Hello,
>
> I am trying to patch the `oauth2.el` elpa library to better integrate
> with the native auth-sources system so that functions like nnimap and
> smtpmail can use it transparently.
>
> I noticed the following inconsistency while playing with this: As
> shown by [1], the netrc backend does not require :host, :user or :port
> as required keys in matched results, however, the plstore backend
> requires :host, :login: :port and :secret.
>
> This is causing an issue for me as I would like to patch the oauth2.el
> library to store the access token as a secret, but without any port
> associated with it. The oauth2.el library already uses plstore and it
> is the more convenient backend to use.
>
> Please let me know if this is indeed a bug and the possible direction
> for a bugfix patch if that is the case.
>
> Thanks
>
>
> [1]: https://git.savannah.gnu.org/cgit/emacs.git/tree/lisp/auth-source.el#n1274
> [2]: https://git.savannah.gnu.org/cgit/emacs.git/tree/lisp/auth-source.el#n2112
> --
> Soham Gumaste
> sohamg2@gmail.com



-- 
Soham Gumaste
sohamg2@gmail.com

Re: Potential bug/inconsistency in auth-source netrc vs plstore backends

[Xiyue Deng]

[-- Attachment #1: Type: text/plain, Size: 2059 bytes --]

Soham Gumaste <sohamg2@gmail.com> writes:

> Hello,
>
> I am trying to patch the `oauth2.el` elpa library to better integrate
> with the native auth-sources system so that functions like nnimap and
> smtpmail can use it transparently.
>
> I noticed the following inconsistency while playing with this: As
> shown by [1], the netrc backend does not require :host, :user or :port
> as required keys in matched results, however, the plstore backend
> requires :host, :login: :port and :secret.
>
> This is causing an issue for me as I would like to patch the oauth2.el
> library to store the access token as a secret, but without any port
> associated with it. The oauth2.el library already uses plstore and it
> is the more convenient backend to use.
>
> Please let me know if this is indeed a bug and the possible direction
> for a bugfix patch if that is the case.
>
> Thanks
>
>
> [1]: https://git.savannah.gnu.org/cgit/emacs.git/tree/lisp/auth-source.el#n1274
> [2]: https://git.savannah.gnu.org/cgit/emacs.git/tree/lisp/auth-source.el#n2112
> -- 
> Soham Gumaste
> sohamg2@gmail.com
>

I have done some recent improvements to `oauth2.el'[1] and provided a
hackish solution to use it with auth-source[2].  I wonder whether you
have tried those out and would like to get your feedback.

I am also interested in what you are adding to/modifying `oauth2.el'.
In case it's related, in the previous bugs I saw people seemed confused
about why `oauth2.el' used a separate storage for storing
refresh_token/access_token/etc. while still keeping the other
credentials (e.g. client_id, client_secret, etc.) in auth-sources.  My
understanding is that the data in the separate plstore are ephemeral,
e.g. they can expire and need to be refreshed, so it makes sense to use
a separate storage.  This also can be helpful in case you want to keep
it in sync among multiple machines.

[1] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=72358
[2] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=72992

-- 
Regards,
Xiyue Deng

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 857 bytes --]

Re: Potential bug/inconsistency in auth-source netrc vs plstore backends

[Soham Gumaste]

Hello, thanks for the reply!

>
> I have done some recent improvements to `oauth2.el'[1] and provided a
> hackish solution to use it with auth-source[2].  I wonder whether you
> have tried those out and would like to get your feedback.
>

My changes are not that elaborate, Im just trying to convince
auth-sources that the oauth2 access token is a "password"

> I am also interested in what you are adding to/modifying `oauth2.el'.
> In case it's related, in the previous bugs I saw people seemed confused
> about why `oauth2.el' used a separate storage for storing
> refresh_token/access_token/etc. while still keeping the other
> credentials (e.g. client_id, client_secret, etc.) in auth-sources.  My
> understanding is that the data in the separate plstore are ephemeral,
> e.g. they can expire and need to be refreshed, so it makes sense to use
> a separate storage.  This also can be helpful in case you want to keep
> it in sync among multiple machines.
>

I didn't take the ephemeral part into account. What I am doing is summarised as:
1) Change the default filename to be `oauth2.plist` instead of
`oauth2.plstore` as that automatically activates the plstore backend
in auth-info when the file is listed in auth-sources
2) Store the access-token a second time as `:secret`
3) Store additional data like :user/:login and :host

I can share my patch if you're curious but it really isn't ready/in
the hacking stage. Basically my reference point is to make it satisfy
the search query in nnimap.el/smtpmail.el which is my use case.

My roadblock currently is that the auth-sources-plstore-search
function REQUIRES that the :port also matches, whereas the netrc
backend has no such requirement. I could just save all the ports im
going to use in the plist file but thats really a non-solution and I'd
like more clarity on this discrepancy.

Thanks,
-- 
Soham Gumaste
sohamg2@gmail.com

Re: Potential bug/inconsistency in auth-source netrc vs plstore backends

[Michael Albinus]

Soham Gumaste <sohamg2@gmail.com> writes:

> Hello,

Hi Soham,

> I am trying to patch the `oauth2.el` elpa library to better integrate
> with the native auth-sources system so that functions like nnimap and
> smtpmail can use it transparently.
>
> I noticed the following inconsistency while playing with this: As
> shown by [1], the netrc backend does not require :host, :user or :port
> as required keys in matched results, however, the plstore backend
> requires :host, :login: :port and :secret.
>
> This is causing an issue for me as I would like to patch the oauth2.el
> library to store the access token as a secret, but without any port
> associated with it. The oauth2.el library already uses plstore and it
> is the more convenient backend to use.

What about using a fixed port like ':port "token"'? A similar (mis-)use
of :port is used by Tramp, where the port keeps the connection method,
like ':port "ssh"'.

This is documented in the auth manual, at the bottom of (info "(auth)
Help for users") . A simiar solution for oauth2.el could be documented
there as well.

> Thanks

Best regards, Michael.

Re: Potential bug/inconsistency in auth-source netrc vs plstore backends

[Soham Gumaste]

> > Hello,
>
> Hi Soham,

Hello, thanks for your time.

> What about using a fixed port like ':port "token"'? A similar (mis-)use
> of :port is used by Tramp, where the port keeps the connection method,
> like ':port "ssh"'.
>
Well, I am trying to satisfy the auth-sources-search query in the
function nnimap-credentials in nnimap.el. That query requests the
:port in the spec, and for the query to succeed with the plstore
backend I need to exactly match the :port attribute in the plist saved
by oauth2.el. This would mean I would have to store every port the
user wants to use oauth creds with. Regardless, I still want to know
if the discrepancy between the netrc and plstore backend is
intentional or if I can patch it.

> This is documented in the auth manual, at the bottom of (info "(auth)
> Help for users") . A simiar solution for oauth2.el could be documented
> there as well.
>
My current workaround is to advise the nnimap-credentials with :around
mode and returning the oauth2 token when appropriate. I'd like to make
this more first class though. It could be done by simply tweaking the
plist file stored by oauth2.el but the aforementioned discrepancy is
the roadblock right now. I dare not try to edit anything in Gnus
itself.
> > Thanks
>
> Best regards, Michael.

Thanks

-- 
Soham Gumaste
sohamg2@gmail.com

Re: Potential bug/inconsistency in auth-source netrc vs plstore backends

[Xiyue Deng]

[-- Attachment #1: Type: text/plain, Size: 2759 bytes --]

Soham Gumaste <sohamg2@gmail.com> writes:

>> > Hello,
>>
>> Hi Soham,
>
> Hello, thanks for your time.
>
>> What about using a fixed port like ':port "token"'? A similar (mis-)use
>> of :port is used by Tramp, where the port keeps the connection method,
>> like ':port "ssh"'.
>>
> Well, I am trying to satisfy the auth-sources-search query in the
> function nnimap-credentials in nnimap.el. That query requests the
> :port in the spec, and for the query to succeed with the plstore
> backend I need to exactly match the :port attribute in the plist saved
> by oauth2.el. This would mean I would have to store every port the
> user wants to use oauth creds with. Regardless, I still want to know
> if the discrepancy between the netrc and plstore backend is
> intentional or if I can patch it.
>

I think that's how auth-source is supposed to work.  People would need 2
entries with different `:port's, one for imap(s), one for smtp, for both to
work.  Or maybe there is a better way now?

>> This is documented in the auth manual, at the bottom of (info "(auth)
>> Help for users") . A simiar solution for oauth2.el could be documented
>> there as well.
>>
> My current workaround is to advise the nnimap-credentials with :around
> mode and returning the oauth2 token when appropriate. I'd like to make
> this more first class though. It could be done by simply tweaking the
> plist file stored by oauth2.el but the aforementioned discrepancy is
> the roadblock right now. I dare not try to edit anything in Gnus
> itself.

I think smtpmail uses the `:smtp-auth' attribute to specify which
mechanism to use[1].  In my plugin[2] I'm using `:auth' in a similar
fashion.  This has the advantage that one can have both the password and
XOAuth2 credentials in auth-source and can choose which to use simply by
changing `:auth', though this also requires coordination from nnimap[3].

Ideally I think it would be helpful if one can directly request which
mechanism to use in auth-source.  For now this has to be done
explicitly, e.g. in nnimap using `(nnimap-authenticator xoauth2)'.  If
that's not set, there can be a default sequence of authentication
mechanisms to try, like what nnimap is doing now, but put xoauth2 in
higher priority.

>> > Thanks
>>
>> Best regards, Michael.
>
> Thanks
>
> -- 
> Soham Gumaste
> sohamg2@gmail.com
>

[1] https://www.gnu.org/software/emacs/manual/html_node/smtpmail/Authentication.html
[2] https://gitlab.com/xiyueden/auth-source-xoauth2-plugin/-/blob/main/auth-source-xoauth2-plugin.el?ref_type=heads#L106-107
[3] https://gitlab.com/xiyueden/auth-source-xoauth2-plugin/-/blob/main/auth-source-xoauth2-plugin.el?ref_type=heads#L48

-- 
Regards,
Xiyue Deng

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 857 bytes --]

Re: Potential bug/inconsistency in auth-source netrc vs plstore backends

[Soham Gumaste]

>
> I think that's how auth-source is supposed to work.  People would need 2
> entries with different `:port's, one for imap(s), one for smtp, for both to
> work.  Or maybe there is a better way now?
>

Well, auth-source should also work if one has simply `password
<password>` in the netrc file, and that password would be used for
every port, every user etc. However, the plstore backed for
auth-sources currently requires that the keys host, user, password,
and port ALL are present in the entry for it to successfully match.

I wanted some insight into this discrepancy. My goal with the
oauth2.el changes is to have minimum viable integration with
auth-sources such that the existing functions using it work. It's
narrower in scope than your plugin, and perhaps when I have a final
patchset ready we can discuss the merits of each.

Thanks
-- 
Soham Gumaste
sohamg2@gmail.com

Re: Potential bug/inconsistency in auth-source netrc vs plstore backends

[Xiyue Deng]

[-- Attachment #1: Type: text/plain, Size: 1508 bytes --]

Soham Gumaste <sohamg2@gmail.com> writes:

>>
>> I think that's how auth-source is supposed to work.  People would need 2
>> entries with different `:port's, one for imap(s), one for smtp, for both to
>> work.  Or maybe there is a better way now?
>>
>
> Well, auth-source should also work if one has simply `password
> <password>` in the netrc file, and that password would be used for
> every port, every user etc.

Hmm, I would expect that a password is associated with an account
identified by some user ID.  Is there any existing usage of this
apply-to-all kind of password?

> However, the plstore backed for auth-sources currently requires that
> the keys host, user, password, and port ALL are present in the entry
> for it to successfully match.
>

I think all 4 fields are also required for other backends like netrc,
json, etc. - when Gnus trying to search for an entry through nnimap, it
tries to search using a tuple of (:host, :user, :port) and requires the
returned entry has :secret.  Or maybe I'm missing some other use case?

> I wanted some insight into this discrepancy. My goal with the
> oauth2.el changes is to have minimum viable integration with
> auth-sources such that the existing functions using it work. It's
> narrower in scope than your plugin, and perhaps when I have a final
> patchset ready we can discuss the merits of each.
>

Definitely, sounds good.

> Thanks
> -- 
> Soham Gumaste
> sohamg2@gmail.com

-- 
Regards,
Xiyue Deng

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 857 bytes --]

Re: Potential bug/inconsistency in auth-source netrc vs plstore backends

[Soham Gumaste]

> Hmm, I would expect that a password is associated with an account
> identified by some user ID.  Is there any existing usage of this
> apply-to-all kind of password?
>

The auth-info manual does mention this towards the start, and mentions
that it is a bad idea cause you will get "pwned". SO its not really a
use-case, but the fact is that netrc backend will match an entry even
if it does not have one of the fields eg port from the query.

I think this discrepancy is because the netrc backend uses a custom
netrc parser, whereas the plstore backend simply uses `plstore-find`
[1], which requires every key to be present and match, unless the key
is specified as "t".

This makes my idea harder, as now I either need to store port numbers
in the oauth2 plstore file, or patch plstore or patch gnus, and my
goal is to make minimum viable changes, so its a bit complicated. I
initially thought editing the "returned keys" binding would fix
this,but that is clearly not the case.

Will report back once I have time to hack on this more. Thank you
everyone in this thread for the feedback.

[1]: https://git.savannah.gnu.org/cgit/emacs.git/tree/lisp/auth-source.el#n2115


-- 
Soham Gumaste
sohamg2@gmail.com