Re: [Freeipa-devel] [PATCH] 0086 Add --ca option to cert-status

[Jan Cholasta]

On 1.7.2016 06:47, Fraser Tweedale wrote:

On Fri, Jul 01, 2016 at 05:55:35AM +0200, Jan Cholasta wrote:

On 29.6.2016 12:18, Jan Cholasta wrote:

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.


ACK on the first version of the patch, since it's better than nothing. The
ticket remains open, please fix the rest ASAP.

Added VERSION bump and pushed to master:
ffb1f5b1f24f0de30529d50f8c8dfb9a896c149e

Honza


New patch 0086 attached, adding the option to cert-status command.


Thanks. We could at least check if the specified CA exists, couldn't we?



(2) will be addressed later due to conflicts with other patches (or
maybe as part of those other patches).


OK.



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

[Freeipa-devel] [PATCH] 0086 Add --ca option to cert-status

[Fraser Tweedale]

On Fri, Jul 01, 2016 at 05:55:35AM +0200, Jan Cholasta wrote:
> On 29.6.2016 12:18, Jan Cholasta wrote:
> > 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.
> 
> ACK on the first version of the patch, since it's better than nothing. The
> ticket remains open, please fix the rest ASAP.
> 
> Added VERSION bump and pushed to master:
> ffb1f5b1f24f0de30529d50f8c8dfb9a896c149e
> 
> Honza
> 
New patch 0086 attached, adding the option to cert-status command.

(2) will be addressed later due to conflicts with other patches (or
maybe as part of those other patches).

Thanks,
Fraser
From b4d49da637035cdd8b4504b09b9790b3fc68c144 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Fri, 1 Jul 2016 14:42:37 +1000
Subject: [PATCH] Add --cn option to cert-status

Add the 'cacn' option to the cert-status command.  Right now there
is nothing we need to (or can) do with it, but we add it anyway for
future use.

Fixes: https://fedorahosted.org/freeipa/ticket/5999
---
 API.txt   |  3 ++-
 VERSION   |  4 ++--
 ipaserver/plugins/cert.py | 14 ++
 3 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/API.txt b/API.txt
index 
c01692e17dc1ed368c14e32ab7e3fc09bc4d1ffc..9f9456d2cefac9c31ff89f3812a862a80e7ad307
 100644
--- a/API.txt
+++ b/API.txt
@@ -799,9 +799,10 @@ output: Entry('result')
 output: Output('summary', type=[, ])
 output: PrimaryKey('value')
 command: cert_status/1
-args: 1,3,3
+args: 1,4,3
 arg: Int('request_id')
 option: Flag('all', autofill=True, cli_name='all', default=False)
+option: Str('cacn?', autofill=True, cli_name='ca', default=u'ipa')
 option: Flag('raw', autofill=True, cli_name='raw', default=False)
 option: Str('version?')
 output: Entry('result')
diff --git a/VERSION b/VERSION
index 
23ceecc98e6ecf9adc21016508ba9feaa1454e6f..212b7d740a12a313395faf3bcdefaf09c41651f9
 100644
--- a/VERSION
+++ b/VERSION
@@ -90,5 +90,5 @@ IPA_DATA_VERSION=2010061412
 #  #
 
 IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=205
-# Last change: Add --ca option to cert-revoke and cert-remove-hold
+IPA_API_VERSION_MINOR=206
+# Last change: Add --ca option to cert-status
diff --git a/ipaserver/plugins/cert.py b/ipaserver/plugins/cert.py
index 
817bdc26f1d8b53a323802079d19e367404528bd..70add0bb38a08ec030969dce6369cde85a33a5fb
 100644
--- a/ipaserver/plugins/cert.py
+++ b/ipaserver/plugins/cert.py
@@ -638,17 +638,15 @@ class cert_status(Retrieve, BaseCertMethod, 
VirtualCommand):
 
 operation = "certificate status"
 
-def get_options(self):
-for option in super(cert_status, self).get_options():
-if option.name == 'cacn':
-# Dogtag requests are uniquely identified by their
-# number; there is no need to distinguish by CA.
-continue
-yield option
-
 def execute(self, request_id, **kw):
 ca_enabled_check()
 self.check_access()
+
+# Dogtag requests are uniquely identified by their number;
+# furthermore, Dogtag (as at v10.3.4) does not report the
+# target CA in request data, so we cannot check.  So for
+# now, there is nothing we can do with the 'cacn' option.
+
 return dict(
 result=self.Backend.ra.check_request_status(str(request_id)),
 value=pkey_to_value(request_id, kw),
-- 
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 0025][Tests] RFE: External trust

[Lenka Doudova]

On 06/30/2016 05:01 PM, Martin Babinsky wrote:

On 06/30/2016 03:47 PM, Lenka Doudova wrote:

Hi,

attaching patch with some basic coverage for external trust feature. Bit
more detailed info in commit message.

Since the feature requires me to run commands previously used only for
forest root domains even for subdomains, I made some changes in
ipatests/test_integration/tasks.py file, so that it would enable me to
reuse existing function without copy-pasting them for one variable 
change.



Lenka





Hi Lenka,

I have a few comments:

1.)

-def establish_trust_with_ad(master, ad, extra_args=()):
+def establish_trust_with_ad(master, ad, extra_args=(), subdomain=False):

This modification seems extremely kludgy to me.

I would rather change the function signature to:

-def establish_trust_with_ad(master, ad, extra_args=()):
+def establish_trust_with_ad(master, ad_domain, extra_args=()):

and pass the domain name itself to the function instead of doing magic 
inside. The same goes with `remove trust_with_ad`. You may want to fix 
the calls to them in existing code so that the domain is passed 
instead of the whole trust object.


2.)

+def sync_time_hostname(host, srv_hostname):
+"""
+Syncs time with the remote server given by its hostname.
+Please note that this function leaves ntpd stopped.
+"""
+host.run_command(['systemctl', 'stop', 'ntpd'])
+host.run_command(['ntpdate', srv_hostname])
+
+

this looks like a duplicate of the function above it, why is it even 
needed?


3.)
+class TestExternalTrustWithSubdomain(ADTrustBase):
+"""
+Test establishing external trust with subdomain
+"""
+
+@classmethod
+def install(cls, mh):
+super(ADTrustBase, cls).install(mh)
+cls.ad = cls.ad_domains[0].ads[0]
+cls.install_adtrust()
+cls.check_sid_generation()
+
+# Determine whether the subdomain AD is available
+try:
+child_ad = cls.host_by_role(cls.optional_extra_roles[0])
+cls.ad_subdomain = 
'.'.join(child_ad.hostname.split('.')[1:])

+cls.ad_subdomain_hostname = child_ad.hostname
+except LookupError:
+cls.ad_subdomain = None
+
+cls.configure_dns_and_time()

Please use proper OOP practices when overriding the behavior of the 
base test class.


For starters you could rewrite the install method like this:

+@classmethod
+def install(cls, mh):
+# Determine whether the subdomain AD is available
+try:
+cls.child_ad = cls.host_by_role(cls.optional_extra_roles[0])
+cls.ad_subdomain = 
'.'.join(child_ad.hostname.split('.')[1:])

