On 07/28/2016 12:51 PM, Martin Babinsky wrote:
On 07/27/2016 11:54 AM, Milan Kubík wrote:
Hi Milan,
the tests seem to work as expected except
`test_enterprise_principal_UPN_overlap_without_additional_suffix`
which crashes on #6099. I have a few comments, however:
This is a test that hits a known bug. I have added an expected fail
marker for it.
Patch 42:
1.)
+class KerberosAliasMixin:
make sure the class inherits from 'object' so that it behaves like
new-style class in Python 2.
Also, I think that 'check_for_tracker' method is slightly redundant.
If somebody would use the mixin directly, then he will still get
"NotImplementedError" exceptions and see that he probably did
something wrong.
If you really really want to treat to prevent the instantiation of the
class, then use ABC module to turn it into proper abstract class.
But in this case it should IMHO be enough that you explained the class
usage in the module docstring.
Ok. Fixed the inheritance and removed the check for Tracker. The check
for krbprincipalname attribute has been kept.
2.)
+ def _make_add_alias_cmd(self):
+ raise RuntimeError("The _make_add_alias_cmd method "
+ "is not implemented.")
+
+ def _make_remove_alias_cmd(self):
+ raise RuntimeError("The _make_remove_alias_cmd method "
+ "is not implemented."
Abstract methods should raise "NotImplementedError" exception.
Fixed.
3.)
is the 'options=None' kwarg in {add,remove}_principals methods
necessary? I didn't see it anywhere in the later commits so I guess
you can safely remove it. Better yet, you can just replace it with
'**options' since all it does is to pass options to the
`*-add/remove-principal` commands.
Fixed to **options.
4.)
a nitpick but IIRC the mixin class should be listed before the actual
hierarchy base so that MRO works as expected.
So instead
+class UserTracker(Tracker, KerberosAliasMixin):
It should say
+class UserTracker(KerberosAliasMixin, Tracker):
Fixed in all three classes.
PATCH 43: LGTM
PATCH 44:
Please do not create any more utility modules. Either put it to
existing 'ipatests/util.py' or better yet, rename the module to
something like 'mock_trust' since the scope of it is to provide mocked
trust entries.
Moved the functions from test_range_plugin.py module to a new mock_trust
module. The fix for the range plugin test introduced a new commit here.
PATCH 45:
It would be nice if you could add '-E' option in the same way to
indicate enterprise principal name resolution.
Done.
PATCH 46: LGTM
PATCH 47:
1.)
+from ipatests.test_xmlrpc.test_range_plugin import (
+ get_trust_dn, get_trusted_dom_dict, encode_mockldap_value)
Since you already introduced a module grouping all trust-mocking
machinery in patch 44, you could extract these functions and put it
there to improve code reuse.
Fixed.
2.)
I am not a big fan of duplicate code in 'trusted_domain' and
'trusted_domain_with_suffix' fixtures. Use some module-level mapping
for common attributes and add more specific attributes per fixture.
Fixed
3.)
I would like to expand the test cases for AD realm namespace overlap
checks. We should reject enterprise principals with suffixes
coinciding with realm names, NETBIOS names, and UPN suffixes and I
would like to test all three cases to get a full coverage.
Extended with explicit checks fot rhe AD realm and NETBIOS name by
constructing the enterprise principal from corresponding LDAP
attributes.
The fixed and rebased version of the commits is in my repo [1].
[1]: https://github.com/apophys/freeipa/tree/krb5-principal-aliases-test
Regards
Tests works and code is ok, however you will need to create a separate
ticket to your patches, since it is not allowed to add fixes to
tickets in closed milestones. Sorry that I didn't notice it earlier.
cond-ACK if you create ticket and remove the xfail from
`test_enterprise_principal_overlap_with_AD_realm` test case as the fix
for #6099 was pushed to master.
Hi,
thanks for the review. The xfail marker was removed. The commit messages
now reffer to new ticket [1].
Since the changes during review introduced new commit in the sequence, I
have abandoned the patches 45 to 47 and renumbered them (with the new
one) from 48 onwards.
[1]: https://fedorahosted.org/freeipa/ticket/6142
Regards
--
Milan Kubik
From 0a56dc99aa1243e525e8c0fdd8ef217077d23ab1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= <mku...@redhat.com>
Date: Fri, 22 Jul 2016 17:07:56 +0200
Subject: [PATCH 1/7] ipatests: Add tracker class for kerberos principal
aliases
The commit implements a mixin class providing capability
to track and modify kerberos principal aliases on supported
types of entries.
The class using the mixin must inherit from the Tracker class
and must provide the implementation of two methods:
* _make_add_alias_cmd
* _make_remove_alias_cmd
These are used to get the type specific command for the particular
entry class. The methods provided will not work on entries that
do not have 'krbprincipalname' attribute.
The service, host and user trackers are being extended to use this
new mixin class.
https://fedorahosted.org/freeipa/ticket/6142
---
ipatests/test_xmlrpc/tracker/host_plugin.py | 10 ++-
ipatests/test_xmlrpc/tracker/kerberos_aliases.py | 99 ++++++++++++++++++++++++
ipatests/test_xmlrpc/tracker/service_plugin.py | 17 +++-
ipatests/test_xmlrpc/tracker/user_plugin.py | 10 ++-
4 files changed, 130 insertions(+), 6 deletions(-)
create mode 100644 ipatests/test_xmlrpc/tracker/kerberos_aliases.py
diff --git a/ipatests/test_xmlrpc/tracker/host_plugin.py b/ipatests/test_xmlrpc/tracker/host_plugin.py
index 03113b8fe7de7bfa2d2383b1903ff05d2543a9ed..45be169e0c9567f8c7d7a73f2eb8155e9c3d6cfc 100644
--- a/ipatests/test_xmlrpc/tracker/host_plugin.py
+++ b/ipatests/test_xmlrpc/tracker/host_plugin.py
@@ -7,13 +7,14 @@ from __future__ import print_function
from ipapython.dn import DN
from ipatests.test_xmlrpc.tracker.base import Tracker
+from ipatests.test_xmlrpc.tracker.kerberos_aliases import KerberosAliasMixin
from ipatests.test_xmlrpc.xmlrpc_test import fuzzy_uuid
from ipatests.test_xmlrpc import objectclasses
from ipatests.util import assert_deepequal
from ipalib import errors
-class HostTracker(Tracker):
+class HostTracker(KerberosAliasMixin, Tracker):
"""Wraps and tracks modifications to a Host object
Implements the helper functions for host plugin.
@@ -175,3 +176,10 @@ class HostTracker(Tracker):
pass
request.addfinalizer(cleanup)
+
+ # Kerberos aliases methods
+ def _make_add_alias_cmd(self):
+ return self.make_command('host_add_principal', self.name)
+
+ def _make_remove_alias_cmd(self):
+ return self.make_command('host_remove_principal', self.name)
diff --git a/ipatests/test_xmlrpc/tracker/kerberos_aliases.py b/ipatests/test_xmlrpc/tracker/kerberos_aliases.py
new file mode 100644
index 0000000000000000000000000000000000000000..da7b04fa454b746a4ffffaac38c6f3e62e41935c
--- /dev/null
+++ b/ipatests/test_xmlrpc/tracker/kerberos_aliases.py
@@ -0,0 +1,99 @@
+#
+# Copyright (C) 2016 FreeIPA Contributors see COPYING for license
+#
+"""kerberos_aliases
+
+The module implements a mixin class that provides an interface
+to the Kerberos Aliases feature of freeIPA.
+
+In order to use the class the child class must implement the
+`_make_add_alias_cmd` and `_make_remove_alias_cmd` methods that
+are different for each entity type.
+
+The KerberosAliasMixin class then provides the implementation
+of the manipulation of the kerberos alias in general.
+
+It is up to the child class or the user to validate the
+alias being added for a particular type of an entry.
+"""
+
+
+class KerberosAliasError(Exception):
+ pass
+
+
+class KerberosAliasMixin(object):
+ """KerberosAliasMixin"""
+
+ def _make_add_alias_cmd(self):
+ raise NotImplementedError("The _make_add_alias_cmd method "
+ "is not implemented.")
+
+ def _make_remove_alias_cmd(self):
+ raise NotImplementedError("The _make_remove_alias_cmd method "
+ "is not implemented.")
+
+ def _check_for_krbprincipalname_attr(self):
+ # Check if the tracker has a principal name
+ # Each compatible entry has at least one kerberos
+ # principal matching the canonical principal name
+ principals = self.attrs.get('krbprincipalname')
+ if self.exists:
+ if not principals:
+ raise KerberosAliasError(
+ "{} doesn't have krbprincipalname attribute"
+ .format(self.__class__.__name__))
+ else:
+ raise ValueError("The entry {} doesn't seem to exist"
+ .format(self.name))
+
+ def _normalize_principal_list(self, principal_list):
+ """Normalize the list for further manipulation."""
+ if not isinstance(principal_list, (list, tuple)):
+ return [principal_list]
+ else:
+ return principal_list
+
+ def _normalize_principal_value(self, principal):
+ """Normalize principal value by appending the realm string."""
+ return u'@'.join((principal, self.api.env.realm))
+
+ def add_principal(self, principal_list, **options):
+ """Add kerberos principal alias to the entity.
+
+ Add principal alias to the underlying entry and
+ update the attributes in the Tracker instance.
+ """
+ self._check_for_krbprincipalname_attr()
+
+ principal_list = self._normalize_principal_list(principal_list)
+
+ cmd = self._make_add_alias_cmd()
+ cmd(principal_list, **options)
+
+ tracker_principals = self.attrs.get('krbprincipalname')
+ tracker_principals.extend((
+ self._normalize_principal_value(item) for item in principal_list))
+
+ def remove_principal(self, principal_list, **options):
+ """Remove kerberos principal alias from an entry.
+
+ Remove principal alias from the tracked entry.
+ """
+ self._check_for_krbprincipalname_attr()
+
+ principal_list = self._normalize_principal_list(principal_list)
+
+ cmd = self._make_remove_alias_cmd()
+ cmd(principal_list, **options)
+
+ # Make a copy of the list so the tracker instance is not modified
+ # if there is an error deleting the aliases
+ # This can happen when deleting multiple aliases and at least
+ # one of them doesn't exist, raising ValueError
+ tracker_principals = self.attrs.get('krbprincipalname')[:]
+
+ for item in principal_list:
+ tracker_principals.remove(self._normalize_principal_value(item))
+
+ self.attrs['krbprincipalname'] = tracker_principals
diff --git a/ipatests/test_xmlrpc/tracker/service_plugin.py b/ipatests/test_xmlrpc/tracker/service_plugin.py
index 89c27e82161b70a77ac4e45ffd8b60d3da657163..e51232f8056908ef298c62db158c128e1d7436b4 100644
--- a/ipatests/test_xmlrpc/tracker/service_plugin.py
+++ b/ipatests/test_xmlrpc/tracker/service_plugin.py
@@ -7,6 +7,7 @@ import six
from ipalib import api
from ipatests.test_xmlrpc.tracker.base import Tracker
+from ipatests.test_xmlrpc.tracker.kerberos_aliases import KerberosAliasMixin
from ipatests.test_xmlrpc.xmlrpc_test import fuzzy_uuid
from ipatests.test_xmlrpc import objectclasses
from ipatests.util import assert_deepequal
@@ -16,7 +17,7 @@ if six.PY3:
unicode = str
-class ServiceTracker(Tracker):
+class ServiceTracker(KerberosAliasMixin, Tracker):
"""
Tracker class for service plugin
@@ -49,14 +50,14 @@ class ServiceTracker(Tracker):
u'usercertificate', u'has_keytab'}
update_keys = retrieve_keys - {u'dn'}
- def __init__(self, name, host_fqdn, options):
+ def __init__(self, name, host_fqdn, options=None):
super(ServiceTracker, self).__init__(default_version=None)
self._name = "{0}/{1}@{2}".format(name, host_fqdn, api.env.realm)
self.dn = DN(
('krbprincipalname', self.name), api.env.container_service,
api.env.basedn)
self.host_fqdn = host_fqdn
- self.options = options
+ self.options = options or {}
@property
def name(self):
@@ -92,7 +93,8 @@ class ServiceTracker(Tracker):
u'objectclass': objectclasses.service,
u'ipauniqueid': [fuzzy_uuid],
u'managedby_host': [self.host_fqdn],
- u'krbcanonicalname': [u'{0}'.format(self.name)]
+ u'krbcanonicalname': [u'{0}'.format(self.name)],
+ u'has_keytab': False
}
for key in self.options:
@@ -150,3 +152,10 @@ class ServiceTracker(Tracker):
u'summary': u'Modified service "{0}"'.format(self.name),
u'result': self.filter_attrs(self.update_keys | set(extra_keys))
}, result)
+
+ # Kerberos aliases methods
+ def _make_add_alias_cmd(self):
+ return self.make_command('service_add_principal', self.name)
+
+ def _make_remove_alias_cmd(self):
+ return self.make_command('service_remove_principal', self.name)
diff --git a/ipatests/test_xmlrpc/tracker/user_plugin.py b/ipatests/test_xmlrpc/tracker/user_plugin.py
index ee49d418cea68f747657d9f5b99986dba06edc1f..ae3d39c4a0446a03e373c310f695d3725a2c05c0 100644
--- a/ipatests/test_xmlrpc/tracker/user_plugin.py
+++ b/ipatests/test_xmlrpc/tracker/user_plugin.py
@@ -12,12 +12,13 @@ from ipatests.test_xmlrpc import objectclasses
from ipatests.test_xmlrpc.xmlrpc_test import (
fuzzy_digits, fuzzy_uuid, raises_exact)
from ipatests.test_xmlrpc.tracker.base import Tracker
+from ipatests.test_xmlrpc.tracker.kerberos_aliases import KerberosAliasMixin
if six.PY3:
unicode = str
-class UserTracker(Tracker):
+class UserTracker(KerberosAliasMixin, Tracker):
""" Class for host plugin like tests """
retrieve_keys = {
@@ -492,3 +493,10 @@ class UserTracker(Tracker):
'description': [u'Account administrators group'],
},
), result)
+
+ # Kerberos aliases methods
+ def _make_add_alias_cmd(self):
+ return self.make_command('user_add_principal', self.name)
+
+ def _make_remove_alias_cmd(self):
+ return self.make_command('user_remove_principal', self.name)
--
2.9.0
From 0416d1d769eccfdccba0bf44afd8c33a26c7342c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= <mku...@redhat.com>
Date: Fri, 22 Jul 2016 17:16:25 +0200
Subject: [PATCH 2/7] ipatests: Extend the MockLDAP utility class
Added mod_entry method to allow modifying existing entries via the
ldap connection.
The commit also implements the context manager protocol for the class.
https://fedorahosted.org/freeipa/ticket/6142
---
ipatests/util.py | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/ipatests/util.py b/ipatests/util.py
index 796f87cbbfb25028a5135394a0a97a6b1f7180b5..27b939e251e6d9e61d3c6d80a2d34f895bcb419c 100644
--- a/ipatests/util.py
+++ b/ipatests/util.py
@@ -652,6 +652,9 @@ class MockLDAP(object):
except ldap.ALREADY_EXISTS:
pass
+ def mod_entry(self, dn, mods):
+ self.connection.modify_s(dn, mods)
+
def del_entry(self, dn):
try:
self.connection.delete_s(dn)
@@ -662,6 +665,13 @@ class MockLDAP(object):
if self.connection is not None:
self.connection.unbind_s()
+ # contextmanager protocol
+ def __enter__(self):
+ return self
+
+ def __exit__(self, exc_type, exc_value, traceback):
+ self.unbind()
+
def prepare_config(template, values):
with open(template) as f:
--
2.9.0
From 8bfaaaa0e948e5de25d78dc8e9850d12dc118647 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= <mku...@redhat.com>
Date: Fri, 22 Jul 2016 17:19:31 +0200
Subject: [PATCH 3/7] ipatests: Provide a context manager for mocking a trust
in RPC tests
The new module contains utility functions and a context manager to
make the mocking of an existing AD trust relation in the XMLRPC tests.
The module provides with two functions that create and delete the
containers for trusts and cifs domains. A context manager using these
is provided as well.
The user of the context manager is responsible for deleting all the
LDAP entries created during the test within the context. If there are
some entries left at the time of exiting the context manager, making
the container entries non-leaf entries, the tests will fail.
The context manager will not work when used on a server that already
has trust established.
https://fedorahosted.org/freeipa/ticket/6142
---
ipatests/test_xmlrpc/mock_trust.py | 52 ++++++++++++++++++++++++++++++++++++++
1 file changed, 52 insertions(+)
create mode 100644 ipatests/test_xmlrpc/mock_trust.py
diff --git a/ipatests/test_xmlrpc/mock_trust.py b/ipatests/test_xmlrpc/mock_trust.py
new file mode 100644
index 0000000000000000000000000000000000000000..8313db62e59eb1a62e2647169488bf4559148e9f
--- /dev/null
+++ b/ipatests/test_xmlrpc/mock_trust.py
@@ -0,0 +1,52 @@
+# coding: utf-8
+#
+# Copyright (C) 2016 FreeIPA Contributors see COPYING for license
+#
+from contextlib import contextmanager
+
+from ipalib import api
+from ipatests.util import MockLDAP
+
+trust_container_dn = "cn=ad,cn=trusts,{basedn}".format(
+ basedn=api.env.basedn)
+trust_container_add = dict(
+ objectClass=[b"nsContainer", b"top"]
+ )
+
+smb_cont_dn = "{cifsdomains},{basedn}".format(
+ cifsdomains=api.env.container_cifsdomains,
+ basedn=api.env.basedn)
+smb_cont_add = dict(
+ objectClass=[b"nsContainer", b"top"]
+ )
+
+
+def create_mock_trust_containers():
+ with MockLDAP() as ldap:
+ ldap.add_entry(trust_container_dn, trust_container_add)
+ ldap.add_entry(smb_cont_dn, smb_cont_add)
+
+
+def remove_mock_trust_containers():
+ with MockLDAP() as ldap:
+ ldap.del_entry(trust_container_dn)
+ ldap.del_entry(smb_cont_dn)
+
+
+@contextmanager
+def mocked_trust_containers():
+ """Mocked trust containers
+
+ Provides containers for the RPC tests:
+ cn=ad,cn=trusts,BASEDN
+ cn=ad,cn=etc,BASEDN
+
+ Upon exiting, it tries to remove the container entries.
+ If the user of the context manager failed to remove
+ all child entries, exiting the context manager will fail.
+ """
+ create_mock_trust_containers()
+ try:
+ yield
+ finally:
+ remove_mock_trust_containers()
--
2.9.0
From 3643b7ee3e7387b7322e275e4f9d186c82a42401 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= <mku...@redhat.com>
Date: Tue, 26 Jul 2016 13:53:29 +0200
Subject: [PATCH 4/7] ipatests: Move trust mock helper functions to a separate
module
Moves helper functions used in range plugin test to a separate
module to allow code reuse.
https://fedorahosted.org/freeipa/ticket/6142
---
ipatests/test_xmlrpc/mock_trust.py | 44 +++++++++++++++++++++++++++
ipatests/test_xmlrpc/test_range_plugin.py | 49 ++-----------------------------
2 files changed, 47 insertions(+), 46 deletions(-)
diff --git a/ipatests/test_xmlrpc/mock_trust.py b/ipatests/test_xmlrpc/mock_trust.py
index 8313db62e59eb1a62e2647169488bf4559148e9f..08c1c291469b588a7b9b5ddb37f21df85975e41a 100644
--- a/ipatests/test_xmlrpc/mock_trust.py
+++ b/ipatests/test_xmlrpc/mock_trust.py
@@ -3,6 +3,7 @@
# Copyright (C) 2016 FreeIPA Contributors see COPYING for license
#
from contextlib import contextmanager
+import six
from ipalib import api
from ipatests.util import MockLDAP
@@ -50,3 +51,46 @@ def mocked_trust_containers():
yield
finally:
remove_mock_trust_containers()
+
+def get_range_dn(name):
+ format_str = "cn={name},cn=ranges,cn=etc,{basedn}"
+ data = dict(name=name, basedn=api.env.basedn)
+ return format_str.format(**data)
+
+
+def get_trust_dn(name):
+ format_str = "cn={name},cn=ad,cn=trusts,{basedn}"
+ data = dict(name=name, basedn=api.env.basedn)
+ return format_str.format(**data)
+
+
+def encode_mockldap_value(value):
+ value = str(value)
+ if six.PY3:
+ return value.encode('utf-8')
+ else:
+ return value
+
+
+def get_trusted_dom_range_dict(name, base_id, size, rangetype, base_rid, sid):
+ return dict(
+ objectClass=[b"ipaIDrange", b"ipatrustedaddomainrange"],
+ ipaBaseID=encode_mockldap_value("{base_id}".format(base_id=base_id)),
+ ipaBaseRID=encode_mockldap_value("{base_rid}".format(base_rid=base_rid)),
+ ipaIDRangeSize=encode_mockldap_value("{size}".format(size=size)),
+ ipaNTTrustedDomainSID=encode_mockldap_value("{sid}".format(sid=sid)),
+ ipaRangeType=encode_mockldap_value("{rangetype}".format(rangetype=rangetype)),
+ )
+
+
+def get_trusted_dom_dict(name, sid):
+ return dict(
+ objectClass=[b"ipaNTTrustedDomain", b"ipaIDobject", b"top"],
+ ipaNTFlatName=encode_mockldap_value(name.split('.')[0].upper()),
+ ipaNTTrustedDomainSID=encode_mockldap_value(sid),
+ ipaNTSIDBlacklistIncoming=b'S-1-0',
+ ipaNTTrustPartner=encode_mockldap_value('{name}.mock'.format(name=name)),
+ ipaNTTrustType=b'2',
+ ipaNTTrustDirection=b'3',
+ ipaNTTrustAttributes=b'8',
+ )
diff --git a/ipatests/test_xmlrpc/test_range_plugin.py b/ipatests/test_xmlrpc/test_range_plugin.py
index 0f0e05a470ea17d49da44322cdbe661b08026202..d0f962ae7eda05b5a16b1160ff7b1757a5835d26 100644
--- a/ipatests/test_xmlrpc/test_range_plugin.py
+++ b/ipatests/test_xmlrpc/test_range_plugin.py
@@ -29,6 +29,9 @@ from ipatests.test_xmlrpc import objectclasses
from ipatests.util import MockLDAP
from ipapython.dn import DN
from ipatests.test_xmlrpc.test_user_plugin import get_user_result
+from ipatests.test_xmlrpc.mock_trust import (
+ get_range_dn, get_trusted_dom_dict, get_trusted_dom_range_dict,
+ get_trust_dn)
import pytest
if six.PY3:
@@ -110,52 +113,6 @@ testrange8_size = 50
testrange8_base_rid = rid_shift + 700
testrange8_secondary_base_rid = rid_shift + 800
-# Helper functions definitions
-
-
-def get_range_dn(name):
- format_str = "cn={name},cn=ranges,cn=etc,{basedn}"
- data = dict(name=name, basedn=api.env.basedn)
- return format_str.format(**data)
-
-
-def get_trust_dn(name):
- format_str = "cn={name},cn=ad,cn=trusts,{basedn}"
- data = dict(name=name, basedn=api.env.basedn)
- return format_str.format(**data)
-
-
-def encode_mockldap_value(value):
- value = str(value)
- if six.PY3:
- return value.encode('utf-8')
- else:
- return value
-
-
-def get_trusted_dom_range_dict(name, base_id, size, rangetype, base_rid, sid):
- return dict(
- objectClass=[b"ipaIDrange", b"ipatrustedaddomainrange"],
- ipaBaseID=encode_mockldap_value("{base_id}".format(base_id=base_id)),
- ipaBaseRID=encode_mockldap_value("{base_rid}".format(base_rid=base_rid)),
- ipaIDRangeSize=encode_mockldap_value("{size}".format(size=size)),
- ipaNTTrustedDomainSID=encode_mockldap_value("{sid}".format(sid=sid)),
- ipaRangeType=encode_mockldap_value("{rangetype}".format(rangetype=rangetype)),
- )
-
-
-def get_trusted_dom_dict(name, sid):
- return dict(
- objectClass=[b"ipaNTTrustedDomain", b"ipaIDobject", b"top"],
- ipaNTFlatName=encode_mockldap_value(name.upper()),
- ipaNTTrustedDomainSID=encode_mockldap_value(sid),
- ipaNTSIDBlacklistIncoming=b'S-1-0',
- ipaNTTrustPartner=encode_mockldap_value('{name}.mock'.format(name=name)),
- ipaNTTrustType=b'2',
- ipaNTTrustDirection=b'3',
- ipaNTTrustAttributes=b'8',
- )
-
# Domain ranges definitions
# Domain1 - AD domain nonactive (not present in LDAP)
--
2.9.0
From 4768cb1171f9029a0c821cfd9cd81b80e6fcd3b9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= <mku...@redhat.com>
Date: Mon, 25 Jul 2016 13:20:54 +0200
Subject: [PATCH 5/7] ipapython: Extend kinit_password to support principal
canonicalization
In order to authenticate with a principal alias it is necessary
to request canonicalization of the principal. This patch extends
the kinit_password with this option.
The option to indicate enterprise principal has been added as well.
https://fedorahosted.org/freeipa/ticket/6142
---
ipapython/ipautil.py | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index 9964fba4f694b57242b3bd3065a418917d977533..17d92b06ffa22019de198b59611585cb3f0b7f42 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -1328,7 +1328,8 @@ def kinit_keytab(principal, keytab, ccache_name, config=None, attempts=1):
def kinit_password(principal, password, ccache_name, config=None,
- armor_ccache_name=None):
+ armor_ccache_name=None, canonicalize=False,
+ enterprise=False):
"""
perform interactive kinit as principal using password. If using FAST for
web-based authentication, use armor_ccache_path to specify http service
@@ -1341,6 +1342,14 @@ def kinit_password(principal, password, ccache_name, config=None,
% armor_ccache_name)
args.extend(['-T', armor_ccache_name])
+ if canonicalize:
+ root_logger.debug("Requesting principal canonicalization")
+ args.append('-C')
+
+ if enterprise:
+ root_logger.debug("Using enterprise principal")
+ args.append('-E')
+
env = {'LC_ALL': 'C'}
if config is not None:
env['KRB5_CONFIG'] = config
--
2.9.0
From c0ff3e1ca734b4164918155e21fc64150c48f5c7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= <mku...@redhat.com>
Date: Mon, 25 Jul 2016 13:24:51 +0200
Subject: [PATCH 6/7] ipatests: Allow change_principal context manager to use
canonicalization
The context manager has been extended to optionally request principal
canonicalization and indicate that the enterprise principal is being
used.
This allows to change the user during the test to an user using the alias
and to test behavior related to enterprise principals.
https://fedorahosted.org/freeipa/ticket/6142
---
ipatests/util.py | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/ipatests/util.py b/ipatests/util.py
index 27b939e251e6d9e61d3c6d80a2d34f895bcb419c..8878993e1b5a992f7e415acc2062d4bd8e63dfdd 100644
--- a/ipatests/util.py
+++ b/ipatests/util.py
@@ -693,7 +693,8 @@ def unlock_principal_password(user, oldpw, newpw):
@contextmanager
-def change_principal(user, password, client=None, path=None):
+def change_principal(user, password, client=None, path=None,
+ canonicalize=False, enterprise=False):
if path:
ccache_name = path
@@ -708,7 +709,8 @@ def change_principal(user, password, client=None, path=None):
try:
with private_ccache(ccache_name):
- kinit_password(user, password, ccache_name)
+ kinit_password(user, password, ccache_name,
+ canonicalize=canonicalize, enterprise=enterprise)
client.Backend.rpcclient.connect()
try:
--
2.9.0
From 9edb3a3d40a9d0028e9ab8cbbb3f3faeb88b3b90 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= <mku...@redhat.com>
Date: Fri, 22 Jul 2016 17:25:06 +0200
Subject: [PATCH 7/7] ipatests: Add kerberos principal alias tests
Add tests for alias manipulation, tests authentication and several
error scenarios.
https://fedorahosted.org/freeipa/ticket/6142
https://fedorahosted.org/freeipa/ticket/6099
---
.../test_xmlrpc/test_kerberos_principal_aliases.py | 290 +++++++++++++++++++++
1 file changed, 290 insertions(+)
create mode 100644 ipatests/test_xmlrpc/test_kerberos_principal_aliases.py
diff --git a/ipatests/test_xmlrpc/test_kerberos_principal_aliases.py b/ipatests/test_xmlrpc/test_kerberos_principal_aliases.py
new file mode 100644
index 0000000000000000000000000000000000000000..3bbc641f1c5ea495d6a8e37e8aff67f1c67d5f4e
--- /dev/null
+++ b/ipatests/test_xmlrpc/test_kerberos_principal_aliases.py
@@ -0,0 +1,290 @@
+# coding: utf-8
+#
+# Copyright (C) 2016 FreeIPA Contributors see COPYING for license
+#
+import copy
+import ldap
+import pytest
+
+from ipalib import errors, api
+from ipapython import ipautil
+from ipaplatform.paths import paths
+
+from ipatests.util import MockLDAP
+from ipatests.test_xmlrpc.xmlrpc_test import XMLRPC_test
+from ipatests.test_xmlrpc.tracker.user_plugin import UserTracker
+from ipatests.test_xmlrpc.tracker.host_plugin import HostTracker
+from ipatests.test_xmlrpc.tracker.service_plugin import ServiceTracker
+from ipatests.test_xmlrpc.mock_trust import (
+ mocked_trust_containers, get_trust_dn, get_trusted_dom_dict,
+ encode_mockldap_value)
+from ipatests.util import unlock_principal_password, change_principal
+
+
+# Shared values for the mocked trusted domain
+TRUSTED_DOMAIN_MOCK = dict(
+ name=u'trusted.domain.net',
+ sid=u'S-1-5-21-2997650941-1802118864-3094776726'
+)
+TRUSTED_DOMAIN_MOCK['dn'] = get_trust_dn(TRUSTED_DOMAIN_MOCK['name'])
+TRUSTED_DOMAIN_MOCK['ldif'] = get_trusted_dom_dict(
+ TRUSTED_DOMAIN_MOCK['name'], TRUSTED_DOMAIN_MOCK['sid']
+)
+
+
+@pytest.yield_fixture
+def trusted_domain():
+ """Fixture providing mocked AD trust entries
+
+ The fixture yields after creating a mock of AD trust
+ entries in the directory server. After the test, the entries
+ are deleted from the directory.
+ """
+
+ trusted_dom = TRUSTED_DOMAIN_MOCK['ldif']
+
+ # Write the changes
+ with mocked_trust_containers(), MockLDAP() as ldap:
+ ldap.add_entry(trusted_dom['dn'], trusted_dom['ldif'])
+ yield trusted_dom
+ ldap.del_entry(trusted_dom['dn'])
+
+
+@pytest.yield_fixture
+def trusted_domain_with_suffix():
+ """Fixture providing mocked AD trust entries
+
+ The fixture yields after creating a mock of AD trust
+ entries in the directory server. After the test, the entries
+ are deleted from the directory.
+ """
+ trusted_dom = copy.deepcopy(TRUSTED_DOMAIN_MOCK)
+
+ trusted_dom['ldif']['ipaNTAdditionalSuffixes'] = (
+ encode_mockldap_value(trusted_dom['name'])
+ )
+
+ # Write the changes
+ with mocked_trust_containers(), MockLDAP() as ldap:
+ ldap.add_entry(trusted_dom['dn'], trusted_dom['ldif'])
+ yield trusted_dom
+ ldap.del_entry(trusted_dom['dn'])
+
+
+@pytest.fixture(scope='function')
+def krbalias_user(request):
+ tracker = UserTracker(u'krbalias_user', u'krbalias', u'test')
+
+ return tracker.make_fixture(request)
+
+
+@pytest.fixture(scope='function')
+def krbalias_user_c(request):
+ tracker = UserTracker(u'krbalias_user_conflict', u'krbalias', u'test')
+
+ return tracker.make_fixture(request)
+
+
+@pytest.fixture(scope='function')
+def krbalias_host(request):
+ tracker = HostTracker(u'testhost-krb')
+
+ return tracker.make_fixture(request)
+
+
+@pytest.fixture
+def krb_service_host(request):
+ tracker = HostTracker(u'krb-srv-host')
+
+ return tracker.make_fixture(request)
+
+
+@pytest.fixture(scope='function')
+def krbalias_service(request, krb_service_host):
+ krb_service_host.ensure_exists()
+
+ tracker = ServiceTracker(name=u'SRV1', host_fqdn=krb_service_host.name)
+
+ return tracker.make_fixture(request)
+
+
+@pytest.fixture
+def ldapservice(request):
+ tracker = ServiceTracker(
+ name=u'ldap', host_fqdn=api.env.host, options={'has_keytab': True})
+
+ tracker.track_create()
+ return tracker
+
+
+class TestKerberosAliasManipulation(XMLRPC_test):
+
+ def test_add_user_principal_alias(self, krbalias_user):
+ krbalias_user.ensure_exists()
+ krbalias_user.add_principal([u'test-user-alias'])
+ krbalias_user.retrieve()
+
+ def test_remove_user_principal_alias(self, krbalias_user):
+ krbalias_user.ensure_exists()
+ krbalias_user.add_principal([u'test-user-alias'])
+ krbalias_user.remove_principal(u'test-user-alias')
+ krbalias_user.retrieve()
+
+ def test_add_host_principal_alias(self, krbalias_host):
+ krbalias_host.ensure_exists()
+ krbalias_host.add_principal([u'testhost-krb-alias'])
+ krbalias_host.retrieve()
+
+ def test_remove_host_principal_alias(self, krbalias_host):
+ krbalias_host.ensure_exists()
+ krbalias_host.add_principal([u'testhost-krb-alias'])
+ krbalias_host.retrieve()
+ krbalias_host.remove_principal([u'testhost-krb-alias'])
+ krbalias_host.retrieve()
+
+ def test_add_service_principal_alias(self, krbalias_service):
+ krbalias_service.ensure_exists()
+ krbalias_service.add_principal(
+ [u'SRV2/{}'.format(krbalias_service.host_fqdn)])
+ krbalias_service.retrieve()
+
+ def test_remove_service_principal_alias(self, krbalias_service):
+ krbalias_service.ensure_exists()
+ krbalias_service.add_principal(
+ [u'SRV2/{}'.format(krbalias_service.host_fqdn)])
+ krbalias_service.retrieve()
+ krbalias_service.remove_principal(
+ [u'SRV2/{}'.format(krbalias_service.host_fqdn)])
+ krbalias_service.retrieve()
+
+ def test_adding_alias_adds_canonical_name(self, krbalias_user):
+ """Test adding alias on an entry without canonical name"""
+ krbalias_user.ensure_exists()
+
+ user_krb_principal = krbalias_user.attrs['krbprincipalname'][0]
+
+ # Delete all values of krbcanonicalname from an LDAP entry
+ dn = str(krbalias_user.dn)
+ modlist = [(ldap.MOD_DELETE, 'krbcanonicalname', None)]
+
+ with MockLDAP() as ldapconn:
+ ldapconn.mod_entry(dn, modlist)
+
+ # add new user principal alias
+ krbalias_user.add_principal(u'krbalias_principal_canonical')
+
+ # verify that the previous principal name is now krbcanonicalname
+ cmd = krbalias_user.make_retrieve_command()
+
+ new_canonical_name = cmd()['result']['krbcanonicalname'][0]
+ assert new_canonical_name == user_krb_principal
+
+ def test_authenticate_against_aliased_service(self, ldapservice):
+ alias = u'ldap/{newname}.{host}'.format(
+ newname='krbalias', host=api.env.host)
+ ldapservice.add_principal(alias)
+
+ rv = ipautil.run([paths.BIN_KVNO, alias],
+ capture_error=True, raiseonerr=False)
+ ldapservice.remove_principal(alias)
+
+ assert rv.returncode == 0, rv.error_output
+
+ def test_authenticate_with_user_alias(self, krbalias_user):
+ krbalias_user.ensure_exists()
+
+ alias = u"{name}-alias".format(name=krbalias_user.name)
+
+ krbalias_user.add_principal(alias)
+
+ oldpw, newpw = u"Secret1234", u"Secret123"
+
+ pwdmod = krbalias_user.make_update_command({'userpassword': oldpw})
+ pwdmod()
+
+ unlock_principal_password(krbalias_user.name, oldpw, newpw)
+
+ with change_principal(alias, newpw, canonicalize=True):
+ api.Command.ping()
+
+
+class TestKerberosAliasExceptions(XMLRPC_test):
+
+ def test_add_user_coliding_with_alias(self, krbalias_user):
+ krbalias_user.ensure_exists()
+
+ user_alias = u'conflicting_name'
+ krbalias_user.add_principal([user_alias])
+
+ conflict_user = UserTracker(user_alias, u'test', u'conflict')
+
+ with pytest.raises(errors.DuplicateEntry):
+ conflict_user.create()
+
+ def test_add_alias_to_two_entries(self, krbalias_user, krbalias_user_c):
+ krbalias_user.ensure_exists()
+ krbalias_user_c.ensure_exists()
+
+ user_alias = u'krbalias-test'
+
+ krbalias_user.add_principal([user_alias])
+
+ with pytest.raises(errors.DuplicateEntry):
+ krbalias_user_c.add_principal([user_alias])
+
+ def test_remove_alias_matching_canonical_name(self, krbalias_user):
+ krbalias_user.ensure_exists()
+
+ with pytest.raises(errors.ValidationError):
+ krbalias_user.remove_principal(
+ krbalias_user.attrs.get('krbcanonicalname'))
+
+ def test_enterprise_principal_overlap_with_AD_realm(
+ self, krbalias_user, trusted_domain):
+ krbalias_user.ensure_exists()
+
+ # Add an alias overlapping the trusted domain realm
+ with pytest.raises(errors.ValidationError):
+ krbalias_user.add_principal(
+ u'{username}\@{trusted_domain}@{realm}'.format(
+ username=krbalias_user.name,
+ trusted_domain=trusted_domain['name'],
+ realm=api.env.realm
+ )
+ )
+
+ def test_enterprise_principal_UPN_overlap(
+ self, krbalias_user, trusted_domain_with_suffix):
+ krbalias_user.ensure_exists()
+
+ # Add an alias overlapping the UPN of a trusted domain
+ upn_suffix = (
+ trusted_domain_with_suffix['ldif']['ipaNTAdditionalSuffixes']
+ )
+
+ with pytest.raises(errors.ValidationError):
+ krbalias_user.add_principal(
+ u'{username}\@{trusted_domain}@{realm}'.format(
+ username=krbalias_user.name,
+ trusted_domain=upn_suffix,
+ realm=api.env.realm
+ )
+ )
+
+ def test_enterprise_principal_NETBIOS_overlap(
+ self, krbalias_user, trusted_domain_with_suffix):
+ krbalias_user.ensure_exists()
+
+ # Add an alias overlapping the NETBIOS name of a trusted domain
+ netbios_name = (
+ trusted_domain_with_suffix['ldif']['ipaNTFlatName']
+ )
+
+ with pytest.raises(errors.ValidationError):
+ krbalias_user.add_principal(
+ u'{username}\@{trusted_domain}@{realm}'.format(
+ username=krbalias_user.name,
+ trusted_domain=netbios_name,
+ realm=api.env.realm
+ )
+ )
--
2.9.0
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
