Re: [Freeipa-devel] [WIP] Thin client

[David Kupka]

On 28/04/16 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza



Hello!

Commit "schema: fix Flag arguments on the client" fixes regression 
reported in https://fedorahosted.org/freeipa/ticket/6009, ACK.


--
David Kupka

--
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 0042] Removed dead code from LDAPRemoveReverseMember

[Stanislav Laznicka]

On 06/28/2016 10:34 AM, Stanislav Laznicka wrote:

On 06/17/2016 09:14 AM, Stanislav Laznicka wrote:

On 06/14/2016 04:40 PM, Jan Cholasta wrote:

On 14.6.2016 16:35, Martin Basti wrote:

On 14.06.2016 16:37, Jan Cholasta wrote:

On 14.6.2016 16:29, Martin Basti wrote:

On 08.06.2016 14:17, Stanislav Laznicka wrote:

On 06/07/2016 10:42 AM, Martin Basti wrote:

On 07.06.2016 10:43, Jan Cholasta wrote:

On 7.6.2016 10:22, Martin Basti wrote:

On 07.06.2016 09:07, Jan Cholasta wrote:

On 6.6.2016 18:29, Martin Basti wrote:

On 03.06.2016 14:28, Stanislav Laznicka wrote:

On 06/03/2016 02:19 PM, Martin Basti wrote:

On 03.06.2016 14:13, Stanislav Laznicka wrote:

https://fedorahosted.org/freeipa/ticket/5892


NACK

please remove it from LDAPAddReverseMember too, it 
contains the

same
code

Martin^2


Please see the modified patch.

Standa


ACK

Pushed to master: c56d65b064e1e0410c03cf1206816cad4d8d86cc


I think the attrs_list was supposed to be passed to the
ldap.get_entry() call rather than removed, which would fix that
every
reverse member command always acts like --all was specified.


I'm really afraid, what can happen if we put attr_list into
get_entry()
instead of '*', because this code were there for 4 years and 
I don't

feel happy enough to change it now, what we may break.

Should I revert this commit then and postpone the ticket?


It's a bug and should be fixed. The fix is easy so I see no 
point in
postponing it. I see no reason to be really afraid, I'm pretty 
sure
that removing the objectclass attribute (which is invisible in 
the
CLI anyway) from the output of all the 4 commands that use 
this code

won't break anything.



Ok

It seems that tests expect objectClass to be always returned in
derived methods. Is that expected behavior? If so, please see the
attached patch. I wonder if the keys of the passed options 
should make

it to attrs_list as well (similarly to LDAPUpdate and LDAPCreate)?


I still don't think that we should use that attrs list, because
according git history, attrs_list was really used only with code 
that

was removed, and as you can see Standa had add extra objectclass
attribute there


So the assumption here is that if it's in git history, it's not a 
bug?


The extra object class should *not* be included by default.

(Also I don't see any reason to have this split into 2 patches.)

Right. I added the objectclass so that it behaves similar to what the 
other commands do and so that it does not break tests but it can be 
removed and tests could be fixed. The question here is - do we want 
it in 4.4, then? If so can we be sure it won't break anything 
(although we know that it's broken as it is, and it's been like that 
for the past 4 years)?


Currently, the LDAP*ReverseMember are used in adding/removing 
permissions/privileges to privileges/roles so I think we don't want 
it to go bad - but could it?


After discussion with Honza we agreed that the patch should not break 
anything while trying to fix the bug. Attached is the new patch with 
fixed tests.



Hopefully last modification - attrs_list should not be updated by 
received options as these don't reflect the entry's actual attributes.
From 29c4db5e6696ae1885a458914fad3f251b0a4e8a Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka 
Date: Tue, 7 Jun 2016 13:51:46 +0200
Subject: [PATCH] The LDAP*ReverseMember shouldn't imply --all is always
 specified

The LDAP*ReverseMember methods would always return the whole LDAP
object even though --all is not specified.
Also had to fix some tests as objectClass will not be returned by
default now.

https://fedorahosted.org/freeipa/ticket/5892
---
 ipaserver/plugins/baseldap.py  | 4 ++--
 ipatests/test_xmlrpc/test_permission_plugin.py | 4 
 ipatests/test_xmlrpc/test_role_plugin.py   | 5 -
 3 files changed, 2 insertions(+), 11 deletions(-)

diff --git a/ipaserver/plugins/baseldap.py b/ipaserver/plugins/baseldap.py
index 62b726da1a0baef9fc9fa4bb386a2101ca1f10b2..c35f660c790290aa0449ea2441a4fd0f5baed880 100644
--- a/ipaserver/plugins/baseldap.py
+++ b/ipaserver/plugins/baseldap.py
@@ -2159,7 +2159,7 @@ class LDAPAddReverseMember(LDAPModReverseMember):
 failed['member'][self.reverse_attr].append((attr, unicode(e)))
 
 # Update the member data.
-entry_attrs = ldap.get_entry(dn, ['*'])
+entry_attrs = ldap.get_entry(dn, attrs_list)
 self.obj.convert_attribute_members(entry_attrs, *keys, **options)
 
 for callback in self.get_callbacks('post'):
@@ -2258,7 +2258,7 @@ class LDAPRemoveReverseMember(LDAPModReverseMember):
 failed['member'][self.reverse_attr].append((attr, unicode(e)))
 
 # Update the member data.
-entry_attrs = ldap.get_entry(dn, ['*'])
+entry_attrs = ldap.get_entry(dn, attrs_list)
 self.obj.convert_attribute_members(entry_attrs, *keys, **options)
 
 for callback in self.get_callbacks('post'):
diff --git 

Re: [Freeipa-devel] [WIP] Thin client

[Jan Cholasta]

On 29.6.2016 09:22, David Kupka wrote:

On 28/04/16 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza



Hello!

Commit "schema: fix Flag arguments on the client" fixes regression
reported in https://fedorahosted.org/freeipa/ticket/6009, ACK.


Thanks, pushed to master: a77e21cbca05be422fe5826857cfba7e0ba6e71f

Attaching the patch for reference.

--
Jan Cholasta
From d61e4aff7e3ac4c01e32428768f01928b2fce023 Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Tue, 28 Jun 2016 08:40:48 +0200
Subject: [PATCH] schema: fix Flag arguments on the client

Fix Flag arguments appearing as Bool on the client.

https://fedorahosted.org/freeipa/ticket/6009
---
 ipaclient/remote_plugins/schema.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ipaclient/remote_plugins/schema.py b/ipaclient/remote_plugins/schema.py
index a54d4eb..7d5c8d0 100644
--- a/ipaclient/remote_plugins/schema.py
+++ b/ipaclient/remote_plugins/schema.py
@@ -220,7 +220,7 @@ class _SchemaPlugin(object):
 sensitive = False
 elif (type_name == 'bool' and
 'default' in schema and
-schema['default'] == [u'False']):
+schema['default'][0] == u'False'):
 cls = Flag
 del schema['default']
 else:
-- 
2.7.4

-- 
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 0042] Removed dead code from LDAPRemoveReverseMember

[Jan Cholasta]

On 29.6.2016 10:14, Stanislav Laznicka wrote:

On 06/28/2016 10:34 AM, Stanislav Laznicka wrote:

On 06/17/2016 09:14 AM, Stanislav Laznicka wrote:

On 06/14/2016 04:40 PM, Jan Cholasta wrote:

On 14.6.2016 16:35, Martin Basti wrote:

On 14.06.2016 16:37, Jan Cholasta wrote:

On 14.6.2016 16:29, Martin Basti wrote:

On 08.06.2016 14:17, Stanislav Laznicka wrote:

On 06/07/2016 10:42 AM, Martin Basti wrote:

On 07.06.2016 10:43, Jan Cholasta wrote:

On 7.6.2016 10:22, Martin Basti wrote:

On 07.06.2016 09:07, Jan Cholasta wrote:

On 6.6.2016 18:29, Martin Basti wrote:

On 03.06.2016 14:28, Stanislav Laznicka wrote:

On 06/03/2016 02:19 PM, Martin Basti wrote:

On 03.06.2016 14:13, Stanislav Laznicka wrote:

https://fedorahosted.org/freeipa/ticket/5892


NACK

please remove it from LDAPAddReverseMember too, it
contains the
same
code

Martin^2


Please see the modified patch.

Standa


ACK

Pushed to master: c56d65b064e1e0410c03cf1206816cad4d8d86cc


I think the attrs_list was supposed to be passed to the
ldap.get_entry() call rather than removed, which would fix that
every
reverse member command always acts like --all was specified.


I'm really afraid, what can happen if we put attr_list into
get_entry()
instead of '*', because this code were there for 4 years and
I don't
feel happy enough to change it now, what we may break.

Should I revert this commit then and postpone the ticket?


It's a bug and should be fixed. The fix is easy so I see no
point in
postponing it. I see no reason to be really afraid, I'm pretty
sure
that removing the objectclass attribute (which is invisible in
the
CLI anyway) from the output of all the 4 commands that use
this code
won't break anything.



Ok

It seems that tests expect objectClass to be always returned in
derived methods. Is that expected behavior? If so, please see the
attached patch. I wonder if the keys of the passed options
should make
it to attrs_list as well (similarly to LDAPUpdate and LDAPCreate)?


I still don't think that we should use that attrs list, because
according git history, attrs_list was really used only with code
that
was removed, and as you can see Standa had add extra objectclass
attribute there


So the assumption here is that if it's in git history, it's not a
bug?

The extra object class should *not* be included by default.

(Also I don't see any reason to have this split into 2 patches.)


Right. I added the objectclass so that it behaves similar to what the
other commands do and so that it does not break tests but it can be
removed and tests could be fixed. The question here is - do we want
it in 4.4, then? If so can we be sure it won't break anything
(although we know that it's broken as it is, and it's been like that
for the past 4 years)?

Currently, the LDAP*ReverseMember are used in adding/removing
permissions/privileges to privileges/roles so I think we don't want
it to go bad - but could it?


After discussion with Honza we agreed that the patch should not break
anything while trying to fix the bug. Attached is the new patch with
fixed tests.



Hopefully last modification - attrs_list should not be updated by
received options as these don't reflect the entry's actual attributes.


Thanks, ACK.

Pushed to master: 427bbf6c0d61cf52f14a9f2606143f994340ec83

--
Jan Cholasta

--
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] 0072..0075 Lightweight CA renewal

[Fraser Tweedale]

On Wed, Jun 29, 2016 at 09:30:17AM +0200, Jan Cholasta wrote:
> On 29.6.2016 08:55, Jan Cholasta wrote:
> > On 24.6.2016 08:49, Fraser Tweedale wrote:
> > > On Thu, Jun 23, 2016 at 09:51:02AM +0200, Jan Cholasta wrote:
> > > > Hi,
> > > > 
> > > > On 21.6.2016 08:24, Fraser Tweedale wrote:
> > > > > The attached patches add lightweight CA renewal.  There are two
> > > > > substantive aspects:
> > > > > 
> > > > > 1. The renew_ca_cert updates the serial number in the lightweight
> > > > > CA's entry in the Dogtag database.  This causes CA clones to observe
> > > > > the renewal and update the certs in their own NSSDBs.
> > > > > 
> > > > > 2. The ipa-certupdate command adds Certmonger tracking requests for
> > > > > lightweight CAs (on the renewal master only).
> > > > > 
> > > > > Correct behaviour also depends on my patch 0069 (in-server API for
> > > > > renew_ca_cert script).
> > > > 
> > > > Patch 0072-0074: LGTM
> > > > 
> > > > Patch 0075:
> > > > 
> > > > 1) Lightweight CA certs should be tracked by certmonger on all CA
> > > > servers,
> > > > not just on the renewal master. The behavior should be the same as
> > > > for the
> > > > main CA cert, i.e. the actual renewal is done only on the renewal
> > > > master,
> > > > other CA servers only update their NSS DBs (this is handled in
> > > > dogtag-ipa-ca-renew-agent-submit).
> > > > 
> > > > This is important because CA renewal master can change at any time, and
> > > > without all CA certs being tracked on all CA servers, there is no
> > > > guarantee
> > > > the renewal would happen.
> > > > 
> > > > 2) Since CA clones update their NSS DBs on their own,
> > > > dogtag-ipa-ca-renew-agent should be updated not to put them in
> > > > cn=ca_renewal,cn=ipa,cn=etc.
> > > > 
> > > Thanks for the review, Honza.  Updated patch 0075-2 attached.
> > 
> > Thanks, ACK.
> > 
> > Rebased patch 0072 and pushed to master:
> > 0078e7a9192a940104d8f6621b33d24d814c109b
> > 
> > It would be nice if lightweight CAs known at replica install time were
> > tracked without having to manually run ipa-certupdate after
> > ipa-replica-install. Shall I file a ticket for this, or will you be able
> > to provide a patch before Friday?
> 
> Also, the certs should be untracked on server uninstall.
> 
File the ticket, and I'll try to address by Friday anyways :)