+cls.ad_subdomain_hostname = child_ad.hostname
+except LookupError:
+raise nose.SkipTest("AFAIK this will skip the whole test 
class")

+super(ADTrustBase, cls).install(mh)

with child_ad stored as class attribute, you can override 
`configure_dns_and_time`:

+def configure_dns_and_time(cls):
+tasks.configure_dns_for_trust(cls.master, cls.ad_subdomain)
 # no need to re-implement the function just to get to the child 
AD DC hostname now

+tasks.sync_time(cls.master, cls.child_ad.hostname)

You can then use this class as an intermediary in the hierarchy and 
inherit the other external/non-external trust test classes from it, 
since most setup method in them are just copy-pasted from this one.



Hi,
thanks for review, fixed patch attached.

Lenka
From ae860f97d701b296f52de1eb0c84dd43d1429f76 Mon Sep 17 00:00:00 2001
From: Lenka Doudova 
Date: Thu, 30 Jun 2016 12:23:02 +0200
Subject: [PATCH] Tests: External trust

Provides basic coverage for external trust feature.
Test cases:
1. verify an external trust with AD subdomain can be established
   - verify only one trustdomain is listed
   - verify subdomain users are resolvable
   - verify trust can be deleted
2. verify non-external trust with AD subdomain cannot be established
3. verify an external trust with AD forest root domain can be established
   - verify that even if AD subdomain is specified, it is not associated with the trust
   - verify trust can be deleted

https://fedorahosted.org/freeipa/ticket/5743
---
 ipatests/test_integration/tasks.py  |  11 +-
 ipatests/test_integration/test_trust.py | 218 
 2 files changed, 197 insertions(+), 32 deletions(-)

diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py
index 5be7cdae3ac777bbf0fc52e6c511969e9fabcd72..48ef6f1963e2405d7db9a18799e9796b7f10bfe5 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -443,7 +443,7 @@ def configure_dns_for_trust(master, ad):
 pass
 
 
-def establish_trust_with_ad(master, ad, extra_args=()):
+def establish_trust_with_ad(master, ad_domain, extra_args=()):
 """
 Establishes trust with Active Directory. Trust type is detected depending
 on the presence of SfU 

Re: [Freeipa-devel] [PATCH] 0072..0075 Lightweight CA renewal

[Jan Cholasta]

On 29.6.2016 10:41, Fraser Tweedale wrote:

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




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

On 29.6.2016 12:18, Jan Cholasta wrote:

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.


ACK on the first version of the patch, since it's better than nothing. 
The ticket remains open, please fix the rest ASAP.


Added VERSION bump and pushed to master: 
ffb1f5b1f24f0de30529d50f8c8dfb9a896c149e


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] Kerberos principal aliases pt. 2

[David Kupka]

On 28/06/16 20:08, Martin Babinsky wrote:

On 06/24/2016 09:52 AM, Martin Babinsky wrote:

Hi list,

I am furiously working on tickets related to the proper support and API
for managing kerberos principal aliases for hosts, users, and
services[1-5].

To better track and comment on my progress, I have forked freeipa on git
and created a branch for you to test and review. The link is here:

https://github.com/martbab/freeipa/tree/krb5-principal-aliases

Please be aware that I may force-push into the branch without warning
when fixing issues we will discover during testing/review.

[1] http://www.freeipa.org/page/V4/Kerberos_principal_aliases
[2] https://fedorahosted.org/freeipa/ticket/3864
[3] https://fedorahosted.org/freeipa/ticket/3961
[4] https://fedorahosted.org/freeipa/ticket/1365
[5] https://fedorahosted.org/freeipa/ticket/5413



Based on Jan's suggestions I have reworked the code substantially and
force-pushed it into the github branch. Please review.



Hello!

I have gone through the code and tested the functionality in basic use 
cases (server-install, upgrade, replica-install, adding/removing 
principals, getting ticket with alias, ...). Code looks good to me and 
everything* seems to work smoothly.


condACK, if Pavel or Petr^1 (or anyone else who tried this) don't report 
any issue really soon.


*except for https://fedorahosted.org/freeipa/ticket/6017

--
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] pwpolicy: Do not expire passwords when maxlife is set to 0 (infinity).

[David Kupka]

On 04/05/16 17:22, Pavel Vomacka wrote:



On 05/04/2016 04:36 PM, Simo Sorce wrote:

On Wed, 2016-05-04 at 15:39 +0200, Martin Kosek wrote:

On 05/02/2016 02:28 PM, David Kupka wrote:

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

That patch looks suspiciously short given the struggles I saw in
http://www.redhat.com/archives/freeipa-devel/2015-June/msg00198.html
:-)

Instead of setting to IPAPWD_END_OF_TIME, should we instead avoid
filling
"krbPasswordExpiration" attribute at all, i.e. have password *without*
expiration? Or is krbPasswordExpiration mandatory?

So I looked at the MIT code, and it seem like they are coping just fine
with a missing (ie value = 0 internally) pw_expiration attribute.

So if we make our code cope with omitting any expiration if the
attribute is missing then yes, we can mark no expiration with simply
removing (or not setting) the krbPasswordExpiration attribute.
The attribute itself is optional and can be omitted.

I think this is a good idea, and is definitely better than inventing a a
magic value.

Simo.


Just a note: I tested David's patch and it actually doesn't work when
the new password policy for ipausers group is created (priority = 0,
which should be the highest priority). The maxlife and minlife values
are empty. Even if I set the new password policy maxlife and minlife to
0 the result was that password will expire in 90 days. The patch worked
correctly when I changed value of maxlife and minlife to 0 in
'global_policy'. Then the password expiration was set to 2038-01-01.



Hello!

I hope I've finally find all the places in ipa-kdb and ipa-pwd-extop 
plugins to tickle in order to have password that don't expire. Updated 
patch attached.


https://fedorahosted.org/freeipa/ticket/2795
--
David Kupka
From dc2250869ca4f527b9b353913ed5a750a6f0f6e5 Mon Sep 17 00:00:00 2001
From: David Kupka 
Date: Thu, 30 Jun 2016 08:52:33 +0200
Subject: [PATCH] Allow unexpiring passwords

Treat maxlife=0 in password policy as "never expire". Delete
krbPasswordExpiration in user entry when password should never expire.

https://fedorahosted.org/freeipa/ticket/2795
---
 daemons/ipa-kdb/ipa_kdb_passwords.c   |  6 +-
 daemons/ipa-kdb/ipa_kdb_principals.c  | 11 +++
 daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c  | 11 ++-
 daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c |  4 
 ipaserver/plugins/pwpolicy.py |  2 +-
 5 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/daemons/ipa-kdb/ipa_kdb_passwords.c b/daemons/ipa-kdb/ipa_kdb_passwords.c
index ad57181d5049f36c69044bb2c9cfe183d7e4ea25..a3d4fe2436da60d081040754780d3e815acb1473 100644
--- a/daemons/ipa-kdb/ipa_kdb_passwords.c
+++ b/daemons/ipa-kdb/ipa_kdb_passwords.c
@@ -253,7 +253,11 @@ krb5_error_code ipadb_get_pwd_expiration(krb5_context context,
 
 if (truexp) {
 if (ied->pol) {
-*expire_time = mod_time + ied->pol->max_pwd_life;
+if (ied->pol->max_pwd_life) {
+*expire_time = mod_time + ied->pol->max_pwd_life;
+} else {
+*expire_time = 0;
+}
 } else {
 *expire_time = mod_time + IPAPWD_DEFAULT_PWDLIFE;
 }
diff --git a/daemons/ipa-kdb/ipa_kdb_principals.c b/daemons/ipa-kdb/ipa_kdb_principals.c
index f1d3e9e89c2016b8a9ebad9c0c6fd46487a33a4b..6cdfa909452a4b55912b2a5a74648abd2053482a 100644
--- a/daemons/ipa-kdb/ipa_kdb_principals.c
+++ b/daemons/ipa-kdb/ipa_kdb_principals.c
@@ -1850,6 +1850,11 @@ static krb5_error_code ipadb_entry_to_mods(krb5_context kcontext,
"krbPasswordExpiration",
entry->pw_expiration,
mod_op);
+if (entry->pw_expiration == 0) {
+kerr = ipadb_get_ldap_mod_time(imods,
+   "krbPasswordExpiration",
+   entry->pw_expiration, LDAP_MOD_DELETE);
+}
 if (kerr) {
 goto done;
 }
@@ -2105,6 +2110,12 @@ static krb5_error_code ipadb_entry_to_mods(krb5_context kcontext,
 kerr = ipadb_get_ldap_mod_time(imods,
"krbPasswordExpiration",
expire_time, mod_op);
+if (expire_time == 0) {
+kerr = ipadb_get_ldap_mod_time(imods,
+   "krbPasswordExpiration",
+   expire_time, LDAP_MOD_DELETE);
+}
+
 if (kerr) {
 goto done;
 }
diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c b/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c
index 5dc606d22305cf63a16feca30aab2728bb20b80d..de295e0b4cbd8eeff556d96be3db68705114b994 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c
+++ 

[Freeipa-devel] [PATCH 0146] Fix internal errors in host-add and other commands caused by DNS resolutio

[Petr Spacek]

Hello,

Fix internal errors in host-add and other commands caused by DNS resolution

Previously resolver was returning CheckedIPAddress objects. This
internal server error in cases where DNS actually returned reserved IP
addresses.

Now the resolver is returning UnsafeIPAddress objects which do syntactic
checks but do not filter IP addresses.

>From now on we can decide if some IP address should be accepted as-is or
if it needs to be contrained to some subset of IP addresses using
CheckedIPAddress class.

This regression was caused by changes for
https://fedorahosted.org/freeipa/ticket/5710



I've split parser and checks into separate classes. Attached script
CheckedIPAddressRefactoring.py uses python-hypothesis to compare results from
old and new implementations. It seems that all valid inputs return the very
same results. The new implementation is a bit stricter when it comes to
invalid inputs (parse_netmask=False & addr=IPNetwork instance) but as far as I
can tell this case could not happen in current IPA anyway.

ipa-server-install, ipa-client-install, ipa-replica-install, and
ipa-ca-install on replica seem to work. DNS records for ipa-ca were properly
updated after replica installation. Also installation on server without A/
record in DNS and subsequent ipa-dns-install worked just fine.

-- 
Petr^2 Spacek
From 52099f1e29d361de228ca5cfb0e05a53dfd88cde Mon Sep 17 00:00:00 2001
From: Petr Spacek 
Date: Thu, 30 Jun 2016 20:41:48 +0200
Subject: [PATCH] Fix internal errors in host-add and other commands caused by
 DNS resolution

Previously resolver was returning CheckedIPAddress objects. This
internal server error in cases where DNS actually returned reserved IP
addresses.

Now the resolver is returning UnsafeIPAddress objects which do syntactic
checks but do not filter IP addresses.

From now on we can decide if some IP address should be accepted as-is or
if it needs to be contrained to some subset of IP addresses using
CheckedIPAddress class.

This regression was caused by changes for
https://fedorahosted.org/freeipa/ticket/5710
---
 ipapython/dnsutil.py  |  12 +--
 ipapython/ipautil.py  | 149 +++---
 ipaserver/install/installutils.py |  13 +---
 3 files changed, 96 insertions(+), 78 deletions(-)

diff --git a/ipapython/dnsutil.py b/ipapython/dnsutil.py
index aca506120ac4c64f3e7af960e0430ae5a3e16d35..16549c8f6a4a0782191526856e918d7ac6b94dcc 100644
--- a/ipapython/dnsutil.py
+++ b/ipapython/dnsutil.py
@@ -24,7 +24,7 @@ import copy
 
 import six
 
-from ipapython.ipautil import CheckedIPAddress
+from ipapython.ipautil import UnsafeIPAddress
 from ipapython.ipa_log_manager import root_logger
 
 if six.PY3:
@@ -323,18 +323,12 @@ def resolve_rrsets(fqdn, rdtypes):
 def resolve_ip_addresses(fqdn):
 """Get IP addresses from DNS A/ records for given host (using DNS).
 :returns:
-list of IP addresses as CheckedIPAddress objects
+list of IP addresses as UnsafeIPAddress objects
 """
 rrsets = resolve_rrsets(fqdn, ['A', ''])
 ip_addresses = set()
 for rrset in rrsets:
-ip_addresses.update({CheckedIPAddress(ip,  # accept whatever is in DNS
-  parse_netmask=False,
-  allow_network=True,
-  allow_loopback=True,
-  allow_broadcast=True,
-  allow_multicast=True)
- for ip in rrset})
+ip_addresses.update({UnsafeIPAddress(ip) for ip in rrset})
 return ip_addresses
 
 
diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index 8506bf2d58a35cb128e44cee34c3fb62d7a0c5e4..763a99c117e22a4ac49d8d34b38230f3da7c8435 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -74,102 +74,133 @@ def get_domain_name():
 
 return domain_name
 
-class CheckedIPAddress(netaddr.IPAddress):
+
+class UnsafeIPAddress(netaddr.IPAddress):
+"""Any valid IP address with or without netmask."""
 
 # Use inet_pton() rather than inet_aton() for IP address parsing. We
 # will use the same function in IPv4/IPv6 conversions + be stricter
 # and don't allow IP addresses such as '1.1.1' in the same time
 netaddr_ip_flags = netaddr.INET_PTON
 
+def __init__(self, addr):
+if isinstance(addr, UnsafeIPAddress):
+self._net = addr._net
+super(UnsafeIPAddress, self).__init__(addr,
+  flags=self.netaddr_ip_flags)
+return
+
+elif isinstance(addr, netaddr.IPAddress):
+self._net = None  # no information about netmask
+super(UnsafeIPAddress, self).__init__(addr,
+  flags=self.netaddr_ip_flags)
+return
+
+elif isinstance(addr, netaddr.IPNetwork):
+   

Re: [Freeipa-devel] [PATCH] 961 webui: prevent infinite reload for users with krbbprincipal alias set

[Martin Babinsky]

On 06/30/2016 07:34 PM, Petr Vobornik wrote:

Web UI has an inbuilt mechanism to reload in case response from a server
contains a different principal than the one loaded during Web UI
startup.

see rpc.js:381

With kerberos aliases support the loaded principal could be different
because krbprincipalname contained multiple values.

In such case krbcanonicalname should be used - it contains the same
principal as the one which will be in future API responses.

part of: https://fedorahosted.org/freeipa/ticket/5927



This patch is the WebUI counterpart to my WIP hacks to rpcserver.py 
which possible to login to webui via enterprise principal (e.g. 
u...@email.com).


for me ACK.

--
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 0546-0547] use timestamps for ipareplica-conncheck.log

[Martin Babinsky]

On 06/30/2016 01:54 PM, Martin Basti wrote:



On 30.06.2016 12:07, Petr Spacek wrote:

On 30.6.2016 10:21, Jan Cholasta wrote:

On 30.6.2016 10:12, Petr Spacek wrote:

On 30.6.2016 10:14, Jan Cholasta wrote:

On 30.6.2016 10:06, Petr Spacek wrote:

On 30.6.2016 10:02, Jan Cholasta wrote:

On 30.6.2016 09:56, Petr Spacek wrote:

On 30.6.2016 09:40, Martin Basti wrote:

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

"The easiest solution would be to add timestamps to logs, or log to
different
logs from oddjob or from installer
(ipareplica-conncheck.local.log and
ipareplica-conncheck.remote.log)"

Actually the easiest solution would be not to log into a file
when executed
from oddjob.

Well, IPA is hard enough to debug even with logs. I would not make
situation
even worse by not logging at all :-)

The commands logs into stderr, and both stdout and stderr are sent
back to the
caller of the oddjob.

Alternatively, it could log into a different file (say
/var/log/ipareplica-conncheck-oddjob.log). IMO timestamps are an
overkill to
fix this bug.

When we are at it, a custom logger is overkill. IMHO we should log
everything
to journal and be done with it ...

It's not, we want to log to at least stderr ourselves.

Also, it would be even harder to implement than timestamps, and time
is a

Sure, we do not have time:
=> ACK for current version of the patch.

Petr^2 Spacek


factor here. It would fit more into
.


Petr^2 Spacek


Patches attached.

I would rather use timestamp format with dashes between numbers
to make it
easier to read and parse for humans.

Compare:

201606270954
201606290954
201606300954

with

2016-06-27-09-54
2016-06-29-09-54
2016-06-30-09-54








New patches attaches




Works for me, ACK.

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

[Freeipa-devel] [PATCH] 961 webui: prevent infinite reload for users with krbbprincipal alias set

[Petr Vobornik]

Web UI has an inbuilt mechanism to reload in case response from a server
contains a different principal than the one loaded during Web UI
startup.

see rpc.js:381

With kerberos aliases support the loaded principal could be different
because krbprincipalname contained multiple values.

In such case krbcanonicalname should be used - it contains the same
principal as the one which will be in future API responses.

part of: https://fedorahosted.org/freeipa/ticket/5927
-- 
Petr Vobornik
From a15518f25eb339ab2bc90dbc304648a9fd266e51 Mon Sep 17 00:00:00 2001
From: Petr Vobornik 
Date: Thu, 30 Jun 2016 19:16:32 +0200
Subject: [PATCH] webui: prevent infinite reload for users with krbbprincipal
 alias set

Web UI has inbuilt mechanism to reload in case response from a server
contains a different principal than the one loaded during Web UI
startup.

see rpc.js:381

With kerberos aliases support the loaded principal could be different
because krbprincipalname contained multiple values.

In such case krbcanonicalname should be used - it contains the same
principal as the one which will be in future API responses.

https://fedorahosted.org/freeipa/ticket/5927
---
 install/ui/src/freeipa/ipa.js | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/install/ui/src/freeipa/ipa.js b/install/ui/src/freeipa/ipa.js
index 830def0542faeb14b62b71fb80c753bc121cace7..e8ad832c3aacc0fcd824af3e1956ff621ae761ed 100644
--- a/install/ui/src/freeipa/ipa.js
+++ b/install/ui/src/freeipa/ipa.js
@@ -259,7 +259,11 @@ var IPA = function () {
 },
 on_success: function(data, text_status, xhr) {
 that.whoami = batch ? data.result[0] : data.result.result[0];
-that.principal = that.whoami.krbprincipalname[0];
+var cn = that.whoami.krbcanonicalname;
+if (cn) that.principal = cn[0];
+if (!that.principal) {
+that.principal = that.whoami.krbprincipalname[0];
+}
 }
 });
 };
-- 
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] Fix minor typo

[Yuri Chornoivan]

Hi,

/ipaserver/plugins/cert.py:120:

Verify that a certificate is owner by a specific user:

It might be

Verify that a certificate is owned by a specific user:

Thanks for reviewing this possible typo fix.

Best regards,
Yuri

0001-Fix-minor-typo.patch
Description: Binary data
-- 
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 0548] Fix replica install with CA

[Martin Basti]

On 30.06.2016 13:20, Martin Basti wrote:



On 30.06.2016 13:18, Petr Spacek wrote:

On 30.6.2016 13:04, Martin Basti wrote:

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

This only for master branch, ipa-4-3 fix will be different (soon)

Patch attached

ACK


Pushed to master: a155f692e7ad7807a5ea28250d1e72b3e821991e



And 4.3 patch attached.
From 675549ac8ebcc3749e2d6aab43ad966f32f18f6c Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Thu, 30 Jun 2016 16:00:08 +0200
Subject: [PATCH] Fix replica install with CA

The incorrect api was used, and CA record updated was duplicated.

https://fedorahosted.org/freeipa/ticket/5966
---
 ipaserver/install/bindinstance.py  | 3 +++
 ipaserver/install/cainstance.py| 5 +++--
 ipaserver/install/server/replicainstall.py | 3 ++-
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/ipaserver/install/bindinstance.py b/ipaserver/install/bindinstance.py
index abd1452efb39f2ac42bf2628d7caf355408444e7..0068ff3a16398fc2d07e1c1944168c1cdb81d0f4 100644
--- a/ipaserver/install/bindinstance.py
+++ b/ipaserver/install/bindinstance.py
@@ -1082,6 +1082,9 @@ class BindInstance(service.Service):
 self.__add_ipa_ca_record()
 
 def add_ipa_ca_dns_records(self, fqdn, domain_name, ca_configured=True):
+if not self.api.Backend.ldap2.isconnected():
+self.api.Backend.ldap2.connect(autobind=True)
+
 host, zone = fqdn.split(".", 1)
 if dns_zone_exists(zone, self.api):
 addrs = get_fwd_rr(zone, host, api=self.api)
diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index fa92aec95be4e812b5764d98e0e331870b9fc90d..3d6c1c07fccffee7265024868e377b2f10002912 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -309,7 +309,7 @@ class CAInstance(DogtagInstance):
 server_cert_name = 'Server-Cert cert-pki-ca'
 
 def __init__(self, realm=None, ra_db=None, host_name=None,
- dm_password=None, ldapi=True):
+ dm_password=None, ldapi=True, api=api):
 super(CAInstance, self).__init__(
 realm=realm,
 subsystem="CA",
@@ -325,6 +325,7 @@ class CAInstance(DogtagInstance):
 self.cert_file = None
 self.cert_chain_file = None
 self.create_ra_agent_db = True
+self.api = api
 
 if realm is not None:
 self.canickname = get_ca_nickname(realm)
@@ -1294,7 +1295,7 @@ class CAInstance(DogtagInstance):
 if bindinstance.dns_container_exists(
 api.env.host, api.env.basedn, ldapi=True, realm=api.env.realm
 ):
-bind = bindinstance.BindInstance(ldapi=True)
+bind = bindinstance.BindInstance(ldapi=True, api=self.api)
 bind.add_ipa_ca_dns_records(api.env.host, api.env.domain)
 
 def configure_replica(self, master_host, subject_base=None,
diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py
index 9ed6ef4bdeab22847774fd9197604f923079b745..7601ce19e31b435a48af899ef83235355f6acb90 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -1477,7 +1477,8 @@ def promote(installer):
 
 ca = cainstance.CAInstance(config.realm_name, certs.NSS_DIR,
host_name=config.host_name,
-   dm_password=config.dirman_password)
+   dm_password=config.dirman_password,
+   api=remote_api)
 ca.configure_replica(config.ca_host_name,
  subject_base=config.subject_base,
  ca_cert_bundle=ca_data)
-- 
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] 0067-72: webui for kerberos aliases

[Petr Vobornik]

On 06/30/2016 02:48 PM, Pavel Vomacka wrote:
> Hello,
> 
> please review these patches. First two patches fix two minor bugs in
> custom_command_multivalued_widget.
> 
> The rest of patches add webui for kerberos aliases.
> 
> https://fedorahosted.org/freeipa/ticket/5927
> 

A preliminary review - I only read the code.

Patch 0067: LGTM,

btw same wrong interface is in on_error_add, but there it is not use


Patch 0068: lGTM

Patch 0069:

1. A nitpick, not necessarily a NACK.
krb_custom_command_multivalued_widget should be named e.g.
krb_principal_multivalued_widget.

2. Doc texts for the new widges are missing. This can be added later.


3. `!principal_name || principal_name === '')` is the same as
`!principal_name`

so

var principal_name = value[0];

if (!principal_name || principal_name === '') {
principal_name = {};
}

could be simplified into
  var principal_name = value[0] || {};

but why is an object set into that.principal_name when it is later used
as a text: `that.principal_text.text(that.principal_name);`


Patch 0070: LGTM

Patch 0071: LGTM

Patch 0072: LGTM if the change of krbprincipalname to krbcanonicalname
is good.


-- 
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 0025][Tests] RFE: External trust

[Martin Babinsky]

On 06/30/2016 03:47 PM, Lenka Doudova wrote:

Hi,

attaching patch with some basic coverage for external trust feature. Bit
more detailed info in commit message.

Since the feature requires me to run commands previously used only for
forest root domains even for subdomains, I made some changes in
ipatests/test_integration/tasks.py file, so that it would enable me to
reuse existing function without copy-pasting them for one variable change.


Lenka





Hi Lenka,

I have a few comments:

1.)

-def establish_trust_with_ad(master, ad, extra_args=()):
+def establish_trust_with_ad(master, ad, extra_args=(), subdomain=False):

This modification seems extremely kludgy to me.

I would rather change the function signature to:

-def establish_trust_with_ad(master, ad, extra_args=()):
+def establish_trust_with_ad(master, ad_domain, extra_args=()):

and pass the domain name itself to the function instead of doing magic 
inside. The same goes with `remove trust_with_ad`. You may want to fix 
the calls to them in existing code so that the domain is passed instead 
of the whole trust object.


2.)

+def sync_time_hostname(host, srv_hostname):
+"""
+Syncs time with the remote server given by its hostname.
+Please note that this function leaves ntpd stopped.
+"""
+host.run_command(['systemctl', 'stop', 'ntpd'])
+host.run_command(['ntpdate', srv_hostname])
+
+

this looks like a duplicate of the function above it, why is it even needed?

3.)
+class TestExternalTrustWithSubdomain(ADTrustBase):
+"""
+Test establishing external trust with subdomain
+"""
+
+@classmethod
+def install(cls, mh):
+super(ADTrustBase, cls).install(mh)
+cls.ad = cls.ad_domains[0].ads[0]
+cls.install_adtrust()
+cls.check_sid_generation()
+
+# Determine whether the subdomain AD is available
+try:
+child_ad = cls.host_by_role(cls.optional_extra_roles[0])
+cls.ad_subdomain = '.'.join(child_ad.hostname.split('.')[1:])
+cls.ad_subdomain_hostname = child_ad.hostname
+except LookupError:
+cls.ad_subdomain = None
+
+cls.configure_dns_and_time()

Please use proper OOP practices when overriding the behavior of the base 
test class.


For starters you could rewrite the install method like this:

+@classmethod
+def install(cls, mh):
+# Determine whether the subdomain AD is available
+try:
+cls.child_ad = cls.host_by_role(cls.optional_extra_roles[0])
+cls.ad_subdomain = '.'.join(child_ad.hostname.split('.')[1:])
+cls.ad_subdomain_hostname = child_ad.hostname
+except LookupError:
+raise nose.SkipTest("AFAIK this will skip the whole test 
class")

+super(ADTrustBase, cls).install(mh)

with child_ad stored as class attribute, you can override 
`configure_dns_and_time`:

+def configure_dns_and_time(cls):
+tasks.configure_dns_for_trust(cls.master, cls.ad_subdomain)
	 # no need to re-implement the function just to get to the child AD DC 
hostname now

+tasks.sync_time(cls.master, cls.child_ad.hostname)

You can then use this class as an intermediary in the hierarchy and 
inherit the other external/non-external trust test classes from it, 
since most setup method in them are just copy-pasted from this one.


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

[Petr Vobornik]

On 06/30/2016 03:55 PM, Nathaniel McCallum wrote:
> On Thu, 2016-06-30 at 13:42 +0200, Petr Vobornik wrote:
>> On 06/29/2016 04:40 PM, Stanislav Laznicka wrote:
>>>
>>> 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.
>>>
>>
>> Attaching new version of Nathnaniel's patch with API.txt and VERSION
>> updated.
>>
>> ACK for 0096-2
>>
>> Pushed to master
>> * 0855b014b1edcb1632a41e380220abd7bb5e481a Add authentication
>> indicators
>> support to Host objects.
>>
>> The  "{Service|Host} {Read|Modify} " permissions looks good to me.
>> ACK
>> if Nathaniel agrees that it doesn't deserved it's own permission for
>> modify.
> 
> I agree. We can add it later if someone needs it.
> 

pushed to master:
* 97db87b383b1ae4639bdb51793354bad30adf5a9 host: Added permissions for
auth. indicators read/modify
* 235b19ba7f9807ecf10436d1a5b28518b4475a70 service: Added permissions
for auth. indicators read/modify


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

[Martin Basti]

On 30.06.2016 06:37, Fraser Tweedale wrote:

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

Thanks,
Fraser



ACK, thanks

Pushed to ipa-4-3: 8ce40940300e0e37191251a8a26bb8a4b5fcd604

-- 
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] 958 admintools: missing python3-ipaclient dependency

[Martin Basti]

On 30.06.2016 16:20, Jan Cholasta wrote:

On 30.6.2016 16:07, Martin Basti wrote:



On 06.06.2016 18:40, Petr Vobornik wrote:

On 06/06/2016 07:28 AM, Jan Cholasta wrote:

Hi,

On 3.6.2016 18:29, Petr Vobornik wrote:

admintools doesn't pull python[2|3]-ipaclient by default which ends
with exception if CLI is used.
Please use this ticket URL: 


done, new patch version attached.




Honza, can be this pushed?


Pushed where? The is no longer required, at least on master.


ok

--
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 30.6.2016 16:25, 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:

server: exclude Local commands from RPC
client: add placeholders for required remote plugins
client: ignore override errors in command overrides
plugable: add option to ignore override errors
cert: fix CLI output of cert_remove_hold
frontend: do not ignore client-side output params
user: add object plugin for user_status
server: define missing virtual attributes

contains mostly fixes for some bugs discovered in thin client. Works for
me, ACK.


Thanks, pushed to master: 2beb72ffa4bea5e22c2ba4685a524df36d1f800c

Attaching the patches for reference.

--
Jan Cholasta
From d31aa24657f5fee5cc5498fe8e43fe3926d36f6f Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Thu, 30 Jun 2016 06:37:16 +0200
Subject: [PATCH 1/8] server: define missing virtual attributes

Move virtual attributes defined in output params of methods into params of
the related object.

This fixes the virtual attributes being ommited in CLI output.

https://fedorahosted.org/freeipa/ticket/4739
---
 ipaserver/plugins/aci.py | 20 +++-
 ipaserver/plugins/baseuser.py|  7 ++--
 ipaserver/plugins/certprofile.py | 10 +++---
 ipaserver/plugins/delegation.py  | 14 +++-
 ipaserver/plugins/dns.py | 21 +++-
 ipaserver/plugins/host.py| 70 +++-
 ipaserver/plugins/idviews.py | 24 +++---
 ipaserver/plugins/otptoken.py|  8 ++---
 ipaserver/plugins/permission.py  | 15 +++--
 ipaserver/plugins/selfservice.py | 15 +++--
 ipaserver/plugins/service.py | 63 
 ipaserver/plugins/trust.py   | 46 ++
 12 files changed, 147 insertions(+), 166 deletions(-)

diff --git a/ipaserver/plugins/aci.py b/ipaserver/plugins/aci.py
index dd14d82..5647827 100644
--- a/ipaserver/plugins/aci.py
+++ b/ipaserver/plugins/aci.py
@@ -507,6 +507,10 @@ class aci(Object):
  flags=('virtual_attribute',),
 ),
 _prefix_option,
+Str('aci',
+label=_('ACI'),
+flags={'no_create', 'no_update', 'no_search'},
+),
 )
 
 
@@ -611,11 +615,6 @@ class aci_mod(crud.Update):
 Modify ACI.
 """
 NO_CLI = True
-has_output_params = (
-Str('aci',
-label=_('ACI'),
-),
-)
 
 takes_options = (_prefix_option,)
 
@@ -886,12 +885,6 @@ class aci_show(crud.Retrieve):
 """
 NO_CLI = True
 
-has_output_params = (
-Str('aci',
-label=_('ACI'),
-),
-)
-
 takes_options = (
 _prefix_option,
 DNParam('location?',
@@ -932,11 +925,6 @@ class aci_rename(crud.Update):
 Rename an ACI.
 """
 NO_CLI = True
-has_output_params = (
-Str('aci',
-label=_('ACI'),
-),
-)
 
 takes_options = (
 _prefix_option,
diff --git a/ipaserver/plugins/baseuser.py b/ipaserver/plugins/baseuser.py
index 7bb2e8a..8087418 100644
--- a/ipaserver/plugins/baseuser.py
+++ b/ipaserver/plugins/baseuser.py
@@ -59,9 +59,6 @@ baseuser_output_params = (
 Flag('has_keytab',
 label=_('Kerberos keys available'),
 ),
-Str('sshpubkeyfp*',
-label=_('SSH public key fingerprint'),
-),
)
 
 status_baseuser_output_params = (
@@ -353,6 +350,10 @@ class baseuser(LDAPObject):
 normalizer=normalize_sshpubkey,
 flags=['no_search'],
 ),
+Str('sshpubkeyfp*',
+label=_('SSH public key fingerprint'),
+flags={'virtual_attribute', 'no_create', 'no_update', 'no_search'},
+),
 StrEnum('ipauserauthtype*',
 cli_name='user_auth_type',
 label=_('User authentication types'),
diff --git a/ipaserver/plugins/certprofile.py b/ipaserver/plugins/certprofile.py
index 6f314e1..f446607 100644
--- a/ipaserver/plugins/certprofile.py
+++ b/ipaserver/plugins/certprofile.py
@@ -122,6 +122,10 @@ class certprofile(LDAPObject):
 label=_('Profile ID'),
 doc=_('Profile ID for referring to this profile'),
 ),
+Str('config',
+label=_('Profile configuration'),
+flags={'virtual_attribute', 'no_create', 'no_update', 'no_search'},
+),
 Str('description',
 required=True,
 cli_name='desc',
@@ -195,12 +199,6 @@ class certprofile_find(LDAPSearch):
 class certprofile_show(LDAPRetrieve):
 __doc__ = _("Display the properties of a Certificate Profile.")
 
-has_output_params = LDAPRetrieve.has_output_params + (
-Str('config',
-

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!

Patch set:

server: exclude Local commands from RPC
client: add placeholders for required remote plugins
client: ignore override errors in command overrides
plugable: add option to ignore override errors
cert: fix CLI output of cert_remove_hold
frontend: do not ignore client-side output params
user: add object plugin for user_status
server: define missing virtual attributes

contains mostly fixes for some bugs discovered in thin client. Works for 
me, 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 0025][Tests] RFE: External trust

[Lenka Doudova]

On 06/30/2016 04:12 PM, Oleg Fayans wrote:

Hi Lenka,

The changes in test_trust.py are fine.
As for tasks.py:
1. I'd rename sync_time_hostname to just sync_time and
There's already one sync_time function with different contents, it 
seemed nicer to create a new function than adding some "switch" into the 
old one, making it three time longer.

2. I would start ntpd again in the same method: it's no good to keep
this thing in mind each time you call it.
Leaving ntpd stopped is "legacy" from original function, and as I am not 
sure which purpose it serves, I decided to leave it as it is.


Besides, I would split the changes into 2 patches: one for tasks.py and
other for test_trust.py
No problem there, + adding ticket info into commit message, as I seem to 
have forgot again.



On 06/30/2016 03:47 PM, Lenka Doudova wrote:

Hi,

attaching patch with some basic coverage for external trust feature. Bit
more detailed info in commit message.

Since the feature requires me to run commands previously used only for
forest root domains even for subdomains, I made some changes in
ipatests/test_integration/tasks.py file, so that it would enable me to
reuse existing function without copy-pasting them for one variable change.


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 0025][Tests] RFE: External trust

[Oleg Fayans]

Hi Lenka,

The changes in test_trust.py are fine.
As for tasks.py:
1. I'd rename sync_time_hostname to just sync_time and
2. I would start ntpd again in the same method: it's no good to keep
this thing in mind each time you call it.

Besides, I would split the changes into 2 patches: one for tasks.py and
other for test_trust.py


On 06/30/2016 03:47 PM, Lenka Doudova wrote:
> Hi,
> 
> attaching patch with some basic coverage for external trust feature. Bit
> more detailed info in commit message.
> 
> Since the feature requires me to run commands previously used only for
> forest root domains even for subdomains, I made some changes in
> ipatests/test_integration/tasks.py file, so that it would enable me to
> reuse existing function without copy-pasting them for one variable change.
> 
> 
> 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] 958 admintools: missing python3-ipaclient dependency

[Jan Cholasta]

On 30.6.2016 16:07, Martin Basti wrote:



On 06.06.2016 18:40, Petr Vobornik wrote:

On 06/06/2016 07:28 AM, Jan Cholasta wrote:

Hi,

On 3.6.2016 18:29, Petr Vobornik wrote:

admintools doesn't pull python[2|3]-ipaclient by default which ends
with exception if CLI is used.

Please use this ticket URL: 

done, new patch version attached.




Honza, can be this pushed?


Pushed where? The is no longer required, at least on master.

--
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 0025][Tests] RFE: External trust

[Martin Babinsky]

On 06/30/2016 04:12 PM, Oleg Fayans wrote:

Hi Lenka,

The changes in test_trust.py are fine.
As for tasks.py:
1. I'd rename sync_time_hostname to just sync_time and
2. I would start ntpd again in the same method: it's no good to keep
this thing in mind each time you call it.

If you start ntpd again you may de-sync AD DC and IPA master again when 
there is substantial time difference between them (as is usually the 
case). It is best to left ntpd stopped and even disable it afterwards.



Besides, I would split the changes into 2 patches: one for tasks.py and
other for test_trust.py


On 06/30/2016 03:47 PM, Lenka Doudova wrote:

Hi,

attaching patch with some basic coverage for external trust feature. Bit
more detailed info in commit message.

Since the feature requires me to run commands previously used only for
forest root domains even for subdomains, I made some changes in
ipatests/test_integration/tasks.py file, so that it would enable me to
reuse existing function without copy-pasting them for one variable change.


Lenka








--
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] 958 admintools: missing python3-ipaclient dependency

