On Mon, Feb 24, 2014 at 07:12:00PM +0100, Jakub Hrozek wrote:
On Thu, Feb 20, 2014 at 10:34:44AM +0100, Sumit Bose wrote:
> On Wed, Feb 19, 2014 at 04:23:55PM +0100, Jakub Hrozek wrote:
> > On Wed, Feb 12, 2014 at 03:43:51PM +0100, Sumit Bose wrote:
> > > Hi,
> > >
> > > currently we trust the idrange data we get from the IPA server. But it
> > > turned out that some checks are missing on the server
> > > (
https://fedorahosted.org/freeipa/ticket/4137) so SSSD should check the
> > > input before saving the data to the cache.
> > >
> > > The first two patches make some code which already exists available for
> > > other callers. The third contains the actual functionality.
> > >
> > > bye,
> > > Sumit
> >
> > Hi, I rebased your patches on top of master, can you check if they look
> > OK to you and resend if they are fine? I'll continue testing with my
> > version in the meantime.
>
> sorry I forgot to send a version for master myself. The patches are
> exactly the same as in my master tree.
>
> Thanks
>
> bye,
> Sumit
...
> err = sss_idmap_add_domain_ex(idmap_ctx->map, name,
sid, &range,
> - r->name, rid, external_mapping);
> - if (err != IDMAP_SUCCESS && err != IDMAP_COLLISION) {
> + range_list[c]->name, rid,
> + external_mapping);
> + if (err != IDMAP_SUCCESS
> + && allow_collisions && err != IDMAP_COLLISION) {
I don't think this condition is right, if allow_collisions is false,
then it never matches.
good catch, thank you, fixed.
> DEBUG(SSSDBG_CRIT_FAILURE, "Could not add range [%s] to ID
map\n",
> - r->name);
> + range_list[c]->name);
> ret = EIO;
> goto done;
> }
> }
[snip]
> diff --git a/src/tests/cmocka/test_ipa_idmap.c b/src/tests/cmocka/test_ipa_idmap.c
[snip]
> +int main(int argc, const char *argv[])
> +{
> + poptContext pc;
> + int opt;
> + struct poptOption long_options[] = {
> + POPT_AUTOHELP
> + SSSD_DEBUG_OPTS
> + POPT_TABLEEND
> + };
> +
> + const UnitTest tests[] = {
> + //unit_test_setup_teardown(test_add_domain,
> + // test_sss_idmap_setup, test_sss_idmap_teardown),
It seems that no further patch includes test_add_domain either, did you
want to remove the commented out code?
yes, I removed it and added a new test to make sure the allow_collisions
check above is working as expected.
New version for master and 1.11 are attached.
bye,
Sumit