On 05/26/2016 12:36 PM, Sumit Bose wrote:
On Mon, May 23, 2016 at 10:26:37PM +0300, Nikolai Kondrashov wrote:
> First of all, I assume you meant "shell" whenever you used
"homedir"
oops, sorry, yes get_homedir_override() is just before
get_shell_override() in nsssrv_cmd.c and I slipped into the wrong
function.
No problem, happens to everyone, was easy to figure out :)
> On 05/20/2016 02:50 PM, Sumit Bose wrote:
>> On Fri, May 20, 2016 at 01:54:33PM +0300, Nikolai Kondrashov wrote:
>>> On 05/20/2016 01:39 PM, Sumit Bose wrote:
>>>> ah, was thought tlog will be switched on differently, so you either can
>>>> have tlog or an overriden shell. And yes, in this case you do not have
to
>>>> check the overriden shell.
>>>>
>>>> But I think you still need an evaluation similar to
>>>> get_homedir_override() because e.g. if there are no POSIX attributes in
>>>> AD users from AD will not have a shell set at all and the shell might be
>>>> chosen based on SSSD's shell related config options.
>>>>
>>>> Instead of sss_view_ldb_msg_find_attr_as_string() you should use
>>>> ldb_msg_find_attr_as_string() and if this is already tlog (this is the
>>>> case where the tlog shell is added to the default override) you should
>>>> check if ORIGINALAD_PREFIX SYSDB_SHELL exists.
>
> I'm sorry, I don't quite understand this sentence. How can it already be
> "tlog" and what is the "default override"?
>
> Could you please elaborate on this piece of logic?
If I understood you comment correctly tlog will be enabled for a user by
using overrides, so you either use local overrides
sss_override user-add user_name --shell=/bin/tloc-rec
or IPA overrides
idoverrideuser-add view_name user_name --shell=/bin/tloc-rec
In IPA there is a special view called "Default Trust View". If overrides
are configured for this view the values are not stored in the SSSD cache
in separate override objects but in the related user object directly
replacinfg the original value, if any. The original value is then
available in the attribute prefixed by ORIGINALAD_PREFIX. So if you call
e.g
idoverrideuser-add "Default Trust View" ad_user(a)ad.domain
--shell=/bin/tloc-rec
SYSDB_SHELL will contain /bin/tloc-rec and ORIGINALAD_PREFIX SYSDB_SHELL
the shell as set in AD. The reason for this is the trusted users in the
"Default Trust View" should be behave in the same way as IPA users
without any special handling.
Right. Thank you for the explanation, Sumit. I heard about ID views, but I
didn't think they also implement overrides. IIRC, the original idea suggested
by SSSD developers was to use local overrides only.
Using ID views for this actually sounds great to me. However, as I know little
of them, I'd like someone else to check if that will be fine regarding overall
design and intended use.
Martin, Jakub, perhaps you can comment?
Otherwise, Sumit, would this kind of logic do for retrieving the actual
(original) user shell:
if ORIGINALAD_PREFIX SYSDB_SHELL is set, use it,
else use SYSDB_SHELL
>> sysdb_getpwnam only reads data from the cache but the cache
basically
>> has what is read from LDAP without additional processing, so the
>> SYSDB_SHELL might even already have the tlog shell in the default
>> override case. Since you do not need the additional override attributes
>> from other view it would be possible to use sysdb_getpwnam() the
>> additional processing is still needed.
>
> Ah, after reading the source code for quite a while, I think I finally
> understand what's going on and what you mean :)
>
> Now, I think adding a special parameter to get_shell_override() to switch
> between ldb_msg_find_attr_as_string() and
> sss_view_ldb_msg_find_attr_as_string() will make the function more complex and
> harder to understand. What if I simply make it accept a shell string and
> retreive that outside the function? The only user of the function
> (fill_pwent()) does a lot of that for the other attributes anyway.
yes, fill_pwent() does this but I think it would be better to have the
whole processing of a single attribute in a separate function like
get_shell_override() and get_homedir_override(). This would make
fill_pwent() which currently has more than 200 lines-of-code more
readable in the long run.
Alright.
> For that matter, what if I do the similar thing for the other
get_*_override
> functions just to keep it uniform?
>
> Regardless of that, what would be the proper way to expose these functions to
> the pamsrv_cmd.c module (and I think they all should be exposed at once, as
> they're doing a similar thing), since they're in nsssrv_cmd.c? Do I move
them
> to another module and rename (to which and rename how?), do I put them into a
> header file (which and under which names), do I create a new header file,
> something else? I can't quite catch the conventions here.
I think src/responder/common/responder.h would be the right header and
src/responder/common/responder_common.c or a new file in
src/responder/common/ the right place to add the code.
Alright, I'll do that.
Thank you, Sumit!
Nick