[Martin Basti]

On 06.06.2016 18:40, Petr Vobornik wrote:

On 06/06/2016 07:28 AM, Jan Cholasta wrote:

Hi,

On 3.6.2016 18:29, Petr Vobornik wrote:

admintools doesn't pull python[2|3]-ipaclient by default which ends
with exception if CLI is used.

Please use this ticket URL: 

done, new patch version attached.




Honza, can be this pushed?
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 0077 Check for CA subject name collision before attempting creation

[Martin Basti]

On 24.06.2016 10:34, Milan Kubík wrote:

On 06/24/2016 09:34 AM, Fraser Tweedale wrote:

Hi,

Attached patch fixes https://fedorahosted.org/freeipa/ticket/5981.

Cheers,
Fraser

Thanks for the patch, ACK.


Pushed to master: 16f33ddb51523fe9a4c68e9151901ece10a5

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

[Nathaniel McCallum]

On Thu, 2016-06-30 at 13:42 +0200, Petr Vobornik wrote:
> On 06/29/2016 04:40 PM, Stanislav Laznicka wrote:
> > 
> > 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.
> > 
> 
> Attaching new version of Nathnaniel's patch with API.txt and VERSION
> updated.
> 
> ACK for 0096-2
> 
> Pushed to master
> * 0855b014b1edcb1632a41e380220abd7bb5e481a Add authentication
> indicators
> support to Host objects.
> 
> The  "{Service|Host} {Read|Modify} " permissions looks good to me.
> ACK
> if Nathaniel agrees that it doesn't deserved it's own permission for
> modify.

I agree. We can add it later if someone needs it.

-- 
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 0025][Tests] RFE: External trust

[Lenka Doudova]

Hi,

attaching patch with some basic coverage for external trust feature. Bit 
more detailed info in commit message.


Since the feature requires me to run commands previously used only for 
forest root domains even for subdomains, I made some changes in 
ipatests/test_integration/tasks.py file, so that it would enable me to 
reuse existing function without copy-pasting them for one variable change.



Lenka

From 71f4720c4b8b2d6ba2f3f7efc22ad75643fe7e53 Mon Sep 17 00:00:00 2001
From: Lenka Doudova 
Date: Thu, 30 Jun 2016 12:23:02 +0200
Subject: [PATCH] Tests: External trust

Provides basic coverage for external trust feature.
Test cases:
1. verify an external trust with AD subdomain can be established
   - verify only one trustdomain is listed
   - verify subdomain users are resolvable
   - verify trust can be deleted
2. verify non-external trust with AD subdomain cannot be established
3. verify an external trust with AD forest root domain can be established
   - verify that even if AD subdomain is specified, it is not associated with the trust
   - verify trust can be deleted
---
 ipatests/test_integration/tasks.py  |  30 -
 ipatests/test_integration/test_trust.py | 208 
 2 files changed, 233 insertions(+), 5 deletions(-)

diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py
index 5be7cdae3ac777bbf0fc52e6c511969e9fabcd72..9277a76c03805aa4dc354c3346ed8505499e4806 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -443,7 +443,7 @@ def configure_dns_for_trust(master, ad):
 pass
 
 
