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