Thanks,
Fraser

-- 
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] 0081 Add --ca option to cert-revoke and cert-remove-hold

[Fraser Tweedale]

On Wed, Jun 29, 2016 at 10:04:05AM +0200, Jan Cholasta wrote:
> Hi,
> 
> On 29.6.2016 06:11, Fraser Tweedale wrote:
> > Dear team,
> > 
> > The attached patch implements the --ca option for the rest of the
> > cert-blah commands (https://fedorahosted.org/freeipa/ticket/5999).
> 
> 1) I don't think cert-status should be treated specially. The operation to
> check status of a certificate request is not specific to Dogtag.
> 
I'm happy to add the option, with the caveat that because (of top of
head) there is not (yet) a way in Dogtag to distinguish/filter
requests by target CA, value may go unused.

> 
> 2) cert-show is called twice in cert-revoke. Can we call it just once?
> 
I'll address this in next patchset.

Thanks for reviewing!
Fraser

-- 
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] 0072..0075 Lightweight CA renewal

[Jan Cholasta]

On 24.6.2016 08:49, Fraser Tweedale wrote:

On Thu, Jun 23, 2016 at 09:51:02AM +0200, Jan Cholasta wrote:

Hi,

On 21.6.2016 08:24, Fraser Tweedale wrote:

The attached patches add lightweight CA renewal.  There are two
substantive aspects:

1. The renew_ca_cert updates the serial number in the lightweight
CA's entry in the Dogtag database.  This causes CA clones to observe
the renewal and update the certs in their own NSSDBs.

2. The ipa-certupdate command adds Certmonger tracking requests for
lightweight CAs (on the renewal master only).

Correct behaviour also depends on my patch 0069 (in-server API for
renew_ca_cert script).


Patch 0072-0074: LGTM

Patch 0075:

1) Lightweight CA certs should be tracked by certmonger on all CA servers,
not just on the renewal master. The behavior should be the same as for the
main CA cert, i.e. the actual renewal is done only on the renewal master,
other CA servers only update their NSS DBs (this is handled in
dogtag-ipa-ca-renew-agent-submit).

This is important because CA renewal master can change at any time, and
without all CA certs being tracked on all CA servers, there is no guarantee
the renewal would happen.

2) Since CA clones update their NSS DBs on their own,
dogtag-ipa-ca-renew-agent should be updated not to put them in
cn=ca_renewal,cn=ipa,cn=etc.


Thanks for the review, Honza.  Updated patch 0075-2 attached.


Thanks, ACK.

Rebased patch 0072 and pushed to master: 
0078e7a9192a940104d8f6621b33d24d814c109b


It would be nice if lightweight CAs known at replica install time were 
tracked without having to manually run ipa-certupdate after 
ipa-replica-install. Shall I file a ticket for this, or will you be able 
to provide a patch before Friday?


--
Jan Cholasta

--
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] 0080 cert-find: fix 'issuer' option

[Jan Cholasta]

On 29.6.2016 06:08, Fraser Tweedale wrote:

Hi,

The attached patch fixes a regression introduced by a patch
https://fedorahosted.org/freeipa/ticket/5381 (commit
d44ffdad4285bf2a1c0b044e07ef1b18c7d50de1).


Thanks, ACK.

Pushed to master: 6e4e522e524f40a6b05aeb1cc4b43655ed722e36

--
Jan Cholasta

--
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] 0072..0075 Lightweight CA renewal

[Jan Cholasta]

On 29.6.2016 08:55, Jan Cholasta wrote:

On 24.6.2016 08:49, Fraser Tweedale wrote:

On Thu, Jun 23, 2016 at 09:51:02AM +0200, Jan Cholasta wrote:

Hi,

On 21.6.2016 08:24, Fraser Tweedale wrote:

The attached patches add lightweight CA renewal.  There are two
substantive aspects:

1. The renew_ca_cert updates the serial number in the lightweight
CA's entry in the Dogtag database.  This causes CA clones to observe
the renewal and update the certs in their own NSSDBs.

2. The ipa-certupdate command adds Certmonger tracking requests for
lightweight CAs (on the renewal master only).

Correct behaviour also depends on my patch 0069 (in-server API for
renew_ca_cert script).


Patch 0072-0074: LGTM

Patch 0075:

1) Lightweight CA certs should be tracked by certmonger on all CA
servers,
not just on the renewal master. The behavior should be the same as
for the
main CA cert, i.e. the actual renewal is done only on the renewal
master,
other CA servers only update their NSS DBs (this is handled in
dogtag-ipa-ca-renew-agent-submit).

This is important because CA renewal master can change at any time, and
without all CA certs being tracked on all CA servers, there is no
guarantee
the renewal would happen.

2) Since CA clones update their NSS DBs on their own,
dogtag-ipa-ca-renew-agent should be updated not to put them in
cn=ca_renewal,cn=ipa,cn=etc.


Thanks for the review, Honza.  Updated patch 0075-2 attached.


Thanks, ACK.

Rebased patch 0072 and pushed to master:
0078e7a9192a940104d8f6621b33d24d814c109b

It would be nice if lightweight CAs known at replica install time were
tracked without having to manually run ipa-certupdate after
ipa-replica-install. Shall I file a ticket for this, or will you be able
to provide a patch before Friday?


Also, the certs should be untracked on server uninstall.

--
Jan Cholasta

--
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] 0081 Add --ca option to cert-revoke and cert-remove-hold

[Jan Cholasta]

Hi,

On 29.6.2016 06:11, Fraser Tweedale wrote:

Dear team,

The attached patch implements the --ca option for the rest of the
cert-blah commands (https://fedorahosted.org/freeipa/ticket/5999).


1) I don't think cert-status should be treated specially. The operation 
to check status of a certificate request is not specific to Dogtag.



2) cert-show is called twice in cert-revoke. Can we call it just once?


Honza

--
Jan Cholasta

--
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] [WIP] Thin client

[Jan Cholasta]

On 27.6.2016 16:44, Jan Cholasta wrote:

On 27.6.2016 14:55, David Kupka wrote:

On 28/04/16 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza



Hello!

Patch set:
schema: client-side cleanup
automember: fix automember to work with thin client
schema: do not crash in command_defaults if argument is None
schema: fix param default value handling

works* for me, ACK.


Thanks, pushed to master: f7cc15f0990ef2db57717a3c6a8e9db2c3dee951

Attaching the patches for reference.



*) xmlrpc test for automember_plugin test started failing with
automember patch. Fix for test attached.


LGTM


ACK.

Pushed to master: 95191e1612f93823bd0e325ef9b97fa9a4d7869c

--
Jan Cholasta

--
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] [Test][patch-0052] Test for incorrect client domain

[Oleg Fayans]

-- 
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.
From 6fe2f67807a2cd3d9519c1c919c884dd18867f74 Mon Sep 17 00:00:00 2001
From: Oleg Fayans 
Date: Wed, 29 Jun 2016 10:53:44 +0200
Subject: [PATCH] Test for incorrect client domain

https://fedorahosted.org/freeipa/ticket/5976
---
 .../test_integration/test_replica_promotion.py | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/ipatests/test_integration/test_replica_promotion.py b/ipatests/test_integration/test_replica_promotion.py
index 1f683b6d5c067ec526b307eea1460cafbadb80cb..e0d198b2385e443c79e6d3f5f6ecd0b564155e7b 100644
--- a/ipatests/test_integration/test_replica_promotion.py
+++ b/ipatests/test_integration/test_replica_promotion.py
@@ -377,3 +377,33 @@ class TestOldReplicaWorksAfterDomainUpgrade(IntegrationTest):
 result1 = self.master.run_command(['ipa', 'user-show', self.username],
   raiseonerr=False)
 assert_error(result1, "%s: user not found" % self.username, 2)
+
+
+class TestWrongClientDomain(IntegrationTest):
+topology = "star"
+num_clients = 1
+domain_name = 'exxample.test'
+realm_name = domain_name.upper()
+
+@classmethod
+def install(cls, mh):
+tasks.install_master(cls.master, domain_level=cls.domain_level)
+
+def test_wrong_client_domain(self):
+client = self.clients[0]
+result = client.run_command(['ipa-client-install', '-U',
+ '--domain', self.domain_name,
+ '--realm', self.realm_name,
+ '-w', self.master.config.dirman_password,
+ '--server', self.master.hostname],
+raiseonerr=False)
+assert_error(result,
+ "Failed to verify that %s is an IPA Server" %
+ self.master.hostname)
+result1 = client.run_command(['ipa-client-install', '-U', '--domain',
+  self.master.domain.realm, '-w',
+  self.master.config.dirman_password,
+  '--server', self.master.hostname],
+ raiseonerr=False)
+assert(result1.returncode == 0), (
+'Failed to setup client with the upcase domain name')
-- 
1.8.3.1

-- 
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] 0082 cert-request: better error msg when 'add' not supported

[Florence Blanc-Renaud]

On 06/29/2016 07:25 AM, Fraser Tweedale wrote:

The attached patch fixes
https://fedorahosted.org/freeipa/ticket/5991.

Thanks,
Fraser




Hi Fraser,

A few cosmetic comments:

PEP8 issues:
./ipalib/errors.py:1399:1: E302 expected 2 blank lines, found 1
./ipaserver/plugins/cert.py:394:80: E501 line too long (98 > 79 characters)
./ipaserver/plugins/cert.py:496:80: E501 line too long (81 > 79 characters)

and there is a typo in ipaserver/plugins/cert.py
+doc=_("automatically add the principal if it doesn't exist 
(service princpals only)"),


should be "princ*i*pals only"

Otherwise LGTM,
Flo

--
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] 0081 Add --ca option to cert-revoke and cert-remove-hold

[Jan Cholasta]

On 29.6.2016 10:47, Fraser Tweedale wrote:

On Wed, Jun 29, 2016 at 10:04:05AM +0200, Jan Cholasta wrote:

Hi,

On 29.6.2016 06:11, Fraser Tweedale wrote:

Dear team,

The attached patch implements the --ca option for the rest of the
cert-blah commands (https://fedorahosted.org/freeipa/ticket/5999).


1) I don't think cert-status should be treated specially. The operation to
check status of a certificate request is not specific to Dogtag.


I'm happy to add the option, with the caveat that because (of top of
head) there is not (yet) a way in Dogtag to distinguish/filter
requests by target CA, value may go unused.


IMO that's OK, since it's a safe non-descructive operation.





2) cert-show is called twice in cert-revoke. Can we call it just once?


I'll address this in next patchset.


