DBUS: Add ListByAttr(attr, filter, limit) #6367

aplopez · 2022-09-27T12:25:10Z

Extended ListByName()'s mechanics to handle an attribute passed as parameters instead of forcing "name". ListByName() will pass "name."

Created a dbus function ListByAttr() passing the attribute requested by the user.

Resolves: #6020

:feature: Introduced the dbus function org.freedesktop.sssd.infopupe.Users.ListByAttr(attr, value, limit) listing users matching the filter attr=value.

Please have in mind these issues below also affect this implementation as they share the mechanics with ListByName(). They have to be addressed separate from this PR.
[D-Bus] ListByName() fails when not using wildcards
[D-Bus] ListByName() returns twice the same entry

src/responder/ifp/ifp_iface/sbus_ifp_client_sync.c

ikerexxe

The code looks good to me but I'm missing some type of automated test, either a unit-test or an integration test.

src/responder/common/cache_req/cache_req_private.h

justin-stephenson · 2022-10-03T15:22:14Z

As the original requestor of the feature, perhaps @flo-renaud may want to review this also.

aplopez · 2022-10-03T15:37:29Z

The code looks good to me but I'm missing some type of automated test, either a unit-test or an integration test.

Added a test into the file you linked.

src/responder/ifp/ifp_iface/sbus_ifp_client_sync.c

justin-stephenson · 2022-10-03T15:41:30Z

Hi Alejandro,

Did I miss something in my configuration, or command I am using?

[root@master /]# dbus-send --system --print-reply --dest=org.freedesktop.sssd.infopipe /org/freedesktop/sssd/infopipe/Users/ipa_2etest/121000009 org.freedesktop.DBus.Properties.Get string:"org.freedesktop.sssd.infopipe.Users.User" string:"extraAttributes"
method return time=1664811583.329400 sender=:1.107 -> destination=:1.106 serial=6 reply_serial=2
   variant       array [
         dict entry(
            string "mail"
            array [
               string "testuser1@ipa.test"
            ]
         )
         dict entry(
            string "sn"
            array [
               string "user1"
            ]
         )
      ]

[root@master /]# dbus-send --system --print-reply --dest=org.freedesktop.sssd.infopipe /org/freedesktop/sssd/infopipe/Users org.freedesktop.sssd.infopipe.Users.ListByAttr "string:sn" "string:user1" "uint32:0"                            
method return time=1664811587.993677 sender=:1.107 -> destination=:1.108 serial=8 reply_serial=2
   array [
   ]

[root@master /]# dbus-send --system --print-reply --dest=org.freedesktop.sssd.infopipe /org/freedesktop/sssd/infopipe/Users org.freedesktop.sssd.infopipe.Users.ListByAttr "string:sn" "string:user*" "uint32:0"
method return time=1664811607.436829 sender=:1.112 -> destination=:1.111 serial=6 reply_serial=2
   array [
   ]

aplopez · 2022-10-03T15:58:53Z

[root@master /]# dbus-send --system --print-reply --dest=org.freedesktop.sssd.infopipe /org/freedesktop/sssd/infopipe/Users org.freedesktop.sssd.infopipe.Users.ListByAttr "string:sn" "string:user1" "uint32:0"                            
method return time=1664811587.993677 sender=:1.107 -> destination=:1.108 serial=8 reply_serial=2
   array [
   ]

The problem here is that there are two pre-existing bugs that affect this new feature as it shares the code with ListByName(). I added the links in the description. So you should look for "user1*" to work around the issue.

[root@master /]# dbus-send --system --print-reply --dest=org.freedesktop.sssd.infopipe /org/freedesktop/sssd/infopipe/Users org.freedesktop.sssd.infopipe.Users.ListByAttr "string:sn" "string:user*" "uint32:0"
method return time=1664811607.436829 sender=:1.112 -> destination=:1.111 serial=6 reply_serial=2
   array [
   ]

I guess you need to add ldap_user_extra_attrs = sn to your domain's configuration for attribute sn to be included in the cache.

justin-stephenson · 2022-10-03T16:11:03Z