-def establish_trust_with_ad(master, ad, extra_args=()):
+def establish_trust_with_ad(master, ad, extra_args=(), subdomain=False):
 """
 Establishes trust with Active Directory. Trust type is detected depending
 on the presence of SfU (Services for Unix) support on the AD.
@@ -461,9 +461,15 @@ def establish_trust_with_ad(master, ad, extra_args=()):
 kinit_admin(master)
 master.run_command(['klist'])
 master.run_command(['smbcontrol', 'all', 'debug', '100'])
+
+if subdomain:
+ad_domain = ad
+else:
+ad_domain = ad.domain.name
+
 util.run_repeatedly(master,
 ['ipa', 'trust-add',
- '--type', 'ad', ad.domain.name,
+ '--type', 'ad', ad_domain,
  '--admin', 'Administrator',
  '--password'] + list(extra_args),
 stdin_text=master.config.ad_admin_password)
@@ -471,18 +477,23 @@ def establish_trust_with_ad(master, ad, extra_args=()):
 clear_sssd_cache(master)
 
 
-def remove_trust_with_ad(master, ad):
+def remove_trust_with_ad(master, ad, subdomain=False):
 """
 Removes trust with Active Directory. Also removes the associated ID range.
 """
 
 kinit_admin(master)
 
+if subdomain:
+ad_domain = ad
+else:
+ad_domain = ad.domain.name
+
 # Remove the trust
-master.run_command(['ipa', 'trust-del', ad.domain.name])
+master.run_command(['ipa', 'trust-del', ad_domain])
 
 # Remove the range
-range_name = ad.domain.name.upper() + '_id_range'
+range_name = ad_domain.upper() + '_id_range'
 master.run_command(['ipa', 'idrange-del', range_name])
 
 
@@ -610,6 +621,15 @@ def sync_time(host, server):
 host.run_command(['ntpdate', server.hostname])
 
 
+def sync_time_hostname(host, srv_hostname):
+"""
+Syncs time with the remote server given by its hostname.
+Please note that this function leaves ntpd stopped.
+"""
+host.run_command(['systemctl', 'stop', 'ntpd'])
+host.run_command(['ntpdate', srv_hostname])
+
+
 def connect_replica(master, replica, domain_level=None):
 if domain_level is None:
 domain_level = master.config.domain_level
diff --git a/ipatests/test_integration/test_trust.py b/ipatests/test_integration/test_trust.py
index 772a50842f7c251b78d68fd4bd3c91668f580d50..6074217e0c738623279cc191c1794f0ce3df0a3a 100644
--- a/ipatests/test_integration/test_trust.py
+++ b/ipatests/test_integration/test_trust.py
@@ -23,6 +23,7 @@ import re
 from ipatests.test_integration.base import IntegrationTest
 from ipatests.test_integration import tasks
 from ipatests.test_integration import util
+from ipaplatform.paths import paths
 
 
 class ADTrustBase(IntegrationTest):
@@ -224,3 +225,210 @@ class TestInvalidRangeTypes(ADTrustBase):
 
 # The trust-add command is supposed to fail
 assert result.returncode == 1
+
+
+class TestExternalTrustWithSubdomain(ADTrustBase):
+"""
+Test establishing external trust with subdomain
+"""
+
+@classmethod
+def install(cls, mh):
+super(ADTrustBase, cls).install(mh)
+cls.ad = cls.ad_domains[0].ads[0]
+cls.install_adtrust()
+cls.check_sid_generation()
+
+# Determine whether the subdomain AD is available
+

Re: [Freeipa-devel] [PATCH 0144] Fix `Conflicts` with ipa-python

[Martin Basti]

On 30.06.2016 15:36, Stanislav Laznicka wrote:

On 06/29/2016 02:36 PM, Petr Spacek wrote:

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.


ACK, worked for me as well.


Pushed to master: 669da991837267435a6e6563794f93c2f207c80b

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

[Martin Basti]

On 30.06.2016 15:16, Florence Blanc-Renaud wrote:

On 06/30/2016 01:30 PM, Fraser Tweedale wrote:

On Thu, Jun 30, 2016 at 07:49:04PM +1000, Fraser Tweedale wrote:

On Thu, Jun 30, 2016 at 11:38:35AM +0200, Florence Blanc-Renaud wrote:

On 06/30/2016 06:29 AM, Fraser Tweedale wrote:
On Wed, Jun 29, 2016 at 11:30:14AM +0200, Florence Blanc-Renaud 
wrote:

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


Thanks for review, Flo.  Updated patch attached.

Cheers,
Fraser


Hi Fraser,

thanks for updated patch. There is still a pep8 error:
./ipalib/errors.py:1399:1: E302 expected 2 blank lines, found 1


Whups!

I am also wondering if the message is clear enough. When running 
the CLI
it's ok because the user typed --add, but the GUI displays "'add' 
is not

supported for user principals"
and I feel
"'add' option is not supported for user principals"
would be more user-friendly. What do you think?


Yes, I concur that mentioning "option" is an improvement. Will cut
a new patch shortly

Thanks!
Fraser


Updated patch attached.  Third time's the charm? ;)

Cheers,
Fraser


Hi Fraser,

thanks for the updated patch. Yes, 3rd time's the charm :)
Ack,
Flo.


Pushed to master: 3fab1b63502c3206d792b7aeaa12d486612f0137

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

[Stanislav Laznicka]

On 06/29/2016 02:36 PM, Petr Spacek wrote:

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.


ACK, worked for me as well.

--
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/30/2016 01:30 PM, Fraser Tweedale wrote:

On Thu, Jun 30, 2016 at 07:49:04PM +1000, Fraser Tweedale wrote:

On Thu, Jun 30, 2016 at 11:38:35AM +0200, Florence Blanc-Renaud wrote:

On 06/30/2016 06:29 AM, Fraser Tweedale wrote:

On Wed, Jun 29, 2016 at 11:30:14AM +0200, Florence Blanc-Renaud wrote:

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


Thanks for review, Flo.  Updated patch attached.

Cheers,
Fraser


Hi Fraser,

thanks for updated patch. There is still a pep8 error:
./ipalib/errors.py:1399:1: E302 expected 2 blank lines, found 1


Whups!


I am also wondering if the message is clear enough. When running the CLI
it's ok because the user typed --add, but the GUI displays "'add' is not
supported for user principals"
and I feel
"'add' option is not supported for user principals"
would be more user-friendly. What do you think?


Yes, I concur that mentioning "option" is an improvement.  Will cut
a new patch shortly

Thanks!
Fraser


Updated patch attached.  Third time's the charm? ;)

Cheers,
Fraser


Hi Fraser,

thanks for the updated patch. Yes, 3rd time's the charm :)
Ack,
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] 0007 Fix ipa-server-certinstall with certs signed by 3rd-party CA

[Petr Vobornik]

On 06/29/2016 02:17 PM, Stanislav Laznicka wrote:
> 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
> 
> 

Pushed to master: 025cfd911bce6214ef2b4311b16c5b6df6ad173a

According to Honza, it doesn't solve all corner cases. This can be fixed
in a future. Honza, please open a ticket with the corner cases when you
have time.

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

[Martin Basti]

On 30.06.2016 14:40, Oleg Fayans wrote:

Hi Martin,

Attached is a new version of the patch with two test cases separated.

On 06/29/2016 12:23 PM, 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}'. "

Right, that's what this ticket is about:
https://fedorahosted.org/freeipa/ticket/6006

Once these changes are implemented, we can update this test


Wat?

You get exactly the right message from ipa-replica-install, tested, 
reviewed by several people.



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

Oh, ok. But it does not seem possible to setup client providing only
--realm without --domain: installer would not do it.



Try to read again: "should not use *random* REALM". Nothing prevents you 
to use, --realm=TEST.REALM --domain=random-blah-domain



Martin^2







NACK

+domain_name = 'exxample.test'
+realm_name = domain_name.upper()

you still use random realm name, and you still don't test 
ipa-replica-install, that ticket has nothing related to domain in 
ipa-client-install, it is related to replica promotion


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

[Freeipa-devel] [PATCH] 0067-72: webui for kerberos aliases

[Pavel Vomacka]

Hello,

please review these patches. First two patches fix two minor bugs in 
custom_command_multivalued_widget.


The rest of patches add webui for kerberos aliases.

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

--
Pavel^3 Vomacka

From d6e0337fd83a4e337c429ecc23038e7af754312e Mon Sep 17 00:00:00 2001
From: Pavel Vomacka 
Date: Thu, 30 Jun 2016 14:32:27 +0200
Subject: [PATCH 1/6] Change error handling of remove command

The custom_command_multivalued_widget handles remove command error correctly and shows
errror message.

Part of: https://fedorahosted.org/freeipa/ticket/5381
---
 install/ui/src/freeipa/widget.js | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/install/ui/src/freeipa/widget.js b/install/ui/src/freeipa/widget.js
index 0972efadb46ac7b5811f4e072121a448618fe434..1f767ed14342bd3bb5c8c7ac5384c34a9b2c3abe 100644
--- a/install/ui/src/freeipa/widget.js
+++ b/install/ui/src/freeipa/widget.js
@@ -1548,8 +1548,11 @@ IPA.custom_command_multivalued_widget = function(spec) {
 /**
  * Called on error of remove command. Override point.
  */
-that.on_error_remove = function(data) {
-IPA.notify(data.result.summary, 'error');
+that.on_error_remove = function(xhr, text_status, error_thrown) {
+if (error_thrown.message) {
+var msg = error_thrown.message;
+IPA.notify(msg, 'error');
+}
 };
 
 /**
-- 
2.5.5

From a132113a19d92c7c3bb3a6f0fdc3b922009c8f0e Mon Sep 17 00:00:00 2001
From: Pavel Vomacka 
Date: Thu, 30 Jun 2016 14:34:10 +0200
Subject: [PATCH 2/6] Set default confirmation button label to 'Remove'

Part of: https://fedorahosted.org/freeipa/ticket/5831
---
 install/ui/src/freeipa/widget.js | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/install/ui/src/freeipa/widget.js b/install/ui/src/freeipa/widget.js
index 1f767ed14342bd3bb5c8c7ac5384c34a9b2c3abe..c422f8aa9aefdf67a5770ff0b58754e0d3eb9e04 100644
--- a/install/ui/src/freeipa/widget.js
+++ b/install/ui/src/freeipa/widget.js
@@ -1672,7 +1672,8 @@ IPA.custom_command_multivalued_widget = function(spec) {
 var spec = that.remove_dialog_spec || {
 title: title,
 message: message,
-on_ok: perform_remove
+on_ok: perform_remove,
+ok_label: '@i18n:buttons.remove'
 };
 
 that.remove_dialog = IPA.confirm_dialog(spec);
-- 
2.5.5

From d7d6a29361b30e09e4c58451ea3bf21c1bd0168d Mon Sep 17 00:00:00 2001
From: Pavel Vomacka 
Date: Thu, 30 Jun 2016 14:34:33 +0200
Subject: [PATCH 3/6] Add widgets for kerberos aliases

Create own custom_command_multivalued_widget for kerberos aliases.

https://fedorahosted.org/freeipa/ticket/5927
---
 install/ui/src/freeipa/widget.js   | 99 ++
 install/ui/test/data/ipa_init.json |  6 +++
 ipaserver/plugins/internal.py  |  6 +++
 3 files changed, 111 insertions(+)

diff --git a/install/ui/src/freeipa/widget.js b/install/ui/src/freeipa/widget.js
index c422f8aa9aefdf67a5770ff0b58754e0d3eb9e04..96a1643bb87980cce674ef97de45d3f2823cfa3e 100644
--- a/install/ui/src/freeipa/widget.js
+++ b/install/ui/src/freeipa/widget.js
@@ -1793,6 +1793,101 @@ IPA.custom_command_multivalued_widget = function(spec) {
 return that;
 };
 
+IPA.krb_custom_command_multivalued_widget = function (spec) {
+
+spec = spec || {};
+
+spec.adder_dialog_spec = spec.adder_dialog_spec || {
+title: '@i18n:krbaliases.adder_title',
+fields: [
+{
+$type: 'text',
+name: 'krbprincalname',
+label: '@i18n:krbaliases.add_krbal_label'
+}
+]
+};
+
+var that = IPA.custom_command_multivalued_widget(spec);
+
+that.create_remove_dialog_title = function(row) {
+return text.get('@i18n:krbaliases.remove_title');
+};
+
+that.create_remove_dialog_message = function(row) {
+var message = text.get('@i18n:krbaliases.remove_message');
+message = message.replace('${alias}', row.widget.principal_name);
+
+return message;
+};
+
+
+that.create_remove_args = function(row) {
+var pkey = that.facet.get_pkey();
+var krbprincipalname = row.widget.principal_name;
+krbprincipalname = [ krbprincipalname ];
+
+var args = [
+pkey,
+krbprincipalname
+];
+
+return args;
+};
+
+that.create_add_args = function(row) {
+var pkey = that.facet.get_pkey();
+var krbprincipalname = that.adder_dialog.get_field('krbprincalname').value;
+
+var args = [
+pkey,
+krbprincipalname
+];
+
+return args;
+};
+
+return that;
+};
+
+IPA.krb_principal_widget = function(spec) {
+spec = spec || {};
+
+var that = IPA.input_widget();
+
+that.create = function(container) {
+that.widget_create(container);
+

Re: [Freeipa-devel] [PATCH 0053] Fix wrong imports in copy-schema-to-ca

[Martin Basti]

On 28.06.2016 16:35, Petr Spacek wrote:

On 28.6.2016 14:52, Stanislav Laznicka wrote:

Hello,

The attached patch fixes wrong imports in copy-schema-to-ca.py script.

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

ACK


Pushed to master: f3858be6e353fadf0b1da1c31b908264ddd636c5

--
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/30/2016 01:57 PM, Pavel Vomacka wrote:
> 
> 
> On 06/29/2016 05:42 PM, Petr Vobornik wrote:
>> 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.
> Thank you, everything works well.
> 
> ACK
> 

master:
* e65ce4fedc8e8e4f16c8d0c56f7618a84e8e7f85 Add support to change button
css class on confirm dialog
* 7f4de88ea1da11d4111a917672c0cb7ab9866c90 Add button for server-del 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] 0064: webui: simplify confirmation messages in confirmation dialogs

[Petr Vobornik]

On 06/30/2016 10:39 AM, Pavel Vomacka wrote:
> 
> 
> On 06/29/2016 04:40 PM, Petr Vobornik wrote:
>> 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.
>>
> Changed. Updated patch attached.
> 

ACK

Pushed to master: a3c7f845e019f79c2cefde0526790dec4f118b56

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

[Martin Basti]

On 29.06.2016 19:38, Petr Spacek wrote:

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


ACK

Pushed to master: 3b79ce005c43c6cb270175dc987eed3ba19e0f53

--
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] [PATCHES 662-665] session: do not initialize session manager on import

[Jan Cholasta]

On 30.6.2016 13:51, Martin Babinsky wrote:

On 06/29/2016 04:53 PM, Jan Cholasta wrote:

Hi,

the attached patches fix .

Honza




ACK.


Thanks.

Pushed to master: 2615103c68e68596473260064dbe84585073eb51

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

[Pavel Vomacka]

On 06/29/2016 05:42 PM, Petr Vobornik wrote:

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.

Thank you, everything works well.

ACK

--
Pavel^3 Vomacka

--
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 0546-0547] use timestamps for ipareplica-conncheck.log

[Martin Basti]

On 30.06.2016 12:07, Petr Spacek wrote:

On 30.6.2016 10:21, Jan Cholasta wrote:

On 30.6.2016 10:12, Petr Spacek wrote:

On 30.6.2016 10:14, Jan Cholasta wrote:

On 30.6.2016 10:06, Petr Spacek wrote:

On 30.6.2016 10:02, Jan Cholasta wrote:

On 30.6.2016 09:56, Petr Spacek wrote:

On 30.6.2016 09:40, Martin Basti wrote:

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

"The easiest solution would be to add timestamps to logs, or log to
different
logs from oddjob or from installer (ipareplica-conncheck.local.log and
ipareplica-conncheck.remote.log)"

Actually the easiest solution would be not to log into a file when executed
from oddjob.

Well, IPA is hard enough to debug even with logs. I would not make situation
even worse by not logging at all :-)

The commands logs into stderr, and both stdout and stderr are sent back to the
caller of the oddjob.

Alternatively, it could log into a different file (say
/var/log/ipareplica-conncheck-oddjob.log). IMO timestamps are an overkill to
fix this bug.

When we are at it, a custom logger is overkill. IMHO we should log everything
to journal and be done with it ...

It's not, we want to log to at least stderr ourselves.

Also, it would be even harder to implement than timestamps, and time is a

Sure, we do not have time:
=> ACK for current version of the patch.

Petr^2 Spacek


factor here. It would fit more into
.


Petr^2 Spacek


Patches attached.

I would rather use timestamp format with dashes between numbers to make it
easier to read and parse for humans.

Compare:

201606270954
201606290954
201606300954

with

2016-06-27-09-54
2016-06-29-09-54
2016-06-30-09-54








New patches attaches
From f3c1532fd4415ab6786e5290b73b272648fd8dc2 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Thu, 30 Jun 2016 11:21:34 +0200
Subject: [PATCH 1/2] Add option --no-log for ipa-replica-conncheck script

When option is sued, ipa-replica-conncheck will not log into file

https://fedorahosted.org/freeipa/ticket/5757
---
 install/tools/ipa-replica-conncheck | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/install/tools/ipa-replica-conncheck b/install/tools/ipa-replica-conncheck
index e308b118f20306107bc62eba2a60187fbc52f4fc..6f69cc81517299f8cf8decef7c79f7de682d0c80 100755
--- a/install/tools/ipa-replica-conncheck
+++ b/install/tools/ipa-replica-conncheck
@@ -181,6 +181,8 @@ def parse_options():
 parser.add_option("-q", "--quiet", dest="quiet",
   action="store_true",
   default=False, help="Output only errors")
+parser.add_option("--no-log", dest="log_to_file", action="store_false",
+  default=True, help="Do not log into file")
 
 options, args = parser.parse_args()
 safe_options = parser.get_safe_opts(options)
@@ -212,7 +214,7 @@ def parse_options():
 def logging_setup(options):
 log_file = None
 
-if os.getegid() == 0:
+if os.getegid() == 0 and options.log_to_file:
 log_file = paths.IPAREPLICA_CONNCHECK_LOG
 
 standard_logging_setup(log_file, debug=options.debug)
-- 
2.5.5

From 48e919d87f7490ddcdf7c60a57bdd93a5bdf1ea4 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Thu, 30 Jun 2016 11:22:31 +0200
Subject: [PATCH 2/2] Do not log to file in remote conncheck side

https://fedorahosted.org/freeipa/ticket/5757
---
 install/oddjob/org.freeipa.server.conncheck | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/install/oddjob/org.freeipa.server.conncheck b/install/oddjob/org.freeipa.server.conncheck
index ab7a46a86d5e3dbf54efa51c82656485a3030f26..9d57ea26e078d7d04f493fd4ce6b18330fd69325 100755
--- a/install/oddjob/org.freeipa.server.conncheck
+++ b/install/oddjob/org.freeipa.server.conncheck
@@ -1,2 +1,2 @@
 #!/bin/sh
-exec /usr/sbin/ipa-replica-conncheck --replica "$1" 2>&1
+exec /usr/sbin/ipa-replica-conncheck --no-log --replica "$1" 2>&1
-- 
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] [PATCHES 662-665] session: do not initialize session manager on import

[Martin Babinsky]

On 06/29/2016 04:53 PM, Jan Cholasta wrote:

Hi,

the attached patches fix .

Honza




ACK.

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

[Petr Vobornik]

On 06/29/2016 06:38 PM, Petr Vobornik wrote:
> 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.
> 

master:
* 55049fceb978f2e20b13800b5377428de386 Add authentication
identificator to host page
* ec6925e775598602e909d7a1f226f0c1e28cb074 Change paths of strings in
auth indicators widget on service 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 0096] Add authentication indicators support to Host objects

[Petr Vobornik]

On 06/29/2016 04:40 PM, Stanislav Laznicka wrote:
> 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.
> 

Attaching new version of Nathnaniel's patch with API.txt and VERSION
updated.

ACK for 0096-2

Pushed to master
* 0855b014b1edcb1632a41e380220abd7bb5e481a Add authentication indicators
support to Host objects.

The  "{Service|Host} {Read|Modify} " permissions looks good to me. ACK
if Nathaniel agrees that it doesn't deserved it's own permission for
modify.
-- 
Petr Vobornik
From 3de08f354a8714a752b567850968b57ffc44553d Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum 
Date: Tue, 21 Jun 2016 14:19:03 -0400
Subject: [PATCH] Add authentication indicators support to Host objects

https://fedorahosted.org/freeipa/ticket/433
---
 API.txt   |  9 ++---
 VERSION   |  4 ++--
 ipaserver/plugins/host.py | 17 -
 3 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/API.txt b/API.txt
index 76e58aeec4301577952f919b17a58b71c06a..19922660ad1787d87337b37e099c7fd9475eda53 100644
--- a/API.txt
+++ b/API.txt
@@ -2257,7 +2257,7 @@ output: Output('summary', type=[, ])
 output: Output('value', type=[])
 output: Output('warning', type=[, , ])
 command: host_add/1
-args: 1,23,3
+args: 1,24,3
 arg: Str('fqdn', cli_name='hostname')
 option: Str('addattr*', cli_name='addattr')
 option: Flag('all', autofill=True, cli_name='all', default=False)
@@ -2268,6 +2268,7 @@ option: Str('ipaassignedidview?')
 option: Bool('ipakrbokasdelegate?', cli_name='ok_as_delegate')
 option: Bool('ipakrbrequirespreauth?', cli_name='requires_pre_auth')
 option: Str('ipasshpubkey*', cli_name='sshpubkey')
+option: Str('krbprincipalauthind*', cli_name='auth_ind')
 option: Str('l?', cli_name='locality')
 option: Str('macaddress*')
 option: Flag('no_members', autofill=True, default=False)
@@ -2380,7 +2381,7 @@ output: Output('completed', type=[])
 output: Output('failed', type=[])
 output: Entry('result')
 command: host_find/1
-args: 1,34,4
+args: 1,35,4
 arg: Str('criteria?')
 option: Flag('all', autofill=True, cli_name='all', default=False)
 option: Str('description?', autofill=False, cli_name='desc')
@@ -2392,6 +2393,7 @@ option: Str('in_netgroup*', cli_name='in_netgroups')
 option: Str('in_role*', cli_name='in_roles')
 option: Str('in_sudorule*', cli_name='in_sudorules')
 option: Str('ipaassignedidview?', autofill=False)
+option: Str('krbprincipalauthind*', autofill=False, cli_name='auth_ind')
 option: Str('l?', autofill=False, cli_name='locality')
 option: Str('macaddress*', autofill=False)
 option: Str('man_by_host*', cli_name='man_by_hosts')
@@ -2421,7 +2423,7 @@ output: ListOfEntries('result')
 output: Output('summary', type=[, ])
 output: Output('truncated', type=[])
 command: host_mod/1
-args: 1,24,3
+args: 1,25,3
 arg: Str('fqdn', cli_name='hostname')
 option: Str('addattr*', cli_name='addattr')
 option: Flag('all', autofill=True, cli_name='all', default=False)
@@ -2431,6 +2433,7 @@ option: Str('ipaassignedidview?', autofill=False)
 option: Bool('ipakrbokasdelegate?', autofill=False, cli_name='ok_as_delegate')
 option: Bool('ipakrbrequirespreauth?', autofill=False, cli_name='requires_pre_auth')
 option: Str('ipasshpubkey*', autofill=False, cli_name='sshpubkey')
+option: Str('krbprincipalauthind*', autofill=False, cli_name='auth_ind')
 option: Str('krbprincipalname?', cli_name='principalname')
 option: Str('l?', autofill=False, cli_name='locality')
 option: Str('macaddress*', autofill=False)
diff --git a/VERSION b/VERSION
index d4d7228edb1e29c8655c058e1e4fb727950aeabc..5c3aef2e40415b869978cb9aa59bf940e0bcfb85 100644
--- a/VERSION
+++ b/VERSION
@@ -90,5 +90,5 @@ IPA_DATA_VERSION=2010061412
 #  #
 
 IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=202
-# Last change: schema: support plugin versioning
+IPA_API_VERSION_MINOR=203
+# Last change: host: added authentication indicators
diff --git 

Re: [Freeipa-devel] [PATCH] 0082 cert-request: better error msg when 'add' not supported

[Fraser Tweedale]

On Thu, Jun 30, 2016 at 07:49:04PM +1000, Fraser Tweedale wrote:
> On Thu, Jun 30, 2016 at 11:38:35AM +0200, Florence Blanc-Renaud wrote:
> > On 06/30/2016 06:29 AM, Fraser Tweedale wrote:
> > > On Wed, Jun 29, 2016 at 11:30:14AM +0200, Florence Blanc-Renaud wrote:
> > > > 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
> > > > 
> > > Thanks for review, Flo.  Updated patch attached.
> > > 
> > > Cheers,
> > > Fraser
> > > 
> > Hi Fraser,
> > 
> > thanks for updated patch. There is still a pep8 error:
> > ./ipalib/errors.py:1399:1: E302 expected 2 blank lines, found 1
> > 
> Whups!
> 
> > I am also wondering if the message is clear enough. When running the CLI
> > it's ok because the user typed --add, but the GUI displays "'add' is not
> > supported for user principals"
> > and I feel
> > "'add' option is not supported for user principals"
> > would be more user-friendly. What do you think?
> > 
> Yes, I concur that mentioning "option" is an improvement.  Will cut
> a new patch shortly
> 
> Thanks!
> Fraser
> 
Updated patch attached.  Third time's the charm? ;)

Cheers,
Fraser
From 99c1d5c32b19a070f5e995fce4de32dee39433c1 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Wed, 29 Jun 2016 15:02:51 +1000
Subject: [PATCH] cert-request: better error msg when 'add' not supported

cert-request supports adding service principals that don't exist.
If add is requested for other principal types, the error message
just says "the principal doesn't exist".

Add a new error type with better error message to explain that 'add'
is not supported for host or user principals.

Fixes: https://fedorahosted.org/freeipa/ticket/5991
---
 ipalib/errors.py  | 10 ++
 ipaserver/plugins/cert.py | 21 ++---
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/ipalib/errors.py b/ipalib/errors.py
index 
10491a94211648df8bda60f3dbc9e52d19e83d10..7b4f15dd60ee80719195ba1b9b85d075b10bdf4f
 100644
--- a/ipalib/errors.py
+++ b/ipalib/errors.py
@@ -1397,6 +1397,16 @@ class ServerRemovalError(ExecutionError):
 format = _('Server removal aborted: %(reason)s.')
 
 
+class OperationNotSupportedForPrincipalType(ExecutionError):
+"""
+**4034** Raised when an operation is not supported for a principal type
+"""
+
+errno = 4034
+format = _(
+'%(operation)s is not supported for %(principal_type)s principals')
+
+
 class BuiltinError(ExecutionError):
 """
 **4100** Base class for builtin execution errors (*4100 - 4199*).
