Hi,
the attached two patches add a GetGroupsList method that adelton's mod_lookup_identity uses.
The first patch is a one-liner that I don't think harms the current sysdb_initgroups() users in any way, also all the sysdb tests pass.
The second patch adds the new method.
On Fri, May 09, 2014 at 09:58:52PM +0200, Jakub Hrozek wrote:
Hi,
the attached two patches add a GetGroupsList method that adelton's mod_lookup_identity uses.
The first patch is a one-liner that I don't think harms the current sysdb_initgroups() users in any way, also all the sysdb tests pass.
The second patch adds the new method.
Pavel did a review on IRC and requested a change -- the patches don't work quite well if the user is not a member of any group except his private one. I'll address this comment.
On Tue, May 13, 2014 at 11:06:47PM +0200, Jakub Hrozek wrote:
On Fri, May 09, 2014 at 09:58:52PM +0200, Jakub Hrozek wrote:
Hi,
the attached two patches add a GetGroupsList method that adelton's mod_lookup_identity uses.
The first patch is a one-liner that I don't think harms the current sysdb_initgroups() users in any way, also all the sysdb tests pass.
The second patch adds the new method.
Pavel did a review on IRC and requested a change -- the patches don't work quite well if the user is not a member of any group except his private one. I'll address this comment.
I'm sorry but I couldn't reproduce that bug. I added a user in IPA: $ ipa user-show nogroups User login: nogroups First name: no secondary Last name: groups Home directory: /home/nogroups Login shell: /bin/sh Email address: nogroups@ipatest.example.com UID: 69800016 GID: 69800016 Account disabled: False Password: False Member of groups: ipausers Kerberos keys available: False
And requesting his groups comes as empty, which is what I'd expect since ipausers is a non-POSIX group: $ dbus-send --print-reply --system --dest=org.freedesktop.sssd.infopipe /org/freedesktop/sssd/infopipe org.freedesktop.sssd.infopipe.GetUserGroups string:nogroups method return sender=:1.31 -> dest=:1.32 reply_serial=2 array [ ] $ id nogroups uid=69800016(nogroups) gid=69800016(nogroups) groups=69800016(nogroups)
I'm attaching a new set of patches anyway to address the some formatting issues.
On 05/14/2014 12:23 PM, Jakub Hrozek wrote:
On Tue, May 13, 2014 at 11:06:47PM +0200, Jakub Hrozek wrote:
On Fri, May 09, 2014 at 09:58:52PM +0200, Jakub Hrozek wrote:
Hi,
the attached two patches add a GetGroupsList method that adelton's mod_lookup_identity uses.
The first patch is a one-liner that I don't think harms the current sysdb_initgroups() users in any way, also all the sysdb tests pass.
The second patch adds the new method.
Pavel did a review on IRC and requested a change -- the patches don't work quite well if the user is not a member of any group except his private one. I'll address this comment.
I'm sorry but I couldn't reproduce that bug. I added a user in IPA: $ ipa user-show nogroups User login: nogroups First name: no secondary Last name: groups Home directory: /home/nogroups Login shell: /bin/sh Email address: nogroups@ipatest.example.com UID: 69800016 GID: 69800016 Account disabled: False Password: False Member of groups: ipausers Kerberos keys available: False
And requesting his groups comes as empty, which is what I'd expect since ipausers is a non-POSIX group: $ dbus-send --print-reply --system --dest=org.freedesktop.sssd.infopipe /org/freedesktop/sssd/infopipe org.freedesktop.sssd.infopipe.GetUserGroups string:nogroups method return sender=:1.31 -> dest=:1.32 reply_serial=2 array [ ] $ id nogroups uid=69800016(nogroups) gid=69800016(nogroups) groups=69800016(nogroups)
I'm attaching a new set of patches anyway to address the some formatting issues.
Interesting, I am unable to reproduce it any more either. Of course I did the review yesterday very quickly, it is possible that I applied the patches to an outdated branch.
Anyway, the patches looks good and I haven't found any issue.
Ack.
On Wed, May 14, 2014 at 01:41:01PM +0200, Pavel Březina wrote:
On 05/14/2014 12:23 PM, Jakub Hrozek wrote:
On Tue, May 13, 2014 at 11:06:47PM +0200, Jakub Hrozek wrote:
On Fri, May 09, 2014 at 09:58:52PM +0200, Jakub Hrozek wrote:
Hi,
the attached two patches add a GetGroupsList method that adelton's mod_lookup_identity uses.
The first patch is a one-liner that I don't think harms the current sysdb_initgroups() users in any way, also all the sysdb tests pass.
The second patch adds the new method.
Pavel did a review on IRC and requested a change -- the patches don't work quite well if the user is not a member of any group except his private one. I'll address this comment.
I'm sorry but I couldn't reproduce that bug. I added a user in IPA: $ ipa user-show nogroups User login: nogroups First name: no secondary Last name: groups Home directory: /home/nogroups Login shell: /bin/sh Email address: nogroups@ipatest.example.com UID: 69800016 GID: 69800016 Account disabled: False Password: False Member of groups: ipausers Kerberos keys available: False
And requesting his groups comes as empty, which is what I'd expect since ipausers is a non-POSIX group: $ dbus-send --print-reply --system --dest=org.freedesktop.sssd.infopipe /org/freedesktop/sssd/infopipe org.freedesktop.sssd.infopipe.GetUserGroups string:nogroups method return sender=:1.31 -> dest=:1.32 reply_serial=2 array [ ] $ id nogroups uid=69800016(nogroups) gid=69800016(nogroups) groups=69800016(nogroups)
I'm attaching a new set of patches anyway to address the some formatting issues.
Interesting, I am unable to reproduce it any more either. Of course I did the review yesterday very quickly, it is possible that I applied the patches to an outdated branch.
Anyway, the patches looks good and I haven't found any issue.
Ack.
Thanks, pushed to master: 3fe339bcba0e211cc666bb3afe34e5c8fce85f4f cf2cc44d6293165379d6470b8bf6bb6a87d62b31
On Wed, May 14, 2014 at 04:54:04PM +0200, Jakub Hrozek wrote:
On Wed, May 14, 2014 at 01:41:01PM +0200, Pavel Březina wrote:
On 05/14/2014 12:23 PM, Jakub Hrozek wrote:
On Tue, May 13, 2014 at 11:06:47PM +0200, Jakub Hrozek wrote:
On Fri, May 09, 2014 at 09:58:52PM +0200, Jakub Hrozek wrote:
Hi,
the attached two patches add a GetGroupsList method that adelton's mod_lookup_identity uses.
The first patch is a one-liner that I don't think harms the current sysdb_initgroups() users in any way, also all the sysdb tests pass.
The second patch adds the new method.
Pavel did a review on IRC and requested a change -- the patches don't work quite well if the user is not a member of any group except his private one. I'll address this comment.
I'm sorry but I couldn't reproduce that bug. I added a user in IPA: $ ipa user-show nogroups User login: nogroups First name: no secondary Last name: groups Home directory: /home/nogroups Login shell: /bin/sh Email address: nogroups@ipatest.example.com UID: 69800016 GID: 69800016 Account disabled: False Password: False Member of groups: ipausers Kerberos keys available: False
And requesting his groups comes as empty, which is what I'd expect since ipausers is a non-POSIX group: $ dbus-send --print-reply --system --dest=org.freedesktop.sssd.infopipe /org/freedesktop/sssd/infopipe org.freedesktop.sssd.infopipe.GetUserGroups string:nogroups method return sender=:1.31 -> dest=:1.32 reply_serial=2 array [ ] $ id nogroups uid=69800016(nogroups) gid=69800016(nogroups) groups=69800016(nogroups)
I'm attaching a new set of patches anyway to address the some formatting issues.
Interesting, I am unable to reproduce it any more either. Of course I did the review yesterday very quickly, it is possible that I applied the patches to an outdated branch.
Anyway, the patches looks good and I haven't found any issue.
Ack.
Thanks, pushed to master: 3fe339bcba0e211cc666bb3afe34e5c8fce85f4f cf2cc44d6293165379d6470b8bf6bb6a87d62b31
Pushed to sssd-1-11: 64f5e4b48926c9c14994e73c5a950a5ac876ec8e bd8cf980dc2e7035a8ce893df4a8ccaf9bbf11ad
sssd-devel@lists.fedorahosted.org
-
Jakub Hrozek -
Jakub Hrozek -
Pavel Březina