Skip to content

Conversation

@AlessioC31
Copy link

Hello, I would like to add OAUTH2 support to this library and I started working on it.

This is what I came up with right now. I'm not 100% sure about the fact that now functions accept both isapop and isoauth2.

So please let me know if you think this should be handled in a different way.

Thank you!

@hsbt
Copy link
Member

hsbt commented Feb 16, 2023

Sorry, I'm not familiar with OAUTH2 protocol. So, I couldn't review this.

@AlessioC31
Copy link
Author

AlessioC31 commented Feb 16, 2023

Sorry, I'm not familiar with OAUTH2 protocol. So, I couldn't review this.

Hello! Thank you in any case. How does it work in such cases? Are there any other maintainer who could have a look?

For reference, it is described in general here: https://www.rfc-editor.org/rfc/rfc1734 and here: https://learn.microsoft.com/en-us/exchange/client-developer/legacy-protocols/how-to-authenticate-an-imap-pop-smtp-application-by-using-oauth

image

Thanks again!

@trobiyo
Copy link

trobiyo commented Mar 23, 2023

Hi @hsbt ,

May we kindly ask you whether there is any other contributor that could move forward on reviewing this PR?

Cheers,
Ismael

`@apop` is being set to strings so this was not
working:
`assert_equal pop.apop?, false` since `apop?`
was returning the string.
@AlessioC31
Copy link
Author

Hello @hsbt, I've added a couple of tests of the functionality and a comment explaining what the new code does.

Would you be more incline to merge this if we change the code to avoid changing the public API of the library? For now we added the isoauth2 parameter to the functions, would it be better for you if we would handle this differently?
Like adding a method to the pop class like def oauth2() which sets a property in the instance, instead of changing the signature of the methods?

Thanks.

nattsw pushed a commit to discourse/discourse that referenced this pull request Jun 26, 2023
While we are unable to support OAUTH2 with pop3 (due to upstream dependency ruby/net-pop#16), we are adding the support for mail pollers plugin. Doing so, it would be possible to write a plugin which then uses other ways (microsoft graph sdk for example) to poll emails from a mailbox.

The idea is that a plugin would define a class which inherits from Email::Poller and defines a poll_mailbox static method which returns an array of strings. Then the plugin could call register_mail_poller(<class_name>) to have it registered. All the configuration (oauth2 tokens, email, etc) could be managed by sitesettings defined in the plugin.
@nevans
Copy link

nevans commented Sep 26, 2023

IMO, rather than make a one-off version only for XOAUTH2, we should implement SASL properly. Or, if we do make a one-off version for XOAUTH2, the API should be designed in such a way that it would work for any SASL mechanism, and not be tied to just one.

I'll try to put together a proposal in either an issue or a PR before the end of the week, after net-imap's big SASL update has been released.

@jbcpollak
Copy link

Any chance progress on this could me made soon? Google is pulling the plug on app-specific passwords in a few months and requiring OAuth for pop access. This means services like Discourse won't be able to check email from GSuite hosted mail servers starting in mid-June.

@martin-brennan
Copy link

Just echoing what @jbcpollak is saying above. I am an engineer at Discourse and we are tracking this in https://meta.discourse.org/t/add-richer-authentication-support-for-pop3/219340 and we definitely have a need for POP3 OAUTH2 support sooner rather than later. XOAUTH2 authenticator was added to net-smtp quite recently back in March ere https://github.com/ruby/net-smtp/releases/tag/v0.5.0 .

Is there anything that we can do to help move this forward?

@nevans
Copy link

nevans commented Jun 7, 2024

I did start working on a bigger proposal for "proper" SASL support, but I never got any feedback on those preliminary PRs:

On the other hand, I very rarely use net-pop myself, so I haven't personally (or professionally) felt the urgency. But I agree that it is long overdue. Gmail and Microsoft 365 are both very popular and both now require XOAUTH2.

Nevertheless, I still think that we should aim for an API that is compatible any SASL mechanism, and not another one-off. That's kind of the entire point of SASL. 😉 XOAUTH2 may be the only way to connect to GMail, but it was obsoleted by the standard (and much better specified) OAUTHBEARER nine years ago. Yahoo Mail still supports XOAUTH2, but its documentation now recommends OAUTHBEARER. And other mechanisms may be important too, for example: EXTERNAL with a client certificate, ANONYMOUS for shared mailboxes, or SCRAM-SHA-256-PLUS for TLS channel binding, etc.

@AlessioC31, would you be okay with some changes to this PR to give it a more extensible API. Adding a new method or class for each mechanism isn't so bad. But adding another positional parameter to existing methods feels like an unsustainable approach to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants