IMAP reconnection prevent plugins from handling password

[jsh]

In method rcube_imap::set_flag() a reconnection to the IMAP server is done :

      // close and re-open connection
      // this prevents connection problems with Courier 
      $this->reconnect();

This reconnection is done with password stored in session but this prevents plugins from handling one time password usage.

Long explanation:
I'm in the process of writing a plugin for CAS SSO authentication. The plugin is working great except when the method set_flag is called. With CAS authentication, a CAS ticket is used as IMAP password. It can only be used once so the plugin request one and store it in $_SESSION[ 'password' ] at every page call. The problem is that the rcube_imap::reconnect() method doesn't allow the plugin to request a new CAS ticket so the IMAP authentication failed.

What solution to this would be best ?
- Add a plugin hook 'before_imap_connect' that would allow plugin to manipulate connection data just before the connection ?
- Drop the reconnect() method (probably not the right option...)

[jsh]

I'm attaching a patch that adds a 'before_imap_login' hook. This hook could perhaps be the new 'authenticate' hook as it serves the same purpose but is nearer to the imap authentication than the current one.

[jsh]

Implementation of before_imap_login hook in rcube_imap.php.

[jsh]

In fact the 'authenticate' hook should not be simply moved as the code tests if the password is not empty before calling the login process. In the case of CAS authentication the password _is_ empty (not in POST data) so the 'authenticate' hook permits to set the password to something not empty. Then the 'before_imap_login' will fill the password with the real one obtained from the CAS server. So if we want to merge the two hooks, there will be some more modifications to do...

[alec]

We've got 'smtp_connect' hook, so we could provide 'imap_connect' hook as well.

[thomasb]

Implemented as hook 'imap_connect' in r2849. Passwords are not passed to plugins but can be set by them.

[alexli]

I've already written a CAS authentication plugin for my organization's RoundCube instance which I plan to submit to the plugin repository. Since I needed to get the plugin working before this feature was added, I actually added in the imap_connect plugin hook myself, except I added it to the imap_connect() function in rcmail.php instead. I'm glad to see this change made, since I can now submit my plugin, but unfortunately this changeset doesn't fully do what it needs to do for CAS authentication to fully work. When using RoundCube as a CAS proxy, it is more or less necessary to have the IMAP server cache the proxy tickets as authentication credentials, otherwise each request to the IMAP server will result in a round-trip validation process to the CAS server and will cause a lot of lag each time. So in my plugin the imap_connect() hook actually tries to connect with the connect() method in rcube_imap.php using the existing proxy ticket in session. If this connection fails it means the cache has expired, and only then does it retrieve a new proxy ticket. It also returns an additional parameter to the calling function that specifies whether the plugin successfully connected to the IMAP server, so the calling function does not connect a second time if the first connection in the plugin succeeded. Is it possible to add the plugin hook to rcube_imap.php in such a way to enable this cache/cache-expiration scenario? I could also attach the modifications I made to rcmail.php for consideration.

[alexli]

The way I added the imap_connect hook to rcmail.php that provides for the caching scenario.

[alexli]

I've uploaded my CAS authentication plugin here. Right now it makes use of the imap_connect hook the way I wrote it a while ago. I could change it to work with the new imap_connect hook but I hope there's a way to allow the plugin to attempt to connect to the IMAP server first, and then returning to the calling function whether the connection was successful and a new proxy ticket if the connection attempt failed, so as to enable the proxy ticket caching scenario.

[robk]

Would it be possible to request the connected boolean (like alexli has suggested) for the imap_connect hook?

The prefered performance IMAP connection for CAS is to use a cached connection reusing the issued ticket(password). When the cache expires and the server rejects the ticket, we need to acquire a new ticket. Currently Roundcube will just fail gracefully and the user has to f5/reload the browser session.

As suggested in alexli's attached diff, it is easier to catch the expired caches.

[alexli]

I realize that my implementation of the imap_connect hook in rcmail.php above has the plugin making connections to the IMAP backend with authentication credentials stored in session, which may not be desirable. So I made an alternative implementation, this time in rcube_imap.php, that's very similar to the one currently in trunk, but provides for the caching scenario by means of a do...while loop.

In this implementation the connect() method keeps track of the number of attempts made to connect to the IMAP backend and passes this as an argument to the plugin. Then the plugin can use that to determine whether to retrieve a new proxy ticket. In the caching scenario a new proxy ticket will only be retrieved on the second attempt. Then the plugin returns an additional argument "retry" which tells the connect() method whether to keep trying to connect to the IMAP backend. By default this is set to false but in the caching scenario on the first attempt, this is set to true by the plugin in case the cache has expired and another attempt need to be made with a new proxy ticket. For reference, I rewrote my plugin to make use of this new implementation of the hook here.

[alexli]

My new implementation of the imap_connect hook.

[thomasb]

Applied changes in r2861