OK.



Thanks for reviewing!
Fraser




--
Jan Cholasta

--
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] [Test][patch-0052] Test for incorrect client domain

[Martin Basti]

On 29.06.2016 10:56, Oleg Fayans wrote:





Hello,

+assert_error(result,
+ "Failed to verify that %s is an IPA Server" %
+ self.master.hostname)


I would expect this error there:

"Cannot promote this client to a replica. Local domain '{local}' does 
not match IPA domain '{ipadomain}'. "


You should not use random REALM, in this case you don't test domains but 
realms. You can leave the test with incorrect realm there, but as 
separated testcase


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

Re: [Freeipa-devel] [PATCH 661] backup: use in-server API in ipa-backup and ipa-restore

[Petr Vobornik]

On 06/29/2016 02:54 PM, Jan Cholasta wrote:
> Hi,
> 
> the attached patch fixes .
> 
> Honza
> 

Milan, could you run backup test suite with this patch? I've a feeling
that something else there might be also broken.
-- 
Petr Vobornik

-- 
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 0096] Add authentication indicators support to Host objects

[Martin Basti]

On 29.06.2016 15:52, Stanislav Laznicka wrote:

On 06/24/2016 03:14 PM, Martin Basti wrote:



On 24.06.2016 15:11, Sumit Bose wrote:

On Tue, Jun 21, 2016 at 02:25:49PM -0400, Nathaniel McCallum wrote:

https://fedorahosted.org/freeipa/ticket/433
The patch works for me as expected, but the API.txt update is 
missing in

the patch.

bye,
Sumit


There are no updated managed permissions for krbprincipalauthind 
attribute in hosts.py, is this omitted on purpose?

Martin^2


The attached patch adds them should these be required.




Then we also needs patch for services.py, because there are missing ACIs too

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

Re: [Freeipa-devel] [PATCH 0096] Add authentication indicators support to Host objects

[Stanislav Laznicka]

On 06/24/2016 03:14 PM, Martin Basti wrote:



On 24.06.2016 15:11, Sumit Bose wrote:

On Tue, Jun 21, 2016 at 02:25:49PM -0400, Nathaniel McCallum wrote:

https://fedorahosted.org/freeipa/ticket/433

The patch works for me as expected, but the API.txt update is missing in
the patch.

bye,
Sumit


There are no updated managed permissions for krbprincipalauthind 
attribute in hosts.py, is this omitted on purpose?

Martin^2


The attached patch adds them should these be required.


From becd1e2d284dcd98a2ba35fcd68e0f9354f0a365 Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka 
Date: Wed, 29 Jun 2016 15:45:28 +0200
Subject: [PATCH] Added permissions for auth. indicators read/modify

Added permissions for Kerberos authentication indicators reading and
modifying to host and service objects.

https://fedorahosted.org/freeipa/ticket/433
---
 ipaserver/plugins/host.py| 3 ++-
 ipaserver/plugins/service.py | 4 ++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/ipaserver/plugins/host.py b/ipaserver/plugins/host.py
index 1091f85748d675c479285ad73465aa9541c61b45..be4a1711f3d6b7ee3bc12cbee1c705a9067f73b2 100644
--- a/ipaserver/plugins/host.py
+++ b/ipaserver/plugins/host.py
@@ -333,7 +333,7 @@ class host(LDAPObject):
 'enrolledby', 'managedby', 'ipaassignedidview',
 'krbprincipalname', 'krbcanonicalname', 'krbprincipalaliases',
 'krbprincipalexpiration', 'krbpasswordexpiration',
-'krblastpwdchange',
+'krblastpwdchange', 'krbprincipalauthind',
 },
 },
 'System: Read Host Membership': {
@@ -411,6 +411,7 @@ class host(LDAPObject):
 'ipapermdefaultattr': {
 'description', 'l', 'nshardwareplatform', 'nshostlocation',
 'nsosversion', 'macaddress', 'userclass', 'ipaassignedidview',
+'krbprincipalauthind',
 },
 'replaces': [
 '(targetattr = "description || l || nshostlocation || nshardwareplatform || nsosversion")(target = "ldap:///fqdn=*,cn=computers,cn=accounts,$SUFFIX;)(version 3.0;acl "permission:Modify Hosts";allow (write) groupdn = "ldap:///cn=Modify Hosts,cn=permissions,cn=pbac,$SUFFIX";)',
diff --git a/ipaserver/plugins/service.py b/ipaserver/plugins/service.py
index 701314f8d9f2ac14c2b92fea1b75c7bf1754dac3..bc5bf529b45568d63e2a5b99906a7755d4ac8d40 100644
--- a/ipaserver/plugins/service.py
+++ b/ipaserver/plugins/service.py
@@ -437,7 +437,7 @@ class service(LDAPObject):
 'krbprincipalname', 'krbcanonicalname', 'krbprincipalaliases',
 'krbprincipalexpiration', 'krbpasswordexpiration',
 'krblastpwdchange', 'ipakrbauthzdata', 'ipakrbprincipalalias',
-'krbobjectreferences',
+'krbobjectreferences', 'krbprincipalauthind',
 },
 },
 'System: Add Services': {
@@ -465,7 +465,7 @@ class service(LDAPObject):
 },
 'System: Modify Services': {
 'ipapermright': {'write'},
-'ipapermdefaultattr': {'usercertificate'},
+'ipapermdefaultattr': {'usercertificate', 'krbprincipalauthind'},
 'replaces': [
 '(targetattr = "usercertificate")(target = "ldap:///krbprincipalname=*,cn=services,cn=accounts,$SUFFIX;)(version 3.0;acl "permission:Modify Services";allow (write) groupdn = "ldap:///cn=Modify Services,cn=permissions,cn=pbac,$SUFFIX";)',
 ],
-- 
2.5.5

-- 
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 0096] Add authentication indicators support to Host objects

[Stanislav Laznicka]

On 06/29/2016 03:53 PM, Martin Basti wrote:



On 29.06.2016 15:52, Stanislav Laznicka wrote:

On 06/24/2016 03:14 PM, Martin Basti wrote:



On 24.06.2016 15:11, Sumit Bose wrote:

On Tue, Jun 21, 2016 at 02:25:49PM -0400, Nathaniel McCallum wrote:

https://fedorahosted.org/freeipa/ticket/433
The patch works for me as expected, but the API.txt update is 
missing in

the patch.

bye,
Sumit


There are no updated managed permissions for krbprincipalauthind 
attribute in hosts.py, is this omitted on purpose?

Martin^2


The attached patch adds them should these be required.




Then we also needs patch for services.py, because there are missing 
ACIs too


Martin^2


It was already included but let me separate it in two patches, then.

From d05969e29aa190602ae9f90c6e6161e517b0ad0d Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka 
Date: Wed, 29 Jun 2016 15:56:55 +0200
Subject: [PATCH 1/2] host: Added permissions for auth. indicators read/modify

Added permissions for Kerberos authentication indicators reading and
modifying to host objects.

https://fedorahosted.org/freeipa/ticket/433
---
 ipaserver/plugins/host.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ipaserver/plugins/host.py b/ipaserver/plugins/host.py
index 0072431de3f130d09066100f12d9fcb34e9fb96b..c54439e9b55de85d871241083ccb512cc1a88f29 100644
--- a/ipaserver/plugins/host.py
+++ b/ipaserver/plugins/host.py
@@ -333,7 +333,7 @@ class host(LDAPObject):
 'enrolledby', 'managedby', 'ipaassignedidview',
 'krbprincipalname', 'krbcanonicalname', 'krbprincipalaliases',
 'krbprincipalexpiration', 'krbpasswordexpiration',
-'krblastpwdchange',
+'krblastpwdchange', 'krbprincipalauthind',
 },
 },
 'System: Read Host Membership': {
@@ -411,6 +411,7 @@ class host(LDAPObject):
 'ipapermdefaultattr': {
 'description', 'l', 'nshardwareplatform', 'nshostlocation',
 'nsosversion', 'macaddress', 'userclass', 'ipaassignedidview',
+'krbprincipalauthind',
 },
 'replaces': [
 '(targetattr = "description || l || nshostlocation || nshardwareplatform || nsosversion")(target = "ldap:///fqdn=*,cn=computers,cn=accounts,$SUFFIX;)(version 3.0;acl "permission:Modify Hosts";allow (write) groupdn = "ldap:///cn=Modify Hosts,cn=permissions,cn=pbac,$SUFFIX";)',
-- 
2.5.5

From 3a503b91680b49afc5bc0ba39ec5451f5b0352a1 Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka 
Date: Wed, 29 Jun 2016 15:58:07 +0200
Subject: [PATCH 2/2] service: Added permissions for auth. indicators
 read/modify

Added permissions for Kerberos authentication indicators reading and
modifying to service objects.
---
 ipaserver/plugins/service.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ipaserver/plugins/service.py b/ipaserver/plugins/service.py
index 701314f8d9f2ac14c2b92fea1b75c7bf1754dac3..bc5bf529b45568d63e2a5b99906a7755d4ac8d40 100644
--- a/ipaserver/plugins/service.py
+++ b/ipaserver/plugins/service.py
@@ -437,7 +437,7 @@ class service(LDAPObject):
 'krbprincipalname', 'krbcanonicalname', 'krbprincipalaliases',
 'krbprincipalexpiration', 'krbpasswordexpiration',
 'krblastpwdchange', 'ipakrbauthzdata', 'ipakrbprincipalalias',
-'krbobjectreferences',
+'krbobjectreferences', 'krbprincipalauthind',
 },
 },
 'System: Add Services': {
@@ -465,7 +465,7 @@ class service(LDAPObject):
 },
 'System: Modify Services': {
 'ipapermright': {'write'},
-'ipapermdefaultattr': {'usercertificate'},
+'ipapermdefaultattr': {'usercertificate', 'krbprincipalauthind'},
 'replaces': [
 '(targetattr = "usercertificate")(target = "ldap:///krbprincipalname=*,cn=services,cn=accounts,$SUFFIX;)(version 3.0;acl "permission:Modify Services";allow (write) groupdn = "ldap:///cn=Modify Services,cn=permissions,cn=pbac,$SUFFIX";)',
 ],
-- 
2.5.5

-- 
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] Kerberos Principal Aliases Testplan review

[Martin Babinsky]

Hi,

I have looked at the testplan[1] and have the following comments:

In general LGTM, but I miss the following test scenarios:

1.) Test principal alias removal, more specifically test that the 
removal of the alias equivalent to the canonical name triggers an error


2.) Test that you cannot create an enterprise principal alias whose 
suffix overlaps with trusted domains UPN[2]. You do not need trust for 
this, just a domain entry in LDAP, see 
`test_xmlrpc/test_range_plugin.py` and MockLDAP class for hints.


Basically you should get an error when adding principal alias such as 
'user\@trusted.domain.upn@REALM' regardless of the case of 
'trusted.domain.upn'.


3.) test that when adding alias to an entry lacking 'krbcanonicalname' 
(e.g. old entry from upgrade), the existing value of 'krbprincipalname' 
is copied to the attribute


That is all I can currently think of off the top of my head.


[1] http://www.freeipa.org/page/V4/Kerberos_principal_aliases/Test_Plan
[2] http://www.freeipa.org/page/V4/Support_of_UPN_for_trusted_domains
--
Martin^3 Babinsky

--
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] 0008 Do not allow installation in FIPS mode

[Martin Basti]

On 29.06.2016 13:04, Martin Basti wrote:



On 28.06.2016 16:57, Florence Blanc-Renaud wrote:

On 06/28/2016 11:05 AM, Martin Basti wrote:



On 28.06.2016 10:51, Florence Blanc-Renaud wrote:

On 06/27/2016 10:18 PM, Rob Crittenden wrote:

