On (23/04/14 22:43), Jakub Hrozek wrote:
>Hi,
>
>the attached patches upstream functionality that mod_lookup_identity has
>been using for a while, but which we couldn't push to master due to the
>pending sbus changes. Now that these are accepted, it's time to merge
>the DBus methods themselves. Previously, mod_lookup_identity relied on a
>private branch with quite a bit of ad-hoc code.
>
>Please note that when the work in progress to expose users, groups and
>domains is merged as well, this interface will become obsolete in favor
>of the objects. But we shouldn't let a released version of
>mod_lookup_identity rely on unreleased sssd code any longer. Moreover,
>all patches except the last one will be used even later on and there's
>no fixed timeline on converting mod_lookup_identity.
>
>[PATCH 1/6] SBUS: two trivial style fixes
>SSIA
>
>[PATCH 2/6] SBUS: Add a convenience function
>Adds a convenience function that constructs a DBusError internally and
>as such can be used to mark an sbus request as failed without having
>to create a DBusError instance by the caller. The error message must be
>either allocated on top of dbus_req or a constant, DBus itself doesn't
>copy the error message! This is reflected in the doxygen docstring.
>
>[PATCH 3/6] IFP: Add utility functions
>Adds a number of utility functions, most importanly ifp_req_create().
>The ifp_req is a structure that will be passed along with the ifp
>request and contains the sbus request and some ifp-specific data, like
>caller ID or the responder context.
>
>[PATCH 4/6] IFP: use a list of allowed_uids for authentication
>Similar to the PAC responder, the InfoPipe uses a list of UIDs that are
>allowed to communicate with the IFP responder.
>
>[PATCH 5/6] IFP: Initialize negative cache timeout
>In order to avoid hitting the back end with repetitive requests, the
>InfoPipe responder needs a negative cache, too. This patch follows the
>convention set by other responders, where the negative cache timeouts are
>read from the [nss] section. This is not ideal, however, and ticket #2318
>tracks moving the configuration to the [ifp] section primarily.
>
>The timeout is also a separate parameter in the NSS context. We should
>consider moving it to the negcache context instead (#2317).
>
>[PATCH 6/6] IFP: Add GetUserAttrs call
>Adds a DBus method that allows the caller to retrieve attributes of a
>user. The synopsis of the call is as follows:
> <method name="GetUserAttr">
> <arg type="s" name="user" direction="in"/>
> <arg type="as" name="attr" direction="in"/>
> <arg type="a{sv}" name="values" direction="out"/>
> </method>
>
>The return value is an array (one attribute per array member) of
>dictionaries. The key of the dictionary is the attribute name, the value
>is a variant containing the attribute values as strings.
>
>If an attribute does not exist or is not permitted to be read, no error
>is returned. If the users does not exist, the method returns an error.
>
>In future patches this function will be marked as obsolete in favor of
>object-oriented approach.
>
>ifp_user_get_attr_unpack_msg is a separate function to allow extending
>it in a later patch.
>
>The function to check the cache validity duplicates quite a bit of code
>with the NSS responder. The refactoring would be nice to get done along
>with #843. I have a patch in my private branch, but it's not complete,
>sorry.
>From 53a9af5e3659293e2845b9a7aafea4e8bfbe818e Mon Sep 17 00:00:00 2001
>From: Jakub Hrozek <jhro...@redhat.com>
>Date: Mon, 6 Jan 2014 15:27:44 +0100
>Subject: [PATCH 3/6] IFP: Add utility functions
>
>Adds a number of utility functions, most importanly ifp_req_create().
>The ifp_req is a structure that will be passed along with the ifp
>request and contains the sbus request and some ifp-specific data, like
>caller ID.
>---
> Makefile.am | 29 ++++-
> src/responder/ifp/ifp_private.h | 16 +++
> src/responder/ifp/ifpsrv_util.c | 197 ++++++++++++++++++++++++++++++++++
> src/tests/cmocka/test_ifp.c | 231 ++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 472 insertions(+), 1 deletion(-)
> create mode 100644 src/responder/ifp/ifpsrv_util.c
> create mode 100644 src/tests/cmocka/test_ifp.c
>
>diff --git a/Makefile.am b/Makefile.am
>index
>56d8e1df648822a1fe8b3ca5fc8995f3c78a8566..e05461a3ab400e34bf685de89f2ec038822648fe
> 100644
>--- a/Makefile.am
>+++ b/Makefile.am
>@@ -176,7 +176,12 @@ if HAVE_CMOCKA
> dp_opt_tests \
> responder-get-domains-tests \
> test_search_bases
>-endif
>+
>+if BUILD_IFP
>+non_interactive_cmocka_based_tests += ifp_tests
>+endif # BUILD_IFP
>+
>+endif # HAVE_CMOCKA
>
> check_PROGRAMS = \
> stress-tests \
>@@ -818,6 +823,7 @@ sssd_ifp_SOURCES = \
> src/responder/ifp/ifpsrv_cmd.c \
> src/responder/ifp/ifp_iface_generated.c \
> src/responder/ifp/ifp_iface_generated.h \
>+ src/responder/ifp/ifpsrv_util.c \
> $(SSSD_UTIL_OBJ) \
> $(SSSD_RESPONDER_OBJ)
> sssd_ifp_CFLAGS = \
>@@ -1676,8 +1682,29 @@ dp_opt_tests_LDADD = \
> $(SSSD_INTERNAL_LTLIBS) \
> libsss_test_common.la
>
>+if BUILD_IFP
>+ifp_tests_DEPENDENCIES = \
^^^^
Use prefix EXTRA_ if you need memberof pluging for this test.
Otherwise, it can break parallel build.
We have this prefix also on other places.
>+ $(ldblib_LTLIBRARIES)
>+ifp_tests_SOURCES = \
>+ $(TEST_MOCK_RESP_OBJ) \
>+ src/tests/cmocka/test_ifp.c \
>+ src/responder/ifp/ifpsrv_cmd.c \
>+ src/responder/ifp/ifpsrv_util.c
>+ifp_tests_CFLAGS = \
>+ $(AM_CFLAGS)
>+ifp_tests_LDFLAGS = \
>+ -Wl,-wrap,sbus_conn_send_reply \
>+ -Wl,-wrap,dbus_message_get_sender \
>+ -Wl,-wrap,dbus_bus_get_unix_user
>+ifp_tests_LDADD = \
>+ $(CMOCKA_LIBS) \
>+ $(SSSD_LIBS) \
>+ $(SSSD_INTERNAL_LTLIBS) \
>+ libsss_test_common.la
> endif
>
>+endif # HAVE_CMOCKA
>+
> noinst_PROGRAMS = pam_test_client
> if BUILD_SUDO
> noinst_PROGRAMS += sss_sudo_cli
>diff --git a/src/responder/ifp/ifp_private.h b/src/responder/ifp/ifp_private.h
>index
>b97cb8a7d9e55f550a3eda0e9acc034329d4ba17..d33009a5dbc95f8c0ca8905d9c8b9e42eaa24988
> 100644
>--- a/src/responder/ifp/ifp_private.h
>+++ b/src/responder/ifp/ifp_private.h
>@@ -46,4 +46,20 @@ struct ifp_ctx {
> * It will be removed later */
> int ifp_ping(struct sbus_request *dbus_req, void *data);
>
>+/* == Utility functions == */
>+struct ifp_req {
>+ struct sbus_request *dbus_req;
>+ struct ifp_ctx *ifp_ctx;
>+ uid_t caller;
>+};
>+
>+struct ifp_req *ifp_req_create(struct sbus_request *dbus_req,
>+ struct ifp_ctx *ifp_ctx);
>+
>+int ifp_enomem(struct ifp_req *ireq);
>+int ifp_return_failure(struct ifp_req *ireq, const char *err_msg);
>+const char *ifp_path_strip_prefix(const char *path, const char *prefix);
>+errno_t ifp_add_ldb_el_to_dict(DBusMessageIter *iter_dict,
>+ struct ldb_message_element *el);
>+
> #endif /* _IFPSRV_PRIVATE_H_ */
//snip
>From 533aa8adef72fb5fa5cece55059ecfe1d1f030b9 Mon Sep 17 00:00:00 2001
>From: Jakub Hrozek <jhro...@redhat.com>
>Date: Tue, 15 Apr 2014 15:42:15 +0200
>Subject: [PATCH 6/6] IFP: Add GetUserAttrs call
>
>Adds a DBus method that allows the caller to retrieve attributes of a
>user. The synopsis of the call is as follows:
> <method name="GetUserAttr">
> <arg type="s" name="user" direction="in"/>
> <arg type="as" name="attr" direction="in"/>
> <arg type="a{sv}" name="values" direction="out"/>
> </method>
>
>The return value is an array (one attribute per array member) of
>dictionaries. The key of the dictionary is the attribute name, the value
>is a variant containing the attribute values as strings.
>
>If an attribute does not exist or is not permitted to be read, no error
>is returned. If the users does not exist, the method returns an error.
>
>In future patches this function will be marked as obsolete in favor of
>object-oriented approach.
>
>ifp_user_get_attr_unpack_msg is a separate function to allow extending
>it in a later patch.
>
>The function to check the cache validity duplicates quite a bit of code
>with the NSS responder. The refactoring would be nice to get done along
>with #843.
>---
There are warnings introduced in this last patch.
src/responder/ifp/ifpsrv_cmd.c: In function ‘ifp_user_get_attr_process’:
src/responder/ifp/ifpsrv_cmd.c:174:38: warning: ‘res’ may be used uninitialized
in this function [-Wmaybe-uninitialized]
el = ldb_msg_find_element(res->msgs[0], attrs[ai]);
^
src/responder/ifp/ifpsrv_cmd.c:119:24: note: ‘res’ was declared here
struct ldb_result *res;
^
And some in tests.
src/tests/cmocka/test_ifp.c: In function ‘test_el_to_dict’:
src/tests/cmocka/test_ifp.c:186:26: warning: cast discards
‘__attribute__((const))’ qualifier from pointer target type [-Wcast-qual]
el->values[0].data = (uint8_t *) "one";
^
src/tests/cmocka/test_ifp.c:188:26: warning: cast discards
‘__attribute__((const))’ qualifier from pointer target type [-Wcast-qual]
el->values[1].data = (uint8_t *) "two";
LS
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
