From 0c87bb984341b51dc1976a70f0cba7d873e4d452 Mon Sep 17 00:00:00 2001 From: Simon Robinson Date: Fri, 23 Jun 2023 08:44:28 +0100 Subject: [PATCH 01/21] Update pywebview version to resolve macOS issue; fixes #170 --- requirements.txt | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/requirements.txt b/requirements.txt index 56be66d..8cee259 100644 --- a/requirements.txt +++ b/requirements.txt @@ -8,10 +8,11 @@ timeago # for displaying the last authenticated activity hint # force pystray version with dummy GUI fix for headless deployments (https://github.com/moses-palmer/pystray/issues/118) pystray>=0.19.4 -# force pywebview 4.1+ to fix Windows issue with PyInstaller/pythonw (https://github.com/r0x0r/pywebview/issues/1086) -# and macOS pre-Mojave crash when opening browser windows (https://github.com/r0x0r/pywebview/pull/1047) - we could do, -# e.g., platform_release < '18' to allow Linux and other macOS versions more flexibility, but that seems over-the-top -pywebview>=4.1 +# force pywebview 4.2.1+ to fix Windows issue with PyInstaller/pythonw (https://github.com/r0x0r/pywebview/issues/1086) +# and a macOS pre-Mojave crash when opening browser windows (https://github.com/r0x0r/pywebview/pull/1047), plus a +# missing macOS dependency issue introduced in 4.1 (https://github.com/r0x0r/pywebview/pull/1154) - note that we could +# do, e.g., platform_release < '18' to allow Linux platforms more flexibility, but that seems over-the-top +pywebview>=4.2.1 # macOS: improve menu bar interaction, provide native notifications and handle system events pyobjc-framework-Cocoa; sys_platform == 'darwin' From 0271ed920b6f2d50684ee2eda15a5dad47c0c701 Mon Sep 17 00:00:00 2001 From: Simon Robinson Date: Tue, 27 Jun 2023 21:14:09 +0100 Subject: [PATCH 02/21] Clarify IPv4 and IPv6 support and defaults --- README.md | 4 ++++ emailproxy.config | 8 ++++---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index f93a4c4..8329a6d 100644 --- a/README.md +++ b/README.md @@ -171,6 +171,10 @@ This can be achieved using `telnet`, [PuTTY](https://www.chiark.greenend.org.uk/ For example, to test the Office 365 IMAP server from the [example configuration](emailproxy.config), first open a connection using `telnet localhost 1993`, and then send a login command: `a1 login e@mail.com password`, replacing `e@mail.com` with your email address, and `password` with any value you like during testing (see above for why the password is irrelevant). If you have already authorised your account with the proxy you should see a response starting with `a1 OK`; if not, this command should trigger a notification from the proxy about authorising your account. +If you are having trouble actually connecting to the proxy, it is always worth double-checking the `local_address` that you are using. +The proxy defaults to `localhost` for this parameter, but this may resolve to different values depending on your environment (see the sample configuration file for further details). +Please try to connect to both IPv4 (i.e., `127.0.0.1`) and IPv6 (i.e., `::1`) loopback addresses before reporting any connection issues with the proxy. + ### Dependencies and setup On macOS the setup and installation instructions above should automatically install all required dependencies. Any error messages you may encounter (for example, with your `pip` version and `cryptography`, or `pillow` and `imagingft` dependencies, or [macOS SSL failures](https://github.com/simonrob/email-oauth2-proxy/issues/14#issuecomment-1077379254)) normally give clear explanations of the issues and point to instructions for resolving these problems. diff --git a/emailproxy.config b/emailproxy.config index e0904f5..d6e290c 100644 --- a/emailproxy.config +++ b/emailproxy.config @@ -27,10 +27,10 @@ documentation = Local servers are specified as demonstrated below where, for exa - The `local_address` property can be used to set an IP address or hostname for the proxy to listen on. Both IPv4 and IPv6 are supported. If not specified, this value is set to `localhost`. When using a hostname the proxy will - first resolve this to an IP address, preferring IPv6 over IPv4 if both are available. When running in an environment - with dual-stack support, the proxy will attempt to listen on both IPv4 and IPv6 hosts simultaneously. To explicitly - request this for the local host, set `local_address = ::`. Note also that tools such as `netstat` do not always - accurately show dual-stack mode; if in doubt it is worth actually testing both IPv4 and IPv6 connections. + first resolve this to an IP address, preferring IPv6 over IPv4 if both are available. When running in an IPv6 + environment with dual-stack support, the proxy will attempt to listen on both IPv4 and IPv6 hosts simultaneously. + To explicitly request this for the local host, set `local_address = ::`. Note also that tools such as `netstat` do + not always accurately show dual-stack mode; if in doubt it is worth actually testing both IPv4 and IPv6 connections. Advanced server configuration: - In the standard configuration the channel between your email client and the proxy is unencrypted. This is not From fdb5483c662caa46396e194650bc00e726917256 Mon Sep 17 00:00:00 2001 From: Simon Robinson Date: Tue, 11 Jul 2023 20:08:23 +0100 Subject: [PATCH 03/21] Make sure `stdin` exists; suppress invalid warnings --- emailproxy.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/emailproxy.py b/emailproxy.py index 65ce43d..5ad54ec 100644 --- a/emailproxy.py +++ b/emailproxy.py @@ -2464,6 +2464,7 @@ def create_authorisation_window(self, request): # noinspection PyDeprecation if pkg_resources.parse_version( pkg_resources.get_distribution('pywebview').version) < pkg_resources.parse_version('3.6'): + # noinspection PyUnresolvedReferences authorisation_window.loaded += self.authorisation_window_loaded else: authorisation_window.events.loaded += self.authorisation_window_loaded @@ -2505,6 +2506,7 @@ def authorisation_window_loaded(self): continue # skip dummy window url = window.get_current_url() + # noinspection PyUnresolvedReferences username = window.get_title(window).split(' ')[-1] # see note above: title *must* match this format if not url or not username: continue # skip any invalid windows @@ -2587,7 +2589,7 @@ def toggle_start_at_login(self, icon, force_rewrite=False): cmd_file.write(windows_start_command) # on Windows we don't have a service to run, but it is still useful to exit the terminal instance - if sys.stdin.isatty() and not recreate_login_file: + if sys.stdin and sys.stdin.isatty() and not recreate_login_file: self.exit(icon, restart_callback=lambda: subprocess.call(windows_start_command, shell=True)) else: os.remove(CMD_FILE_PATH) @@ -2609,7 +2611,7 @@ def toggle_start_at_login(self, icon, force_rewrite=False): desktop_file.write('%s=%s\n' % (key, value)) # like on Windows we don't have a service to run, but it is still useful to exit the terminal instance - if sys.stdin.isatty() and not recreate_login_file: + if sys.stdin and sys.stdin.isatty() and not recreate_login_file: AppConfig.save() # because linux_restart needs to unload to prevent saving on exit self.linux_restart(icon) else: @@ -2879,7 +2881,7 @@ def post_create(self, icon): data['local_server_auth'] = True RESPONSE_QUEUE.put(data) # local server auth is handled by the client/server connections elif self.args.external_auth and self.args.no_gui: - if sys.stdin.isatty(): + if sys.stdin and sys.stdin.isatty(): self.notify(APP_NAME, 'No-GUI external auth mode: please authorise a request for account ' '%s' % data['username']) self.terminal_external_auth_prompt(data) From 01941d2c2e9fb57d15de8472de0e077b6e50a3a3 Mon Sep 17 00:00:00 2001 From: Simon Robinson Date: Tue, 11 Jul 2023 20:19:32 +0100 Subject: [PATCH 04/21] Switch to `::` as the default local address - closes #179 --- README.md | 3 ++- emailproxy.config | 11 +++++------ emailproxy.py | 17 ++++++++++------- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 8329a6d..2aec9ec 100644 --- a/README.md +++ b/README.md @@ -172,7 +172,8 @@ For example, to test the Office 365 IMAP server from the [example configuration] If you have already authorised your account with the proxy you should see a response starting with `a1 OK`; if not, this command should trigger a notification from the proxy about authorising your account. If you are having trouble actually connecting to the proxy, it is always worth double-checking the `local_address` that you are using. -The proxy defaults to `localhost` for this parameter, but this may resolve to different values depending on your environment (see the sample configuration file for further details). +The proxy defaults to `::` for this parameter, which in most cases resolves to `localhost` for both IPv4 and IPv6 configurations, but it is possible that this differs depending on your environment. +If you are unable to connect to the proxy from your client, it is worth setting this value explicitly – see the [sample configuration file](emailproxy.config) for further details about how to do this. Please try to connect to both IPv4 (i.e., `127.0.0.1`) and IPv6 (i.e., `::1`) loopback addresses before reporting any connection issues with the proxy. ### Dependencies and setup diff --git a/emailproxy.config b/emailproxy.config index d6e290c..deb8b87 100644 --- a/emailproxy.config +++ b/emailproxy.config @@ -26,11 +26,11 @@ documentation = Local servers are specified as demonstrated below where, for exa behalf (i.e., do not enable STARTTLS in your client). IMAP STARTTLS and POP STARTTLS are not currently supported. - The `local_address` property can be used to set an IP address or hostname for the proxy to listen on. Both IPv4 - and IPv6 are supported. If not specified, this value is set to `localhost`. When using a hostname the proxy will - first resolve this to an IP address, preferring IPv6 over IPv4 if both are available. When running in an IPv6 - environment with dual-stack support, the proxy will attempt to listen on both IPv4 and IPv6 hosts simultaneously. - To explicitly request this for the local host, set `local_address = ::`. Note also that tools such as `netstat` do - not always accurately show dual-stack mode; if in doubt it is worth actually testing both IPv4 and IPv6 connections. + and IPv6 are supported. If not specified, this value is set to `::` (i.e., dual-stack IPv4 and IPv6 `localhost`). + When a hostname is set the proxy will first resolve this to an IP address, preferring IPv6 over IPv4 if both are + available. When running in an IPv6 environment with dual-stack support, the proxy will attempt to listen on both + IPv4 and IPv6 hosts simultaneously. Note that tools such as `netstat` do not always accurately show dual-stack mode; + if you are having trouble connecting to the proxy, it is worth actually testing both IPv4 and IPv6 connections. Advanced server configuration: - In the standard configuration the channel between your email client and the proxy is unencrypted. This is not @@ -40,7 +40,6 @@ documentation = Local servers are specified as demonstrated below where, for exa these to set up a secure connection between itself and your email client. [IMAP-1993] -local_address = localhost server_address = outlook.office365.com server_port = 993 diff --git a/emailproxy.py b/emailproxy.py index 5ad54ec..04f7fce 100644 --- a/emailproxy.py +++ b/emailproxy.py @@ -6,7 +6,7 @@ __author__ = 'Simon Robinson' __copyright__ = 'Copyright (c) 2023 Simon Robinson' __license__ = 'Apache 2.0' -__version__ = '2023-05-18' # ISO 8601 (YYYY-MM-DD) +__version__ = '2023-07-11' # ISO 8601 (YYYY-MM-DD) import abc import argparse @@ -18,6 +18,7 @@ import enum import errno import io +import ipaddress import json import logging import logging.handlers @@ -1927,8 +1928,7 @@ def create_socket(self, socket_family=socket.AF_UNSPEC, socket_type=socket.SOCK_ socket_family = socket.AF_INET6 if socket_family == socket.AF_UNSPEC else socket_family if socket_family != socket.AF_INET: try: - host, port = self.local_address - socket.getaddrinfo(host, port, socket_family, socket.SOCK_STREAM) + socket.getaddrinfo(self.local_address[0], self.local_address[1], socket_family, socket.SOCK_STREAM) except OSError: socket_family = socket.AF_INET new_socket = socket.socket(socket_family, socket_type) @@ -2373,10 +2373,13 @@ def get_config_menu_servers(proxies, server_type): if not heading_appended: items.append(pystray.MenuItem('%s servers:' % server_type, None, enabled=False)) heading_appended = True + formatted_host = proxy.local_address[0] + with contextlib.suppress(ValueError): + ip = ipaddress.ip_address(formatted_host) + formatted_host = '[%s]' % formatted_host if type(ip) is ipaddress.IPv6Address else formatted_host items.append(pystray.MenuItem('%s %s:%d ➝ %s:%d' % ( - ('Y_SSL' if proxy.ssl_connection else 'N_SSL') if sys.platform == 'darwin' else '', - proxy.local_address[0], proxy.local_address[1], proxy.server_address[0], proxy.server_address[1]), - None, enabled=False)) + ('Y_SSL' if proxy.ssl_connection else 'N_SSL') if sys.platform == 'darwin' else '', formatted_host, + proxy.local_address[1], proxy.server_address[0], proxy.server_address[1]), None, enabled=False)) if heading_appended: items.append(pystray.Menu.SEPARATOR) return items @@ -2740,7 +2743,7 @@ def load_and_start_servers(self, icon=None, reload=True): match = CONFIG_SERVER_MATCHER.match(section) server_type = match.group('type') - local_address = config.get(section, 'local_address', fallback='localhost') + local_address = config.get(section, 'local_address', fallback='::') str_local_port = match.group('port') local_port = -1 try: From db5bdfc39628309d4d6e3ced103c29891196cdc3 Mon Sep 17 00:00:00 2001 From: Simon Robinson Date: Wed, 12 Jul 2023 20:04:27 +0100 Subject: [PATCH 05/21] Better separation of access/refresh token invalidation --- emailproxy.py | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/emailproxy.py b/emailproxy.py index 04f7fce..c14bec5 100644 --- a/emailproxy.py +++ b/emailproxy.py @@ -6,7 +6,7 @@ __author__ = 'Simon Robinson' __copyright__ = 'Copyright (c) 2023 Simon Robinson' __license__ = 'Apache 2.0' -__version__ = '2023-07-11' # ISO 8601 (YYYY-MM-DD) +__version__ = '2023-07-12' # ISO 8601 (YYYY-MM-DD) import abc import argparse @@ -625,8 +625,8 @@ def get_account_with_catch_all_fallback(option): if client_secret_encrypted and not client_secret: client_secret = OAuth2Helper.decrypt(fernet, client_secret_encrypted) - if access_token: - if access_token_expiry - current_time < TOKEN_EXPIRY_MARGIN: # refresh if expiring soon (if possible) + if access_token or refresh_token: # if possible, refresh the existing token(s) + if not access_token or access_token_expiry - current_time < TOKEN_EXPIRY_MARGIN: if refresh_token: response = OAuth2Helper.refresh_oauth2_access_token(token_url, client_id, client_secret, OAuth2Helper.decrypt(fernet, refresh_token)) @@ -698,22 +698,34 @@ def get_account_with_catch_all_fallback(option): return True, oauth2_string except InvalidToken as e: + # we always remove the access token - we can easily request another using the refresh token + has_access_token = True if config.get(username, 'access_token', fallback=None) else False + config.remove_option(username, 'access_token') + config.remove_option(username, 'access_token_expiry') + if has_access_token: + AppConfig.save() + # if invalid details are the reason for failure we remove our cached version and re-authenticate - this can # be disabled by a configuration setting, but note that we always remove credentials on 400 Bad Request if e.args == (400, APP_PACKAGE) or AppConfig.globals().getboolean('delete_account_token_on_password_error', fallback=True): - config.remove_option(username, 'token_salt') - config.remove_option(username, 'access_token') - config.remove_option(username, 'access_token_expiry') - config.remove_option(username, 'refresh_token') - AppConfig.save() + # try authentication again with no cached details - note that if we have just removed an invalid access + # token this will trigger an unnecessary reload from the cache store, but it is worth doing this to + # avoid an unnecessary re-authentication request + recurse_retries = True + + # if this is already a second attempt, remove the refresh token as well, and force re-authentication + if not has_access_token: + config.remove_option(username, 'token_salt') + config.remove_option(username, 'refresh_token') + AppConfig.save() else: - recurse_retries = False # no need to recurse if we are just trying the same credentials again + recurse_retries = has_access_token # no need to recurse if we are trying the same credentials again if recurse_retries: Log.info('Retrying login due to exception while requesting OAuth 2.0 credentials for %s:' % username, Log.error_string(e)) - return OAuth2Helper.get_oauth2_credentials(username, password, recurse_retries=False) + return OAuth2Helper.get_oauth2_credentials(username, password, recurse_retries=has_access_token) Log.error('Invalid password to decrypt', username, 'credentials - aborting login:', Log.error_string(e)) return False, '%s: Login failed - the password for account %s is incorrect' % (APP_NAME, username) From 0c30b74935ea2f5f8a83a853c43694080f8a9c86 Mon Sep 17 00:00:00 2001 From: Simon Robinson Date: Wed, 12 Jul 2023 20:31:24 +0100 Subject: [PATCH 06/21] Consistent, clearer formatting of IPv4 vs. IPv6 addresses --- emailproxy.py | 67 ++++++++++++++++++++++++++++----------------------- 1 file changed, 37 insertions(+), 30 deletions(-) diff --git a/emailproxy.py b/emailproxy.py index 04f7fce..85f7cbd 100644 --- a/emailproxy.py +++ b/emailproxy.py @@ -6,7 +6,7 @@ __author__ = 'Simon Robinson' __copyright__ = 'Copyright (c) 2023 Simon Robinson' __license__ = 'Apache 2.0' -__version__ = '2023-07-11' # ISO 8601 (YYYY-MM-DD) +__version__ = '2023-07-12' # ISO 8601 (YYYY-MM-DD) import abc import argparse @@ -275,6 +275,14 @@ def error(*args): def error_string(error): return getattr(error, 'message', repr(error)) + @staticmethod + def format_host_port(address): + host, port, *_ = address + with contextlib.suppress(ValueError): + ip = ipaddress.ip_address(host) + host = '[%s]' % host if type(ip) is ipaddress.IPv6Address else host + return '%s:%d' % (host, port) + @staticmethod def get_last_error(): error_type, value, _traceback = sys.exc_info() @@ -758,13 +766,13 @@ def start_redirection_receiver_server(token_request): redirect_listen_type = 'redirect_listen_address' if token_request['redirect_listen_address'] else 'redirect_uri' parsed_uri = urllib.parse.urlparse(token_request[redirect_listen_type]) parsed_port = 80 if parsed_uri.port is None else parsed_uri.port - Log.debug('Local server auth mode (%s:%d): starting server to listen for authentication response' % ( - parsed_uri.hostname, parsed_port)) + Log.debug('Local server auth mode (%s): starting server to listen for authentication response' % + Log.format_host_port((parsed_uri.hostname, parsed_port))) class LoggingWSGIRequestHandler(wsgiref.simple_server.WSGIRequestHandler): def log_message(self, _format_string, *args): - Log.debug('Local server auth mode (%s:%d): received authentication response' % ( - parsed_uri.hostname, parsed_port), *args) + Log.debug('Local server auth mode (%s): received authentication response' % Log.format_host_port( + (parsed_uri.hostname, parsed_port)), *args) class RedirectionReceiverWSGIApplication: def __call__(self, environ, start_response): @@ -790,20 +798,22 @@ def __call__(self, environ, start_response): redirection_server.server_close() if 'response_url' in token_request: - Log.debug('Local server auth mode (%s:%d): closing local server and returning response' % ( - parsed_uri.hostname, parsed_port), token_request['response_url']) + Log.debug('Local server auth mode (%s): closing local server and returning response' % + Log.format_host_port((parsed_uri.hostname, parsed_port)), token_request['response_url']) else: # failed, likely because of an incorrect address (e.g., https vs http), but can also be due to timeout - Log.info('Local server auth mode (%s:%d):' % (parsed_uri.hostname, parsed_port), 'request failed - if', - 'this error reoccurs, please check `%s` for' % redirect_listen_type, token_request['username'], - 'is not specified as `https` mistakenly. See the sample configuration file for documentation') + Log.info('Local server auth mode (%s):' % Log.format_host_port((parsed_uri.hostname, parsed_port)), + 'request failed - if this error reoccurs, please check `%s` for' % redirect_listen_type, + token_request['username'], 'is not specified as `https` mistakenly. See the sample ' + 'configuration file for documentation') token_request['expired'] = True except socket.error as e: - Log.error('Local server auth mode (%s:%d):' % (parsed_uri.hostname, parsed_port), 'unable to start local', - 'server. Please check that `%s` for %s is unique across accounts, specifies a port number, and ' - 'is not already in use. See the documentation in the proxy\'s sample configuration file.' % ( - redirect_listen_type, token_request['username']), Log.error_string(e)) + Log.error('Local server auth mode (%s):' % Log.format_host_port((parsed_uri.hostname, parsed_port)), + 'unable to start local server. Please check that `%s` for %s is unique across accounts, ' + 'specifies a port number, and is not already in use. See the documentation in the proxy\'s ' + 'sample configuration file.' % (redirect_listen_type, token_request['username']), + Log.error_string(e)) token_request['expired'] = True del token_request['local_server_auth'] @@ -1114,11 +1124,11 @@ def __init__(self, proxy_type, connection, socket_map, connection_info, server_c bool(custom_configuration['local_certificate_path'] and custom_configuration['local_key_path'])) def info_string(self): - debug_string = '; %s:%d->%s:%d' % (self.connection_info[0], self.connection_info[1], self.server_address[0], - self.server_address[1]) if Log.get_level() == logging.DEBUG else '' + debug_string = '; %s->%s' % (Log.format_host_port(self.connection_info), Log.format_host_port( + self.server_address)) if Log.get_level() == logging.DEBUG else '' account = '; %s' % self.server_connection.authenticated_username if \ self.server_connection and self.server_connection.authenticated_username else '' - return '%s (%s:%d%s%s)' % (self.proxy_type, self.local_address[0], self.local_address[1], debug_string, account) + return '%s (%s%s%s)' % (self.proxy_type, Log.format_host_port(self.local_address), debug_string, account) def handle_read(self): byte_data = self.recv(RECEIVE_BUFFER_SIZE) @@ -1521,10 +1531,10 @@ def create_socket(self, socket_family=socket.AF_UNSPEC, socket_type=socket.SOCK_ return def info_string(self): - debug_string = '; %s:%d->%s:%d' % (self.connection_info[0], self.connection_info[1], self.server_address[0], - self.server_address[1]) if Log.get_level() == logging.DEBUG else '' + debug_string = '; %s->%s' % (Log.format_host_port(self.connection_info), Log.format_host_port( + self.server_address)) if Log.get_level() == logging.DEBUG else '' account = '; %s' % self.authenticated_username if self.authenticated_username else '' - return '%s (%s:%d%s%s)' % (self.proxy_type, self.local_address[0], self.local_address[1], debug_string, account) + return '%s (%s%s%s)' % (self.proxy_type, Log.format_host_port(self.local_address), debug_string, account) def handle_connect(self): Log.debug(self.info_string(), '--> [ Client connected ]') @@ -1861,9 +1871,9 @@ def __init__(self, proxy_type, local_address, server_address, custom_configurati self.client_connections = [] def info_string(self): - return '%s server at %s:%d (%s) proxying %s:%d (%s)' % ( - self.proxy_type, self.local_address[0], self.local_address[1], - 'TLS' if self.ssl_connection else 'unsecured', self.server_address[0], self.server_address[1], + return '%s server at %s (%s) proxying %s (%s)' % ( + self.proxy_type, Log.format_host_port(self.local_address), + 'TLS' if self.ssl_connection else 'unsecured', Log.format_host_port(self.server_address), 'STARTTLS' if self.custom_configuration['starttls'] else 'SSL/TLS') def handle_accept(self): @@ -2373,13 +2383,10 @@ def get_config_menu_servers(proxies, server_type): if not heading_appended: items.append(pystray.MenuItem('%s servers:' % server_type, None, enabled=False)) heading_appended = True - formatted_host = proxy.local_address[0] - with contextlib.suppress(ValueError): - ip = ipaddress.ip_address(formatted_host) - formatted_host = '[%s]' % formatted_host if type(ip) is ipaddress.IPv6Address else formatted_host - items.append(pystray.MenuItem('%s %s:%d ➝ %s:%d' % ( - ('Y_SSL' if proxy.ssl_connection else 'N_SSL') if sys.platform == 'darwin' else '', formatted_host, - proxy.local_address[1], proxy.server_address[0], proxy.server_address[1]), None, enabled=False)) + items.append(pystray.MenuItem('%s %s ➝ %s' % ( + ('Y_SSL' if proxy.ssl_connection else 'N_SSL') if sys.platform == 'darwin' else '', + Log.format_host_port(proxy.local_address), Log.format_host_port(proxy.server_address)), + None, enabled=False)) if heading_appended: items.append(pystray.Menu.SEPARATOR) return items From 4c5ede348eae3922c8d4e6e35bcdf55fa9aa979d Mon Sep 17 00:00:00 2001 From: Simon Robinson Date: Fri, 14 Jul 2023 20:52:02 +0530 Subject: [PATCH 07/21] Ensure the GUI menu updates when new catch-all accounts are seen Closes #175 --- emailproxy.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/emailproxy.py b/emailproxy.py index 85f7cbd..9b5faa0 100644 --- a/emailproxy.py +++ b/emailproxy.py @@ -6,7 +6,7 @@ __author__ = 'Simon Robinson' __copyright__ = 'Copyright (c) 2023 Simon Robinson' __license__ = 'Apache 2.0' -__version__ = '2023-07-12' # ISO 8601 (YYYY-MM-DD) +__version__ = '2023-07-14' # ISO 8601 (YYYY-MM-DD) import abc import argparse @@ -159,6 +159,7 @@ class NSObject: RESPONSE_QUEUE = queue.Queue() # responses from user WEBVIEW_QUEUE = queue.Queue() # authentication window events (macOS only) QUEUE_SENTINEL = object() # object to send to signify queues should exit loops +MENU_UPDATE = object() # object to send to trigger a force-refresh of the GUI menu (new catch-all account added) PLIST_FILE_PATH = pathlib.Path('~/Library/LaunchAgents/%s.plist' % APP_PACKAGE).expanduser() # launchctl file location CMD_FILE_PATH = pathlib.Path('~/AppData/Roaming/Microsoft/Windows/Start Menu/Programs/Startup/%s.cmd' % @@ -681,6 +682,7 @@ def get_account_with_catch_all_fallback(option): access_token = response['access_token'] if not config.has_section(username): AppConfig.add_account(username) # in wildcard mode the section may not yet exist + REQUEST_QUEUE.put(MENU_UPDATE) # make sure the menu shows the newly-added account config.set(username, 'token_salt', token_salt) config.set(username, 'access_token', OAuth2Helper.encrypt(fernet, access_token)) config.set(username, 'access_token_expiry', str(current_time + response['expires_in'])) @@ -2881,6 +2883,10 @@ def post_create(self, icon): data = REQUEST_QUEUE.get() # note: blocking call if data is QUEUE_SENTINEL: # app is closing break + if data is MENU_UPDATE: + if icon: + icon.update_menu() + break if not data['expired']: Log.info('Authorisation request received for', data['username'], '(local server auth mode)' if self.args.local_server_auth else '(external auth mode)' if From 9c42b6b4ab98e78aac9b3383cfedba00bfe3d4d4 Mon Sep 17 00:00:00 2001 From: Simon Robinson Date: Sat, 15 Jul 2023 22:21:30 +0530 Subject: [PATCH 08/21] Separate token decryption errors from token renewal errors --- emailproxy.py | 62 ++++++++++++++++++++++++++------------------------- 1 file changed, 32 insertions(+), 30 deletions(-) diff --git a/emailproxy.py b/emailproxy.py index c14bec5..270f357 100644 --- a/emailproxy.py +++ b/emailproxy.py @@ -6,7 +6,7 @@ __author__ = 'Simon Robinson' __copyright__ = 'Copyright (c) 2023 Simon Robinson' __license__ = 'Apache 2.0' -__version__ = '2023-07-12' # ISO 8601 (YYYY-MM-DD) +__version__ = '2023-07-13' # ISO 8601 (YYYY-MM-DD) import abc import argparse @@ -539,8 +539,11 @@ def _save_cache(cache_store_identifier, output_config_parser): class OAuth2Helper: + class TokenRefreshError(Exception): + pass + @staticmethod - def get_oauth2_credentials(username, password, recurse_retries=True): + def get_oauth2_credentials(username, password, reload_remote_accounts=True): """Using the given username (i.e., email address) and password, reads account details from AppConfig and handles OAuth 2.0 token request and renewal, saving the updated details back to AppConfig (or removing them if invalid). Returns either (True, '[OAuth2 string for authentication]') or (False, '[Error message]')""" @@ -602,9 +605,9 @@ def get_account_with_catch_all_fallback(option): refresh_token = config.get(username, 'refresh_token', fallback=None) # try reloading remotely cached tokens if possible - if not access_token and CACHE_STORE != CONFIG_FILE_PATH and recurse_retries: + if not access_token and CACHE_STORE != CONFIG_FILE_PATH and reload_remote_accounts: AppConfig.reload() - return OAuth2Helper.get_oauth2_credentials(username, password, recurse_retries=False) + return OAuth2Helper.get_oauth2_credentials(username, password, reload_remote_accounts=False) # we hash locally-stored tokens with the given password if not token_salt: @@ -697,35 +700,34 @@ def get_account_with_catch_all_fallback(option): oauth2_string = OAuth2Helper.construct_oauth2_string(username, access_token) return True, oauth2_string - except InvalidToken as e: - # we always remove the access token - we can easily request another using the refresh token + except OAuth2Helper.TokenRefreshError as e: + # always clear access tokens - can easily request another via the refresh token (with no user interaction) has_access_token = True if config.get(username, 'access_token', fallback=None) else False config.remove_option(username, 'access_token') config.remove_option(username, 'access_token_expiry') - if has_access_token: - AppConfig.save() - # if invalid details are the reason for failure we remove our cached version and re-authenticate - this can - # be disabled by a configuration setting, but note that we always remove credentials on 400 Bad Request - if e.args == (400, APP_PACKAGE) or AppConfig.globals().getboolean('delete_account_token_on_password_error', - fallback=True): - # try authentication again with no cached details - note that if we have just removed an invalid access - # token this will trigger an unnecessary reload from the cache store, but it is worth doing this to - # avoid an unnecessary re-authentication request - recurse_retries = True - - # if this is already a second attempt, remove the refresh token as well, and force re-authentication - if not has_access_token: - config.remove_option(username, 'token_salt') - config.remove_option(username, 'refresh_token') - AppConfig.save() - else: - recurse_retries = has_access_token # no need to recurse if we are trying the same credentials again + if not has_access_token: + # if this is already a second failure, remove the refresh token as well, and force re-authentication + config.remove_option(username, 'token_salt') + config.remove_option(username, 'refresh_token') + + AppConfig.save() + + Log.info('Retrying login due to exception while refreshing OAuth 2.0 tokens for', username, + '(attempt %d):' % (1 if has_access_token else 2), Log.error_string(e)) + return OAuth2Helper.get_oauth2_credentials(username, password, reload_remote_accounts=False) + + except InvalidToken as e: + if AppConfig.globals().getboolean('delete_account_token_on_password_error', fallback=True): + config.remove_option(username, 'access_token') + config.remove_option(username, 'access_token_expiry') + config.remove_option(username, 'token_salt') + config.remove_option(username, 'refresh_token') + AppConfig.save() - if recurse_retries: - Log.info('Retrying login due to exception while requesting OAuth 2.0 credentials for %s:' % username, - Log.error_string(e)) - return OAuth2Helper.get_oauth2_credentials(username, password, recurse_retries=has_access_token) + Log.info('Retrying login due to exception while decrypting OAuth 2.0 credentials for', username, + '(invalid password):', Log.error_string(e)) + return OAuth2Helper.get_oauth2_credentials(username, password, reload_remote_accounts=False) Log.error('Invalid password to decrypt', username, 'credentials - aborting login:', Log.error_string(e)) return False, '%s: Login failed - the password for account %s is incorrect' % (APP_NAME, username) @@ -930,8 +932,8 @@ def refresh_oauth2_access_token(token_url, client_id, client_secret, refresh_tok except urllib.error.HTTPError as e: e.message = json.loads(e.read()) Log.debug('Error refreshing access token - received invalid response:', e.message) - if e.code == 400: # 400 Bad Request typically means re-authentication is required (refresh token expired) - raise InvalidToken(e.code, APP_PACKAGE) from e + if e.code == 400: # 400 Bad Request typically means re-authentication is required (token expired) + raise OAuth2Helper.TokenRefreshError from e raise e @staticmethod From 56c4d7beb03faec96d96c395534256daebabb00d Mon Sep 17 00:00:00 2001 From: Simon Robinson Date: Thu, 10 Aug 2023 08:55:55 +0100 Subject: [PATCH 09/21] Catch additional connection errors; update Docker sample link Closes #186 --- README.md | 4 ++-- emailproxy.py | 7 ++++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 2aec9ec..8b3299f 100644 --- a/README.md +++ b/README.md @@ -73,7 +73,7 @@ See the [sample configuration file](emailproxy.config) for further details. ## Optional arguments and configuration When starting the proxy there are several optional arguments that can be set to customise its behaviour. -- `--no-gui` will launch the proxy without an icon, which allows it to be run as a `systemctl` service as demonstrated in [this example](https://github.com/simonrob/email-oauth2-proxy/issues/2#issuecomment-839713677), or fully headless as demonstrated in [various](https://github.com/michaelstepner/email-oauth2-proxy-aws) [other](https://github.com/interone-ms/email-oauth2-proxy/commits/feature/docker-build) subprojects. +- `--no-gui` will launch the proxy without an icon, which allows it to be run as a `systemctl` service as demonstrated in [this example](https://github.com/simonrob/email-oauth2-proxy/issues/2#issuecomment-839713677), or fully headless as demonstrated in [various](https://github.com/michaelstepner/email-oauth2-proxy-aws) [other](https://github.com/blacktirion/email-oauth2-proxy-docker) subprojects. Please note that on its own this mode is only of use if you have already authorised your accounts through the proxy in GUI mode, or are importing a pre-authorised proxy configuration file from elsewhere. Unless this option is used in conjunction with `--external-auth` or `--local-server-auth`, accounts that have not yet been authorised (or for whatever reason require re-authorisation) will time out when authenticating, and an error will be printed to the log. @@ -230,7 +230,7 @@ See the documentation and examples in this branch for further details, additiona ## Related projects and alternatives Michael Stepner has created a [Terraform configuration](https://github.com/michaelstepner/email-oauth2-proxy-aws) that helps run this proxy on a lightweight cloud server (AWS EC2). Thiago Macieira has provided a [makefile and systemd configuration files](https://github.com/thiagomacieira/email-oauth2-proxy/tree/Add_a_Makefile_and_systemd_configuration_files_to_install_system_wide). -For Docker, interone-ms has provided an [example configuration](https://github.com/interone-ms/email-oauth2-proxy/commits/feature/docker-build) (though please note that the fork is otherwise outdated, and it is better to use this repository for the proxy script itself). +For Docker, blacktirion has an [example configuration](https://github.com/blacktirion/email-oauth2-proxy-docker). If you already use postfix, the [sasl-xoauth2](https://github.com/tarickb/sasl-xoauth2) plugin is probably a better solution than running this proxy. Similarly, if you use an application that is able to handle OAuth 2.0 tokens but just cannot retrieve them itself, then [pizauth](https://github.com/ltratt/pizauth), [mailctl](https://github.com/pdobsan/mailctl) or [oauth-helper-office-365](https://github.com/ahrex/oauth-helper-office-365) may be more appropriate. diff --git a/emailproxy.py b/emailproxy.py index 9b5faa0..fc35e56 100644 --- a/emailproxy.py +++ b/emailproxy.py @@ -6,7 +6,7 @@ __author__ = 'Simon Robinson' __copyright__ = 'Copyright (c) 2023 Simon Robinson' __license__ = 'Apache 2.0' -__version__ = '2023-07-14' # ISO 8601 (YYYY-MM-DD) +__version__ = '2023-08-10' # ISO 8601 (YYYY-MM-DD) import abc import argparse @@ -1610,9 +1610,10 @@ def handle_error(self): error_type, value = Log.get_last_error() if error_type == TimeoutError and value.errno == errno.ETIMEDOUT or \ issubclass(error_type, ConnectionError) and value.errno in [errno.ECONNRESET, errno.ECONNREFUSED] or \ - error_type == OSError and value.errno in [0, errno.ENETDOWN, errno.EHOSTUNREACH]: + error_type == OSError and value.errno in [0, errno.ENETDOWN, errno.EHOSTDOWN, errno.EHOSTUNREACH]: # TimeoutError 60 = 'Operation timed out'; ConnectionError 54 = 'Connection reset by peer', 61 = 'Connection - # refused; OSError 0 = 'Error' (typically network failure), 50 = 'Network is down', 65 = 'No route to host' + # refused; OSError 0 = 'Error' (typically network failure), 50 = 'Network is down', 64 = 'Host is down'; + # 65 = 'No route to host' Log.info(self.info_string(), 'Caught network error (server) - is there a network connection?', 'Error type', error_type, 'with message:', value) self.close() From f7b165e8b0c51fb1cd59c402550f9cf4c991af3e Mon Sep 17 00:00:00 2001 From: Simon Robinson Date: Thu, 10 Aug 2023 19:37:50 +0100 Subject: [PATCH 10/21] Add hint about encryption mismatch Note: this is not triggered in all cases, and depends on client behaviour (e.g., Geary triggers this; `openssl s_client` does not) See #185 --- README.md | 3 +++ emailproxy.py | 5 ++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 8b3299f..9e39fd3 100644 --- a/README.md +++ b/README.md @@ -171,6 +171,9 @@ This can be achieved using `telnet`, [PuTTY](https://www.chiark.greenend.org.uk/ For example, to test the Office 365 IMAP server from the [example configuration](emailproxy.config), first open a connection using `telnet localhost 1993`, and then send a login command: `a1 login e@mail.com password`, replacing `e@mail.com` with your email address, and `password` with any value you like during testing (see above for why the password is irrelevant). If you have already authorised your account with the proxy you should see a response starting with `a1 OK`; if not, this command should trigger a notification from the proxy about authorising your account. +If you are using a [secure local connection](emailproxy.config) the interaction with the remote email server is the same as above, but you will need to use a local debugging tool that supports encryption. +The easiest approach here is to use [OpenSSL](https://www.openssl.org/): `openssl s_client -crlf -connect localhost:1993`. + If you are having trouble actually connecting to the proxy, it is always worth double-checking the `local_address` that you are using. The proxy defaults to `::` for this parameter, which in most cases resolves to `localhost` for both IPv4 and IPv6 configurations, but it is possible that this differs depending on your environment. If you are unable to connect to the proxy from your client, it is worth setting this value explicitly – see the [sample configuration file](emailproxy.config) for further details about how to do this. diff --git a/emailproxy.py b/emailproxy.py index fc35e56..c280ca8 100644 --- a/emailproxy.py +++ b/emailproxy.py @@ -1205,7 +1205,10 @@ def log_info(self, message, message_type='info'): def handle_close(self): error_type, value = Log.get_last_error() if error_type and value: - Log.info(self.info_string(), 'Caught connection error (client) -', error_type.__name__, ':', value) + message = 'Caught connection error (client)' + if error_type == ConnectionResetError: + message = '%s [ Are you attempting an encrypted connection to a non-encrypted server? ]' % message + Log.info(self.info_string(), message, '-', error_type.__name__, ':', value) self.close() def close(self): From bb93f2e092e5a3409781e0b24b390d04061f5f87 Mon Sep 17 00:00:00 2001 From: Simon Robinson Date: Fri, 11 Aug 2023 20:39:05 +0100 Subject: [PATCH 11/21] Work around pystray issue; suppress unnecessary warning --- emailproxy.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/emailproxy.py b/emailproxy.py index c280ca8..5f4de88 100644 --- a/emailproxy.py +++ b/emailproxy.py @@ -6,7 +6,7 @@ __author__ = 'Simon Robinson' __copyright__ = 'Copyright (c) 2023 Simon Robinson' __license__ = 'Apache 2.0' -__version__ = '2023-08-10' # ISO 8601 (YYYY-MM-DD) +__version__ = '2023-08-11' # ISO 8601 (YYYY-MM-DD) import abc import argparse @@ -68,8 +68,10 @@ no_gui_parser.add_argument('--external-auth', action='store_true') no_gui_args = no_gui_parser.parse_known_args()[0] if not no_gui_args.no_gui: - # noinspection PyDeprecation - import pkg_resources # from setuptools - to be changed to importlib.metadata and packaging.version once 3.8 is min. + with warnings.catch_warnings(): + warnings.simplefilter('ignore', DeprecationWarning) + # noinspection PyDeprecation + import pkg_resources # from setuptools - to change to importlib.metadata and packaging.version once min. is 3.8 import pystray # the menu bar/taskbar GUI import timeago # the last authenticated activity hint from PIL import Image, ImageDraw, ImageFont # draw the menu bar icon from the TTF font stored in APP_ICON @@ -2286,6 +2288,7 @@ def macos_nsworkspace_notification_listener_(self, notification): self.exit(self.icon) def create_icon(self): + Image.ANTIALIAS = Image.LANCZOS # temporary fix for pystray incompatibility with PIL >= 10.0.0 icon_class = RetinaIcon if sys.platform == 'darwin' else pystray.Icon return icon_class(APP_NAME, App.get_image(), APP_NAME, menu=pystray.Menu( pystray.MenuItem('Servers and accounts', pystray.Menu(self.create_config_menu)), From c9f54ced678f712a8d80abe1bf8ba876e9d37bd9 Mon Sep 17 00:00:00 2001 From: Simon Robinson Date: Wed, 16 Aug 2023 18:10:03 +0100 Subject: [PATCH 12/21] Add locking wrapper to ConfigParser --- emailproxy.py | 121 +++++++++++++++++++++++++++++++------------------- 1 file changed, 75 insertions(+), 46 deletions(-) diff --git a/emailproxy.py b/emailproxy.py index 5f4de88..bf3a0aa 100644 --- a/emailproxy.py +++ b/emailproxy.py @@ -6,7 +6,7 @@ __author__ = 'Simon Robinson' __copyright__ = 'Copyright (c) 2023 Simon Robinson' __license__ = 'Apache 2.0' -__version__ = '2023-08-11' # ISO 8601 (YYYY-MM-DD) +__version__ = '2023-08-16' # ISO 8601 (YYYY-MM-DD) import abc import argparse @@ -405,16 +405,61 @@ def save(store_id, config_dict, create_secret=True): Log.error('Unable to get AWS SDK client; cannot cache credentials to AWS Secrets Manager') +class ConcurrentConfigParser: + """Add locking to a ConfigParser object""" + + def __init__(self): + self.config = configparser.ConfigParser() + self.lock = threading.Lock() + + def read(self, filename): + with self.lock: + self.config.read(filename) + + def sections(self): + with self.lock: + return self.config.sections() + + def add_section(self, section): + with self.lock: + self.config.add_section(section) + + def get(self, section, option, fallback=None): + with self.lock: + if not self.config.has_section(section): + return fallback + return self.config.get(section, option, fallback=fallback) + + def getint(self, section, option, fallback=None): + with self.lock: + return self.config.getint(section, option, fallback=fallback) + + def getboolean(self, section, option, fallback=None): + with self.lock: + return self.config.getboolean(section, option, fallback=fallback) + + def set(self, section, option, value): + with self.lock: + if not self.config.has_section(section): + self.config.add_section(section) + self.config.set(section, option, value) + + def remove_option(self, section, option): + with self.lock: + if self.config.has_option(section, option): + self.config.remove_option(section, option) + + def write(self, file): + with self.lock: + self.config.write(file) + + class AppConfig: """Helper wrapper around ConfigParser to cache servers/accounts, and avoid writing to the file until necessary""" _PARSER = None _LOADED = False - _GLOBALS = None - _SERVERS = [] - _ACCOUNTS = [] - # note: removing the unencrypted version of `client_secret_encrypted` is not automatic with --cache-store (see docs) _CACHED_OPTION_KEYS = ['token_salt', 'access_token', 'access_token_expiry', 'refresh_token', 'last_activity', 'client_secret_encrypted'] @@ -425,20 +470,15 @@ class AppConfig: @staticmethod def _load(): AppConfig.unload() - AppConfig._PARSER = configparser.ConfigParser() + AppConfig._PARSER = ConcurrentConfigParser() AppConfig._PARSER.read(CONFIG_FILE_PATH) - config_sections = AppConfig._PARSER.sections() - if APP_SHORT_NAME in config_sections: - AppConfig._GLOBALS = AppConfig._PARSER[APP_SHORT_NAME] - else: - AppConfig._GLOBALS = configparser.SectionProxy(AppConfig._PARSER, APP_SHORT_NAME) - # cached account credentials can be stored in the configuration file (default) or, via `--cache-store`, a # separate local file or external service (such as a secrets manager) - we combine these sources at load time if CACHE_STORE != CONFIG_FILE_PATH: # it would be cleaner to avoid specific options here, but best to load unexpected sections only when enabled - allow_catch_all_accounts = AppConfig._GLOBALS.getboolean('allow_catch_all_accounts', fallback=False) + allow_catch_all_accounts = AppConfig._PARSER.getboolean(APP_SHORT_NAME, 'allow_catch_all_accounts', + fallback=False) cache_file_parser = AppConfig._load_cache(CACHE_STORE) cache_file_accounts = [s for s in cache_file_parser.sections() if '@' in s] @@ -449,12 +489,6 @@ def _load(): if option in AppConfig._CACHED_OPTION_KEYS: AppConfig._PARSER.set(account, option, cache_file_parser.get(account, option)) - if allow_catch_all_accounts: - config_sections = AppConfig._PARSER.sections() # new sections may have been added - - AppConfig._SERVERS = [s for s in config_sections if CONFIG_SERVER_MATCHER.match(s)] - AppConfig._ACCOUNTS = [s for s in config_sections if '@' in s] - AppConfig._LOADED = True @staticmethod @@ -478,34 +512,25 @@ def unload(): AppConfig._PARSER = None AppConfig._LOADED = False - AppConfig._GLOBALS = None - AppConfig._SERVERS = [] - AppConfig._ACCOUNTS = [] - @staticmethod def reload(): AppConfig.unload() return AppConfig.get() @staticmethod - def globals(): + def get_global(name, fallback): AppConfig.get() # make sure config is loaded - return AppConfig._GLOBALS + return AppConfig._PARSER.getboolean(APP_SHORT_NAME, name, fallback) @staticmethod def servers(): AppConfig.get() # make sure config is loaded - return AppConfig._SERVERS + return [s for s in AppConfig._PARSER.sections() if CONFIG_SERVER_MATCHER.match(s)] @staticmethod def accounts(): AppConfig.get() # make sure config is loaded - return AppConfig._ACCOUNTS - - @staticmethod - def add_account(username): - AppConfig._PARSER.add_section(username) - AppConfig._ACCOUNTS = [s for s in AppConfig._PARSER.sections() if '@' in s] + return [s for s in AppConfig._PARSER.sections() if '@' in s] @staticmethod def save(): @@ -513,18 +538,21 @@ def save(): if CACHE_STORE != CONFIG_FILE_PATH: # in `--cache-store` mode we ignore everything except _CACHED_OPTION_KEYS (OAuth 2.0 tokens, etc) output_config_parser = configparser.ConfigParser() - output_config_parser.read_dict(AppConfig._PARSER) # a deep copy of the current configuration + with AppConfig._PARSER.lock: + output_config_parser.read_dict(AppConfig._PARSER) # a deep copy of the current configuration + config_accounts = AppConfig.accounts() - for account in AppConfig._ACCOUNTS: + for account in config_accounts: for option in output_config_parser.options(account): if option not in AppConfig._CACHED_OPTION_KEYS: output_config_parser.remove_option(account, option) for section in output_config_parser.sections(): - if section not in AppConfig._ACCOUNTS or len(output_config_parser.options(section)) <= 0: + if section not in config_accounts or len(output_config_parser.options(section)) <= 0: output_config_parser.remove_section(section) - AppConfig._save_cache(CACHE_STORE, output_config_parser) + with AppConfig._PARSER.lock: + AppConfig._save_cache(CACHE_STORE, output_config_parser) else: # by default we cache to the local configuration file, and rewrite all values each time @@ -557,10 +585,11 @@ def get_oauth2_credentials(username, password, recurse_retries=True): if invalid). Returns either (True, '[OAuth2 string for authentication]') or (False, '[Error message]')""" # we support broader catch-all account names (e.g., `@domain.com` / `@`) if enabled - valid_accounts = [username in AppConfig.accounts()] - if AppConfig.globals().getboolean('allow_catch_all_accounts', fallback=False): + config_accounts = AppConfig.accounts() + valid_accounts = [username in config_accounts] + if AppConfig.get_global('allow_catch_all_accounts', fallback=False): user_domain = '@%s' % username.split('@')[-1] - valid_accounts.extend([account in AppConfig.accounts() for account in [user_domain, '@']]) + valid_accounts.extend([account in config_accounts for account in [user_domain, '@']]) if not any(valid_accounts): Log.error('Proxy config file entry missing for account', username, '- aborting login') @@ -572,7 +601,7 @@ def get_oauth2_credentials(username, password, recurse_retries=True): def get_account_with_catch_all_fallback(option): fallback = None - if AppConfig.globals().getboolean('allow_catch_all_accounts', fallback=False): + if AppConfig.get_global('allow_catch_all_accounts', fallback=False): fallback = config.get(user_domain, option, fallback=config.get('@', option, fallback=None)) return config.get(username, option, fallback=fallback) @@ -682,8 +711,8 @@ def get_account_with_catch_all_fallback(option): oauth2_flow, username, password) access_token = response['access_token'] - if not config.has_section(username): - AppConfig.add_account(username) # in wildcard mode the section may not yet exist + if username not in config.sections(): + config.add_section(username) # in wildcard mode the section may not yet exist REQUEST_QUEUE.put(MENU_UPDATE) # make sure the menu shows the newly-added account config.set(username, 'token_salt', token_salt) config.set(username, 'access_token', OAuth2Helper.encrypt(fernet, access_token)) @@ -695,7 +724,7 @@ def get_account_with_catch_all_fallback(option): Log.info('Warning: no refresh token returned for', username, '- you will need to re-authenticate', 'each time the access token expires (does your `oauth2_scope` value allow `offline` use?)') - if AppConfig.globals().getboolean('encrypt_client_secret_on_first_use', fallback=False): + if AppConfig.get_global('encrypt_client_secret_on_first_use', fallback=False): if client_secret: # note: save to the `username` entry even if `user_domain` exists, avoiding conflicts when using # incompatible `encrypt_client_secret_on_first_use` and `allow_catch_all_accounts` options @@ -712,8 +741,8 @@ def get_account_with_catch_all_fallback(option): except InvalidToken as e: # if invalid details are the reason for failure we remove our cached version and re-authenticate - this can # be disabled by a configuration setting, but note that we always remove credentials on 400 Bad Request - if e.args == (400, APP_PACKAGE) or AppConfig.globals().getboolean('delete_account_token_on_password_error', - fallback=True): + if e.args == (400, APP_PACKAGE) or AppConfig.get_global('delete_account_token_on_password_error', + fallback=True): config.remove_option(username, 'token_salt') config.remove_option(username, 'access_token') config.remove_option(username, 'access_token_expiry') @@ -2360,7 +2389,7 @@ def create_config_menu(self): if len(config_accounts) <= 0: items.append(pystray.MenuItem(' No accounts configured', None, enabled=False)) else: - catch_all_enabled = AppConfig.globals().getboolean('allow_catch_all_accounts', fallback=False) + catch_all_enabled = AppConfig.get_global('allow_catch_all_accounts', fallback=False) catch_all_accounts = [] for account in config_accounts: if account.startswith('@') and catch_all_enabled: From c807c4b68555a5a27a4b26c7a04bc01be94a7f6a Mon Sep 17 00:00:00 2001 From: Simon Robinson Date: Wed, 16 Aug 2023 20:26:26 +0100 Subject: [PATCH 13/21] Adjust Windows icon colour based on system theme --- emailproxy.py | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/emailproxy.py b/emailproxy.py index 5f4de88..35c7a22 100644 --- a/emailproxy.py +++ b/emailproxy.py @@ -6,7 +6,7 @@ __author__ = 'Simon Robinson' __copyright__ = 'Copyright (c) 2023 Simon Robinson' __license__ = 'Apache 2.0' -__version__ = '2023-08-11' # ISO 8601 (YYYY-MM-DD) +__version__ = '2023-08-16' # ISO 8601 (YYYY-MM-DD) import abc import argparse @@ -2288,7 +2288,9 @@ def macos_nsworkspace_notification_listener_(self, notification): self.exit(self.icon) def create_icon(self): - Image.ANTIALIAS = Image.LANCZOS # temporary fix for pystray incompatibility with PIL >= 10.0.0 + with warnings.catch_warnings(): + warnings.simplefilter('ignore', DeprecationWarning) + Image.ANTIALIAS = Image.LANCZOS # temporary fix for pystray <= 0.19.4 incompatibility with PIL >= 10.0.0 icon_class = RetinaIcon if sys.platform == 'darwin' else pystray.Icon return icon_class(APP_NAME, App.get_image(), APP_NAME, menu=pystray.Menu( pystray.MenuItem('Servers and accounts', pystray.Menu(self.create_config_menu)), @@ -2302,12 +2304,23 @@ def create_icon(self): @staticmethod def get_image(): # we use an icon font for better multiplatform compatibility and icon size flexibility - icon_colour = 'white' # note: value is irrelevant on macOS - we set as a template to get the platform's colours + icon_colour = 'white' # see below: colour is handled differently per-platform icon_character = 'e' icon_background_width = 44 icon_background_height = 44 icon_width = 40 # to allow for padding between icon and background image size + # the colour value is irrelevant on macOS - we configure the menu bar icon as a template to get the platform's + # colours - but on Windows (and in future potentially Linux) we need to set based on the current theme type + if sys.platform == 'win32': + import winreg + try: + key = winreg.OpenKey(winreg.HKEY_CURRENT_USER, + r'Software\Microsoft\Windows\CurrentVersion\Themes\Personalize') + icon_colour = 'white' if winreg.QueryValueEx(key, 'AppsUseLightTheme')[0] == 0 else 'black' + except FileNotFoundError: + pass + # find the largest font size that will let us draw the icon within the available width minimum_font_size = 1 maximum_font_size = 255 From 10cbfacb71c0eaeb067e3edddc0641cfb75747e4 Mon Sep 17 00:00:00 2001 From: Simon Robinson Date: Wed, 16 Aug 2023 20:45:24 +0100 Subject: [PATCH 14/21] Only apply pystray workaround if pillow is >= 10.0.0 --- emailproxy.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/emailproxy.py b/emailproxy.py index 35c7a22..75a073b 100644 --- a/emailproxy.py +++ b/emailproxy.py @@ -2290,7 +2290,10 @@ def macos_nsworkspace_notification_listener_(self, notification): def create_icon(self): with warnings.catch_warnings(): warnings.simplefilter('ignore', DeprecationWarning) - Image.ANTIALIAS = Image.LANCZOS # temporary fix for pystray <= 0.19.4 incompatibility with PIL >= 10.0.0 + # noinspection PyDeprecation + if pkg_resources.parse_version( + pkg_resources.get_distribution('pillow').version) >= pkg_resources.parse_version('10.0.0'): + Image.ANTIALIAS = Image.LANCZOS # temporary fix for pystray <= 0.19.4 incompatibility with PIL 10.0.0+ icon_class = RetinaIcon if sys.platform == 'darwin' else pystray.Icon return icon_class(APP_NAME, App.get_image(), APP_NAME, menu=pystray.Menu( pystray.MenuItem('Servers and accounts', pystray.Menu(self.create_config_menu)), From 013f3feddc6b3a300ccea49523cebe956beb7c87 Mon Sep 17 00:00:00 2001 From: Simon Robinson Date: Thu, 24 Aug 2023 20:36:40 +0100 Subject: [PATCH 15/21] More helpful error message when local certificate/key is not present --- emailproxy.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/emailproxy.py b/emailproxy.py index 5f4de88..cafb587 100644 --- a/emailproxy.py +++ b/emailproxy.py @@ -6,7 +6,7 @@ __author__ = 'Simon Robinson' __copyright__ = 'Copyright (c) 2023 Simon Robinson' __license__ = 'Apache 2.0' -__version__ = '2023-08-11' # ISO 8601 (YYYY-MM-DD) +__version__ = '2023-08-24' # ISO 8601 (YYYY-MM-DD) import abc import argparse @@ -1957,8 +1957,11 @@ def create_socket(self, socket_family=socket.AF_UNSPEC, socket_type=socket.SOCK_ if self.ssl_connection: # noinspection PyTypeChecker ssl_context = ssl.create_default_context(purpose=ssl.Purpose.CLIENT_AUTH) - ssl_context.load_cert_chain(certfile=self.custom_configuration['local_certificate_path'], - keyfile=self.custom_configuration['local_key_path']) + try: + ssl_context.load_cert_chain(certfile=self.custom_configuration['local_certificate_path'], + keyfile=self.custom_configuration['local_key_path']) + except FileNotFoundError as e: + raise FileNotFoundError('Unable to open `local_certificate_path` and/or `local_key_path`') from e # suppress_ragged_eofs=True: see test_ssl.py documentation in https://github.com/python/cpython/pull/5266 self.set_socket(ssl_context.wrap_socket(new_socket, server_side=True, suppress_ragged_eofs=True, From 952b2555facebdae6526bfbf7b0c770f9b05de60 Mon Sep 17 00:00:00 2001 From: Simon Robinson Date: Fri, 25 Aug 2023 22:22:35 +0100 Subject: [PATCH 16/21] Fix regression in 4c5ede3 that caused an exit after catch-all auth Fixes #190 --- emailproxy.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/emailproxy.py b/emailproxy.py index cafb587..48ada13 100644 --- a/emailproxy.py +++ b/emailproxy.py @@ -6,7 +6,7 @@ __author__ = 'Simon Robinson' __copyright__ = 'Copyright (c) 2023 Simon Robinson' __license__ = 'Apache 2.0' -__version__ = '2023-08-24' # ISO 8601 (YYYY-MM-DD) +__version__ = '2023-08-25' # ISO 8601 (YYYY-MM-DD) import abc import argparse @@ -2896,7 +2896,7 @@ def post_create(self, icon): if data is MENU_UPDATE: if icon: icon.update_menu() - break + continue if not data['expired']: Log.info('Authorisation request received for', data['username'], '(local server auth mode)' if self.args.local_server_auth else '(external auth mode)' if From df420ad802ca211f134b9814796b898acf92027d Mon Sep 17 00:00:00 2001 From: Simon Robinson Date: Mon, 28 Aug 2023 22:53:24 +0100 Subject: [PATCH 17/21] Target pystray <= 0.19.4 only now that PR #147 has been merged --- emailproxy.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/emailproxy.py b/emailproxy.py index 75a073b..6c0cca2 100644 --- a/emailproxy.py +++ b/emailproxy.py @@ -6,7 +6,7 @@ __author__ = 'Simon Robinson' __copyright__ = 'Copyright (c) 2023 Simon Robinson' __license__ = 'Apache 2.0' -__version__ = '2023-08-16' # ISO 8601 (YYYY-MM-DD) +__version__ = '2023-08-28' # ISO 8601 (YYYY-MM-DD) import abc import argparse @@ -2288,12 +2288,15 @@ def macos_nsworkspace_notification_listener_(self, notification): self.exit(self.icon) def create_icon(self): + # temporary fix for pystray <= 0.19.4 incompatibility with PIL 10.0.0+; fixed once pystray PR #147 is released with warnings.catch_warnings(): warnings.simplefilter('ignore', DeprecationWarning) # noinspection PyDeprecation - if pkg_resources.parse_version( - pkg_resources.get_distribution('pillow').version) >= pkg_resources.parse_version('10.0.0'): - Image.ANTIALIAS = Image.LANCZOS # temporary fix for pystray <= 0.19.4 incompatibility with PIL 10.0.0+ + pystray_version = pkg_resources.get_distribution('pystray').version + pillow_version = pkg_resources.get_distribution('pillow').version + if pkg_resources.parse_version(pystray_version) <= pkg_resources.parse_version('0.19.4') and \ + pkg_resources.parse_version(pillow_version) >= pkg_resources.parse_version('10.0.0'): + Image.ANTIALIAS = Image.LANCZOS icon_class = RetinaIcon if sys.platform == 'darwin' else pystray.Icon return icon_class(APP_NAME, App.get_image(), APP_NAME, menu=pystray.Menu( pystray.MenuItem('Servers and accounts', pystray.Menu(self.create_config_menu)), From 246ec314e17aea4f000d02657cc36ce8bd7c2a78 Mon Sep 17 00:00:00 2001 From: Simon Robinson Date: Sat, 2 Sep 2023 20:41:54 +0100 Subject: [PATCH 18/21] Check `SystemUsesLightTheme` rather than `AppsUseLightTheme` --- emailproxy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/emailproxy.py b/emailproxy.py index 6c0cca2..acd9320 100644 --- a/emailproxy.py +++ b/emailproxy.py @@ -2323,7 +2323,7 @@ def get_image(): try: key = winreg.OpenKey(winreg.HKEY_CURRENT_USER, r'Software\Microsoft\Windows\CurrentVersion\Themes\Personalize') - icon_colour = 'white' if winreg.QueryValueEx(key, 'AppsUseLightTheme')[0] == 0 else 'black' + icon_colour = 'black' if winreg.QueryValueEx(key, 'SystemUsesLightTheme')[0] == 0 else 'white' except FileNotFoundError: pass From a62a1d6c25b71d188472263e43beb8b8e4003558 Mon Sep 17 00:00:00 2001 From: Simon Robinson Date: Sun, 3 Sep 2023 20:52:30 +0100 Subject: [PATCH 19/21] Fix theme icon colour logic --- emailproxy.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/emailproxy.py b/emailproxy.py index acd9320..46b96ab 100644 --- a/emailproxy.py +++ b/emailproxy.py @@ -6,7 +6,7 @@ __author__ = 'Simon Robinson' __copyright__ = 'Copyright (c) 2023 Simon Robinson' __license__ = 'Apache 2.0' -__version__ = '2023-08-28' # ISO 8601 (YYYY-MM-DD) +__version__ = '2023-09-03' # ISO 8601 (YYYY-MM-DD) import abc import argparse @@ -2323,7 +2323,7 @@ def get_image(): try: key = winreg.OpenKey(winreg.HKEY_CURRENT_USER, r'Software\Microsoft\Windows\CurrentVersion\Themes\Personalize') - icon_colour = 'black' if winreg.QueryValueEx(key, 'SystemUsesLightTheme')[0] == 0 else 'white' + icon_colour = 'black' if winreg.QueryValueEx(key, 'SystemUsesLightTheme')[0] else 'white' except FileNotFoundError: pass From 8f4362cb8027855e4ea8be836ba723d64823c4f1 Mon Sep 17 00:00:00 2001 From: Simon Robinson Date: Fri, 25 Aug 2023 21:59:49 +0100 Subject: [PATCH 20/21] Fix locking when saving to cache store --- emailproxy.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/emailproxy.py b/emailproxy.py index bf3a0aa..a902378 100644 --- a/emailproxy.py +++ b/emailproxy.py @@ -453,6 +453,10 @@ def write(self, file): with self.lock: self.config.write(file) + def items(self): + with self.lock: + return self.config.items() # used in read_dict when saving to cache store + class AppConfig: """Helper wrapper around ConfigParser to cache servers/accounts, and avoid writing to the file until necessary""" @@ -538,9 +542,8 @@ def save(): if CACHE_STORE != CONFIG_FILE_PATH: # in `--cache-store` mode we ignore everything except _CACHED_OPTION_KEYS (OAuth 2.0 tokens, etc) output_config_parser = configparser.ConfigParser() - with AppConfig._PARSER.lock: - output_config_parser.read_dict(AppConfig._PARSER) # a deep copy of the current configuration - config_accounts = AppConfig.accounts() + output_config_parser.read_dict(AppConfig._PARSER) # a deep copy of the current configuration + config_accounts = [s for s in output_config_parser.sections() if '@' in s] for account in config_accounts: for option in output_config_parser.options(account): From 33e7eccd61829282e6575e033253c6bfe9a11212 Mon Sep 17 00:00:00 2001 From: Simon Robinson Date: Sat, 26 Aug 2023 22:29:12 +0100 Subject: [PATCH 21/21] Simplify config unload/reload; add locking --- emailproxy.py | 60 ++++++++++++++++++++++++--------------------------- 1 file changed, 28 insertions(+), 32 deletions(-) diff --git a/emailproxy.py b/emailproxy.py index a902378..d157b6f 100644 --- a/emailproxy.py +++ b/emailproxy.py @@ -6,7 +6,7 @@ __author__ = 'Simon Robinson' __copyright__ = 'Copyright (c) 2023 Simon Robinson' __license__ = 'Apache 2.0' -__version__ = '2023-08-16' # ISO 8601 (YYYY-MM-DD) +__version__ = '2023-08-26' # ISO 8601 (YYYY-MM-DD) import abc import argparse @@ -462,7 +462,7 @@ class AppConfig: """Helper wrapper around ConfigParser to cache servers/accounts, and avoid writing to the file until necessary""" _PARSER = None - _LOADED = False + _PARSER_LOCK = threading.Lock() # note: removing the unencrypted version of `client_secret_encrypted` is not automatic with --cache-store (see docs) _CACHED_OPTION_KEYS = ['token_salt', 'access_token', 'access_token_expiry', 'refresh_token', 'last_activity', @@ -473,27 +473,26 @@ class AppConfig: @staticmethod def _load(): - AppConfig.unload() - AppConfig._PARSER = ConcurrentConfigParser() - AppConfig._PARSER.read(CONFIG_FILE_PATH) + config_parser = ConcurrentConfigParser() + config_parser.read(CONFIG_FILE_PATH) # cached account credentials can be stored in the configuration file (default) or, via `--cache-store`, a # separate local file or external service (such as a secrets manager) - we combine these sources at load time if CACHE_STORE != CONFIG_FILE_PATH: # it would be cleaner to avoid specific options here, but best to load unexpected sections only when enabled - allow_catch_all_accounts = AppConfig._PARSER.getboolean(APP_SHORT_NAME, 'allow_catch_all_accounts', - fallback=False) + allow_catch_all_accounts = config_parser.getboolean(APP_SHORT_NAME, 'allow_catch_all_accounts', + fallback=False) cache_file_parser = AppConfig._load_cache(CACHE_STORE) cache_file_accounts = [s for s in cache_file_parser.sections() if '@' in s] for account in cache_file_accounts: - if allow_catch_all_accounts and account not in AppConfig._PARSER.sections(): # missing sub-accounts - AppConfig._PARSER.add_section(account) + if allow_catch_all_accounts and account not in config_parser.sections(): # missing sub-accounts + config_parser.add_section(account) for option in cache_file_parser.options(account): if option in AppConfig._CACHED_OPTION_KEYS: - AppConfig._PARSER.set(account, option, cache_file_parser.get(account, option)) + config_parser.set(account, option, cache_file_parser.get(account, option)) - AppConfig._LOADED = True + return config_parser @staticmethod def _load_cache(cache_store_identifier): @@ -507,38 +506,34 @@ def _load_cache(cache_store_identifier): @staticmethod def get(): - if not AppConfig._LOADED: - AppConfig._load() - return AppConfig._PARSER + with AppConfig._PARSER_LOCK: + if AppConfig._PARSER is None: + AppConfig._PARSER = AppConfig._load() + return AppConfig._PARSER @staticmethod def unload(): - AppConfig._PARSER = None - AppConfig._LOADED = False - - @staticmethod - def reload(): - AppConfig.unload() - return AppConfig.get() + with AppConfig._PARSER_LOCK: + AppConfig._PARSER = None @staticmethod def get_global(name, fallback): - AppConfig.get() # make sure config is loaded - return AppConfig._PARSER.getboolean(APP_SHORT_NAME, name, fallback) + return AppConfig.get().getboolean(APP_SHORT_NAME, name, fallback) @staticmethod def servers(): - AppConfig.get() # make sure config is loaded - return [s for s in AppConfig._PARSER.sections() if CONFIG_SERVER_MATCHER.match(s)] + return [s for s in AppConfig.get().sections() if CONFIG_SERVER_MATCHER.match(s)] @staticmethod def accounts(): - AppConfig.get() # make sure config is loaded - return [s for s in AppConfig._PARSER.sections() if '@' in s] + return [s for s in AppConfig.get().sections() if '@' in s] @staticmethod def save(): - if AppConfig._LOADED: + with AppConfig._PARSER_LOCK: + if AppConfig._PARSER is None: # intentionally using _PARSER not get() so we don't (re)load if unloaded + return + if CACHE_STORE != CONFIG_FILE_PATH: # in `--cache-store` mode we ignore everything except _CACHED_OPTION_KEYS (OAuth 2.0 tokens, etc) output_config_parser = configparser.ConfigParser() @@ -554,8 +549,7 @@ def save(): if section not in config_accounts or len(output_config_parser.options(section)) <= 0: output_config_parser.remove_section(section) - with AppConfig._PARSER.lock: - AppConfig._save_cache(CACHE_STORE, output_config_parser) + AppConfig._save_cache(CACHE_STORE, output_config_parser) else: # by default we cache to the local configuration file, and rewrite all values each time @@ -646,7 +640,7 @@ def get_account_with_catch_all_fallback(option): # try reloading remotely cached tokens if possible if not access_token and CACHE_STORE != CONFIG_FILE_PATH and recurse_retries: - AppConfig.reload() + AppConfig.unload() return OAuth2Helper.get_oauth2_credentials(username, password, recurse_retries=False) # we hash locally-stored tokens with the given password @@ -2782,7 +2776,9 @@ def load_and_start_servers(self, icon=None, reload=True): # we allow reloading, so must first stop any existing servers self.stop_servers() Log.info('Initialising', APP_NAME, '(version %s)' % __version__, 'from config file', CONFIG_FILE_PATH) - config = AppConfig.reload() if reload else AppConfig.get() + if reload: + AppConfig.unload() + config = AppConfig.get() # load server types and configurations server_load_error = False