Florence Blanc-Renaud wrote:

Hi all,

thanks for your suggestions. Updated patch attached.
Flo.



The invocation in ipactl should say server, not client.

Otherwise LGTM (untested).

rob


Hi all,

thanks to Rob for catching the typo.
Patch with updated message is attached,
Flo.




Thank you for the patch I have two comments:

1)
+except Exception:
+# Consider that the host is not fips-enabled if the file does
not exist
+pass

exceptions should be as much specific as possible, otherwise it may 
mask

real issues
please use 'except IOError' if you want catch the case that file does
not exist

2)
in replicainstall.py and install.py please raise exception
(RuntimeError) instead of sys.exit() to keep proper logging, 
cleanup, etc.


Sys.exit() should not be used in modules, it is hard to debug etc. It
can be used only in scripts (ipa-client-install, ipa-replica-manage, 
etc..)


Martin^2


Hi,

hopefully converging with this updated patch :)
Thanks for all the comments, I'm learning tips with each iteration.

Flo.

I propose following changes (in attached patch). If you agree I can 
squash patches and push it.


Martin^2




ACK

pushed to
master:
* 3c40d3aa9e3d431be1e625aa91cdcbeffd0d1271 Do not allow installation in 
FIPS mode


ipa-4-3:
* 4ce0ff61a8e46de4a2f2dfca41610323f9569d8a Do not allow installation in 
FIPS mode
-- 
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] 0061: webui: Add support for 'dns_update_system_records' command

[Petr Vobornik]

On 06/28/2016 05:50 PM, Petr Vobornik wrote:
> On 06/28/2016 05:38 PM, Pavel Vomacka wrote:
>>
> 
> ACK
> 

master:
* 31a13c9e9849eca794aa7908bc252185c4b36678 Add button for
dns_update_system_records command

-- 
Petr Vobornik

-- 
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 0096] Add authentication indicators support to Host objects

[Stanislav Laznicka]

On 06/29/2016 04:02 PM, Stanislav Laznicka wrote:

On 06/29/2016 03:53 PM, Martin Basti wrote:



On 29.06.2016 15:52, Stanislav Laznicka wrote:

On 06/24/2016 03:14 PM, Martin Basti wrote:



On 24.06.2016 15:11, Sumit Bose wrote:

On Tue, Jun 21, 2016 at 02:25:49PM -0400, Nathaniel McCallum wrote:

https://fedorahosted.org/freeipa/ticket/433
The patch works for me as expected, but the API.txt update is 
missing in

the patch.

bye,
Sumit


There are no updated managed permissions for krbprincipalauthind 
attribute in hosts.py, is this omitted on purpose?

Martin^2


The attached patch adds them should these be required.




Then we also needs patch for services.py, because there are missing 
ACIs too


Martin^2


It was already included but let me separate it in two patches, then.



Good catch from Petr Vobornik - the rebuilt ACI.txt should also be included.

From 9a80066123e8e97fb9c9daed4f339a5d5368faf3 Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka 
Date: Wed, 29 Jun 2016 15:56:55 +0200
Subject: [PATCH 1/2] host: Added permissions for auth. indicators read/modify

Added permissions for Kerberos authentication indicators reading and
modifying to host objects.

https://fedorahosted.org/freeipa/ticket/433
---
 ACI.txt   | 4 ++--
 ipaserver/plugins/host.py | 3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/ACI.txt b/ACI.txt
index 98566de..86955c5 100644
--- a/ACI.txt
+++ b/ACI.txt
@@ -137,13 +137,13 @@ aci: (targetattr = "createtimestamp || entryusn || ipaallowedtoperform;read_keys
 dn: cn=computers,cn=accounts,dc=ipa,dc=example
 aci: (targetattr = "ipasshpubkey")(targetfilter = "(objectclass=ipahost)")(version 3.0;acl "permission:System: Manage Host SSH Public Keys";allow (write) groupdn = "ldap:///cn=System: Manage Host SSH Public Keys,cn=permissions,cn=pbac,dc=ipa,dc=example";)
 dn: cn=computers,cn=accounts,dc=ipa,dc=example
-aci: (targetattr = "description || ipaassignedidview || l || macaddress || nshardwareplatform || nshostlocation || nsosversion || userclass")(targetfilter = "(objectclass=ipahost)")(version 3.0;acl "permission:System: Modify Hosts";allow (write) groupdn = "ldap:///cn=System: Modify Hosts,cn=permissions,cn=pbac,dc=ipa,dc=example";)
+aci: (targetattr = "description || ipaassignedidview || krbprincipalauthind || l || macaddress || nshardwareplatform || nshostlocation || nsosversion || userclass")(targetfilter = "(objectclass=ipahost)")(version 3.0;acl "permission:System: Modify Hosts";allow (write) groupdn = "ldap:///cn=System: Modify Hosts,cn=permissions,cn=pbac,dc=ipa,dc=example";)
 dn: dc=ipa,dc=example
 aci: (targetattr = "cn || createtimestamp || entryusn || macaddress || modifytimestamp || objectclass")(target = "ldap:///cn=computers,cn=compat,dc=ipa,dc=example;)(version 3.0;acl "permission:System: Read Host Compat Tree";allow (compare,read,search) userdn = "ldap:///anyone;;)
 dn: cn=computers,cn=accounts,dc=ipa,dc=example
 aci: (targetattr = "memberof")(targetfilter = "(objectclass=ipahost)")(version 3.0;acl "permission:System: Read Host Membership";allow (compare,read,search) userdn = "ldap:///all;;)
 dn: cn=computers,cn=accounts,dc=ipa,dc=example
-aci: (targetattr = "cn || createtimestamp || description || enrolledby || entryusn || fqdn || ipaassignedidview || ipaclientversion || ipakrbauthzdata || ipasshpubkey || ipauniqueid || krbcanonicalname || krblastpwdchange || krbpasswordexpiration || krbprincipalaliases || krbprincipalexpiration || krbprincipalname || l || macaddress || managedby || modifytimestamp || nshardwareplatform || nshostlocation || nsosversion || objectclass || serverhostname || usercertificate || userclass")(targetfilter = "(objectclass=ipahost)")(version 3.0;acl "permission:System: Read Hosts";allow (compare,read,search) userdn = "ldap:///all;;)
+aci: (targetattr = "cn || createtimestamp || description || enrolledby || entryusn || fqdn || ipaassignedidview || ipaclientversion || ipakrbauthzdata || ipasshpubkey || ipauniqueid || krbcanonicalname || krblastpwdchange || krbpasswordexpiration || krbprincipalaliases || krbprincipalauthind || krbprincipalexpiration || krbprincipalname || l || macaddress || managedby || modifytimestamp || nshardwareplatform || nshostlocation || nsosversion || objectclass || serverhostname || usercertificate || userclass")(targetfilter = "(objectclass=ipahost)")(version 3.0;acl "permission:System: Read Hosts";allow (compare,read,search) userdn = "ldap:///all;;)
 dn: cn=computers,cn=accounts,dc=ipa,dc=example
 aci: (targetfilter = "(objectclass=ipahost)")(version 3.0;acl "permission:System: Remove Hosts";allow (delete) groupdn = "ldap:///cn=System: Remove Hosts,cn=permissions,cn=pbac,dc=ipa,dc=example";)
 dn: cn=hostgroups,cn=accounts,dc=ipa,dc=example
diff --git a/ipaserver/plugins/host.py b/ipaserver/plugins/host.py
index 0072431..c54439e 100644
--- a/ipaserver/plugins/host.py
+++ b/ipaserver/plugins/host.py
@@ -333,7 +333,7 @@ class host(LDAPObject):

Re: [Freeipa-devel] [PATCH] 0058 WebUI: certificate widget on ID override user page

[Petr Vobornik]

On 06/27/2016 04:34 PM, Pavel Vomacka wrote:
> 
> 
> On 06/23/2016 04:25 PM, Petr Vobornik wrote:
>> On 06/20/2016 06:54 PM, Pavel Vomacka wrote:
>>> Hello,
>>>
>>> please review attached patch.
>>>
>>> https://fedorahosted.org/freeipa/ticket/5926
>>>
>> 1. I'm not sure whether to include the certificate field in the adder
>> dialog. But if so then it is not good that it accepts different output
>> then a cert widget. The difference is that cert widget is able to work
>> also with "-BEGIN CERTIFICATE-" and "-END CERTIFICATE-"
>> values.
> Oh, you are right, I forgot to add custom widget which handles that.
> Fixed now.
>>
>> Otherwise it works good.
>>
>> If we decided not to fix 1. in this patch then ACK.
>>
>> Push should wait on the other cert patches.
>>
> Updated patch attached.
> 


ACK

master:
* aaf65e9c56c75d78d1c1f7dcefdb52dd3ddc419a Add certificate widget to ID
override user details page.
-- 
Petr Vobornik

-- 
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] 0064: webui: simplify confirmation messages in confirmation dialogs

[Petr Vobornik]

On 06/27/2016 05:50 PM, Pavel Vomacka wrote:
> Hello,
> 
> Please review attached patch which simplifies confirmation messages for
> 'remove cert hold' and 'restore cert' actions.
> 

I'd change:
  You can select a reason from the pull-down list.
To:
  Select a reason from the pull-down list.

-- 
Petr Vobornik

-- 
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] 0062, 63: webui: Add button for 'server-del' command

[Petr Vobornik]

On 06/24/2016 12:40 PM, Pavel Vomacka wrote:
> Hello,
> 
> please review attached patches, they add 'Delete Server' button.
> 

1. there is a whitespace warning while applying patch 63.

2. It breaks expectation of no_init.
Instead of
  var that = IPA.details_facet(spec);
Use
  var that = IPA.details_facet(spec, no_init);

Then rename:
  that.init_facet = function() {
ẗo
  that.init_servers_facet = function() {

Add there at beginning:
  that.init_details_facet


3. IPA.action and IPA.delete_action has the functionality of
server_delete_action build-in, including redirection to server search
page which is missing in this patch.

Updated patch with the issues fixed is attached.

I did not test final version of the patch because my testing env. died.
-- 
Petr Vobornik
From d0ea026cebcda0e301ed75b4dc53c78675c20ef0 Mon Sep 17 00:00:00 2001
From: Pavel Vomacka 
Date: Fri, 24 Jun 2016 12:08:04 +0200
Subject: [PATCH] Add button for server-del command

WebUI counterpart of: https://fedorahosted.org/freeipa/ticket/5588
---
 install/ui/src/freeipa/topology.js | 65 +-
 install/ui/test/data/ipa_init.json |  4 +++
 ipaserver/plugins/internal.py  |  4 +++
 3 files changed, 72 insertions(+), 1 deletion(-)

diff --git a/install/ui/src/freeipa/topology.js b/install/ui/src/freeipa/topology.js
index a9776a5567adf1dcea7a4e62c89f90a573e9ca66..7e501eb3506587ea653497d2806938890066e4f0 100644
--- a/install/ui/src/freeipa/topology.js
+++ b/install/ui/src/freeipa/topology.js
@@ -222,6 +222,7 @@ return {
 },
 {
 $type: 'details',
+$factory: topology.servers_facet,
 disable_facet_tabs: true,
 sections: [
 {
@@ -253,6 +254,30 @@ return {
 }
 ]
 }
+],
+actions: [
+{
+$factory: IPA.delete_action,
+name: 'server_del',
+method: 'del',
+label: '@i18n:objects.servers.remove_server',
+needs_confirm: true,
+confirm_msg: '@i18n:objects.servers.remove_server_msg',
+confirm_dialog: {
+$factory: IPA.confirm_dialog,
+title: '@i18n:objects.servers.remove_server',
+ok_label: '@i18n:buttons.remove',
+ok_button_class: 'btn btn-danger'
+}
+}
+],
+control_buttons_right: [
+{
+name: 'server_del',
+label: '@i18n:objects.servers.remove_server',
+icon: 'fa-exclamation-circle',
+button_class: 'btn btn-danger'
+}
 ]
 }
 ]
@@ -458,6 +483,45 @@ topology.location_adapter = declare([mod_field.Adapter], {
 }
 });
 
+topology.servers_facet = function(spec, no_init) {
+spec = spec || {};
+
+var that = IPA.details_facet(spec, no_init);
+
+/**
+ * Creates buttons on the right side of facet.
+ */
+that.create_controls = function() {
+
+that.create_control_buttons(that.controls_left);
+that.create_action_dropdown(that.controls_left);
+
+that.control_buttons = that.control_buttons_right;
+that.create_control_buttons(that.controls_right);
+};
+
+/**
+ * Inits right facet buttons.
+ */
+that.init_servers_facet = function() {
+
+that.init_details_facet();
+var buttons_spec = {
+$factory: IPA.control_buttons_widget,
+name: 'control-buttons',
+css_class: 'control-buttons',
+buttons: spec.control_buttons_right
+};
+
+that.control_buttons_right = IPA.build(buttons_spec);
+that.control_buttons_right.init(that);
+};
+
+if (!no_init) that.init_servers_facet();
+
+return that;
+};
+
 topology.serverroles_search_facet = function(spec) {
 
 spec = spec || {};
@@ -1404,7 +1468,6 @@ topology.register = function() {
 ctor: topology.TopologyGraphFacet,
 spec: topology.topology_graph_facet_spec
 });
-
 };
 
 phases.on('registration', topology.register);
diff --git a/install/ui/test/data/ipa_init.json b/install/ui/test/data/ipa_init.json
index 49eca4de9d40a20b1aee593e32071c762ff052d1..c18f78cc316a77da46f14fa2b3bbbabe8ee09208 100644
--- a/install/ui/test/data/ipa_init.json
+++ b/install/ui/test/data/ipa_init.json
@@ -539,6 +539,10 @@
 "label": "Server Roles",
 "label_singular": "Server Role",
 },