diff --git a/ipaserver/plugins/cert.py b/ipaserver/plugins/cert.py
index 
63351c54c134f2bd58e8eb18bb0dc3bc6a5734b3..526360bb6b777abd398ad6d5e63f040fb52ac529
 100644
--- a/ipaserver/plugins/cert.py
+++ b/ipaserver/plugins/cert.py
@@ -145,6 +145,12 @@ http://www.ietf.org/rfc/rfc5280.txt
 
 USER, HOST, SERVICE = range(3)
 
+PRINCIPAL_TYPE_STRING_MAP = {
+USER: _('user'),
+HOST: _('host'),
+SERVICE: _('service'),
+}
+
 register = Registry()
 
 PKIDATE_FORMAT = '%Y-%m-%d'
@@ -385,7 +391,9 @@ class cert_request(Create, BaseCertMethod, VirtualCommand):
 ),
 Flag(
 'add',
-doc=_("automatically add the principal if it doesn't exist"),
+doc=_(
+"automatically add the principal if it doesn't exist "
+"(service principals only)"),
 ),
 )
 
@@ -480,8 +488,15 @@ class cert_request(Create, BaseCertMethod, VirtualCommand):
 elif principal_type == USER:
 principal_obj = api.Command['user_show'](principal_name, 
all=True)
 except errors.NotFound as e:
-if principal_type == SERVICE and add:
-principal_obj = api.Command['service_add'](principal_string, 
force=True)
+if add:
+if principal_type == SERVICE:
+principal_obj = api.Command['service_add'](
+principal_string, force=True)
+else:
+princtype_str = PRINCIPAL_TYPE_STRING_MAP[principal_type]
+raise errors.OperationNotSupportedForPrincipalType(
+operation=_("'add' option"),
+

Re: [Freeipa-devel] [PATCH 0545] cert.py: split doctring to multiple ugettext strings

[Martin Basti]

On 30.06.2016 13:20, Stanislav Laznicka wrote:

On 06/30/2016 01:08 PM, Martin Basti wrote:



On 30.06.2016 10:13, Stanislav Laznicka wrote:

On 06/30/2016 09:18 AM, Martin Basti wrote:
Make life of translators easier, there was recent change in cert.py 
docstring, so they have to translate the whole docstring again, so 
I'm splitting it to multiple parts.



Patch attached

I'm not sure whether the "See RFC 5280 for more details" should be 
split from the link.

It doesn't matter
Also, you need to add "\n" at the end of each reason string (you're 
not using multiline strings there).

Fixed, patch attached


Looks good to me, ACK.


Pushed to master: fed9d9aaa73604f6e100acbe2d3c192f4e4676e8

--
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 0548] Fix replica install with CA

[Martin Basti]

On 30.06.2016 13:18, Petr Spacek wrote:

On 30.6.2016 13:04, Martin Basti wrote:

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

This only for master branch, ipa-4-3 fix will be different (soon)

Patch attached

ACK


Pushed to master: a155f692e7ad7807a5ea28250d1e72b3e821991e

--
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 0545] cert.py: split doctring to multiple ugettext strings

[Stanislav Laznicka]

On 06/30/2016 01:08 PM, Martin Basti wrote:



On 30.06.2016 10:13, Stanislav Laznicka wrote:

On 06/30/2016 09:18 AM, Martin Basti wrote:
Make life of translators easier, there was recent change in cert.py 
docstring, so they have to translate the whole docstring again, so 
I'm splitting it to multiple parts.



Patch attached

I'm not sure whether the "See RFC 5280 for more details" should be 
split from the link.

It doesn't matter
Also, you need to add "\n" at the end of each reason string (you're 
not using multiline strings there).

Fixed, patch attached


Looks good to me, ACK.

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0548] Fix replica install with CA

[Petr Spacek]

On 30.6.2016 13:04, Martin Basti wrote:
> https://fedorahosted.org/freeipa/ticket/5966
> 
> This only for master branch, ipa-4-3 fix will be different (soon)
> 
> Patch attached

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

[Freeipa-devel] [PATCH] 0085 Fix upgrade when Dogtag also upgraded from 10.2 -> 10.3

[Fraser Tweedale]

Hullo,

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

Cheers,
Fraser
From c92ed38c0ef41814dec6ddf4a003948af5bc0beb Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Thu, 30 Jun 2016 21:01:07 +1000
Subject: [PATCH] Fix upgrade when Dogtag also upgraded from 10.2 -> 10.3

ipa-server-upgrade from pre-lightweight CAs version fails when
Dogtag is also being upgraded from pre-lightweight CAs version,
because Dogtag needs to be restarted after adding the lightweight
CAs container, before requesting information about the host
authority.

Move the addition of the Dogtag lightweight CAs container entry a
bit earlier in the upgrade procedure, ensuring restart.

Fixes: https://fedorahosted.org/freeipa/ticket/6011
---
 ipaserver/install/cainstance.py | 14 +++---
 ipaserver/install/server/upgrade.py |  2 +-
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index 
ef69c898bcd4f9d8d7e698b04117047a33c1e45f..28f8fe156ff828fdcfafdf602fa6675a4ee84fea
 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -1695,7 +1695,7 @@ def ensure_ldap_profiles_container():
 )
 
 def ensure_lightweight_cas_container():
-ensure_entry(
+return ensure_entry(
 DN(('ou', 'authorities'), ('ou', 'ca'), ('o', 'ipaca')),
 objectclass=['top', 'organizationalUnit'],
 ou=['authorities'],
@@ -1703,6 +1703,12 @@ def ensure_lightweight_cas_container():
 
 
 def ensure_entry(dn, **attrs):
+"""Ensure an entry exists.
+
+If an entry with the given DN already exists, return ``False``,
+otherwise add the entry and return ``True``.
+
+"""
 server_id = installutils.realm_to_serverid(api.env.realm)
 dogtag_uri = 'ldapi://%%2fvar%%2frun%%2fslapd-%s.socket' % server_id
 
@@ -1712,12 +1718,14 @@ def ensure_entry(dn, **attrs):
 
 try:
 conn.get_entry(dn)
+return False
 except errors.NotFound:
 # entry doesn't exist; add it
 entry = conn.make_entry(dn, **attrs)
 conn.add_entry(entry)
-
-conn.disconnect()
+return True
+finally:
+conn.disconnect()
 
 
 def configure_profiles_acl():
diff --git a/ipaserver/install/server/upgrade.py 
b/ipaserver/install/server/upgrade.py
index 
3955a8cb9faf8e5c3350fc3912ea9f05a4b97719..43427178b11f63797a9537eadee836d7cf224311
 100644
--- a/ipaserver/install/server/upgrade.py
+++ b/ipaserver/install/server/upgrade.py
@@ -1747,6 +1747,7 @@ def upgrade_configuration():
 ca_enable_pkix(ca),
 ca_configure_profiles_acl(ca),
 ca_configure_lightweight_ca_acls(ca),
+ca_ensure_lightweight_cas_container(ca),
 ca_add_default_ocsp_uri(ca),
 ])
 
