From 8c874c2ff3d503ac20c7d32f46e08547fcb9e23f Mon Sep 17 00:00:00 2001 From: Simon Robinson Date: Tue, 6 Sep 2022 07:20:57 +0100 Subject: [PATCH] Improve handling of `oauth2_scope` and `redirect_uri` misconfiguration (#60) Support (but discourage) use without `offline` scopes; add further documentation for `redirect_uri` and O365 `offline_access`, and try to catch misconfigurations of this parameter. --- README.md | 2 +- emailproxy.config | 8 ++- emailproxy.py | 168 +++++++++++++++++++++++++++------------------- 3 files changed, 106 insertions(+), 72 deletions(-) diff --git a/README.md b/README.md index 0a3099f..e649e33 100644 --- a/README.md +++ b/README.md @@ -33,7 +33,7 @@ As part of the proxy setup process you need to provide an OAuth 2.0 `client_id` If you have an existing client ID and secret for a desktop app, you can use these directly in the proxy. If this is not possible, you can also reuse the client ID and secret from any email client that supports IMAP/POP/SMTP OAuth 2.0 authentication with the email server you would like to connect to (such as the [various](https://github.com/mozilla/releases-comm-central/blob/master/mailnews/base/src/OAuth2Providers.jsm) [open](https://github.com/Foundry376/Mailspring/blob/master/app/internal_packages/onboarding/lib/onboarding-constants.ts) [source](https://gitlab.gnome.org/GNOME/evolution-data-server/-/blob/master/CMakeLists.txt) [clients](https://gitlab.gnome.org/GNOME/gnome-online-accounts/-/blob/master/meson_options.txt) with OAuth 2.0 support), but please do this with care and restraint as access through reused tokens will be associated with the token owner rather than your own client. -If you do not have access to credentials for an existing client you will need to register your own. The process to do this is different for each provider, but the registration guides for several common ones are linked below. In all cases, when registering, make sure your client is set up to use an OAuth scope that will give it permission to access IMAP/POP/SMTP as desired (see the sample configuration file for examples). +If you do not have access to credentials for an existing client you will need to register your own. The process to do this is different for each provider, but the registration guides for several common ones are linked below. In all cases, when registering, make sure your client is set up to use an OAuth scope that will give it permission to access IMAP/POP/SMTP as desired. It is also highly recommended to use a scope that will grant "offline" access (i.e., a way to [refresh the OAuth 2.0 authentication token](https://oauth.net/2/refresh-tokens/) without user intervention). The sample configuration file provides example scope values for several common providers. - Office 365: register a new [Microsoft identity application](https://docs.microsoft.com/en-us/azure/active-directory/develop/quickstart-register-app) - Gmail / Google Workspace: register a [Google API desktop app client](https://developers.google.com/identity/protocols/oauth2/native-app) diff --git a/emailproxy.config b/emailproxy.config index 61a9665..9c946c9 100644 --- a/emailproxy.config +++ b/emailproxy.config @@ -82,6 +82,10 @@ documentation = Accounts are specified using your email address as the section h See the proxy's README.md file for more information and further configuration options. Office 365 customisation: + - Unlike other providers, Office 365 requires an OAuth 2.0 scope that explicitly specifies `offline_access` (shown + in the example below) in order to allow the proxy to refresh its access token on your behalf. The proxy will still + work if this parameter is not included, but you will need to re-authenticate extremely often (about once per hour). + - If your Office 365 configuration requires a tenant ID, place it in both `permission_url` and `token_url` in place of `common` in the example below. For more detail about this, and guides for setting up your desktop app API client, see the documentation at https://docs.microsoft.com/en-us/azure/active-directory/develop/quickstart-register-app. @@ -93,7 +97,9 @@ documentation = Accounts are specified using your email address as the section h setup, delete the `client_secret` line from your account's configuration entry (do not leave the default value). Advanced account configuration: - - For most configurations the default `redirect_uri` value of `http://localhost` is correct, but when using the + - For most configurations the default `redirect_uri` value of `http://localhost` is correct, unless you have + explicitly set your OAuth 2.0 client to use a different address here (which you will need to manually redirect to + the proxy). Please also note that this address is `http` and not `https` (which is not supported). When using the `--local-server-auth` proxy option you will need to either run the script via sudo (to use the implicit default port 80) or specify a different port (and/or a different host) - for example, `redirect_uri = http://localhost:8080`. In addition, if you are using this proxy option in a configuration that is not directly exposed (such as a container or diff --git a/emailproxy.py b/emailproxy.py index 71c8744..3d640dc 100644 --- a/emailproxy.py +++ b/emailproxy.py @@ -4,7 +4,7 @@ __author__ = 'Simon Robinson' __copyright__ = 'Copyright (c) 2022 Simon Robinson' __license__ = 'Apache 2.0' -__version__ = '2022-08-30' # ISO 8601 (YYYY-MM-DD) +__version__ = '2022-09-05' # ISO 8601 (YYYY-MM-DD) import argparse import base64 @@ -129,7 +129,7 @@ class NSObject: IMAP_CAPABILITY_MATCHER = re.compile(r'^(\* |\* OK \[)CAPABILITY .*$', flags=re.IGNORECASE) # note: '* ' and '* OK [' REQUEST_QUEUE = queue.Queue() # requests for authentication -RESPONSE_QUEUE = queue.Queue() # responses from client web view +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 @@ -310,7 +310,7 @@ def save(): class OAuth2Helper: @staticmethod - def get_oauth2_credentials(username, password, connection_info, recurse_retries=True): + def get_oauth2_credentials(username, password, recurse_retries=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]')""" @@ -367,13 +367,35 @@ def get_oauth2_credentials(username, password, connection_info, recurse_retries= cryptographer = Fernet(key) try: - if not refresh_token: + if access_token: + if access_token_expiry - current_time < TOKEN_EXPIRY_MARGIN: + if refresh_token: + # if expiring soon, refresh token (if possible) + response = OAuth2Helper.refresh_oauth2_access_token(token_url, client_id, client_secret, + OAuth2Helper.decrypt(cryptographer, + refresh_token)) + + access_token = response['access_token'] + config.set(username, 'access_token', OAuth2Helper.encrypt(cryptographer, access_token)) + config.set(username, 'access_token_expiry', str(current_time + response['expires_in'])) + if 'refresh_token' in response: + config.set(username, 'refresh_token', + OAuth2Helper.encrypt(cryptographer, response['refresh_token'])) + AppConfig.save() + + elif access_token_expiry <= current_time: + # cannot get another access token without a refresh token - must submit another manual request + access_token = None + else: + access_token = OAuth2Helper.decrypt(cryptographer, access_token) + + if not access_token: permission_url = OAuth2Helper.construct_oauth2_permission_url(permission_url, redirect_uri, client_id, oauth2_scope, username) # note: get_oauth2_authorisation_code is a blocking call success, authorisation_code = OAuth2Helper.get_oauth2_authorisation_code(permission_url, redirect_uri, redirect_listen_address, - username, connection_info) + username) if not success: Log.info('Authentication request failed or expired for account', username, '- aborting login') return False, '%s: Login failed - the authentication request expired or was cancelled for ' \ @@ -386,24 +408,13 @@ def get_oauth2_credentials(username, password, connection_info, recurse_retries= config.set(username, 'token_salt', token_salt) config.set(username, 'access_token', OAuth2Helper.encrypt(cryptographer, access_token)) config.set(username, 'access_token_expiry', str(current_time + response['expires_in'])) - config.set(username, 'refresh_token', OAuth2Helper.encrypt(cryptographer, response['refresh_token'])) - AppConfig.save() - - else: - if access_token_expiry - current_time < TOKEN_EXPIRY_MARGIN: # if expiring soon, refresh token - response = OAuth2Helper.refresh_oauth2_access_token(token_url, client_id, client_secret, - OAuth2Helper.decrypt(cryptographer, - refresh_token)) - - access_token = response['access_token'] - config.set(username, 'access_token', OAuth2Helper.encrypt(cryptographer, access_token)) - config.set(username, 'access_token_expiry', str(current_time + response['expires_in'])) - if 'refresh_token' in response: - config.set(username, 'refresh_token', - OAuth2Helper.encrypt(cryptographer, response['refresh_token'])) - AppConfig.save() + if 'refresh_token' in response: + config.set(username, 'refresh_token', + OAuth2Helper.encrypt(cryptographer, response['refresh_token'])) else: - access_token = OAuth2Helper.decrypt(cryptographer, access_token) + 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?)') + AppConfig.save() # send authentication command to server (response checked in ServerConnection) - note: we only support # single-trip authentication (SASL) without actually checking the server's capabilities - improve? @@ -425,15 +436,17 @@ def get_oauth2_credentials(username, password, connection_info, recurse_retries= if recurse_retries: Log.info('Retrying login due to exception while requesting OAuth 2.0 credentials:', Log.error_string(e)) - return OAuth2Helper.get_oauth2_credentials(username, password, connection_info, recurse_retries=False) + return OAuth2Helper.get_oauth2_credentials(username, password, recurse_retries=False) else: 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) except Exception as e: - # note that we don't currently remove cached credentials here, as failures on the initial request are - # before caching happens, and the assumption is that refresh token request exceptions are temporal (e.g., - # network errors: URLError(OSError(50, 'Network is down'))) rather than e.g., bad requests + # note that we don't currently remove cached credentials here, as failures on the initial request are before + # caching happens, and the assumption is that refresh token request exceptions are temporal (e.g., network + # errors: URLError(OSError(50, 'Network is down'))) - access token 400 Bad Request HTTPErrors with messages + # such as 'authorisation code was already redeemed' are caused by our support for simultaneous requests, + # and will work from the next request; however, please report an issue if you encounter problems here Log.info('Caught exception while requesting OAuth 2.0 credentials:', Log.error_string(e)) return False, '%s: Login failed for account %s - please check your internet connection and retry' % ( APP_NAME, username) @@ -457,9 +470,8 @@ def oauth2_url_unescape(text): @staticmethod def start_redirection_receiver_server(token_request): """Starts a local WSGI web server at token_request['redirect_uri'] to receive OAuth responses""" - wsgi_address = token_request['redirect_listen_address'] if token_request['redirect_listen_address'] else \ - token_request['redirect_uri'] - parsed_uri = urllib.parse.urlparse(wsgi_address) + 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)) @@ -480,28 +492,38 @@ def __call__(self, environ, start_response): try: wsgiref.simple_server.WSGIServer.allow_reuse_address = False + wsgiref.simple_server.WSGIServer.timeout = AUTHENTICATION_TIMEOUT redirection_server = wsgiref.simple_server.make_server(str(parsed_uri.hostname), parsed_port, RedirectionReceiverWSGIApplication(), handler_class=LoggingWSGIRequestHandler) - token_request['local_server_auth_wsgi'] = redirection_server + Log.info('Please visit the following URL to authenticate account %s: %s' % (token_request['username'], token_request['permission_url'])) redirection_server.handle_request() - redirection_server.server_close() + try: + redirection_server.server_close() + except socket.error: + pass - Log.debug('Local server auth mode (%s:%d): closing local server and returning response' % ( - parsed_uri.hostname, parsed_port), token_request['response_url']) - del token_request['local_server_auth'] - del token_request['local_server_auth_wsgi'] - RESPONSE_QUEUE.put(token_request) + 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']) + 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') + token_request['expired'] = True except socket.error as e: - Log.error('Local server auth mode (%s:%d): unable to start local server. Please check that the %s for ' - 'account %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 for further detail' % ( - parsed_uri.hostname, parsed_port, - 'redirect_listen_address' if token_request['redirect_listen_address'] else 'redirect_uri', - token_request['username']), Log.error_string(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)) + token_request['expired'] = True + + del token_request['local_server_auth'] + RESPONSE_QUEUE.put(token_request) @staticmethod def construct_oauth2_permission_url(permission_url, redirect_uri, client_id, scope, username): @@ -516,11 +538,10 @@ def construct_oauth2_permission_url(permission_url, redirect_uri, client_id, sco return '%s?%s' % (permission_url, '&'.join(param_pairs)) @staticmethod - def get_oauth2_authorisation_code(permission_url, redirect_uri, redirect_listen_address, username, connection_info): + def get_oauth2_authorisation_code(permission_url, redirect_uri, redirect_listen_address, username): """Submit an authorisation request to the parent app and block until it is provided (or the request fails)""" - token_request = {'connection': connection_info, 'permission_url': permission_url, - 'redirect_uri': redirect_uri, 'redirect_listen_address': redirect_listen_address, - 'username': username, 'expired': False} + token_request = {'permission_url': permission_url, 'redirect_uri': redirect_uri, + 'redirect_listen_address': redirect_listen_address, 'username': username, 'expired': False} REQUEST_QUEUE.put(token_request) wait_time = 0 while True: @@ -532,8 +553,6 @@ def get_oauth2_authorisation_code(permission_url, redirect_uri, redirect_listen_ continue else: token_request['expired'] = True - if 'local_server_auth_wsgi' in token_request: - token_request['local_server_auth_wsgi'].server_close() REQUEST_QUEUE.put(token_request) # re-insert the request as expired so the parent app can remove it return False, None @@ -541,15 +560,19 @@ def get_oauth2_authorisation_code(permission_url, redirect_uri, redirect_listen_ RESPONSE_QUEUE.put(QUEUE_SENTINEL) # make sure all watchers exit return False, None - elif data['connection'] == connection_info: # found an authentication response meant for us + elif data['permission_url'] == permission_url and data['username'] == username: # a response meant for us # to improve no-GUI mode we also support the use of a local server to receive the OAuth redirection # (note: not enabled by default because no-GUI mode is typically unattended, but useful in some cases) - if 'local_server_auth' in data: + if 'expired' in data and data['expired']: # local server auth wsgi request error or failure + return False, None + + elif 'local_server_auth' in data: threading.Thread(target=OAuth2Helper.start_redirection_receiver_server, args=(data,), name='EmailOAuth2Proxy-auth-%s' % data['username'], daemon=True).start() else: - if 'response_url' in data and 'code=' in data['response_url']: + if 'response_url' in data and 'code=' in data['response_url'] and data['response_url'].startswith( + token_request['redirect_uri']): authorisation_code = OAuth2Helper.oauth2_url_unescape( data['response_url'].split('code=')[1].split('&')[0]) if authorisation_code: @@ -835,9 +858,8 @@ def handle_read(self): def process_data(self, byte_data, censor_server_log=False): try: self.server_connection.send(byte_data, censor_log=censor_server_log) # default = send everything to server - except AttributeError as e: - Log.info(self.info_string(), 'Caught client exception; server connection closed before data could be sent:', - Log.error_string(e)) + except AttributeError: # AttributeError("'NoneType' object has no attribute 'send'") + Log.info(self.info_string(), 'Caught client exception; server connection closed before data could be sent') self.close() def send(self, byte_data): @@ -928,14 +950,17 @@ def process_data(self, byte_data, censor_server_log=False): super().process_data(byte_data) def authenticate_connection(self, username, password, command='login'): - success, result = OAuth2Helper.get_oauth2_credentials(username, password, self.connection_info) + success, result = OAuth2Helper.get_oauth2_credentials(username, password) if success: # send authentication command to server (response checked in ServerConnection) # note: we only support single-trip authentication (SASL) without checking server capabilities - improve? super().process_data(b'%s AUTHENTICATE XOAUTH2 ' % self.authentication_tag.encode('utf-8')) super().process_data(OAuth2Helper.encode_oauth2_string(result), censor_server_log=True) super().process_data(b'\r\n') - self.server_connection.authenticated_username = username + + # because get_oauth2_credentials blocks, the server could have disconnected, and may no-longer exist + if self.server_connection: + self.server_connection.authenticated_username = username else: error_message = '%s NO %s %s\r\n' % (self.authentication_tag, command.upper(), result) @@ -1190,9 +1215,8 @@ def handle_read(self): def process_data(self, byte_data): try: self.client_connection.send(byte_data) # by default we just send everything straight to the client - except AttributeError as e: - Log.info(self.info_string(), 'Caught server exception; client connection closed before data could be sent:', - Log.error_string(e)) + except AttributeError: # AttributeError("'NoneType' object has no attribute 'send'") + Log.info(self.info_string(), 'Caught server exception; client connection closed before data could be sent') self.close() def send(self, byte_data, censor_log=False): @@ -1322,8 +1346,7 @@ def process_data(self, byte_data): elif self.client_connection.connection_state is POPOAuth2ClientConnection.STATE.XOAUTH2_AWAITING_CONFIRMATION: if str_data.startswith('+') and self.username and self.password: # '+ ' = 'please send credentials' - success, result = OAuth2Helper.get_oauth2_credentials(self.username, self.password, - self.connection_info) + success, result = OAuth2Helper.get_oauth2_credentials(self.username, self.password) if success: self.client_connection.connection_state = POPOAuth2ClientConnection.STATE.XOAUTH2_CREDENTIALS_SENT self.send(b'%s\r\n' % OAuth2Helper.encode_oauth2_string(result), censor_log=True) @@ -1417,8 +1440,7 @@ def process_data(self, byte_data): # ...then, once we have the username and password we can respond to the '334 ' response with credentials elif self.client_connection.connection_state is SMTPOAuth2ClientConnection.STATE.XOAUTH2_AWAITING_CONFIRMATION: if str_data.startswith('334') and self.username and self.password: # '334 ' = 'please send credentials' - success, result = OAuth2Helper.get_oauth2_credentials(self.username, self.password, - self.connection_info) + success, result = OAuth2Helper.get_oauth2_credentials(self.username, self.password) if success: self.client_connection.connection_state = SMTPOAuth2ClientConnection.STATE.XOAUTH2_CREDENTIALS_SENT self.authenticated_username = self.username @@ -1999,16 +2021,17 @@ def authorisation_window_loaded(self): continue # skip dummy window url = window.get_current_url() - account_name = window.get_title(window).split(' ')[-1] # see note above: title *must* match this format - if not url or not account_name: + 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 # respond to both the original request and any duplicates in the list completed_request = None for request in self.authorisation_requests[:]: # iterate over a copy; remove from original - if url.startswith(request['redirect_uri']) and account_name == request['username']: + if url.startswith(request['redirect_uri']) and username == request['username']: Log.info('Successfully authorised request for', request['username']) - RESPONSE_QUEUE.put({'connection': request['connection'], 'response_url': url}) + RESPONSE_QUEUE.put( + {'permission_url': request['permission_url'], 'response_url': url, 'username': username}) self.authorisation_requests.remove(request) completed_request = request else: @@ -2208,6 +2231,11 @@ def stop_servers(self): global RESPONSE_QUEUE RESPONSE_QUEUE.put(QUEUE_SENTINEL) RESPONSE_QUEUE = queue.Queue() # recreate so existing queue closes watchers but we don't have to wait here + while True: + try: + REQUEST_QUEUE.get(block=False) # remove any pending requests (unlikely any exist, but safest) + except queue.Empty: + break for proxy in self.proxies: # noinspection PyBroadException try: @@ -2314,9 +2342,9 @@ def post_create(self, icon): self.notify(APP_NAME, 'Please authorise your account %s from the menu' % data['username']) else: for request in self.authorisation_requests[:]: # iterate over a copy; remove from original - if request['connection'] == data['connection']: + if request['permission_url'] == data['permission_url']: self.authorisation_requests.remove(request) - break + break # we could have multiple simultaneous requests, some not yet expired @staticmethod def run_proxy(): @@ -2328,7 +2356,7 @@ def run_proxy(): # exit on server start failure), otherwise this will throw an error every time and loop indefinitely asyncore.loop() except Exception as e: - if not EXITING: + if not EXITING and not (isinstance(e, OSError) and e.errno == errno.EBADF): Log.info('Caught asyncore exception in main loop; attempting to continue:', Log.error_string(e)) error_count += 1 time.sleep(error_count)