+"servers": {
+"remove_server": "Delete Server",
+"remove_server_msg": "Deleting a server removes it permanently from the topology. Note that this is a non-reversible action."
+   

Re: [Freeipa-devel] [PATCH 0018][Tests] Fix some of the failing tests in test_ipalib/test_frontend.py

[Ganna Kaihorodova]

Hello!

ACK

Best regards,
Ganna Kaihorodova
Associate Software Quality Engineer


- Original Message -
From: "Lenka Doudova" 
To: "freeipa-devel" 
Sent: Tuesday, June 21, 2016 10:21:44 AM
Subject: [Freeipa-devel] [PATCH 0018][Tests] Fix some of the failing tests in 
test_ipalib/test_frontend.py

Hi,

attaching patch with fix for a few failing tests in 
ipatests/test_ipalib/test_frontend.py.


Lenka


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

-- 
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 0023][Tests] Fix frontend tests - #5987

[Ganna Kaihorodova]

Hello!

ACK

Best regards,
Ganna Kaihorodova
Associate Software Quality Engineer


- Original Message -
From: "Lenka Doudova" 
To: "freeipa-devel" 
Sent: Tuesday, June 28, 2016 6:34:33 AM
Subject: [Freeipa-devel] [PATCH 0023][Tests] Fix frontend tests - #5987

Hi,

I've made patch to fix for https://fedorahosted.org/freeipa/ticket/5987.

Please note, that this patch must be applied on top on my patch no. 
0018, which provides other fixes on the same file (and same test).


Lenka


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

-- 
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 0024][Tests] Fix integration tests not to produce incorrect /etc/hosts file

[Lenka Doudova]

Hi all,

a function 'fix_etc_hosts' in ipatests/test_integration/tasks.py 
produces incorrect /etc/hosts file (solitary IPv6 address), and 
currently parser is not able to resolve the issue, causing 
ipa-server-install to fail with 'list index out of range' error.


Hence I'm attaching patch to fix this issue before parser is fixed 
(related ticket to it #6014). The fix is just change of regexs 
responsible for creating incorrect file so that all the lines containing 
defined hostname are removed, not just specific IP/hostname/shortname 
strings.



Lenka

From a2be2991041725d03d53d1cff04fc45290acd092 Mon Sep 17 00:00:00 2001
From: Lenka Doudova 
Date: Wed, 29 Jun 2016 18:13:23 +0200
Subject: [PATCH] Tests: Fix integration tests not to produce incorrect
 /etc/hosts file

Function 'fix_etc_hosts' in ipatests/test_integration/tasks.py produces incorrectly formatted /etc/hosts file (solitary IPv6 address), which causes ipa-server-install failure.
This provides fix of the function so that the regex changing /etc/hosts file remove entire lines with hostname entry in them.
---
 ipatests/test_integration/tasks.py | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py
index 38218fa709c2c220d5fea98a092b55e995d48d77..61f772638b319a74e935d4962b5921e451e9bb27 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -124,11 +124,7 @@ def fix_etc_hosts(host):
 # Remove existing mentions of the host's FQDN, short name, and IP
 # Removing of IP must be done as first, otherwise hosts file may be
 # corrupted
-contents = re.sub('^%s.*' % re.escape(host.ip), '', contents,
-  flags=re.MULTILINE)
-contents = re.sub('\s%s(\s|$)' % re.escape(host.hostname), ' ', contents,
-  flags=re.MULTILINE)
-contents = re.sub('\s%s(\s|$)' % re.escape(host.shortname), ' ', contents,
+contents = re.sub('^.*%s.*$' % re.escape(host.hostname), '', contents,
   flags=re.MULTILINE)
 # Add the host's info again
 contents += '\n%s %s %s\n' % (host.ip, host.hostname, host.shortname)
-- 
2.7.4

-- 
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] 0065, 66: webui: authentication indicators on host page

[Petr Vobornik]

On 06/28/2016 04:32 PM, Pavel Vomacka wrote:
> Hello,
> 
> please review attached patches. I moved strings used by authentication
> indicators widget to another dict so the second patch changes strings in
> custom_checkbox widget on service page.
> 
> https://fedorahosted.org/freeipa/ticket/5872
> 


ACK

push should wait on server side.

-- 
Petr Vobornik

-- 
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 0024][Tests] Fix integration tests not to produce incorrect /etc/hosts file

[Oleg Fayans]

In fact, I believe /etc/hosts file should not be touched at all.
Hostname resolution is usually governed by the DNS system of the lab in
which tests are running. We do not modify it when perform tests
manually, so I'd rather remove this method at all.

On 06/29/2016 06:27 PM, Lenka Doudova wrote:
> Hi all,
> 
> a function 'fix_etc_hosts' in ipatests/test_integration/tasks.py
> produces incorrect /etc/hosts file (solitary IPv6 address), and
> currently parser is not able to resolve the issue, causing
> ipa-server-install to fail with 'list index out of range' error.
> 
> Hence I'm attaching patch to fix this issue before parser is fixed
> (related ticket to it #6014). The fix is just change of regexs
> responsible for creating incorrect file so that all the lines containing
> defined hostname are removed, not just specific IP/hostname/shortname
> strings.
> 
> 
> Lenka
> 
> 
> 

-- 
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.

-- 
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 0024][Tests] Fix integration tests not to produce incorrect /etc/hosts file

[Petr Spacek]

On 29.6.2016 18:39, Oleg Fayans wrote:
> In fact, I believe /etc/hosts file should not be touched at all.
> Hostname resolution is usually governed by the DNS system of the lab in
> which tests are running. We do not modify it when perform tests
> manually, so I'd rather remove this method at all.

+1, it should not be need. Let me know if it is needed for some reason and I
will have a look.

Petr^2 Spacek

> On 06/29/2016 06:27 PM, Lenka Doudova wrote:
>> Hi all,
>>
>> a function 'fix_etc_hosts' in ipatests/test_integration/tasks.py
>> produces incorrect /etc/hosts file (solitary IPv6 address), and
>> currently parser is not able to resolve the issue, causing
>> ipa-server-install to fail with 'list index out of range' error.
>>
>> Hence I'm attaching patch to fix this issue before parser is fixed
>> (related ticket to it #6014). The fix is just change of regexs
>> responsible for creating incorrect file so that all the lines containing
>> defined hostname are removed, not just specific IP/hostname/shortname
>> strings.
>>
>>
>> Lenka

-- 
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 0022][Tests] Prevent trust test failures cause by adding duplicate DNS forward zone

[Petr Spacek]

On 29.6.2016 18:48, Lenka Doudova wrote:
> 
> 
> On 06/27/2016 11:05 AM, Lenka Doudova wrote:
>>
>>
>> On 06/27/2016 10:33 AM, Martin Babinsky wrote:
>>> On 06/27/2016 10:28 AM, Petr Spacek wrote:
 On 27.6.2016 10:26, Petr Spacek wrote:
> On 27.6.2016 10:18, Martin Babinsky wrote:
>> On 06/27/2016 10:04 AM, Petr Vobornik wrote:
>>> On 06/27/2016 09:42 AM, Lenka Doudova wrote:
 Hi!

 With newly created AD machines in Brno lab, existing trust tests fail 
 on
 'ipa dnsforwardzone-add' command claiming the zone is already present,
 as new AD domain is dom-221.idm.lab.eng.brq.redhat.com.

 To prevent these failures I prepared attached patch, that will still
 attempt to add the forward zone, but in case of non-zero return code
 will check the message if it says that the forward zone is already
 configured, and lets the tests continue, if it is so.


 Lenka

>>>
>>>
>>> Current approach expects that every error of ipa dnsforward-add here
>>> will mean that the zone exists. So it might hide other issues - not very
>>> good.
>>>
>>> On the other hand it is not very robust to parse error message.
>>>
>>> Question for general audience: What do you think if IPA client's exit
>>> status would be the IPA error code instead of "1" for every error. E.g.
>>> in DuplicateEntry case it's 4002.
>>>
>>> Btw, this is not a NACK.
>>>
>>
>> Well AFAIK the exit status on POSIX systems is encoded into a single
>> byte so
>> you cannot have the return value greater that 255. We would have to 
>> devise
>> some mapping between our XMLRPC status codes and subprocess return codes.
>>
>> Some of our exceptions have defined return values outside plain '1', e.g.
>> NotFound has return value of 2. It would be possible to extend this
>> concept on
>> other common errors.
>
> Even more importantly, the forward zone is completely unnecessary because
> DNS
> when DNS is set up properly. I would simply remove the dnsforwardzone-add.
>
 Grr, I meant this:
 Even more importantly, the forward zone is completely unnecessary when DNS 
 is
 set up properly. I would simply remove the dnsforwardzone-add.

>>> +1, our tests should not fiddle with the provisioned environment as much as
>>> they sometimes do.
>>>
>> Well, I have nothing against removing it completely, but left it there just
>> because with previous AD machines with "wild" domains it was necessary.
>> Looking at the code, your suggestion would probably mean to entirely remove
>> method configure_dns_for_trust from ipatests/test_integration/tasks.py,
>> right? I'll have to verify this won't break anything else.
>>
>> Lenka
>>
> Hi,
> 
> to get back to this issue: do we really want to have the DNS configuration
> method removed? I mean, we no longer need it for our CI tests, with new AD VMs
> it works without it, but should somebody else with different setup run these
> tests, they could experience failures because their AD domain would not be
> configured in DNS by default and the test would no longer provide that
> configuration. It doesn't feel right to delete something we needed before but
> don't need anymore, in case somebody else is depending on the same
> configuration. But of course, I'll abide by your counsel.
> In case the call on DNS configuration method really is removed, should I
> remove even it's definition? It's not used anywhere else, so it would be quite
> logical.

