9:50 a.m.
URL: https://github.com/SSSD/sssd/pull/5784
Author: ikerexxe
Title: #5784: proxy: allow removing group members
Action: opened
PR body:
"""
The proxy provider doesn't allow to remove group members once they have
been added. This patch allows to do it by looping the member list from
the cache and comparing it with the actual membership list. If a member
is missing then it's removed from the cache.
Resolves: https://github.com/SSSD/sssd/issues/5783
"""
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5784/head:pr5784
git checkout pr5784
9:50 a.m.
New subject: [sssd PR#5784][+Bugzilla] proxy: allow removing group members
URL: https://github.com/SSSD/sssd/pull/5784
Title: #5784: proxy: allow removing group members
Label: +Bugzilla
9:50 a.m.
New subject: [sssd PR#5784][+Waiting for review] proxy: allow removing group members
URL: https://github.com/SSSD/sssd/pull/5784
Title: #5784: proxy: allow removing group members
Label: +Waiting for review
7:56 a.m.
New subject: [sssd PR#5784][synchronized] proxy: allow removing group members
URL: https://github.com/SSSD/sssd/pull/5784
Author: ikerexxe
Title: #5784: proxy: allow removing group members
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5784/head:pr5784
git checkout pr5784
8:02 a.m.
New subject: [sssd PR#5784][comment] proxy: allow removing group members
URL: https://github.com/SSSD/sssd/pull/5784
Title: #5784: proxy: allow removing group members
alexey-tikhonov commented:
"""
Hi @ikerexxe ,
I've left few comment inline. In general patch should work.
But the first question to answer:
You search for groups user is currently member of (`SYSDB_MEMBEROF`) => got list of DNs
=> convert every DN to RDN (group name) => and then for every RDN you iterate over
initgroups GID list to get a group name of every GID => convert it to FQDN and then do
a string comparison (RDN with FQDN), right?
This feels extremely inefficient: you repeat **the same set** of lookups and conversions
over initgroups GIDs for every RDN (and it will be also repeated later in
`get_initgr_groups_process()`::`NSS_STATUS_SUCCESS`)...
Wouldn't it make sense to instead lookup GID of SYSDB_MEMBEROF DN and compare GIDs
(numerical values)?
(If this is not feasible then at the very least you should compose a list of RDNs first
and then do initgroups GIDs iteration once)
@sumit-bose , can it happen (is it supported) two groups in different domains have the
same GID?
"""
See the full comment at https://github.com/SSSD/sssd/pull/5784#issuecomment-933460178
9:31 a.m.
New subject: [sssd PR#5784][+Changes requested] proxy: allow removing group members
URL: https://github.com/SSSD/sssd/pull/5784
Title: #5784: proxy: allow removing group members
Label: +Changes requested
9:31 a.m.
New subject: [sssd PR#5784][-Waiting for review] proxy: allow removing group members
URL: https://github.com/SSSD/sssd/pull/5784
Title: #5784: proxy: allow removing group members
Label: -Waiting for review
9:47 a.m.
New subject: [sssd PR#5784][comment] proxy: allow removing group members
URL: https://github.com/SSSD/sssd/pull/5784
Title: #5784: proxy: allow removing group members
sumit-bose commented:
"""
@sumit-bose , can it happen (is it supported) two groups in different
domains have the same GID?
Hi,
if the two domains are managed separately it can of course happen that two groups have the
same GID. For lookups by GID the group from the first domain listed in the `domains`
option will win. A lookup by name, especially with a fully-qualified name, will most
probably resolve both groups.
Given that the `id` command will do lookups by GID an `id` lookup for the user from the
second domain which is a member of the group with the duplicated GID will return the wrong
group name. In this sense I would say we do not support this kind of configuration.
Additionally, the filesystem is doing access control based with respect to groups with the
help of the GID using the same GID in different domains might give users access to files
of users from the other domain, which is typically not expected.
HTH
bye,
Sumit
"""
See the full comment at https://github.com/SSSD/sssd/pull/5784#issuecomment-934479813
10:08 a.m.
New subject: [sssd PR#5784][comment] proxy: allow removing group members
URL: https://github.com/SSSD/sssd/pull/5784
Title: #5784: proxy: allow removing group members
alexey-tikhonov commented:
"""
Given that the `id` command will do lookups by GID an `id` lookup for
the user from the second domain which is a member of the group with the duplicated GID
will return the wrong group name. In this sense I would say we do not support this kind of
configuration.
Additionally, the filesystem is doing access control based with respect to groups with
the help of the GID using the same GID in different domains might give users access to
files of users from the other domain, which is typically not expected.
I read this as GIDs collisions aren't supported (i.e. GIDs are unique) and hence we
can compare GIDs.
"""
See the full comment at https://github.com/SSSD/sssd/pull/5784#issuecomment-934497065
8:31 a.m.
New subject: [sssd PR#5784][synchronized] proxy: allow removing group members
URL: https://github.com/SSSD/sssd/pull/5784
Author: ikerexxe
Title: #5784: proxy: allow removing group members
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5784/head:pr5784
git checkout pr5784
8:40 a.m.
New subject: [sssd PR#5784][comment] proxy: allow removing group members
URL: https://github.com/SSSD/sssd/pull/5784
Title: #5784: proxy: allow removing group members
ikerexxe commented:
"""
Wouldn't it make sense to instead lookup GID of SYSDB_MEMBEROF DN
and compare GIDs (numerical values)?
Yes, so I've updated the PR accordingly. Nowadays, a list of GIDs is composed from the
cache and this is compared with the actual GIDs list. This should optimize the execution
flow compared with the previous proposed patch.
"""
See the full comment at https://github.com/SSSD/sssd/pull/5784#issuecomment-936256484
8:40 a.m.
New subject: [sssd PR#5784][+Waiting for review] proxy: allow removing group members
URL: https://github.com/SSSD/sssd/pull/5784
Title: #5784: proxy: allow removing group members
Label: +Waiting for review
8:40 a.m.
New subject: [sssd PR#5784][-Changes requested] proxy: allow removing group members
URL: https://github.com/SSSD/sssd/pull/5784
Title: #5784: proxy: allow removing group members
Label: -Changes requested
8:37 a.m.
New subject: [sssd PR#5784][synchronized] proxy: allow removing group members
URL: https://github.com/SSSD/sssd/pull/5784
Author: ikerexxe
Title: #5784: proxy: allow removing group members
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5784/head:pr5784
git checkout pr5784
8:37 a.m.
New subject: [sssd PR#5784][comment] proxy: allow removing group members
URL: https://github.com/SSSD/sssd/pull/5784
Title: #5784: proxy: allow removing group members
ikerexxe commented:
"""
I've updated the patch taking into account your suggestions.
"""
See the full comment at https://github.com/SSSD/sssd/pull/5784#issuecomment-945777100
8:38 a.m.
New subject: [sssd PR#5784][-Changes requested] proxy: allow removing group members
URL: https://github.com/SSSD/sssd/pull/5784
Title: #5784: proxy: allow removing group members
Label: -Changes requested
8:38 a.m.
New subject: [sssd PR#5784][+Waiting for review] proxy: allow removing group members
URL: https://github.com/SSSD/sssd/pull/5784
Title: #5784: proxy: allow removing group members
Label: +Waiting for review
2:11 p.m.
New subject: [sssd PR#5784][+Changes requested] proxy: allow removing group members
URL: https://github.com/SSSD/sssd/pull/5784
Title: #5784: proxy: allow removing group members
Label: +Changes requested
2:11 p.m.
New subject: [sssd PR#5784][-Waiting for review] proxy: allow removing group members
URL: https://github.com/SSSD/sssd/pull/5784
Title: #5784: proxy: allow removing group members
Label: -Waiting for review
2:57 a.m.
New subject: [sssd PR#5784][synchronized] proxy: allow removing group members
URL: https://github.com/SSSD/sssd/pull/5784
Author: ikerexxe
Title: #5784: proxy: allow removing group members
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5784/head:pr5784
git checkout pr5784
9:31 a.m.
New subject: [sssd PR#5784][synchronized] proxy: allow removing group members
URL: https://github.com/SSSD/sssd/pull/5784
Author: ikerexxe
Title: #5784: proxy: allow removing group members
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5784/head:pr5784
git checkout pr5784
9:37 a.m.
New subject: [sssd PR#5784][-Changes requested] proxy: allow removing group members
URL: https://github.com/SSSD/sssd/pull/5784
Title: #5784: proxy: allow removing group members
Label: -Changes requested
9:37 a.m.
New subject: [sssd PR#5784][+Waiting for review] proxy: allow removing group members
URL: https://github.com/SSSD/sssd/pull/5784
Title: #5784: proxy: allow removing group members
Label: +Waiting for review
9:37 a.m.
New subject: [sssd PR#5784][comment] proxy: allow removing group members
URL: https://github.com/SSSD/sssd/pull/5784
Title: #5784: proxy: allow removing group members
ikerexxe commented:
"""
Ready for a new rounds of reviews.
"""
See the full comment at https://github.com/SSSD/sssd/pull/5784#issuecomment-948683748
7:22 a.m.
New subject: [sssd PR#5784][comment] proxy: allow removing group members
URL: https://github.com/SSSD/sssd/pull/5784
Title: #5784: proxy: allow removing group members
alexey-tikhonov commented:
"""
Thank you for changes. ACK from me.
Since I somewhat influenced final solution, would be good to if somebody else took a
look.
"""
See the full comment at https://github.com/SSSD/sssd/pull/5784#issuecomment-950870013
1:14 p.m.
New subject: [sssd PR#5784][comment] proxy: allow removing group members
URL: https://github.com/SSSD/sssd/pull/5784
Title: #5784: proxy: allow removing group members
alexey-tikhonov commented:
"""
Thank you for changes. ACK from me.
Since I somewhat influenced final solution, would be good to if somebody else took a
look.
@sumit-bose , @pbrezina , @thalman , @justin-stephenson , could anyone of you please take
a look?
"""
See the full comment at https://github.com/SSSD/sssd/pull/5784#issuecomment-953189103
6:52 a.m.
New subject: [sssd PR#5784][comment] proxy: allow removing group members
URL: https://github.com/SSSD/sssd/pull/5784
Title: #5784: proxy: allow removing group members
pbrezina commented:
"""
I did not test it, but codewise ack.
"""
See the full comment at https://github.com/SSSD/sssd/pull/5784#issuecomment-954680724
6:52 a.m.
New subject: [sssd PR#5784][-Waiting for review] proxy: allow removing group members
URL: https://github.com/SSSD/sssd/pull/5784
Title: #5784: proxy: allow removing group members
Label: -Waiting for review
6:52 a.m.
New subject: [sssd PR#5784][+Accepted] proxy: allow removing group members
URL: https://github.com/SSSD/sssd/pull/5784
Title: #5784: proxy: allow removing group members
Label: +Accepted
5:16 a.m.
New subject: [sssd PR#5784][+Ready to push] proxy: allow removing group members
URL: https://github.com/SSSD/sssd/pull/5784
Title: #5784: proxy: allow removing group members
Label: +Ready to push
5:17 a.m.
New subject: [sssd PR#5784][comment] proxy: allow removing group members
URL: https://github.com/SSSD/sssd/pull/5784
Title: #5784: proxy: allow removing group members
pbrezina commented:
"""
Pushed PR: https://github.com/SSSD/sssd/pull/5784
* `master`
* 301659a662a7a7aac11096fd0409f83b45cb41d1 - proxy: allow removing group members
"""
See the full comment at https://github.com/SSSD/sssd/pull/5784#issuecomment-956103712
5:17 a.m.
New subject: [sssd PR#5784][+Pushed] proxy: allow removing group members
URL: https://github.com/SSSD/sssd/pull/5784
Title: #5784: proxy: allow removing group members
Label: +Pushed
5:17 a.m.
New subject: [sssd PR#5784][-Accepted] proxy: allow removing group members
URL: https://github.com/SSSD/sssd/pull/5784
Title: #5784: proxy: allow removing group members
Label: -Accepted
5:17 a.m.
New subject: [sssd PR#5784][closed] proxy: allow removing group members
URL: https://github.com/SSSD/sssd/pull/5784
Author: ikerexxe
Title: #5784: proxy: allow removing group members
Action: closed
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5784/head:pr5784
git checkout pr5784
5:17 a.m.
New subject: [sssd PR#5784][-Ready to push] proxy: allow removing group members
URL: https://github.com/SSSD/sssd/pull/5784
Title: #5784: proxy: allow removing group members
Label: -Ready to push
909
days inactive
957
days old
sssd-devel@lists.fedorahosted.org
34 comments
4 participants
participants (4)
-
alexey-tikhonov -
ikerexxe
-
pbrezina
-
sumit-bose