I guess you need to add ldap_user_extra_attrs = sn to your domain's configuration for attribute sn to be included in the cache.

I do have this and user_attributes in my sssd configuration.

Attached log of the failed ListByAttr lookup with "string:sn" "string:user1" "uint32:0"

justin-stephenson · 2022-10-03T16:13:49Z

https://paste.centos.org/view/b6dbbb15

justin-stephenson · 2022-10-03T18:18:21Z

flake8 check is failing to post the result to the PR, but manually running it shows:

# flake8 src/tests/intg/test_infopipe.py 
src/tests/intg/test_infopipe.py:806:1: E302 expected 2 blank lines, found 1
src/tests/intg/test_infopipe.py:822:1: W391 blank line at end of file

aplopez · 2022-10-04T07:14:52Z

flake8 check is failing to post the result to the PR, but manually running it shows:
# flake8 src/tests/intg/test_infopipe.py 
src/tests/intg/test_infopipe.py:806:1: E302 expected 2 blank lines, found 1
src/tests/intg/test_infopipe.py:822:1: W391 blank line at end of file

Thanks. I was looking for this information. (Good to know I can run it manually)

ikerexxe · 2022-10-04T08:25:32Z

Added a test into the file you linked.

Thanks!

Once the flake8 issues are fixed it would be good for me.

Extended ListByName()'s mechanics to handle an attribute passed as parameters instead of forcing "name." ListByName() will pass "name." Created a dbus function ListByAttr() using ListByName()'s mechanics but passing the attribute requested by the user. Resolves: SSSD#6020 :feature: Introduced the dbus function org.freedesktop.sssd.infopipe.Users.ListByAttr(attr, value, limit) listing upto limit users matching the filter attr=value.

src/responder/ifp/ifp_iface/sbus_ifp_client_sync.c

aplopez · 2022-10-04T20:02:10Z

I guess you need to add ldap_user_extra_attrs = sn to your domain's configuration for attribute sn to be included in the cache.

I do have this and user_attributes in my sssd configuration.

Attached log of the failed ListByAttr lookup with "string:sn" "string:user1" "uint32:0"

Should be fixed now. Let me know if it is not the case.

aplopez · 2022-10-04T20:04:33Z

@ikerexxe In addition to the flake8 fix, I made some changes related to the problem found by @justin-stephenson . You may want to review them.

alexey-tikhonov · 2022-10-04T21:00:29Z

Covscan failed due to infra failure.

ikerexxe · 2022-10-05T07:19:14Z

LGTM. Thanks for the patches.

justin-stephenson · 2022-10-05T12:46:55Z

Should be fixed now. Let me know if it is not the case.

Hi, this works now but output is repeated twice. If this is acceptable and will be addressed separately then ack from my side.

dbus-send --system --print-reply --dest=org.freedesktop.sssd.infopipe /org/freedesktop/sssd/infopipe/Users org.freedesktop.sssd.infopipe.Users.ListByAttr "string:sn" "string:user*" "uint32:0"
method return time=1664973962.592147 sender=:1.191 -> destination=:1.205 serial=32 reply_serial=2
   array [
      object path "/org/freedesktop/sssd/infopipe/Users/ipa_2etest/121000009"
      object path "/org/freedesktop/sssd/infopipe/Users/ipa_2etest/121000009"
   ]

aplopez · 2022-10-05T13:01:58Z

Hi, this works now but output is repeated twice. If this is acceptable and will be addressed separately then ack from my side.

Hi. It is one of the preexisting bugs mentioned in the description.
#6360

It will be fixed at some time in the future.

pbrezina · 2022-10-07T10:41:37Z

Pushed PR: #6367

master
- acfe3b2 - DBUS: Add ListByAttr(attr, filter, limit)

github-actions bot reviewed Sep 27, 2022

View reviewed changes

src/responder/ifp/ifp_iface/sbus_ifp_client_sync.c Show resolved Hide resolved

aplopez force-pushed the dbus branch from b58b520 to 34b6b7f Compare September 28, 2022 09:12

github-actions bot reviewed Sep 28, 2022

View reviewed changes