Feel free to keep empty method around as a "hook" for other people. The
important thing is that it should do nothing by default.

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

Re: [Freeipa-devel] [PATCH 661] backup: use in-server API in ipa-backup and ipa-restore

[Milan Kubík]

On 06/29/2016 02:54 PM, Jan Cholasta wrote:

Hi,

the attached patch fixes .

Honza





The restore works with the patch. ACK.


--
Milan Kubik

-- 
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 0145] DNS: Reinitialize DNS resolver after changing resolv.con

[Petr Spacek]

Hello,

DNS: Reinitialize DNS resolver after changing resolv.conf

Previously the installer did not reinitialize resolver so queries for
records created using --ip-address option might not be answered. This led
to incorrect results during 'Updating DNS system records' phase at the
end of installation.

This is kind of hack but right now we do not have enough time to extend
python-dns's interface with resolver_reinit() method.

https://fedorahosted.org/freeipa/ticket/5962

-- 
Petr^2 Spacek
From 4a55df8ad821140fddfefe2835b0dd01f41cb466 Mon Sep 17 00:00:00 2001
From: Petr Spacek 
Date: Wed, 29 Jun 2016 19:35:35 +0200
Subject: [PATCH] DNS: Reinitialize DNS resolver after changing resolv.conf

Previously the installer did not reinitialize resolver so queries for
records created using --ip-address option might not be answered. This led
to incorrect results during 'Updating DNS system records' phase at the
end of installation.

This is kind of hack but right now we do not have enough time to extend
python-dns's interface with resolver_reinit() method.

https://fedorahosted.org/freeipa/ticket/5962
---
 ipaserver/install/bindinstance.py | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/ipaserver/install/bindinstance.py b/ipaserver/install/bindinstance.py
index ebb4212ab161456dc3898456567d7b97a6a9939e..f4ed63141cf25dfcfdc72d37d6ff4563e4acccf1 100644
--- a/ipaserver/install/bindinstance.py
+++ b/ipaserver/install/bindinstance.py
@@ -17,6 +17,7 @@
 # along with this program.  If not, see .
 #
 
+from __future__ import absolute_import
 from __future__ import print_function
 
 import tempfile
@@ -27,6 +28,7 @@ import re
 import sys
 import time
 
+import dns.resolver
 import ldap
 import six
 
@@ -982,6 +984,10 @@ class BindInstance(service.Service):
 resolv_fd.close()
 except IOError as e:
 root_logger.error('Could not write to resolv.conf: %s', e)
+else:
+# python DNS might have global resolver cached in this variable
+# we have to re-initialize it because resolv.conf has changed
+dns.resolver.default_resolver = None
 
 def __generate_rndc_key(self):
 installutils.check_entropy()
-- 
2.7.4

-- 
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 0144] Fix `Conflicts` with ipa-python

[Petr Spacek]

Hello,

Fix `Conflicts` with ipa-python

The conflicts should have constant version in it because it is related
to package split.

https://fedorahosted.org/freeipa/ticket/6004


I've tested the same change in RHEL 7.2->7.3 upgrade and it worked just fine.
Upgrade from IPA 4.3.1 to master on Fedora 24 worked just fine, too.

-- 
Petr^2 Spacek
From 25ca086660f4bacfbaab4a151c732d2ca6e9d3b9 Mon Sep 17 00:00:00 2001
From: Petr Spacek 
Date: Wed, 29 Jun 2016 14:00:51 +0200
Subject: [PATCH] Fix `Conflicts` with ipa-python

The conflicts should have constant version in it because it is related
to package split.

https://fedorahosted.org/freeipa/ticket/6004
---
 freeipa.spec.in | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index c86fc3157920f77e66f38241692c3cf45c637ebb..7765946d99e96122046039436558ae27b8944951 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -469,7 +469,7 @@ python2-ipalib and %{name}-common. Packages still depending on
 Summary: Python libraries used by IPA
 Group: System Environment/Libraries
 BuildArch: noarch
-Conflicts: %{name}-python < %{version}-%{release}
+Conflicts: %{name}-python < 4.2.91
 %{?python_provide:%python_provide python2-ipalib}
 Provides: python2-ipapython = %{version}-%{release}
 %{?python_provide:%python_provide python2-ipapython}
@@ -566,7 +566,7 @@ If you are using IPA with Python 3, you need to install this package.
 Summary: Common files used by IPA
 Group: System Environment/Libraries
 BuildArch: noarch
-Conflicts: %{name}-python < %{version}-%{release}
+Conflicts: %{name}-python < 4.2.91
 
 Provides: %{alt_name}-common = %{version}
 Conflicts: %{alt_name}-common
-- 
2.7.4

-- 
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 0022][Tests] Prevent trust test failures cause by adding duplicate DNS forward zone

[Petr Spacek]

On 29.6.2016 18:52, Lenka Doudova wrote:
> 
> 
> On 06/29/2016 06:51 PM, Petr Spacek wrote:
>> On 29.6.2016 18:48, Lenka Doudova wrote:
>>>
>>> On 06/27/2016 11:05 AM, Lenka Doudova wrote:

 On 06/27/2016 10:33 AM, Martin Babinsky wrote:
> On 06/27/2016 10:28 AM, Petr Spacek wrote:
>> On 27.6.2016 10:26, Petr Spacek wrote:
>>> On 27.6.2016 10:18, Martin Babinsky wrote:
 On 06/27/2016 10:04 AM, Petr Vobornik wrote:
> On 06/27/2016 09:42 AM, Lenka Doudova wrote:
>> Hi!
>>
>> With newly created AD machines in Brno lab, existing trust tests
>> fail on
>> 'ipa dnsforwardzone-add' command claiming the zone is already 
>> present,
>> as new AD domain is dom-221.idm.lab.eng.brq.redhat.com.
>>
>> To prevent these failures I prepared attached patch, that will still
>> attempt to add the forward zone, but in case of non-zero return code
>> will check the message if it says that the forward zone is already
>> configured, and lets the tests continue, if it is so.
>>
>>
>> Lenka
>>
>
> Current approach expects that every error of ipa dnsforward-add here
> will mean that the zone exists. So it might hide other issues - not 
> very
> good.
>
> On the other hand it is not very robust to parse error message.
>
> Question for general audience: What do you think if IPA client's exit
> status would be the IPA error code instead of "1" for every error. 
> E.g.
> in DuplicateEntry case it's 4002.
>
> Btw, this is not a NACK.
>
 Well AFAIK the exit status on POSIX systems is encoded into a single
 byte so
 you cannot have the return value greater that 255. We would have to
 devise
 some mapping between our XMLRPC status codes and subprocess return 
 codes.

 Some of our exceptions have defined return values outside plain '1', 
 e.g.
 NotFound has return value of 2. It would be possible to extend this
 concept on
 other common errors.
>>> Even more importantly, the forward zone is completely unnecessary 
>>> because
>>> DNS
>>> when DNS is set up properly. I would simply remove the 
>>> dnsforwardzone-add.
>>>
>> Grr, I meant this:
>> Even more importantly, the forward zone is completely unnecessary when
>> DNS is
>> set up properly. I would simply remove the dnsforwardzone-add.
>>
> +1, our tests should not fiddle with the provisioned environment as much 
> as
> they sometimes do.
>
 Well, I have nothing against removing it completely, but left it there just
 because with previous AD machines with "wild" domains it was necessary.
 Looking at the code, your suggestion would probably mean to entirely remove
 method configure_dns_for_trust from ipatests/test_integration/tasks.py,
 right? I'll have to verify this won't break anything else.

 Lenka

>>> Hi,
>>>
>>> to get back to this issue: do we really want to have the DNS configuration
>>> method removed? I mean, we no longer need it for our CI tests, with new AD 
>>> VMs
>>> it works without it, but should somebody else with different setup run these
>>> tests, they could experience failures because their AD domain would not be
>>> configured in DNS by default and the test would no longer provide that
>>> configuration. It doesn't feel right to delete something we needed before 
>>> but
>>> don't need anymore, in case somebody else is depending on the same
>>> configuration. But of course, I'll abide by your counsel.
>>> In case the call on DNS configuration method really is removed, should I
>>> remove even it's definition? It's not used anywhere else, so it would be 
>>> quite
>>> logical.
>> Feel free to keep empty method around as a "hook" for other people. The
>> important thing is that it should do nothing by default.
>>
> So leave the method call, but erase method contents and let it just pass?

Fine with me. (List re-added.)

-- 
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] [PATCH] 0001: Silence sshd messages during install

[Ben Lipton]

The attached patch silences some annoying messages I've been getting 
when upgrading the freeipa-client package on F24:

"""
WARNING: 'UseLogin yes' is not supported in Fedora and may cause several 
problems.

Could not load host key: /etc/ssh/ssh_host_dsa_key
"""

Since the script causing the message only looks at the return code from 
sshd to determine the right options to use, I thought it might be ok to 
discard the output. What do you think?


Ben
From bb102411cceb557d9869a384af7d7473483f8d9a Mon Sep 17 00:00:00 2001
From: Ben Lipton 
Date: Wed, 29 Jun 2016 14:44:23 -0400
Subject: [PATCH] Silence sshd messages during install

During install we call sshd with no config file, sometimes leading to it
complaining about missing files or bad config options. Since we're just
looking for the return code to see if the options are correct, we can
discard these error messages.
---
 freeipa.spec.in | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index a4d3c067c6c2c0cf911476d950257170fa74e059..73fd47a7469ea27a3ff2883b513bed2be0d420da 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -1007,17 +1007,17 @@ if [ -f '/etc/ssh/sshd_config' -a $restore -ge 2 ]; then
 /^(AuthorizedKeysCommand(User|RunAs)|PubKeyAgentRunAs)[ \t]/ d
 ' /etc/ssh/sshd_config >/etc/ssh/sshd_config.ipanew
 
-if /usr/sbin/sshd -t -f /dev/null -o 'AuthorizedKeysCommand=/usr/bin/sss_ssh_authorizedkeys' -o 'AuthorizedKeysCommandUser=nobody'; then
+if /usr/sbin/sshd -t -f /dev/null -o 'AuthorizedKeysCommand=/usr/bin/sss_ssh_authorizedkeys' -o 'AuthorizedKeysCommandUser=nobody' 2>/dev/null; then
 sed -ri '
 s/^PubKeyAgent (.+) %u$/AuthorizedKeysCommand \1/
 s/^AuthorizedKeysCommand .*$/\0\nAuthorizedKeysCommandUser nobody/
 ' /etc/ssh/sshd_config.ipanew
-elif /usr/sbin/sshd -t -f /dev/null -o 'AuthorizedKeysCommand=/usr/bin/sss_ssh_authorizedkeys' -o 'AuthorizedKeysCommandRunAs=nobody'; then
+elif /usr/sbin/sshd -t -f /dev/null -o 'AuthorizedKeysCommand=/usr/bin/sss_ssh_authorizedkeys' -o 'AuthorizedKeysCommandRunAs=nobody' 2>/dev/null; then
 sed -ri '
 s/^PubKeyAgent (.+) %u$/AuthorizedKeysCommand \1/
 s/^AuthorizedKeysCommand .*$/\0\nAuthorizedKeysCommandRunAs nobody/
 ' /etc/ssh/sshd_config.ipanew
-elif /usr/sbin/sshd -t -f /dev/null -o 'PubKeyAgent=/usr/bin/sss_ssh_authorizedkeys %u' -o 'PubKeyAgentRunAs=nobody'; then
+elif /usr/sbin/sshd -t -f /dev/null -o 'PubKeyAgent=/usr/bin/sss_ssh_authorizedkeys %u' -o 'PubKeyAgentRunAs=nobody' 2>/dev/null; then
 sed -ri '
 s/^AuthorizedKeysCommand (.+)$/PubKeyAgent \1 %u/
 s/^PubKeyAgent .*$/\0\nPubKeyAgentRunAs nobody/
