[Fedora-directory-devel] Please review: Bug 206450: pass thru auth plugin should be configured by default

[Rich Megginson]

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=206450
Bug(s) fixed: 206450
Bug Description: pass thru auth plugin should be configured by default
Reviewed by: ???
Files: see diff
Branch: HEAD
Fix Description: If you do a core DS build, you don't have a config DS 
or a user DS, and
therefore the pass thru auth plugin is not added to the server config.  It
should always be added, disabled if not used immediately.  The fix is to 
add it in this case, disabled.
Platforms tested: RHEL4
Flag Day: no
Doc impact: no
https://bugzilla.redhat.com/bugzilla/attachment.cgi?id=136265

[Rob Crittenden]

I have mixed feelings. It duplicates a fair bit of code but then again, 
it is moderately clear why. Would it be overly complex to merge these, 
something like:

if (cf->use_existing_config_ds || cf->use_existing_user_ds)
     {
         LDAPURLDesc *desc = 0;
         char *url = cf->use_existing_config_ds ? cf->config_ldap_url :
             cf->user_ldap_url;
         char *suffix = NULL;

if (url && !ldap_url_parse(url, &desc) && desc)
         {
             suffix = desc->lud_dn;
             char *service = !strncmp(url, "ldaps:", strlen("ldaps:")) ?
                 "ldaps" : "ldap";
             if (cf->use_existing_config_ds)
             {
                 suffix = cf->netscaperoot;
             }

suffix = ds_URL_encode(suffix);
         }
         fprintf ...
         fprintf(f, "nsslapd-pluginenabled: %s\n", "on" : suffix ? "off");
         if ( suffix ) {
             fprintf(f, "nsslapd-pluginarg0: %s://%s:%d/%s\n", service, 
desc->lud_host, desc->lud_port, suffix);
             free(suffix);
             ldap_free_urldesc(desc);

}
         ...

rob

Richard Megginson wrote:
> https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=206450
> Bug(s) fixed: 206450
> Bug Description: pass thru auth plugin should be configured by default
> Reviewed by: ???
> Files: see diff
> Branch: HEAD
> Fix Description: If you do a core DS build, you don't have a config DS 
> or a user DS, and
> therefore the pass thru auth plugin is not added to the server config.  It
> should always be added, disabled if not used immediately.  The fix is to 
> add it in this case, disabled.
> Platforms tested: RHEL4
> Flag Day: no
> Doc impact: no
> https://bugzilla.redhat.com/bugzilla/attachment.cgi?id=136265
>
> --
> Fedora-directory-devel mailing list
> [e-mail]
> https://www.redhat.com/mailman/listinfo/fedora-directory-devel

[Rich Megginson]

Rob Crittenden wrote:
> I have mixed feelings. It duplicates a fair bit of code but then 
> again, it is moderately clear why. Would it be overly complex to merge 
> these, something like:
> if (cf->use_existing_config_ds || cf->use_existing_user_ds)
> {

Thanks Rob.  Actually, this will not work, because if you install just 
the core DS both cf->use_existing_config_ds and cf->use_existing_user_ds 
will be false.  I would have to rewrite more stuff in order to avoid 
code duplication.

This code will be changing quite a bit in the near future, as we rip out 
all of the admin server/setuputil related code from the core DS.  This 
fix is really just to make testing easier, so you don't have to add the 
pass thru auth plugin config entry - you can just enable it and set the url.
>     LDAPURLDesc *desc = 0;
>     char *url = cf->use_existing_config_ds ? cf->config_ldap_url :
>         cf->user_ldap_url;
>     char *suffix = NULL;
>
>     if (url && !ldap_url_parse(url, &desc) && desc)
>     {
>         suffix = desc->lud_dn;
>         char *service = !strncmp(url, "ldaps:", strlen("ldaps:")) ?
>             "ldaps" : "ldap";
>         if (cf->use_existing_config_ds)
>         {
>             suffix = cf->netscaperoot;
>         }
>
>         suffix = ds_URL_encode(suffix);
>     }
>     fprintf ...
>     fprintf(f, "nsslapd-pluginenabled: %s\n", "on" : suffix ? "off");
>     if ( suffix ) {
>         fprintf(f, "nsslapd-pluginarg0: %s://%s:%d/%s\n", service, 
>
> desc->lud_host, desc->lud_port, suffix);
>             free(suffix);
>             ldap_free_urldesc(desc);
>     }
>     ...
>
> rob
>
> Richard Megginson wrote:
> > https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=206450
> > Bug(s) fixed: 206450
> > Bug Description: pass thru auth plugin should be configured by default
> > Reviewed by: ???
> > Files: see diff
> > Branch: HEAD
> > Fix Description: If you do a core DS build, you don't have a config 
> > DS or a user DS, and
> > therefore the pass thru auth plugin is not added to the server 
> > config.  It
> > should always be added, disabled if not used immediately.  The fix is 
> > to add it in this case, disabled.
> > Platforms tested: RHEL4
> > Flag Day: no
> > Doc impact: no
> > https://bugzilla.redhat.com/bugzilla/attachment.cgi?id=136265
> >
> > -- 
> > Fedora-directory-devel mailing list
> > [e-mail]
> > https://www.redhat.com/mailman/listinfo/fedora-directory-devel
>
> --
> Fedora-directory-devel mailing list
> [e-mail]
> https://www.redhat.com/mailman/listinfo/fedora-directory-devel

[Rob Crittenden]

Ok, the change seems fine then.

rob

Richard Megginson wrote:
> Rob Crittenden wrote:
> > I have mixed feelings. It duplicates a fair bit of code but then 
> > again, it is moderately clear why. Would it be overly complex to merge 
> > these, something like:
> > if (cf->use_existing_config_ds || cf->use_existing_user_ds)
> > {
>
> Thanks Rob.  Actually, this will not work, because if you install just 
> the core DS both cf->use_existing_config_ds and cf->use_existing_user_ds 
> will be false.  I would have to rewrite more stuff in order to avoid 
> code duplication.
>
> This code will be changing quite a bit in the near future, as we rip out 
> all of the admin server/setuputil related code from the core DS.  This 
> fix is really just to make testing easier, so you don't have to add the 
> pass thru auth plugin config entry - you can just enable it and set the 
> url.
> >     LDAPURLDesc *desc = 0;
> >     char *url = cf->use_existing_config_ds ? cf->config_ldap_url :
> >         cf->user_ldap_url;
> >     char *suffix = NULL;
> >
> >     if (url && !ldap_url_parse(url, &desc) && desc)
> >     {
> >         suffix = desc->lud_dn;
> >         char *service = !strncmp(url, "ldaps:", strlen("ldaps:")) ?
> >             "ldaps" : "ldap";
> >         if (cf->use_existing_config_ds)
> >         {
> >             suffix = cf->netscaperoot;
> >         }
> >
> >         suffix = ds_URL_encode(suffix);
> >     }
> >     fprintf ...
> >     fprintf(f, "nsslapd-pluginenabled: %s\n", "on" : suffix ? "off");
> >     if ( suffix ) {
> >         fprintf(f, "nsslapd-pluginarg0: %s://%s:%d/%s\n", service, 
> >
> > desc->lud_host, desc->lud_port, suffix);
> >             free(suffix);
> >             ldap_free_urldesc(desc);
> >     }
> >     ...
> >
> > rob
> >
> > Richard Megginson wrote:
> > > https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=206450
> > > Bug(s) fixed: 206450
> > > Bug Description: pass thru auth plugin should be configured by default
> > > Reviewed by: ???
> > > Files: see diff
> > > Branch: HEAD
> > > Fix Description: If you do a core DS build, you don't have a config 
> > > DS or a user DS, and
> > > therefore the pass thru auth plugin is not added to the server 
> > > config.  It
> > > should always be added, disabled if not used immediately.  The fix is 
> > > to add it in this case, disabled.
> > > Platforms tested: RHEL4
> > > Flag Day: no
> > > Doc impact: no
> > > https://bugzilla.redhat.com/bugzilla/attachment.cgi?id=136265
> > >
> > > -- 
> > > Fedora-directory-devel mailing list
> > > [e-mail]
> > > https://www.redhat.com/mailman/listinfo/fedora-directory-devel
> >
> > -- 
> > Fedora-directory-devel mailing list
> > [e-mail]
> > https://www.redhat.com/mailman/listinfo/fedora-directory-devel
>
> --
> Fedora-directory-devel mailing list
> [e-mail]
> https://www.redhat.com/mailman/listinfo/fedora-directory-devel