On 08/18/2014 04:24 PM, Pavel Reichl wrote:
>
> Thank you Pavel for comments, I was mostly able to fix patches as you
> asked for but I have a little problem with default_dn. Could you please
> see my comments inline?
>
>
> On 08/18/2014 12:13 PM, Pavel Březina wrote:
>> On 08/15/2014 03:13 PM, Pavel Reichl wrote:
>>>
>>> On 08/11/2014 04:51 PM, Jakub Hrozek wrote:
>>>> On Mon, Aug 11, 2014 at 04:19:22PM +0200, Pavel Reichl wrote:
>>>>>>> Subject: [PATCH 7/9] SDAP: new option - DN to ppolicy on
LDAP
>>>>>> Do we need to make the DN configurable? When you install the
>>>>>> ppolicy,
>>>>>> can you actually set the DN?
>>>>> I haven't tested setting different DN, but I believe it's
possible.
>>>>> During weekend I was putting my notes for setting ppolicy on
>>>>> OpenLDAP on
>>>>> this wiki page
>>>>>
https://fedorahosted.org/sssd/wiki/openldap_ppolicy#Loadingppolicyoverlay
>>>>>
>>>>>
>>>>>
>>>>> (this is work in progress and I'm not even sure we care to have
>>>>> such a
>>>>> how-to on our wiki), but I believe you can see from there that DN is
>>>>> configurable.
>>>> Any howto is good as long as it's up to date.
>>>>
>>>> And from the howto it seems clear that olcPPolicyDefault is the DN
>>>> setting, so yes, this should be configurable.
>>>>
>>>>> Still we could use a default DN if user didn't set DN himself:
>>>>>
>>>>> "cn=ppolicy,ou=policies,$search_base"
>>>> That sounds like a good idea.
>>>> _______________________________________________
>>>> sssd-devel mailing list
>>>> sssd-devel(a)lists.fedorahosted.org
>>>>
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>>> Hello,
>>>
>>> please note that attached patches requires to be patches from "SDAP:
>>> refactor sdap_access" thread to pushed first.
>>
>> Patch 7:
>> Nack.
>>
>> + /* cached results of access control checks */
>> ^ cached_access contain only one result
>> bool cached_access;
>> const char *basedn;
>> };
>>
>> Please remove sdap_access_{filter|lockout}_decide_offline and use only
>> sdap_access_decide_offline that will take bool instead of tevent_req
>> as parameter.
>>
>> Patch 8:
>> Ack.
>>
>> Patch 9:
>>
>> + case LDAP_ACCESS_LOCKOUT:
>> + ret = EOK;
>> +
>> + subreq = sdap_access_lock_send(state, state->ev,
>> state->be_ctx,
>> + state->domain,
>> + state->access_ctx,
>> + state->conn,
>> + state->pd->user,
>> + state->user_entry);
>> + if (subreq == NULL) {
>> + DEBUG(SSSDBG_CRIT_FAILURE, "sdap_access_lock_send
>> failed.\n");
>> + return ENOMEM;
>> + }
>> +
>> + state->ac_type = SDAP_ACCESS_CONTROL_PPOLICY_LOCK;
>> +
>> + tevent_req_set_callback(subreq, sdap_access_done, req);
>> + return EAGAIN;
>> + break;
>> +
>>
>> There is no need for ret = EOK and break.
>>
>> +static char*
>> +get_default_ppolicy_dn(TALLOC_CTX *mem_ctx, struct sdap_options *opts)
>> +{
>> + char *search_base;
>> +
>> + search_base = dp_opt_get_string(opts->basic, SDAP_SEARCH_BASE);
>> +
>> + return talloc_asprintf(mem_ctx, "cn=ppolicy,ou=policies,%s",
>> search_base);
>> +}
>>
>> ldap_search_base option may contain more than one search base and it
>> may also contain filter. This option is parsed during initialization
>> of the provider and stored in sdap_domain->*search_base. But since
>> there may be more, you need to iterate over them.
> Could you please elaborate a bit more about this? I'm afraid I don't
> fully understand.
>
> Would you like me to create an array of possible default DN's and query
> ldap server for every one? Or is there a way how to use multiple dn in a
> single ldap query?
Yes, you have to iterate over all of them. There is no way how to
search all of them in one query AFAIK.
However, I think it is sufficient to use only domain components to
create default dn, i.e. basedn of domain (dc=example,dc=com). This can
be obtained from sdap_domain structure. Then you wouldn't have to
iterate over all search bases.
>>
>> However, I think it is sufficient to use only dc part here so you can
>> use basedn from sdap_domain structure.
>>
>> Also, I think it would be better if this function is removed and the
>> default value is set in ldap_get_options() with dp_opt_set_string if
>> the option is not set.
>>
>>
>>
>> _______________________________________________
>> sssd-devel mailing list
>> sssd-devel(a)lists.fedorahosted.org
>>
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>
> _______________________________________________
> sssd-devel mailing list
> sssd-devel(a)lists.fedorahosted.org
>
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel