[SSSD] NSS: Add config option to expand homedir template format
Pavel Reichl
preichl at redhat.com
Wed Mar 12 13:52:46 UTC 2014
On Tue, 2014-03-04 at 10:29 +0100, Lukas Slebodnik wrote:
> On (03/03/14 13:35), Lukas Slebodnik wrote:
> >On (31/01/14 23:58), Jakub Hrozek wrote:
> >>On Fri, Jan 31, 2014 at 11:55:25AM +0100, Pavel Březina wrote:
> >>> On 12/02/2013 04:12 PM, Lukas Slebodnik wrote:
> >>> >ehlo,
> >>> >
> >>> >attached patches address ticket #1853
> >>> >
> >>> >LS
> >>>
> >>> Hi,
> >>> these patches no longer apply.
> >>
> >>for patches targeting 1.12, i would prefer to rebase them on top of Nikolai's
> > ^^^^
> > 1.11
> >>and Stephen's logging patches, those are the first to go to master now.
> >
> >Rebased patches are attached,
> >
> >LS
>
> Sending rebased patches on top of "ipa-server-mode: use lower-case user name
> for home dir"
>
> LS
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Hello Lukas,
I'm afraid I have some troubles witch patches:
ipa user-find test_home
--------------
1 user matched
--------------
User login: test_home
First name: test
Last name: home
Home directory: %H/test_home
Login shell: /bin/bash
Email address: test_home at ipa.work
UID: 1994000007
GID: 1994000007
Account disabled: False
Password: False
Kerberos keys available: False
----------------------------
Number of entries returned 1
----------------------------
# getent passwd test_home
test_home:*:1994000007:1994000007:test home:/:/bin/bash]
I suppose homedir should rather be /home/test_home then /.
>
> @@ -318,6 +316,7 @@ static int fill_pwent(struct sss_packet *packet,
> bool packet_initialized = false;
> int ncret;
> TALLOC_CTX *tmp_ctx = NULL;
> + struct sss_nss_homedir_ctx *homedir_ctx;
>
> to_sized_string(&pwfield, nctx->pwfield);
>
> @@ -373,7 +372,19 @@ static int fill_pwent(struct sss_packet *packet,
> } else {
> to_sized_string(&gecos, tmpstr);
> }
> - tmpstr = get_homedir_override(tmp_ctx, msg, nctx, dom,
name.str, uid);
> +
> + homedir_ctx = talloc_zero(tmp_ctx, struct
sss_nss_homedir_ctx);
> + if (! homedir_ctx) {
> + num = 0;
> + ret = ENOMEM;
> + goto done;;
> + }
> +
> + homedir_ctx->username = name.str;
> + homedir_ctx->uid = uid;
> + homedir_ctx->domain = dom->name;
Should not be homedir_ctx->config_homedir_substr set to
nctx->homedir_substring or rather domain->homedir_substring if not
empty?
> +
> + tmpstr = get_homedir_override(tmp_ctx, msg, nctx, dom,
homedir_ctx);
> if (!tmpstr) {
> to_sized_string(&homedir, "/");
> } else {
> }
I have noticed a few interesting construction regarding our code style:
patch 2:
> From f06de6cf3130d0b01b04152e11933d1c4d875ea6 Mon Sep 17 00:00:00 2001
> From: Lukas Slebodnik <lslebodn at redhat.com>
> Date: Mon, 2 Dec 2013 13:03:33 +0100
> Subject: [PATCH 2/3] NSS: Refactor expand_homedir_template
>
> Function expand_homedir_template had lot of parameters.
> After adding new expand option, all function call should be rewritten,
> (usually argument NULL will be added)
>
> This patch wraps all necessary arguments to structure.
> ---
> src/providers/ipa/ipa_s2n_exop.c | 17 ++++++++++----
> src/providers/ipa/ipa_subdomains_id.c | 27 ++++++++++++++--------
> src/responder/nss/nsssrv_cmd.c | 35
++++++++++++++++++----------
> src/responder/pac/pacsrv_utils.c | 14 ++++++++++--
> src/util/sss_nss.c | 43
++++++++++++++++++++---------------
> src/util/sss_nss.h | 12 +++++++---
> 6 files changed, 99 insertions(+), 49 deletions(-)
>
> diff --git a/src/providers/ipa/ipa_s2n_exop.c
b/src/providers/ipa/ipa_s2n_exop.c
> index
59a99061ac3ad6f24a8af50e9c1d574282301a1d..02e87f86b75f4b5603b8be2e11e25c0a9fcdec7c 100644
> --- a/src/providers/ipa/ipa_s2n_exop.c
> +++ b/src/providers/ipa/ipa_s2n_exop.c
> @@ -648,6 +648,7 @@ static void ipa_s2n_get_user_done(struct
tevent_req *subreq)
> struct resp_attrs *simple_attrs = NULL;
> time_t now;
> uint64_t timeout = 10*60*60; /* FIXME: find a better timeout ! */
> + struct sss_nss_homedir_ctx *homedir_ctx;
> const char *homedir = NULL;
> struct sysdb_attrs *user_attrs = NULL;
> struct sysdb_attrs *group_attrs = NULL;
> @@ -738,13 +739,19 @@ static void ipa_s2n_get_user_done(struct
tevent_req *subreq)
> switch (attrs->response_type) {
> case RESP_USER:
> if (state->dom->subdomain_homedir) {
> + homedir_ctx = talloc_zero(state, struct
sss_nss_homedir_ctx);
> + if (! homedir_ctx) {
> + ret = ENOMEM;
> + goto done;
> + }
I think it is now prefered to use if (homedir_ctx == NULL) to highlight
homedir_ctx is a pointer, but I defintly think that the space after ! is
redundant.
> + homedir_ctx->username = attrs->a.user.pw_name;
> + homedir_ctx->uid = attrs->a.user.pw_uid;
> + homedir_ctx->domain = state->dom->name;
> + homedir_ctx->flatname = state->dom->flat_name;
> +
> homedir = expand_homedir_template(state,
>
state->dom->subdomain_homedir,
> -
attrs->a.user.pw_name,
> -
attrs->a.user.pw_uid,
> - NULL,
> - state->dom->name,
> -
state->dom->flat_name);
> + homedir_ctx);
> if (homedir == NULL) {
> ret = ENOMEM;
> goto done;
> diff --git a/src/providers/ipa/ipa_subdomains_id.c
b/src/providers/ipa/ipa_subdomains_id.c
> index
0adf806066ec34b8c42ddf4d0cbd2d23f05caa50..1766e33708cd29da18c3ac4d0b94ccc33583a851 100644
> --- a/src/providers/ipa/ipa_subdomains_id.c
> +++ b/src/providers/ipa/ipa_subdomains_id.c
> @@ -357,10 +357,10 @@ get_subdomain_homedir_of_user(TALLOC_CTX
*mem_ctx, struct sss_domain_info *dom,
> const char **_homedir)
> {
> errno_t ret;
> - char *name;
> - char *lc_name;
> + const char *name;
> const char *homedir;
> TALLOC_CTX *tmp_ctx;
> + struct sss_nss_homedir_ctx *homedir_ctx;
>
> tmp_ctx = talloc_new(mem_ctx);
> if (tmp_ctx == NULL) {
> @@ -368,22 +368,31 @@ get_subdomain_homedir_of_user(TALLOC_CTX
*mem_ctx, struct sss_domain_info *dom,
> goto done;
> }
>
> - ret = sss_parse_name(tmp_ctx, dom->names, fqname, NULL, &name);
> + homedir_ctx = talloc_zero(tmp_ctx, struct sss_nss_homedir_ctx);
> + if (! homedir_ctx) {
> + ret = ENOMEM;
> + goto done;
> + }
same as above
> +
> + homedir_ctx->uid = uid;
> + homedir_ctx->domain = dom->name;
> + homedir_ctx->flatname = dom->flat_name;
> + ret = sss_parse_name_const(tmp_ctx, dom->names, fqname,
> + NULL, &name);
> if (ret != EOK) {
> goto done;
> }
>
> /* To be compatible with the old winbind based user lookups and
IPA
> * clients the user name in the home directory path will be
lower-case. */
> - lc_name = sss_tc_utf8_str_tolower(tmp_ctx, name);
> - if (lc_name == NULL) {
> - ret =ENOMEM;
> + homedir_ctx->username = sss_tc_utf8_str_tolower(tmp_ctx, name);
> + if (homedir_ctx->username == NULL) {
> + ret = ENOMEM;
> goto done;
> }
>
> - homedir = expand_homedir_template(tmp_ctx,
dom->subdomain_homedir, lc_name,
> - uid, NULL, dom->name,
dom->flat_name);
> -
> + homedir = expand_homedir_template(tmp_ctx,
dom->subdomain_homedir,
> + homedir_ctx);
> if (homedir == NULL) {
> DEBUG(SSSDBG_OP_FAILURE, "expand_homedir_template failed\n");
> ret = ENOMEM;
> @@ -373,7 +372,19 @@ static int fill_pwent(struct sss_packet *packet,
> } else {
> to_sized_string(&gecos, tmpstr);
> }
> - tmpstr = get_homedir_override(tmp_ctx, msg, nctx, dom,
name.str, uid);
> +
> + homedir_ctx = talloc_zero(tmp_ctx, struct
sss_nss_homedir_ctx);
> + if (! homedir_ctx) {
> + num = 0;
> + ret = ENOMEM;
> + goto done;;
> + }
same as above and please remove the redundant ;
> +
> + homedir_ctx->username = name.str;
> + homedir_ctx->uid = uid;
> + homedir_ctx->domain = dom->name;
> +
> + tmpstr = get_homedir_override(tmp_ctx, msg, nctx, dom,
homedir_ctx);
> if (!tmpstr) {
> to_sized_string(&homedir, "/");
> } else {
> diff --git a/src/responder/pac/pacsrv_utils.c
b/src/responder/pac/pacsrv_utils.c
> index
035fe84c0f39aaa40a403a8ac5f19527b2191267..4bf5585e8de667850ef520fa372d7e114f2cab1a 100644
> --- a/src/responder/pac/pacsrv_utils.c
> +++ b/src/responder/pac/pacsrv_utils.c
> @@ -353,6 +353,7 @@ errno_t get_pwd_from_pac(TALLOC_CTX *mem_ctx,
> char *upn;
> hash_key_t key;
> hash_value_t value;
> + struct sss_nss_homedir_ctx *homedir_ctx;
>
> pwd = talloc_zero(mem_ctx, struct passwd);
> if (pwd == NULL) {
> @@ -439,9 +440,18 @@ errno_t get_pwd_from_pac(TALLOC_CTX *mem_ctx,
> /* Check if there is a special homedir template for sub-domains.
If not a
> * fallback will be added by the NSS responder. */
> if (IS_SUBDOMAIN(dom) && dom->subdomain_homedir) {
> + homedir_ctx = talloc_zero(pwd, struct
sss_nss_homedir_ctx);
> + if (! homedir_ctx) {
> + ret = ENOMEM;
> + goto done;;
> + }
same as above and please remove the redundant ;
> + homedir_ctx->username = lname;
> + homedir_ctx->uid = pwd->pw_uid;
> + homedir_ctx->domain = dom->name;
> + homedir_ctx->flatname = dom->flat_name;
> +
> pwd->pw_dir = expand_homedir_template(pwd,
dom->subdomain_homedir,
> - lname, pwd->pw_uid,
NULL,
> - dom->name,
dom->flat_name);
> + homedir_ctx);
> if (pwd->pw_dir == NULL) {
> ret = ENOMEM;
> goto done;
> diff --git a/src/util/sss_nss.c b/src/util/sss_nss.c
> index
406c95cd0f76c9e73705bc49b4261160d26bfc58..efc6e3e2c66944c236a5d62df9e573b15a9f5840 100644
> --- a/src/util/sss_nss.c
> +++ b/src/util/sss_nss.c
> @@ -23,9 +23,7 @@
> #include "util/sss_nss.h"
>
> char *expand_homedir_template(TALLOC_CTX *mem_ctx, const char
*template,
> - const char *username, uint32_t uid,
> - const char *original, const char
*domain,
> - const char *flatname)
> + struct sss_nss_homedir_ctx
*homedir_ctx)
> {
> char *copy;
> char *p;
> @@ -40,6 +38,11 @@ char *expand_homedir_template(TALLOC_CTX *mem_ctx,
const char *template,
> return NULL;
> }
>
> + if (! homedir_ctx) {
> + DEBUG(SSSDBG_CRIT_FAILURE, ("Missing home directory
data.\n"));
> + return NULL;
> + }
same as above
patch 3
> diff --git a/src/confdb/confdb.c b/src/confdb/confdb.c
> index
9a13f723de601413205cbb580a92791015b94aa1..cc8b2691262ed9b893bcb693eb1cbb2c05446a60 100644
> --- a/src/confdb/confdb.c
> +++ b/src/confdb/confdb.c
> @@ -1118,6 +1118,16 @@ static int confdb_get_domain_internal(struct
confdb_ctx *cdb,
> }
>
> tmp = ldb_msg_find_attr_as_string(res->msgs[0],
> + CONFDB_NSS_HOMEDIR_SUBSTRING,
NULL);
> + if (tmp != NULL) {
> + domain->homedir_substr = talloc_strdup(domain, tmp);
> + if (!domain->homedir_substr) {
> + ret = ENOMEM;
> + goto done;
> + }
> + }
Please use uniform form of testing to NULL IMO the first one is
prefered.
> diff --git a/src/man/include/homedir_substring.xml
b/src/man/include/homedir_substring.xml
> new file mode 100644
> index
0000000000000000000000000000000000000000..8ac83b7d665cfa8bc9b81bcbea780b40cb64ef62
> --- /dev/null
> +++ b/src/man/include/homedir_substring.xml
> @@ -0,0 +1,16 @@
> +<varlistentry>
> + <term>homedir_substring (string)</term>
> + <listitem>
> + <para>
> + Value of this option will be used in the expansion of
> + <emphasis>override_homedir</emphasis> option if template
contains
> + format string <emphasis>%H</emphasis>. LDAP directory can
directly
> + contain template and this option can be used to expand
home
> + directory for each machine (or Operating system).
> + It can be set in the [nss] section or per-domain.
If this option is set in both [nss] and domain section which value is
then used?
> + </para>
> + <para>
> + Default: /home
> + </para>
> + </listitem>
> +</varlistentry>
Thanks for reading.
Pavel Reichl
More information about the sssd-devel
mailing list