On Wed, Aug 17, 2016 at 03:13:57PM +0200, Fabiano Fidêncio wrote: > The attached patch for #3125 [0]is based on Jakub's "secrets" branch > on github [1], as there are (at least) a few issues with the current > secrets code. > > [0]: https://fedorahosted.org/sssd/ticket/312 > [1]: https://github.com/jhrozek/sssd/tree/secrets > > Although I didn't fire a CI build, the local "make intgcheck" passes > without issues. > > Best Regards, > -- > Fabiano Fidêncio
Sorry about the late reply. The patch works fine as I would expect: sudo curl -H "Content-Type: application/json" --unix-socket /var/run/secrets.socket -XDELETE http://localhost/secrets/xxx <html> <head> <title>404 Not Found</title></head> <body> <h1>Not Found</h1> <p>The requested resource was not found.</p> </body> Nonetheless, I think the patch can be a bit simpler: > From bc39f7ba0ffa75a9ca972266bbc2741cbb0c8c73 Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> > Date: Wed, 17 Aug 2016 13:12:21 +0200 > Subject: [PATCH] SECRETS: Check whether a secret exists before trying to > delete it > MIME-Version: 1.0 > Content-Type: text/plain; charset=UTF-8 > Content-Transfer-Encoding: 8bit > > Checking whether a secret exists or not before trying to delete it > allows SSSD to report the proper error code in case of trying to delete > a non-existent secret. > > Resolves: > https://fedorahosted.org/sssd/ticket/3125 > > Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> > --- > src/responder/secrets/local.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/src/responder/secrets/local.c b/src/responder/secrets/local.c > index e91766f..3985b6d 100644 > --- a/src/responder/secrets/local.c > +++ b/src/responder/secrets/local.c > @@ -376,12 +376,26 @@ int local_db_delete(TALLOC_CTX *mem_ctx, > struct local_context *lctx, > const char *req_path) > { > + static const char *attrs[] = { "secret", NULL }; > struct ldb_dn *dn; > + struct ldb_result *res; > int ret; > > ret = local_db_dn(mem_ctx, lctx->ldb, req_path, &dn); > if (ret != EOK) goto done; > > + ret = ldb_search(lctx->ldb, mem_ctx, &res, dn, LDB_SCOPE_BASE, > + attrs, "%s", LOCAL_SIMPLE_FILTER); > + if (ret != EOK) { > + ret = ENOENT; > + goto done; > + } > + > + if (res->count == 0) { > + ret = ENOENT; > + goto done; > + } > + > ret = ldb_delete(lctx->ldb, dn); I think it's OK to just try to delete the secret and act on the return value. This is what we already have in sysdb_delete_cache_entry(): 105 ret = ldb_delete(ldb, dn); 106 switch (ret) { 107 case LDB_SUCCESS: 108 return EOK; 109 case LDB_ERR_NO_SUCH_OBJECT: 110 if (ignore_not_found) { 111 return EOK; 112 } So I think we can use something else in secrets to return ENOENT on receiving LDB_ERR_NO_SUCH_OBJECT from ldb_delete(). _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
