On Fri, Jan 09, 2015 at 11:54:27AM +0100, Jakub Hrozek wrote:
On Wed, Dec 17, 2014 at 11:27:46AM +0100, Sumit Bose wrote:
> yes, the check was useless because state->obj_msg is always set if
> get_object_from_cache() returns EOK.
>
> New version attached.
>
> bye,
> Sumit
>
> > _______________________________________________
> > sssd-devel mailing list
> > sssd-devel(a)lists.fedorahosted.org
> >
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>
After testing the patches, I can confirm that group resolution of a
group that contains a user with overriden login attribute:
ipa idoverrideuser-mod --login=overriden_tuser
Now works OK. But getent group still shows the original user name (b/c
the original user name is set in the memberUid attribute, set by the
memberof plugin). Is that expected.
no, the new version contains patches 3 and 4 which should fix it.
There is a compilation warning in the code:
src/providers/ipa/ipa_id.c: In function 'ipa_resolve_user_list_get_user_step':
src/providers/ipa/ipa_id.c:218:11: warning: pointer targets in passing argument 2 of
'get_be_acct_req_for_user_name' differ in signedness [-Wpointer-sign]
ret = get_be_acct_req_for_user_name(state,
^
In file included from src/providers/ipa/ipa_id.c:31:0:
./src/providers/ipa/ipa_id.h:90:9: note: expected 'const char *' but argument is
of type 'uint8_t *'
fixed
Two nitpicks in the code:
> +static struct tevent_req *
> +ipa_resolve_user_list_send(TALLOC_CTX *memctx, struct tevent_context *ev,
> + struct be_req *be_req,
> + struct sdap_id_ctx *sdap_id_ctx,
> + const char *domain_name,
> + struct ldb_message_element *users)
> +{
> + int ret;
> + struct tevent_req *req;
> + struct ipa_resolve_user_list_state *state;
> +
> + req = tevent_req_create(memctx, &state,
> + struct ipa_resolve_user_list_state);
> + if (req == NULL) {
> + DEBUG(SSSDBG_OP_FAILURE, "tevent_req_create failed.\n");
> + return NULL;
> + }
> +
> + state->ev = ev;
> + state->sdap_id_ctx = sdap_id_ctx;
> + state->be_req = be_req;
> + state->domain_name = domain_name;
> + state->users = users;
> + state->user_idx = 0;
> + state->dp_error = DP_ERR_FATAL;
> +
> + ret = ipa_resolve_user_list_get_user_step(req);
> + if (ret == EAGAIN) {
> + return req;
> + }
Can you add else if into this check:
if EAGAIN
else if ret != EOK
To make it more clear that it's one check, not two.
fixed
> + if (ret != EOK) {
> + DEBUG(SSSDBG_OP_FAILURE,
> + "ipa_resolve_user_list_get_user_step failed.\n");
> + }
[...]
> @@ -166,6 +315,7 @@ static errno_t ipa_id_get_account_info_get_original_step(struct
tevent_req *req,
> struct be_acct_req *ar);
> static void ipa_id_get_account_info_orig_done(struct tevent_req *subreq);
> static void ipa_id_get_account_info_done(struct tevent_req *subreq);
> +static void ipa_id_get_user_list_done(struct tevent_req *subreq);
>
> static struct tevent_req *
> ipa_id_get_account_info_send(TALLOC_CTX *memctx, struct tevent_context *ev,
> @@ -405,6 +555,16 @@ static void ipa_id_get_account_info_orig_done(struct tevent_req
*subreq)
> goto fail;
> }
>
> + if ((state->ar->entry_type & BE_REQ_TYPE_MASK) == BE_REQ_GROUP
> + && state->ipa_ctx->view_name != NULL
> + && strcmp(state->ipa_ctx->view_name,
> + SYSDB_DEFAULT_VIEW_NAME) != 0) {
> + /* check for ghost members because shost members are not allowed if a
typo: s/shost/ghost/
fixed
New version attached.
Thank you for the review.
bye,
Sumit
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel