simonrob / email-oauth2-proxy

An IMAP/POP/SMTP proxy that transparently adds OAuth 2.0 authentication for email clients that don't support this method.
Apache License 2.0
843 stars 94 forks source link

Client Credentials Flow does not work with encrypt_client_secret_on_first_use #213

Closed ft3411 closed 10 months ago

ft3411 commented 10 months ago

Thank you for adding support for the Client Credentials grant flow which makes our life much easier.

I tried to protect the password that is used between the proxy and the client. If a user enters a wrong password the access token is dropped and a new one is retrieved. When I activated the option encrypt_client_secret_on_first_use some nasty things happen:

At first it is possible to authenticate and use the proxy as before. When a wrong password is used the access token is deleted and the proxy starts to loop. Restarting the application helps but the encrypted client secret cannot be used and so the account configuration becomes unusable.

The log file looks like this:

2023-12-14 14:26:23,046: New incoming connection to POP server at 127.0.0.1:1995 (unsecured) proxying outlook.office365.com:995 (SSL/TLS)
2023-12-14 14:26:23,047: Ignoring incoming connection to POP server at 127.0.0.1:1995 (unsecured) proxying outlook.office365.com:995 (SSL/TLS) - no connection information
2023-12-14 14:26:23,101: POP (127.0.0.1:50250-{127.0.0.1:1995}-outlook.office365.com:995) --> [ Client connected ]
2023-12-14 14:26:23,138: POP (127.0.0.1:50250-{127.0.0.1:1995}-outlook.office365.com:995) <-> [ Starting TLS handshake ]
2023-12-14 14:26:23,156: POP (127.0.0.1:50250-{127.0.0.1:1995}-outlook.office365.com:995) <-> [ TLSv1.2 handshake complete ]
2023-12-14 14:26:23,202: POP (127.0.0.1:50250-{127.0.0.1:1995}-outlook.office365.com:995)     <-- b'+OK The Microsoft Exchange POP3 service is ready. [...]\r\n'
2023-12-14 14:26:23,202: POP (127.0.0.1:50250-{127.0.0.1:1995}-outlook.office365.com:995) <-- b'+OK The Microsoft Exchange POP3 service is ready. [...]\r\n'
2023-12-14 14:26:31,824: POP (127.0.0.1:50250-{127.0.0.1:1995}-outlook.office365.com:995) --> b'user mail.address@company.com\r\n'
2023-12-14 14:26:31,824: POP (127.0.0.1:50250-{127.0.0.1:1995}-outlook.office365.com:995) <-- b'+OK\r\n'
2023-12-14 14:26:36,215: POP (127.0.0.1:50250-{127.0.0.1:1995}-outlook.office365.com:995) --> b'pass [[ Credentials removed from proxy log ]]\r\n'
2023-12-14 14:26:36,215: POP (127.0.0.1:50250-{127.0.0.1:1995}-outlook.office365.com:995)     --> b'AUTH XOAUTH2\r\n'
2023-12-14 14:26:36,230: POP (127.0.0.1:50250-{127.0.0.1:1995}-outlook.office365.com:995)     <-- b'+ \r\n'
2023-12-14 14:26:37,542: Retrying login due to exception while decrypting OAuth 2.0 credentials for mail.address@company.com (invalid password): InvalidToken()
2023-12-14 14:26:38,593: Retrying login due to exception while decrypting OAuth 2.0 credentials for mail.address@company.com (invalid password): InvalidToken()
2023-12-14 14:26:40,073: Retrying login due to exception while decrypting OAuth 2.0 credentials for mail.address@company.com (invalid password): InvalidToken()
2023-12-14 14:26:41,137: Retrying login due to exception while decrypting OAuth 2.0 credentials for mail.address@company.com (invalid password): InvalidToken()

This continues forever - even after closing the connection.

Do you have any idea how I could get a local password which is checked by the proxy? (and still use the client credentials flow)

Thank you tom

simonrob commented 10 months ago

Thanks for reporting this. #214 discusses a potentially related issue (though perhaps without knowledge of the encrypt_client_secret_on_first_use option).

I suspect this is a regression around the changes in #183, which separated token renewal from other potential exceptions that could be raised in that part of the code. I'll take a look when I get chance, but if want to look yourself, 9c42b6b might be a good place to start.

gerneio commented 10 months ago

I resolved the looping error by adding another try-block around the decrypting of the client_secret_encrypted stored variable, within the get_oauth2_credentials function (here). I tried to put it down in the current try-block InvalidToken catch, but it would keep causing decryption errors even after switching back to the correct password. Either way, this might be a more accurate fix anyways since the more general InvalidToken catch block doesn't have any knowledge of which variable failed to decrypt, so this fix is more specific.

            # if both secret values are present we use the unencrypted version (as it may have been user-edited)
            if client_secret_encrypted and not client_secret:
                try:
                    client_secret = cryptographer.decrypt(client_secret_encrypted)
                except InvalidToken as e:
                    Log.error('Invalid password to decrypt', username, 'client_secret - aborting login:', Log.error_string(e))
                    # When decryption fails for `client_secret_encrypted`, immediately return and don't retry logging in
                    return False, '%s: Login failed - the password for account %s is incorrect' % (APP_NAME, username)

I'm not overly familiar with python, nor the exact configuration of your script, so didn't want to immediately submit a PR in case this was bad form.

simonrob commented 10 months ago

Thanks for following up here, and for suggesting a fix for this issue.

I had a look just now, and one more thing to point out is that this only happens if delete_account_token_on_password_error = True is also set (which is the default). Your fix is pretty much exactly what I would have done, though, and I've pushed that to the issue-213 branch, which I'll merge shortly unless I hear otherwise.

Thanks for the help!