Hello Folks,
I have a question regarding the identities_list hook. I would like to replace the full list of identities by my own using a plugin. It seems to me the identities_list hook is only used for display, as the original identities from the database are added. Is this just an oversight, or is it supposed to be that way?
I would like to propose to make use of the identities_list hook more often. The ideal solution would be to query the plugin hook in rcube_user.php's list_identities, returning the plugin defined list of identities everywhere. The only downside is that the current caller in program/steps/settings/func.inc will have to query the plugin twice, once directly to get the array of columns and once indirectly via rcube_user.php.
I'm happy to provide the patch myself, I would just like to ask here before I send a PR.
Alternatively, is there another way to intercept the identities and user management? We would like to use our own user management backend.
Philipp
On 06/04/2013 05:47 PM, Philipp Kewisch wrote:
Alternatively, is there another way to intercept the identities and user management? We would like to use our own user management backend.
Maybe for your use-case would be better to replace the whole $RCMAIL->user object with some new class where you could overwrite methods used for identities management.
On 6/6/13 3:19 PM, A.L.E.C wrote:
On 06/04/2013 05:47 PM, Philipp Kewisch wrote:
Alternatively, is there another way to intercept the identities and user management? We would like to use our own user management backend.
Maybe for your use-case would be better to replace the whole $RCMAIL->user object with some new class where you could overwrite methods used for identities management.
Indeed, replacing the whole rcube_user class with my own implementation is ideal, I haven't gone this route because there is some code that assumes rcube_user is the only class to handle users and this sounds more invasive to me.
I initially tried to just call set_user() from the init function of the plugin, but this is already too late. Also I found out there are a few static functions that are called on rcube_user which need to be called on my class.
To fix this here is my new proposal. The functions are not tested, but give you an idea where I'm going:
Add a method to class rcube that allows replacing classes, i.e: function factory($rcubeClass, $args) { $cls = array( "className" => $rcubeClass); $cls = $this->plugins->exec_hook("factory", $cls); return call_user_func_array($cls["className"], $args); }
Add a method to class rcube that calls static methods of factory classes: function staticfactory($rcubeClass, $method, $args) { $cls = array( "className" => $rcubeClass); $cls = $this->plugins->exec_hook("factory", $cls); return call_user_func_array(array($cls["className"], $method),
$args); }
- Change everything that creates rcube_user or calls its static methods
to use these functions
How does this sound for you? I'd like to bring roundcube to a state where we can do everything from a plugin.
Thanks, Philipp
On 06/07/2013 02:01 PM, Philipp Kewisch wrote:
Indeed, replacing the whole rcube_user class with my own implementation is ideal, I haven't gone this route because there is some code that assumes rcube_user is the only class to handle users and this sounds more invasive to me.
Just make your class to inherit from rcube_user, no?
I initially tried to just call set_user() from the init function of the plugin, but this is already too late. Also I found out there are a few static functions that are called on rcube_user which need to be called on my class.
It's query() and create(). Query() shoudn't be a problem. In create(), after user record creation, identities are created, but you can use identity_create hook with abort=true to handle identity creation and skip default action.
A good place to replace user object is the 'ready' hook.
On 6/7/13 2:26 PM, A.L.E.C wrote:
On 06/07/2013 02:01 PM, Philipp Kewisch wrote:
Indeed, replacing the whole rcube_user class with my own implementation is ideal, I haven't gone this route because there is some code that assumes rcube_user is the only class to handle users and this sounds more invasive to me.
Just make your class to inherit from rcube_user, no?
Sorry I was unclear, this is what I am doing, yes.
I initially tried to just call set_user() from the init function of the plugin, but this is already too late. Also I found out there are a few static functions that are called on rcube_user which need to be called on my class.
It's query() and create(). Query() shoudn't be a problem. In create(), after user record creation, identities are created, but you can use identity_create hook with abort=true to handle identity creation and skip default action.
A good place to replace user object is the 'ready' hook.
Actually, query seems to be a problem. Here the user is initially set: https://github.com/roundcube/roundcubemail/blob/master/program/include/rcmai...
If the user doesn't exist in the roundcube database, $user will be empty. I've disabled auto_user_create since I don't want double entries in the database, so I end up with the error message that the user will not be created.
From the looks of it I was able to work around all other issues.
What do you think?
Philipp
On 06/08/2013 09:55 AM, Philipp Kewisch wrote:
Actually, query seems to be a problem. Here the user is initially set: https://github.com/roundcube/roundcubemail/blob/master/program/include/rcmai...
If the user doesn't exist in the roundcube database, $user will be empty. I've disabled auto_user_create since I don't want double entries in the database, so I end up with the error message that the user will not be created.
I don't see a problem here. $user will be empty but after successful login rcube_user::create() is used. Do you want to override identities or maybe users database too? You initially talked about identities and I don't see a problem for identities.
On 6/8/13 10:11 AM, A.L.E.C wrote:
On 06/08/2013 09:55 AM, Philipp Kewisch wrote:
Actually, query seems to be a problem. Here the user is initially set: https://github.com/roundcube/roundcubemail/blob/master/program/include/rcmai...
If the user doesn't exist in the roundcube database, $user will be empty. I've disabled auto_user_create since I don't want double entries in the database, so I end up with the error message that the user will not be created.
I don't see a problem here. $user will be empty but after successful login rcube_user::create() is used. Do you want to override identities or maybe users database too? You initially talked about identities and I don't see a problem for identities.
Sorry, I'm being very confusing and I feel bad for it. Let me start at the beginning.
Initially I was looking for a way to use the identities from our system within roundcube. We also have an addressbook and authentication plugin that use the provided hooks. Roundcube users were being created in the database, auto_create_user is on. My initial thought was the same as you later on suggested, replace rcube_user. I then noticed the seemingly easy way of using identities_list, but afterwards found out its only for the columns and display.
I wrote my first email, where you replied that extending rcube_user would make sense. I went back to my original thought of replacing rcube_user and had success, the identities are now shown from our system. As I now have my own rcube_user class to work with, it makes sense to consolidate the authentication plugin and avoid essentially duplicating the users table from our system. For this to work I would need to replace the rcube_user::query method.
I think it would be a win for roundcube to allow plugins to easily replace the rcube_user class, so if you agree I'd like to discuss how to best go about it.
I hope this explains a bit better, I promise to be more concise in future emails :)
Philipp
On 6/10/13 9:48 PM, Philipp Kewisch wrote:
On 6/8/13 10:11 AM, A.L.E.C wrote:
On 06/08/2013 09:55 AM, Philipp Kewisch wrote:
Actually, query seems to be a problem. Here the user is initially set: https://github.com/roundcube/roundcubemail/blob/master/program/include/rcmai...
If the user doesn't exist in the roundcube database, $user will be empty. I've disabled auto_user_create since I don't want double entries in the database, so I end up with the error message that the user will not be created.
I don't see a problem here. $user will be empty but after successful login rcube_user::create() is used. Do you want to override identities or maybe users database too? You initially talked about identities and I don't see a problem for identities.
Sorry, I'm being very confusing and I feel bad for it. Let me start at the beginning.
Initially I was looking for a way to use the identities from our system within roundcube. We also have an addressbook and authentication plugin that use the provided hooks. Roundcube users were being created in the database, auto_create_user is on. My initial thought was the same as you later on suggested, replace rcube_user. I then noticed the seemingly easy way of using identities_list, but afterwards found out its only for the columns and display.
I wrote my first email, where you replied that extending rcube_user would make sense. I went back to my original thought of replacing rcube_user and had success, the identities are now shown from our system. As I now have my own rcube_user class to work with, it makes sense to consolidate the authentication plugin and avoid essentially duplicating the users table from our system. For this to work I would need to replace the rcube_user::query method.
I think it would be a win for roundcube to allow plugins to easily replace the rcube_user class, so if you agree I'd like to discuss how to best go about it.
I hope this explains a bit better, I promise to be more concise in future emails :)
Philipp
Any updates? I've done some more testing and found out that with the current way I am attempting to replace the user object (i.e doing so in the init() method) doesn't fully work. Login works without problems, but navigating to the settings and clicking on an identity doesn't show the identities form but instead the login window. Using the "ready" hook doesn't help either.
Instead of my earlier proposal, maybe it would be more roundcube-like to add a plugin hook that takes care of creating the user objects. Maybe something like this?
user_set -- called before set_user is called params: $session_id return: rcube_user instance user_create -- called before rcube_user::create params: $user, $host return: rcube_user instance user_query -- called before rcube_user::query params: $user, $host return: rcube_user instance
Philipp
On 06/14/2013 12:16 AM, Philipp Kewisch wrote:
Any updates? I've done some more testing and found out that with the current way I am attempting to replace the user object (i.e doing so in the init() method) doesn't fully work. Login works without problems, but navigating to the settings and clicking on an identity doesn't show the identities form but instead the login window. Using the "ready" hook doesn't help either.
I suppose this might be a problem in your code.
Instead of my earlier proposal, maybe it would be more roundcube-like to add a plugin hook that takes care of creating the user objects. Maybe something like this?
user_set -- called before set_user is called params: $session_id return: rcube_user instance user_create -- called before rcube_user::create params: $user, $host return: rcube_user instance user_query -- called before rcube_user::query params: $user, $host return: rcube_user instance
Really, replacing user storage is not good idea. Users table stores user preferences and is referenced by foreign keys. So, you need user records in users table. If so, I don't see a reason for these hooks.
rcube_user::query() - the only problem with identities here is user_aliases option handling. You should disable it. and use virtuser_query plugin functionality.
rcube_user::create() - you need to abort identities creation on user create using identity_create hook.
On 7/2/13 1:55 PM, A.L.E.C wrote:
Really, replacing user storage is not good idea. Users table stores user preferences and is referenced by foreign keys. So, you need user records in users table. If so, I don't see a reason for these hooks.
I've went with a different solution now that synchronizes the databases.
Nevertheless, thank you for your input!
Philipp
-
A.L.E.C -
Philipp Kewisch