[SSSD] [PATCH] AD: add config and processing support gpo_map_* options
Jakub Hrozek
jhrozek at redhat.com
Fri Sep 5 15:08:03 UTC 2014
On Fri, Sep 05, 2014 at 04:40:05PM +0200, Sumit Bose wrote:
> On Fri, Sep 05, 2014 at 04:08:56PM +0200, Jakub Hrozek wrote:
> > On Fri, Sep 05, 2014 at 04:01:06PM +0200, Sumit Bose wrote:
> > > > +errno_t
> > > > +ad_gpo_parse_map_options(struct ad_access_ctx *access_ctx)
> > > > +{
> > > > + char *gpo_default_right_config;
> > > > + enum gpo_map_type gpo_default_right;
> > > > + errno_t ret;
> > > > + int i;
> > > > +
> > > > + for (i = 0; i < GPO_MAP_NUM_OPTS; i++) {
> > > > +
> > > > + struct gpo_map_option_entry entry = gpo_map_option_entries[i];
> > > > +
> > > > + char *entry_config = dp_opt_get_string(access_ctx->ad_options,
> > > > + entry.ad_basic_opt);
> > > > +
> > > > + ret = ad_gpo_parse_map_option(access_ctx, entry.gpo_map_type,
> > > > + access_ctx->gpo_map_options_table,
> > > > + entry_config, entry.gpo_map_defaults);
> > >
> > > ret is not checked here. As a result an invalid configuration where a
> > > service is added to two different maps is not detected. I would suggest
> > > to terminate SSSD here because the configuration is invalid and it is
> > > not clear to the admin to which login right the service is added.
> >
> > Thank you for the review. Do you think we need to terminate the back
> > end? Could we get away with denying all access so at least identity
> > provider works?
>
> That sounds like a good idea, but I'm afraid that this might be much
> more work then just terminating. Maybe we can just terminate here and
> add a ticket to add a more tolerant solution later?
Sure, this shouldn't block the patch review!
More information about the sssd-devel
mailing list