-- 
2.5.5

-- 
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] 0083 Fix regression on ipa-4-3 branch

[Fraser Tweedale]

The attached patch fixes a regression on the ipa-4-3 branch, caused
by commit 3d71c43504ea7837ea14bb9dd4a469c07337293f.

Thanks,
Fraser
From 4d4c62a2c26affb82a7f2e40f36ad0de66beabf9 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Thu, 30 Jun 2016 14:30:30 +1000
Subject: [PATCH] Move normalize_hostname to where it is expected

Commit 3d71c43504ea7837ea14bb9dd4a469c07337293f broke
ipa-client-install by importing normalize_hostname from the wrong
module.  Move the function.

https://fedorahosted.org/freeipa/ticket/5976
---
 ipalib/plugins/host.py | 10 +-
 ipalib/util.py |  8 
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/ipalib/plugins/host.py b/ipalib/plugins/host.py
index 
3fe9c5f9cace71210125371b0aa891490a3dca99..0bb5a535bddfbd3032ab91e410ec6d4fa33f889f
 100644
--- a/ipalib/plugins/host.py
+++ b/ipalib/plugins/host.py
@@ -44,7 +44,7 @@ from ipalib import x509
 from ipalib import output
 from ipalib.request import context
 from ipalib.util import (normalize_sshpubkey, validate_sshpubkey_no_options,
-convert_sshpubkey_post, validate_hostname)
+convert_sshpubkey_post, validate_hostname, normalize_hostname)
 from ipapython.ipautil import ipa_generate_password, CheckedIPAddress
 from ipapython.dnsutil import DNSName
 from ipapython.ssh import SSHPublicKey
@@ -267,14 +267,6 @@ def validate_ipaddr(ugettext, ipaddr):
 return None
 
 
-def normalize_hostname(hostname):
-"""Use common fqdn form without the trailing dot"""
-if hostname.endswith(u'.'):
-hostname = hostname[:-1]
-hostname = hostname.lower()
-return hostname
-
-
 def _hostname_validator(ugettext, value):
 try:
 validate_hostname(value)
diff --git a/ipalib/util.py b/ipalib/util.py
index 
7a991490f76694261deda995ea138f35cf73a1f6..6caf3962d354292834a6ff7bdec424f1df1975e7
 100644
--- a/ipalib/util.py
+++ b/ipalib/util.py
@@ -844,3 +844,11 @@ def detect_dns_zone_realm_type(api, domain):
 def has_managed_topology(api):
 domainlevel = api.Command['domainlevel_get']().get('result', 
DOMAIN_LEVEL_0)
 return domainlevel > DOMAIN_LEVEL_0
+
+
+def normalize_hostname(hostname):
+"""Use common fqdn form without the trailing dot"""
+if hostname.endswith(u'.'):
+hostname = hostname[:-1]
+hostname = hostname.lower()
+return hostname
-- 
2.5.5

-- 
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 0140-0142] Use NSS for name->resolution in IPA installer & relax some DNS checks

[Martin Basti]

On 28.06.2016 19:40, Petr Spacek wrote:

Hello,

DNS: Remove unnecessary DNS check from installer

Previously we were checking content of DNS before actually adding DNS
records for replicas. This is causing cycle in logic and adds weird
corner cases to the installer which can blow up on DNS timeout or so.

The check was completely unnecessary because the installer knows IP
addresses and name of the machine. Removal of the check makes
the installer more reliable.

https://fedorahosted.org/freeipa/ticket/5962

Use NSS for name->resolution in IPA installer

This fixes scenarios where IPA server is not able to resolve own name
and option --ip-address was not specified by the user.

This partially reverts changes from commit
dc405005f537cf278fd6ddfe6b87060bd13d9a67

https://fedorahosted.org/freeipa/ticket/5962

client-install: do not fail if DNS times out during DNS update generation

https://fedorahosted.org/freeipa/ticket/5962


ACK

master:
* 1802f7a2258c793d11c7a9c2a4786cea42b9b058 client-install: do not fail 
if DNS times out during DNS update generation
* 7be50ea7150b36adf9051fc1003dd36f61d68451 Use NSS for name->resolution 
in IPA installer
* 954f6095fd2783e631cba042f86bec87394f9224 DNS: Remove unnecessary DNS 
check from installer


Patches for ipa-4-3 need rebase

--
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 0140-0142] Use NSS for name->resolution in IPA installer & relax some DNS checks

[Petr Spacek]

On 29.6.2016 14:22, Martin Basti wrote:
> 
> 
> On 28.06.2016 19:40, Petr Spacek wrote:
>> Hello,
>>
>> DNS: Remove unnecessary DNS check from installer
>>
>> Previously we were checking content of DNS before actually adding DNS
>> records for replicas. This is causing cycle in logic and adds weird
>> corner cases to the installer which can blow up on DNS timeout or so.
>>
>> The check was completely unnecessary because the installer knows IP
>> addresses and name of the machine. Removal of the check makes
>> the installer more reliable.
>>
>> https://fedorahosted.org/freeipa/ticket/5962
>>
>> Use NSS for name->resolution in IPA installer
>>
>> This fixes scenarios where IPA server is not able to resolve own name
>> and option --ip-address was not specified by the user.
>>
>> This partially reverts changes from commit
>> dc405005f537cf278fd6ddfe6b87060bd13d9a67
>>
>> https://fedorahosted.org/freeipa/ticket/5962
>>
>> client-install: do not fail if DNS times out during DNS update generation
>>
>> https://fedorahosted.org/freeipa/ticket/5962
>>
> ACK
> 
> master:
> * 1802f7a2258c793d11c7a9c2a4786cea42b9b058 client-install: do not fail if DNS
> times out during DNS update generation
> * 7be50ea7150b36adf9051fc1003dd36f61d68451 Use NSS for name->resolution in IPA
> installer
> * 954f6095fd2783e631cba042f86bec87394f9224 DNS: Remove unnecessary DNS check
> from installer
> 
> Patches for ipa-4-3 need rebase

Here is the rebase.

-- 
Petr^2 Spacek
From 92585dd70adaf490a1d6a2ebed14697c6f763d3a Mon Sep 17 00:00:00 2001
From: Petr Spacek 
Date: Tue, 28 Jun 2016 18:13:58 +0200
Subject: [PATCH] client-install: do not fail if DNS times out during DNS
 update generation

https://fedorahosted.org/freeipa/ticket/5962
---
 client/ipa-client-install | 4 
 1 file changed, 4 insertions(+)

diff --git a/client/ipa-client-install b/client/ipa-client-install
index 8ba6f9c1ba441d6a73dcd6c2598ed5463d6a9e3b..b900eca4ed9e3dce3641e176d6d1651535dabcdc 100755
--- a/client/ipa-client-install
+++ b/client/ipa-client-install
@@ -1764,6 +1764,10 @@ def client_dns(server, hostname, options):
 root_logger.warning("Hostname (%s) does not have A/ record.",
 hostname)
 dns_ok = False
+except errors.DNSResolverError as ex:
+root_logger.warning("DNS resolution for hostname %s failed: %s",
+hostname, ex)
+dns_ok = False
 
 if (options.dns_updates or options.all_ip_addresses or options.ip_addresses
 or not dns_ok):
-- 
2.7.4

From b4d24d7241fa929d0cb49af4c59978420074222e Mon Sep 17 00:00:00 2001
From: Petr Spacek 
Date: Tue, 28 Jun 2016 13:53:58 +0200
Subject: [PATCH] Use NSS for name->resolution in IPA installer

This fixes scenarios where IPA server is not able to resolve own name
and option --ip-address was not specified by the user.

This partially reverts changes from commit
dc405005f537cf278fd6ddfe6b87060bd13d9a67

https://fedorahosted.org/freeipa/ticket/5962
---
 ipapython/dnsutil.py  |  2 +-
 ipaserver/install/bindinstance.py |  4 +---
 ipaserver/install/installutils.py | 43 +--
 3 files changed, 43 insertions(+), 6 deletions(-)

diff --git a/ipapython/dnsutil.py b/ipapython/dnsutil.py
index 6aa0e0772d2a3339a18e06c33419083a58e237e4..aca506120ac4c64f3e7af960e0430ae5a3e16d35 100644
--- a/ipapython/dnsutil.py
+++ b/ipapython/dnsutil.py
@@ -321,7 +321,7 @@ def resolve_rrsets(fqdn, rdtypes):
 
 
 def resolve_ip_addresses(fqdn):
-"""Get IP addresses from DNS A/ records for given host.
+"""Get IP addresses from DNS A/ records for given host (using DNS).
 :returns:
 list of IP addresses as CheckedIPAddress objects
 """
diff --git a/ipaserver/install/bindinstance.py b/ipaserver/install/bindinstance.py
index 3e6e26ccdd7bbfb25a19f210307d6597be901a37..efabab167fdaa30cd1483b097c7939f9fcbe4cea 100644
--- a/ipaserver/install/bindinstance.py
+++ b/ipaserver/install/bindinstance.py
@@ -910,9 +910,7 @@ class BindInstance(service.Service):
 if fqdn == self.fqdn:
 continue
 
-addrs = dnsutil.resolve_ip_addresses(fqdn)
-# hack, will go away with locations
-addrs = [str(addr) for addr in addrs]
+addrs = installutils.resolve_ip_addresses_nss(fqdn)
 
 root_logger.debug("Adding DNS records for master %s" % fqdn)
 self.__add_master_records(fqdn, addrs)
diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py
index baa0d3d69987584afd6bf7186a236c4b21fbd748..49336a864791aed74ef4736b43900d7977e49a0c 100644
--- a/ipaserver/install/installutils.py
+++ b/ipaserver/install/installutils.py
@@ -447,6 +447,46 @@ def create_keytab(path, principal):
 
 kadmin("ktadd -k " + path + " " + principal)
 
+def resolve_ip_addresses_nss(fqdn):
+"""Get list of IP addresses for given host (using NSS/getaddrinfo).
+

[Freeipa-devel] [PATCH 661] backup: use in-server API in ipa-backup and ipa-restore

[Jan Cholasta]

Hi,

the attached patch fixes .

Honza

--
Jan Cholasta
From c8ce2e5776a4baa06dfd83090ab2e626ae202e89 Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Wed, 29 Jun 2016 14:28:29 +0200
Subject: [PATCH] backup: use in-server API in ipa-backup and ipa-restore

Use in-server API so that the commands don't try to fetch API schema and
fail.

https://fedorahosted.org/freeipa/ticket/5995
---
 ipaserver/install/ipa_backup.py  | 2 +-
 ipaserver/install/ipa_restore.py | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/ipaserver/install/ipa_backup.py b/ipaserver/install/ipa_backup.py
index 69af6e2..6517696 100644
--- a/ipaserver/install/ipa_backup.py
+++ b/ipaserver/install/ipa_backup.py
@@ -255,7 +255,7 @@ class Backup(admintool.AdminTool):
 options = self.options
 super(Backup, self).run()
 
-api.bootstrap(in_server=False, context='backup')
+api.bootstrap(in_server=True, context='backup')
 api.finalize()
 
 self.log.info("Preparing backup on %s", api.env.host)
diff --git a/ipaserver/install/ipa_restore.py b/ipaserver/install/ipa_restore.py
index 2656536..e172a30 100644
--- a/ipaserver/install/ipa_restore.py
+++ b/ipaserver/install/ipa_restore.py
@@ -844,7 +844,7 @@ class Restore(admintool.AdminTool):
 services.knownservices.certmonger.restart()
 
 def init_api(self, **overrides):
