On 10/24/2013 01:29 PM, Petr Viktorin wrote:
On 10/22/2013 02:24 PM, Tomas Babej wrote:
On 10/22/2013 02:15 PM, Tomas Babej wrote:
On 10/22/2013 12:27 PM, Tomas Babej wrote:
On 10/22/2013 10:37 AM, Petr Viktorin wrote:
Replying to one part only:
On 10/21/2013 04:50 PM, Tomas Babej wrote:
On 10/16/2013 03:44 PM, Petr Viktorin wrote:
I still think it would be simpler if IPA and AD domains shared the
numbering namespace (users would need to define $AD_env2; if
they had
$MASTER_env1 and $AD_env1 they would be in the same Domain).
But if we use _env1 for both the AD and the IPA domain, they
need to
be separated in Domain.from_env. With patch 0101 both
MASTER_env1 and
AD_env1 will be in both domains[0] and ad_domains[0].
I would rather not join IPA and AD domains as they even cannot be
in the
same domain, as the service records would clash. So these will
always be separate / sub / super domain relationship.
You're right that they should never share the same domain. But you
should never say never, especially in testing -- what if we'll want
to, in the future, test that the records *do* clash, or that IPA
refuses to install in an AD domain?
You could still set AD_env1 and MASTER_env1 to have the same domain.
Another problem is that they are now separate namespaces. In all
code that deals with domains you have to deal separately with the
list of AD domains and separately with IPA domains. This makes every
piece of code that doesn't care much about what type of domain it's
dealing with (configuration, listing, possible automation scripts
for turning on the VMs, etc.) more complicated.
Also, in this scheme, adding a new type of domain would be quite
hard, especially after more code is written with this split in mind.
Do keep the domain type, though. tl;dr I'd really prefer "domain 1
(IPA); domain 2 (AD)" rather than "IPA domain 1; AD domain 1".
This will, however, require filtering the domains depending on the
fact whether they contain AD or not. If a testcase wants to access an
AD domain, it will still need to loop over the list of domains to see
which ones are of AD type.
Any code that does not care what domain type it's dealing with, can
easily access all the domains by chaining the respective iterables.
We could have a wrapper in the Config class for that, along the lines
of get_all_domains().
So what I see here is that we're trading one complexity over another.
I think we can agree on your approach since it hides the complexity
from the user, especially in the ipa-test-config, which I admit is
getting rather ugly, as we need to introduce new option there and
that causes splitting.
If needed we can have a special check that would reject IPA masters
in AD domains and vice versa, if that really turns out to be
necessary.
With this check we should be fine.
As we already pass ad_domain flag to Domain.from_env, I did
incorporate
code that joins the machines to the domain depending on the their
role.
Is that a viable solution for you?
Sorry. I think this design is less sustainable than having a shared
namespace for the domains.
I'll send revised patchset soon.
Updated patchset attached.
Patch 105:
Typo: 'Sets DNS ib given host for trust with the given AD
^^
Should be "in", right? I'll fix it before pushing. c
Otherwise, these are good to go!
Patch 106:
In ADTrustBase, it looks like if test_install_adtrust or
test_configure_dns_and_time fail, it doesn't make much sense to run
the other tests.
If that's the case they can go in an install() classmethod. Same with
test_establish_trust* in the subclasses.
I made them part of the install() classmethod.
As for the test_estabilish_trust, I would have to still override that in
each class that uses it, since all of them use it in a slightly
different way.
Also, there's a typo in test_estabilish_trusts several times.
-----^----
Typo fixed. Also attaching a patch that fixes the same type in the other
parts of the codebase.
--
Tomas Babej
Associate Software Engeneer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org
From e7f777b229534a65eb7f6bc39e77c2a5b5c51983 Mon Sep 17 00:00:00 2001
From: Tomas Babej <[email protected]>
Date: Thu, 24 Oct 2013 14:24:33 +0200
Subject: [PATCH] trusts: Fix typo in error message for realm-domain mismatch
---
ipalib/plugins/trust.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/ipalib/plugins/trust.py b/ipalib/plugins/trust.py
index 0d651f8861446cf8d31eb1ea303237bcd2b73201..32a93834394273c9f896ff5fd17bfcc753fe7b8e 100644
--- a/ipalib/plugins/trust.py
+++ b/ipalib/plugins/trust.py
@@ -424,14 +424,14 @@ sides.
)
# If domain name and realm does not match, IPA server is not be able
- # to estabilish trust with Active Directory.
+ # to establish trust with Active Directory.
realm_not_matching_domain = (api.env.domain.upper() != api.env.realm)
if options['trust_type'] == u'ad' and realm_not_matching_domain:
raise errors.ValidationError(
name=_('Realm-domain mismatch'),
- error=_('To estabilish trust with Active Directory, the '
+ error=_('To establish trust with Active Directory, the '
'domain name and the realm name of the IPA server '
'must match')
)
--
1.8.3.1
From ce0b7d09cb51973317550a5a6e0b749bf2e129a6 Mon Sep 17 00:00:00 2001
From: Tomas Babej <[email protected]>
Date: Mon, 14 Oct 2013 16:37:55 +0200
Subject: [PATCH] ipatests: Add AD integration test case
Part of: https://fedorahosted.org/freeipa/ticket/3834
---
ipatests/test_integration/test_trust.py | 188 ++++++++++++++++++++++++++++++++
1 file changed, 188 insertions(+)
create mode 100644 ipatests/test_integration/test_trust.py
diff --git a/ipatests/test_integration/test_trust.py b/ipatests/test_integration/test_trust.py
new file mode 100644
index 0000000000000000000000000000000000000000..c5167301503ef0c7fb8518d8c865617e17da9da4
--- /dev/null
+++ b/ipatests/test_integration/test_trust.py
@@ -0,0 +1,188 @@
+# Authors:
+# Tomas Babej <[email protected]>
+#
+# Copyright (C) 2013 Red Hat
+# see file 'COPYING' for use and warranty information
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+import re
+
+from ipatests.test_integration.base import IntegrationTest
+from ipatests.test_integration import tasks
+from ipatests.test_integration import util
+
+
+class ADTrustBase(IntegrationTest):
+ """Provides common checks for the AD trust integration testing."""
+
+ topology = 'line'
+ num_ad_domains = 1
+
+ @classmethod
+ def install(cls):
+ super(ADTrustBase, cls).install()
+ cls.ad = cls.ad_domains[0].ads[0]
+ cls.install_adtrust()
+ cls.check_sid_generation()
+ cls.configure_dns_and_time()
+
+ @classmethod
+ def install_adtrust(cls):
+ """Test adtrust support installation"""
+
+ tasks.install_adtrust(cls.master)
+
+ @classmethod
+ def check_sid_generation(cls):
+ """Test SID generation"""
+
+ command = ['ipa', 'user-show', 'admin', '--all', '--raw']
+
+ # TODO: remove duplicate definition and import from common module
+ _sid_identifier_authority = '(0x[0-9a-f]{1,12}|[0-9]{1,10})'
+ sid_regex = 'S-1-5-21-%(idauth)s-%(idauth)s-%(idauth)s'\
+ % dict(idauth=_sid_identifier_authority)
+ stdout_re = re.escape(' ipaNTSecurityIdentifier: ') + sid_regex
+
+ util.run_repeatedly(cls.master, command,
+ test=lambda x: re.search(stdout_re, x))
+
+ @classmethod
+ def configure_dns_and_time(cls):
+ tasks.configure_dns_for_trust(cls.master, cls.ad)
+ tasks.sync_time(cls.master, cls.ad)
+
+
+class TestBasicADTrust(ADTrustBase):
+ """Basic Integration test for Active Directory"""
+
+ def test_establish_trust(self):
+ """Tests establishing trust with Active Directory"""
+
+ tasks.establish_trust_with_ad(self.master, self.ad,
+ extra_args=['--range-type', 'ipa-ad-trust'])
+
+ def test_range_properties_in_nonposix_trust(self):
+ """Check the properties of the created range"""
+
+ range_name = self.ad.domain.name.upper() + '_id_range'
+ result = self.master.run_command(['ipa', 'idrange-show', range_name,
+ '--all', '--raw'])
+ assert " ipaRangeType: ipa-ad-trust" in result.stdout_text
+ assert " ipaIDRangeSize: 200000" in result.stdout_text
+
+ def test_user_gid_uid_resolution_in_nonposix_trust(self):
+ """Check that user has SID-generated UID"""
+
+ testuser = 'testuser@%s' % self.ad.domain.realm
+ result = self.master.run_command(['getent', 'passwd', testuser])
+
+ # This regex checks that Test User does not have UID 10042 nor belongs
+ # to the group with GID 10047
+ testuser_regex = "^testuser@%s:\*:(?!10042)(\d+):(?!10047)(\d+):"\
+ "Test User:/home/testuser:/bin/sh$"\
+ % re.escape(self.ad.domain.name)
+
+ assert re.search(testuser_regex, result.stdout_text)
+
+ def test_remove_nonposix_trust(self):
+ tasks.remove_trust_with_ad(self.master, self.ad)
+ tasks.clear_sssd_cache(self.master)
+
+
+class TestPosixADTrust(ADTrustBase):
+ """Integration test for Active Directory with POSIX support"""
+
+ def test_establish_trust_with_posix_attributes(self):
+ # Not specifying the --range-type directly, it should be detected
+ tasks.establish_trust_with_ad(self.master, self.ad)
+
+ def test_range_properties_in_posix_trust(self):
+ # Check the properties of the created range
+
+ range_name = self.ad.domain.name.upper() + '_id_range'
+
+ result = self.master.run_command(['ipa', 'idrange-show', range_name,
+ '--all', '--raw'])
+
+ # Check the range type and size
+ assert " ipaRangeType: ipa-ad-trust-posix" in result.stdout_text
+ assert " ipaIDRangeSize: 200000" in result.stdout_text
+
+ def test_user_uid_gid_resolution_in_posix_trust(self):
+ # Check that user has AD-defined UID
+
+ testuser = 'testuser@%s' % self.ad.domain.realm
+ result = self.master.run_command(['getent', 'passwd', testuser])
+
+ testuser_stdout = "testuser@%s:*:10042:10047:"\
+ "Test User:/home/testuser:/bin/sh"\
+ % self.ad.domain.name
+
+ assert testuser_stdout in result.stdout_text
+
+ def test_user_without_posix_attributes_not_visible(self):
+ # Check that user has AD-defined UID
+
+ nonposixuser = 'nonposixuser@%s' % self.ad.domain.realm
+ result = self.master.run_command(['getent', 'passwd', nonposixuser],
+ raiseonerr=False)
+
+ # Getent exits with 2 for non-existent user
+ assert result.returncode == 2
+
+ def test_remove_trust_with_posix_attributes(self):
+ tasks.remove_trust_with_ad(self.master, self.ad)
+ tasks.clear_sssd_cache(self.master)
+
+
+class TestEnforcedPosixADTrust(TestPosixADTrust):
+ """
+ This test is intented to copycat PosixADTrust, since enforcing the POSIX
+ trust type should not make a difference.
+ """
+
+ def test_establish_trust_with_posix_attributes(self):
+ tasks.establish_trust_with_ad(self.master, self.ad,
+ extra_args=['--range-type', 'ipa-ad-trust-posix'])
+
+
+class TestInvalidRangeTypes(ADTrustBase):
+ """
+ Tests invalid values being put into trust-add command.
+ """
+
+ def test_invalid_range_types(self):
+
+ invalid_range_types = ['ipa-local',
+ 'ipa-ad-winsync',
+ 'ipa-ipa-trust',
+ 'random-invalid',
+ 're@ll%ybad12!']
+
+ for range_type in invalid_range_types:
+ tasks.kinit_admin(self.master)
+
+ result = self.master.run_command(
+ ['ipa', 'trust-add',
+ '--type', 'ad', self.ad.domain.name,
+ '--admin', 'Administrator',
+ '--range-type', range_type,
+ '--password'],
+ raiseonerr=False,
+ stdin_text=self.master.config.ad_admin_password)
+
+ # The trust-add command is supposed to fail
+ assert result.returncode == 1
--
1.8.3.1
_______________________________________________
Freeipa-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/freeipa-devel