@@ -1758,7 +1759,6 @@ def upgrade_configuration():
 except ipautil.CalledProcessError as e:
 root_logger.error("Failed to restart %s: %s", ca.service_name, e)
 
-ca_ensure_lightweight_cas_container(ca)
 ca_enable_ldap_profile_subsystem(ca)
 
 # This step MUST be done after ca_enable_ldap_profile_subsystem and
-- 
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 0018][Tests] Fix some of the failing tests in test_ipalib/test_frontend.py

[Martin Basti]

On 29.06.2016 17:51, Ganna Kaihorodova wrote:

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



Pushed to master: 35d3a58421bc96b2a3c0352cb7d5976042f9cc03

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

[Martin Basti]

On 30.06.2016 12:58, Lenka Doudova wrote:



On 06/30/2016 12:51 PM, Petr Spacek wrote:

On 30.6.2016 12:32, Lenka Doudova wrote:


On 06/29/2016 07:49 PM, Petr Spacek wrote:

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


Ah, sorry for doing the wrong reply.
Anyway, fixed patch attached. I decided to do as you suggested - the 
original
DNS configuring function is now empty, I modified the comment to 
explain
significance of this strange thing. I also changed patch title to 
better

reflect proposed changes.

ACK if it passes your tests.


Yes, I've had no problems running the tests since I started to use this.
Thanks.


Pushed to master: 1d9e1521c59a5b43c2322892ce5cbe8cceff2790

--
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 0545] cert.py: split doctring to multiple ugettext strings

[Martin Basti]

On 30.06.2016 10:13, Stanislav Laznicka wrote:

On 06/30/2016 09:18 AM, Martin Basti wrote:
Make life of translators easier, there was recent change in cert.py 
docstring, so they have to translate the whole docstring again, so 
I'm splitting it to multiple parts.



Patch attached

I'm not sure whether the "See RFC 5280 for more details" should be 
split from the link.

It doesn't matter
Also, you need to add "\n" at the end of each reason string (you're 
not using multiline strings there).

Fixed, patch attached
From 40b5c73a1528fedeacc0a38bab0417c9f91966a5 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Wed, 29 Jun 2016 17:49:47 +0200
Subject: [PATCH] cert.py split module docstring to multiple ugetext string

It is hard to translate whole dosctring again and again aftear each
minor change. This split will make life for translators easier. (Just note: dosctring was
changed and that is the reason why I'm sending this, because translators
must translate it again anyway)
---
 ipaserver/plugins/cert.py | 74 +++
 1 file changed, 37 insertions(+), 37 deletions(-)

diff --git a/ipaserver/plugins/cert.py b/ipaserver/plugins/cert.py
index 888621fc5af634b95addc9c0ade58c76ce42edfe..63351c54c134f2bd58e8eb18bb0dc3bc6a5734b3 100644
--- a/ipaserver/plugins/cert.py
+++ b/ipaserver/plugins/cert.py
@@ -56,89 +56,89 @@ if six.PY3:
 
 __doc__ = _("""
 IPA certificate operations
-
+""") + _("""
 Implements a set of commands for managing server SSL certificates.
-
+""") + _("""
 Certificate requests exist in the form of a Certificate Signing Request (CSR)
 in PEM format.
-
+""") + _("""
 The dogtag CA uses just the CN value of the CSR and forces the rest of the
 subject to values configured in the server.
-
+""") + _("""
 A certificate is stored with a service principal and a service principal
 needs a host.
-
+""") + _("""
 In order to request a certificate:
-
+""") + _("""
 * The host must exist
 * The service must exist (or you use the --add option to automatically add it)
-
+""") + _("""
 SEARCHING:
-
+""") + _("""
 Certificates may be searched on by certificate subject, serial number,
 revocation reason, validity dates and the issued date.
-
+""") + _("""
 When searching on dates the _from date does a >= search and the _to date
 does a <= search. When combined these are done as an AND.
-
+""") + _("""
 Dates are treated as GMT to match the dates in the certificates.
-
+""") + _("""
 The date format is -mm-dd.
-
+""") + _("""
 EXAMPLES:
-
+""") + _("""
  Request a new certificate and add the principal:
ipa cert-request --add --principal=HTTP/lion.example.com example.csr
-
+""") + _("""
  Retrieve an existing certificate:
ipa cert-show 1032
-
+""") + _("""
  Revoke a certificate (see RFC 5280 for reason details):
ipa cert-revoke --revocation-reason=6 1032
-
+""") + _("""
  Remove a certificate from revocation hold status:
ipa cert-remove-hold 1032
-
+""") + _("""
  Check the status of a signing request:
ipa cert-status 10
-
+""") + _("""
  Search for certificates by hostname:
ipa cert-find --subject=ipaserver.example.com
-
+""") + _("""
  Search for revoked certificates by reason:
ipa cert-find --revocation-reason=5
-
+""") + _("""
  Search for certificates based on issuance date
ipa cert-find --issuedon-from=2013-02-01 --issuedon-to=2013-02-07
-
+""") + _("""
  Search for certificates owned by a specific user:
ipa cert-find --user=user
-
+""") + _("""
  Examine a certificate:
ipa cert-find --file=cert.pem --all
-
+""") + _("""
  Verify that a certificate is owner by a specific user:
ipa cert-find --file=cert.pem --user=user
-
+""") + _("""
 IPA currently immediately issues (or declines) all certificate requests so
 the status of a request is not normally useful. This is for future use
 or the case where a CA does not immediately issue a certificate.
-
+""") + _("""
 The following revocation reasons are supported:
 
-* 0 - unspecified
-* 1 - keyCompromise
-* 2 - cACompromise
-* 3 - affiliationChanged
-* 4 - superseded
-* 5 - cessationOfOperation
-* 6 - certificateHold
-* 8 - removeFromCRL
-* 9 - privilegeWithdrawn
-* 10 - aACompromise
-
+""") + _("""* 0 - unspecified
+""") + _("""* 1 - keyCompromise
+""") + _("""* 2 - cACompromise
+""") + _("""* 3 - affiliationChanged
+""") + _("""* 4 - superseded
+""") + _("""* 5 - cessationOfOperation
+""") + _("""* 6 - certificateHold
+""") + _("""* 8 - removeFromCRL
+""") + _("""* 9 - privilegeWithdrawn
+""") + _("""* 10 - aACompromise
+""") + _("""
 Note that reason code 7 is not used.  See RFC 5280 for more details:
-
+""") + _("""
 http://www.ietf.org/rfc/rfc5280.txt
 
 """)
-- 
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 0548] Fix replica install with CA

[Martin Basti]

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

This only for master branch, ipa-4-3 fix will be different (soon)

Patch attached

From 1324ea9584aaefc8943bed87460166c68c3bd2c1 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Wed, 29 Jun 2016 19:49:43 +0200
Subject: [PATCH] Fix replica install with CA

The incorrect api was used, and CA record updated was duplicated.

https://fedorahosted.org/freeipa/ticket/5966
---
 install/tools/ipa-ca-install|  7 ++-
 ipaserver/install/cainstance.py | 10 --
 2 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/install/tools/ipa-ca-install b/install/tools/ipa-ca-install
index 1bc5def03bf687a1e4f9fb38a54363b5429c8fc4..ed685920cbadb9cd3fc80865afb1610ca42f8b13 100755
--- a/install/tools/ipa-ca-install
+++ b/install/tools/ipa-ca-install
@@ -28,7 +28,7 @@ from ipaserver.install import installutils
 from ipaserver.install import certs
 from ipaserver.install.installutils import create_replica_config
 from ipaserver.install.installutils import check_creds, ReplicaConfig
-from ipaserver.install import dsinstance, ca
+from ipaserver.install import bindinstance, dsinstance, ca
 from ipaserver.install import cainstance, custodiainstance, service
 from ipapython import version
 from ipalib import api
@@ -195,6 +195,11 @@ def install_replica(safe_options, options, filename):
 CA.configure_replica(config.ca_host_name,
  subject_base=config.subject_base,
  ca_cert_bundle=ca_data)
+# Install CA DNS records
+if bindinstance.dns_container_exists(api.env.host, api.env.basedn,
+ ldapi=True, realm=api.env.realm):
+bind = bindinstance.BindInstance(ldapi=True)
+bind.update_system_records()
 else:
 ca.install(True, config, options)
 
diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index ef69c898bcd4f9d8d7e698b04117047a33c1e45f..18e3902a52b2f693bda01d67e8e514f284b1a695 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -63,7 +63,6 @@ from ipapython.ipa_log_manager import log_mgr,\
 from ipapython.secrets.kem import IPAKEMKeys
 
 from ipaserver.install import certs
-from ipaserver.install import bindinstance
 from ipaserver.install import dsinstance
 from ipaserver.install import installutils
 from ipaserver.install import ldapupdate
@@ -1298,14 +1297,6 @@ class CAInstance(DogtagInstance):
 basedn = ipautil.realm_to_suffix(self.realm)
 self.ldap_enable('CA', self.fqdn, None, basedn)
 
-def __update_ca_records(self):
-# Install CA DNS records
-if bindinstance.dns_container_exists(
-api.env.host, api.env.basedn, ldapi=True, realm=api.env.realm
-):
-bind = bindinstance.BindInstance(ldapi=True)
-bind.update_system_records()
-
 def configure_replica(self, master_host, subject_base=None,
   ca_cert_bundle=None, ca_signing_algorithm=None,
   ca_type=None):
@@ -1376,7 +1367,6 @@ class CAInstance(DogtagInstance):
   self.__restart_http_instance)
 
 self.step("enabling CA instance", self.__enable_instance)
-self.step("Updating DNS CA records", self.__update_ca_records)
 
 self.start_creation(runtime=210)
 
-- 
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 0022][Tests] Prevent trust test failures cause by adding duplicate DNS forward zone

[Lenka Doudova]

On 06/30/2016 12:51 PM, Petr Spacek wrote:

On 30.6.2016 12:32, Lenka Doudova wrote:


On 06/29/2016 07:49 PM, Petr Spacek wrote:

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


Ah, sorry for doing the wrong reply.
Anyway, fixed patch attached. I decided to do as you suggested - the original
DNS configuring function is now empty, I modified the comment to explain
significance of this strange thing. I also changed patch title to better
reflect proposed changes.

ACK if it passes your tests.


Yes, I've had no problems running the tests since I started to use this.
Thanks.

--
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 30.6.2016 12:32, Lenka Doudova wrote:
> 
> 
> On 06/29/2016 07:49 PM, Petr Spacek wrote:
>> 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.)
>>
> Ah, sorry for doing the wrong reply.
> Anyway, fixed patch attached. I decided to do as you suggested - the original
> DNS configuring function is now empty, I modified the comment to explain
> significance of this strange thing. I also changed patch title to better
> reflect proposed changes.

ACK if it passes your tests.

-- 
Petr^2 Spacek

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to 

Re: [Freeipa-devel] [PATCH 0022][Tests] Prevent trust test failures cause by adding duplicate DNS forward zone

[Oleg Fayans]

On 06/30/2016 12:41 PM, Lenka Doudova wrote:
> 
> 
> On 06/30/2016 12:32 PM, Lenka Doudova wrote:
>>
>>
>> On 06/29/2016 07:49 PM, Petr Spacek wrote:
>>> 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.)
>>>
>> Ah, sorry for doing the wrong reply.
>> Anyway, fixed patch attached. I decided to do as you suggested - the
>> original DNS configuring function is now empty, I modified the 

Re: [Freeipa-devel] [PATCH 0022][Tests] Prevent trust test failures cause by adding duplicate DNS forward zone

[Lenka Doudova]

On 06/30/2016 12:32 PM, Lenka Doudova wrote:



On 06/29/2016 07:49 PM, Petr Spacek wrote:

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


Ah, sorry for doing the wrong reply.
Anyway, fixed patch attached. I decided to do as you suggested - the 
original DNS configuring function is now empty, I modified the comment 
to explain significance of this strange thing. I also changed patch 
title to better reflect proposed changes.


Lenka



NACK the previous one, forgot PEP8. New patch attached.

Lenka
From a4772a1b8cc31f0020c86acabbeb944c6d5269b7 Mon Sep 17 00:00:00 2001
From: Lenka Doudova 
Date: Thu, 30 Jun 2016 12:26:28 +0200
Subject: [PATCH] Tests: Remove DNS configuration from trust tests

Since DNS configuration is no longer needed for running trust tests, this method's contents are removed. Method is left empty as reference for others, should they have issues with DNS configuration.
---
 ipatests/test_integration/tasks.py | 44 --
 1 file changed, 4 insertions(+), 40 deletions(-)

diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py
index 38218fa709c2c220d5fea98a092b55e995d48d77..5be7cdae3ac777bbf0fc52e6c511969e9fabcd72 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -436,47 +436,11 @@ def install_adtrust(host):
 
 def configure_dns_for_trust(master, ad):
 """
-This configures DNS on IPA master according to the relationship of the
-  

Re: [Freeipa-devel] [PATCH 0022][Tests] Prevent trust test failures cause by adding duplicate DNS forward zone

[Lenka Doudova]

On 06/29/2016 07:49 PM, Petr Spacek wrote:

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


Ah, sorry for doing the wrong reply.
Anyway, fixed patch attached. I decided to do as you suggested - the 
original DNS configuring function is now empty, I modified the comment 
to explain significance of this strange thing. I also changed patch 
title to better reflect proposed changes.


Lenka
From c931a57aa4a0162b7f28d2fe0e01d43c8c95c146 Mon Sep 17 00:00:00 2001
From: Lenka Doudova 
Date: Thu, 30 Jun 2016 12:26:28 +0200
Subject: [PATCH] Tests: Remove DNS configuration from trust tests

Since DNS configuration is no longer needed for running trust tests, this method's contents are removed. Method is left empty as reference for others, should they have issues with DNS configuration.
---
 ipatests/test_integration/tasks.py | 44 --
 1 file changed, 4 insertions(+), 40 deletions(-)

diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py
index 38218fa709c2c220d5fea98a092b55e995d48d77..1aa7772a1b59b4ad78bd3b3f653c8782ae6041de 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -436,47 +436,11 @@ def install_adtrust(host):
 
 def configure_dns_for_trust(master, ad):
 """
-This configures DNS on IPA master according to the relationship of the
-IPA's and AD's domains.
+This method is intentionally left empty. Originally it served for DNS 
+configuration on IPA master according to the 

Re: [Freeipa-devel] [PATCH 0546-0547] use timestamps for ipareplica-conncheck.log

[Petr Spacek]

On 30.6.2016 10:21, Jan Cholasta wrote:
> On 30.6.2016 10:12, Petr Spacek wrote:
>> On 30.6.2016 10:14, Jan Cholasta wrote:
>>> On 30.6.2016 10:06, Petr Spacek wrote:
 On 30.6.2016 10:02, Jan Cholasta wrote:
> On 30.6.2016 09:56, Petr Spacek wrote:
>> On 30.6.2016 09:40, Martin Basti wrote:
>>> https://fedorahosted.org/freeipa/ticket/5757
>
> "The easiest solution would be to add timestamps to logs, or log to
> different
> logs from oddjob or from installer (ipareplica-conncheck.local.log and
> ipareplica-conncheck.remote.log)"
>
> Actually the easiest solution would be not to log into a file when 
> executed
> from oddjob.

 Well, IPA is hard enough to debug even with logs. I would not make 
 situation
 even worse by not logging at all :-)