-api.bootstrap(in_server=False, context='restore', **overrides)
+api.bootstrap(in_server=True, context='restore', **overrides)
 api.finalize()
 
 self.instances = [installutils.realm_to_serverid(api.env.realm)]
-- 
2.7.4

-- 
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 0107] test: cert: Reflect change in behavior in tests

[Martin Basti]

On 29.06.2016 14:13, Petr Spacek wrote:

On 28.6.2016 16:58, David Kupka wrote:

--
David Kupka

freeipa-dkupka-0107.0-test-cert-Reflect-change-in-behavior-in-tests.patch


 From 4331e9a62a3cea81b548c555001a7d7ed1127574 Mon Sep 17 00:00:00 2001
From: David Kupka 
Date: Tue, 28 Jun 2016 10:47:10 +0200
Subject: [PATCH] test: cert: Reflect change in behavior in tests

Command cert-find with parameter sizelimit set to 0 no longer returns 0
certificates but returns all.

More precise ConversionError is returned when parameter is not
convertible to its type.

https://fedorahosted.org/freeipa/ticket/5381
https://fedorahosted.org/freeipa/ticket/4739
---
  ipatests/test_xmlrpc/test_cert_plugin.py | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/ipatests/test_xmlrpc/test_cert_plugin.py 
b/ipatests/test_xmlrpc/test_cert_plugin.py
index 
1a6168da29393eb041273da3bbfa845858cdaf72..8127ef224b24a0b3a63c3d07ef72d4b53feda4be
 100644
--- a/ipatests/test_xmlrpc/test_cert_plugin.py
+++ b/ipatests/test_xmlrpc/test_cert_plugin.py
@@ -429,8 +429,9 @@ class test_cert_find(XMLRPC_test):
  """
  Search with a sizelimit of 0
  """
+count_all = api.Command['cert_find']()['count']
  res = api.Command['cert_find'](sizelimit=0)
-assert 'count' in res and res['count'] == 0
+assert 'count' in res and res['count'] == count_all
  
  @raises(errors.ValidationError)

  def test_0028_find_negative_size(self):
@@ -453,7 +454,7 @@ class test_cert_find(XMLRPC_test):
  res = api.Command['cert_find'](subject=u'ipatestcert.%s' % 
api.env.domain)
  assert 'count' in res and res['count'] >= 1
  
-@raises(errors.ValidationError)

+@raises(errors.ConversionError)
  def test_0031_search_on_invalid_date(self):
  """
  Search using invalid date format
-- 2.7.4

ACK


Pushed to master: 573819eb07ea67311004850b7726f3368bacb49b

--
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] 0007 Fix ipa-server-certinstall with certs signed by 3rd-party CA

[Stanislav Laznicka]

On 06/22/2016 09:29 PM, Florence Blanc-Renaud wrote:

Hi,

This patch fixes ipa-server-certinstall when used with 3rd-party certs.
The scenario is the following:
- install the server with an embedded CA
- use ipa-cacert-manage to install a 3rd party CA
- use ipa-certupdate to put the 3rd party CA cert in the relevant NSS 
databases (/etc/ipa/nssdb /etc/httpd/alias /etc/pki/pki-tomcat/alias 
and /etc/dirsrv/slapd-XXX)
- use ipa-server-certinstall to replace the Directory/Apache server 
certificates with a cert signed by the 3rd party CA.


Note that I had to run ipa-certupdate after putting selinux mode to 
permissive (otherwise the cert does not get into 
/etc/pki/pki-tomcat/alias) and a bz has been opened against 
selinux-policy to solve this issue.


https://fedorahosted.org/freeipa/ticket/4785
https://fedorahosted.org/freeipa/ticket/4786



Hello,

The patch works as expected with the selinux requirement you mentioned. 
I will just add Honza for code sanity check. Therefore conditional ACK 
if the code can take no further improvements.


Standa

-- 
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 0107] test: cert: Reflect change in behavior in tests

[Petr Spacek]

On 28.6.2016 16:58, David Kupka wrote:
> 
> -- 
> David Kupka
> 
> freeipa-dkupka-0107.0-test-cert-Reflect-change-in-behavior-in-tests.patch
> 
> 
> From 4331e9a62a3cea81b548c555001a7d7ed1127574 Mon Sep 17 00:00:00 2001
> From: David Kupka 
> Date: Tue, 28 Jun 2016 10:47:10 +0200
> Subject: [PATCH] test: cert: Reflect change in behavior in tests
> 
> Command cert-find with parameter sizelimit set to 0 no longer returns 0
> certificates but returns all.
> 
> More precise ConversionError is returned when parameter is not
> convertible to its type.
> 
> https://fedorahosted.org/freeipa/ticket/5381
> https://fedorahosted.org/freeipa/ticket/4739
> ---
>  ipatests/test_xmlrpc/test_cert_plugin.py | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/ipatests/test_xmlrpc/test_cert_plugin.py 
> b/ipatests/test_xmlrpc/test_cert_plugin.py
> index 
> 1a6168da29393eb041273da3bbfa845858cdaf72..8127ef224b24a0b3a63c3d07ef72d4b53feda4be
>  100644
> --- a/ipatests/test_xmlrpc/test_cert_plugin.py
> +++ b/ipatests/test_xmlrpc/test_cert_plugin.py
> @@ -429,8 +429,9 @@ class test_cert_find(XMLRPC_test):
>  """
>  Search with a sizelimit of 0
>  """
> +count_all = api.Command['cert_find']()['count']
>  res = api.Command['cert_find'](sizelimit=0)
> -assert 'count' in res and res['count'] == 0
> +assert 'count' in res and res['count'] == count_all
>  
>  @raises(errors.ValidationError)
>  def test_0028_find_negative_size(self):
> @@ -453,7 +454,7 @@ class test_cert_find(XMLRPC_test):
>  res = api.Command['cert_find'](subject=u'ipatestcert.%s' % 
> api.env.domain)
>  assert 'count' in res and res['count'] >= 1
>  
> -@raises(errors.ValidationError)
> +@raises(errors.ConversionError)
>  def test_0031_search_on_invalid_date(self):
>  """
>  Search using invalid date format
> -- 2.7.4

ACK

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

Re: [Freeipa-devel] [PATCH] 0008 Do not allow installation in FIPS mode

[Florence Blanc-Renaud]

On 06/29/2016 01:04 PM, Martin Basti wrote:



On 28.06.2016 16:57, Florence Blanc-Renaud wrote:

On 06/28/2016 11:05 AM, Martin Basti wrote:



On 28.06.2016 10:51, Florence Blanc-Renaud wrote:

On 06/27/2016 10:18 PM, Rob Crittenden wrote:

Florence Blanc-Renaud wrote:

Hi all,

thanks for your suggestions. Updated patch attached.
Flo.



The invocation in ipactl should say server, not client.

Otherwise LGTM (untested).

rob


Hi all,

thanks to Rob for catching the typo.
Patch with updated message is attached,
Flo.




Thank you for the patch I have two comments:

1)
+except Exception:
+# Consider that the host is not fips-enabled if the file does
not exist
+pass

exceptions should be as much specific as possible, otherwise it may mask
real issues
please use 'except IOError' if you want catch the case that file does
not exist

2)
in replicainstall.py and install.py please raise exception
(RuntimeError) instead of sys.exit() to keep proper logging, cleanup,
etc.

Sys.exit() should not be used in modules, it is hard to debug etc. It
can be used only in scripts (ipa-client-install, ipa-replica-manage,
etc..)

Martin^2


Hi,

hopefully converging with this updated patch :)
Thanks for all the comments, I'm learning tips with each iteration.

Flo.


I propose following changes (in attached patch). If you agree I can
squash patches and push it.

Martin^2


Hi Martin,

thanks for the proposal, OK for me.
Flo.

--
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] [Test][patch-0052] Test for incorrect client domain

[Petr Spacek]

On 29.6.2016 12:23, Martin Basti wrote:
> 
> 
> On 29.06.2016 10:56, Oleg Fayans wrote:
>>
>>
> 
> Hello,
> 
> +assert_error(result,
> + "Failed to verify that %s is an IPA Server" %
> + self.master.hostname)
> 
> 
> I would expect this error there:
> 
> "Cannot promote this client to a replica. Local domain '{local}' does not
> match IPA domain '{ipadomain}'. "
> 
> You should not use random REALM, in this case you don't test domains but
> realms. You can leave the test with incorrect realm there, but as separated
> testcase

Even more importantly, the check is part of ipa-replica-install - and this
test is testing anything in ipa-replica-install.

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

Re: [Freeipa-devel] [PATCH] 0008 Do not allow installation in FIPS mode

[Martin Basti]

On 28.06.2016 16:57, Florence Blanc-Renaud wrote:

On 06/28/2016 11:05 AM, Martin Basti wrote:



On 28.06.2016 10:51, Florence Blanc-Renaud wrote:

On 06/27/2016 10:18 PM, Rob Crittenden wrote:

Florence Blanc-Renaud wrote:

Hi all,

thanks for your suggestions. Updated patch attached.
Flo.



The invocation in ipactl should say server, not client.

Otherwise LGTM (untested).

rob


Hi all,

thanks to Rob for catching the typo.
Patch with updated message is attached,
Flo.




Thank you for the patch I have two comments:

1)
+except Exception:
+# Consider that the host is not fips-enabled if the file does
not exist
+pass

exceptions should be as much specific as possible, otherwise it may mask
real issues
please use 'except IOError' if you want catch the case that file does
not exist

2)
in replicainstall.py and install.py please raise exception
(RuntimeError) instead of sys.exit() to keep proper logging, cleanup, 
etc.


Sys.exit() should not be used in modules, it is hard to debug etc. It
can be used only in scripts (ipa-client-install, ipa-replica-manage, 
etc..)


Martin^2


Hi,

hopefully converging with this updated patch :)
Thanks for all the comments, I'm learning tips with each iteration.

Flo.

I propose following changes (in attached patch). If you agree I can 
squash patches and push it.


Martin^2
From a3e91642a83877f45708094f391104fbcb894fd4 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Wed, 29 Jun 2016 13:02:59 +0200
Subject: [PATCH] FIPS: reviewer proposed changes

---
 ipaplatform/base/paths.py | 1 +
 ipapython/ipautil.py  | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/ipaplatform/base/paths.py b/ipaplatform/base/paths.py
index dddefea0b558010ac24334d041201a80a05587be..d6fbe32f6839a5db40148777132ba1454cbc3382 100644
--- a/ipaplatform/base/paths.py
+++ b/ipaplatform/base/paths.py
@@ -134,6 +134,7 @@ class BasePathNamespace(object):
 SYSTEMD_PKI_TOMCAT_SERVICE = "/etc/systemd/system/pki-tomcatd.target.wants/pki-tomcatd@pki-tomcat.service"
 DNSSEC_TRUSTED_KEY = "/etc/trusted-key.key"
 HOME_DIR = "/home"
+PROC_FIPS_ENABLED = "/proc/sys/crypto/fips_enabled"
 ROOT_IPA_CACHE = "/root/.ipa_cache"
 ROOT_PKI = "/root/.pki"
 DOGTAG_ADMIN_P12 = "/root/ca-agent.p12"
diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index 4ef9770e92c3ba86ffa5c6523268475a026705d0..c7e20c5102efaa006c10d4c3af849bc259da43e7 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -1440,7 +1440,7 @@ def is_fips_enabled():
 the function returns False.
 """
 try:
-with open('/proc/sys/crypto/fips_enabled', 'r') as f:
+with open(paths.PROC_FIPS_ENABLED, 'r') as f:
 if f.read().strip() != '0':
 return True
 except IOError:
-- 
2.5.5

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