Skip to content

Conversation

@jstanden
Copy link
Contributor

Authentication with XOAUTH2 tokens was previously implemented for the IMAP protocol but not for POP3.

This PR implements the auth method for POP3 accounts.

Gmail supports XOAUTH2 tokens for both protocols:

$ openssl s_client -connect pop.gmail.com:995 -crlf

+OK Gpop ready for requests from 192.168.1.1 a1b2c3d4e5
capa
+OK Capability list follows
USER
RESP-CODES
EXPIRE 0
LOGIN-DELAY 300
TOP
UIDL
X-GOOGLE-RICO
SASL PLAIN XOAUTH2 OAUTHBEARER

This uses the same xoauth2_token parameter as IMAP, and the same Horde_Imap_Client_Password_Xoauth2 class.

In POP3, the command is auth xoauth2 <base64-encoded auth-bearer>.

This patch also moves XOAUTH2 to the first attempted authentication mechanism if the xoauth2_token parameter is set; which makes USER with an app-specific password the optional fallback.

@what-the-diff
Copy link

what-the-diff bot commented May 10, 2025

PR Summary

  • Enhanced Authentication Method
    We've added a feature where the system will always use XOAUTH2 (a secure method for authentication) when a token (kind of like an access key) is provided.

  • Updated Login System
    The part of the program that handles the login operation has been upgraded to manage the XOAUTH2 process, validating the presence of the token before proceeding.

  • Debug Information for Enhanced Authentication
    This update also includes the ability to track the authentication process, logging the user related to the token for debugging and error-tracking purposes.

Copy link
Contributor

@TDannhauer TDannhauer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good for me.

@TDannhauer TDannhauer self-requested a review May 16, 2025 22:38

case 'XOAUTH2':
if (!($xoauth2_token = $this->getParam('xoauth2_token')))
throw new Horde_Imap_Client_Exception("Expected an XOAUTH2 token");
Copy link
Contributor

@TDannhauer TDannhauer May 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If Horde_Imap_Client_Password_Xoauth2 is now used for IMAP as well as POP3, shouldn't the class be renamed/relocated to avoid confusion and fit poperly into the structure?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TDannhauer Hello Torben! I'm not sure I follow.

The library is named Horde_Imap, so all of the existing classes (including POP3) are already prefixed:

  • Horde_Imap_Client_Socket_Connection_Pop3
  • Horde_Imap_Client_Socket_Pop3
  • Horde_Imap_Client_Ids_Pop3

The password classes are:

  • Horde_Imap_Client_Base_Password
  • Horde_Imap_Client_Password_Xoauth2

That feels consistent. The directory structure follows the class naming convention.

I do need to fix an XOAUTH2 protocol issue between Microsoft POP3 and Gmail POP3, though. The implementation uses a single line AUTH statement, but Microsoft only supports the two-line style (with the token on a second line).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry missed that. Fine for me.

@jstanden
Copy link
Contributor Author

It looks like Microsoft POP3 requires a two-line AUTH XOAUTH command, with the token on the second line.

Gmail POP3 is fine with either one-line or two-line.

I'll update the PR to always use the two-line format.

@jstanden jstanden force-pushed the feat/pop3-xoauth2 branch from 8932178 to a260140 Compare May 29, 2025 01:00
@jstanden
Copy link
Contributor Author

I updated the commit to always use the two-line AUTH XOAUTH2 format.

@jstanden jstanden requested a review from TDannhauer May 29, 2025 01:01
@TDannhauer TDannhauer merged commit e143690 into horde:FRAMEWORK_6_0 Aug 7, 2025
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.

2 participants