On Mon, Sep 10, 2012 at 09:35:25AM +0200, Pavel Březina wrote:
On 09/07/2012 11:31 AM, Sumit Bose wrote:
On Wed, Sep 05, 2012 at 03:05:49PM +0200, Pavel Březina wrote:
0001 fixes https://fedorahosted.org/sssd/ticket/1458 0002 fixes a memory leak when be_process_init() fails. I think it should be fixed, even though the backend is in this case immediately terminated
Hi,
the patches compile without errors and work as expected. Nevertheless I have a few comments:
0001:
- I think it make sense to skip the initialization of the sudo target if there is no responder configured. But to avoid surprises on the user side I would recommend to do the initialization if it is configured explicitly in sssd.conf, i.e sudo_provider = ldap, and give a warning in the logs that the corresponding responder will not be running.
I discussed this with Jakub before writing the patch. sudo_provider currently defaults to id_provider. If I get it right, you are suggesting to make the default value "none".
We agreed with Jakub that this makes more pressure on the user to understand the configuration so we chose not to do it. Although the configurations I've been seeing so far are mostly generated by authconfig or ipa-client-install (where are all providers set explicitly) so I don't think it is a problem after all. What do you say Jakub?
I would personally prefer that the value is inherited from id_provider to keep the configuration simple..
One place I can think of where this inheritance might get confusing is multidomain environment where enabling the [sudo] service would automatically enable the sudo provider in all domains.