[SSSD] [sssd PR#246][comment] filter_users and filter_groups stop working properly in v 1.15

2017-04-25 Thread fidencio
  URL: https://github.com/SSSD/sssd/pull/246
Title: #246: filter_users and filter_groups stop working properly in v 1.15

fidencio commented:
"""
New patch set pushed and in the end the changes were not intrusive at all.

@pbrezina, I've partially taken your idea (with some changes).
Please, let me know if you're okay of the way the new plugin function has been 
introduced.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/246#issuecomment-297186170
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org


[SSSD] [sssd PR#246][synchronized] filter_users and filter_groups stop working properly in v 1.15

2017-04-25 Thread fidencio
   URL: https://github.com/SSSD/sssd/pull/246
Author: fidencio
 Title: #246: filter_users and filter_groups stop working properly in v 1.15
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/246/head:pr246
git checkout pr246
From 6a6c9866301ed6bd7b35f5799e6c058ac81eea15 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= 
Date: Mon, 24 Apr 2017 12:11:46 +0200
Subject: [PATCH 1/5] NSS: Use fqnames when performing a ncache check
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The names stored in the negative cache are fully qualified, so we have
to use fully qualified names when checking whether a user/group is part
of negative cache or not.

Related:
https://pagure.io/SSSD/sssd/issue/3362

Signed-off-by: Fabiano Fidêncio 
---
 src/responder/nss/nss_protocol_grent.c | 12 +++-
 src/responder/nss/nss_protocol_pwent.c | 12 +++-
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/src/responder/nss/nss_protocol_grent.c b/src/responder/nss/nss_protocol_grent.c
index fae1d47..6f6ad9e 100644
--- a/src/responder/nss/nss_protocol_grent.c
+++ b/src/responder/nss/nss_protocol_grent.c
@@ -205,6 +205,7 @@ nss_protocol_fill_grent(struct nss_ctx *nss_ctx,
 uint32_t num_results;
 uint32_t num_members;
 char *members;
+char *fqname;
 size_t members_size;
 size_t rp;
 size_t rp_members;
@@ -243,8 +244,17 @@ nss_protocol_fill_grent(struct nss_ctx *nss_ctx,
 
 /* Check negative cache during enumeration. */
 if (cmd_ctx->enumeration) {
+fqname = sss_create_internal_fqname(tmp_ctx, name->str,
+result->domain->name);
+if (fqname == NULL) {
+DEBUG(SSSDBG_OP_FAILURE,
+  "sss_create_internal_fqname() failed\n");
+ret = ENOMEM;
+goto done;
+}
+
 ret = sss_ncache_check_group(nss_ctx->rctx->ncache,
- result->domain, name->str);
+ result->domain, fqname);
 if (ret == EEXIST) {
 DEBUG(SSSDBG_TRACE_FUNC,
   "User [%s] filtered out! (negative cache)\n",
diff --git a/src/responder/nss/nss_protocol_pwent.c b/src/responder/nss/nss_protocol_pwent.c
index edda9d3..e781352 100644
--- a/src/responder/nss/nss_protocol_pwent.c
+++ b/src/responder/nss/nss_protocol_pwent.c
@@ -273,6 +273,7 @@ nss_protocol_fill_pwent(struct nss_ctx *nss_ctx,
 struct sized_string gecos;
 struct sized_string homedir;
 struct sized_string shell;
+char *fqname;
 uint32_t gid;
 uint32_t uid;
 uint32_t num_results;
@@ -311,8 +312,17 @@ nss_protocol_fill_pwent(struct nss_ctx *nss_ctx,
 
 /* Check negative cache during enumeration. */
 if (cmd_ctx->enumeration) {
+fqname = sss_create_internal_fqname(tmp_ctx, name->str,
+result->domain->name);
+if (fqname == NULL) {
+DEBUG(SSSDBG_OP_FAILURE,
+  "sss_create_internal_fqname() failed\n");
+ret = ENOMEM;
+goto done;
+}
+
 ret = sss_ncache_check_user(nss_ctx->rctx->ncache,
-result->domain, name->str);
+result->domain, fqname);
 if (ret == EEXIST) {
 DEBUG(SSSDBG_TRACE_FUNC,
   "User [%s] filtered out! (negative cache)\n", name->str);

From 2b7bb36497e540429255d3d42d00919726a4f5a2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= 
Date: Tue, 25 Apr 2017 16:41:56 +0200
Subject: [PATCH 2/5] RESPONDER: Make nss_get_name_from_msg() part of
 responder_utils
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

In order to do so, the function got renamed to sss_get_name_from_msg()
and will be used as part of cache_req as well in the follow up patches.

Related:
https://pagure.io/SSSD/sssd/issue/3362

Signed-off-by: Fabiano Fidêncio 
---
 src/responder/common/responder.h   |  3 +++
 src/responder/common/responder_utils.c | 26 ++
 src/responder/nss/nss_private.h|  4 
 src/responder/nss/nss_protocol_grent.c |  2 +-
 src/responder/nss/nss_protocol_pwent.c |  2 +-
 src/responder/nss/nss_protocol_sid.c   |  2 +-
 src/responder/nss/nss_utils.c  | 27 ---
 7 files changed, 32 insertions(+), 34 deletions(-)

diff --git a/src/responder/common/responder.h b/src/responder/common/responder.h
index dfe1ec4..e5b9c5b 100644
--- a/src/responder/common/responder.h
+++ b/src/responder/common/responder.h
@@ -391,6 +391,9 @@ char 

[SSSD] [sssd PR#248][edited] IPA: Improve s2n debug message for missing ipaNTSecurityIdentifier

2017-04-25 Thread justin-stephenson
   URL: https://github.com/SSSD/sssd/pull/248
Author: justin-stephenson
 Title: #248: IPA: Improve s2n debug message for missing ipaNTSecurityIdentifier
Action: edited

 Changed field: body
Original value:
"""
This patch is piggyback


This patch improves the log message to be more information for
the SSSD user troubleshooting issues.

If the IDM POSIX group used for AD trust HBAC/SUDO operation is missing
the ipaNTSecurityIdentifier it can cause client s2n operations failures
resolving the group which resulted in the inability to login for the AD
user.
"""

___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org


[SSSD] [sssd PR#248][comment] IPA: Improve s2n debug message for missing ipaNTSecurityIdentifier

2017-04-25 Thread centos-ci
  URL: https://github.com/SSSD/sssd/pull/248
Title: #248: IPA: Improve s2n debug message for missing ipaNTSecurityIdentifier

centos-ci commented:
"""
Can one of the admins verify this patch?
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/248#issuecomment-297100901
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org


[SSSD] [sssd PR#248][comment] IPA: Improve s2n debug message for missing ipaNTSecurityIdentifier

2017-04-25 Thread centos-ci
  URL: https://github.com/SSSD/sssd/pull/248
Title: #248: IPA: Improve s2n debug message for missing ipaNTSecurityIdentifier

centos-ci commented:
"""
Can one of the admins verify this patch?
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/248#issuecomment-297100893
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org


[SSSD] [sssd PR#248][opened] IPA: Improve s2n debug message for missing ipaNTSecurityIdentifier

2017-04-25 Thread justin-stephenson
   URL: https://github.com/SSSD/sssd/pull/248
Author: justin-stephenson
 Title: #248: IPA: Improve s2n debug message for missing ipaNTSecurityIdentifier
Action: opened

PR body:
"""
This patch is piggyback


This patch improves the log message to be more information for
the SSSD user troubleshooting issues.

If the IDM POSIX group used for AD trust HBAC/SUDO operation is missing
the ipaNTSecurityIdentifier it can cause client s2n operations failures
resolving the group which resulted in the inability to login for the AD
user.
"""

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/248/head:pr248
git checkout pr248
From 7fea36a4aa3fb0c1edea54ecd37bcfa99af72a5b Mon Sep 17 00:00:00 2001
From: Justin Stephenson 
Date: Tue, 25 Apr 2017 13:02:10 -0400
Subject: [PATCH] IPA: Improve s2n debug message for missing
 ipaNTSecurityIdentifier

This patch improves the log message to be more information for
the SSSD user troubleshooting issues.

If the IDM POSIX group used for AD trust HBAC/SUDO operation is missing
the ipaNTSecurityIdentifier it can cause client s2n operations failures
resolving the group which resulted in the inability to login for the AD
user.
---
 src/providers/ipa/ipa_s2n_exop.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/providers/ipa/ipa_s2n_exop.c b/src/providers/ipa/ipa_s2n_exop.c
index 55ec904..0608e81 100644
--- a/src/providers/ipa/ipa_s2n_exop.c
+++ b/src/providers/ipa/ipa_s2n_exop.c
@@ -2580,7 +2580,9 @@ static errno_t ipa_s2n_save_objects(struct sss_domain_info *dom,
 ret = sysdb_attrs_get_string(attrs->sysdb_attrs, SYSDB_SID_STR, _str);
 if (ret != EOK) {
 DEBUG(SSSDBG_CRIT_FAILURE,
-  "Cannot find SID of object with override.\n");
+  "Object [%s] has no SID, please check the "
+  "ipaNTSecurityIdentifier attribute on the server-side.\n",
+  attrs->a.name);
 goto done;
 }
 
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org


[SSSD] [sssd PR#136][comment] Tlog integration

2017-04-25 Thread spbnick
  URL: https://github.com/SSSD/sssd/pull/136
Title: #136: Tlog integration

spbnick commented:
"""
BTW, should I perhaps include an update to the sssd.conf man page?
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/136#issuecomment-297017382
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org


[SSSD] [sssd PR#136][comment] Tlog integration

2017-04-25 Thread spbnick
  URL: https://github.com/SSSD/sssd/pull/136
Title: #136: Tlog integration

spbnick commented:
"""
Hi Pavel, thank you for your review! I'll be addressing your comments soon, but 
for now here is how to test this.

The patches add support for a new section in sssd.conf: `session_recording`. 
The section can have up to three options: `scope`, `users`, and `groups`. The 
`scope` option accepts one of three values: `none`, `all`, and `some`. They 
mean "no users will be recorded", "all users will be recorded", and "some 
(specified) users and/or groups will be recorded", respectively.

If `scope` is set to `some`, then the `users` option accepts a 
whitespace-separated list of users to have session recording enabled, same goes 
for `groups` option, but only for groups.

Enabling session recording for a user should result in SSSD reporting user 
shell as `/usr/bin/tlog-rec` through NSS, and exporting `TLOG_REC_SHELL` 
environment variable during PAM session setup. That variable should contain the 
actual user shell.

Testing for those should be sufficient, but if you'd like, you can get tlog 
here: https://github.com/Scribery/tlog

You can either build and install it yourself, or use an RPM from the latest 
release:
https://github.com/Scribery/tlog/releases/tag/v3

You can also take a look at the tests in 
`src/tests/intg/test_session_recording.py` for configuration examples.

Please let me know if you needed some other information.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/136#issuecomment-297016412
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org


[SSSD] [sssd PR#235][comment] Allow using the "shortnames" feature without requiring any configuration from the client side

2017-04-25 Thread fidencio
  URL: https://github.com/SSSD/sssd/pull/235
Title: #235: Allow using the "shortnames" feature without requiring any 
configuration from the client side

fidencio commented:
"""
On Tue, Apr 25, 2017 at 1:56 PM, Pavel Březina 
wrote:

> Yes, I think we should log it so we know what's used when we will debug
> issues :-)
>

Changes done and patchset updated!


> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> , or mute
> the thread
> 
> .
>

"""

See the full comment at 
https://github.com/SSSD/sssd/pull/235#issuecomment-297014695
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org


[SSSD] [sssd PR#235][synchronized] Allow using the "shortnames" feature without requiring any configuration from the client side

2017-04-25 Thread fidencio
   URL: https://github.com/SSSD/sssd/pull/235
Author: fidencio
 Title: #235: Allow using the "shortnames" feature without requiring any 
configuration from the client side
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/235/head:pr235
git checkout pr235
From 694e1ab706e82141487aeb3f08ab53bef399227b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= 
Date: Wed, 12 Apr 2017 10:43:25 +0200
Subject: [PATCH 1/6] RESPONDER: Fallback to global domain resolution order in
 case the view doesn't have this option set
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The current code has been ignoring the domain resolution order set
globally on IPA in case there's a view but this doesn't have any domain
resolution order set.

It happens because we haven't been checking whether the view attribute
didn't exist and then we ended up populating the list cache_req domains'
list assuming that no order has been set instead of falling back to the
next preferred method.

Related:
https://pagure.io/SSSD/sssd/issue/3001

Signed-off-by: Fabiano Fidêncio 
---
 src/responder/common/cache_req/cache_req_domain.c |  14 ++-
 src/responder/common/cache_req/cache_req_domain.h |   5 +-
 src/responder/common/responder_common.c   | 108 +-
 3 files changed, 74 insertions(+), 53 deletions(-)

diff --git a/src/responder/common/cache_req/cache_req_domain.c b/src/responder/common/cache_req/cache_req_domain.c
index bbabd69..86a88ef 100644
--- a/src/responder/common/cache_req/cache_req_domain.c
+++ b/src/responder/common/cache_req/cache_req_domain.c
@@ -120,20 +120,21 @@ cache_req_domain_new_list_from_string_list(TALLOC_CTX *mem_ctx,
 return cr_domains;
 }
 
-struct cache_req_domain *
+errno_t
 cache_req_domain_new_list_from_domain_resolution_order(
 TALLOC_CTX *mem_ctx,
 struct sss_domain_info *domains,
-const char *domain_resolution_order)
+const char *domain_resolution_order,
+struct cache_req_domain **_cr_domains)
 {
 TALLOC_CTX *tmp_ctx;
-struct cache_req_domain *cr_domains = NULL;
+struct cache_req_domain *cr_domains;
 char **list = NULL;
 errno_t ret;
 
 tmp_ctx = talloc_new(NULL);
 if (tmp_ctx == NULL) {
-return NULL;
+return ENOMEM;
 }
 
 if (domain_resolution_order != NULL) {
@@ -160,7 +161,10 @@ cache_req_domain_new_list_from_domain_resolution_order(
 goto done;
 }
 
+*_cr_domains = cr_domains;
+ret = EOK;
+
 done:
 talloc_free(tmp_ctx);
-return cr_domains;
+return ret;
 }
diff --git a/src/responder/common/cache_req/cache_req_domain.h b/src/responder/common/cache_req/cache_req_domain.h
index 41c50e8..87e 100644
--- a/src/responder/common/cache_req/cache_req_domain.h
+++ b/src/responder/common/cache_req/cache_req_domain.h
@@ -34,11 +34,12 @@ struct cache_req_domain *
 cache_req_domain_get_domain_by_name(struct cache_req_domain *domains,
 const char *name);
 
-struct cache_req_domain *
+errno_t
 cache_req_domain_new_list_from_domain_resolution_order(
 TALLOC_CTX *mem_ctx,
 struct sss_domain_info *domains,
-const char *domain_resolution_order);
+const char *domain_resolution_order,
+struct cache_req_domain **_cr_domains);
 
 void cache_req_domain_list_zfree(struct cache_req_domain **cr_domains);
 
diff --git a/src/responder/common/responder_common.c b/src/responder/common/responder_common.c
index ac6320b..62b71b5 100644
--- a/src/responder/common/responder_common.c
+++ b/src/responder/common/responder_common.c
@@ -1486,10 +1486,11 @@ errno_t responder_setup_idle_timeout_config(struct resp_ctx *rctx)
 }
 
 /* == Helper functions for the domain resolution order === */
-static struct cache_req_domain *
+static errno_t
 sss_resp_new_cr_domains_from_ipa_id_view(TALLOC_CTX *mem_ctx,
  struct sss_domain_info *domains,
- struct sysdb_ctx *sysdb)
+ struct sysdb_ctx *sysdb,
+ struct cache_req_domain **_cr_domains)
 {
 TALLOC_CTX *tmp_ctx;
 struct cache_req_domain *cr_domains = NULL;
@@ -1498,7 +1499,7 @@ sss_resp_new_cr_domains_from_ipa_id_view(TALLOC_CTX *mem_ctx,
 
 tmp_ctx = talloc_new(NULL);
 if (tmp_ctx == NULL) {
-return NULL;
+return ENOMEM;
 }
 
 ret = sysdb_get_view_domain_resolution_order(tmp_ctx, sysdb,
@@ -1510,12 

[SSSD] [sssd PR#235][comment] Allow using the "shortnames" feature without requiring any configuration from the client side

2017-04-25 Thread pbrezina
  URL: https://github.com/SSSD/sssd/pull/235
Title: #235: Allow using the "shortnames" feature without requiring any 
configuration from the client side

pbrezina commented:
"""
Yes, I think we should log it so we know what's used when we will debug issues 
:-)
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/235#issuecomment-297006716
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org


[SSSD] [sssd PR#136][comment] Tlog integration

2017-04-25 Thread pbrezina
  URL: https://github.com/SSSD/sssd/pull/136
Title: #136: Tlog integration

pbrezina commented:
"""
Hi, here are some comments. Mostly nitpicks.

Can you also share some link how to enable tlog and test this code?

* **CACHE_REQ: Rename search_domains_done to search_domains_next_done```**
As far as tevent coding style goes `cache_req_search_domains_done` is correct 
since the request name is `cache_req_search_domains`. Name 
`cache_req_search_domains_next_done` is incorrect in this context where it 
serves as the last callback.

The tevent coding style should keep the following form:
```
struct tevent_req *request_name_send(...)
errno_t request_name_$step(...)
void request_name_$stepdone(...)
void request_name_done(...)
errno_t request_name_recv(..)
```
Where `$step` and  `$stepdone` is something that describes this intermediate 
step. `$stepdone` may be simply `$step_done` where it works or any other string 
if it works better. But the last callback name must be `request_name_done`. 
Please, discard this patch.

* **CACHE_REQ: Rename done to search_domains_done**
Here is the same problem now. But in addtion, you are messing up with the order 
of the functions where the last callback called is place before another 
callback. So I propose the following order and name changes:

```
!. cache_req_send
2. cache_req_process_input
3. cache_req_input_parsed
4. cache_req_select_domains
5. cache_req_search_domains
6. cache_req_search_domains_done => cache_req_process_result
7. cache_req_sr_overlay_done => cache_req_done
```
So just change those names and switch order of these two functions. Please, 
discard this patch and do this change in commit where you create 
`cache_req_sr_overlay_done`.

* **DP: Overlay sessionRecording attribute on initgr**
Please remove unneeded parameters instead of silencing the compilator with 
`(void)req_name; (void)reply;`. Also remove unused `reply` from 
`dp_req_initgr_pp_nss_notify`. 

* **CACHE_REQ: Pull sessionRecording attrs from initgr**
See previous tevent comment and also please read [this 
comment](https://github.com/SSSD/sssd/pull/154#issuecomment-284717277) where I 
explained tevent coding style to Fabiano and I would like to obey it in 
`cache_req_sr_overlay.c`. The style is mostly about keeping order and proper 
names of function. This code should read as:
```
cache_req_sr_overlay_send
cache_req_sr_overlay_match_users
cache_req_sr_overlay_match_all
cache_req_sr_overlay_done
cache_req_sr_overlay_recv
```
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/136#issuecomment-297006120
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org


[SSSD] [sssd PR#246][comment] filter_users and filter_groups stop working properly in v 1.15

2017-04-25 Thread lslebodn
  URL: https://github.com/SSSD/sssd/pull/246
Title: #246: filter_users and filter_groups stop working properly in v 1.15

lslebodn commented:
"""
On (25/04/17 03:59), fidencio wrote:
>On Tue, Apr 25, 2017 at 12:16 PM, Pavel Březina 
>wrote:
>
>> Hi, we should solve this on cache_req level so we get the same resultt in
>> nss and ifp (and others) responders. What I had in mind was to create
>> another plugin function that will be called in the and of cache_req
>> process. I.e. something like this:
>>
>> /** * Filter the result through negative cache.  * * This is useful for 
>> plugins that don't use name as an input token but can be affected by 
>> filter_users and filter_groups options.  * * @return EOKIf the object is 
>> not found. * @return EEXIST If the object is found in negative cache. * 
>> @return Other errno code in case of an error. */typedef errno_t
>> (*cache_req_ncache_filter_fn)(struct sss_nc_ctx *ncache,
>>  struct sss_domain_info *domain,
>>  struct ldb_message *msg);
>>
>> In the end, we should iterate over the result and pass each ldb_message to
>> the function (if defined) and remove it from the result if it is present in
>> the negative cache.
>>
>
>I had the same thought Yesterday but for doing this I'd have to expose some
>internals from NSS responder in order to actually have the name and group
>relative to the searched id ... as it is, IMO, too intrusive at this point.
>
>As it's a regression, I really would prefer to keep the patch fixing the
>regression and introduce the new cache_req module later on (without being
>worried on breaking something else right now).
>
-1.

There is no reason to use temporary solution.

LS

"""

See the full comment at 
https://github.com/SSSD/sssd/pull/246#issuecomment-296999474
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org


[SSSD] [sssd PR#246][comment] filter_users and filter_groups stop working properly in v 1.15

2017-04-25 Thread fidencio
  URL: https://github.com/SSSD/sssd/pull/246
Title: #246: filter_users and filter_groups stop working properly in v 1.15

fidencio commented:
"""
On Tue, Apr 25, 2017 at 12:16 PM, Pavel Březina 
wrote:

> Hi, we should solve this on cache_req level so we get the same resultt in
> nss and ifp (and others) responders. What I had in mind was to create
> another plugin function that will be called in the and of cache_req
> process. I.e. something like this:
>
> /** * Filter the result through negative cache.  * * This is useful for 
> plugins that don't use name as an input token but can be affected by 
> filter_users and filter_groups options.  * * @return EOKIf the object is 
> not found. * @return EEXIST If the object is found in negative cache. * 
> @return Other errno code in case of an error. */typedef errno_t
> (*cache_req_ncache_filter_fn)(struct sss_nc_ctx *ncache,
>  struct sss_domain_info *domain,
>  struct ldb_message *msg);
>
> In the end, we should iterate over the result and pass each ldb_message to
> the function (if defined) and remove it from the result if it is present in
> the negative cache.
>

I had the same thought Yesterday but for doing this I'd have to expose some
internals from NSS responder in order to actually have the name and group
relative to the searched id ... as it is, IMO, too intrusive at this point.

As it's a regression, I really would prefer to keep the patch fixing the
regression and introduce the new cache_req module later on (without being
worried on breaking something else right now).

Anyways, it's up to you. Would you prefer to have these changes done *now*?


> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> , or mute
> the thread
> 
> .
>

"""

See the full comment at 
https://github.com/SSSD/sssd/pull/246#issuecomment-296994965
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org


[SSSD] [sssd PR#235][comment] Allow using the "shortnames" feature without requiring any configuration from the client side

2017-04-25 Thread fidencio
  URL: https://github.com/SSSD/sssd/pull/235
Title: #235: Allow using the "shortnames" feature without requiring any 
configuration from the client side

fidencio commented:
"""
On Tue, Apr 25, 2017 at 12:37 PM, Pavel Březina 
wrote:

> Thank you. Last trivial change, you have typo in commit message (domaiN):
> RESPONDER_COMMON: Improve domaiN_resolution_order debug messages
>
> Thank you for the debug messages, it's good to know where it comes from.
> The local resolution order from the configuration file is visible in the
> logs. I don't have currently and IPA setup supporting this prepared -- do
> we print the resolution order from IPA as well?
>

We print ""Using domain_resolution_order from IPA ID View\n"" or "Using
domain_resolution_order from IPA Config\n" and that's it.

I don't specifically print the order being used (and I'm not exactly sure
we should). Do you think we should?


> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> , or mute
> the thread
> 
> .
>

"""

See the full comment at 
https://github.com/SSSD/sssd/pull/235#issuecomment-296994058
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org


[SSSD] [sssd PR#235][comment] Allow using the "shortnames" feature without requiring any configuration from the client side

2017-04-25 Thread pbrezina
  URL: https://github.com/SSSD/sssd/pull/235
Title: #235: Allow using the "shortnames" feature without requiring any 
configuration from the client side

pbrezina commented:
"""
Thank you. Last trivial change, you have typo in commit message (domaiN):
`RESPONDER_COMMON: Improve domaiN_resolution_order debug messages`

Thank you for the debug messages, it's good to know where it comes from. The 
local resolution order from the configuration file is visible in the logs. I 
don't have currently and IPA setup supporting this prepared -- do we print the 
resolution order from IPA as well?
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/235#issuecomment-296990547
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org


[SSSD] [sssd PR#247][comment] Subdomain inherit

2017-04-25 Thread lslebodn
  URL: https://github.com/SSSD/sssd/pull/247
Title: #247: Subdomain inherit

lslebodn commented:
"""
This  patch set modified  the function `check_subdom_config_file` which is 
called from the function `new_subdomain`.  And this function is already tested 
in unitests.

```
sh$ git grep new_subdomain -- src/tests/
src/tests/cmocka/test_fqnames.c:/* just to make new_subdomain happy */
src/tests/cmocka/test_fqnames.c:test_ctx->subdom = new_subdomain(dom, dom, 
SUBDOMNAME, NULL, SUBFLATNAME,
src/tests/cmocka/test_nss_srv.c:#include "db/sysdb_private.h"   /* 
new_subdomain() */
src/tests/cmocka/test_nss_srv.c:subdomain = new_subdomain(nss_test_ctx, 
nss_test_ctx->tctx->dom,
src/tests/cmocka/test_simple_access.c:#include "db/sysdb_private.h"   /* 
new_subdomain() */
src/tests/sysdb-tests.c:subdomain = new_subdomain(test_ctx, 
test_ctx->domain,
src/tests/sysdb-tests.c:subdomain = new_subdomain(test_ctx, 
test_ctx->domain,
src/tests/sysdb-tests.c:subdomain = new_subdomain(test_ctx, 
test_ctx->domain,
src/tests/sysdb-tests.c:subdomain = new_subdomain(test_ctx, 
test_ctx->domain,
```

Please provide unit test. for this change.

"""

See the full comment at 
https://github.com/SSSD/sssd/pull/247#issuecomment-296985832
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org


[SSSD] [sssd PR#246][comment] filter_users and filter_groups stop working properly in v 1.15

2017-04-25 Thread pbrezina
  URL: https://github.com/SSSD/sssd/pull/246
Title: #246: filter_users and filter_groups stop working properly in v 1.15

pbrezina commented:
"""
This is really from top of my mind, there may be another, easier way to do it. 
But we should do it on cache_req level.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/246#issuecomment-296986260
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org


[SSSD] [sssd PR#246][comment] filter_users and filter_groups stop working properly in v 1.15

2017-04-25 Thread pbrezina
  URL: https://github.com/SSSD/sssd/pull/246
Title: #246: filter_users and filter_groups stop working properly in v 1.15

pbrezina commented:
"""
Hi, we should solve this on `cache_req` level so we get the same resultt in 
`nss` and `ifp` (and others) responders. What I had in mind was to create 
another plugin function that will be called in the and of `cache_req` process. 
I.e. something like this:

```c
/**
 * Filter the result through negative cache. 
 *
 * This is useful for plugins that don't use name as an input token but can be 
affected by filter_users and filter_groups options. 
 *
 * @return EOKIf the object is not found.
 * @return EEXIST If the object is found in negative cache.
 * @return Other errno code in case of an error.
 */
typedef errno_t
(*cache_req_ncache_filter_fn)(struct sss_nc_ctx *ncache,
 struct sss_domain_info *domain,
 struct ldb_message *msg);
```
In the end, we should iterate over the result and pass each ldb_message to the 
function (if defined) and remove it from the result if it is present in the 
negative cache.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/246#issuecomment-296985950
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org


[SSSD] [sssd PR#247][comment] Subdomain inherit

2017-04-25 Thread lslebodn
  URL: https://github.com/SSSD/sssd/pull/247
Title: #247: Subdomain inherit

lslebodn commented:
"""
This  patch set modified  the function `check_subdom_config_file` which is 
called from the function `new_subdomain`.  And this function is already tested 
in unitests.

```
sh$ git grep new_subdomain -- src/tests/
src/tests/cmocka/test_fqnames.c:/* just to make new_subdomain happy */
src/tests/cmocka/test_fqnames.c:test_ctx->subdom = new_subdomain(dom, dom, 
SUBDOMNAME, NULL, SUBFLATNAME,
src/tests/cmocka/test_nss_srv.c:#include "db/sysdb_private.h"   /* 
new_subdomain() */
src/tests/cmocka/test_nss_srv.c:subdomain = new_subdomain(nss_test_ctx, 
nss_test_ctx->tctx->dom,
src/tests/cmocka/test_simple_access.c:#include "db/sysdb_private.h"   /* 
new_subdomain() */
src/tests/sysdb-tests.c:subdomain = new_subdomain(test_ctx, 
test_ctx->domain,
src/tests/sysdb-tests.c:subdomain = new_subdomain(test_ctx, 
test_ctx->domain,
src/tests/sysdb-tests.c:subdomain = new_subdomain(test_ctx, 
test_ctx->domain,
src/tests/sysdb-tests.c:subdomain = new_subdomain(test_ctx, 
test_ctx->domain,
```

Please provide unit test. for this change.

"""

See the full comment at 
https://github.com/SSSD/sssd/pull/247#issuecomment-296985832
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org


[SSSD] [sssd PR#247][+Changes requested] Subdomain inherit

2017-04-25 Thread lslebodn
  URL: https://github.com/SSSD/sssd/pull/247
Title: #247: Subdomain inherit

Label: +Changes requested
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org


[SSSD] [sssd PR#247][opened] Subdomain inherit

2017-04-25 Thread mzidek-rh
   URL: https://github.com/SSSD/sssd/pull/247
Author: mzidek-rh
 Title: #247: Subdomain inherit
Action: opened

PR body:
"""
I tested if the options that work in subdomain inherit also work in trusted 
domain section in sssd.conf. Most seem to work without any changes in the code 
except for two. With these two patches only one that does not work remains (I 
wanted to send patchset that adds all the options, but I got stuck on the 
option that sets the ldap principal, so I am sending this in the meantime).
"""

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/247/head:pr247
git checkout pr247
From b4207ed070860509736e152b4788f21b6588c9c6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= 
Date: Fri, 21 Apr 2017 13:11:39 +0200
Subject: [PATCH 1/2] SUBDOMAINS: Configurable ignore_group_members

Allow ignore_group_members in the subdomain section in sssd.conf.

Resolves:
https://pagure.io/SSSD/sssd/issue/3337
---
 src/db/sysdb_subdomains.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/src/db/sysdb_subdomains.c b/src/db/sysdb_subdomains.c
index e2a4f7b..063177d 100644
--- a/src/db/sysdb_subdomains.c
+++ b/src/db/sysdb_subdomains.c
@@ -218,6 +218,22 @@ check_subdom_config_file(struct confdb_ctx *confdb,
   sd_conf_path, CONFDB_DOMAIN_FQ,
   subdomain->fqnames ? "TRUE" : "FALSE");
 
+
+/* ignore_group_members */
+ret = confdb_get_bool(confdb, sd_conf_path,
+  CONFDB_DOMAIN_IGNORE_GROUP_MEMBERS,
+  false, >ignore_group_members);
+if (ret != EOK) {
+DEBUG(SSSDBG_OP_FAILURE,
+  "Failed to get %s option for the subdomain: %s\n",
+  CONFDB_DOMAIN_FQ, subdomain->name);
+goto done;
+}
+
+DEBUG(SSSDBG_CONF_SETTINGS, "%s/%s has value %s\n",
+  sd_conf_path, CONFDB_DOMAIN_IGNORE_GROUP_MEMBERS,
+  subdomain->ignore_group_members ? "TRUE" : "FALSE");
+
 ret = EOK;
 done:
 talloc_free(tmp_ctx);

From 5f2d1616421d6f9c32bb07cd530abf2c4e756293 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= 
Date: Fri, 21 Apr 2017 17:44:41 +0200
Subject: [PATCH 2/2] MAN: Add options for subdomains

Add options supported in subdomain_inherit to the subdomain section
of sssd.conf.

Resolves:
https://pagure.io/SSSD/sssd/issue/3337
---
 src/man/sssd.conf.5.xml | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml
index c712870..a5cdcba 100644
--- a/src/man/sssd.conf.5.xml
+++ b/src/man/sssd.conf.5.xml
@@ -2923,7 +2923,12 @@ ldap_user_extra_attrs = phone:telephoneNumber
 ad_server,
 ad_backup_server,
 ad_site,
-use_fully_qualified_names
+use_fully_qualified_names,
+ignore_group_members,
+ldap_purge_cache_timeout,
+ldap_use_tokengroups,
+ldap_user_principal.
+
 
 For more details about these options see their individual description
 in the manual page.
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org