[Freeipa-devel] [freeipa PR#73][synchronized] Tests for certificates with SAN

2016-09-27 Thread apophys
   URL: https://github.com/freeipa/freeipa/pull/73
Author: apophys
 Title: #73: Tests for certificates with SAN
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/73/head:pr73
git checkout pr73
From 909ab9fa6405acb346162508729cda8b56e08f9e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= 
Date: Mon, 12 Sep 2016 14:52:05 +0200
Subject: [PATCH 1/5] ipatests: provide context manager for keytab usage in RPC
 tests

https://fedorahosted.org/freeipa/ticket/6291
---
 ipatests/util.py | 52 +++-
 1 file changed, 47 insertions(+), 5 deletions(-)

diff --git a/ipatests/util.py b/ipatests/util.py
index 8878993..4c1a77a 100644
--- a/ipatests/util.py
+++ b/ipatests/util.py
@@ -40,7 +40,9 @@
 from ipalib.plugable import Plugin
 from ipalib.request import context
 from ipapython.dn import DN
-from ipapython.ipautil import private_ccache, kinit_password, run
+from ipapython.ipautil import (
+private_ccache, kinit_password, kinit_keytab, run
+)
 from ipaplatform.paths import paths
 
 if six.PY3:
@@ -693,8 +695,8 @@ def unlock_principal_password(user, oldpw, newpw):
 
 
 @contextmanager
-def change_principal(user, password, client=None, path=None,
- canonicalize=False, enterprise=False):
+def change_principal(principal, password=None, client=None, path=None,
+ canonicalize=False, enterprise=False, keytab=None):
 
 if path:
 ccache_name = path
@@ -709,8 +711,12 @@ def change_principal(user, password, client=None, path=None,
 
 try:
 with private_ccache(ccache_name):
-kinit_password(user, password, ccache_name,
-   canonicalize=canonicalize, enterprise=enterprise)
+if keytab:
+kinit_keytab(principal, keytab, ccache_name)
+else:
+kinit_password(principal, password, ccache_name,
+   canonicalize=canonicalize,
+   enterprise=enterprise)
 client.Backend.rpcclient.connect()
 
 try:
@@ -720,6 +726,42 @@ def change_principal(user, password, client=None, path=None,
 finally:
 client.Backend.rpcclient.connect()
 
+
+@contextmanager
+def get_entity_keytab(principal, options=None):
+"""Requests a keytab for an entity
+
+The keytab will generate new keys if not specified
+otherwise in the options.
+To retrieve existing keytab, use the -r option
+"""
+keytab_filename = os.path.join('/tmp', str(uuid.uuid4()))
+
+try:
+cmd = [paths.IPA_GETKEYTAB, '-p', principal, '-k', keytab_filename]
+
+if options:
+cmd.extend(options)
+run(cmd)
+
+yield keytab_filename
+finally:
+os.remove(keytab_filename)
+
+
+@contextmanager
+def host_keytab(hostname, options=None):
+"""Retrieves keytab for a particular host
+
+After leaving the context manager, the keytab file is
+deleted.
+"""
+principal = u'host/{}'.format(hostname)
+
+with get_entity_keytab(principal, options) as keytab:
+yield keytab
+
+
 def get_group_dn(cn):
 return DN(('cn', cn), api.env.container_group, api.env.basedn)
 

From 7b962b358198559966f0f750edd9210a140b57c3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= 
Date: Mon, 12 Sep 2016 14:53:48 +0200
Subject: [PATCH 2/5] ipatests: Fix name property on a service tracker

https://fedorahosted.org/freeipa/ticket/6291
---
 ipatests/test_xmlrpc/tracker/service_plugin.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ipatests/test_xmlrpc/tracker/service_plugin.py b/ipatests/test_xmlrpc/tracker/service_plugin.py
index a0bb884..0a90115 100644
--- a/ipatests/test_xmlrpc/tracker/service_plugin.py
+++ b/ipatests/test_xmlrpc/tracker/service_plugin.py
@@ -52,7 +52,7 @@ class ServiceTracker(KerberosAliasMixin, Tracker):
 
 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._name = u"{0}/{1}@{2}".format(name, host_fqdn, api.env.realm)
 self.dn = DN(
 ('krbprincipalname', self.name), api.env.container_service,
 api.env.basedn)

From 941b2f8e7b0661837d6fb10bfd9a4d26c45158e1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= 
Date: Mon, 12 Sep 2016 14:54:40 +0200
Subject: [PATCH 3/5] ipatests: Implement tests with CSRs requesting SAN

The patch implements several test cases testing the enforcement
of CA ACLs on certificate requests with subject alternative names.

https://fedorahosted.org/freeipa/ticket/6291
---
 freeipa.spec.in|   2 +
 .../test_xmlrpc/test_caacl_profile_enforcement.py  | 240 -
 2 files 

Re: [Freeipa-devel] CA-less installs: passive certmonger - watch-and-warn mode

2016-09-27 Thread Petr Spacek
On 18.7.2016 08:22, Jan Cholasta wrote:
> On 8.7.2016 15:59, Rob Crittenden wrote:
>> Petr Spacek wrote:
>>> On 8.7.2016 15:31, Rob Crittenden wrote:
 Petr Spacek wrote:
> Hi,
>
> our docs
>
> https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/7/html/Linux_Domain_Identity_Authentication_and_Policy_Guide/install-server.html#install-determine-ca
>
>
>
>
> claim this:
> "The certmonger service is not used to track certificates.
> Therefore, it does
> not warn you of impending certificate expiration."
>
> Is this correct?
>
> Can we at least configure certmonger to passively track the
> certificates and
> throw warning about impending expiration into logs?
> 
> +1, I have already suggested we do this several times.
> 
>

 Throw a warning where? Register an e-mail address as part of the
 tracking
 perhaps?

 It would probably be fairly easy to write a "CA" that sends an
 e-mail. The
 trick, and this has always tripped us up, is having an MTA configured.
>>>
>>> I would start with logs, as I wrote in the original message. This will
>>> naturally evolve into something else when we finally get
>>> user-configurable hooks.
>>>
>>> In any case, having certmonger configured to track the certs is
>>> prerequisite
>>> for all cases...
>>
>> "Logs" is not very specific, do you mean syslog/journal?
>>
>> Feel free to open an RFE against certmonger with your proposal. I
>> suspect that anything logged will just get lost in most cases.

Finally, here is the ticket:
https://fedorahosted.org/certmonger/ticket/59

> For IPA CA certificate, we log warnings to syslog with ALERT level. I think
> doing that for other certs would be good enough for starters.

-- 
Petr^2 Spacek

-- 
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


[Freeipa-devel] [freeipa PR#114][comment] Raise errors from service.py:_ldap_mod() by default

2016-09-27 Thread mbasti-rh
  URL: https://github.com/freeipa/freeipa/pull/114
Title: #114: Raise errors from service.py:_ldap_mod() by default

mbasti-rh commented:
"""
This issues are caused mostly by newer replica install, so I don't think that 
earlier devel cycle will help us , we need good upgrade testing.

However I agree that is better to stop with first error and not continou later 
and break things even more
"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/114#issuecomment-249827802
-- 
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

Re: [Freeipa-devel] What would break if loopback addresses were allowed for IPA server?

2016-09-27 Thread Jan Pazdziora
On Wed, Sep 21, 2016 at 12:01:44PM +0200, Jan Pazdziora wrote:
> 
> I've recently hit again the situation of IPA installer not happy
> about the provided IP address not being local to it, this time in
> containerized environment:
> 
>   https://bugzilla.redhat.com/show_bug.cgi?id=1377973
> 
> During the discussion, we came to an interesting question:
> 
>   What would break if loopback addresses were allowed for IPA
>   server?
> 
> Of course, the idea is that it would only be used for installation and
> then IPA would change its IP address in DNS to whatever is the real IP
> address under which it is accessible.
> 
> Where does the allow_loopback=False requirement in the installer come
> from and what would break if it was removed altogether?

I also see messages like

Adding [10.11.12.13 ipa.example.com] to your /etc/hosts file

in some cases. Actually, it's

10.11.12.13 ipa.example.com ipa

which gets added so the message is not accurate.

Modification of /etc/hosts itself seems unfortunate. Should the IP
address change in the future, there will be one more place where
the IP address stays hardcoded.

I wonder why

hosts:  files dns myhostname

isn't enough, and whether

hosts:  files myhostname dns

might actually be better order. When the value is not in /etc/hosts,
I see weird startup issues, presumably because individual components
time out resolving $HOSTNAME, so systemctl start ipa fails. Perhaps
it has something to do with named being up at that point, rather than
unreachable, just not resolving anything yet. Chicken and egg.

I wonder why we cannot add ipa.example.com to 127.0.0.1. I've tried
that and have seen

named-pkcs11[453]: LDAP error: Local error: SASL(-1): generic
failure: GSSAPI Error: Unspecified GSS failure.  Minor code
may provide more information (Server
ldap/localh...@example.test not found in Kerberos database):
bind to LDAP server failed

which suggests something derives the hostname and thus the principal
from the IP address used. Why is not $HOSTNAME used everywhere? What
part of the system cares about the IP address (and the reverse
resolution)?

If overloading 127.0.0.1 with the $HOSTNAME does not work, could
127.0.0.2 do the trick? It seems to work for subsequent starts (did
not try it during ipa-server-install) in containers.

-- 
Jan Pazdziora
Senior Principal Software Engineer, Identity Management Engineering, Red Hat

-- 
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


[Freeipa-devel] [freeipa PR#122][opened] Acceptance tests

2016-09-27 Thread dkupka
   URL: https://github.com/freeipa/freeipa/pull/122
Author: dkupka
 Title: #122: Acceptance tests
Action: opened

PR body:
"""
Starting with minimal suite that will grow as necessary.
"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/122/head:pr122
git checkout pr122
From c4f25ae2c78584299c80894b62253372eea4f309 Mon Sep 17 00:00:00 2001
From: David Kupka 
Date: Wed, 21 Sep 2016 13:09:19 +0200
Subject: [PATCH 1/2] tests: Mark Dogtag acceptance tests

---
 ipatests/pytest.ini|  1 +
 ipatests/test_integration/test_installation.py | 21 +
 2 files changed, 22 insertions(+)

diff --git a/ipatests/pytest.ini b/ipatests/pytest.ini
index 233cf43..277e26d 100644
--- a/ipatests/pytest.ini
+++ b/ipatests/pytest.ini
@@ -23,3 +23,4 @@ addopts = --doctest-modules
 markers =
 tier0: basic unit tests and critical functionality
 tier1: functional API tests
+cs_acceptance: Acceptance test suite for Dogtag Certificate Server
diff --git a/ipatests/test_integration/test_installation.py b/ipatests/test_integration/test_installation.py
index c3d194f..868e8e7 100644
--- a/ipatests/test_integration/test_installation.py
+++ b/ipatests/test_integration/test_installation.py
@@ -7,6 +7,7 @@
 installed.
 """
 
+import pytest
 from ipatests.test_integration.base import IntegrationTest
 from ipatests.test_integration import tasks
 
@@ -196,3 +197,23 @@ def test_install_master(self):
 
 def test_install_kra(self):
 tasks.install_kra(self.master, first_instance=True)
+
+
+@pytest.mark.cs_acceptance
+class TestCSAcceptance(IntegrationTest):
+num_replicas = 2
+
+@classmethod
+def install(cls, mh):
+tasks.install_master(cls.master, setup_dns=True)
+
+def test_install_master_kra(self):
+tasks.install_kra(self.master, first_instance=True)
+
+def test_install_replica0_with_ca(self):
+tasks.install_replica(self.master, self.replicas[0], setup_ca=True,
+  setup_kra=False, setup_dns=False)
+
+def test_install_replica1_with_ca_kra(self):
+tasks.install_replica(self.master, self.replicas[1], setup_ca=True,
+  setup_kra=True, setup_dns=False)

From b63cb30e0f48b0b94a52b909372f924d17d2ab3c Mon Sep 17 00:00:00 2001
From: David Kupka 
Date: Wed, 21 Sep 2016 13:09:43 +0200
Subject: [PATCH 2/2] tests: Mark 389-ds acceptance tests

---
 ipatests/pytest.ini|  1 +
 ipatests/test_integration/test_installation.py | 13 +
 2 files changed, 14 insertions(+)

diff --git a/ipatests/pytest.ini b/ipatests/pytest.ini
index 277e26d..11198dd 100644
--- a/ipatests/pytest.ini
+++ b/ipatests/pytest.ini
@@ -24,3 +24,4 @@ markers =
 tier0: basic unit tests and critical functionality
 tier1: functional API tests
 cs_acceptance: Acceptance test suite for Dogtag Certificate Server
+ds_acceptance: Acceptance test suite for 389 Directory Server
diff --git a/ipatests/test_integration/test_installation.py b/ipatests/test_integration/test_installation.py
index 868e8e7..d4aba00 100644
--- a/ipatests/test_integration/test_installation.py
+++ b/ipatests/test_integration/test_installation.py
@@ -199,6 +199,19 @@ def test_install_kra(self):
 tasks.install_kra(self.master, first_instance=True)
 
 
+@pytest.mark.ds_acceptance
+class TestDSAcceptance(IntegrationTest):
+num_replicas = 1
+
+@classmethod
+def install(cls, mh):
+tasks.install_master(cls.master, setup_dns=True)
+
+def test_install_replica0(self):
+tasks.install_replica(self.master, self.replicas[0], setup_ca=False,
+  setup_kra=False, setup_dns=False)
+
+
 @pytest.mark.cs_acceptance
 class TestCSAcceptance(IntegrationTest):
 num_replicas = 2
-- 
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

[Freeipa-devel] [freeipa PR#115][synchronized] Don't show traceback when ipa config file is not an absolute path

2016-09-27 Thread tomaskrizek
   URL: https://github.com/freeipa/freeipa/pull/115
Author: tomaskrizek
 Title: #115: Don't show traceback when ipa config file is not an absolute path
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/115/head:pr115
git checkout pr115
From 278f0133cfe40d322455a7ecc5ce24d780c16aae Mon Sep 17 00:00:00 2001
From: Tomas Krizek 
Date: Tue, 27 Sep 2016 13:39:22 +0200
Subject: [PATCH] ipa: file path validation for config file option

Convert the config file attribute of ipa command to an absolute
path. Additionaly, check the file exists.

https://fedorahosted.org/freeipa/ticket/6114
---
 ipalib/plugable.py | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/ipalib/plugable.py b/ipalib/plugable.py
index af35f5b..27db8cb 100644
--- a/ipalib/plugable.py
+++ b/ipalib/plugable.py
@@ -494,6 +494,13 @@ def build_global_parser(self, parser=None, context=None):
 """
 Add global options to an optparse.OptionParser instance.
 """
+def config_file_callback(option, opt, value, parser):
+value = os.path.abspath(value)
+if not os.path.isfile(value):
+raise optparse.OptionValueError(
+"%s option '%s' is not a file" % (opt, value))
+parser.values.conf = value
+
 if parser is None:
 parser = optparse.OptionParser(
 add_help_option=False,
@@ -517,8 +524,9 @@ def build_global_parser(self, parser=None, context=None):
 parser.add_option('-e', dest='env', metavar='KEY=VAL', action='append',
 help='Set environment variable KEY to VAL',
 )
-parser.add_option('-c', dest='conf', metavar='FILE',
-help='Load configuration from FILE',
+parser.add_option('-c', dest='conf', metavar='FILE', action='callback',
+callback=config_file_callback, type='string',
+help='Load configuration from FILE. This must be an absolute path',
 )
 parser.add_option('-d', '--debug', action='store_true',
 help='Produce full debuging output',
-- 
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

[Freeipa-devel] [freeipa PR#121][comment] Pylint: enable unused-variable check

2016-09-27 Thread mbasti-rh
  URL: https://github.com/freeipa/freeipa/pull/121
Title: #121: Pylint: enable unused-variable check

mbasti-rh commented:
"""
I disagree, I really think that there should not be assert
"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/121#issuecomment-249816977
-- 
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

[Freeipa-devel] [freeipa PR#115][comment] Don't show traceback when ipa config file is not an absolute path

2016-09-27 Thread mbasti-rh
  URL: https://github.com/freeipa/freeipa/pull/115
Title: #115: Don't show traceback when ipa config file is not an absolute path

mbasti-rh commented:
"""
NACK, please see inline comments
"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/115#issuecomment-249846654
-- 
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

[Freeipa-devel] [freeipa PR#122][comment] Acceptance tests

2016-09-27 Thread mbasti-rh
  URL: https://github.com/freeipa/freeipa/pull/122
Title: #122: Acceptance tests

mbasti-rh commented:
"""
I quite disagree with marks, please read my inline comments
"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/122#issuecomment-249858212
-- 
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

[Freeipa-devel] [freeipa PR#123][opened] Tests: Remove silent deleting and creating entries by tracker

2016-09-27 Thread mirielka
   URL: https://github.com/freeipa/freeipa/pull/123
Author: mirielka
 Title: #123: Tests: Remove silent deleting and creating entries by tracker
Action: opened

PR body:
"""
https://fedorahosted.org/freeipa/ticket/6123
"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/123/head:pr123
git checkout pr123
From 6fd6fcfc282675014e2a4704d4211a2f8e18d8c9 Mon Sep 17 00:00:00 2001
From: Lenka Doudova 
Date: Tue, 27 Sep 2016 14:46:32 +0200
Subject: [PATCH] Tests: Remove silent deleting and creating entries by tracker

https://fedorahosted.org/freeipa/ticket/6123
---
 ipatests/test_xmlrpc/test_group_plugin.py | 4 ++--
 ipatests/test_xmlrpc/test_stageuser_plugin.py | 1 +
 ipatests/test_xmlrpc/test_user_plugin.py  | 2 ++
 ipatests/test_xmlrpc/tracker/base.py  | 5 -
 4 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/ipatests/test_xmlrpc/test_group_plugin.py b/ipatests/test_xmlrpc/test_group_plugin.py
index 27a8a33..2f824de 100644
--- a/ipatests/test_xmlrpc/test_group_plugin.py
+++ b/ipatests/test_xmlrpc/test_group_plugin.py
@@ -231,7 +231,7 @@ def test_search_for_all_groups_with_members(self, group, group2):
 def test_search_for_all_groups(self, group, group2):
 """ Search for all groups """
 group.ensure_exists()
-group2.create()
+group2.ensure_exists()
 command = group.make_command('group_find')
 result = command()
 assert_deepequal(dict(
@@ -631,7 +631,7 @@ class TestManagedGroupObjectclasses(XMLRPC_test):
 def test_check_objectclasses_after_detach(self, user, managed_group):
 """ Check objectclasses after user was detached from managed group """
 # https://fedorahosted.org/freeipa/ticket/4909#comment:1
-user.create()
+user.ensure_exists()
 user.run_command('group_detach', *[user.uid])
 managed_group.retrieve(all=True)
 managed_group.add_member(dict(user=user.uid))
diff --git a/ipatests/test_xmlrpc/test_stageuser_plugin.py b/ipatests/test_xmlrpc/test_stageuser_plugin.py
index 34cfaf8..4a859e8 100644
--- a/ipatests/test_xmlrpc/test_stageuser_plugin.py
+++ b/ipatests/test_xmlrpc/test_stageuser_plugin.py
@@ -255,6 +255,7 @@ def test_delete_stageduser(self, stageduser):
 stageduser.delete()
 
 def test_find_stageduser(self, stageduser):
+stageduser.ensure_exists()
 stageduser.find()
 
 def test_findall_stageduser(self, stageduser):
diff --git a/ipatests/test_xmlrpc/test_user_plugin.py b/ipatests/test_xmlrpc/test_user_plugin.py
index 7c27abc..7508578 100644
--- a/ipatests/test_xmlrpc/test_user_plugin.py
+++ b/ipatests/test_xmlrpc/test_user_plugin.py
@@ -177,6 +177,7 @@ def test_rename_nonexistent(self, user, renameduser):
 class TestUser(XMLRPC_test):
 def test_retrieve(self, user):
 """ Create user and try to retrieve it """
+user.ensure_exists()
 user.retrieve()
 
 def test_delete(self, user):
@@ -216,6 +217,7 @@ def test_remove_userclass(self, user):
 class TestFind(XMLRPC_test):
 def test_find(self, user):
 """ Basic check of user-find """
+user.ensure_exists()
 user.find()
 
 def test_find_with_all(self, user):
diff --git a/ipatests/test_xmlrpc/tracker/base.py b/ipatests/test_xmlrpc/tracker/base.py
index ecaadc6..a2b7406 100644
--- a/ipatests/test_xmlrpc/tracker/base.py
+++ b/ipatests/test_xmlrpc/tracker/base.py
@@ -199,7 +199,6 @@ def make_update_command(self, updates):
 
 def create(self):
 """Helper function to create an entry and check the result"""
-self.ensure_missing()
 self.track_create()
 command = self.make_create_command()
 result = command()
@@ -227,7 +226,6 @@ def check_create(self, result):
 
 def delete(self):
 """Helper function to delete a host and check the result"""
-self.ensure_exists()
 self.track_delete()
 command = self.make_delete_command()
 result = command()
@@ -244,7 +242,6 @@ def check_delete(self, result):
 
 def retrieve(self, all=False, raw=False):
 """Helper function to retrieve an entry and check the result"""
-self.ensure_exists()
 command = self.make_retrieve_command(all=all, raw=raw)
 result = command()
 self.check_retrieve(result, all=all, raw=raw)
@@ -255,7 +252,6 @@ def check_retrieve(self, result, all=False, raw=False):
 
 def find(self, all=False, raw=False):
 """Helper function to search for this hosts and check the result"""
-self.ensure_exists()
 command = self.make_find_command(self.name, all=all, raw=raw)
 result = command()
 self.check_find(result, all=all, raw=raw)
@@ -274,7 +270,6 @@ def update(self, updates, expected_updates=None):
 if expected_updates is None:
 expected_updates = {}
 
-self.ensure_exists()
 command = 

[Freeipa-devel] [freeipa PR#121][comment] Pylint: enable unused-variable check

2016-09-27 Thread flo-renaud
  URL: https://github.com/freeipa/freeipa/pull/121
Title: #121: Pylint: enable unused-variable check

flo-renaud commented:
"""
Agree with you, ACK.
"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/121#issuecomment-249822167
-- 
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

[Freeipa-devel] [freeipa PR#114][comment] Raise errors from service.py:_ldap_mod() by default

2016-09-27 Thread mbasti-rh
  URL: https://github.com/freeipa/freeipa/pull/114
Title: #114: Raise errors from service.py:_ldap_mod() by default

mbasti-rh commented:
"""
This issues are caused mostly by newer replica install, so I don't think that 
earlier devel cycle will help us , we need good upgrade testing.

However I agree that is better to stop with first error and not continue later 
and break things even more
"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/114#issuecomment-249827802
-- 
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

[Freeipa-devel] [freeipa PR#114][comment] Raise errors from service.py:_ldap_mod() by default

2016-09-27 Thread mbasti-rh
  URL: https://github.com/freeipa/freeipa/pull/114
Title: #114: Raise errors from service.py:_ldap_mod() by default

mbasti-rh commented:
"""
This issues are caused mostly by newer replica install, so I don't think that 
earlier devel cycle will help us, we need good upgrade testing.

However I agree that is better to stop with first error and not continue and 
break things even more later
"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/114#issuecomment-249827802
-- 
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

[Freeipa-devel] [freeipa PR#121][+ack] Pylint: enable unused-variable check

2016-09-27 Thread stlaz
  URL: https://github.com/freeipa/freeipa/pull/121
Title: #121: Pylint: enable unused-variable check

Label: +ack
-- 
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

[Freeipa-devel] [freeipa PR#121][comment] Pylint: enable unused-variable check

2016-09-27 Thread stlaz
  URL: https://github.com/freeipa/freeipa/pull/121
Title: #121: Pylint: enable unused-variable check

stlaz commented:
"""
The latest changes fixed the nitpicks mentioned, ACK. Thanks :+1: 
"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/121#issuecomment-249830361
-- 
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

[Freeipa-devel] [freeipa PR#121][closed] Pylint: enable unused-variable check

2016-09-27 Thread mbasti-rh
   URL: https://github.com/freeipa/freeipa/pull/121
Author: mbasti-rh
 Title: #121: Pylint: enable unused-variable check
Action: closed

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/121/head:pr121
git checkout pr121
-- 
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

[Freeipa-devel] [freeipa PR#121][+pushed] Pylint: enable unused-variable check

2016-09-27 Thread mbasti-rh
  URL: https://github.com/freeipa/freeipa/pull/121
Title: #121: Pylint: enable unused-variable check

Label: +pushed
-- 
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

[Freeipa-devel] [freeipa PR#121][comment] Pylint: enable unused-variable check

2016-09-27 Thread mbasti-rh
  URL: https://github.com/freeipa/freeipa/pull/121
Title: #121: Pylint: enable unused-variable check

mbasti-rh commented:
"""
Fixed upstream
master:
https://fedorahosted.org/freeipa/changeset/0f88f8fe889ae4801fc8d5ece1ad51c5246718ac
https://fedorahosted.org/freeipa/changeset/9d83be3647547cfca4e129cfeb63771213232cf7
https://fedorahosted.org/freeipa/changeset/45e3aee35219c89c07d590003a334f8db658a3b2
"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/121#issuecomment-249840099
-- 
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

[Freeipa-devel] [freeipa PR#115][comment] Don't show traceback when ipa config file is not an absolute path

2016-09-27 Thread pspacek
  URL: https://github.com/freeipa/freeipa/pull/115
Title: #115: Don't show traceback when ipa config file is not an absolute path

pspacek commented:
"""
Why the file must be absolute? I would rather remove this requirement and be 
done with it. `open()` the file and if it succeeds - use it. If it fails, print 
error returned from `open`.
"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/115#issuecomment-249854141
-- 
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

[Freeipa-devel] [freeipa PR#117][synchronized] Make ipa-replica-install run in interactive mode

2016-09-27 Thread stlaz
   URL: https://github.com/freeipa/freeipa/pull/117
Author: stlaz
 Title: #117: Make ipa-replica-install run in interactive mode
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/117/head:pr117
git checkout pr117
From 30d1e65e23ca099f91f2c43f2d57127cc66c142c Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka 
Date: Mon, 26 Sep 2016 12:43:24 +0200
Subject: [PATCH 1/2] replicainstall: don't assume default principal

If --admin-password is set during ipa-replica-install but
--principal is not, 'admin' is assumed. This is wrong and
it's not advertised anywhere so fail instead.

https://fedorahosted.org/freeipa/ticket/6068
---
 ipaserver/install/server/replicainstall.py | 77 +++---
 1 file changed, 39 insertions(+), 38 deletions(-)

diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py
index aefe158..65ea6bb 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -918,47 +918,48 @@ def install(installer):
 
 
 def ensure_enrolled(installer):
-config = installer._config
+# Prepare options for the installer script
+args = [paths.IPA_CLIENT_INSTALL, "--no-ntp"]
+stdin = None
+
+if installer.domain_name:
+args.extend(["--domain", installer.domain_name])
+if installer.server:
+args.extend(["--server", installer.server])
+if installer.realm_name:
+args.extend(["--realm", installer.realm_name])
+if installer.host_name:
+args.extend(["--hostname", installer.host_name])
+if installer.password:
+args.extend(["--password", installer.password])
+else:
+if installer.principal:
+args.extend(["--principal", installer.principal])
+if installer.admin_password:
+if installer.principal is None:
+raise ScriptError("The --admin-password option must be used "
+  "with the --principal option.")
+stdin = installer.admin_password
+if installer.keytab:
+args.extend(["--keytab", installer.keytab])
+
+if installer.no_dns_sshfp:
+args.append("--no-dns-sshfp")
+if installer.ssh_trust_dns:
+args.append("--ssh-trust-dns")
+if installer.no_ssh:
+args.append("--no-ssh")
+if installer.no_sshd:
+args.append("--no-sshd")
+if installer.mkhomedir:
+args.append("--mkhomedir")
 
-# Call client install script
-service.print_msg("Configuring client side components")
 try:
+service.print_msg("Configuring client side components")
+# Set _enrollment_performed to True so that any mess left behind in
+# case of an enrollment failure gets cleaned
 installer._enrollment_performed = True
-
-args = [paths.IPA_CLIENT_INSTALL, "--unattended", "--no-ntp"]
-stdin = None
-
-if installer.domain_name:
-args.extend(["--domain", installer.domain_name])
-if installer.server:
-args.extend(["--server", installer.server])
-if installer.realm_name:
-args.extend(["--realm", installer.realm_name])
-if installer.host_name:
-args.extend(["--hostname", installer.host_name])
-
-if installer.password:
-args.extend(["--password", installer.password])
-else:
-if installer.admin_password:
-# Always set principal if password was set explicitly,
-# the password itself gets passed directly via stdin
-args.extend(["--principal", installer.principal or "admin"])
-stdin = installer.admin_password
-if installer.keytab:
-args.extend(["--keytab", installer.keytab])
-
-if installer.no_dns_sshfp:
-args.append("--no-dns-sshfp")
-if installer.ssh_trust_dns:
-args.append("--ssh-trust-dns")
-if installer.no_ssh:
-args.append("--no-ssh")
-if installer.no_sshd:
-args.append("--no-sshd")
-if installer.mkhomedir:
-args.append("--mkhomedir")
-
+# Call client install script
 ipautil.run(args, stdin=stdin, redirect_output=True)
 print()
 except Exception:

From 13c6d00733be4235b171348e00b06cb3387b025c Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka 
Date: Mon, 26 Sep 2016 12:45:49 +0200
Subject: [PATCH 2/2] replicainstall: run client-install in attended mode by
 default

Running ipa-client-install in unattended mode during enrollment
process in ipa-replica-install only made everyone confused,
run it in attended mode by default instead.

https://fedorahosted.org/freeipa/ticket/6068
---
 ipaserver/install/server/replicainstall.py | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git 

Re: [Freeipa-devel] [PATCH 0097] Properly handle LDAP socket closures in ipa-otpd

2016-09-27 Thread Alexander Bokovoy

On ti, 27 syys 2016, Nathaniel McCallum wrote:

In at least one case, when an LDAP socket closes, a read event is fired
rather than an error event. Without this patch, ipa-otpd silently
ignores this event and enters a state where all bind auths fail.

To remedy this problem, we pass error events along the same path as
read events. Should the actual read fail, we exit.

Please add the bugzilla link.

--
/ Alexander Bokovoy

--
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


[Freeipa-devel] [freeipa PR#117][comment] Make ipa-replica-install run in interactive mode

2016-09-27 Thread tomaskrizek
  URL: https://github.com/freeipa/freeipa/pull/117
Title: #117: Make ipa-replica-install run in interactive mode

tomaskrizek commented:
"""
NACK, please see inline comments.
"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/117#issuecomment-249814914
-- 
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

[Freeipa-devel] [freeipa PR#124][opened] Fix: find OSCP certificate test

2016-09-27 Thread mbasti-rh
   URL: https://github.com/freeipa/freeipa/pull/124
Author: mbasti-rh
 Title: #124: Fix: find OSCP certificate test
Action: opened

PR body:
"""
Test should check if any OSCP certificate has been returned

https://fedorahosted.org/freeipa/ticket/6359
"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/124/head:pr124
git checkout pr124
From d462bab999d037dff50207e3b8d05559202f727e Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Tue, 27 Sep 2016 15:10:19 +0200
Subject: [PATCH] Fix: find OSCP certificate test

Test should check if any OSCP certificate has been returned

https://fedorahosted.org/freeipa/ticket/6359
---
 ipatests/test_xmlrpc/test_cert_plugin.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/ipatests/test_xmlrpc/test_cert_plugin.py b/ipatests/test_xmlrpc/test_cert_plugin.py
index f07bb17..4537002 100644
--- a/ipatests/test_xmlrpc/test_cert_plugin.py
+++ b/ipatests/test_xmlrpc/test_cert_plugin.py
@@ -267,7 +267,9 @@ def test_0003_find_OCSP(self):
 """
 Search for the OCSP certificate.
 """
-api.Command['cert_find'](subject=u'OCSP Subsystem')
+res = api.Command['cert_find'](subject=u'OCSP Subsystem')
+assert 'count' in res
+assert res['count'], "No OSCP certificate found"
 
 def test_0004_find_this_host(self):
 """
-- 
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

[Freeipa-devel] [freeipa PR#73][synchronized] Tests for certificates with SAN

2016-09-27 Thread apophys
   URL: https://github.com/freeipa/freeipa/pull/73
Author: apophys
 Title: #73: Tests for certificates with SAN
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/73/head:pr73
git checkout pr73
From 2159887dc5d01d3cf578d45825163e1add52e8a3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= 
Date: Mon, 12 Sep 2016 14:52:05 +0200
Subject: [PATCH 1/5] ipatests: provide context manager for keytab usage in RPC
 tests

https://fedorahosted.org/freeipa/ticket/6291
---
 ipatests/util.py | 52 +++-
 1 file changed, 47 insertions(+), 5 deletions(-)

diff --git a/ipatests/util.py b/ipatests/util.py
index 0b50f85..48a0faf 100644
--- a/ipatests/util.py
+++ b/ipatests/util.py
@@ -40,7 +40,9 @@
 from ipalib.plugable import Plugin
 from ipalib.request import context
 from ipapython.dn import DN
-from ipapython.ipautil import private_ccache, kinit_password, run
+from ipapython.ipautil import (
+private_ccache, kinit_password, kinit_keytab, run
+)
 from ipaplatform.paths import paths
 
 if six.PY3:
@@ -693,8 +695,8 @@ def unlock_principal_password(user, oldpw, newpw):
 
 
 @contextmanager
-def change_principal(user, password, client=None, path=None,
- canonicalize=False, enterprise=False):
+def change_principal(principal, password=None, client=None, path=None,
+ canonicalize=False, enterprise=False, keytab=None):
 
 if path:
 ccache_name = path
@@ -709,8 +711,12 @@ def change_principal(user, password, client=None, path=None,
 
 try:
 with private_ccache(ccache_name):
-kinit_password(user, password, ccache_name,
-   canonicalize=canonicalize, enterprise=enterprise)
+if keytab:
+kinit_keytab(principal, keytab, ccache_name)
+else:
+kinit_password(principal, password, ccache_name,
+   canonicalize=canonicalize,
+   enterprise=enterprise)
 client.Backend.rpcclient.connect()
 
 try:
@@ -720,6 +726,42 @@ def change_principal(user, password, client=None, path=None,
 finally:
 client.Backend.rpcclient.connect()
 
+
+@contextmanager
+def get_entity_keytab(principal, options=None):
+"""Requests a keytab for an entity
+
+The keytab will generate new keys if not specified
+otherwise in the options.
+To retrieve existing keytab, use the -r option
+"""
+keytab_filename = os.path.join('/tmp', str(uuid.uuid4()))
+
+try:
+cmd = [paths.IPA_GETKEYTAB, '-p', principal, '-k', keytab_filename]
+
+if options:
+cmd.extend(options)
+run(cmd)
+
+yield keytab_filename
+finally:
+os.remove(keytab_filename)
+
+
+@contextmanager
+def host_keytab(hostname, options=None):
+"""Retrieves keytab for a particular host
+
+After leaving the context manager, the keytab file is
+deleted.
+"""
+principal = u'host/{}'.format(hostname)
+
+with get_entity_keytab(principal, options) as keytab:
+yield keytab
+
+
 def get_group_dn(cn):
 return DN(('cn', cn), api.env.container_group, api.env.basedn)
 

From 9ed0d71a133fdd0d888f0b588a8e56e67b12e774 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= 
Date: Mon, 12 Sep 2016 14:53:48 +0200
Subject: [PATCH 2/5] ipatests: Fix name property on a service tracker

https://fedorahosted.org/freeipa/ticket/6291
---
 ipatests/test_xmlrpc/tracker/service_plugin.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ipatests/test_xmlrpc/tracker/service_plugin.py b/ipatests/test_xmlrpc/tracker/service_plugin.py
index a0bb884..0a90115 100644
--- a/ipatests/test_xmlrpc/tracker/service_plugin.py
+++ b/ipatests/test_xmlrpc/tracker/service_plugin.py
@@ -52,7 +52,7 @@ class ServiceTracker(KerberosAliasMixin, Tracker):
 
 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._name = u"{0}/{1}@{2}".format(name, host_fqdn, api.env.realm)
 self.dn = DN(
 ('krbprincipalname', self.name), api.env.container_service,
 api.env.basedn)

From 2d75883302db07061f8062751aae392ece23bcf9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= 
Date: Mon, 12 Sep 2016 14:54:40 +0200
Subject: [PATCH 3/5] ipatests: Implement tests with CSRs requesting SAN

The patch implements several test cases testing the enforcement
of CA ACLs on certificate requests with subject alternative names.

https://fedorahosted.org/freeipa/ticket/6291
---
 freeipa.spec.in|   2 +
 .../test_xmlrpc/test_caacl_profile_enforcement.py  | 240 -
 2 files 

[Freeipa-devel] [freeipa PR#115][synchronized] Don't show traceback when ipa config file is not an absolute path

2016-09-27 Thread tomaskrizek
   URL: https://github.com/freeipa/freeipa/pull/115
Author: tomaskrizek
 Title: #115: Don't show traceback when ipa config file is not an absolute path
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/115/head:pr115
git checkout pr115
From d625b8071a828e283cad863958acc832b9a33da9 Mon Sep 17 00:00:00 2001
From: Tomas Krizek 
Date: Tue, 27 Sep 2016 17:23:17 +0200
Subject: [PATCH 1/2] ipa: allow relative paths for config file

Remove unnecessary check for absolute file paths for config file.

https://fedorahosted.org/freeipa/ticket/6114
---
 ipalib/config.py | 15 +--
 1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/ipalib/config.py b/ipalib/config.py
index eb6c3ae..a273e3d 100644
--- a/ipalib/config.py
+++ b/ipalib/config.py
@@ -352,23 +352,10 @@ def _merge_from_file(self, config_file):
 containing first the number of variables that were actually set, and
 second the total number of variables found in ``config_file``.
 
-This method will raise a ``ValueError`` if ``config_file`` is not an
-absolute path.  For example:
-
->>> env = Env()
->>> env._merge_from_file('my/config.conf')
-Traceback (most recent call last):
-  ...
-ValueError: config_file must be an absolute path; got 'my/config.conf'
-
 Also see `Env._merge()`.
 
-:param config_file: Absolute path of the configuration file to load.
+:param config_file: Path of the configuration file to load.
 """
-if path.abspath(config_file) != config_file:
-raise ValueError(
-'config_file must be an absolute path; got %r' % config_file
-)
 if not path.isfile(config_file):
 return
 parser = RawConfigParser()

From 5d4b4609d1b788d26703f07f5d9c7ad195162274 Mon Sep 17 00:00:00 2001
From: Tomas Krizek 
Date: Tue, 27 Sep 2016 17:23:38 +0200
Subject: [PATCH 2/2] ipa: check if provided config file exists

Add a parser check to verify config file supplied to the ipa
command exists. Previously, invalid file paths would not results
in any error and would just silently proceed with default config.

https://fedorahosted.org/freeipa/ticket/6114
---
 ipalib/plugable.py | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/ipalib/plugable.py b/ipalib/plugable.py
index af35f5b..28c4042 100644
--- a/ipalib/plugable.py
+++ b/ipalib/plugable.py
@@ -44,6 +44,7 @@
 from ipalib.util import classproperty
 from ipalib.base import ReadOnly, lock, islocked
 from ipalib.constants import DEFAULT_CONFIG
+from ipapython import ipautil
 from ipapython.ipa_log_manager import (
 log_mgr,
 LOGGING_FORMAT_FILE,
@@ -494,6 +495,13 @@ def build_global_parser(self, parser=None, context=None):
 """
 Add global options to an optparse.OptionParser instance.
 """
+def config_file_callback(option, opt, value, parser):
+if not ipautil.file_exists(value):
+raise optparse.OptionValueError(
+_("%s: file not found") % value)
+
+parser.values.conf = value
+
 if parser is None:
 parser = optparse.OptionParser(
 add_help_option=False,
@@ -517,8 +525,9 @@ def build_global_parser(self, parser=None, context=None):
 parser.add_option('-e', dest='env', metavar='KEY=VAL', action='append',
 help='Set environment variable KEY to VAL',
 )
-parser.add_option('-c', dest='conf', metavar='FILE',
-help='Load configuration from FILE',
+parser.add_option('-c', dest='conf', metavar='FILE', action='callback',
+callback=config_file_callback, type='string',
+help='Load configuration from FILE.',
 )
 parser.add_option('-d', '--debug', action='store_true',
 help='Produce full debuging output',
-- 
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

[Freeipa-devel] [freeipa PR#115][synchronized] Don't show traceback when ipa config file is not an absolute path

2016-09-27 Thread tomaskrizek
   URL: https://github.com/freeipa/freeipa/pull/115
Author: tomaskrizek
 Title: #115: Don't show traceback when ipa config file is not an absolute path
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/115/head:pr115
git checkout pr115
From f445e300e4098da8ab10fd7952f57e421bc79f59 Mon Sep 17 00:00:00 2001
From: Tomas Krizek 
Date: Tue, 27 Sep 2016 17:05:48 +0200
Subject: [PATCH 1/2] ipa: check if provided config file exists

Add a parser check to verify config file supplied to the ipa
command exists. Previously, invalid file paths would not results
in any error and would just silently proceed with default config.

https://fedorahosted.org/freeipa/ticket/6114
---
 ipalib/plugable.py | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/ipalib/plugable.py b/ipalib/plugable.py
index af35f5b..d45e9ee 100644
--- a/ipalib/plugable.py
+++ b/ipalib/plugable.py
@@ -494,6 +494,12 @@ def build_global_parser(self, parser=None, context=None):
 """
 Add global options to an optparse.OptionParser instance.
 """
+def config_file_callback(option, opt, value, parser):
+if not os.path.isfile(value):
+raise optparse.OptionValueError(
+"%s option '%s' is not a file" % (opt, value))
+parser.values.conf = value
+
 if parser is None:
 parser = optparse.OptionParser(
 add_help_option=False,
@@ -517,8 +523,9 @@ def build_global_parser(self, parser=None, context=None):
 parser.add_option('-e', dest='env', metavar='KEY=VAL', action='append',
 help='Set environment variable KEY to VAL',
 )
-parser.add_option('-c', dest='conf', metavar='FILE',
-help='Load configuration from FILE',
+parser.add_option('-c', dest='conf', metavar='FILE', action='callback',
+callback=config_file_callback, type='string',
+help='Load configuration from FILE.',
 )
 parser.add_option('-d', '--debug', action='store_true',
 help='Produce full debuging output',

From add8f1898c321d5b3bc1de86b14c75a8903ecf5e Mon Sep 17 00:00:00 2001
From: Tomas Krizek 
Date: Tue, 27 Sep 2016 17:09:55 +0200
Subject: [PATCH 2/2] ipa: allow relative paths for config file

Remove unnecessary check for absolute file paths for config file.

https://fedorahosted.org/freeipa/ticket/6114
---
 ipalib/config.py | 15 +--
 1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/ipalib/config.py b/ipalib/config.py
index eb6c3ae..a273e3d 100644
--- a/ipalib/config.py
+++ b/ipalib/config.py
@@ -352,23 +352,10 @@ def _merge_from_file(self, config_file):
 containing first the number of variables that were actually set, and
 second the total number of variables found in ``config_file``.
 
-This method will raise a ``ValueError`` if ``config_file`` is not an
-absolute path.  For example:
-
->>> env = Env()
->>> env._merge_from_file('my/config.conf')
-Traceback (most recent call last):
-  ...
-ValueError: config_file must be an absolute path; got 'my/config.conf'
-
 Also see `Env._merge()`.
 
-:param config_file: Absolute path of the configuration file to load.
+:param config_file: Path of the configuration file to load.
 """
-if path.abspath(config_file) != config_file:
-raise ValueError(
-'config_file must be an absolute path; got %r' % config_file
-)
 if not path.isfile(config_file):
 return
 parser = RawConfigParser()
-- 
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

[Freeipa-devel] [freeipa PR#115][comment] Don't show traceback when ipa config file is not an absolute path

2016-09-27 Thread tomaskrizek
  URL: https://github.com/freeipa/freeipa/pull/115
Title: #115: Don't show traceback when ipa config file is not an absolute path

tomaskrizek commented:
"""
I found no reason why the path should be absolute, so I removed that constraint.

The parser check to verify if file exists should remain, since non-existent 
files are otherwise ignored without any notice.
"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/115#issuecomment-249901658
-- 
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

[Freeipa-devel] [freeipa PR#115][comment] Don't show traceback when ipa config file is not an absolute path

2016-09-27 Thread mbasti-rh
  URL: https://github.com/freeipa/freeipa/pull/115
Title: #115: Don't show traceback when ipa config file is not an absolute path

mbasti-rh commented:
"""
NACK, please see my inline comments
"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/115#issuecomment-249783814
-- 
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

[Freeipa-devel] [PATCH 0097] Properly handle LDAP socket closures in ipa-otpd

2016-09-27 Thread Nathaniel McCallum
In at least one case, when an LDAP socket closes, a read event is fired
rather than an error event. Without this patch, ipa-otpd silently
ignores this event and enters a state where all bind auths fail.

To remedy this problem, we pass error events along the same path as
read events. Should the actual read fail, we exit.From 43a8cd4f991115bcebcbe829b4b1be13849e288f Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum 
Date: Tue, 27 Sep 2016 14:34:05 -0400
Subject: [PATCH] Properly handle LDAP socket closures in ipa-otpd

In at least one case, when an LDAP socket closes, a read event is fired
rather than an error event. Without this patch, ipa-otpd silently
ignores this event and enters a state where all bind auths fail.

To remedy this problem, we pass error events along the same path as read
events. Should the actual read fail, we exit.
---
 daemons/ipa-otpd/bind.c  | 10 --
 daemons/ipa-otpd/query.c | 13 ++---
 2 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/daemons/ipa-otpd/bind.c b/daemons/ipa-otpd/bind.c
index 022525b786705b4f58f861bc3b0a745ab8693755..a98312f906a785bfa9c98603a3577561552bfc0a 100644
--- a/daemons/ipa-otpd/bind.c
+++ b/daemons/ipa-otpd/bind.c
@@ -85,6 +85,9 @@ static void on_bind_readable(verto_ctx *vctx, verto_ev *ev)
 if (rslt <= 0)
 results = NULL;
 ldap_msgfree(results);
+otpd_log_err(EIO, "IO error received on bind socket");
+verto_break(ctx.vctx);
+ctx.exitstatus = 1;
 return;
 }
 
@@ -137,11 +140,6 @@ void otpd_on_bind_io(verto_ctx *vctx, verto_ev *ev)
 flags = verto_get_fd_state(ev);
 if (flags & VERTO_EV_FLAG_IO_WRITE)
 on_bind_writable(vctx, ev);
-if (flags & VERTO_EV_FLAG_IO_READ)
+if (flags & (VERTO_EV_FLAG_IO_READ | VERTO_EV_FLAG_IO_ERROR))
 on_bind_readable(vctx, ev);
-if (flags & VERTO_EV_FLAG_IO_ERROR) {
-otpd_log_err(EIO, "IO error received on bind socket");
-verto_break(ctx.vctx);
-ctx.exitstatus = 1;
-}
 }
diff --git a/daemons/ipa-otpd/query.c b/daemons/ipa-otpd/query.c
index 67e2d751d8d1511d077a93d7673439be11812e6f..50e15603322c550a0eb14e1e3c502e1a229d1ebe 100644
--- a/daemons/ipa-otpd/query.c
+++ b/daemons/ipa-otpd/query.c
@@ -133,7 +133,11 @@ static void on_query_readable(verto_ctx *vctx, verto_ev *ev)
 if (i != LDAP_RES_SEARCH_ENTRY && i != LDAP_RES_SEARCH_RESULT) {
 if (i <= 0)
 results = NULL;
-goto egress;
+ldap_msgfree(results);
+otpd_log_err(EIO, "IO error received on query socket");
+verto_break(ctx.vctx);
+ctx.exitstatus = 1;
+return;
 }
 
 item = otpd_queue_pop_msgid(, ldap_msgid(results));
@@ -243,11 +247,6 @@ void otpd_on_query_io(verto_ctx *vctx, verto_ev *ev)
 flags = verto_get_fd_state(ev);
 if (flags & VERTO_EV_FLAG_IO_WRITE)
 on_query_writable(vctx, ev);
-if (flags & VERTO_EV_FLAG_IO_READ)
+if (flags & (VERTO_EV_FLAG_IO_READ | VERTO_EV_FLAG_IO_ERROR))
 on_query_readable(vctx, ev);
-if (flags & VERTO_EV_FLAG_IO_ERROR) {
-otpd_log_err(EIO, "IO error received on query socket");
-verto_break(ctx.vctx);
-ctx.exitstatus = 1;
-}
 }
-- 
2.10.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

Re: [Freeipa-devel] [PATCH 0097] Properly handle LDAP socket closures in ipa-otpd

2016-09-27 Thread Simo Sorce
On Tue, 2016-09-27 at 14:54 -0400, Nathaniel McCallum wrote:
> In at least one case, when an LDAP socket closes, a read event is
> fired
> rather than an error event. Without this patch, ipa-otpd silently
> ignores this event and enters a state where all bind auths fail.
> 
> To remedy this problem, we pass error events along the same path as
> read events. Should the actual read fail, we exit.

LGTM

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York

-- 
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


[Freeipa-devel] [freeipa PR#121][comment] Pylint: enable unused-variable check

2016-09-27 Thread stlaz
  URL: https://github.com/freeipa/freeipa/pull/121
Title: #121: Pylint: enable unused-variable check

stlaz commented:
"""
The changes seem fine except for the two small nitpicks.
"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/121#issuecomment-249823191
-- 
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

[Freeipa-devel] [freeipa PR#121][synchronized] Pylint: enable unused-variable check

2016-09-27 Thread mbasti-rh
   URL: https://github.com/freeipa/freeipa/pull/121
Author: mbasti-rh
 Title: #121: Pylint: enable unused-variable check
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/121/head:pr121
git checkout pr121
From cd58bc977d4594795cd934d54c22af8115bdce56 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Mon, 26 Sep 2016 14:08:17 +0200
Subject: [PATCH 1/3] Remove unused variables in the code

This commit removes unused variables or rename variables as "expected to
be unused" by using "_" prefix.

This covers only cases where fix was easy or only one unused variable
was in a module
---
 daemons/dnssec/ipa-dnskeysync-replica   | 2 +-
 doc/examples/examples.py| 2 +-
 install/oddjob/com.redhat.idm.trust-fetch-domains   | 2 +-
 install/share/copy-schema-to-ca.py  | 2 +-
 install/tools/ipa-csreplica-manage  | 4 
 install/tools/ipa-dns-install   | 2 +-
 install/tools/ipa-replica-conncheck | 2 +-
 ipaclient/ipa_certupdate.py | 2 +-
 ipaclient/plugins/automount.py  | 6 +++---
 ipaclient/plugins/location.py   | 2 +-
 ipaclient/plugins/vault.py  | 6 --
 ipalib/aci.py   | 2 --
 ipalib/cli.py   | 2 +-
 ipapython/dn.py | 9 -
 ipapython/dogtag.py | 2 +-
 ipapython/graph.py  | 4 ++--
 ipapython/install/cli.py| 2 +-
 ipapython/install/common.py | 4 ++--
 ipapython/nsslib.py | 2 +-
 ipaserver/install/adtrustinstance.py| 4 ++--
 ipaserver/install/installutils.py   | 2 +-
 ipaserver/install/ipa_otptoken_import.py| 2 +-
 ipaserver/install/krbinstance.py| 2 +-
 ipaserver/install/ldapupdate.py | 2 +-
 ipaserver/install/ntpinstance.py| 2 +-
 ipaserver/install/odsexporterinstance.py| 2 --
 ipaserver/install/plugins/adtrust.py| 4 ++--
 ipaserver/install/plugins/dns.py| 1 -
 ipaserver/install/plugins/update_idranges.py| 2 +-
 ipaserver/install/plugins/update_managed_permissions.py | 2 +-
 ipaserver/install/plugins/update_passsync.py| 2 +-
 ipaserver/install/plugins/update_uniqueness.py  | 2 +-
 ipaserver/install/plugins/upload_cacrt.py   | 2 +-
 ipaserver/install/schemaupdate.py   | 2 +-
 ipaserver/install/service.py| 4 ++--
 ipaserver/plugins/config.py | 4 ++--
 ipaserver/plugins/domainlevel.py| 2 +-
 ipaserver/plugins/group.py  | 2 +-
 ipaserver/plugins/hbactest.py   | 3 ++-
 ipaserver/plugins/host.py   | 2 +-
 ipaserver/plugins/privilege.py  | 2 +-
 ipaserver/plugins/selinuxusermap.py | 2 +-
 ipaserver/plugins/server.py | 2 +-
 ipaserver/plugins/service.py| 2 +-
 ipaserver/plugins/sudocmd.py| 2 +-
 ipaserver/session.py| 2 +-
 makeapi | 2 +-
 47 files changed, 54 insertions(+), 69 deletions(-)

diff --git a/daemons/dnssec/ipa-dnskeysync-replica b/daemons/dnssec/ipa-dnskeysync-replica
index 69a3a68..fbfee93 100755
--- a/daemons/dnssec/ipa-dnskeysync-replica
+++ b/daemons/dnssec/ipa-dnskeysync-replica
@@ -49,7 +49,7 @@ def update_metadata_set(log, source_set, target_set):
 def find_unwrapping_key(log, localhsm, wrapping_key_uri):
 wrap_keys = localhsm.find_keys(uri=wrapping_key_uri)
 # find usable unwrapping key with matching ID
-for key_id, key in wrap_keys.items():
+for key_id in wrap_keys.keys():
 unwrap_keys = localhsm.find_keys(id=key_id, cka_unwrap=True)
 if len(unwrap_keys) > 0:
 return unwrap_keys.popitem()[1]
diff --git a/doc/examples/examples.py b/doc/examples/examples.py
index 0ecbf1e..3389230 100644
--- a/doc/examples/examples.py
+++ b/doc/examples/examples.py
@@ -415,7 +415,7 @@ def execute(self, *args, **options):
 # patter expects them in one dict. We need to arrange that.
 for e in entries:
 e[1]['dn'] = e[0]
-entries = [e for (dn, e) in entries]
+entries = [e for (_dn, e) in entries]
 
 return dict(result=entries, count=len(entries), truncated=truncated)
 
diff --git