On 28.9.2013 22:01, Alexander Bokovoy wrote:
On Fri, 27 Sep 2013, Sumit Bose wrote:
On Fri, Sep 27, 2013 at 03:53:08PM +0300, Alexander Bokovoy wrote:
On Mon, 23 Sep 2013, Alexander Bokovoy wrote:
>On Mon, 23 Sep 2013, Alexander Bokovoy wrote:
>>On Mon, 23 Sep 2013, Alexander Bokovoy wrote:
>>>On Mon, 23 Sep 2013, Martin Kosek wrote:
>>>>>>However, we don't have trust type available so it needs to
discovered
>>>>>>every time. This doesn't play well with the framework, it is
simply not
>>>>>>expecting dynamic containers.
>>>>>
>>>>>This doesn't sound like a big obstacle to me. Right now the
trust_type lookup
>>>>>is done in trust_show.execute() for some reason, which is not
the best place to
>>>>>do it IMHO. Doing it in trust.get_dn() instead should simplify
things enough to
>>>>>make parent_object work.
>>>>
>>>>Yup, get_dn() is the method where object DN lookup should be
done. See for
>>>>example host.py plugin get_dn method, we also do a dynamic lookup
for correct
>>>>host name.
>>>I'll see if that would work.
>>>
>>>>the best way to implement dynamic DN gathering is the get_dn()
method. That
>>>>way, it could be implemented in one place and all commands could
take advantage
>>>>of it instead of re-implementing it several times in pre_callback
- this is
>>>>just hackish.
>>>I'd suggest you look into the code. The commands use pre_callback
for a
>>>different purpose than implementing dynamic DN gathering.
>>>
>>>>I think it would have been very useful to have a design page
before sending a
>>>>patch. It is then easier to make design decisions without having
to dig into
>>>>the patch.
>>>The design page is there for long time:
>>>http://www.freeipa.org/page/V3/Transitive_Trusts
>>Ok, here is new version of the patch and updated version of my 0117
>>patch as Sumit noticed I've sent wrong version.
>Ok, here is updated 0118 which fixes API.txt change for trustdomain_add
>-- I renamed trustdomain_create to trustdomain_add but forgot to rerun
>makeapi.
New edition attached for all subdomain-related patches:

I did some tests and all is working as expected.


freeipa-abbra-0117-ipaserver-dcerpc.py-populate-forest-trust-informatio-3.patch

  Use realmdomains to report name suffix routes at the time we
establish trust

freeipa-abbra-0118-trusts-support-subdomains-in-a-forest-3.patch
  Introduce trustdomain-* commands to fetch list of domains associated
  with a forest trust and allow filtering them off

We talked on irc that ipaNTSupportedEncryptionTypes in the filter
for the trusted domains should be replace by a different attribute.
Because of an error in ipasam the ipaNTSupportedEncryptionTypes is only
set in recent versions and might not be present in the directory trees of
older versions.
Fixed in the attached patch 0118 version 4.

Also attached first attempt to implement transiting through trusted
domains, as patch 0123. In this patch we grant transition only if all
three realms (client, transited realm, and server realm) match any of
our trusted domains and our domain. This is probably a bit wider but it
worked for me bidirectionally, from a child domain to a service in IPA,
and from IPA realm to a service in a child domain of a forest trust.


+        if not trust_type:
+            for ttype in self.trust_types:
+                dn=make_trust_dn(self.env, ttype, DN(*sdn))
+                try:
+                    object_exists = self.backend.get_entry(dn, [''])
+                    return object_exists.dn
+                except errors.NotFound:
+                    pass
+            # if no trust object of any type exist, default to 'ad'
+            # this is required for *_add calls.
+            trust_type = u'ad'

This could be optimized to do a single LDAP search, i.e. something like:

        if trust_type is None:
            ldap = self.backend
filter = ldap.make_filter({'objectclass': ['ipaNTTrustedDomain'], 'cn': [keys[-1]]})
            try:
result = ldap.get_entries(base_dn, ldap.SCOPE_SUBTREE, filter, [''])
            except errors.NotFound:
                trust_type = u'ad'
            else:
                if len(result) > 1:
                    raise errors.OnlyOneValueAllowed(attr='trust_type')
                return result[0].dn


+        # First see if the trust is already in place
+        # Force retrieval of the trust object by not passing trust_type
+        dn = self.obj.get_dn(keys[-1], {})

The "{}" does not seem right to me (you are passing it as a second key to get_dn, which is probably not what you want).


             (dn, attrs) = entry
-
+
             # Translate ipanttrusttype to trusttype if --raw not used

No trailing whitespace please.


+    takes_options = LDAPCreate.takes_options + (_trust_type_option,
+        Str('ipanttrustpartner?',
+            label=_('Trusted domain partner'),
+        ),
+    )
+
+ def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
+        if 'ipanttrustpartner' in options:
+ entry_attrs['ipanttrustpartner'] = [options['ipanttrustpartner']]
+        return dn

Why is ipanttrustpartner a trustdomain_add option rather than trustdomain virtual attribute?


+class trustdomain_fetch(LDAPRetrieve):

I know I caused some confusion here, but I *do* think that this single command should be in the trust namespace, as it is actually method of the trust object (it takes a trust object name argument and creates trustdomain objects for all the subdomains in the trust, right?) I would suggest "trust_domain_fetch" or "trust_fetch_domains".


+class trustdomain_filter(LDAPRetrieve):

It seems this command toggles the blacklist state of a trustdomain object. AFAIK we don't have any toggle command in IPA and I am opposed to the idea of introducing them - it's better to be explicit than implicit. I think two commands would be better for this ("trustdomain_enable" and "trustdomain_disable" perhaps?)


Honza

--
Jan Cholasta

_______________________________________________
Freeipa-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to