Hello,
please see attached patches.
I'm not sure that the 2nd patch is needed to fulfill the ticket requirements. (https://fedorahosted.org/sssd/ticket/2308)
Pavel Reichl
On Thu, Apr 10, 2014 at 05:47:33PM +0200, Pavel Reichl wrote:
Hello,
please see attached patches.
I'm not sure that the 2nd patch is needed to fulfill the ticket requirements. (https://fedorahosted.org/sssd/ticket/2308)
Pavel Reichl
From 31e951439e1f2215adb64e4409c717ac759e397c Mon Sep 17 00:00:00 2001 From: Pavel Reichl preichl@redhat.com Date: Thu, 10 Apr 2014 16:25:45 +0100 Subject: [PATCH 1/2] MAN: hint nested groups by simple access provider
sssd-ldap hints to use the simple access provider if a nested group membership is needed.
Resolves: https://fedorahosted.org/sssd/ticket/2308
src/man/sssd-ldap.5.xml | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/src/man/sssd-ldap.5.xml b/src/man/sssd-ldap.5.xml index f93b418c45d9bcd32499860a858c3f829bb245ca..8b41a67c7728851480cd0631a9ad9239f1e320a9 100644 --- a/src/man/sssd-ldap.5.xml +++ b/src/man/sssd-ldap.5.xml @@ -1799,6 +1799,15 @@ ldap_access_filter = (employeeType=admin) vice-versa. </para> <para>
Please note that the nested group membership is notsupported by the ldap access provider, however, itis supported by the simple access provider. See<citerefentry><refentrytitle>sssd-simple</refentrytitle><manvolnum>5</manvolnum></citerefentry> manualpage for more information about the simple accessprovider.</para><para> Default: Empty </para> </listitem>
This is not accurate. The root of the problem is not nested group membership not being supported, but the fact that the filter is applied on the user entry. The user entry would contain the group memberships in the 'memberof' attribute, but typically (notably in AD), the memberof attribute only points to the direct parents. Some servers (IPA) include also the parent groups in the memberof attribute, so the ldap_access_filter would work OK for those.
I think we could just expand on the last sentence in the opening paragraph (currently it says "...applied on the LDAP user entry only).
-- 1.8.4.2
From 417bedfdfb47127cdeb6bd17c91401cabdf3132b Mon Sep 17 00:00:00 2001 From: Pavel Reichl preichl@redhat.com Date: Thu, 10 Apr 2014 16:30:03 +0100 Subject: [PATCH 2/2] MAN: sssd-simple - nested group membership support
Explicit notice in sssd-simple about support of nested group membership.
Resolves: https://fedorahosted.org/sssd/ticket/2308
src/man/sssd-simple.5.xml | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/src/man/sssd-simple.5.xml b/src/man/sssd-simple.5.xml index 8f94990da9d94dca2f6b5730aaab6b4468fed487..4843e12da6900fe6c784e36d4001d319d42c749c 100644 --- a/src/man/sssd-simple.5.xml +++ b/src/man/sssd-simple.5.xml @@ -144,6 +144,13 @@ </para> </refsect1>
<refsect1 id='notes'>
<title>NOTES</title><para>The simple access provider supports a nested group membership.</para></refsect1>
- <xi:include xmlns:xi="http://www.w3.org/2001/XInclude" href="include/seealso.xml" />
</refentry> -- 1.8.4.2
Maybe we should say a bit more, something like "the complete group memberships are resolved before the access check, so even nested groups can be included in the access lists" ?
OK, please see updated patch. PR
On Fri, 2014-04-11 at 12:03 +0200, Jakub Hrozek wrote:
On Thu, Apr 10, 2014 at 05:47:33PM +0200, Pavel Reichl wrote:
Hello,
please see attached patches.
I'm not sure that the 2nd patch is needed to fulfill the ticket requirements. (https://fedorahosted.org/sssd/ticket/2308)
Pavel Reichl
From 31e951439e1f2215adb64e4409c717ac759e397c Mon Sep 17 00:00:00 2001 From: Pavel Reichl preichl@redhat.com Date: Thu, 10 Apr 2014 16:25:45 +0100 Subject: [PATCH 1/2] MAN: hint nested groups by simple access provider
sssd-ldap hints to use the simple access provider if a nested group membership is needed.
Resolves: https://fedorahosted.org/sssd/ticket/2308
src/man/sssd-ldap.5.xml | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/src/man/sssd-ldap.5.xml b/src/man/sssd-ldap.5.xml index f93b418c45d9bcd32499860a858c3f829bb245ca..8b41a67c7728851480cd0631a9ad9239f1e320a9 100644 --- a/src/man/sssd-ldap.5.xml +++ b/src/man/sssd-ldap.5.xml @@ -1799,6 +1799,15 @@ ldap_access_filter = (employeeType=admin) vice-versa. </para> <para>
Please note that the nested group membership is notsupported by the ldap access provider, however, itis supported by the simple access provider. See<citerefentry><refentrytitle>sssd-simple</refentrytitle><manvolnum>5</manvolnum></citerefentry> manualpage for more information about the simple accessprovider.</para><para> Default: Empty </para> </listitem>This is not accurate. The root of the problem is not nested group membership not being supported, but the fact that the filter is applied on the user entry. The user entry would contain the group memberships in the 'memberof' attribute, but typically (notably in AD), the memberof attribute only points to the direct parents. Some servers (IPA) include also the parent groups in the memberof attribute, so the ldap_access_filter would work OK for those.
I think we could just expand on the last sentence in the opening paragraph (currently it says "...applied on the LDAP user entry only).
-- 1.8.4.2
From 417bedfdfb47127cdeb6bd17c91401cabdf3132b Mon Sep 17 00:00:00 2001 From: Pavel Reichl preichl@redhat.com Date: Thu, 10 Apr 2014 16:30:03 +0100 Subject: [PATCH 2/2] MAN: sssd-simple - nested group membership support
Explicit notice in sssd-simple about support of nested group membership.
Resolves: https://fedorahosted.org/sssd/ticket/2308
src/man/sssd-simple.5.xml | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/src/man/sssd-simple.5.xml b/src/man/sssd-simple.5.xml index 8f94990da9d94dca2f6b5730aaab6b4468fed487..4843e12da6900fe6c784e36d4001d319d42c749c 100644 --- a/src/man/sssd-simple.5.xml +++ b/src/man/sssd-simple.5.xml @@ -144,6 +144,13 @@ </para> </refsect1>
<refsect1 id='notes'>
<title>NOTES</title><para>The simple access provider supports a nested group membership.</para></refsect1>
- <xi:include xmlns:xi="http://www.w3.org/2001/XInclude" href="include/seealso.xml" />
</refentry> -- 1.8.4.2
Maybe we should say a bit more, something like "the complete group memberships are resolved before the access check, so even nested groups can be included in the access lists" ? _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
On Fri, May 30, 2014 at 06:04:01PM +0200, Pavel Reichl wrote:
OK, please see updated patch. PR
Thank you, from content point of view, this looks correct to me.
Can you please ping some native English speaker before I push the patch? There are two parts below that I'm not sure about, but as I said, it's not about content, just ironing out the language.
[snip]
@@ -1776,7 +1776,14 @@ users being denied access. Use access_provider = permit to change this default behavior. Please note that this filter is applied on
the LDAP user entry only.
the LDAP user entry only and thus filtering basedon nested groups may not work (e.g. memberOfattribute on AD entries points only on directparents). If nested group based filtering isdesired please see
Maybe required instead of desired?
<citerefentry><refentrytitle>sssd-simple</refentrytitle><manvolnum>5</manvolnum></citerefentry>. </para> <para> Example:diff --git a/src/man/sssd-simple.5.xml b/src/man/sssd-simple.5.xml index 8f94990da9d94dca2f6b5730aaab6b4468fed487..5a0af337e3a45175aaa7a1a36fae5c1da2ead0c4 100644 --- a/src/man/sssd-simple.5.xml +++ b/src/man/sssd-simple.5.xml @@ -144,6 +144,18 @@ </para> </refsect1>
<refsect1 id='notes'>
<title>NOTES</title><para>The complete group memberships are resolved before the access check,
I'm not sure if "group memberships are" should read "group membership is", iow if it's better to use singular or plural here..
so even nested groups can be included in the access lists. Please beaware of ldap_group_nesting_level(<citerefentry><refentrytitle>sssd-ldap</refentrytitle><manvolnum>5</manvolnum></citerefentry>) option.</para></refsect1>
- <xi:include xmlns:xi="http://www.w3.org/2001/XInclude" href="include/seealso.xml" />
</refentry> -- 1.8.4.2
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 06/01/2014 04:09 PM, Jakub Hrozek wrote:
On Fri, May 30, 2014 at 06:04:01PM +0200, Pavel Reichl wrote:
OK, please see updated patch. PR
Thank you, from content point of view, this looks correct to me.
Can you please ping some native English speaker before I push the patch? There are two parts below that I'm not sure about, but as I said, it's not about content, just ironing out the language.
[snip]
@@ -1776,7 +1776,14 @@ users being denied access. Use access_provider = permit to change this default behavior. Please note that this filter is applied on - the LDAP user entry only. + the LDAP user entry only and thus filtering based + on nested groups may not work (e.g. memberOf + attribute on AD entries points only on direct
"points only *to* direct parents"
parents). If nested group basedfiltering is + desired please see
"If filtering based on nested groups is required, please see"
Maybe required instead of desired?
<citerefentry> +<refentrytitle>sssd-simple</refentrytitle><manvolnum>5</manvolnum>
+ </citerefentry>.
</para> <para> Example: diff --git a/src/man/sssd-simple.5.xml b/src/man/sssd-simple.5.xml index 8f94990da9d94dca2f6b5730aaab6b4468fed487..5a0af337e3a45175aaa7a1a36fae5c1da2ead0c4 100644 --- a/src/man/sssd-simple.5.xml +++ b/src/man/sssd-simple.5.xml @@ -144,6 +144,18 @@ </para>
</refsect1>
- <refsect1 id='notes'> + <title>NOTES</title> +
<para> + The complete group memberships are resolved before the access check,
I'm not sure if "group memberships are" should read "group membership is", iow if it's better to use singular or plural here..
"The complete group membership hierarchy is resolved before the access check, thus even nested groups..."
so even nested groups can be included in the accesslists. Please be + aware of ldap_group_nesting_level
"Please be aware that the 'ldap_group_nesting_level' option may impact the results and should be set to a sufficient value."
(<citerefentry> +<refentrytitle>sssd-ldap</refentrytitle><manvolnum>5</manvolnum>
</citerefentry>) option. + </para> +</refsect1> + <xi:include xmlns:xi="http://www.w3.org/2001/XInclude" href="include/seealso.xml" />
</refentry> -- 1.8.4.2
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
On Mon, Jun 02, 2014 at 08:22:14AM -0400, Stephen Gallagher wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 06/01/2014 04:09 PM, Jakub Hrozek wrote:
On Fri, May 30, 2014 at 06:04:01PM +0200, Pavel Reichl wrote:
OK, please see updated patch. PR
Thank you, from content point of view, this looks correct to me.
Can you please ping some native English speaker before I push the patch? There are two parts below that I'm not sure about, but as I said, it's not about content, just ironing out the language.
[snip]
@@ -1776,7 +1776,14 @@ users being denied access. Use access_provider = permit to change this default behavior. Please note that this filter is applied on - the LDAP user entry only. + the LDAP user entry only and thus filtering based + on nested groups may not work (e.g. memberOf + attribute on AD entries points only on direct
"points only *to* direct parents"
parents). If nested group basedfiltering is + desired please see
"If filtering based on nested groups is required, please see"
Maybe required instead of desired?
<citerefentry> +<refentrytitle>sssd-simple</refentrytitle><manvolnum>5</manvolnum>
</citerefentry>.</para> <para> Example: diff --git a/src/man/sssd-simple.5.xml b/src/man/sssd-simple.5.xml index 8f94990da9d94dca2f6b5730aaab6b4468fed487..5a0af337e3a45175aaa7a1a36fae5c1da2ead0c4 100644 --- a/src/man/sssd-simple.5.xml +++ b/src/man/sssd-simple.5.xml @@ -144,6 +144,18 @@ </para>
</refsect1>
- <refsect1 id='notes'> + <title>NOTES</title> +
<para> + The complete group memberships are resolved before the access check,
I'm not sure if "group memberships are" should read "group membership is", iow if it's better to use singular or plural here..
"The complete group membership hierarchy is resolved before the access check, thus even nested groups..."
so even nested groups can be included in the accesslists. Please be + aware of ldap_group_nesting_level
"Please be aware that the 'ldap_group_nesting_level' option may impact the results and should be set to a sufficient value."
Thanks for the review!
Pavel is not around today, so I took the liberty of updating the patch with your suggestions so we can move forward.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On Mon 02 Jun 2014 09:13:32 AM EDT, Jakub Hrozek wrote:
On Mon, Jun 02, 2014 at 08:22:14AM -0400, Stephen Gallagher wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 06/01/2014 04:09 PM, Jakub Hrozek wrote:
On Fri, May 30, 2014 at 06:04:01PM +0200, Pavel Reichl wrote:
OK, please see updated patch. PR
Thank you, from content point of view, this looks correct to me.
Can you please ping some native English speaker before I push the patch? There are two parts below that I'm not sure about, but as I said, it's not about content, just ironing out the language.
[snip]
@@ -1776,7 +1776,14 @@ users being denied access. Use access_provider = permit to change this default behavior. Please note that this filter is applied on - the LDAP user entry only. + the LDAP user entry only and thus filtering based + on nested groups may not work (e.g. memberOf + attribute on AD entries points only on direct
"points only *to* direct parents"
parents). If nested group basedfiltering is + desired please see
"If filtering based on nested groups is required, please see"
Maybe required instead of desired?
<citerefentry> +<refentrytitle>sssd-simple</refentrytitle><manvolnum>5</manvolnum>
</citerefentry>.</para> <para> Example: diff --git a/src/man/sssd-simple.5.xml b/src/man/sssd-simple.5.xml index 8f94990da9d94dca2f6b5730aaab6b4468fed487..5a0af337e3a45175aaa7a1a36fae5c1da2ead0c4 100644 --- a/src/man/sssd-simple.5.xml +++ b/src/man/sssd-simple.5.xml @@ -144,6 +144,18 @@ </para>
</refsect1>
- <refsect1 id='notes'> + <title>NOTES</title> +
<para> + The complete group memberships are resolved before the access check,
I'm not sure if "group memberships are" should read "group membership is", iow if it's better to use singular or plural here..
"The complete group membership hierarchy is resolved before the access check, thus even nested groups..."
so even nested groups can be included in the accesslists. Please be + aware of ldap_group_nesting_level
"Please be aware that the 'ldap_group_nesting_level' option may impact the results and should be set to a sufficient value."
Thanks for the review!
Pavel is not around today, so I took the liberty of updating the patch with your suggestions so we can move forward.
Ack
sssd-devel@lists.fedorahosted.org
-
Jakub Hrozek -
Pavel Reichl -
Stephen Gallagher