src/responder/ifp/ifp_iface/sbus_ifp_client_sync.c Show resolved Hide resolved

aplopez marked this pull request as ready for review September 28, 2022 12:01

aplopez added the Waiting for review label Sep 28, 2022

alexey-tikhonov added the no-backport This should go to target branch only. label Sep 28, 2022

justin-stephenson self-assigned this Sep 28, 2022

andreboscatto assigned ikerexxe Sep 29, 2022

ikerexxe reviewed Sep 30, 2022

View reviewed changes

alexey-tikhonov added the Bugzilla label Oct 3, 2022

justin-stephenson reviewed Oct 3, 2022

View reviewed changes

src/responder/common/cache_req/cache_req_private.h Show resolved Hide resolved

aplopez force-pushed the dbus branch from 34b6b7f to 4360ca3 Compare October 3, 2022 15:36

github-actions bot reviewed Oct 3, 2022

View reviewed changes

src/responder/ifp/ifp_iface/sbus_ifp_client_sync.c Show resolved Hide resolved

aplopez requested review from ikerexxe and justin-stephenson October 3, 2022 15:38

aplopez force-pushed the dbus branch from 4360ca3 to 786b4ac Compare October 4, 2022 20:00

github-actions bot reviewed Oct 4, 2022

View reviewed changes

src/responder/ifp/ifp_iface/sbus_ifp_client_sync.c Show resolved Hide resolved

ikerexxe approved these changes Oct 5, 2022

View reviewed changes

justin-stephenson approved these changes Oct 5, 2022

View reviewed changes

aplopez added Accepted Ready to push Ready to push and removed Waiting for review labels Oct 5, 2022

pbrezina added Pushed and removed Accepted Ready to push Ready to push labels Oct 7, 2022

pbrezina closed this Oct 7, 2022

pbrezina mentioned this pull request Oct 7, 2022

[RFE] provide dbus method to find users by attr #6020

Closed

aplopez deleted the dbus branch November 25, 2022 08:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DBUS: Add ListByAttr(attr, filter, limit) #6367

DBUS: Add ListByAttr(attr, filter, limit) #6367

aplopez commented Sep 27, 2022 •

edited

ikerexxe left a comment •

edited

justin-stephenson commented Oct 3, 2022

aplopez commented Oct 3, 2022

justin-stephenson commented Oct 3, 2022

aplopez commented Oct 3, 2022 •

edited

justin-stephenson commented Oct 3, 2022

justin-stephenson commented Oct 3, 2022

justin-stephenson commented Oct 3, 2022

aplopez commented Oct 4, 2022

ikerexxe commented Oct 4, 2022

aplopez commented Oct 4, 2022

aplopez commented Oct 4, 2022

alexey-tikhonov commented Oct 4, 2022

ikerexxe commented Oct 5, 2022

justin-stephenson commented Oct 5, 2022

aplopez commented Oct 5, 2022 •

edited

pbrezina commented Oct 7, 2022

DBUS: Add ListByAttr(attr, filter, limit) #6367

DBUS: Add ListByAttr(attr, filter, limit) #6367

Conversation

aplopez commented Sep 27, 2022 • edited

ikerexxe left a comment • edited

Choose a reason for hiding this comment

justin-stephenson commented Oct 3, 2022

aplopez commented Oct 3, 2022

justin-stephenson commented Oct 3, 2022

aplopez commented Oct 3, 2022 • edited

justin-stephenson commented Oct 3, 2022

justin-stephenson commented Oct 3, 2022

justin-stephenson commented Oct 3, 2022

aplopez commented Oct 4, 2022

ikerexxe commented Oct 4, 2022

aplopez commented Oct 4, 2022

aplopez commented Oct 4, 2022

alexey-tikhonov commented Oct 4, 2022

ikerexxe commented Oct 5, 2022

justin-stephenson commented Oct 5, 2022

aplopez commented Oct 5, 2022 • edited

pbrezina commented Oct 7, 2022

aplopez commented Sep 27, 2022 •

edited

ikerexxe left a comment •

edited

aplopez commented Oct 3, 2022 •

edited

aplopez commented Oct 5, 2022 •

edited