[sssd PR#241][opened] FleetCommander Integration

[fidencio]

URL: https://github.com/SSSD/sssd/pull/241
Author: fidencio
 Title: #241: FleetCommander Integration
Action: opened

PR body:
"""
This patch series contains:
- some refactoring done on access module (and, consequently, on HBAC) in order to reuse a some code in the session module (an, consequently, on FleetCommander code);
- the new session module that provides the FleetCommander Integration;
"""

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/241/head:pr241
git checkout pr241

[sssd PR#241][synchronized] FleetCommander Integration

[fidencio]

URL: https://github.com/SSSD/sssd/pull/241
Author: fidencio
 Title: #241: FleetCommander Integration
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/241/head:pr241
git checkout pr241

[sssd PR#241][synchronized] FleetCommander Integration

[fidencio]

URL: https://github.com/SSSD/sssd/pull/241
Author: fidencio
 Title: #241: FleetCommander Integration
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/241/head:pr241
git checkout pr241

[sssd PR#241][comment] FleetCommander Integration

[fidencio]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

fidencio commented:
"""
I've updated the patch set accordingly to https://pagure.io/SSSD/sssd/issue/2995#comment-447913
"""

See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-314018566

[sssd PR#241][comment] FleetCommander Integration

[fidencio]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

fidencio commented:
"""
CI: http://sssd-ci.idm.lab.eng.brq.redhat.com:8080/job/ci/7173/

Failures do not  seem to be related to this patch set.
"""

See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-314036112

[sssd PR#241][comment] FleetCommander Integration

[fidencio]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

fidencio commented:
"""
Here [0] you can find some step-by-step on how to test the functionality of those patches.
It's important to mention that @olivergs has been doing some functional tests on those patches, using the copr repo provided by [0].

[0]: https://copr.fedorainfracloud.org/coprs/fidencio/deskprofile/
"""

See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-314434411

[sssd PR#241][synchronized] FleetCommander Integration

[fidencio]

URL: https://github.com/SSSD/sssd/pull/241
Author: fidencio
 Title: #241: FleetCommander Integration
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/241/head:pr241
git checkout pr241

[sssd PR#241][comment] FleetCommander Integration

[fidencio]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

fidencio commented:
"""
I've updated the patch set and the changes are:
- as @jhrozek agreed on the approach proposed by FleetCommander guys, the 2 "fixup" patches got squashed into "DESKPROFILE: Introduce the new IPA session provider";
- updated the link to thefor design page;
"""

See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-314467404

[sssd PR#241][synchronized] FleetCommander Integration

[fidencio]

URL: https://github.com/SSSD/sssd/pull/241
Author: fidencio
 Title: #241: FleetCommander Integration
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/241/head:pr241
git checkout pr241

[sssd PR#241][comment] FleetCommander Integration

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

jhrozek commented:
"""
There are some issues Coverity found:
```
Error: TOCTOU (CWE-367):
sssd-1.15.4/src/providers/ipa/ipa_deskprofile_rules_util.c:157: fs_check_call: Calling function "stat" to perform check on "domain_dir".
sssd-1.15.4/src/providers/ipa/ipa_deskprofile_rules_util.c:159: toctou: Calling function "mkdir" that uses "domain_dir" after a check function. This can cause a time-of-check, time-of-use race condition.
#  157|       ret = stat(domain_dir, &info);
#  158|       if (ret != EOK) {
#  159|->         ret = mkdir(domain_dir, 0755);
#  160|           if (ret != EOK) {
#  161|               ret = errno;

Error: TOCTOU (CWE-367):
sssd-1.15.4/src/providers/ipa/ipa_deskprofile_rules_util.c:214: fs_check_call: Calling function "stat" to perform check on "user_dir".
sssd-1.15.4/src/providers/ipa/ipa_deskprofile_rules_util.c:216: toctou: Calling function "mkdir" that uses "user_dir" after a check function. This can cause a time-of-check, time-of-use race condition.
#  214|       ret = stat(user_dir, &info);
#  215|       if (ret != EOK) {
#  216|->         ret = mkdir(user_dir, 0600);
#  217|           if (ret != EOK) {
#  218|               ret = errno;

Error: TOCTOU (CWE-367):
sssd-1.15.4/src/providers/ipa/ipa_deskprofile_rules_util.c:227: fs_check_call: Calling function "stat" to perform check on "user_dir".
sssd-1.15.4/src/providers/ipa/ipa_deskprofile_rules_util.c:247: toctou: Calling function "chown" that uses "user_dir" after a check function. This can cause a time-of-check, time-of-use race condition.
#  245|       }
#  246|   
#  247|->     ret = chown(user_dir, uid, gid);
#  248|       if (ret != EOK) {
#  249|           ret = errno;

Error: TOCTOU (CWE-367):
sssd-1.15.4/src/providers/ipa/ipa_deskprofile_rules_util.c:687: fs_check_call: Calling function "stat" to perform check on "filepath".
sssd-1.15.4/src/providers/ipa/ipa_deskprofile_rules_util.c:702: toctou: Calling function "remove" that uses "filepath" after a check function. This can cause a time-of-check, time-of-use race condition.
#  700|           }
#  701|   
#  702|->         ret = remove(filepath);
#  703|           if (ret != EOK) {
#  704|               ret = errno;

Error: RESOURCE_LEAK (CWE-772):
sssd-1.15.4/src/providers/ipa/ipa_deskprofile_rules_util.c:659: alloc_fn: Storage is returned from allocation function "opendir".
sssd-1.15.4/src/providers/ipa/ipa_deskprofile_rules_util.c:659: var_assign: Assigning: "dir" = storage returned from "opendir(user_dir)".
sssd-1.15.4/src/providers/ipa/ipa_deskprofile_rules_util.c:680: noescape: Resource "dir" is not freed or pointed-to in "readdir".
sssd-1.15.4/src/providers/ipa/ipa_deskprofile_rules_util.c:725: leaked_storage: Variable "dir" going out of scope leaks the storage it points to.
#  723|   done:
#  724|       talloc_free(tmp_ctx);
#  725|->     return ret;
#  726|   }
#  727|

Error: UNINIT (CWE-457):
sssd-1.15.4/src/responder/common/cache_req/cache_req_sr_overlay.c:122: var_decl: Declaring variable "ret" without initializer.
sssd-1.15.4/src/responder/common/cache_req/cache_req_sr_overlay.c:209: uninit_use: Using uninitialized value "ret".
#  207|   done:
#  208|       talloc_zfree(tmp_ctx);
#  209|->     return ret;
#  210|   }
#  211|   
```
"""

See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-319315814

[sssd PR#241][comment] FleetCommander Integration

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

jhrozek commented:
"""
In general, to get rid of the TOCTOU issues, the key is using the `*at` and `*fd` family of functions - you might open the top level directory to get a fd and then instead of `stat` call `fstat`. You can see how we deal with that in `src/tools/files.c`.
"""

See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-319316366

[sssd PR#241][comment] FleetCommander Integration

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

jhrozek commented:
"""
With an old server that has no support for the desktop profiles, I'm getting:
```
(Tue Aug  1 10:19:17 2017) [sssd[be[ipa.test]]] [ipa_deskprofile_get_config_done] (0x0040): Unexpected number of results, expected 1, got 0.
(Tue Aug  1 10:19:17 2017) [sssd[be[ipa.test]]] [ipa_pam_session_handler_done] (0x0020): Unable to fetch Desktop Profile rules [22]: Invalid argument
```
"""

See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-319330842

[sssd PR#241][comment] FleetCommander Integration

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

jhrozek commented:
"""
sorry, I hit comment too soon. I think we want to handle that more gracefully and not throw an error.

By the way, I'm still not finished with the review of the last patch (the previous ones LGTM), but because I'm running out of time today, I'm already adding the "Changes Requested" keyword. I will continue my review in the evening.
"""

See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-319331120

[sssd PR#241][+Changes requested] FleetCommander Integration

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

Label: +Changes requested

[sssd PR#241][comment] FleetCommander Integration

[fidencio]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

fidencio commented:
"""
@jhrozek, about the issue when connecting to old server that has no support for the desktop profiles ...

In that case we'd always get a reply could as 0. Would be okay, for you, to just return ENOENT in those cases? ipa_pam_session_handler_done() handles well ENOENT :-)

AI: Still have to fix the TUCTOU issues.
"""

See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-319394677

[sssd PR#241][comment] FleetCommander Integration

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

jhrozek commented:
"""
I think the error condition is not really as much of a code issue as a user-friendliness issue. Admins of older IPA servers would suddendly see an odd message saying that something failed. I think returning ENOENT or a special code is both fine as long as there is just an informational message that the server doesn't support desktop profiles.
"""

See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-319449557

[sssd PR#241][synchronized] FleetCommander Integration

[fidencio]

URL: https://github.com/SSSD/sssd/pull/241
Author: fidencio
 Title: #241: FleetCommander Integration
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/241/head:pr241
git checkout pr241

[sssd PR#241][comment] FleetCommander Integration

[fidencio]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

fidencio commented:
"""
@jhrozek, a new version of the patchset has been pushed and hopefully all the comments have been addressed.

A few more patches were introduced in this new patchset.
"""

See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-320668549

[sssd PR#241][-Changes requested] FleetCommander Integration

[fidencio]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

Label: -Changes requested

[sssd PR#241][comment] FleetCommander Integration

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

jhrozek commented:
"""
There are still some Coverity warnings:
```
Error: NEGATIVE_RETURNS (CWE-394):
sssd-1.15.4/src/providers/ipa/ipa_deskprofile_rules_util.c:232: negative_return_fn: Function "sss_create_dir("/var/lib/sss/deskprofile", domain, 493U, getuid(), getgid())" returns a negative number.
sssd-1.15.4/src/util/files.c:843:5: var_tested_neg: Variable "ret" is negative.
sssd-1.15.4/src/util/files.c:878:5: return_negative_variable: Explicitly returning negative variable "ret".
sssd-1.15.4/src/providers/ipa/ipa_deskprofile_rules_util.c:232: var_assign: Assigning: signed variable "ret" = "sss_create_dir".
sssd-1.15.4/src/providers/ipa/ipa_deskprofile_rules_util.c:235: negative_returns: "ret" is passed to a parameter that cannot be negative.
sssd-1.15.4/src/util/util_errors.c:128:5: neg_sink_parm_call: Passing "error" to "strerror", which cannot accept a negative number.
#  233|                            getuid(), getgid());
#  234|       if (ret != EOK) {
#  235|->         DEBUG(SSSDBG_TRACE_FUNC,
#  236|                 "Failed to create the directory "%s/%s" that would be used to "
#  237|                 "store the Desktop Profile rules users' directory [%d]: %s\n",

Error: NEGATIVE_RETURNS (CWE-394):
sssd-1.15.4/src/providers/ipa/ipa_deskprofile_rules_util.c:250: negative_return_fn: Function "sss_create_dir(domain_dir, shortname, 384U, uid, gid)" returns a negative number.
sssd-1.15.4/src/util/files.c:843:5: var_tested_neg: Variable "ret" is negative.
sssd-1.15.4/src/util/files.c:878:5: return_negative_variable: Explicitly returning negative variable "ret".
sssd-1.15.4/src/providers/ipa/ipa_deskprofile_rules_util.c:250: var_assign: Assigning: signed variable "ret" = "sss_create_dir".
sssd-1.15.4/src/providers/ipa/ipa_deskprofile_rules_util.c:252: negative_returns: "ret" is passed to a parameter that cannot be negative.
sssd-1.15.4/src/util/util_errors.c:128:5: neg_sink_parm_call: Passing "error" to "strerror", which cannot accept a negative number.
#  250|       ret = sss_create_dir(domain_dir, shortname, 0600, uid, gid);
#  251|       if (ret != EOK) {
#  252|->         DEBUG(SSSDBG_TRACE_FUNC,
#  253|                  "Failed to create the directory "%s/%s/%s" that would be used "
#  254|                  "to store the Desktop Profile rules for the user "%s" [%d]: "
```
"""

See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-322232061

[sssd PR#241][comment] FleetCommander Integration

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

jhrozek commented:
"""
I think to fix the warnings, you just need to `errno=ret` before printing the DEBUG message
"""

See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-322233270

[sssd PR#241][comment] FleetCommander Integration

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

jhrozek commented:
"""
retest this please
"""

See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-322233302

[sssd PR#241][comment] FleetCommander Integration

[fidencio]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

fidencio commented:
"""
@jhrozek, about the coverity warnings, I guess the safest path to take is just use int instead of errno_t in that function where the warning happend (and, of course, change the whole chain ...)

It's still not fixed locally.
"""

See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-322265238

[sssd PR#241][comment] FleetCommander Integration

[fidencio]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

fidencio commented:
"""
Hmm. That's actually a bad idea and I'm struggling to see a better solution for this. Suggestions are welcome.

"""

See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-322272067

[sssd PR#241][comment] FleetCommander Integration

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

jhrozek commented:
"""
Well, I'm struggling to see how would `sss_create_dir` return -1. All control paths either return a directly assigned return value (ENOMEM) or errno. 
"""

See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-322288785

[sssd PR#241][comment] FleetCommander Integration

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

jhrozek commented:
"""
Since most of the comments were minor issues with little impact on functionality, I went ahead and scheduled regression runs.
"""

See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-322440673

[sssd PR#241][+Changes requested] FleetCommander Integration

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

Label: +Changes requested

[sssd PR#241][comment] FleetCommander Integration

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

jhrozek commented:
"""
btw the downstream regression test passed (job ID 2005455)
"""

See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-322566071

[sssd PR#241][synchronized] FleetCommander Integration

[fidencio]

URL: https://github.com/SSSD/sssd/pull/241
Author: fidencio
 Title: #241: FleetCommander Integration
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/241/head:pr241
git checkout pr241

[sssd PR#241][comment] FleetCommander Integration

[fidencio]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

fidencio commented:
"""
I have updated the patch set and, hopefully, all your comments have been addressed.

For a few comments I ended up taking the path I believe is the right one. Let me know whether you need some changes and those will be done according to your answers for the questions I've raised as answers for your comments.

This version of the patches includes support for https://pagure.io/SSSD/sssd/issue/3449 and https://github.com/abbra/freeipa-desktop-profile/issues/3.
"""

See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-323014987

[sssd PR#241][-Changes requested] FleetCommander Integration

[fidencio]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

Label: -Changes requested

[sssd PR#241][comment] FleetCommander Integration

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

jhrozek commented:
"""
Thank you for the updates. I think only the `permuts` dereference should be changed to happen after the check.

In the meantime, I've started CI, Coverity and downstream HBAC tests.
"""

See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-323043057

[sssd PR#241][+Changes requested] FleetCommander Integration

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

Label: +Changes requested

[sssd PR#241][comment] FleetCommander Integration

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

jhrozek commented:
"""
On Thu, Aug 17, 2017 at 04:25:07AM -0700, Jakub Hrozek wrote:
> Thank you for the updates. I think only the `permuts` dereference should be changed to happen after the check.
>
> In the meantime, I've started CI, Coverity and downstream HBAC tests.

All passed by the way. So once the one-liner with the array index check
is addressed, I'll be happy about the patches.

"""

See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-323077931

[sssd PR#241][synchronized] FleetCommander Integration

[fidencio]

URL: https://github.com/SSSD/sssd/pull/241
Author: fidencio
 Title: #241: FleetCommander Integration
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/241/head:pr241
git checkout pr241

[sssd PR#241][comment] FleetCommander Integration

[fidencio]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

fidencio commented:
"""
Patch set updated.

This is the patch that I've squashed:
```
From 97f189d3a4fcbbfe453cc6ba8eab8710ca31c4ac Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= 
Date: Thu, 17 Aug 2017 13:29:24 +0200
Subject: [PATCH] fixup! DESKPROFILE: Introduce the new IPA session provider

---
 src/providers/ipa/ipa_deskprofile_rules_util.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/providers/ipa/ipa_deskprofile_rules_util.c b/src/providers/ipa/ipa_deskprofile_rules_util.c
index 12c6492ba..2ece5a1ee 100644
--- a/src/providers/ipa/ipa_deskprofile_rules_util.c
+++ b/src/providers/ipa/ipa_deskprofile_rules_util.c
@@ -150,7 +150,7 @@ ipa_deskprofile_get_filename_path(TALLOC_CTX *mem_ctx,
         extension,
         NULL,
     };
-    const uint8_t *perms = permuts[config_priority - 1];
+    const uint8_t *perms;
     char *result;
     errno_t ret;

@@ -167,6 +167,8 @@ ipa_deskprofile_get_filename_path(TALLOC_CTX *mem_ctx,
         goto done;
     }

+    perms = permuts[config_priority - 1];
+
     result = talloc_strdup(tmp_ctx, "");
     if (result == NULL) {
         ret = ENOMEM;

[sssd PR#241][comment] FleetCommander Integration

[fidencio]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

fidencio commented:
"""
Patch set updated.

This is the patch that I've squashed:
```
From 97f189d3a4fcbbfe453cc6ba8eab8710ca31c4ac Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= 
Date: Thu, 17 Aug 2017 13:29:24 +0200
Subject: [PATCH] fixup! DESKPROFILE: Introduce the new IPA session provider

---
 src/providers/ipa/ipa_deskprofile_rules_util.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/providers/ipa/ipa_deskprofile_rules_util.c b/src/providers/ipa/ipa_deskprofile_rules_util.c
index 12c6492ba..2ece5a1ee 100644
--- a/src/providers/ipa/ipa_deskprofile_rules_util.c
+++ b/src/providers/ipa/ipa_deskprofile_rules_util.c
@@ -150,7 +150,7 @@ ipa_deskprofile_get_filename_path(TALLOC_CTX *mem_ctx,
         extension,
         NULL,
     };
-    const uint8_t *perms = permuts[config_priority - 1];
+    const uint8_t *perms;
     char *result;
     errno_t ret;

@@ -167,6 +167,8 @@ ipa_deskprofile_get_filename_path(TALLOC_CTX *mem_ctx,
         goto done;
     }

+    perms = permuts[config_priority - 1];
+
     result = talloc_strdup(tmp_ctx, "");
     if (result == NULL) {
         ret = ENOMEM;

[sssd PR#241][synchronized] FleetCommander Integration

[fidencio]

URL: https://github.com/SSSD/sssd/pull/241
Author: fidencio
 Title: #241: FleetCommander Integration
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/241/head:pr241
git checkout pr241

[sssd PR#241][-Changes requested] FleetCommander Integration

[fidencio]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

Label: -Changes requested

[sssd PR#241][comment] FleetCommander Integration

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

jhrozek commented:
"""
Thank you, code-wise ACK. I'm not going to add the accepted label so that the patches don't get pushed by accident until we hear from the desktop group about results of their testing.
Thank you for accomodating all my comments, looking forward to merging the patches!
"""

See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-323112506

[sssd PR#241][synchronized] FleetCommander Integration

[fidencio]

URL: https://github.com/SSSD/sssd/pull/241
Author: fidencio
 Title: #241: FleetCommander Integration
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/241/head:pr241
git checkout pr241

[sssd PR#241][comment] FleetCommander Integration

[fidencio]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

fidencio commented:
"""
@jhrozek,

I've updated the patch set including **DESKPROFILE: Add ipa_deskprofile_request_interval**, which fix for https://pagure.io/SSSD/sssd/issue/3482
"""

See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-323209189

[sssd PR#241][comment] FleetCommander Integration

[fidencio]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

fidencio commented:
"""
@jhrozek,

I've updated the patch set including **DESKPROFILE: Add ipa_deskprofile_request_interval**, which is a fix for https://pagure.io/SSSD/sssd/issue/3482
"""

See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-323209189

[sssd PR#241][synchronized] FleetCommander Integration

[fidencio]

URL: https://github.com/SSSD/sssd/pull/241
Author: fidencio
 Title: #241: FleetCommander Integration
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/241/head:pr241
git checkout pr241

[sssd PR#241][comment] FleetCommander Integration

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

pbrezina commented:
"""
I just briefly read the code, haven't tried it.

```c
@@ -126,10 +126,9 @@ static void choose_target(struct data_provider *provider,
             name = "PAM Chpass 2nd";
             break;
         case SSS_PAM_OPEN_SESSION:
-            target = DP_TARGET_SENTINEL;
-            method = DP_METHOD_SENTINEL;
+            target = DPT_SESSION;
+            method = DPM_SESSION_HANDLER;
             name = "PAM Open Session";
-            pd->pam_status = PAM_SUCCESS;
             break;
...
    /* Check that target is configured. */
    if (target != DP_TARGET_SENTINEL
            && !dp_target_enabled(provider, NULL, target)) {
        target = DP_TARGET_SENTINEL;
        method = DP_METHOD_SENTINEL;
        pd->pam_status = PAM_MODULE_UNKNOWN;
    }
```

What happens here if session provider is not set? With this patch we return `PAM_MODULE_UNKNOWN` but it was `PAM_SUCCESS` before. Doesn't this cause any kind of problem?
"""

See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-323305584

[sssd PR#241][comment] FleetCommander Integration

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

jhrozek commented:
"""
On Fri, Aug 18, 2017 at 09:26:14AM +0000, Pavel Březina wrote:
> I just briefly read the code, haven't tried it.
> @@ -126,10 +126,9 @@ static void choose_target(struct data_provider *provider,
>              name = "PAM Chpass 2nd";
>              break;
>          case SSS_PAM_OPEN_SESSION:
> -            target = DP_TARGET_SENTINEL;
> -            method = DP_METHOD_SENTINEL;
> +            target = DPT_SESSION;
> +            method = DPM_SESSION_HANDLER;
>              name = "PAM Open Session";
> -            pd->pam_status = PAM_SUCCESS;
>              break;
> ...
>     /* Check that target is configured. */
>     if (target != DP_TARGET_SENTINEL
>             && !dp_target_enabled(provider, NULL, target)) {
>         target = DP_TARGET_SENTINEL;
>         method = DP_METHOD_SENTINEL;
>         pd->pam_status = PAM_MODULE_UNKNOWN;
>     }
>
> What happens here if session provider is not set? With this patch we return `PAM_MODULE_UNKNOWN` but it was `PAM_SUCCESS` before. Doesn't this cause any kind of problem?

I don't follow. The IPA session provider is enabled by default (all
providers except access are derived from the id_provider).

Setting session_provider = none gives me:
```
(Fri Aug 18 10:21:17 2017) [sssd[be[ipa.test]]] [dp_req_reply_gen_error]
(0x0080): DP Request [PAM Open Session #13]: Finished. Target is not
supported with this configuration.
pam_dp_send_req returned 0
(Fri Aug 18 10:21:17 2017) [sssd[pam]] [pam_dp_process_reply] (0x0010):
Reply error.
(Fri Aug 18 10:21:17 2017) [sssd[pam]] [filter_responses] (0x0100):
[pam_response_filter] not available, not fatal.
```

Which, except the reply error is also expected, right?

"""

See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-323317998

[sssd PR#241][comment] FleetCommander Integration

[fidencio]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

fidencio commented:
"""
@jhrozek: @pbrezina told me (on #sssd) to test with `ipa_enable_deskprofile = false`.

Here's the result:
```
(Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [dp_pam_handler] (0x0100): Got request with the following data
(Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): command: SSS_PAM_OPEN_SESSION
(Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): domain: ipa.example
(Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): user: [e-mail]
(Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): service: su-l
(Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): tty: pts/0
(Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): ruser: root
(Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): rhost:
(Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): authtok type: 0
(Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): newauthtok type: 0
(Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): priv: 1
(Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): cli_pid: 2324
(Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): logon name: not set
(Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [dp_attach_req] (0x0400): DP Request [PAM Open Session #7]: New request. Flags [0000].
(Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [dp_attach_req] (0x0400): Number of active DP request: 1
(Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [sss_domain_get_state] (0x1000): Domain ipa.example is Active
(Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [dp_find_method] (0x0020): Bug: Invalid combination of target [session] and method [9]
(Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [_dp_req_recv] (0x0400): DP Request [PAM Open Session #7]: Receiving request data.
(Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [dp_req_destructor] (0x0400): DP Request [PAM Open Session #7]: Request removed.
(Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [dp_req_destructor] (0x0400): Number of active DP request: 0
(Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [dp_req_reply_gen_error] (0x0020): DP Request [PAM Open Session #7]: Finished. Error [1432158209]: Internal Error
(Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [sbus_dispatch] (0x4000): dbus conn: 0x560901ec30b0
(Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [sbus_dispatch] (0x4000): Dispatching.
(Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [sbus_message_handler] (0x2000): Received SBUS method org.freedesktop.sssd.dataprovider.pamHandler on path /org/freedesktop/sssd/dataprovider
(Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [sbus_get_sender_id_send] (0x2000): Not a sysbus message, quit
(Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [dp_pam_handler] (0x0100): Got request with the following data
(Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): command: SSS_PAM_CLOSE_SESSION
(Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): domain: ipa.example
(Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): user: [e-mail]
(Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): service: su-l
(Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): tty: pts/0
(Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): ruser: root
(Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): rhost:
(Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): authtok type: 0
(Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): newauthtok type: 0
(Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): priv: 1
(Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): cli_pid: 2324
(Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): logon name: not set
(Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [dp_pam_reply] (0x1000): DP Request [PAM Close Session]: Sending result [0][ipa.example]
```

Seems that we throw an internal error there. I'll fix it and update the patch to be squashed so @pbrezina can review.
"""

See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-323318997

[sssd PR#241][comment] FleetCommander Integration

[fidencio]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

fidencio commented:
"""
@pbrezina: So, here's the patch that solves the issue:
```
From ae60eae181c7a3214d76b3ff00d9e431f060bbc2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= 
Date: Fri, 18 Aug 2017 19:46:20 +0200
Subject: [PATCH] fixup! DESKPROFILE: Introduce the new IPA session provider

---
 src/providers/ipa/ipa_init.c    |  9 ---------
 src/providers/ipa/ipa_session.c | 10 ++++++++++
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/src/providers/ipa/ipa_init.c b/src/providers/ipa/ipa_init.c
index b3daa4921..7cae43c06 100644
--- a/src/providers/ipa/ipa_init.c
+++ b/src/providers/ipa/ipa_init.c
@@ -950,20 +950,11 @@ errno_t sssm_ipa_session_init(TALLOC_CTX *mem_ctx,
     struct ipa_session_ctx *session_ctx;
     struct ipa_init_ctx *init_ctx;
     struct ipa_id_ctx *id_ctx;
-    bool enabled;
     errno_t ret;

init_ctx = talloc_get_type(module_data, struct ipa_init_ctx);
     id_ctx = init_ctx->id_ctx;

-    enabled = dp_opt_get_bool(id_ctx->ipa_options->basic,
-                              IPA_ENABLE_DESKPROFILE);
-    if (!enabled) {
-        DEBUG(SSSDBG_TRACE_FUNC, "ipa_enable_deskprofile is set to FALSE\n");
-
-        return EOK;
-    }
-
     session_ctx = talloc_zero(mem_ctx, struct ipa_session_ctx);
     if (session_ctx == NULL) {
         DEBUG(SSSDBG_CRIT_FAILURE, "talloc_zero() failed.\n");
diff --git a/src/providers/ipa/ipa_session.c b/src/providers/ipa/ipa_session.c
index 8559284c9..800ea665a 100644
--- a/src/providers/ipa/ipa_session.c
+++ b/src/providers/ipa/ipa_session.c
@@ -485,6 +485,7 @@ ipa_pam_session_handler_send(TALLOC_CTX *mem_ctx,
     struct tevent_req *req;
     struct tevent_req *subreq;
     struct ipa_pam_session_handler_state *state;
+    bool enabled;
     errno_t ret;

DEBUG(SSSDBG_TRACE_FUNC, "Retrieving Desktop Profile rules\n");
@@ -500,6 +501,15 @@ ipa_pam_session_handler_send(TALLOC_CTX *mem_ctx,
     state->be_ctx = params->be_ctx;
     state->session_ctx = session_ctx;

+    enabled = dp_opt_get_bool(session_ctx->ipa_options,
+                              IPA_ENABLE_DESKPROFILE);
+    if (!enabled) {
+        ret = EOK;
+        DEBUG(SSSDBG_TRACE_FUNC, "ipa_enable_deskprofile is set to FALSE\n");
+        state->pd->pam_status = PAM_SUCCESS;
+        goto done;
+    }
+
     /* Get all the user info that will be needed in order the delete the
      * user's deskprofile directory from the disk, create the user's directory,
      * save the fetched rules to the disk and notify the deskprofile client

[sssd PR#241][synchronized] FleetCommander Integration

[fidencio]

URL: https://github.com/SSSD/sssd/pull/241
Author: fidencio
 Title: #241: FleetCommander Integration
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/241/head:pr241
git checkout pr241

[sssd PR#241][comment] FleetCommander Integration

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

pbrezina commented:
"""
I think it is better to set the method only when needed. Try this diff instead of your patch:

```c
--- a/src/providers/data_provider/dp_target_auth.c
+++ b/src/providers/data_provider/dp_target_auth.c
@@ -126,9 +126,16 @@ static void choose_target(struct data_provider *provider,
             name = "PAM Chpass 2nd";
             break;
         case SSS_PAM_OPEN_SESSION:
-            target = DPT_SESSION;
-            method = DPM_SESSION_HANDLER;
             name = "PAM Open Session";
+            if (dp_method_enabled(provider, DPT_SESSION, DPM_SESSION_HANDLER)) {
+                target = DPT_SESSION;
+                method = DPM_SESSION_HANDLER;
+                break;
+            }
+
+            target = DP_TARGET_SENTINEL;
+            method = DP_METHOD_SENTINEL;
+            pd->pam_status = PAM_SUCCESS;
             break;
         case SSS_PAM_SETCRED:
             target = DP_TARGET_SENTINEL;

```

Also, as I read through the patches, I don't see `session_provider` documented anywhere and since `ipa_deskprofile_enable ` defaults to `true` it is basically an equivalent to `session_provider = ipa | not  set`. Wouldn't it be better to remove this option and document `session_provider` instead as we do with other providers (e.g. hostid)?
"""

See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-323710521

[sssd PR#241][comment] FleetCommander Integration

[fidencio]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

fidencio commented:
"""
On Mon, Aug 21, 2017 at 12:39 PM, Pavel Březina 
wrote:
> I think it is better to set the method only when needed. Try this diff
> instead of your patch:
>
> --- a/src/providers/data_provider/dp_target_auth.c
> +++ b/src/providers/data_provider/dp_target_auth.c
> @@ -126,9 +126,16 @@ static void choose_target(struct data_provider *provider,
>              name = "PAM Chpass 2nd";
>              break;
>          case SSS_PAM_OPEN_SESSION:
>        target = DPT_SESSION;
>        method = DPM_SESSION_HANDLER;
>        name = "PAM Open Session";
>        if (dp_method_enabled(provider, DPT_SESSION, DPM_SESSION_HANDLER)) {
>            target = DPT_SESSION;
>            method = DPM_SESSION_HANDLER;
>            break;
>        }
>        target = DP_TARGET_SENTINEL;
>        method = DP_METHOD_SENTINEL;
>        pd->pam_status = PAM_SUCCESS;
>        break;
>    case SSS_PAM_SETCRED:
>        target = DP_TARGET_SENTINEL;
>
> Also, as I read through the patches, I don't see session_provider
> documented anywhere and since ipa_deskprofile_enable defaults to true it
> is basically an equivalent to session_provider = ipa | not set. Wouldn't
> it be better to remove this option and document session_provider instead
> as we do with other providers (e.g. hostid)?

I like the idea to have only the session_provider option documented/exposed.
@jhrozek, would that be okay for you? Then we can drop the
ipa_enable_deskprofile option entirely.
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> https://github.com/SSSD/sssd/pull/241#issuecomment-323710521, or mute
> the thread
> https://github.com/notifications/unsubscribe-auth/AAG4etDXxe7hxYhC8DpwTR2TzZ2gOM-Oks5saV51gaJpZM4NDNM2
> .

"""

See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-323711365

[sssd PR#241][synchronized] FleetCommander Integration

[fidencio]

URL: https://github.com/SSSD/sssd/pull/241
Author: fidencio
 Title: #241: FleetCommander Integration
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/241/head:pr241
git checkout pr241

[sssd PR#241][synchronized] FleetCommander Integration

[fidencio]

URL: https://github.com/SSSD/sssd/pull/241
Author: fidencio
 Title: #241: FleetCommander Integration
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/241/head:pr241
git checkout pr241

[sssd PR#241][comment] FleetCommander Integration

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

pbrezina commented:
"""
Typo in "related":
```xml
+                        <para>
+                            Default: <quote>id_provider</quote> is used if it
+                            is set and can perform session reltted tasks.
+                        </para>

```

This works and it looks good in provider logs but in pam log is a system error which may be confusing. Please add the diff I provided so we can avoid it.

```
275 (Tue Aug 22 09:59:23 2017) [sssd[pam]] [pam_dp_send_req] (0x0100): Sending request with the following data:
276 (Tue Aug 22 09:59:23 2017) [sssd[pam]] [pam_print_data] (0x0100): command: SSS_PAM_OPEN_SESSION
277 (Tue Aug 22 09:59:23 2017) [sssd[pam]] [pam_print_data] (0x0100): domain: LDAP.PB
278 (Tue Aug 22 09:59:23 2017) [sssd[pam]] [pam_print_data] (0x0100): user: [e-mail]
279 (Tue Aug 22 09:59:23 2017) [sssd[pam]] [pam_print_data] (0x0100): service: su
280 (Tue Aug 22 09:59:23 2017) [sssd[pam]] [pam_print_data] (0x0100): tty: pts/7
281 (Tue Aug 22 09:59:23 2017) [sssd[pam]] [pam_print_data] (0x0100): ruser: pbrezina
282 (Tue Aug 22 09:59:23 2017) [sssd[pam]] [pam_print_data] (0x0100): rhost: not set
283 (Tue Aug 22 09:59:23 2017) [sssd[pam]] [pam_print_data] (0x0100): authtok type: 0
284 (Tue Aug 22 09:59:23 2017) [sssd[pam]] [pam_print_data] (0x0100): newauthtok type: 0
285 (Tue Aug 22 09:59:23 2017) [sssd[pam]] [pam_print_data] (0x0100): priv: 0
286 (Tue Aug 22 09:59:23 2017) [sssd[pam]] [pam_print_data] (0x0100): cli_pid: 10586
287 (Tue Aug 22 09:59:23 2017) [sssd[pam]] [pam_print_data] (0x0100): logon name: user-1
288 (Tue Aug 22 09:59:23 2017) [sssd[pam]] [sbus_add_timeout] (0x2000): 0xe9e330
289 (Tue Aug 22 09:59:23 2017) [sssd[pam]] [pam_dom_forwarder] (0x0100): pam_dp_send_req returned 0
290 (Tue Aug 22 09:59:23 2017) [sssd[pam]] [sbus_remove_timeout] (0x2000): 0xe9e330
291 (Tue Aug 22 09:59:23 2017) [sssd[pam]] [pam_dp_process_reply] (0x0010): Reply error.
292 (Tue Aug 22 09:59:23 2017) [sssd[pam]] [pam_reply] (0x0200): pam_reply called with result [4]: System error.
```
"""

See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-323951221

[sssd PR#241][synchronized] FleetCommander Integration

[fidencio]

URL: https://github.com/SSSD/sssd/pull/241
Author: fidencio
 Title: #241: FleetCommander Integration
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/241/head:pr241
git checkout pr241

[sssd PR#241][comment] FleetCommander Integration

[fidencio]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

fidencio commented:
"""
@pbrezina: patchset has been updated. Thanks for your suggestions!
"""

See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-324003707

[sssd PR#241][comment] FleetCommander Integration

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

pbrezina commented:
"""
On 08/22/2017 01:54 PM, fidencio wrote:
> @pbrezina https://github.com/pbrezina: patchset has been updated.
> Thanks for your suggestions!

Ack.

"""

See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-324011406

[sssd PR#241][comment] FleetCommander Integration

[olivergs]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

olivergs commented:
"""
Tested the latest build for the patches from @fidencio COPR repo and it
worked as expected.

On Thu, Aug 17, 2017 at 4:23 PM, fidencio  wrote:
> Patch set updated.
>
> This is the patch that I've squashed:
>
> From 97f189d3a4fcbbfe453cc6ba8eab8710ca31c4ac Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= 
> Date: Thu, 17 Aug 2017 13:29:24 +0200
> Subject: [PATCH] fixup! DESKPROFILE: Introduce the new IPA session provider
>
> src/providers/ipa/ipa_deskprofile_rules_util.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/src/providers/ipa/ipa_deskprofile_rules_util.c b/src/providers/ipa/ipa_deskprofile_rules_util.c
> index 12c6492ba..2ece5a1ee 100644
> --- a/src/providers/ipa/ipa_deskprofile_rules_util.c
> +++ b/src/providers/ipa/ipa_deskprofile_rules_util.c
> @@ -150,7 +150,7 @@ ipa_deskprofile_get_filename_path(TALLOC_CTX *mem_ctx,
>          extension,
>          NULL,
>      };
> const uint8_t *perms = permuts[config_priority - 1];
> const uint8_t *perms;
> char *result;
> errno_t ret;
>
> @@ -167,6 +167,8 @@ ipa_deskprofile_get_filename_path(TALLOC_CTX *mem_ctx,
>          goto done;
>      }
> perms = permuts[config_priority - 1];
> result = talloc_strdup(tmp_ctx, "");
>    if (result == NULL) {
>        ret = ENOMEM;
>
> --
> 2.13.4
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> https://github.com/SSSD/sssd/pull/241#issuecomment-323102642, or mute
> the thread
> https://github.com/notifications/unsubscribe-auth/ABX4oQhaAXHRCzP43H7qrQ3aCbpAVR9Zks5sZFhZgaJpZM4NDNM2
> .

[sssd PR#241][comment] FleetCommander Integration

[olivergs]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

olivergs commented:
"""
Tested the latest build for the patches from @fidencio COPR repo and it
worked as expected.
"""

See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-325337915

[sssd PR#241][comment] FleetCommander Integration

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

jhrozek commented:
"""
ACK, CI: http://vm-058-233.XXX/logs/job/74/33/summary.html
"""

See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-325391191

[sssd PR#241][+Accepted] FleetCommander Integration

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

Label: +Accepted

[sssd PR#241][comment] FleetCommander Integration

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

jhrozek commented:
"""
* master:
4a311702045b065a97a0c0fc0ccc7a1fc84b38cf
85517b57685809ff96818bbd3e3b4678ac74b461
85a93ca67ae020607006cd035170c9360fb0a450
684a13e8de1526257ca2e40b6bf2e05585d4eaca
4b37ee7d370003514916c793046577ea4b6e736b
dd6a4fb9ae4825caf4ccb835f8b8221c96bbb6f5
c9e104f17b6c4cf5741dea9fdbe864619125fab1
9d98e98ab37d86323034e7bc342f196b81fa07bc
b054e7d8c43b024ee33e9343b4a15e124861f68c
f982039c75ec064894deb676ae53ee57de868590
6f466e0a3d950d21bd750ef53cb93b75dc023f9e
5b93634c7f0e34f69b4cf8fb9b2e77b9179024a7
7c1d1393537dec95e09b83b607ce9d0e8f49584c
18d898d9cb30f298b3a35dc1c1bace95ef4e0b3b
ee164913f9c12a557044eb469f4498b9be9a8f50
0f623456437c96f50330fe0ff21afd9638d14e57
d2a0b4a6a220bf9a58c7306c3f673891efc419eb
e17e37cd0e2109e7f1bd4ae48edfc8cca85b3f93
21909d3b620d97e81dd946b959a47efe88d2b7d8
9a18f78f38e274f4906af6ef8e1a82d844fde4cc
8a26d32bc9b71e85a42b7832891100a7249f92aa

"""

See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-325442601

[sssd PR#241][+Pushed] FleetCommander Integration

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/241
Title: #241: FleetCommander Integration

Label: +Pushed

[sssd PR#241][closed] FleetCommander Integration

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/241
Author: fidencio
 Title: #241: FleetCommander Integration
Action: closed

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/241/head:pr241
git checkout pr241