>>>
>>> The commands logs into stderr, and both stdout and stderr are sent back to 
>>> the
>>> caller of the oddjob.
>>>
>>> Alternatively, it could log into a different file (say
>>> /var/log/ipareplica-conncheck-oddjob.log). IMO timestamps are an overkill to
>>> fix this bug.
>>
>> When we are at it, a custom logger is overkill. IMHO we should log everything
>> to journal and be done with it ...
> 
> It's not, we want to log to at least stderr ourselves.
> 
> Also, it would be even harder to implement than timestamps, and time is a

Sure, we do not have time:
=> ACK for current version of the patch.

Petr^2 Spacek

> factor here. It would fit more into
> .
> 
>>
>> Petr^2 Spacek
>>
>>> Patches attached.
>>
>> I would rather use timestamp format with dashes between numbers to make 
>> it
>> easier to read and parse for humans.
>>
>> Compare:
>>
>> 201606270954
>> 201606290954
>> 201606300954
>>
>> with
>>
>> 2016-06-27-09-54
>> 2016-06-29-09-54
>> 2016-06-30-09-54
>>
>>
> 
> 


-- 
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 660] replica install: don't allow install against a newer server

[Jan Cholasta]

On 29.6.2016 17:46, Petr Spacek wrote:

On 29.6.2016 14:25, Jan Cholasta wrote:

Hi,

the attached patch fixes .


ACK


Thanks.

Pushed to master: 99339bf7892fcc1201e06e6a8105b0bb4681c4f4

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

[Fraser Tweedale]

On Thu, Jun 30, 2016 at 11:38:35AM +0200, Florence Blanc-Renaud wrote:
> On 06/30/2016 06:29 AM, Fraser Tweedale wrote:
> > On Wed, Jun 29, 2016 at 11:30:14AM +0200, Florence Blanc-Renaud wrote:
> > > 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
> > > 
> > Thanks for review, Flo.  Updated patch attached.
> > 
> > Cheers,
> > Fraser
> > 
> Hi Fraser,
> 
> thanks for updated patch. There is still a pep8 error:
> ./ipalib/errors.py:1399:1: E302 expected 2 blank lines, found 1
> 
Whups!

> I am also wondering if the message is clear enough. When running the CLI
> it's ok because the user typed --add, but the GUI displays "'add' is not
> supported for user principals"
> and I feel
> "'add' option is not supported for user principals"
> would be more user-friendly. What do you think?
> 
Yes, I concur that mentioning "option" is an improvement.  Will cut
a new patch shortly

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

[Florence Blanc-Renaud]

On 06/30/2016 06:29 AM, Fraser Tweedale wrote:

On Wed, Jun 29, 2016 at 11:30:14AM +0200, Florence Blanc-Renaud wrote:

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


Thanks for review, Flo.  Updated patch attached.

Cheers,
Fraser


Hi Fraser,

thanks for updated patch. There is still a pep8 error:
./ipalib/errors.py:1399:1: E302 expected 2 blank lines, found 1

I am also wondering if the message is clear enough. When running the CLI 
it's ok because the user typed --add, but the GUI displays "'add' is not 
supported for user principals"

and I feel
"'add' option is not supported for user principals"
would be more user-friendly. What do you think?

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 0546-0547] use timestamps for ipareplica-conncheck.log

[Jan Cholasta]

On 30.6.2016 11:07, Martin Basti wrote:



On 30.06.2016 10:21, Jan Cholasta wrote:

On 30.6.2016 10:12, Petr Spacek wrote:

On 30.6.2016 10:14, Jan Cholasta wrote:

On 30.6.2016 10:06, Petr Spacek wrote:

On 30.6.2016 10:02, Jan Cholasta wrote:

On 30.6.2016 09:56, Petr Spacek wrote:

On 30.6.2016 09:40, Martin Basti wrote:

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


"The easiest solution would be to add timestamps to logs, or log
to different
logs from oddjob or from installer (ipareplica-conncheck.local.log
and
ipareplica-conncheck.remote.log)"

Actually the easiest solution would be not to log into a file when
executed
from oddjob.


Well, IPA is hard enough to debug even with logs. I would not make
situation
even worse by not logging at all :-)


The commands logs into stderr, and both stdout and stderr are sent
back to the
caller of the oddjob.

Alternatively, it could log into a different file (say
/var/log/ipareplica-conncheck-oddjob.log). IMO timestamps are an
overkill to
fix this bug.


When we are at it, a custom logger is overkill. IMHO we should log
everything
to journal and be done with it ...


It's not, we want to log to at least stderr ourselves.

Also, it would be even harder to implement than timestamps, and time
is a factor here. It would fit more into
.


I can add option --no-log to ipa-replica-conncheck


Works for me, but don't forget to update 
install/oddjob/org.freeipa.server.conncheck.






Petr^2 Spacek


Patches attached.


I would rather use timestamp format with dashes between numbers
to make it
easier to read and parse for humans.

Compare:

201606270954
201606290954
201606300954

with

2016-06-27-09-54
2016-06-29-09-54
2016-06-30-09-54











--
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 0546-0547] use timestamps for ipareplica-conncheck.log

[Martin Basti]

On 30.06.2016 10:21, Jan Cholasta wrote:

On 30.6.2016 10:12, Petr Spacek wrote:

On 30.6.2016 10:14, Jan Cholasta wrote:

On 30.6.2016 10:06, Petr Spacek wrote:

On 30.6.2016 10:02, Jan Cholasta wrote:

On 30.6.2016 09:56, Petr Spacek wrote:

On 30.6.2016 09:40, Martin Basti wrote:

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


"The easiest solution would be to add timestamps to logs, or log 
to different
logs from oddjob or from installer (ipareplica-conncheck.local.log 
and

ipareplica-conncheck.remote.log)"

Actually the easiest solution would be not to log into a file when 
executed

from oddjob.


Well, IPA is hard enough to debug even with logs. I would not make 
situation

even worse by not logging at all :-)


The commands logs into stderr, and both stdout and stderr are sent 
back to the

caller of the oddjob.

Alternatively, it could log into a different file (say
/var/log/ipareplica-conncheck-oddjob.log). IMO timestamps are an 
overkill to

fix this bug.


When we are at it, a custom logger is overkill. IMHO we should log 
everything

to journal and be done with it ...


It's not, we want to log to at least stderr ourselves.

Also, it would be even harder to implement than timestamps, and time 
is a factor here. It would fit more into 
.



I can add option --no-log to ipa-replica-conncheck



Petr^2 Spacek


Patches attached.


I would rather use timestamp format with dashes between numbers 
to make it

easier to read and parse for humans.

Compare:

201606270954
201606290954
201606300954

with

2016-06-27-09-54
2016-06-29-09-54
2016-06-30-09-54








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

[Jan Cholasta]

On 29.6.2016 19:06, Milan Kubík wrote:

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

Hi,

the attached patch fixes .

Honza





The restore works with the patch. ACK.


Thanks.

Pushed to master: ce93b091d2ffbafedc721551f437631eed0e5a86

--
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 30.6.2016 10:51, David Kupka wrote:

On 30/06/16 10:45, Jan Cholasta wrote:

On 30.6.2016 10:32, Jan Cholasta wrote:

On 29.6.2016 10:21, Jan Cholasta wrote:

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.


Martin found a regression caused by this patch. The attached patch
should fix it.


Self-NACK, correct patch attached.


Works for me, ACK.


Thanks.

Pushed to master: 8d5272e68775046db450d860c173a4d531a95ff2

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

[David Kupka]

On 30/06/16 10:45, Jan Cholasta wrote:

On 30.6.2016 10:32, Jan Cholasta wrote:

On 29.6.2016 10:21, Jan Cholasta wrote:

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.


Martin found a regression caused by this patch. The attached patch
should fix it.


Self-NACK, correct patch attached.


Works for me, 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] [WIP] Thin client

[Jan Cholasta]

On 30.6.2016 10:32, Jan Cholasta wrote:

On 29.6.2016 10:21, Jan Cholasta wrote:

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.


Martin found a regression caused by this patch. The attached patch
should fix it.


Self-NACK, correct patch attached.

--
Jan Cholasta
From 195dd2f8acf9e7461e25b505a2a6a7d2067765ed Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Thu, 30 Jun 2016 10:27:05 +0200
Subject: [PATCH] schema: properly fix Flag arguments on the client

The previous fix in commit a77e21cbca05be422fe5826857cfba7e0ba6e71f made
some Bool arguments appear as Flag on the client. This change fixes that.

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

diff --git a/ipaclient/remote_plugins/schema.py b/ipaclient/remote_plugins/schema.py
index 7d5c8d0..da917a9 100644
--- a/ipaclient/remote_plugins/schema.py
+++ b/ipaclient/remote_plugins/schema.py
@@ -219,8 +219,8 @@ class _SchemaPlugin(object):
 cls = Password
 sensitive = False
 elif (type_name == 'bool' and
-'default' in schema and
-schema['default'][0] == u'False'):
+'default' in schema and schema['default'][0] == u'False' and
+not schema.get('alwaysask', 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] 0064: webui: simplify confirmation messages in confirmation dialogs

[Pavel Vomacka]

On 06/29/2016 04:40 PM, Petr Vobornik wrote:

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.


Changed. Updated patch attached.

--
Pavel^3 Vomacka

From 66ae0b053fd1a7136c11dacaf26f218e5f68af54 Mon Sep 17 00:00:00 2001
From: Pavel Vomacka 
Date: Mon, 27 Jun 2016 17:35:31 +0200
Subject: [PATCH] Simplify the confirmation messages

The confirmation of revoke and remove the certificate hold action is simplier
and more consistent with another parts of WebUI.

Part of: https://fedorahosted.org/freeipa/ticket/5381
---
 install/ui/test/data/ipa_init.json | 4 ++--
 ipaserver/plugins/internal.py  | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/install/ui/test/data/ipa_init.json b/install/ui/test/data/ipa_init.json
index 8060d69933cd96f4cb56f8a99b3d371358867cc4..b05504a54a6aca7d5241929dfb7348c648d2ff60 100644
--- a/install/ui/test/data/ipa_init.json
+++ b/install/ui/test/data/ipa_init.json
@@ -280,7 +280,7 @@
 "remove_hold": "Remove Hold",
 "remove_certificate_hold": "Remove Certificate Hold for ${entity} ${primary_key}",
 "remove_certificate_hold_simple": "Remove Certificate Hold",
-"remove_certificate_hold_confirmation": "To confirm your intention to remove the certificate hold, click the \"Remove hold\" button.",
+"remove_certificate_hold_confirmation": "Do you want to remove the certificate hold?",
 "remove_from_crl": "Remove from CRL",
 "request_message": " Create a certificate database or use an existing one. To create a new database: # certutil -N -d database path  Create a CSR with subject CN=${cn_name},O=realm, for example: # certutil -R -d database path -a -g key size -s 'CN=${cn},O=${realm}'${san}   Copy and paste the CSR (from -BEGIN NEW CERTIFICATE REQUEST- to -END NEW CERTIFICATE REQUEST-) into the text area below:  ",
 "request_message_san": " -8 '${cn}'",
@@ -288,7 +288,7 @@
 "revocation_reason": "Revocation reason",
 "revoke_certificate": "Revoke Certificate for ${entity} ${primary_key}",
 "revoke_certificate_simple": "Revoke Certificate",
-"revoke_confirmation": "To confirm your intention to revoke this certificate, select a reason from the pull-down list, and click the \"Revoke\" button.",
+"revoke_confirmation": "Do you want to revoke this certificate? Select a reason from the pull-down list.",
 "revoked": "Certificate Revoked",
 "revoked_status": "REVOKED",
 "serial_number": "Serial Number",
diff --git a/ipaserver/plugins/internal.py b/ipaserver/plugins/internal.py
index 204c153a8a573e2e2057f8b49b3dac714b6bfcd3..514d48cea3a7e05bdf3196290721d57737920c20 100644
--- a/ipaserver/plugins/internal.py
+++ b/ipaserver/plugins/internal.py
@@ -423,7 +423,7 @@ class i18n_messages(Command):
 "remove_hold": _("Remove Hold"),
 "remove_certificate_hold": _("Remove Certificate Hold for ${entity} ${primary_key}"),
 "remove_certificate_hold_simple": _("Remove Certificate Hold"),
-"remove_certificate_hold_confirmation": _("To confirm your intention to remove the certificate hold, click the \"Remove hold\" button."),
+"remove_certificate_hold_confirmation": _("Do you want to remove the certificate hold?"),
 "remove_from_crl": _("Remove from CRL"),
 "request_message": _(" Create a certificate database or use an existing one. To create a new database: # certutil -N -d database path  Create a CSR with subject CN=${cn_name},O=realm, for example: # certutil -R -d database path -a -g key size -s 'CN=${cn},O=${realm}'${san}   Copy and paste the CSR (from -BEGIN NEW CERTIFICATE REQUEST- to -END NEW CERTIFICATE REQUEST-) into the text area below:  "),
 "request_message_san": _(" -8 '${cn}'"),
@@ -431,7 +431,7 @@ class i18n_messages(Command):
 "revocation_reason": _("Revocation reason"),
 "revoke_certificate": _("Revoke Certificate for ${entity} ${primary_key}"),
 "revoke_certificate_simple": _("Revoke Certificate"),
-"revoke_confirmation": _("To confirm your intention to revoke this certificate, select a reason from the pull-down list, and click the \"Revoke\" button."),
+"revoke_confirmation": _("Do you want to revoke this certificate? Select a reason from the 

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

[Jan Cholasta]

On 29.6.2016 10:21, Jan Cholasta wrote:

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.


Martin found a regression caused by this patch. The attached patch 
should fix it.


--
Jan Cholasta
From 61028180135f1f5967778fba6ecabd28f8b2da7d Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Thu, 30 Jun 2016 10:27:05 +0200
Subject: [PATCH] schema: properly fix Flag arguments on the client

The previous fix in commit a77e21cbca05be422fe5826857cfba7e0ba6e71f made
some Bool arguments appear as Flag on the client. This change fixes that.

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

diff --git a/ipaclient/remote_plugins/schema.py b/ipaclient/remote_plugins/schema.py
index 7d5c8d0..08ed2b7 100644
--- a/ipaclient/remote_plugins/schema.py
+++ b/ipaclient/remote_plugins/schema.py
@@ -219,8 +219,8 @@ class _SchemaPlugin(object):
 cls = Password
 sensitive = False
 elif (type_name == 'bool' and
-'default' in schema and
-schema['default'][0] == u'False'):
+'default' in schema and schema['default'][0] == u'False' and
+schema.get('alwaysask', u'False') == 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

[Freeipa-devel] [PATCH] 0009 Do not log error when removing a non-existing file

[Florence Blanc-Renaud]

Hi,

this patch fixes issue 1) of the following ticket:
Uninstallation complains about missing 'ipa.conf'

Issue 2) is not reproducible on the master, and issue 3) is handled in a 
separate ticket.


https://fedorahosted.org/freeipa/ticket/6012
>From 01f8955634e8dbdc9a976ac72250aa38c1969b3c Mon Sep 17 00:00:00 2001
From: Florence Blanc-Renaud 
Date: Thu, 30 Jun 2016 09:15:45 +0200
Subject: [PATCH] Do not log error when removing a non-existing file

When the uninstaller tries to remove /etc/systemd/system/httpd.d/ipa.conf and
the file does not exist, only log to debug instead of error.

https://fedorahosted.org/freeipa/ticket/6012
---
 ipaplatform/redhat/tasks.py | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/ipaplatform/redhat/tasks.py b/ipaplatform/redhat/tasks.py
index 294a9fe1abe48423495cf6207288c78bbd00b02e..8ac88511e94d640f077c7a0e202bc545ec8bcbbe 100644
--- a/ipaplatform/redhat/tasks.py
+++ b/ipaplatform/redhat/tasks.py
@@ -29,6 +29,7 @@ import os
 import socket
 import base64
 import traceback
+import errno
 
 from cffi import FFI
 from ctypes.util import find_library
@@ -466,10 +467,16 @@ class RedHatTaskNamespace(BaseTaskNamespace):
 try:
 os.unlink(paths.SYSTEMD_SYSTEM_HTTPD_IPA_CONF)
 except OSError as e:
