On Fri, Oct 11, 2013 at 09:59:17AM +0200, Jakub Hrozek wrote:
> On Fri, Oct 11, 2013 at 09:37:54AM +0200, Sumit Bose wrote:
> > On Thu, Oct 10, 2013 at 02:49:13PM -0400, Stephen Gallagher wrote:
> > > -----BEGIN PGP SIGNED MESSAGE-----
> > > Hash: SHA1
> > >
> > > On 10/10/2013 02:13 PM, Jakub Hrozek wrote:
> > > > Hi,
> > > >
> > > > if an entry is removed from LDAP and searched by SID, the SID
> > > > lookup code doesn't handle ENOENT and doesn't remove the stray
> > > > entry from cache. The attached patch fixes that.
> > > >
> > >
> > > One minor nitpick (no need to resend for review): the
> > > SSSBG_FATAL_FAILURE line is > 79 characters. Please reflow it.
> > >
> > > Otherwise, code looks good to me. Ack.
> >
> > I agree, I just wonder if the new code can be move to a new call like
> > e.g. sysdb_delete_by_sid() to make it easier to reuse it?
> >
> > bye,
> > Sumit
>
> Yes, I also noticed there is a duplication of search-by-sid functions in
> master. I will send updated patches, probably next week when I'm back. I
> don't think it's a pressing issue.
New patches attached. I haven't done anything about the duplication, I
think it should better be solved when following up on Ondra's patches as
the duplication (sysdb_search_object_by_sid vs
sysdb_search_entry_by_sid) was introduced there.
>From fb239353ba83dbdf9bb83c0d43c128bfe38bc225 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Tue, 22 Oct 2013 13:50:51 +0200
Subject: [PATCH 1/2] SYSDB: Add sysdb_delete_by_sid
---
src/db/sysdb.h | 4 ++++
src/db/sysdb_ops.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
src/tests/sysdb-tests.c | 12 ++++++++++++
3 files changed, 65 insertions(+)
diff --git a/src/db/sysdb.h b/src/db/sysdb.h
index
2f73873f0c0be4f8b021519ad0953dcd68932180..4d5ef0b4794a85aa7fbc8f261d42eddd1043284e
100644
--- a/src/db/sysdb.h
+++ b/src/db/sysdb.h
@@ -812,6 +812,10 @@ int sysdb_delete_netgroup(struct sysdb_ctx *sysdb,
struct sss_domain_info *domain,
const char *name);
+int sysdb_delete_by_sid(struct sysdb_ctx *sysdb,
+ struct sss_domain_info *domain,
+ const char *sid_str);
+
errno_t sysdb_attrs_to_list(TALLOC_CTX *mem_ctx,
struct sysdb_attrs **attrs,
int attr_count,
diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c
index
ca23fadbf0ed3306795c6a63d11b8dbbbce6b549..094c27b7f478e0a53a3b6666c727e86eb36a249e
100644
--- a/src/db/sysdb_ops.c
+++ b/src/db/sysdb_ops.c
@@ -2841,6 +2841,55 @@ done:
return ret;
}
+int sysdb_delete_by_sid(struct sysdb_ctx *sysdb,
+ struct sss_domain_info *domain,
+ const char *sid_str)
+{
+ TALLOC_CTX *tmp_ctx;
+ struct ldb_result *res;
+ int ret;
+
+ if (!sid_str) return EINVAL;
+
+ tmp_ctx = talloc_new(NULL);
+ if (!tmp_ctx) {
+ return ENOMEM;
+ }
+
+ ret = sysdb_search_object_by_sid(tmp_ctx, sysdb, domain,
+ sid_str, NULL, &res);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_OP_FAILURE, ("search by sid failed: %d (%s)\n",
+ ret, strerror(ret)));
+ goto done;
+ }
+
+ if (res->count > 1) {
+ DEBUG(SSSDBG_FATAL_FAILURE, ("getbysid call returned more than one " \
+ "result !?!\n"));
+ ret = EIO;
+ goto done;
+ }
+
+ if (res->count == 0) {
+ /* No existing entry. Just quit. */
+ ret = EOK;
+ goto done;
+ }
+
+ ret = sysdb_delete_entry(sysdb, res->msgs[0]->dn, false);
+ if (ret != EOK) {
+ goto done;
+ }
+
+done:
+ if (ret != EOK) {
+ DEBUG(SSSDBG_OP_FAILURE, ("Error: %d (%s)\n", ret, strerror(ret)));
+ }
+ talloc_free(tmp_ctx);
+ return ret;
+}
+
/* ========= Authentication against cached password ============ */
diff --git a/src/tests/sysdb-tests.c b/src/tests/sysdb-tests.c
index
74b78917fa85f6c1551a3b55c394aa05419a4057..1c28526e06df012b8749e1540e70a27948c17ab2
100644
--- a/src/tests/sysdb-tests.c
+++ b/src/tests/sysdb-tests.c
@@ -4499,6 +4499,18 @@ START_TEST(test_sysdb_search_sid_str)
fail_unless(ret == EOK, "sysdb_search_group_by_sid_str failed with
[%d][%s].",
ret, strerror(ret));
+ /* Delete the group by SID */
+ ret = sysdb_delete_by_sid(test_ctx->sysdb, test_ctx->domain, "S-1-2-3-4");
+ fail_unless(ret == EOK, "sysdb_delete_by_sid failed with [%d][%s].",
+ ret, strerror(ret));
+
+ /* Verify it's gone */
+ ret = sysdb_search_group_by_sid_str(test_ctx, test_ctx->sysdb,
+ test_ctx->domain, "S-1-2-3-4",
+ NULL, &msg);
+ fail_unless(ret == ENOENT, "sysdb_search_group_by_sid_str failed with
[%d][%s].",
+ ret, strerror(ret));
+
talloc_free(msg);
msg = NULL;
--
1.8.3.1
>From febd16d816e817fdf8d293cecc16c673344d625a Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Thu, 10 Oct 2013 19:21:07 +0200
Subject: [PATCH 2/2] LDAP: Delete entry by SID if not found
In case the entry was deleted from the server, the search didn't notice
and kept returning the cached data.
---
src/providers/ldap/ldap_id.c | 22 +++++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/src/providers/ldap/ldap_id.c b/src/providers/ldap/ldap_id.c
index
59dfd0a5d41fa9adc14ab1297563cfe499a4b675..d9c565c21cfc48dae5036f967c8e3b904928441d
100644
--- a/src/providers/ldap/ldap_id.c
+++ b/src/providers/ldap/ldap_id.c
@@ -1551,12 +1551,28 @@ static void get_user_and_group_users_done(struct
tevent_req *subreq)
ret = users_get_recv(subreq, &state->dp_error, &state->sdap_ret);
talloc_zfree(subreq);
- if (ret == EOK) { /* Matching user found */
- tevent_req_done(req);
- } else {
+ if (ret != EOK) {
tevent_req_error(req, ret);
+ return;
}
+ if (state->sdap_ret == ENOENT) {
+ /* The search ran to completion, but nothing was found.
+ * Delete the existing entry, if any. */
+ ret = sysdb_delete_by_sid(state->sysdb, state->domain,
+ state->filter_val);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_OP_FAILURE, ("Could not delete entry by SID!\n"));
+ tevent_req_error(req, ret);
+ return;
+ }
+ } else if (state->sdap_ret != EOK) {
+ tevent_req_error(req, EIO);
+ return;
+ }
+
+ /* Both ret and sdap->ret are EOK. Matching user found */
+ tevent_req_done(req);
return;
}
--
1.8.3.1
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
