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
785 stars 84 forks source link

Don't close IMAP connection if password is wrong #228

Closed bemoody closed 6 months ago

bemoody commented 6 months ago

For security, I want my email proxy to require a password. When I mistype the password, I want it to ask me to try again (I don't want it to go off and try to obtain a new OAuth2 token.) Hence, I've set the delete_account_token_on_password_error = False option.

However, when I enter an incorrect password, what currently happens is:

claws-mail seems to interpret the broken connection as a permanent error, and therefore doesn't ask me to try again; in fact, if the "remember password for this session" box is checked, it will remember the wrong password until I close and restart the program, which is annoying.

(The relevant bit of code in claws-mail appears to be in src/imap.c, imap_session_authenticate(): if imap_auth() returns a "fatal" error, it won't try again.)

I know very little about IMAP, so I don't really know whether claws-mail or email-oauth2-proxy is at fault here. But it seems to me that this behavior (closing the connection as soon as the client gives a wrong password) is unusual behavior for an IMAP server, and possibly violates the standards.

All of which is to say that, if I delete these two lines in emailproxy.py:

            self.send(b'* BYE Autologout; authentication failed\r\n')
            self.close()

then everything appears to work the way I want it to. But I don't know if I'm missing something important.

simonrob commented 6 months ago

Thanks for this. I haven't yet revisited the specifications in detail to check, but you may well be correct.

That particular behaviour of the proxy was added in one of the earliest commits. To be honest, given the use of the term Autologout, I suspect I saw this response in an existing IMAP server and just copied it without referencing the RFC.

I'm happy to change this (and also the POP/SMTP responses if appropriate), but would need to be careful that the initial login state of both sides of the connection is restored when login fails. If it wasn't a clone of an existing server's response, ending the connection may well have been a shortcut for resetting the server state.

simonrob commented 6 months ago

You are correct that the specification does permit login retries, so the local-auth-retry branch changes this behaviour for IMAP, POP and SMTP local login. Please could you check that this works as expected in your configuration before it is merged?

Please note that this does not change the proxy's behaviour if the remote login fails – in that situation you're going to need to edit the proxy's configuration file and restart the server anyway, so it still just passes through the remote error and closes the connection.

bemoody commented 6 months ago

Yes, the updated branch works for me.

Please note that this does not change the proxy's behaviour if the remote login fails

That makes sense. Thanks!