-root_logger.error(
-'Error removing %s: %s',
-paths.SYSTEMD_SYSTEM_HTTPD_IPA_CONF, e
-)
+if e.errno == errno.ENOENT:
+root_logger.debug(
+'Trying to remove %s but file does not exist',
+paths.SYSTEMD_SYSTEM_HTTPD_IPA_CONF
+)
+else:
+root_logger.error(
+'Error removing %s: %s',
+paths.SYSTEMD_SYSTEM_HTTPD_IPA_CONF, e
+)
 
 def set_hostname(self, hostname):
 ipautil.run([paths.BIN_HOSTNAMECTL, 'set-hostname', hostname])
-- 
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 0546-0547] use timestamps for ipareplica-conncheck.log

[Jan Cholasta]

On 30.6.2016 10:12, Petr Spacek wrote:

On 30.6.2016 10:14, Jan Cholasta wrote:

On 30.6.2016 10:06, Petr Spacek wrote:

On 30.6.2016 10:02, Jan Cholasta wrote:

On 30.6.2016 09:56, Petr Spacek wrote:

On 30.6.2016 09:40, Martin Basti wrote:

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


"The easiest solution would be to add timestamps to logs, or log to different
logs from oddjob or from installer (ipareplica-conncheck.local.log and
ipareplica-conncheck.remote.log)"

Actually the easiest solution would be not to log into a file when executed
from oddjob.


Well, IPA is hard enough to debug even with logs. I would not make situation
even worse by not logging at all :-)


The commands logs into stderr, and both stdout and stderr are sent back to the
caller of the oddjob.

Alternatively, it could log into a different file (say
/var/log/ipareplica-conncheck-oddjob.log). IMO timestamps are an overkill to
fix this bug.


When we are at it, a custom logger is overkill. IMHO we should log everything
to journal and be done with it ...


It's not, we want to log to at least stderr ourselves.

Also, it would be even harder to implement than timestamps, and time is 
a factor here. It would fit more into 
.




Petr^2 Spacek


Patches attached.


I would rather use timestamp format with dashes between numbers to make it
easier to read and parse for humans.

Compare:

201606270954
201606290954
201606300954

with

2016-06-27-09-54
2016-06-29-09-54
2016-06-30-09-54






--
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 0545] cert.py: split doctring to multiple ugettext strings

[Stanislav Laznicka]

On 06/30/2016 09:18 AM, Martin Basti wrote:
Make life of translators easier, there was recent change in cert.py 
docstring, so they have to translate the whole docstring again, so I'm 
splitting it to multiple parts.



Patch attached

I'm not sure whether the "See RFC 5280 for more details" should be split 
from the link. Also, you need to add "\n" at the end of each reason 
string (you're not using multiline strings there).


--
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 0546-0547] use timestamps for ipareplica-conncheck.log

[Petr Spacek]

On 30.6.2016 10:14, Jan Cholasta wrote:
> On 30.6.2016 10:06, Petr Spacek wrote:
>> On 30.6.2016 10:02, Jan Cholasta wrote:
>>> On 30.6.2016 09:56, Petr Spacek wrote:
 On 30.6.2016 09:40, Martin Basti wrote:
> https://fedorahosted.org/freeipa/ticket/5757
>>>
>>> "The easiest solution would be to add timestamps to logs, or log to 
>>> different
>>> logs from oddjob or from installer (ipareplica-conncheck.local.log and
>>> ipareplica-conncheck.remote.log)"
>>>
>>> Actually the easiest solution would be not to log into a file when executed
>>> from oddjob.
>>
>> Well, IPA is hard enough to debug even with logs. I would not make situation
>> even worse by not logging at all :-)
> 
> The commands logs into stderr, and both stdout and stderr are sent back to the
> caller of the oddjob.
> 
> Alternatively, it could log into a different file (say
> /var/log/ipareplica-conncheck-oddjob.log). IMO timestamps are an overkill to
> fix this bug.

When we are at it, a custom logger is overkill. IMHO we should log everything
to journal and be done with it ...

Petr^2 Spacek

> Patches attached.

 I would rather use timestamp format with dashes between numbers to make it
 easier to read and parse for humans.

 Compare:

 201606270954
 201606290954
 201606300954

 with

 2016-06-27-09-54
 2016-06-29-09-54
 2016-06-30-09-54


-- 
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 0546-0547] use timestamps for ipareplica-conncheck.log

[Jan Cholasta]

On 30.6.2016 10:06, Petr Spacek wrote:

On 30.6.2016 10:02, Jan Cholasta wrote:

On 30.6.2016 09:56, Petr Spacek wrote:

On 30.6.2016 09:40, Martin Basti wrote:

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


"The easiest solution would be to add timestamps to logs, or log to different
logs from oddjob or from installer (ipareplica-conncheck.local.log and
ipareplica-conncheck.remote.log)"

Actually the easiest solution would be not to log into a file when executed
from oddjob.


Well, IPA is hard enough to debug even with logs. I would not make situation
even worse by not logging at all :-)


The commands logs into stderr, and both stdout and stderr are sent back 
to the caller of the oddjob.


Alternatively, it could log into a different file (say 
/var/log/ipareplica-conncheck-oddjob.log). IMO timestamps are an 
overkill to fix this bug.





Patches attached.


I would rather use timestamp format with dashes between numbers to make it
easier to read and parse for humans.

Compare:

201606270954
201606290954
201606300954

with

2016-06-27-09-54
2016-06-29-09-54
2016-06-30-09-54



--
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 0546-0547] use timestamps for ipareplica-conncheck.log

[Petr Spacek]

On 30.6.2016 10:02, Jan Cholasta wrote:
> On 30.6.2016 09:56, Petr Spacek wrote:
>> On 30.6.2016 09:40, Martin Basti wrote:
>>> https://fedorahosted.org/freeipa/ticket/5757
> 
> "The easiest solution would be to add timestamps to logs, or log to different
> logs from oddjob or from installer (ipareplica-conncheck.local.log and
> ipareplica-conncheck.remote.log)"
> 
> Actually the easiest solution would be not to log into a file when executed
> from oddjob.

Well, IPA is hard enough to debug even with logs. I would not make situation
even worse by not logging at all :-)

>>> Patches attached.
>>
>> I would rather use timestamp format with dashes between numbers to make it
>> easier to read and parse for humans.
>>
>> Compare:
>>
>> 201606270954
>> 201606290954
>> 201606300954
>>
>> with
>>
>> 2016-06-27-09-54
>> 2016-06-29-09-54
>> 2016-06-30-09-54

-- 
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 0546-0547] use timestamps for ipareplica-conncheck.log

[Jan Cholasta]

On 30.6.2016 09:56, Petr Spacek wrote:

On 30.6.2016 09:40, Martin Basti wrote:

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


"The easiest solution would be to add timestamps to logs, or log to 
different logs from oddjob or from installer 
(ipareplica-conncheck.local.log and ipareplica-conncheck.remote.log)"


Actually the easiest solution would be not to log into a file when 
executed from oddjob.





Patches attached.


I would rather use timestamp format with dashes between numbers to make it
easier to read and parse for humans.

Compare:

201606270954
201606290954
201606300954

with

2016-06-27-09-54
2016-06-29-09-54
2016-06-30-09-54




--
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 0546-0547] use timestamps for ipareplica-conncheck.log

[Petr Spacek]

On 30.6.2016 09:40, Martin Basti wrote:
> https://fedorahosted.org/freeipa/ticket/5757
> 
> 
> Patches attached.

I would rather use timestamp format with dashes between numbers to make it
easier to read and parse for humans.

Compare:

201606270954
201606290954
201606300954

with

2016-06-27-09-54
2016-06-29-09-54
2016-06-30-09-54

-- 
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 0546-0547] use timestamps for ipareplica-conncheck.log

[Martin Basti]

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


Patches attached.

From a76d63aea17b3b429defd059a026b2764e92e5b7 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Wed, 29 Jun 2016 13:34:19 +0200
Subject: [PATCH 1/2] Enhance logger to allow logfiles with timestamps

Keyword '{timestamp}' in logfile will be automatically replaced with
date and time in format "%Y%m%d%H%M"

Required for: https://fedorahosted.org/freeipa/ticket/5757
---
 ipapython/ipa_log_manager.py | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/ipapython/ipa_log_manager.py b/ipapython/ipa_log_manager.py
index 8a555ebda7919214c20d40ec45b9b5f7863f0298..4471b7f13ab83726f7f927dc82d77c5dcd596694 100644
--- a/ipapython/ipa_log_manager.py
+++ b/ipapython/ipa_log_manager.py
@@ -29,6 +29,8 @@ import sys
 import re
 import copy
 
+from datetime import datetime
+
 from ipapython.log_manager import LogManager, parse_log_level
 
 #---
@@ -62,6 +64,8 @@ LOGGING_FORMAT_STANDARD_CONSOLE = '%(name)-12s: %(levelname)-8s %(message)s'
 # Used by standard_logging_setup() for file message
 LOGGING_FORMAT_STANDARD_FILE = '%(asctime)s %(levelname)s %(message)s'
 
+FILENAME_TIMESTAMP_FORMAT = '%Y%m%d%H%M'
+
 #---
 
 class IPALogManager(LogManager):
@@ -177,6 +181,8 @@ def standard_logging_setup(filename=None, verbose=False, debug=False,
 
 # File output is always logged at debug level
 if filename is not None:
+filename = filename.format(
+timestamp = datetime.now().strftime(FILENAME_TIMESTAMP_FORMAT))
 file_handler = dict(name='file',
 filename=filename,
 filemode=filemode,
-- 
2.5.5

From 678ffbdbda2b776c6f8edcc513b6fd3674067fc3 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Wed, 29 Jun 2016 14:28:33 +0200
Subject: [PATCH 2/2] Use timestamps for ipa-replica-conncheck with timestamps

ipareplica-conncheck log file is used under different SELinux context
depending on if it called locally or remotely via oddjob. We need
to separate logfiles with different contexts and the best way according
the future plans is to use timestamps for logfiles.

https://fedorahosted.org/freeipa/ticket/5757
---
 ipaplatform/base/paths.py  | 2 +-
 ipatests/test_integration/tasks.py | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/ipaplatform/base/paths.py b/ipaplatform/base/paths.py
index d6fbe32f6839a5db40148777132ba1454cbc3382..2b76faf487cf39f641adad1b7ca8ea0119d73eef 100644
--- a/ipaplatform/base/paths.py
+++ b/ipaplatform/base/paths.py
@@ -304,7 +304,7 @@ class BasePathNamespace(object):
 IPACLIENT_INSTALL_LOG = "/var/log/ipaclient-install.log"
 IPACLIENT_UNINSTALL_LOG = "/var/log/ipaclient-uninstall.log"
 IPAREPLICA_CA_INSTALL_LOG = "/var/log/ipareplica-ca-install.log"
-IPAREPLICA_CONNCHECK_LOG = "/var/log/ipareplica-conncheck.log"
+IPAREPLICA_CONNCHECK_LOG = "/var/log/ipareplica-conncheck-{timestamp}.log"
 IPAREPLICA_INSTALL_LOG = "/var/log/ipareplica-install.log"
 IPARESTORE_LOG = "/var/log/iparestore.log"
 IPASERVER_CA_INSTALL_LOG = "/var/log/ipaserver-ca-install.log"
diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py
index 38218fa709c2c220d5fea98a092b55e995d48d77..373fb637aec051e9086f11e9ec51dcf4bab13e4d 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -322,7 +322,7 @@ def install_replica(master, replica, setup_ca=True, setup_dns=False,
 domain_level = domainlevel(master)
 apply_common_fixes(replica)
 replica.collect_log(paths.IPAREPLICA_INSTALL_LOG)
-replica.collect_log(paths.IPAREPLICA_CONNCHECK_LOG)
+replica.collect_log(paths.IPAREPLICA_CONNCHECK_LOG.format(timestamp='*'))
 allow_sync_ptr(master)
 # Otherwise ipa-client-install would not create a PTR
 # and replica installation would fail
-- 
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 0545] cert.py: split doctring to multiple ugettext strings

[Martin Basti]

Make life of translators easier, there was recent change in cert.py 
docstring, so they have to translate the whole docstring again, so I'm 
splitting it to multiple parts.



Patch attached

From c2b71ea436cef0901493a742d15fa762f37677ed Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Wed, 29 Jun 2016 17:49:47 +0200
Subject: [PATCH] cert.py split module docstring to multiple ugetext string

It is hard to translate whole dosctring again and again aftear each
minor change. This split will make life for translators easier. (Just note: dosctring was
changed and that is the reason why I'm sending this, because translators
must translate it again anyway)
---
 ipaserver/plugins/cert.py | 76 +++
 1 file changed, 38 insertions(+), 38 deletions(-)

diff --git a/ipaserver/plugins/cert.py b/ipaserver/plugins/cert.py
index 888621fc5af634b95addc9c0ade58c76ce42edfe..a7e31a5c4c4504752ffd963b5a772ac6cfb7c302 100644
--- a/ipaserver/plugins/cert.py
+++ b/ipaserver/plugins/cert.py
@@ -56,89 +56,89 @@ if six.PY3:
 
 __doc__ = _("""
 IPA certificate operations
-
+""") + _("""
 Implements a set of commands for managing server SSL certificates.
-
+""") + _("""
 Certificate requests exist in the form of a Certificate Signing Request (CSR)
 in PEM format.
-
+""") + _("""
 The dogtag CA uses just the CN value of the CSR and forces the rest of the
 subject to values configured in the server.
-
+""") + _("""
 A certificate is stored with a service principal and a service principal
 needs a host.
-
+""") + _("""
 In order to request a certificate:
-
+""") + _("""
 * The host must exist
 * The service must exist (or you use the --add option to automatically add it)
-
+""") + _("""
 SEARCHING:
-
+""") + _("""
 Certificates may be searched on by certificate subject, serial number,
 revocation reason, validity dates and the issued date.
-
+""") + _("""
 When searching on dates the _from date does a >= search and the _to date
 does a <= search. When combined these are done as an AND.
-
+""") + _("""
 Dates are treated as GMT to match the dates in the certificates.
-
+""") + _("""
 The date format is -mm-dd.
-
+""") + _("""
 EXAMPLES:
-
+""") + _("""
  Request a new certificate and add the principal:
ipa cert-request --add --principal=HTTP/lion.example.com example.csr
-
+""") + _("""
  Retrieve an existing certificate:
ipa cert-show 1032
-
+""") + _("""
  Revoke a certificate (see RFC 5280 for reason details):
ipa cert-revoke --revocation-reason=6 1032
-
+""") + _("""
  Remove a certificate from revocation hold status:
ipa cert-remove-hold 1032
-
+""") + _("""
  Check the status of a signing request:
ipa cert-status 10
-
+""") + _("""
  Search for certificates by hostname:
ipa cert-find --subject=ipaserver.example.com
-
+""") + _("""
  Search for revoked certificates by reason:
ipa cert-find --revocation-reason=5
-
+""") + _("""
  Search for certificates based on issuance date
ipa cert-find --issuedon-from=2013-02-01 --issuedon-to=2013-02-07
-
+""") + _("""
  Search for certificates owned by a specific user:
ipa cert-find --user=user
-
+""") + _("""
  Examine a certificate:
ipa cert-find --file=cert.pem --all
-
+""") + _("""
  Verify that a certificate is owner by a specific user:
ipa cert-find --file=cert.pem --user=user
-
+""") + _("""
 IPA currently immediately issues (or declines) all certificate requests so
 the status of a request is not normally useful. This is for future use
 or the case where a CA does not immediately issue a certificate.
-
+""") + _("""
 The following revocation reasons are supported:
-
-* 0 - unspecified
-* 1 - keyCompromise
-* 2 - cACompromise
-* 3 - affiliationChanged
-* 4 - superseded
-* 5 - cessationOfOperation
-* 6 - certificateHold
-* 8 - removeFromCRL
-* 9 - privilegeWithdrawn
-* 10 - aACompromise
-
+""") + (
+_("* 0 - unspecified") +
+_("* 1 - keyCompromise") +
+_("* 2 - cACompromise") +
+_("* 3 - affiliationChanged") +
+_("* 4 - superseded") +
+_("* 5 - cessationOfOperation") +
+_("* 6 - certificateHold") +
+_("* 8 - removeFromCRL") +
+_("* 9 - privilegeWithdrawn") +
+_("* 10 - aACompromise")
+) + _("""
 Note that reason code 7 is not used.  See RFC 5280 for more details:
-
+""") + _("""
 http://www.ietf.org/rfc/rfc5280.txt
 
 """)
-- 
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