URL: https://github.com/SSSD/sssd/pull/260 Title: #260: Update sss_override.c
fidencio commented: """ Amit, Sorry for the long time taken for reviewing this patch. I didn't go through the code but I still can see some functional issues with your patches. (same one mentioned in the [in the comment above](https://github.com/SSSD/sssd/pull/260#issuecomment-299434151). Let me show you what happens when I have your patches applied: ``` [root@client sssd]# sss_override user-add administra...@fidencio.lan -u 11111 SSSD needs to be restarted for the changes to take effect. Setting uid or gid as 0 is not allowed ``` This is *not* the expected behaviour. For instance, if the admin sets a user's id override without passing the gid ... it's expected that the gid will be kept as the same. With your patch, what happens is we fail the operation because the gid was not set. The reason why it happens is because when parsing the user passed in the command line (please, take a look on `parse_cmdlinser_user_add` function the default value taken for uid/gid is 0 (when none is passed). Here's what the struct override_user looks like: ``` struct override_user { const char *input_name; const char *orig_name; const char *sysdb_name; struct sss_domain_info *domain; const char *name; uid_t uid; gid_t gid; const char *home; const char *shell;struct override_user { const char *input_name; const char *orig_name; const char *sysdb_name; struct sss_domain_info *domain; const char *name; uid_t uid; gid_t gid; const char *home; const char *shell; const char *gecos; const char *cert; }; ``` So, uid and gid cannot be set to a negative number (like -1) in order to differentiate whether the 0 was intentionally set or just was not set at all (and the admin wants to keep the current user's uid/gid). As suggested in the previous comment, you'll have to dig a bit more in the code in order to propose a better solution. Maybe a better solution would be to populate the override_user structure before hand and then go and set it in the sysdb only in case we are sure a 0 wasn't passed in the command line? I'm not even sure it would work, this is just coming from the top of my head and I haven't any experience with this code (yet). Please, let me know if you're interested on doing such changes/digging deeper into the issue. """ See the full comment at https://github.com/SSSD/sssd/pull/260#issuecomment-306409624
_______________________________________________ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
