On 06/28/2012 01:09 PM, Martin Kosek wrote:
> On 06/28/2012 12:19 PM, Sumit Bose wrote:
>> On Thu, Jun 28, 2012 at 09:52:14AM +0200, Martin Kosek wrote:
>>> On 06/27/2012 06:38 PM, Alexander Bokovoy wrote:
>>>> On Wed, 27 Jun 2012, Sumit Bose wrote:
>>>>> On Wed, Jun 13, 2012 at 12:37:49PM +0200, Sumit Bose wrote:
>>>>>> On Wed, Jun 13, 2012 at 12:26:43PM +0200, Sumit Bose wrote:
>>>>>>> On Mon, Jun 11, 2012 at 05:46:17PM +0300, Alexander Bokovoy wrote:
>>>>>>>> On Thu, 07 Jun 2012, Sumit Bose wrote:
>>>>>>>>> On Fri, Mar 23, 2012 at 01:52:34PM +0100, Sumit Bose wrote:
>>>>>>>>>> Hi,
>>>>>>>>>> these two patches introduce a new extended operation to the IPA 
>>>>>>>>>> server
>>>>>>>>>> which can be used by clients in the IPA domain to obtain information
>>>>>>>>>> about users and groups from trusted domains. Currently this exop is 
>>>>>>>>>> used
>>>>>>>>>> by the sssd sub-domain patch to map user names from a trusted AD 
>>>>>>>>>> domain
>>>>>>>>>> to a SID and back. There is also some code for other kind of requests
>>>>>>>>>> which might become useful in future, e.g. with trusted IPA domain.
>>>>>>>>>> I added some unit test and added check for the check unit test 
>>>>>>>>>> framework
>>>>>>>>>> for C (http://check.sourceforge.net/) which is used by sssd as well. 
>>>>>>>>>> I
>>>>>>>>>> modified the spec file that the test is run during the build of the
>>>>>>>>>> packages. I hope this is ok.
>>>>>>>>>> The patches depend on the idmap library patch which was ACKed 
>>>>>>>>>> recently
>>>>>>>>>> on sssd-devel and as mentioned before the sub-domain patches on
>>>>>>>>>> sssd-devel can only be fully tested with an IPA server which has 
>>>>>>>>>> these
>>>>>>>>>> patches applied.
>>>>>>>>>> Since Alexander is currently rewriting parts of the 
>>>>>>>>>> ipa-adtrust-install
>>>>>>>>>> utility I stand back from adding activation code for the exop to
>>>>>>>>>> ipa-adtrust-install and will send a patch when Alexander's changes 
>>>>>>>>>> are
>>>>>>>>>> available. So currently extdom-extop-conf.ldif has to be loaded 
>>>>>>>>>> manually
>>>>>>>>>> after replacing $SUFFIX to activate the new exop.
>>>>>>>>>> bye,
>>>>>>>>>> Sumit
>>>>>>>>> Please find a rebased version of the patches which work on top of
>>>>>>>>> Alexander's latest series of patches. The patches now also contain the
>>>>>>>>> loading of extdom-extop-conf.ldif and the activation of winbind.
>>>>>>>> Thanks for the rebase.
>>>>>>>> Few comments.
>>>>>>>> 1.The extdom plugin should support IDMAP_BOTH. We do provide user 
>>>>>>>> private
>>>>>>>> groups so in our case it should be viewed as preferred output. Thus you
>>>>>>>> would need to add new response type to cover this case.
>>>>>>> Currently the plugin only uses winbind to map SIDs to names and back and
>>>>>>> in the returned user data the user private groups are already respected
>>>>>>> by setting the GID to the UID. On the client side sssd handles the
>>>>>>> trusted domains a mpg (magic private group) domains.
>>>>>>>> 2. I have tried to look at the plugin description from point of view of
>>>>>>>> a system administrator and I failed to understand what it does:
>>>>>>>>> +#define IPA_EXTDOM_PLUGIN_NAME   "ipa-extdom-extop"
>>>>>>>>> +#define IPA_EXTDOM_FEATURE_DESC  "IPA EXTDOM ID mapper"
>>>>>>>>> +#define IPA_EXTDOM_PLUGIN_DESC   "IPA EXTDOM ID mapper Extended
>>>>>> Operation plugin"
>>>>>>>> In the ipa-extdom-extop-conf.ldif you have following description:
>>>>>>>>> +nsslapd-plugindescription: Support resolving IDs in trusted domains 
>>>>>>>>> to
>>>>>> names and back
>>>>>>>> Probably it is better to reuse the same description in
>>>>>>>> This is a minor point but EXTDOM itself is vague. Maybe we should be
>>>>>> more clear
>>>>>>>> and call it 'IPA trusted domain ID mapper' as it really limits itself 
>>>>>>>> to
>>>>>>>> only trusted domains? We don't dispatch winbind request if the domain 
>>>>>>>> is
>>>>>>>> not found in our list of trusted domains.
>>>>>>> I have updated the descriptions. I prefer the EXTDOM prefix because
>>>>>>> there might be future use cases where we might want to get some data
>>>>>>> from other domains without trust. But I'm happy to change it if you like
>>>>>>> a different prefix better.
>>>>>>>> 3. Could you please define the oid in ipa_extdom.h so that it could be
>>>>>>>> useful for client code as well?
>>>>>>>>> +#define EXOP_EXTDOM_OID "2.16.840.1.113730."
>>>>>>> done
>>>>>>> New version attached.
>>>>>> ah. sorry, forgot to squash in some changes.
>>>>>> Additionally I moved the binary to the freeipa-server-trust-ad package
>>>>>> to avoid additional dependencies in the freeipa-server package.
>>>>>> bye,
>>>>>> Sumit
>>>>>>>> 4. Do we have 'check' tool in RHEL6?
>>>>>>> yes, current version is check-0.9.8-1.1.el6
>>>>>>> Thank you for the review.
>>>>>>> bye,
>>>>>>> Sumit
>>>>>>>> --
>>>>>>>> / Alexander Bokovoy
>>>>> rebased version with some changes by Alexander attached.
>>>> ACK from my side. Works in tests I've run.
>>> Patch 17 pushed to master.
>>> Patch 18 does not apply. I also have one question related to this patch:
>> a rebased version is attached.
>>> We added a winbind service to ADTRUSTInstance which is now being configured 
>>> as
>>> a part of ipa-adtrust-install. To make this cleaner, we may want to write
>>> winbind's own service.Service class which would do the necessary 
>>> configuration
>>> and could be also better expanded in the future.
>> Currently none of the configuration steps are done exclusively for
>> winbind, e.g. winbind will use the same credential as the smbd to access
>> the directory server. I would agree to create an class for winbind if it
>> turns out that we have to add special winbind options, but for now we
>> only need to start the winbind process.
> Ok, lets keep current setup and expand when needed. I also did few 
> installation
> tests with your patch, everything worked fine.
> So ACK #2, pushed to master.
> Martin

There was a missing Requires for libsss_idmap on freeipa-server-trust-ad 

Fixed and pushed as a one-liner (attached).

>From 28623cc8bb69c95e87b46f7667d8d7aa0b6e181f Mon Sep 17 00:00:00 2001
From: Martin Kosek <mko...@redhat.com>
Date: Thu, 28 Jun 2012 13:47:27 +0200
Subject: [PATCH] Add missing libsss_idmap Requires on freeipa-server-trust-ad

 freeipa.spec.in |    1 +
 1 file changed, 1 insertion(+)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 72ac0e7a2ad139aba1fd7b4ecc94f961a6743ab4..f7b115202bc8086ba26b25fbe1848fb4ad1fec2a 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -206,6 +206,7 @@ Requires: %{name}-server = %version-%release
 Requires: python-crypto
 Requires: samba4-python
 Requires: samba4
+Requires: libsss_idmap
 %description server-trust-ad
 Cross-realm trusts with Active Directory in IPA require working Samba 4 installation.

Freeipa-devel mailing list

Reply via email to