[SSSD] [PATCHES] backend: initialize sudo only when it is enabled in services
Sumit Bose
sbose at redhat.com
Mon Sep 10 08:48:16 UTC 2012
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".
No, sorry for not being clear here. I think the id_provider default is
right. I just wanted to say that if for whatever reasons the
sudo_provider parameter is used explicitly in sssd.conf the
sudo_provider should be initialization even if sudo is not listed in
services.
bye,
Sumit
>
> 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?
>
> If we decide to do it this way, it will also require changes in man page.
>
> >>+ ret = confdb_get_string(ctx->cdb, ctx, CONFDB_MONITOR_CONF_ENTRY,
> >>+ CONFDB_MONITOR_ACTIVE_SERVICES, NULL, &services);
> >>+ if (ret != EOK) {
> >>+ DEBUG(SSSDBG_FATAL_FAILURE,
> >>+ ("Unable to read from confdb [%d]: %s\n", ret, strerror(ret)));
> >>+ return ret;
> >>+ }
> >>+
> >
> > - I would prefer to use confdb_get_string_as_list() and use strcmp()
> > later on, to be on the safe side.
> >
> >0002: ACK
> >
> >bye,
> >Sumit
> >_______________________________________________
> >sssd-devel mailing list
> >sssd-devel at lists.fedorahosted.org
> >https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> >
>
>
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
More information about the sssd-devel
mailing list