URL: https://github.com/SSSD/sssd/pull/607 Author: fidencio Title: #607: Do not apply override_homedir and override_shell to files provider Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/607/head:pr607 git checkout pr607
From 6f1b1cea4da20a16b1cfea8302d950d03651867c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Tue, 26 Jun 2018 19:40:29 +0200 Subject: [PATCH 1/6] files: do not apply override_homedir to files provider MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit override_homedir should not be applied to files provider as the provider should always return *only* what's in the files and nothing else. Resolves: https://pagure.io/SSSD/sssd/issue/3758 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/confdb/confdb.c | 4 +++- src/responder/nss/nss_protocol_pwent.c | 21 +++++++++++++-------- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/confdb/confdb.c b/src/confdb/confdb.c index 5b4cbec8e..c330586e5 100644 --- a/src/confdb/confdb.c +++ b/src/confdb/confdb.c @@ -1283,7 +1283,9 @@ static int confdb_get_domain_internal(struct confdb_ctx *cdb, tmp = ldb_msg_find_attr_as_string(res->msgs[0], CONFDB_NSS_OVERRIDE_HOMEDIR, NULL); - if (tmp != NULL) { + /* Here we skip the files provider as it should always return *only* + * what's in the files and nothing else. */ + if (tmp != NULL && strcasecmp(domain->provider, "files") != 0) { domain->override_homedir = talloc_strdup(domain, tmp); if (!domain->override_homedir) { ret = ENOMEM; diff --git a/src/responder/nss/nss_protocol_pwent.c b/src/responder/nss/nss_protocol_pwent.c index f449ec69b..af9e74fc8 100644 --- a/src/responder/nss/nss_protocol_pwent.c +++ b/src/responder/nss/nss_protocol_pwent.c @@ -65,15 +65,20 @@ nss_get_homedir_override(TALLOC_CTX *mem_ctx, homedir_ctx->config_homedir_substr = nctx->homedir_substr; } - /* Check whether we are unconditionally overriding the server - * for home directory locations. + /* Here we skip the files provider as it should always return *only* + * what's in the files and nothing else. */ - if (dom->override_homedir) { - return expand_homedir_template(mem_ctx, dom->override_homedir, - dom->case_preserve, homedir_ctx); - } else if (nctx->override_homedir) { - return expand_homedir_template(mem_ctx, nctx->override_homedir, - dom->case_preserve, homedir_ctx); + if (strcasecmp(dom->provider, "files") != 0) { + /* Check whether we are unconditionally overriding the server + * for home directory locations. + */ + if (dom->override_homedir) { + return expand_homedir_template(mem_ctx, dom->override_homedir, + dom->case_preserve, homedir_ctx); + } else if (nctx->override_homedir) { + return expand_homedir_template(mem_ctx, nctx->override_homedir, + dom->case_preserve, homedir_ctx); + } } if (!homedir || *homedir == '\0') { From a193d1b321b3611c2cfa5432e47b1e27fe027015 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Wed, 27 Jun 2018 12:59:06 +0200 Subject: [PATCH 2/6] tests: add override_homedir tests for files provider MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves: http://pagure.io/SSSD/sssd/issue/3758 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/tests/intg/test_files_provider.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/tests/intg/test_files_provider.py b/src/tests/intg/test_files_provider.py index 5c95c68d2..dac41b196 100644 --- a/src/tests/intg/test_files_provider.py +++ b/src/tests/intg/test_files_provider.py @@ -310,6 +310,25 @@ def domain_resolution_order(request): return None +@pytest.fixture +def override_homedir(request): + conf = unindent("""\ + [sssd] + domains = files + services = nss + + [domain/files] + id_provider = files + override_homedir = /test/bar + + [nss] + override_homedir = /test/foo + """).format(**locals()) + create_conf_fixture(request, conf) + create_sssd_fixture(request) + return None + + def setup_pw_with_list(request, user_list): pwd_ops = passwd_ops_setup(request) for user in user_list: @@ -1198,3 +1217,10 @@ def test_files_with_domain_resolution_order(add_user_with_canary, its fully-qualified name. """ check_user(USER1) + + +def test_files_with_override_homedir(add_user_with_canary, + override_homedir): + res, user = sssd_getpwnam_sync(USER1["name"]) + assert res == NssReturnCode.SUCCESS + assert user["dir"] == USER1["dir"] From 5aa1b992e6fa58a6c0b9e2be46cd9c0231eb4b6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Tue, 26 Jun 2018 19:48:39 +0200 Subject: [PATCH 3/6] files: do not apply override_shell to files provider MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit override_shell should not be applied to files provider as the provider should always return *only* what's in the files and nothing else. Resolves: https://pagure.io/SSSD/sssd/issue/3758 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/confdb/confdb.c | 4 +++- src/responder/common/responder_utils.c | 16 ++++++++++------ 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/confdb/confdb.c b/src/confdb/confdb.c index c330586e5..f95a97ec3 100644 --- a/src/confdb/confdb.c +++ b/src/confdb/confdb.c @@ -1326,7 +1326,9 @@ static int confdb_get_domain_internal(struct confdb_ctx *cdb, tmp = ldb_msg_find_attr_as_string(res->msgs[0], CONFDB_NSS_OVERRIDE_SHELL, NULL); - if (tmp != NULL) { + /* Here we skip the files provider as it should always return *only* + * what's in the files and nothing else. */ + if (tmp != NULL && strcasecmp(domain->provider, "files") != 0) { domain->override_shell = talloc_strdup(domain, tmp); if (!domain->override_shell) { ret = ENOMEM; diff --git a/src/responder/common/responder_utils.c b/src/responder/common/responder_utils.c index 521896088..d10a5bb73 100644 --- a/src/responder/common/responder_utils.c +++ b/src/responder/common/responder_utils.c @@ -408,12 +408,16 @@ sss_resp_get_shell_override(struct ldb_message *msg, const char *shell; int i; - /* Check whether we are unconditionally overriding - * the server for the login shell. */ - if (domain->override_shell) { - return domain->override_shell; - } else if (rctx->override_shell) { - return rctx->override_shell; + /* Here we skip the files provider as it should always return *only* + * what's in the files and nothing else. */ + if (strcasecmp(domain->provider, "files") != 0) { + /* Check whether we are unconditionally overriding + * the server for the login shell. */ + if (domain->override_shell) { + return domain->override_shell; + } else if (rctx->override_shell) { + return rctx->override_shell; + } } shell = sss_view_ldb_msg_find_attr_as_string(domain, msg, SYSDB_SHELL, From 524e84cffa5d71c628c4d3bedb8c71cbabd3026d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Wed, 27 Jun 2018 12:59:45 +0200 Subject: [PATCH 4/6] tests: add override_shell tests for files provider MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves: https://pagure.io/SSSD/sssd/issue/3758 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/tests/intg/test_files_provider.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/tests/intg/test_files_provider.py b/src/tests/intg/test_files_provider.py index dac41b196..ab790a700 100644 --- a/src/tests/intg/test_files_provider.py +++ b/src/tests/intg/test_files_provider.py @@ -311,7 +311,7 @@ def domain_resolution_order(request): @pytest.fixture -def override_homedir(request): +def override_homedir_and_shell(request): conf = unindent("""\ [sssd] domains = files @@ -320,9 +320,11 @@ def override_homedir(request): [domain/files] id_provider = files override_homedir = /test/bar + override_shell = /bin/bar [nss] override_homedir = /test/foo + override_shell = /bin/foo """).format(**locals()) create_conf_fixture(request, conf) create_sssd_fixture(request) @@ -1220,7 +1222,14 @@ def test_files_with_domain_resolution_order(add_user_with_canary, def test_files_with_override_homedir(add_user_with_canary, - override_homedir): + override_homedir_and_shell): res, user = sssd_getpwnam_sync(USER1["name"]) assert res == NssReturnCode.SUCCESS assert user["dir"] == USER1["dir"] + + +def test_files_with_override_shell(add_user_with_canary, + override_homedir_and_shell): + res, user = sssd_getpwnam_sync(USER1["name"]) + assert res == NssReturnCode.SUCCESS + assert user["shell"] == USER1["shell"] From a7baf0d1983de3a406ed3b3bcfcdf16d42563459 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Mon, 2 Jul 2018 09:55:49 +0200 Subject: [PATCH 5/6] util: add is_files_provider() helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In a bunch of differente places we end up checking whether the domain's provider is the "files" provider or not. Let's just add some helper function to standardize the checks. Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/util/domain_info_utils.c | 6 ++++++ src/util/util.h | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/src/util/domain_info_utils.c b/src/util/domain_info_utils.c index 9d608ef20..8bef6c9af 100644 --- a/src/util/domain_info_utils.c +++ b/src/util/domain_info_utils.c @@ -928,3 +928,9 @@ bool sss_domain_info_get_output_fqnames(struct sss_domain_info *domain) { return domain->output_fqnames; } + +bool is_files_provider(struct sss_domain_info *domain) +{ + return domain->provider != NULL && + strcasecmp(domain->provider, "files") == 0; +} diff --git a/src/util/util.h b/src/util/util.h index 11220c83c..bc89ecbc2 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -593,6 +593,10 @@ errno_t get_dom_names(TALLOC_CTX *mem_ctx, char ***_dom_names, int *_dom_names_count); +/* Returns true if the provider used for the passed domain is the "files" + * one. Otherwise returns false. */ +bool is_files_provider(struct sss_domain_info *domain); + /* from util_lock.c */ errno_t sss_br_lock_file(int fd, size_t start, size_t len, int num_tries, useconds_t wait); From 8037113acf96d168ea32bba027f65bc2dd44d6e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Mon, 2 Jul 2018 10:01:24 +0200 Subject: [PATCH 6/6] files: make use of is_files_provider() helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/confdb/confdb.c | 8 ++++---- src/responder/common/cache_req/cache_req_domain.c | 2 +- src/responder/common/responder_utils.c | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/confdb/confdb.c b/src/confdb/confdb.c index f95a97ec3..a3eb9c66d 100644 --- a/src/confdb/confdb.c +++ b/src/confdb/confdb.c @@ -998,7 +998,7 @@ static int confdb_get_domain_internal(struct confdb_ctx *cdb, "Interpreting as true\n", domain->name); domain->enumerate = true; } else { /* assume the new format */ - enum_default = strcasecmp(domain->provider, "files") == 0 ? true : false; + enum_default = is_files_provider(domain); ret = get_entry_as_bool(res->msgs[0], &domain->enumerate, CONFDB_DOMAIN_ENUMERATE, enum_default); @@ -1009,7 +1009,7 @@ static int confdb_get_domain_internal(struct confdb_ctx *cdb, } } - if (strcasecmp(domain->provider, "files") == 0) { + if (is_files_provider(domain)) { /* The password field must be reported as 'x', else pam_unix won't * authenticate this entry. See man pwconv(8) for more details. */ @@ -1285,7 +1285,7 @@ static int confdb_get_domain_internal(struct confdb_ctx *cdb, CONFDB_NSS_OVERRIDE_HOMEDIR, NULL); /* Here we skip the files provider as it should always return *only* * what's in the files and nothing else. */ - if (tmp != NULL && strcasecmp(domain->provider, "files") != 0) { + if (tmp != NULL && !is_files_provider(domain)) { domain->override_homedir = talloc_strdup(domain, tmp); if (!domain->override_homedir) { ret = ENOMEM; @@ -1328,7 +1328,7 @@ static int confdb_get_domain_internal(struct confdb_ctx *cdb, CONFDB_NSS_OVERRIDE_SHELL, NULL); /* Here we skip the files provider as it should always return *only* * what's in the files and nothing else. */ - if (tmp != NULL && strcasecmp(domain->provider, "files") != 0) { + if (tmp != NULL && !is_files_provider(domain)) { domain->override_shell = talloc_strdup(domain, tmp); if (!domain->override_shell) { ret = ENOMEM; diff --git a/src/responder/common/cache_req/cache_req_domain.c b/src/responder/common/cache_req/cache_req_domain.c index e7bcd8000..ad86d1252 100644 --- a/src/responder/common/cache_req/cache_req_domain.c +++ b/src/responder/common/cache_req/cache_req_domain.c @@ -207,7 +207,7 @@ cache_req_domain_new_list_from_string_list(TALLOC_CTX *mem_ctx, * NOTE: we do *not* want to use fully qualified names for the * files provider.*/ if (resolution_order != NULL) { - if (strcmp(cr_domain->domain->provider, "files") != 0) { + if (!is_files_provider(cr_domain->domain)) { sss_domain_info_set_output_fqnames(cr_domain->domain, true); } } diff --git a/src/responder/common/responder_utils.c b/src/responder/common/responder_utils.c index d10a5bb73..a63eab421 100644 --- a/src/responder/common/responder_utils.c +++ b/src/responder/common/responder_utils.c @@ -410,7 +410,7 @@ sss_resp_get_shell_override(struct ldb_message *msg, /* Here we skip the files provider as it should always return *only* * what's in the files and nothing else. */ - if (strcasecmp(domain->provider, "files") != 0) { + if (!is_files_provider(domain)) { /* Check whether we are unconditionally overriding * the server for the login shell. */ if (domain->override_shell) {
_______________________________________________ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/sssd-devel@lists.fedorahosted.org/message/UPAQS227CMCIJTQQPDATSNROCEJUWDJ6/
