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 <notifications(a)github.com>
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.
"""
See the full comment at
https://github.com/SSSD/sssd/pull/241#issuecomment-323711365