From 7cea5601c657675178532b85a322403cc835e3ed Mon Sep 17 00:00:00 2001 From: Robert Virkus Date: Sat, 4 Nov 2023 12:56:36 +0100 Subject: [PATCH] feat: show error details when SMTP OAuth fails, improve refresh handling --- lib/src/imap/imap_client.dart | 9 +- lib/src/mail/mail_authentication.dart | 23 +++- lib/src/mail/mail_client.dart | 106 ++++++++++-------- .../commands/smtp_auth_xoauth2_command.dart | 19 +++- 4 files changed, 104 insertions(+), 53 deletions(-) diff --git a/lib/src/imap/imap_client.dart b/lib/src/imap/imap_client.dart index 2bd27bc1..89069694 100644 --- a/lib/src/imap/imap_client.dart +++ b/lib/src/imap/imap_client.dart @@ -218,18 +218,23 @@ enum StatusFlags { class ImapClient extends ClientBase { /// Creates a new ImapClient instance. /// - /// Set the [eventBus] to add your specific `EventBus` to listen to + /// Set the [bus] to add your specific `EventBus` to listen to /// IMAP events. + /// /// Set [isLogEnabled] to `true` for getting log outputs on the standard /// output. + /// /// Optionally specify a [logName] that is given out at logs to differentiate /// between different imap clients. + /// /// Set the [defaultWriteTimeout] in case the connection connection should /// timeout automatically after the given time. + /// /// [onBadCertificate] is an optional handler for unverifiable certificates. /// The handler receives the [X509Certificate], and can inspect it and decide /// (or let the user decide) whether to accept the connection or not. - /// The handler should return true to continue the [SecureSocket] connection. + /// The handler should return `true` to continue the [SecureSocket] + /// connection. ImapClient({ EventBus? bus, bool isLogEnabled = false, diff --git a/lib/src/mail/mail_authentication.dart b/lib/src/mail/mail_authentication.dart index 46b75f15..c672f4f6 100644 --- a/lib/src/mail/mail_authentication.dart +++ b/lib/src/mail/mail_authentication.dart @@ -170,6 +170,9 @@ class OauthToken { final String accessToken; /// Expiration in seconds from [created] time + /// + /// Compare [expiresDateTime], [willExpireIn] + /// and [isExpired] @JsonKey(name: 'expires_in') final int expiresIn; @@ -193,12 +196,30 @@ class OauthToken { final String? provider; /// Checks if this token is expired + /// + /// Compare [willExpireIn] and [isValid] bool get isExpired => expiresDateTime.isBefore(DateTime.now().toUtc()); + /// Checks if the token is already expired or will expire + /// within the given (positive) [duration]. + bool willExpireIn(Duration duration) { + print( + 'willExpireIn(): \n' + 'expiresDateTime=$expiresDateTime, now=${DateTime.now().toUtc()},\n' + 'duration=$duration, ' + 'compare=${DateTime.now().toUtc().subtract(duration)}', + ); + return expiresDateTime.isBefore(DateTime.now().toUtc().subtract(duration)); + } + /// Retrieves the expiry date time + /// + /// Compare [willExpireIn] DateTime get expiresDateTime => created.add(Duration(seconds: expiresIn)); - /// Checks if this token is still valid, ie not expired + /// Checks if this token is still valid, ie not expired. + /// + /// Compare [isExpired] bool get isValid => !isExpired; /// Refreshes this token with the new [accessToken] and [expiresIn]. diff --git a/lib/src/mail/mail_client.dart b/lib/src/mail/mail_client.dart index 29d8c79c..dc477207 100644 --- a/lib/src/mail/mail_client.dart +++ b/lib/src/mail/mail_client.dart @@ -93,16 +93,26 @@ class MailClient { ); } else if (config.serverConfig.type == ServerType.pop) { _incomingMailClient = _IncomingPopClient( - _downloadSizeLimit, _eventBus, logName, config, this, - isLogEnabled: _isLogEnabled, onBadCertificate: onBadCertificate); + _downloadSizeLimit, + _eventBus, + logName, + config, + this, + isLogEnabled: _isLogEnabled, + onBadCertificate: onBadCertificate, + ); } else { - throw InvalidArgumentException('Unsupported incoming' - 'server type [${config.serverConfig.typeName}].'); + throw InvalidArgumentException( + 'Unsupported incoming' + 'server type [${config.serverConfig.typeName}].', + ); } final outgoingConfig = _account.outgoing; if (outgoingConfig.serverConfig.type != ServerType.smtp) { - print('Warning: unknown outgoing server ' - 'type ${outgoingConfig.serverConfig.typeName}.'); + print( + 'Warning: unknown outgoing server ' + 'type ${outgoingConfig.serverConfig.typeName}.', + ); } _outgoingMailClient = _OutgoingSmtpClient( this, @@ -231,6 +241,7 @@ class MailClient { late _IncomingMailClient _incomingMailClient; late _OutgoingMailClient _outgoingMailClient; final _incomingLock = Lock(); + final _outgoingLock = Lock(); /// Adds the specified mail event [filter]. /// @@ -281,7 +292,8 @@ class MailClient { final refresh = _refreshOAuthToken; if (refresh != null) { final auth = account.incoming.authentication; - if (auth is OauthAuthentication && auth.token.isExpired) { + if (auth is OauthAuthentication && + auth.token.willExpireIn(const Duration(minutes: 15))) { OauthToken? refreshed; try { _incomingMailClient.log('Refreshing token...'); @@ -309,6 +321,8 @@ class MailClient { incoming: incoming, outgoing: outgoing, ); + _incomingMailClient._config = _account.incoming; + _outgoingMailClient._mailConfig = _account.outgoing; final onConfigChanged = _onConfigChanged; if (onConfigChanged != null) { try { @@ -329,21 +343,29 @@ class MailClient { Future disconnect() async { final futures = [ stopPollingIfNeeded(), - _incomingMailClient.disconnect(), - _outgoingMailClient.disconnect(), + _incomingLock.synchronized( + () => _incomingMailClient.disconnect(), + ), + _outgoingLock.synchronized( + () => _outgoingMailClient.disconnect(), + ), ]; _isConnected = false; await Future.wait(futures); } - /// Enforces to reconnect with the service. + /// Enforces to reconnect with the incoming service. /// /// Also compare [disconnect]. /// Also compare [connect]. Future reconnect() async { - await _incomingMailClient.disconnect(); - await _incomingMailClient.reconnect(); - _isConnected = true; + await _incomingLock.synchronized( + () async { + await _incomingMailClient.disconnect(); + await _incomingMailClient.reconnect(); + _isConnected = true; + }, + ); } // Future tryAuthenticate( @@ -880,7 +902,7 @@ class MailClient { /// /// Optionally specify the [sentMailbox] when the mail system does not /// support mailbox flags. - Future sendMessageBuilder( + Future sendMessageBuilder( MessageBuilder messageBuilder, { MailAddress? from, bool appendToSent = true, @@ -894,27 +916,14 @@ class MailClient { final message = messageBuilder.buildMimeMessage(); final use8Bit = builderEncoding == TransferEncoding.eightBit; - final futures = [ - _sendMessageViaOutgoing(message, from, use8Bit, recipients), - ]; - if (appendToSent && _incomingMailClient.supportsAppendingMessages) { - sentMailbox ??= getMailbox(MailboxFlag.sent); - if (sentMailbox == null) { - _incomingMailClient.log( - 'Error: unable to append sent message: no no mailbox with flag ' - 'sent found in $mailboxes'); - } else { - futures.add( - appendMessage( - message, - sentMailbox, - flags: [MessageFlags.seen], - ), - ); - } - } - - return Future.wait(futures); + return sendMessage( + message, + from: from, + appendToSent: appendToSent, + sentMailbox: sentMailbox, + use8BitEncoding: use8Bit, + recipients: recipients, + ); } /// Sends the specified [message]. @@ -945,9 +954,13 @@ class MailClient { Mailbox? sentMailbox, bool use8BitEncoding = false, List? recipients, - }) { + }) async { + await _prepareConnect(); final futures = [ - _sendMessageViaOutgoing(message, from, use8BitEncoding, recipients), + _outgoingLock.synchronized( + () => + _sendMessageViaOutgoing(message, from, use8BitEncoding, recipients), + ), ]; if (appendToSent && _incomingMailClient.supportsAppendingMessages) { sentMailbox ??= getMailbox(MailboxFlag.sent); @@ -966,7 +979,7 @@ class MailClient { } } - return Future.wait(futures); + await Future.wait(futures); } Future _sendMessageViaOutgoing( @@ -3334,18 +3347,24 @@ class _IncomingPopClient extends _IncomingMailClient { } abstract class _OutgoingMailClient { + _OutgoingMailClient({required MailServerConfig mailConfig}) + : _mailConfig = mailConfig; + ClientBase get client; ServerType get clientType; + MailServerConfig _mailConfig; /// Checks if the incoming mail client supports 8 bit encoded messages. /// /// Is only correct after authorizing. Future supports8BitEncoding(); - Future sendMessage(MimeMessage message, - {MailAddress? from, - bool use8BitEncoding = false, - List? recipients}); + Future sendMessage( + MimeMessage message, { + MailAddress? from, + bool use8BitEncoding = false, + List? recipients, + }); Future disconnect(); } @@ -3367,7 +3386,7 @@ class _OutgoingSmtpClient extends _OutgoingMailClient { // defaultWriteTimeout: connectionTimeout, onBadCertificate: onBadCertificate, ), - _mailConfig = mailConfig; + super(mailConfig: mailConfig); @override ClientBase get client => _smtpClient; @@ -3375,7 +3394,6 @@ class _OutgoingSmtpClient extends _OutgoingMailClient { ServerType get clientType => ServerType.smtp; final MailClient mailClient; final SmtpClient _smtpClient; - final MailServerConfig _mailConfig; Future _connectOutgoingIfRequired() async { if (!_smtpClient.isLoggedIn) { diff --git a/lib/src/private/smtp/commands/smtp_auth_xoauth2_command.dart b/lib/src/private/smtp/commands/smtp_auth_xoauth2_command.dart index 80a6bafd..996c4f32 100644 --- a/lib/src/private/smtp/commands/smtp_auth_xoauth2_command.dart +++ b/lib/src/private/smtp/commands/smtp_auth_xoauth2_command.dart @@ -11,7 +11,7 @@ class SmtpAuthXOauth2Command extends SmtpCommand { final String? _userName; final String? _accessToken; - bool _authSent = false; + var _authSentSentCounter = 0; @override String get command => 'AUTH XOAUTH2'; @@ -19,12 +19,18 @@ class SmtpAuthXOauth2Command extends SmtpCommand { @override String? nextCommand(SmtpResponse response) { if (response.code != 334 && response.code != 235) { - print('Warning: Unexpected status code during AUTH XOAUTH2: ' - '${response.code}. Expected: 334 or 235. \nauthSent=$_authSent'); + print( + 'Warning: Unexpected status code during AUTH XOAUTH2: ' + '${response.code}. Expected: 334 or 235.\n' + 'authSentCounter=$_authSentSentCounter ', + ); } - if (!_authSent) { - _authSent = true; + if (_authSentSentCounter == 0) { + _authSentSentCounter = 1; return getBase64EncodedData(); + } else if (response.code == 334 && _authSentSentCounter == 1) { + _authSentSentCounter++; + return ''; // send empty line to receive error details } else { return null; } @@ -39,7 +45,8 @@ class SmtpAuthXOauth2Command extends SmtpCommand { } @override - bool isCommandDone(SmtpResponse response) => _authSent; + bool isCommandDone(SmtpResponse response) => + response.code != 334 && _authSentSentCounter > 0; @override String toString() => 'AUTH XOAUTH2 ';