[Freeipa-devel] [PATCH 0179] Preserve user principal aliases during rename operation

[Martin Babinsky]

Quick hacky fix for https://fedorahosted.org/freeipa/ticket/6028

Admittedly a more systematic solution exists. We may discuss it further 
in this thread (such as add options to modrdn plugin to append to 
multivalue attributes). If time is the issue, we may use this fix 
instead and revert it later when more robust solution is implemented.


--
Martin^3 Babinsky


.freeipa-mbabinsk-0179-Preserve-user-principal-aliases-during-rename-operat.patch.swp
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] [Test][patch-0052] Test for incorrect client domain

[Martin Basti]

On 01.07.2016 16:55, Oleg Fayans wrote:

Hi Martin,

Thanks for the review. The updated patch is attached

On 07/01/2016 04:09 PM, Martin Basti wrote:


On 01.07.2016 14:38, Oleg Fayans wrote:

Hi Martin. Now I have this client installation thing sorted out. The
test works as expected

On 06/30/2016 02:57 PM, Martin Basti wrote:

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

I have a few comments:

1)
This is unused and should not be there
+realm_name = domain_name.upper()

Done

2)
teardown_method
shouldn't be more robust, what happens if client uninstall raises an error?

Agree. Done


3) in both tests
+'-w', self.master.config.dirman_password,

-w means admin password (ipa-client-install --help), so you should use
admin not directory manager password

Fixed


4)
+result = client.run_command(['ipa-client-install', '-U',
'--domain',
+ self.master.domain.realm, '-w',

did you mean:  '--domain', self.master.domain.name.upper()

Yes. Fixed.


ACK
master: f784532d4ed6f25cf8ba12f83a7c322515434855
ipa-4-3: 844364bd2770b267c6e913de75b7caa926489c75


--
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 0014-0016][Tests] Authentication indicators

[Lenka Doudova]

On 07/01/2016 02:42 PM, Milan Kubík wrote:

On 06/16/2016 03:23 PM, Lenka Doudova wrote:

Hi,

attached are tests for authentication indicators. Please note:

1. newly created service tracker is not exactly complete, list of 
unimplemented methods is in doc. These methods can be filled in when 
existing declarative tests are refactored.


2. patch 0015 depends on 0014, so it should not be pushed without it.


Lenka





patch 0014:

In the update method, what happens when the updated attributes contain 
addattr? It is not clear to me. Is it necessary?



Example:
ipa service-mod SRV --addattr="authind=radius"

Result:
The way the tracker works, this adds /u'addattr="authind=radius"'/ 
to the list of expected results (result of /self.attrs.update(updates)/. 
Of course nothing like that appears anywhere, so in case there's the 
/--addattr/ option, it's necessary to ensure it won't get to the 
/self.attrs/ atribute.



patch 0015:

host1 and service2 do not tell anything about the purpose of the 
fixture. Please assign more descriptive names to them.
Why do the fixtures have 'function' scope? Does the service entry 
exist during the second and third test case?



Renamed.


patch 0016:

Per offline discussion, admin user has no special privileges here, LGTM.

--
Milan Kubik


Thanks for review, fixed patches (14.2 and 15.2) attached.
Lenka
From 7f8626d9b0cb25f9fe12fdb853f5f3af213ce3ad Mon Sep 17 00:00:00 2001
From: Lenka Doudova 
Date: Wed, 15 Jun 2016 13:40:00 +0200
Subject: [PATCH] Tests: Tracker class for services

Provides basic service tracker, so far for purposes of [1].
Tracker is not complete, some methods will need to be added in case of service test refactoring.

[1] https://fedorahosted.org/freeipa/ticket/433
---
 ipatests/test_xmlrpc/tracker/service_plugin.py | 173 +
 1 file changed, 173 insertions(+)
 create mode 100644 ipatests/test_xmlrpc/tracker/service_plugin.py

diff --git a/ipatests/test_xmlrpc/tracker/service_plugin.py b/ipatests/test_xmlrpc/tracker/service_plugin.py
new file mode 100644
index ..f56a95fd23438108b2afd5ce3dc8db72b8ba4e95
--- /dev/null
+++ b/ipatests/test_xmlrpc/tracker/service_plugin.py
@@ -0,0 +1,173 @@
+# -*- coding: utf-8 -*-
+#
+# Copyright (C) 2016  FreeIPA Contributors see COPYING for license
+#
+
+import six
+
+from ipalib import api
+from ipatests.test_xmlrpc.tracker.base import Tracker
+from ipatests.test_xmlrpc.xmlrpc_test import fuzzy_uuid
+from ipatests.test_xmlrpc import objectclasses
+from ipatests.util import assert_deepequal
+from ipapython.dn import DN
+
+if six.PY3:
+unicode = str
+
+
+class ServiceTracker(Tracker):
+"""
+Tracker class for service plugin
+
+So far does not include methods for these commands:
+service-add-host
+service-remove-host
+service-allow-retrieve-keytab
+service-disallow-retrieve-keytab
+service-allow-create-keytab
+service-disallow-create-keytab
+service-disable
+service-add-cert
+service-remove-cert
+"""
+
+retrieve_keys = {
+u'dn', u'krbprincipalname', u'usercertificate', u'has_keytab',
+u'ipakrbauthzdata', u'ipaallowedtoperform', u'subject',
+u'managedby', u'serial_number', u'serial_number_hex', u'issuer',
+u'valid_not_before', u'valid_not_after', u'md5_fingerprint',
+u'sha1_fingerprint', u'krbprincipalauthind', u'managedby_host',
+u'krbcanonicalname'}
+retrieve_all_keys = retrieve_keys | {
+u'ipaKrbPrincipalAlias', u'ipaUniqueID', u'krbExtraData',
+u'krbLastPwdChange', u'krbLoginFailedCount', u'memberof',
+u'objectClass', u'ipakrbrequirespreauth',
+u'ipakrbokasdelegate'}
+
+create_keys = (retrieve_keys | {u'objectclass', u'ipauniqueid'}) - {
+u'usercertificate', u'has_keytab'}
+update_keys = retrieve_keys - {u'dn'}
+
+def __init__(self, name, host_fqdn, options):
+super(ServiceTracker, self).__init__(default_version=None)
+self._name = "{0}/{1}@{2}".format(name, host_fqdn, api.env.realm)
+self.dn = DN(
+('krbprincipalname', self.name), api.env.container_service,
+api.env.basedn)
+self.host_fqdn = host_fqdn
+self.options = options
+
+@property
+def name(self):
+return self._name
+
+def make_create_command(self, force=True):
+""" Make function that creates a service """
+return self.make_command('service_add', self.name,
+ force=force, **self.options)
+
+def make_delete_command(self):
+""" Make function that deletes a service """
+return self.make_command('service_del', self.name)
+
+def make_retrieve_command(self, all=False, raw=False):
+""" Make function that retrieves a service """
+return self.make_command('service_show', self.name, all=all)
+
+def 

Re: [Freeipa-devel] [Test][patch-0052] Test for incorrect client domain

[Oleg Fayans]

Hi Martin,

Thanks for the review. The updated patch is attached

On 07/01/2016 04:09 PM, Martin Basti wrote:
> 
> 
> On 01.07.2016 14:38, Oleg Fayans wrote:
>> Hi Martin. Now I have this client installation thing sorted out. The
>> test works as expected
>>
>> On 06/30/2016 02:57 PM, Martin Basti wrote:
>>>
>>> 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
> 
> I have a few comments:
> 
> 1)
> This is unused and should not be there
> +realm_name = domain_name.upper()
Done
> 
> 2)
> teardown_method
> shouldn't be more robust, what happens if client uninstall raises an error?

Agree. Done

> 
> 3) in both tests
> +'-w', self.master.config.dirman_password,
> 
> -w means admin password (ipa-client-install --help), so you should use
> admin not directory manager password

Fixed

> 
> 4)
> +result = client.run_command(['ipa-client-install', '-U',
> '--domain',
> + self.master.domain.realm, '-w',
> 
> did you mean:  '--domain', self.master.domain.name.upper()
Yes. Fixed.

> 

-- 
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.
From 28740cad2c6a1a8da80617579db1983bb35114d4 Mon Sep 17 00:00:00 2001
From: Oleg Fayans 
Date: Fri, 1 Jul 2016 16:52:22 +0200
Subject: [PATCH] Test for incorrect client domain

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

diff --git a/ipatests/test_integration/test_replica_promotion.py b/ipatests/test_integration/test_replica_promotion.py
index 1f683b6d5c067ec526b307eea1460cafbadb80cb..7bc1d5281880221578df3c269a3d7715777bb8e0 100644
--- a/ipatests/test_integration/test_replica_promotion.py
+++ b/ipatests/test_integration/test_replica_promotion.py
@@ -377,3 +377,55 @@ class TestOldReplicaWorksAfterDomainUpgrade(IntegrationTest):
 result1 = self.master.run_command(['ipa', 'user-show', self.username],
   raiseonerr=False)
 assert_error(result1, "%s: user not found" % self.username, 2)
+
+
+class TestWrongClientDomain(IntegrationTest):
+topology = "star"
+num_clients = 1
+domain_name = 'exxample.test'
+
+@classmethod
+def install(cls, mh):
+tasks.install_master(cls.master, domain_level=cls.domain_level)
+
+def teardown_method(self, method):
+self.clients[0].run_command(['ipa-client-install',
+ '--uninstall', '-U'],
+raiseonerr=False)
+tasks.kinit_admin(self.master)
+self.master.run_command(['ipa', 'host-del',
+ self.clients[0].hostname],
+raiseonerr=False)
+
+def test_wrong_client_domain(self):
+client = self.clients[0]
+client.run_command(['ipa-client-install', '-U',
+'--domain', self.domain_name,
+'--realm', self.master.domain.realm,
+'-p', 'admin',
+'-w', self.master.config.admin_password,
+'--server', self.master.hostname,
+'--force-join'])
+result = client.run_command(['ipa-replica-install', '-U', '-w',
+ 

Re: [Freeipa-devel] [PATCH 0026][Tests] RFE: Support UPN for trusted domains

[Lenka Doudova]

On 07/01/2016 03:04 PM, Martin Babinsky wrote:

On 07/01/2016 11:13 AM, Lenka Doudova wrote:

And, of course, a patch file :)


On 07/01/2016 11:09 AM, Lenka Doudova wrote:

Hi all,

here's patch with basic test suite for support of UPN.

Note: it needs to be applied on top of my patch 0025.2 (or later, if
there's will be more fixes to that patch).


Lenka







Hi Lenka,

test data such as usernames, etc. should be stored either in separate 
resource files or at least as class attributes like this:


diff --git a/ipatests/test_integration/test_trust.py 
b/ipatests/test_integration/test_trust.py

index e8fdc6b..86ba7cc 100644
--- a/ipatests/test_integration/test_trust.py
+++ b/ipatests/test_integration/test_trust.py
@@ -394,28 +394,33 @@ class TestTrustWithUPN(ADTrustBase):
 """
 Test support of UPN for trusted domains
 """
+upn_suffix = 'UPNsuffix.com'
+upn_username = 'upnuser'
+upn_princ = '{}@{}'.format(upn_username, upn_suffix)
+upn_password = 'Secret123456'
+
 def test_upn_in_nonposix_trust(self):
 """ Check that UPN is listed as trust attribute """
 result = self.master.run_command(['ipa', 'trust-show', 
self.ad_domain,

   '--all', '--raw'])

-assert "ipantadditionalsuffixes: UPNsuffix.com" in 
result.stdout_text

+assert ("ipantadditionalsuffixes: {}".format(self.upn_suffix) in
+result.stdout_text)

 def test_upn_user_resolution_in_nonposix_trust(self):
 """ Check that user with UPN can be resolved """
-upnuser = 'upnu...@upnsuffix.com'
-result = self.master.run_command(['getent', 'passwd', upnuser])
+result = self.master.run_command(['getent', 'passwd', 
self.upn_princ])


 # result will contain AD domain, not UPN
-upnuser_regex = "^upnuser@{0}:\*:(\d+):(\d+):UPN 
User:/:$".format(

-self.ad_domain)
+upnuser_regex = "^{}@{}:\*:(\d+):(\d+):UPN User:/:$".format(
+self.upn_username, self.ad_domain)
 assert re.search(upnuser_regex, result.stdout_text)

 def test_upn_user_authentication(self):
 """ Check that AD user with UPN can authenticate in IPA """
 self.master.run_command(['systemctl', 'restart', 'krb5kdc'])
-self.master.run_command(['kinit', '-C', '-E', 
'upnu...@upnsuffix.com'],

-stdin_text='Secret123456')
+self.master.run_command(['kinit', '-C', '-E', self.upn_princ],
+stdin_text=self.upn_password)

otherwise LGTM.


Thanks for review, fixed patch attached.

Few notes:
1. mbabinsky's suggestion to store testdata as class attributes or 
separate resource file: I decided to use the class attribute approach. 
The separate resource file is a nice idea, which I have already put on 
my "to do" list - there's a lot of hardcoded stuff in the trust tests, 
even in the original ones (before my patches), so when there's time I'll 
work on a way how to dynamically provide this data as test configuration
2. previous discussion about getent vs. pwd.getpwnam(): I'll leave the 
getent command, since according to mbasti the alternative would not work 
in CI.


Lenka
From 997ae46d6ee2ab5a147e9f57ef17778cad943cdd Mon Sep 17 00:00:00 2001
From: Lenka Doudova 
Date: Fri, 1 Jul 2016 11:00:57 +0200
Subject: [PATCH] Tests: Support of UPN for trusted domains

Basic set of tests to verify support of UPN functionality.

Test cases:
- establish trust
- verify the trust recognizes UPN
- verify AD user with UPN can be resolved
- verify AD user with UPN can authenticate
- remove trust

https://fedorahosted.org/freeipa/ticket/5354
---
 ipatests/test_integration/test_trust.py | 40 +
 1 file changed, 40 insertions(+)

diff --git a/ipatests/test_integration/test_trust.py b/ipatests/test_integration/test_trust.py
index ba7ab8fdc0703369d55302ae3c20e79bd1b01daa..2507bf1747bfcdfdda4ae269ea403aad66fa903a 100644
--- a/ipatests/test_integration/test_trust.py
+++ b/ipatests/test_integration/test_trust.py
@@ -345,3 +345,43 @@ class TestExternalTrustWithRootDomain(ADTrustSubdomainBase):
 def test_remove_nonposix_trust(self):
 tasks.remove_trust_with_ad(self.master, self.ad_domain)
 tasks.clear_sssd_cache(self.master)
+
+
+class TestTrustWithUPN(ADTrustBase):
+"""
+Test support of UPN for trusted domains
+"""
+
+upn_suffix = 'UPNsuffix.com'
+upn_username = 'upnuser'
+upn_name = 'UPN User'
+upn_principal = '{}@{}'.format(upn_username, upn_suffix)
+upn_password = 'Secret123456'
+
+def test_upn_in_nonposix_trust(self):
+""" Check that UPN is listed as trust attribute """
+result = self.master.run_command(['ipa', 'trust-show', self.ad_domain,
+  '--all', '--raw'])
+
+assert ("ipantadditionalsuffixes: {}".format(self.upn_suffix) in
+

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

[Martin Basti]

On 01.07.2016 14:48, Petr Spacek wrote:

On 30.6.2016 18:05, Martin Basti wrote:


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.

ACK


Pushed to ipa-4-3: 4edd39fb05dfd92924fc7f0c37fee3d269edf298

--
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 0146] Fix internal errors in host-add and other commands caused by DNS resolutio

[Martin Basti]

On 01.07.2016 10:37, Martin Basti wrote:



On 01.07.2016 09:05, Petr Spacek wrote:

On 30.6.2016 21:23, Petr Spacek wrote:

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.


My bad, I forgot to attach cleanup patch 147 which is prerequisite 
for 146.

(Sorry for the numbering.)


ACK

master:
* ce1f9ca51bd91ed66233c1bac7eb05fac9c855c7 Remove unused is_local(), 
interface, and defaultnet from CheckedIPAddress
* 5e78b54d7c532bec0ee5a4ce3f1b6d6c94d17c51 Fix internal errors in 
host-add and other commands caused by DNS resolution


I will review 4.3 later


ACK

ipa-4-3:
* 0db277eb224b92319aede31d1840db781c10 Remove unused is_local(), 
interface, and defaultnet from CheckedIPAddress
* b8d5881ba93b00653ba42c61369f19ca27fb7a64 Fix internal errors in 
host-add and other commands caused by DNS resolution


--
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 07/01/2016 02:38 PM, Martin Babinsky wrote:

On 07/01/2016 06:36 AM, Lenka Doudova wrote:



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


Hi Lenka,

I am still not happy about the patch underutilizing the potential for 
a proper inheritance hierarchy and instead relying on a staggering 
amounts of copy-pasting for implementation.


I am attaching a quick untested patch illustrating how the 
implementation should look like.


Also you have some leftovers from previous revision, please fix them:

Pylint is running, please wait ...
* Module ipatests.test_integration.test_trust
ipatests/test_integration/test_trust.py:236: [E1101(no-member), 
TestExternalTrustWithSubdomain.configure_dns_and_time] Module 
'ipatests.test_integration.tasks' has no 'sync_time_hostname' member)
ipatests/test_integration/test_trust.py:243: 
[E1123(unexpected-keyword-arg), 
TestExternalTrustWithSubdomain.test_establish_trust] Unexpected 
keyword argument 'subdomain' in function call)
ipatests/test_integration/test_trust.py:279: 
[E1123(unexpected-keyword-arg), 
TestExternalTrustWithSubdomain.test_remove_nonposix_trust] Unexpected 
keyword argument 'subdomain' in function call)
ipatests/test_integration/test_trust.py:330: [E1101(no-member), 
TestNonexternalTrustWithSubdomain.configure_dns_and_time] Module 
'ipatests.test_integration.tasks' has no 'sync_time_hostname' member)

Makefile:137: recipe for target 'lint' failed
make: *** [lint] Error 1
sending incremental file list

(this is before I started meddling with it)



Thank you very much for the example, fixed 

Re: [Freeipa-devel] [Test][patch-0052] Test for incorrect client domain

[Martin Basti]

On 01.07.2016 14:38, Oleg Fayans wrote:

Hi Martin. Now I have this client installation thing sorted out. The
test works as expected

On 06/30/2016 02:57 PM, Martin Basti wrote:


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


I have a few comments:

1)
This is unused and should not be there
+realm_name = domain_name.upper()

2)
teardown_method
shouldn't be more robust, what happens if client uninstall raises an error?

3) in both tests
+'-w', self.master.config.dirman_password,

-w means admin password (ipa-client-install --help), so you should use 
admin not directory manager password


4)
+result = client.run_command(['ipa-client-install', '-U', 
'--domain',

+ self.master.domain.realm, '-w',

did you mean:  '--domain', self.master.domain.name.upper()

--
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 0038-0040] Sub CA test patches

[Milan Kubík]

On 06/27/2016 01:31 PM, Milan Kubík wrote:

On 06/27/2016 02:57 AM, Fraser Tweedale wrote:

On Fri, Jun 24, 2016 at 12:08:24PM +0200, Milan Kubík wrote:

On 06/24/2016 03:42 AM, Fraser Tweedale wrote:

On Tue, Jun 21, 2016 at 05:01:35PM +0200, Milan Kubík wrote:

Hi Fraser and list,

I have made changes to the test plan on the wiki [1] according to the
information in "[Testplan review] Sub CAs" thread.

I also implemented the tests in the test plan:

patch 0038 - CATracker and CA CRUD test
patch 0039 - extension to CA ACL test
patch 0040 - functional test with ACLs and certificate profile, 
reusing my
previous S/MIME based tests. This patch also tests for the 
cert-request

behavior when profile ID or CA cn are ommited.

The tests ATM do not verify the Issuer name in the certificate 
itself, just

from the ipa entry of the certificate.


The approach you are using::

  assert cert_info['result']['issuer'] == 
smime_signing_ca.ipasubjectdn


is not quite as you describe (these are virtual attributes, not
attributes of an actual entry); but the approach is valid.
The issue then is in the wording? The other approach I could have 
used here

is to retrieve the two certificates and compare the fields manually.
Are these virtual attributes created from the certificate itself?


That's correct.

Fraser, could you please verify my reasoning behind the test cases 
for

cert-request in the patch 40?


The tests look OK.  With the default CA / default profiles, is there
appropriate isolation between test cases to ensure that if, e.g.
some other test case adds/modifies CA ACLs such that these
expected-to-fail tests now pass, that this does not affect the
TestCertSignMIMEwithSubCA test case?

Thanks,
Fraser
The ACL, SMIME CA and S/MIME profile lifetime is constrained by the 
class

scope
enforced by pytest.
The two test cases depend on the fact documented in the designs and 
that is

what
cert-request fallbacks to when CA or profile ID are not provided.
Unless something changes caIPAserviceCert profile or affiliated ACL, 
then

the test cases
are safe.


If you have thought about possible interference from other tests, I
am happy.

Note another problematic scenario: what if a different (preceding)
test adds a CA ACL that would allow the requests that you expect to
fail?  Just something to think about :)

Thanks,
Fraser
Then the failure would be problem of the preceding test and we would 
need to fix it. We are dealing with test side effects

in other parts of the execution already...

The test is constructed in a way that isolates it (to a certain 
degree) by the mechanisms available
in pytest. Of course I cannot make the test future-proof or guarantee 
that a bug in some other test
will not affect the execution of other tests as they all run against 
one IPA instance.
I do not think, however, that potential misbehaving test case that 
will interfere

should prevent us from implementing this and similar test cases.

If you have some specific issue that is in the patch, I'm happy to fix 
them.

I will try to think more about corner cases here.

[1]: http://www.freeipa.org/page/V4/Sub-CAs/Test_Plan

Cheers

--
Milan Kubik

Attaching rebased patches and removing the expected fail from one of 
the

tests as ticket 5981 has fix posted.

--
Milan Kubik






Hi Fraser,

can we continue with the review, please?

--
Milan Kubik

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

Re: [Freeipa-devel] [PATCH] pylint fixes

[Florence Blanc-Renaud]

On 06/21/2016 01:51 PM, Martin Basti wrote:



On 21.06.2016 08:38, Florence Blanc-Renaud wrote:

On 06/20/2016 07:08 PM, Martin Basti wrote:



On 20.06.2016 19:06, Martin Basti wrote:




On 20.06.2016 12:00, Florence Blanc-Renaud wrote:

On 06/09/2016 05:10 PM, Petr Spacek wrote:

Hello,

I've received a bunch of pylint fixes produced by upstream
contributor who is
not subscribed to the list so I'm resending them here.

All credit goes to Bárta Jan <55042ba...@sstebrno.eu>.

Flo, if you have time for it I think that it could be a good
exercise which
will lead you to various dark corners in IPA :-)

Petr^2 Spacek


 Forwarded Message 
Date: Fri, 3 Jun 2016 14:57:16 +0200
From: Bárta Jan <55042ba...@sstebrno.eu>
To: pspa...@redhat.com

___- In the patch
0002-pylint-fix-simplifiable-if-statement-warnings.patch:_

diff --git a/ipatests/test_integration/tasks.py
b/ipatests/test_integration/tasks.py
index aebd907..ca2e10f 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -149,11 +149,7 @@ def host_service_active(host, service):
 res = host.run_command(['systemctl', 'is-active', '--quiet',
service],
raiseonerr=False)

-if res.returncode == 0:
-return True
-else:
-return False
-
+return res.returncode

should be instead: return res.returncode *== 0* (otherwise the return
type is an int and not a boolean).

In the same file:
@@ -295,11 +291,7 @@ def
master_authoritative_for_client_domain(master, client):
 zone = ".".join(client.hostname.split('.')[1:])
 result = master.run_command(["ipa", "dnszone-show", zone],
 raiseonerr=False)
-if result.returncode == 0:
-return True
-else:
-return False
-
+result.returncode == 0

should be instead: *return* result.returncode == 0 (otherwise there
is no return statement)

diff --git a/ipaserver/plugins/dogtag.py b/ipaserver/plugins/dogtag.py
index 197814c..36b6ba5 100644
--- a/ipaserver/plugins/dogtag.py
+++ b/ipaserver/plugins/dogtag.py
@@ -1689,12 +1689,7 @@ class ra(rabase.rabase):
 # Return command result
 cmd_result = {}

-if parse_result.get('revoked') == 'yes':
-cmd_result['revoked'] = True
-else:
-cmd_result['revoked'] = False
-
-return cmd_result
+cmd_result['revoked'] = parse_result.get('revoked')

Should be instead: cmd_result['revoked'] =
parse_result.get('revoked') *== 'yes'* (otherwise the type is a
string and not a boolean)

_- in the patch 00__04-pylint-fix-unneeded-not.patch_

@@ -632,7 +632,7 @@ class host_add(LDAPCreate):
 options['ip_address'],
 check_forward=True,
 check_reverse=check_reverse)
-if not options.get('force', False) and not 'ip_address' in
options:
+if options.get('force', False) and 'ip_address' not in
options:

Should be instead: if *not* options.get('force', False) and
'ip_address' not in options:
because of operators precedence

I will review patches 0005 to 0010 later today.
Flo.




How about patches 1, and 3? Because patches are independent, we can
separately ACK them and push them.

Martin^2




Sorry, I just noticed that there is no patch 1 :)



Patch 0003 is OK, ACK for this one.
Flo.


Patch 0003: Pushed to master: 94909d21dbf033cbe34089782c430ec25b9ad0bc


Hi,

please find my comments on the remaining patches.

- Patch 0005 must be rebased because of changes in 
ipatests/test_integration/tasks.py

the patch can also modify pylintrc (remove pointless-statement)

- Patch 0006:
no need to rename the items in "for e in ...", renaming the Exception as 
exc should be enough


- Patch 0007:
pylintrc should remove old-style-class instead of bad-classmethod-argument

- Patch 0008:
this one should remove bad-classmethod-argument in pylintrc

- Patch 0009:
ok

- Patch 0010:
In the __bind method(self, obj_type), cls variable is already used thus 
replacing self with cls can be done only if cls is also renamed into 
something else.


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 0146] Fix internal errors in host-add and other commands caused by DNS resolutio

[Martin Basti]

On 01.07.2016 10:37, Martin Basti wrote:



On 01.07.2016 09:05, Petr Spacek wrote:

On 30.6.2016 21:23, Petr Spacek wrote:

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.


My bad, I forgot to attach cleanup patch 147 which is prerequisite 
for 146.

(Sorry for the numbering.)


ACK

master:
* ce1f9ca51bd91ed66233c1bac7eb05fac9c855c7 Remove unused is_local(), 
interface, and defaultnet from CheckedIPAddress
* 5e78b54d7c532bec0ee5a4ce3f1b6d6c94d17c51 Fix internal errors in 
host-add and other commands caused by DNS resolution


I will review 4.3 later


ACK

ipa-4-3:
* 0db277eb224b92319aede31d1840db781c10 Remove unused is_local(), 
interface, and defaultnet from CheckedIPAddress
* b8d5881ba93b00653ba42c61369f19ca27fb7a64 Fix internal errors in 
host-add and other commands caused by DNS resolution


--
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 0026][Tests] RFE: Support UPN for trusted domains

[Martin Babinsky]

On 07/01/2016 11:13 AM, Lenka Doudova wrote:

And, of course, a patch file :)


On 07/01/2016 11:09 AM, Lenka Doudova wrote:

Hi all,

here's patch with basic test suite for support of UPN.

Note: it needs to be applied on top of my patch 0025.2 (or later, if
there's will be more fixes to that patch).


Lenka







Hi Lenka,

test data such as usernames, etc. should be stored either in separate 
resource files or at least as class attributes like this:


diff --git a/ipatests/test_integration/test_trust.py 
b/ipatests/test_integration/test_trust.py

index e8fdc6b..86ba7cc 100644
--- a/ipatests/test_integration/test_trust.py
+++ b/ipatests/test_integration/test_trust.py
@@ -394,28 +394,33 @@ class TestTrustWithUPN(ADTrustBase):
 """
 Test support of UPN for trusted domains
 """
+upn_suffix = 'UPNsuffix.com'
+upn_username = 'upnuser'
+upn_princ = '{}@{}'.format(upn_username, upn_suffix)
+upn_password = 'Secret123456'
+
 def test_upn_in_nonposix_trust(self):
 """ Check that UPN is listed as trust attribute """
 result = self.master.run_command(['ipa', 'trust-show', 
self.ad_domain,

   '--all', '--raw'])

-assert "ipantadditionalsuffixes: UPNsuffix.com" in 
result.stdout_text

+assert ("ipantadditionalsuffixes: {}".format(self.upn_suffix) in
+result.stdout_text)

 def test_upn_user_resolution_in_nonposix_trust(self):
 """ Check that user with UPN can be resolved """
-upnuser = 'upnu...@upnsuffix.com'
-result = self.master.run_command(['getent', 'passwd', upnuser])
+result = self.master.run_command(['getent', 'passwd', 
self.upn_princ])


 # result will contain AD domain, not UPN
-upnuser_regex = "^upnuser@{0}:\*:(\d+):(\d+):UPN User:/:$".format(
-self.ad_domain)
+upnuser_regex = "^{}@{}:\*:(\d+):(\d+):UPN User:/:$".format(
+self.upn_username, self.ad_domain)
 assert re.search(upnuser_regex, result.stdout_text)

 def test_upn_user_authentication(self):
 """ Check that AD user with UPN can authenticate in IPA """
 self.master.run_command(['systemctl', 'restart', 'krb5kdc'])
-self.master.run_command(['kinit', '-C', '-E', 
'upnu...@upnsuffix.com'],

-stdin_text='Secret123456')
+self.master.run_command(['kinit', '-C', '-E', self.upn_princ],
+stdin_text=self.upn_password)

otherwise LGTM.

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

[Petr Spacek]

On 30.6.2016 18:05, Martin Basti wrote:
> 
> 
> 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.

ACK

-- 
Petr^2 Spacek

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

Re: [Freeipa-devel] [PATCH 0014-0016][Tests] Authentication indicators

[Milan Kubík]

On 06/16/2016 03:23 PM, Lenka Doudova wrote:

Hi,

attached are tests for authentication indicators. Please note:

1. newly created service tracker is not exactly complete, list of 
unimplemented methods is in doc. These methods can be filled in when 
existing declarative tests are refactored.


2. patch 0015 depends on 0014, so it should not be pushed without it.


Lenka





patch 0014:

In the update method, what happens when the updated attributes contain 
addattr? It is not clear to me. Is it necessary?


patch 0015:

host1 and service2 do not tell anything about the purpose of the 
fixture. Please assign more descriptive names to them.
Why do the fixtures have 'function' scope? Does the service entry exist 
during the second and third test case?


patch 0016:

Per offline discussion, admin user has no special privileges here, LGTM.

--
Milan Kubik

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

Re: [Freeipa-devel] [Test][patch-0052] Test for incorrect client domain

[Oleg Fayans]

Hi Martin. Now I have this client installation thing sorted out. The
test works as expected

On 06/30/2016 02:57 PM, Martin Basti wrote:
> 
> 
> 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

-- 
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.
From 056105d13fda655469d7a9c8ca2526a09ae31373 Mon Sep 17 00:00:00 2001
From: Oleg Fayans 
Date: Fri, 1 Jul 2016 14:33:06 +0200
Subject: [PATCH] Test for incorrect client domain

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

diff --git a/ipatests/test_integration/test_replica_promotion.py b/ipatests/test_integration/test_replica_promotion.py
index 1f683b6d5c067ec526b307eea1460cafbadb80cb..1d72d08120a874eb93976d48b8b062bc28d64a0a 100644
--- a/ipatests/test_integration/test_replica_promotion.py
+++ b/ipatests/test_integration/test_replica_promotion.py
@@ -377,3 +377,53 @@ class TestOldReplicaWorksAfterDomainUpgrade(IntegrationTest):
 result1 = self.master.run_command(['ipa', 'user-show', self.username],
   raiseonerr=False)
 assert_error(result1, "%s: user not found" % self.username, 2)
+
+
+class TestWrongClientDomain(IntegrationTest):
+topology = "star"
+num_clients = 1
+domain_name = 'exxample.test'
+realm_name = domain_name.upper()
+
+@classmethod
+def install(cls, mh):
+tasks.install_master(cls.master, domain_level=cls.domain_level)
+
+def teardown_method(self, method):
+self.clients[0].run_command(['ipa-client-install',
+ '--uninstall', '-U'])
+tasks.kinit_admin(self.master)
+self.master.run_command(['ipa', 'host-del', self.clients[0].hostname])
+
+def test_wrong_client_domain(self):
+client = self.clients[0]
+client.run_command(['ipa-client-install', '-U',
+'--domain', self.domain_name,
+'--realm', self.master.domain.realm,
+'-p', 'admin',
+'-w', self.master.config.dirman_password,
+'--server', self.master.hostname,
+'--force-join'])
+result = client.run_command(['ipa-replica-install', '-U', '-w',
+ self.master.config.dirman_password],
+raiseonerr=False)
+assert_error(result,
+ "Cannot promote this client to a replica. Local domain "
+ "'%s' does not match IPA domain "
+ "'%s'" % (self.domain_name, self.master.domain.name))
+
+def test_upcase_client_domain(self):
+client = self.clients[0]
+result = client.run_command(['ipa-client-install', '-U', '--domain',
+ self.master.domain.realm, '-w',
+ self.master.config.dirman_password,
+ '-p', 'admin',
+ '--server', self.master.hostname,
+ '--force-join'], raiseonerr=False)
+assert(result.returncode == 0), (
+'Failed to setup client with the upcase domain name')
+result1 = client.run_command(['ipa-replica-install', '-U', '-w',
+   

Re: [Freeipa-devel] [PATCH 0025][Tests] RFE: External trust

[Martin Babinsky]

On 07/01/2016 06:36 AM, Lenka Doudova wrote:



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


Hi Lenka,

I am still not happy about the patch underutilizing the potential for a 
proper inheritance hierarchy and instead relying on a staggering amounts 
of copy-pasting for implementation.


I am attaching a quick untested patch illustrating how the 
implementation should look like.


Also you have some leftovers from previous revision, please fix them:

Pylint is running, please wait ...
* Module ipatests.test_integration.test_trust
ipatests/test_integration/test_trust.py:236: [E1101(no-member), 
TestExternalTrustWithSubdomain.configure_dns_and_time] Module 
'ipatests.test_integration.tasks' has no 'sync_time_hostname' member)
ipatests/test_integration/test_trust.py:243: 
[E1123(unexpected-keyword-arg), 
TestExternalTrustWithSubdomain.test_establish_trust] Unexpected keyword 
argument 'subdomain' in function call)
ipatests/test_integration/test_trust.py:279: 
[E1123(unexpected-keyword-arg), 
TestExternalTrustWithSubdomain.test_remove_nonposix_trust] Unexpected 
keyword argument 'subdomain' in function call)
ipatests/test_integration/test_trust.py:330: [E1101(no-member), 
TestNonexternalTrustWithSubdomain.configure_dns_and_time] Module 
'ipatests.test_integration.tasks' has no 'sync_time_hostname' member)

Makefile:137: recipe for target 'lint' failed
make: *** [lint] Error 1
sending incremental file list

(this is before I started meddling with it)


--
Martin^3 Babinsky
From f9849e9667e595d67ffd631f312106ae4278c135 Mon Sep 17 00:00:00 2001
From: Martin 

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

[Fraser Tweedale]

On Fri, Jul 01, 2016 at 10:05:48AM +0200, Jan Cholasta wrote:
> On 1.7.2016 08:57, Jan Cholasta wrote:
> > On 1.7.2016 06:54, Jan Cholasta wrote:
> > > 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?
> > 
> > To speed things up, I have updated your patch with this, see the
> > attachment.
> > 
> > If the change looks good to you, we can push the patch.
> 
> Your original patch works for me, ACK. My change can be pushed under the
> one-liner rule, so pushing them combined in the modified patch.
> 
> Pushed to master: 4844eaec197690e21c6cf44743df7f456d0e185d
> 
Jan, thanks for pushing that along.  My WIP was much the same, but I
got bitten by stale API schema on client side and it did not know
about the --ca option.  I revisited it tonight, but by then you had
pushed the commit :)

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

[Martin Basti]

On 29.06.2016 20:46, Ben Lipton wrote:
The attached patch silences some annoying messages I've been getting 
when upgrading the freeipa-client package on F24:

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

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

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


Ben




Hello, I don't like to hiding errors/warnings. Can you determine and 
solve the root cause?


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

Re: [Freeipa-devel] [PATCH 0148] client-install: log exceptions from certmonger.request_cer

[Martin Basti]

On 01.07.2016 11:57, Petr Spacek wrote:

Hello,

client-install: log exceptions from certmonger.request_cert




ACK

Pushed to master: dc5b2eaa772fda5673b222bc9107cf5b85c1295d

-- 
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 0026][Tests] RFE: Support UPN for trusted domains

[Martin Basti]

On 01.07.2016 13:08, Alexander Bokovoy wrote:

On Fri, 01 Jul 2016, Lukas Slebodnik wrote:

On (01/07/16 11:13), Lenka Doudova wrote:

And, of course, a patch file :)


On 07/01/2016 11:09 AM, Lenka Doudova wrote:

Hi all,

here's patch with basic test suite for support of UPN.

Note: it needs to be applied on top of my patch 0025.2 (or later, if
there's will be more fixes to that patch).


Lenka






From 5c8cb8727322371b7246f6d939b38ac1cbd61e4c Mon Sep 17 00:00:00 2001
From: Lenka Doudova 
Date: Fri, 1 Jul 2016 11:00:57 +0200
Subject: [PATCH] Tests: Support of UPN for trusted domains

Basic set of tests to verify support of UPN functionality.

Test cases:
- establish trust
- verify the trust recognizes UPN
- verify AD user with UPN can be resolved
- verify AD user with UPN can authenticate
- remove trust

https://fedorahosted.org/freeipa/ticket/5354
---
ipatests/test_integration/test_trust.py | 32 


1 file changed, 32 insertions(+)

diff --git a/ipatests/test_integration/test_trust.py 
b/ipatests/test_integration/test_trust.py
index 
d662e80727b6eab3df93166d35ddbaea6a0f6f7a..e8fdc6ba68fb6275a0d7920c76ca434ed830ed84 
100644

--- a/ipatests/test_integration/test_trust.py
+++ b/ipatests/test_integration/test_trust.py
@@ -388,3 +388,35 @@ class 
TestExternalTrustWithRootDomain(ADTrustBase):


tasks.remove_trust_with_ad(self.master, self.ad_domain)
tasks.clear_sssd_cache(self.master)
+
+
+class TestTrustWithUPN(ADTrustBase):
+"""
+Test support of UPN for trusted domains
+"""
+def test_upn_in_nonposix_trust(self):
+""" Check that UPN is listed as trust attribute """
+result = self.master.run_command(['ipa', 'trust-show', 
self.ad_domain,

+  '--all', '--raw'])
+
+assert "ipantadditionalsuffixes: UPNsuffix.com" in 
result.stdout_text

+
+def test_upn_user_resolution_in_nonposix_trust(self):
+""" Check that user with UPN can be resolved """
+upnuser = 'upnu...@upnsuffix.com'
+result = self.master.run_command(['getent', 'passwd', 
upnuser])

Is there a special reason for not using pwd.getpwnam() ?

Technically -- yes. In case there was a change in the system
configuration (/etc/nsswitch.conf), then these changes wouldn't be
reflected in the application that is already using NSSWITCH interface.

However, in this particular case no change to config files is expected
so pwd.getpwnam() can be used.


Please note that the commands are executed remotely in CI tests, 
pwd.getpwnam() provides only local data.

Martin^2

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

Re: [Freeipa-devel] [PATCH] 0046 Create server certs with DNS altname

[Petr Spacek]

On 20.1.2016 05:04, Fraser Tweedale wrote:
> On Tue, Dec 08, 2015 at 07:06:39PM +1000, Fraser Tweedale wrote:
>> On Mon, Dec 07, 2015 at 05:50:05PM -0500, Rob Crittenden wrote:
>>> Fraser Tweedale wrote:
 On Mon, Dec 07, 2015 at 01:53:15PM +0100, Martin Kosek wrote:
> On 12/07/2015 06:26 AM, Fraser Tweedale wrote:
>> The attached patch fixes
>> https://fedorahosted.org/freeipa/ticket/4970.
>>
>> Note that the problem is addressed by adding the appropriate request
>> extension to the CSR; the fix does not involve changing the default
>> profile behaviour, which is complicated (see ticket for details).
>
> Thanks for the patch! This is something we should really fix, I already 
> get
> warnings in my Python scripts when I hit sites protected by such HTTPS 
> cert:
>
> /usr/lib/python2.7/site-packages/requests/packages/urllib3/connection.py:264:
> SubjectAltNameWarning: Certificate for projects.engineering.redhat.com 
> has no
> `subjectAltName`, falling back to check for a `commonName` for now. This
> feature is being removed by major browsers and deprecated by RFC 2818. 
> (See
> https://github.com/shazow/urllib3/issues/497 for details.)
>
> Should we split ticket 4970, for the FreeIPA server part and then for cert
> profile part? As it looks like the FreeIPA server will be fixed even in 
> FreeIPA
> 4.3.x and the other part later.
>
> How difficult do you see the general FreeIPA Certificate Profile part of 
> this
> request? Is it a too big task to handle in 4.4 time frame?
>
 I will split the ticket and would suggest 4.4 Backlog - it might be
 doable but is a lower priority than e.g. Sub-CAs.
>>>
>>> If you are going to defer the profile part then you should probably
>>> update the client to also include a SAN if --request-cert is provided.
>>>
>>> rob
>>>
>> Yes, good idea.  Updated patch attached.
>>
>> Cheers,
>> Fraser
> 
> Bump, with rebased patch.

Hi,

this seems to work for Apache on IPA server & client cert. ACK.

Interestingly enough I found out that Dogtag cert used on port 8443 does not
have any SAN.

Is it in scope of this ticket?

-- 
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 031] RedHatCAService should wait for local Dogtag instance

[Petr Spacek]

On 1.7.2016 11:43, Petr Spacek wrote:
> On 1.7.2016 11:17, Petr Spacek wrote:
>> On 1.7.2016 11:04, Christian Heimes wrote:
>>> On 2016-07-01 10:59, Petr Spacek wrote:
 On 1.7.2016 10:55, Christian Heimes wrote:
> On 2016-07-01 10:48, Petr Spacek wrote:
>> On 1.7.2016 10:42, Christian Heimes wrote:
>>> RedHatCAService.wait_until_running() uses dogtag.ca_status() to make a
>>> HTTP(s) request to Dogtag in order to check if /ca/admin/ca/getStatus
>>> returns OK. The ca_status() function defaults to api.env.ca_host as
>>> host.
>>>
>>> On a replica without CA ca_host is a remote host (e.g. master's
>>> FQDN). ipa-ca-install waits for master:8080 instead of replica:8080,
>>> which might be blocked by a firewall.
>>>
>>> https://fedorahosted.org/freeipa/ticket/6016
>>
>> Interesting. How it happens that replica without CA is calling 
>> RedHatCAService?
>>
>> Also, why replica should be waiting for CA if it is not installed?
>>
>> I'm confused.
>
> There is a hint in the last sentence: ipa-ca-install
>
> The patch fixes ipa-ca-install on replicas. Right now ipa-ca-install
> doesn't wait for the local Dogtag to come up but connects to a remote
> Dogtag to check if it's up. It uses 8443 or 8080, which might be
> blocked. In my test setup I have both ports blocked so ipa-ca-install
> never succeeds.

 Oh, I missed that, thanks!

 Isn't the root cause that ipa.env.ca_host does not get updated during
 ipa-ca-install?
>>>
>>> Been there, tried it, didn't work:
>>> https://fedorahosted.org/freeipa/ticket/6016#comment:1
>>
>> I understand that it does not work right now but it does not mean that it is
>> an actual problem in api.env :-)
>>
>> Anyway, I'm testing your patch but I'm not sure we can get it into 4.4.0 as
>> Petr^1 is about to push the RELEASE button any minute now.
>>
>> Petr^2 Spacek
>>
>>> It just doesn't make sense that RedHatCAService should ever check a
>>> remote instance. The rest of the class is about the local systemd
>>> service. As soon as we have sd_notify
>>> https://fedorahosted.org/pki/ticket/1233 implemented, we can use systemd
>>> to wait for Dogtag.
> 
> It seems to work but ipa-client-install blows up on certificate request.
> 
> # getcert list
> Number of certificates and requests being tracked: 1.
> Request ID '20160701093734':
> status: CA_UNREACHABLE
> ca-error: Server at
> https://vm-058-082.abc.idm.lab.eng.brq.redhat.com/ipa/xml failed request, will
> retry: 903 (RPC failed at server.  an internal error has occurred).
> stuck: no
> key pair storage: type=NSSDB,location='/etc/ipa/nssdb',nickname='Local
> IPA host',token='NSS Certificate DB',pinfile='/etc/ipa/nssdb/pwdfile.txt'
> certificate: type=NSSDB,location='/etc/ipa/nssdb',nickname='Local IPA
> host'
> CA: IPA
> issuer:
> subject:
> expires: unknown
> pre-save command:
> post-save command:
> track: yes
> auto-renew: yes
> 
> error log on the server:
> 
> [Fri Jul 01 11:37:34.294677 2016] [wsgi:error] [pid 38273] ipa: INFO:
> [jsonserver_kerb]
> host/vm-046.abc.idm.lab.eng.brq.redhat@dom-058-082.abc.idm.lab.eng.brq.redhat.com:
> host_mod(u'vm-046.abc.idm.lab.eng.brq.redhat.com', ipasshpubkey=(u'ssh-rsa
> B3NzaC1yc2EDAQABAAABAQCtrWFHeOF6UxI/DNdlLsUUazTpol2sRqQgbZplpkB9t/HUSjUHq0OY1mwaUfxvJp/E9yDmuHZgUgzKMSAdUf2apwFm5bw3T7qSdJ0Y7hC9vG0v6kLT0EaPuQmfJ8Rt4xOyva9htKbzkxs9Kr0ujB6V4u41ZZW2oevqtGunC2+aCxkQzd42we0c47ypxnvl8gGAa76CDXenGaChPKSfeEMddnhFvjGfkSyqjD+dCxBF+IyTRDPtt6f5iF80lfv/559rsKYlHdbbgv30i5C/F2DzaB011BmcQwK1eWSGWsEWVFtQKNMdahTl2IMgvZwHcaw8TMqgqqgZ7ZZ6lMR+UA8l',
> u'ecdsa-sha2-nistp256
> E2VjZHNhLXNoYTItbmlzdHAyNTYIbmlzdHAyNTYAAABBBHkoeGOzfQzqYOGQs2bdgL0jOBul+/eTBZ0HBM8HW3Wb5O15Fv3rt8jRp+xdSQcdG3DV5yPfjd66Fyz5hCTKS6s=',
> u'ssh-ed25519
> C3NzaC1lZDI1NTE5IH5/uXvdJ1l+uTAk0rgbjKeTBx9HRWk7w+xJLHMt/yRx'),
> updatedns=False, version=u'2.26'): SUCCESS
> [Fri Jul 01 11:37:37.961175 2016] [wsgi:error] [pid 38272] ipa: ERROR:
> non-public: ValueError: User name is defined only for user and enterprise
> principals
> [Fri Jul 01 11:37:37.961220 2016] [wsgi:error] [pid 38272] Traceback (most
> recent call last):
> [Fri Jul 01 11:37:37.961224 2016] [wsgi:error] [pid 38272]   File
> "/usr/lib/python2.7/site-packages/ipaserver/rpcserver.py", line 352, in
> wsgi_execute
> [Fri Jul 01 11:37:37.961226 2016] [wsgi:error] [pid 38272] result =
> self.Command[name](*args, **options)
> [Fri Jul 01 11:37:37.961229 2016] [wsgi:error] [pid 38272]   File
> "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 447, in __call__
> [Fri Jul 01 11:37:37.961259 2016] [wsgi:error] [pid 38272] return
> self.__do_call(*args, **options)
> [Fri Jul 01 11:37:37.961262 2016] [wsgi:error] [pid 38272]   File
> "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 475, in __do_call
> [Fri 

Re: [Freeipa-devel] [PATCH 0178] Fix incorrect check for principal type when evaluating CA ACLs

[Jan Cholasta]

On 1.7.2016 13:08, Petr Spacek wrote:

On 1.7.2016 11:58, Martin Babinsky wrote:

Fixing first regression caused by principal alias work.

Thanks Petr Spacek for finding it.


ACK, please add ticket number before push:
https://fedorahosted.org/freeipa/ticket/3864



Pushed to master: 0ade41abbad324d8c54449f3b1024a7651dc259d

--
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 0178] Fix incorrect check for principal type when evaluating CA ACLs

[Petr Spacek]

On 1.7.2016 11:58, Martin Babinsky wrote:
> Fixing first regression caused by principal alias work.
> 
> Thanks Petr Spacek for finding it.

ACK, please add ticket number before push:
https://fedorahosted.org/freeipa/ticket/3864

-- 
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 0026][Tests] RFE: Support UPN for trusted domains

[Alexander Bokovoy]

On Fri, 01 Jul 2016, Lukas Slebodnik wrote:

On (01/07/16 11:13), Lenka Doudova wrote:

And, of course, a patch file :)


On 07/01/2016 11:09 AM, Lenka Doudova wrote:

Hi all,

here's patch with basic test suite for support of UPN.

Note: it needs to be applied on top of my patch 0025.2 (or later, if
there's will be more fixes to that patch).


Lenka






From 5c8cb8727322371b7246f6d939b38ac1cbd61e4c Mon Sep 17 00:00:00 2001
From: Lenka Doudova 
Date: Fri, 1 Jul 2016 11:00:57 +0200
Subject: [PATCH] Tests: Support of UPN for trusted domains

Basic set of tests to verify support of UPN functionality.

Test cases:
- establish trust
- verify the trust recognizes UPN
- verify AD user with UPN can be resolved
- verify AD user with UPN can authenticate
- remove trust

https://fedorahosted.org/freeipa/ticket/5354
---
ipatests/test_integration/test_trust.py | 32 
1 file changed, 32 insertions(+)

diff --git a/ipatests/test_integration/test_trust.py 
b/ipatests/test_integration/test_trust.py
index 
d662e80727b6eab3df93166d35ddbaea6a0f6f7a..e8fdc6ba68fb6275a0d7920c76ca434ed830ed84
 100644
--- a/ipatests/test_integration/test_trust.py
+++ b/ipatests/test_integration/test_trust.py
@@ -388,3 +388,35 @@ class TestExternalTrustWithRootDomain(ADTrustBase):

tasks.remove_trust_with_ad(self.master, self.ad_domain)
tasks.clear_sssd_cache(self.master)
+
+
+class TestTrustWithUPN(ADTrustBase):
+"""
+Test support of UPN for trusted domains
+"""
+def test_upn_in_nonposix_trust(self):
+""" Check that UPN is listed as trust attribute """
+result = self.master.run_command(['ipa', 'trust-show', self.ad_domain,
+  '--all', '--raw'])
+
+assert "ipantadditionalsuffixes: UPNsuffix.com" in result.stdout_text
+
+def test_upn_user_resolution_in_nonposix_trust(self):
+""" Check that user with UPN can be resolved """
+upnuser = 'upnu...@upnsuffix.com'
+result = self.master.run_command(['getent', 'passwd', upnuser])

Is there a special reason for not using pwd.getpwnam() ?

Technically -- yes. In case there was a change in the system
configuration (/etc/nsswitch.conf), then these changes wouldn't be
reflected in the application that is already using NSSWITCH interface.

However, in this particular case no change to config files is expected
so pwd.getpwnam() can be used.
--
/ Alexander Bokovoy

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

Re: [Freeipa-devel] [PATCH 0026][Tests] RFE: Support UPN for trusted domains

[Lukas Slebodnik]

On (01/07/16 11:13), Lenka Doudova wrote:
>And, of course, a patch file :)
>
>
>On 07/01/2016 11:09 AM, Lenka Doudova wrote:
>> Hi all,
>> 
>> here's patch with basic test suite for support of UPN.
>> 
>> Note: it needs to be applied on top of my patch 0025.2 (or later, if
>> there's will be more fixes to that patch).
>> 
>> 
>> Lenka
>> 
>

>From 5c8cb8727322371b7246f6d939b38ac1cbd61e4c Mon Sep 17 00:00:00 2001
>From: Lenka Doudova 
>Date: Fri, 1 Jul 2016 11:00:57 +0200
>Subject: [PATCH] Tests: Support of UPN for trusted domains
>
>Basic set of tests to verify support of UPN functionality.
>
>Test cases:
>- establish trust
>- verify the trust recognizes UPN
>- verify AD user with UPN can be resolved
>- verify AD user with UPN can authenticate
>- remove trust
>
>https://fedorahosted.org/freeipa/ticket/5354
>---
> ipatests/test_integration/test_trust.py | 32 
> 1 file changed, 32 insertions(+)
>
>diff --git a/ipatests/test_integration/test_trust.py 
>b/ipatests/test_integration/test_trust.py
>index 
>d662e80727b6eab3df93166d35ddbaea6a0f6f7a..e8fdc6ba68fb6275a0d7920c76ca434ed830ed84
> 100644
>--- a/ipatests/test_integration/test_trust.py
>+++ b/ipatests/test_integration/test_trust.py
>@@ -388,3 +388,35 @@ class TestExternalTrustWithRootDomain(ADTrustBase):
> 
> tasks.remove_trust_with_ad(self.master, self.ad_domain)
> tasks.clear_sssd_cache(self.master)
>+
>+
>+class TestTrustWithUPN(ADTrustBase):
>+"""
>+Test support of UPN for trusted domains
>+"""
>+def test_upn_in_nonposix_trust(self):
>+""" Check that UPN is listed as trust attribute """
>+result = self.master.run_command(['ipa', 'trust-show', self.ad_domain,
>+  '--all', '--raw'])
>+
>+assert "ipantadditionalsuffixes: UPNsuffix.com" in result.stdout_text
>+
>+def test_upn_user_resolution_in_nonposix_trust(self):
>+""" Check that user with UPN can be resolved """
>+upnuser = 'upnu...@upnsuffix.com'
>+result = self.master.run_command(['getent', 'passwd', upnuser])
Is there a special reason for not using pwd.getpwnam() ?

LS

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

[Freeipa-devel] FreeIPA 4.4.0 tagged

[Petr Vobornik]

FreeIPA 4.4.0 was tagged.

Release notes will follow soon.

* http://www.freeipa.org/page/Downloads#Latest_Release_-_FreeIPA_4.4.0
* http://freeipa.org/downloads/src/freeipa-4.4.0.tar.gz
SHA1: 441ef8cb2b0ac103723d03b0478da641d697e104
MD5: 078697b25e02361fca37d00a1144130d
-- 
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

[Freeipa-devel] [PATCH 0178] Fix incorrect check for principal type when evaluating CA ACLs

[Martin Babinsky]

Fixing first regression caused by principal alias work.

Thanks Petr Spacek for finding it.

--
Martin^3 Babinsky
From da8e18addcc172777977e50f2d4d34603243077f Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Fri, 1 Jul 2016 11:55:47 +0200
Subject: [PATCH] Fix incorrect check for principal type when evaluating CA
 ACLs

This error prevented hosts to request certificates for themselves.
---
 ipaserver/plugins/caacl.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/ipaserver/plugins/caacl.py b/ipaserver/plugins/caacl.py
index 3f813a7efb9e554abcb8dd2946eea73065c93414..9a60f7e27809c4f41b160647efafde94dbe90bf0 100644
--- a/ipaserver/plugins/caacl.py
+++ b/ipaserver/plugins/caacl.py
@@ -64,8 +64,10 @@ def _acl_make_request(principal_type, principal, ca_id, profile_id):
 req = pyhbac.HbacRequest()
 req.targethost.name = ca_id
 req.service.name = profile_id
-if principal_type == 'user' or principal_type == 'host':
+if principal_type == 'user':
 req.user.name = principal.username
+elif principal_type == 'host':
+req.user.name = principal.hostname
 elif principal_type == 'service':
 req.user.name = unicode(principal)
 groups = []
-- 
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 0148] client-install: log exceptions from certmonger.request_cer

[Petr Spacek]

Hello,

client-install: log exceptions from certmonger.request_cert

-- 
Petr^2 Spacek
From 7082057a37e00c2745d6e5561d78bd5ae307e96c Mon Sep 17 00:00:00 2001
From: Petr Spacek 
Date: Fri, 1 Jul 2016 11:57:35 +0200
Subject: [PATCH] client-install: log exceptions from certmonger.request_cert

---
 client/ipa-client-install | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/client/ipa-client-install b/client/ipa-client-install
index cee202f89e0f40f4b7ee77e5c38a2c7d50e0dee9..8546ff8b0dbea0f28fba12a00c2ee1868ec7c3c6 100755
--- a/client/ipa-client-install
+++ b/client/ipa-client-install
@@ -1173,9 +1173,9 @@ def configure_certmonger(fstore, subject_base, cli_realm, hostname, options,
 subject=subject,
 principal=principal,
 passwd_fname=passwd_fname)
-except Exception:
-root_logger.error("%s request for host certificate failed",
-  cmonger.service_name)
+except Exception as ex:
+root_logger.error("%s request for host certificate failed: %s",
+  cmonger.service_name, ex)
 
 def configure_sssd_conf(fstore, cli_realm, cli_domain, cli_server, options, client_domain, client_hostname):
 try:
-- 
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] pwpolicy: Do not expire passwords when maxlife is set to 0 (infinity).

[thierry bordaz]

On 07/01/2016 11:31 AM, David Kupka wrote:

On 01/07/16 11:22, thierry bordaz wrote:



On 07/01/2016 10:46 AM, David Kupka wrote:

Hello Thierry!

Thanks for looking into it. I will try to answer your questions and
comments inline.

On 01/07/16 10:26, thierry bordaz wrote:

Hi David,

The patch looks good but being not familiar with that code, my 
comments

may be absolutely wrong

In ipadb_get_pwd_expiration, if it is not 'self' we set
'*export=mod_time'.
If for some reason 'mod_time==0', it has now a specific meaning 'not
expiring' . Does it match the comment '* not 'self', so reset */'


mod_time should be timestamp of the modification operation. So
mod_time == 0 should happen only when the change was performed in the
very beginning of 70's.


Right. My fault I did not understand that code.




In ipadb_entry_to_mods, it deletes krbPasswordExpiration. But just
before it adds in the mods krbPasswordExpiration=0 or
krbPasswordExpiration=entry->pw_expiration
Could we skip those mods if entry->pw_expiration==0 or 
expire_time==0 ?


That is exactly what I thought. But in that part of code I have no
chance to check if the attribute is present in the entry or not. Also
I can't catch and ignore the resulting error when deleting nonexistent
attribute. Here only LDAPMod structures are filled but the execution
is done in an other part of code.
So I choose the easy path and always set the attribute and delete it
right after if necessary.


I think there is something a bit strange here.
To be able to delete the attribute we first need to set it to a specific
value then deleting the value we manage to delete the attribute.
I did not find a routine like ipa_get_ldap_mod_delattr. With such
routine I wonder if we could to something like:

if (entry->pw_expiration == 0) {
kerr = ipadb_get_ldap_mod_delattr(imods,
"krbPasswordExpiration");
} else {

   kerr = ipadb_get_ldap_mod_time(imods,
"krbPasswordExpiration",
entry->pw_expiration,
   mod_op);
}


Instead of

kerr = ipadb_get_ldap_mod_time(imods,
   "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);
}





At some point I have added such routine and tried same approach as you 
do. But when the krbPasswordExpiration is not present in the entry 
(user already has non-expiring password) there is an error later when 
the mods are applied (err=16 ~ no such attribute).


When I found this I decided to always LDAP_MOD_REPLACE the attribute 
(replace operation does not fail when the attribute is missing). And 
then remove it when it shouldn't be there. After that I decided to 
remove my _del_attr routine because I didn't want to add new 
unnecessary code.


Yes I agree and the code is working fine !
My concern is that ipadb_mods_new is trying to find empty mod slot. 
Hopefully there is no such empty slot currently.

But in theory the mods can have a random order and I think it is dangerous.
Adding a value to be able to delete the attribute afterward can fail if 
the ops are done in the opposite order.








In ipapwd_SetPassword, ipapwd_post_modadd, same remark as above.

Something that I am not sure is what is the expected relation between
passwordexpirationtime and krbPasswordExpiration


I'm not sure either. I think we don't use passwordexpirationtime
attribute.

I think they should be somehow linked, in fact it is looking it is what
happen in ipapwd_write_krb_keys.
But it looks it happens only when the krb keys are created.


Probablby, I don't recall seeing passwordexpirationtime attribute used 
anywhere.






thanks
thierry

On 06/30/2016 09:34 PM, David Kupka wrote:

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.

Re: [Freeipa-devel] [PATCH 031] RedHatCAService should wait for local Dogtag instance

[Petr Spacek]

On 1.7.2016 11:17, Petr Spacek wrote:
> On 1.7.2016 11:04, Christian Heimes wrote:
>> On 2016-07-01 10:59, Petr Spacek wrote:
>>> On 1.7.2016 10:55, Christian Heimes wrote:
 On 2016-07-01 10:48, Petr Spacek wrote:
> On 1.7.2016 10:42, Christian Heimes wrote:
>> RedHatCAService.wait_until_running() uses dogtag.ca_status() to make a
>> HTTP(s) request to Dogtag in order to check if /ca/admin/ca/getStatus
>> returns OK. The ca_status() function defaults to api.env.ca_host as
>> host.
>>
>> On a replica without CA ca_host is a remote host (e.g. master's
>> FQDN). ipa-ca-install waits for master:8080 instead of replica:8080,
>> which might be blocked by a firewall.
>>
>> https://fedorahosted.org/freeipa/ticket/6016
>
> Interesting. How it happens that replica without CA is calling 
> RedHatCAService?
>
> Also, why replica should be waiting for CA if it is not installed?
>
> I'm confused.

 There is a hint in the last sentence: ipa-ca-install

 The patch fixes ipa-ca-install on replicas. Right now ipa-ca-install
 doesn't wait for the local Dogtag to come up but connects to a remote
 Dogtag to check if it's up. It uses 8443 or 8080, which might be
 blocked. In my test setup I have both ports blocked so ipa-ca-install
 never succeeds.
>>>
>>> Oh, I missed that, thanks!
>>>
>>> Isn't the root cause that ipa.env.ca_host does not get updated during
>>> ipa-ca-install?
>>
>> Been there, tried it, didn't work:
>> https://fedorahosted.org/freeipa/ticket/6016#comment:1
> 
> I understand that it does not work right now but it does not mean that it is
> an actual problem in api.env :-)
> 
> Anyway, I'm testing your patch but I'm not sure we can get it into 4.4.0 as
> Petr^1 is about to push the RELEASE button any minute now.
> 
> Petr^2 Spacek
> 
>> It just doesn't make sense that RedHatCAService should ever check a
>> remote instance. The rest of the class is about the local systemd
>> service. As soon as we have sd_notify
>> https://fedorahosted.org/pki/ticket/1233 implemented, we can use systemd
>> to wait for Dogtag.

It seems to work but ipa-client-install blows up on certificate request.

# getcert list
Number of certificates and requests being tracked: 1.
Request ID '20160701093734':
status: CA_UNREACHABLE
ca-error: Server at
https://vm-058-082.abc.idm.lab.eng.brq.redhat.com/ipa/xml failed request, will
retry: 903 (RPC failed at server.  an internal error has occurred).
stuck: no
key pair storage: type=NSSDB,location='/etc/ipa/nssdb',nickname='Local
IPA host',token='NSS Certificate DB',pinfile='/etc/ipa/nssdb/pwdfile.txt'
certificate: type=NSSDB,location='/etc/ipa/nssdb',nickname='Local IPA
host'
CA: IPA
issuer:
subject:
expires: unknown
pre-save command:
post-save command:
track: yes
auto-renew: yes

error log on the server:

[Fri Jul 01 11:37:34.294677 2016] [wsgi:error] [pid 38273] ipa: INFO:
[jsonserver_kerb]
host/vm-046.abc.idm.lab.eng.brq.redhat@dom-058-082.abc.idm.lab.eng.brq.redhat.com:
host_mod(u'vm-046.abc.idm.lab.eng.brq.redhat.com', ipasshpubkey=(u'ssh-rsa
B3NzaC1yc2EDAQABAAABAQCtrWFHeOF6UxI/DNdlLsUUazTpol2sRqQgbZplpkB9t/HUSjUHq0OY1mwaUfxvJp/E9yDmuHZgUgzKMSAdUf2apwFm5bw3T7qSdJ0Y7hC9vG0v6kLT0EaPuQmfJ8Rt4xOyva9htKbzkxs9Kr0ujB6V4u41ZZW2oevqtGunC2+aCxkQzd42we0c47ypxnvl8gGAa76CDXenGaChPKSfeEMddnhFvjGfkSyqjD+dCxBF+IyTRDPtt6f5iF80lfv/559rsKYlHdbbgv30i5C/F2DzaB011BmcQwK1eWSGWsEWVFtQKNMdahTl2IMgvZwHcaw8TMqgqqgZ7ZZ6lMR+UA8l',
u'ecdsa-sha2-nistp256
E2VjZHNhLXNoYTItbmlzdHAyNTYIbmlzdHAyNTYAAABBBHkoeGOzfQzqYOGQs2bdgL0jOBul+/eTBZ0HBM8HW3Wb5O15Fv3rt8jRp+xdSQcdG3DV5yPfjd66Fyz5hCTKS6s=',
u'ssh-ed25519
C3NzaC1lZDI1NTE5IH5/uXvdJ1l+uTAk0rgbjKeTBx9HRWk7w+xJLHMt/yRx'),
updatedns=False, version=u'2.26'): SUCCESS
[Fri Jul 01 11:37:37.961175 2016] [wsgi:error] [pid 38272] ipa: ERROR:
non-public: ValueError: User name is defined only for user and enterprise
principals
[Fri Jul 01 11:37:37.961220 2016] [wsgi:error] [pid 38272] Traceback (most
recent call last):
[Fri Jul 01 11:37:37.961224 2016] [wsgi:error] [pid 38272]   File
"/usr/lib/python2.7/site-packages/ipaserver/rpcserver.py", line 352, in
wsgi_execute
[Fri Jul 01 11:37:37.961226 2016] [wsgi:error] [pid 38272] result =
self.Command[name](*args, **options)
[Fri Jul 01 11:37:37.961229 2016] [wsgi:error] [pid 38272]   File
"/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 447, in __call__
[Fri Jul 01 11:37:37.961259 2016] [wsgi:error] [pid 38272] return
self.__do_call(*args, **options)
[Fri Jul 01 11:37:37.961262 2016] [wsgi:error] [pid 38272]   File
"/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 475, in __do_call
[Fri Jul 01 11:37:37.961265 2016] [wsgi:error] [pid 38272] ret =
self.run(*args, **options)
[Fri Jul 01 11:37:37.961267 2016] [wsgi:error] [pid 38272]   File

Re: [Freeipa-devel] [PATCH] pwpolicy: Do not expire passwords when maxlife is set to 0 (infinity).

[David Kupka]

On 01/07/16 11:22, thierry bordaz wrote:



On 07/01/2016 10:46 AM, David Kupka wrote:

Hello Thierry!

Thanks for looking into it. I will try to answer your questions and
comments inline.

On 01/07/16 10:26, thierry bordaz wrote:

Hi David,

The patch looks good but being not familiar with that code, my comments
may be absolutely wrong

In ipadb_get_pwd_expiration, if it is not 'self' we set
'*export=mod_time'.
If for some reason 'mod_time==0', it has now a specific meaning 'not
expiring' . Does it match the comment '* not 'self', so reset */'


mod_time should be timestamp of the modification operation. So
mod_time == 0 should happen only when the change was performed in the
very beginning of 70's.


Right. My fault I did not understand that code.




In ipadb_entry_to_mods, it deletes krbPasswordExpiration. But just
before it adds in the mods krbPasswordExpiration=0 or
krbPasswordExpiration=entry->pw_expiration
Could we skip those mods if entry->pw_expiration==0 or expire_time==0 ?


That is exactly what I thought. But in that part of code I have no
chance to check if the attribute is present in the entry or not. Also
I can't catch and ignore the resulting error when deleting nonexistent
attribute. Here only LDAPMod structures are filled but the execution
is done in an other part of code.
So I choose the easy path and always set the attribute and delete it
right after if necessary.


I think there is something a bit strange here.
To be able to delete the attribute we first need to set it to a specific
value then deleting the value we manage to delete the attribute.
I did not find a routine like ipa_get_ldap_mod_delattr. With such
routine I wonder if we could to something like:

if (entry->pw_expiration == 0) {
kerr = ipadb_get_ldap_mod_delattr(imods,
"krbPasswordExpiration");
} else {

   kerr = ipadb_get_ldap_mod_time(imods,
"krbPasswordExpiration",
entry->pw_expiration,
   mod_op);
}


Instead of

kerr = ipadb_get_ldap_mod_time(imods,
   "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);
}





At some point I have added such routine and tried same approach as you 
do. But when the krbPasswordExpiration is not present in the entry (user 
already has non-expiring password) there is an error later when the mods 
are applied (err=16 ~ no such attribute).


When I found this I decided to always LDAP_MOD_REPLACE the attribute 
(replace operation does not fail when the attribute is missing). And 
then remove it when it shouldn't be there. After that I decided to 
remove my _del_attr routine because I didn't want to add new unnecessary 
code.






In ipapwd_SetPassword, ipapwd_post_modadd, same remark as above.

Something that I am not sure is what is the expected relation between
passwordexpirationtime and krbPasswordExpiration


I'm not sure either. I think we don't use passwordexpirationtime
attribute.

I think they should be somehow linked, in fact it is looking it is what
happen in ipapwd_write_krb_keys.
But it looks it happens only when the krb keys are created.


Probablby, I don't recall seeing passwordexpirationtime attribute used 
anywhere.






thanks
thierry

On 06/30/2016 09:34 PM, David Kupka wrote:

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 

Re: [Freeipa-devel] [PATCH 031] RedHatCAService should wait for local Dogtag instance

[Christian Heimes]

On 2016-07-01 11:17, Petr Spacek wrote:
> On 1.7.2016 11:04, Christian Heimes wrote:
>> On 2016-07-01 10:59, Petr Spacek wrote:
>>> On 1.7.2016 10:55, Christian Heimes wrote:
 On 2016-07-01 10:48, Petr Spacek wrote:
> On 1.7.2016 10:42, Christian Heimes wrote:
>> RedHatCAService.wait_until_running() uses dogtag.ca_status() to make a
>> HTTP(s) request to Dogtag in order to check if /ca/admin/ca/getStatus
>> returns OK. The ca_status() function defaults to api.env.ca_host as
>> host.
>>
>> On a replica without CA ca_host is a remote host (e.g. master's
>> FQDN). ipa-ca-install waits for master:8080 instead of replica:8080,
>> which might be blocked by a firewall.
>>
>> https://fedorahosted.org/freeipa/ticket/6016
>
> Interesting. How it happens that replica without CA is calling 
> RedHatCAService?
>
> Also, why replica should be waiting for CA if it is not installed?
>
> I'm confused.

 There is a hint in the last sentence: ipa-ca-install

 The patch fixes ipa-ca-install on replicas. Right now ipa-ca-install
 doesn't wait for the local Dogtag to come up but connects to a remote
 Dogtag to check if it's up. It uses 8443 or 8080, which might be
 blocked. In my test setup I have both ports blocked so ipa-ca-install
 never succeeds.
>>>
>>> Oh, I missed that, thanks!
>>>
>>> Isn't the root cause that ipa.env.ca_host does not get updated during
>>> ipa-ca-install?
>>
>> Been there, tried it, didn't work:
>> https://fedorahosted.org/freeipa/ticket/6016#comment:1
> 
> I understand that it does not work right now but it does not mean that it is
> an actual problem in api.env :-)
> 
> Anyway, I'm testing your patch but I'm not sure we can get it into 4.4.0 as
> Petr^1 is about to push the RELEASE button any minute now.

Thanks Petr.

I talked to Petr^1 on IRC. Since 4.2 and 4.3 are also affected, it's
better not to rush this patch. It won't make it into 4.4.0.

Christian




signature.asc
Description: OpenPGP digital signature
-- 
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 0549] Translations IPA 4.4.0

[Martin Babinsky]

On 07/01/2016 10:34 AM, Martin Basti wrote:



On 01.07.2016 09:27, Martin Basti wrote:

Patch attached.




Updated patch

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

[Petr Vobornik]

On 07/01/2016 10:46 AM, David Kupka wrote:
> Hello Thierry!
> 
> Thanks for looking into it. I will try to answer your questions and
> comments inline.
> 
> On 01/07/16 10:26, thierry bordaz wrote:
>> Hi David,
>>
>> The patch looks good but being not familiar with that code, my comments
>> may be absolutely wrong
>>
>> In ipadb_get_pwd_expiration, if it is not 'self' we set
>> '*export=mod_time'.
>> If for some reason 'mod_time==0', it has now a specific meaning 'not
>> expiring' . Does it match the comment '* not 'self', so reset */'
> 
> mod_time should be timestamp of the modification operation. So mod_time
> == 0 should happen only when the change was performed in the very
> beginning of 70's.
> 
>>
>> In ipadb_entry_to_mods, it deletes krbPasswordExpiration. But just
>> before it adds in the mods krbPasswordExpiration=0 or
>> krbPasswordExpiration=entry->pw_expiration
>> Could we skip those mods if entry->pw_expiration==0 or expire_time==0 ?
> 
> That is exactly what I thought. But in that part of code I have no
> chance to check if the attribute is present in the entry or not. Also I
> can't catch and ignore the resulting error when deleting nonexistent
> attribute. Here only LDAPMod structures are filled but the execution is
> done in an other part of code.
> So I choose the easy path and always set the attribute and delete it
> right after if necessary.
> 
>>
>> In ipapwd_SetPassword, ipapwd_post_modadd, same remark as above.
>>
>> Something that I am not sure is what is the expected relation between
>> passwordexpirationtime and krbPasswordExpiration
> 
> I'm not sure either. I think we don't use passwordexpirationtime attribute.
> 
>>
>> thanks
>> thierry

I don't see a strict NACK here. Given pvomacka's functional ACK, I have
pushed it. We can fix corner cases later.

Pushed to master: d2cb9ed327ee4003598d5e45d80ab7918b89eeed

>>
>> On 06/30/2016 09:34 PM, David Kupka wrote:
>>> 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
>>>
>>
-- 
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] pwpolicy: Do not expire passwords when maxlife is set to 0 (infinity).

[thierry bordaz]

On 07/01/2016 10:46 AM, David Kupka wrote:

Hello Thierry!

Thanks for looking into it. I will try to answer your questions and 
comments inline.


On 01/07/16 10:26, thierry bordaz wrote:

Hi David,

The patch looks good but being not familiar with that code, my comments
may be absolutely wrong

In ipadb_get_pwd_expiration, if it is not 'self' we set 
'*export=mod_time'.

If for some reason 'mod_time==0', it has now a specific meaning 'not
expiring' . Does it match the comment '* not 'self', so reset */'


mod_time should be timestamp of the modification operation. So 
mod_time == 0 should happen only when the change was performed in the 
very beginning of 70's.


Right. My fault I did not understand that code.




In ipadb_entry_to_mods, it deletes krbPasswordExpiration. But just
before it adds in the mods krbPasswordExpiration=0 or
krbPasswordExpiration=entry->pw_expiration
Could we skip those mods if entry->pw_expiration==0 or expire_time==0 ?


That is exactly what I thought. But in that part of code I have no 
chance to check if the attribute is present in the entry or not. Also 
I can't catch and ignore the resulting error when deleting nonexistent 
attribute. Here only LDAPMod structures are filled but the execution 
is done in an other part of code.
So I choose the easy path and always set the attribute and delete it 
right after if necessary.


I think there is something a bit strange here.
To be able to delete the attribute we first need to set it to a specific 
value then deleting the value we manage to delete the attribute.
I did not find a routine like ipa_get_ldap_mod_delattr. With such 
routine I wonder if we could to something like:


if (entry->pw_expiration == 0) {
kerr = ipadb_get_ldap_mod_delattr(imods,
"krbPasswordExpiration");
} else {

   kerr = ipadb_get_ldap_mod_time(imods,
"krbPasswordExpiration",
entry->pw_expiration,
   mod_op);
}


Instead of

kerr = ipadb_get_ldap_mod_time(imods,
   "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);
}







In ipapwd_SetPassword, ipapwd_post_modadd, same remark as above.

Something that I am not sure is what is the expected relation between
passwordexpirationtime and krbPasswordExpiration


I'm not sure either. I think we don't use passwordexpirationtime 
attribute.
I think they should be somehow linked, in fact it is looking it is what 
happen in ipapwd_write_krb_keys.

But it looks it happens only when the krb keys are created.




thanks
thierry

On 06/30/2016 09:34 PM, David Kupka wrote:

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









--
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 031] RedHatCAService should wait for local Dogtag instance

[Petr Spacek]

On 1.7.2016 11:04, Christian Heimes wrote:
> On 2016-07-01 10:59, Petr Spacek wrote:
>> On 1.7.2016 10:55, Christian Heimes wrote:
>>> On 2016-07-01 10:48, Petr Spacek wrote:
 On 1.7.2016 10:42, Christian Heimes wrote:
> RedHatCAService.wait_until_running() uses dogtag.ca_status() to make a
> HTTP(s) request to Dogtag in order to check if /ca/admin/ca/getStatus
> returns OK. The ca_status() function defaults to api.env.ca_host as
> host.
>
> On a replica without CA ca_host is a remote host (e.g. master's
> FQDN). ipa-ca-install waits for master:8080 instead of replica:8080,
> which might be blocked by a firewall.
>
> https://fedorahosted.org/freeipa/ticket/6016

 Interesting. How it happens that replica without CA is calling 
 RedHatCAService?

 Also, why replica should be waiting for CA if it is not installed?

 I'm confused.
>>>
>>> There is a hint in the last sentence: ipa-ca-install
>>>
>>> The patch fixes ipa-ca-install on replicas. Right now ipa-ca-install
>>> doesn't wait for the local Dogtag to come up but connects to a remote
>>> Dogtag to check if it's up. It uses 8443 or 8080, which might be
>>> blocked. In my test setup I have both ports blocked so ipa-ca-install
>>> never succeeds.
>>
>> Oh, I missed that, thanks!
>>
>> Isn't the root cause that ipa.env.ca_host does not get updated during
>> ipa-ca-install?
> 
> Been there, tried it, didn't work:
> https://fedorahosted.org/freeipa/ticket/6016#comment:1

I understand that it does not work right now but it does not mean that it is
an actual problem in api.env :-)

Anyway, I'm testing your patch but I'm not sure we can get it into 4.4.0 as
Petr^1 is about to push the RELEASE button any minute now.

Petr^2 Spacek

> It just doesn't make sense that RedHatCAService should ever check a
> remote instance. The rest of the class is about the local systemd
> service. As soon as we have sd_notify
> https://fedorahosted.org/pki/ticket/1233 implemented, we can use systemd
> to wait for Dogtag.

-- 
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 0026][Tests] RFE: Support UPN for trusted domains

[Lenka Doudova]

And, of course, a patch file :)


On 07/01/2016 11:09 AM, Lenka Doudova wrote:

Hi all,

here's patch with basic test suite for support of UPN.

Note: it needs to be applied on top of my patch 0025.2 (or later, if 
there's will be more fixes to that patch).



Lenka



From 5c8cb8727322371b7246f6d939b38ac1cbd61e4c Mon Sep 17 00:00:00 2001
From: Lenka Doudova 
Date: Fri, 1 Jul 2016 11:00:57 +0200
Subject: [PATCH] Tests: Support of UPN for trusted domains

Basic set of tests to verify support of UPN functionality.

Test cases:
- establish trust
- verify the trust recognizes UPN
- verify AD user with UPN can be resolved
- verify AD user with UPN can authenticate
- remove trust

https://fedorahosted.org/freeipa/ticket/5354
---
 ipatests/test_integration/test_trust.py | 32 
 1 file changed, 32 insertions(+)

diff --git a/ipatests/test_integration/test_trust.py b/ipatests/test_integration/test_trust.py
index d662e80727b6eab3df93166d35ddbaea6a0f6f7a..e8fdc6ba68fb6275a0d7920c76ca434ed830ed84 100644
--- a/ipatests/test_integration/test_trust.py
+++ b/ipatests/test_integration/test_trust.py
@@ -388,3 +388,35 @@ class TestExternalTrustWithRootDomain(ADTrustBase):
 
 tasks.remove_trust_with_ad(self.master, self.ad_domain)
 tasks.clear_sssd_cache(self.master)
+
+
+class TestTrustWithUPN(ADTrustBase):
+"""
+Test support of UPN for trusted domains
+"""
+def test_upn_in_nonposix_trust(self):
+""" Check that UPN is listed as trust attribute """
+result = self.master.run_command(['ipa', 'trust-show', self.ad_domain,
+  '--all', '--raw'])
+
+assert "ipantadditionalsuffixes: UPNsuffix.com" in result.stdout_text
+
+def test_upn_user_resolution_in_nonposix_trust(self):
+""" Check that user with UPN can be resolved """
+upnuser = 'upnu...@upnsuffix.com'
+result = self.master.run_command(['getent', 'passwd', upnuser])
+
+# result will contain AD domain, not UPN
+upnuser_regex = "^upnuser@{0}:\*:(\d+):(\d+):UPN User:/:$".format(
+self.ad_domain)
+assert re.search(upnuser_regex, result.stdout_text)
+
+def test_upn_user_authentication(self):
+""" Check that AD user with UPN can authenticate in IPA """
+self.master.run_command(['systemctl', 'restart', 'krb5kdc'])
+self.master.run_command(['kinit', '-C', '-E', 'upnu...@upnsuffix.com'],
+stdin_text='Secret123456')
+
+def test_remove_nonposix_trust(self):
+tasks.remove_trust_with_ad(self.master, self.ad_domain)
+tasks.clear_sssd_cache(self.master)
-- 
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] 0085 Fix upgrade when Dogtag also upgraded from 10.2 -> 10.3

[Petr Vobornik]

On 07/01/2016 11:02 AM, Martin Babinsky wrote:
> On 06/30/2016 01:16 PM, Fraser Tweedale wrote:
>> Hullo,
>>
>> The attached patch fixes
>> https://fedorahosted.org/freeipa/ticket/6011.
>>
>> Cheers,
>> Fraser
>>
>>
>>
> ACK
> 

Pushed to master: 3691e39a62da5134f911f6a798f79a3a2ae0c025

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

[Freeipa-devel] [PATCH 0026][Tests] RFE: Support UPN for trusted domains

[Lenka Doudova]

Hi all,

here's patch with basic test suite for support of UPN.

Note: it needs to be applied on top of my patch 0025.2 (or later, if 
there's will be more fixes to that patch).



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 558] Allow disabling requireing preauth by default for Service Principal Names

[Martin Babinsky]

On 07/01/2016 10:38 AM, Petr Vobornik wrote:

On 03/08/2016 06:02 PM, Martin Babinsky wrote:

On 03/08/2016 05:50 PM, Simo Sorce wrote:

On Tue, 2016-03-08 at 17:20 +0100, Martin Babinsky wrote:

On 03/08/2016 05:00 PM, Simo Sorce wrote:

On Tue, 2016-03-08 at 16:51 +0100, Martin Babinsky wrote:

On 03/08/2016 04:49 PM, Simo Sorce wrote:

On Fri, 2015-12-04 at 14:23 +0100, Martin Babinsky wrote:

On 12/01/2015 10:08 PM, Simo Sorce wrote:

On Tue, 2015-12-01 at 15:59 +0100, Martin Babinsky wrote:

On 11/30/2015 07:42 PM, Simo Sorce wrote:

On Wed, 2015-11-25 at 10:33 +0100, Martin Babinsky wrote:

On 11/24/2015 10:20 PM, Simo Sorce wrote:

This addresses #3860, giving admins the option to not
require preauth
for Hosts and services.

I did not add this option by default, although it does
reduce the load
on the KDC as well as speed up TGT acquisition for service
principal
accounts that acquire TGTs.

Tested and working as expected (SPNs are not returned
PREAUTH_NEEDED
error while normal users are).

HTH,
Simo.




Hi Simo,

I was not able to apply the patch on current master branch:

"""
git am
../review/ssorce/3860/freeipa-simo-558-1-Allow-admins-to-disable-preauth-for-SPNs.patch

-3

Applying: Allow admins to disable preauth for SPNs.
error: invalid object 100644
a6b4d4349a9ac6de453d9ad3c679ec32add4e43b
for 'ipalib/plugins/config.py'
fatal: git-write-tree: error building trees
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 Allow admins to disable preauth for SPNs.
"""

It seems that I nedd to apply some of your other patches
first (which one?)


Sorry did not see this question earlier, it requires 556 and
557, I just
bumped that thread.

Simo.


It seems that I need something else, patch 556-2 applies
cleanly, but
patch 557-3 fails with http://fpaste.org/296230/89819431/ on
both master
and 4-2 branch.



Rebased 556,557 in their thread, and here is the rebase for 558
on top
of them.

Simo.



ACK. I'm afraid that this patch and 556, 557 will require another
round
of rebase before pushing, though.


Rebased on top of master (not on 556/557) per Petr's request.

Simo.




NACK, if you do API changes please increment API version in VERSION.


Why wasn't this a problem in the previous ACK ?

Simo.



Probably because I missed it, sorry.



Fixed.

Simo.



Thanks, ACK.



was this pushed?


Yes I see 3e45c9be0aefb03751665a951f426ac59c50a551 in master.

--
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] 0085 Fix upgrade when Dogtag also upgraded from 10.2 -> 10.3

[Martin Babinsky]

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

Hullo,

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

Cheers,
Fraser




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 031] RedHatCAService should wait for local Dogtag instance

[Petr Spacek]

On 1.7.2016 10:55, Christian Heimes wrote:
> On 2016-07-01 10:48, Petr Spacek wrote:
>> On 1.7.2016 10:42, Christian Heimes wrote:
>>> RedHatCAService.wait_until_running() uses dogtag.ca_status() to make a
>>> HTTP(s) request to Dogtag in order to check if /ca/admin/ca/getStatus
>>> returns OK. The ca_status() function defaults to api.env.ca_host as
>>> host.
>>>
>>> On a replica without CA ca_host is a remote host (e.g. master's
>>> FQDN). ipa-ca-install waits for master:8080 instead of replica:8080,
>>> which might be blocked by a firewall.
>>>
>>> https://fedorahosted.org/freeipa/ticket/6016
>>
>> Interesting. How it happens that replica without CA is calling 
>> RedHatCAService?
>>
>> Also, why replica should be waiting for CA if it is not installed?
>>
>> I'm confused.
> 
> There is a hint in the last sentence: ipa-ca-install
> 
> The patch fixes ipa-ca-install on replicas. Right now ipa-ca-install
> doesn't wait for the local Dogtag to come up but connects to a remote
> Dogtag to check if it's up. It uses 8443 or 8080, which might be
> blocked. In my test setup I have both ports blocked so ipa-ca-install
> never succeeds.

Oh, I missed that, thanks!

Isn't the root cause that ipa.env.ca_host does not get updated during
ipa-ca-install?

-- 
Petr^2 Spacek

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

Re: [Freeipa-devel] [PATCH 031] RedHatCAService should wait for local Dogtag instance

[Christian Heimes]

On 2016-07-01 10:48, Petr Spacek wrote:
> On 1.7.2016 10:42, Christian Heimes wrote:
>> RedHatCAService.wait_until_running() uses dogtag.ca_status() to make a
>> HTTP(s) request to Dogtag in order to check if /ca/admin/ca/getStatus
>> returns OK. The ca_status() function defaults to api.env.ca_host as
>> host.
>>
>> On a replica without CA ca_host is a remote host (e.g. master's
>> FQDN). ipa-ca-install waits for master:8080 instead of replica:8080,
>> which might be blocked by a firewall.
>>
>> https://fedorahosted.org/freeipa/ticket/6016
> 
> Interesting. How it happens that replica without CA is calling 
> RedHatCAService?
> 
> Also, why replica should be waiting for CA if it is not installed?
> 
> I'm confused.

There is a hint in the last sentence: ipa-ca-install

The patch fixes ipa-ca-install on replicas. Right now ipa-ca-install
doesn't wait for the local Dogtag to come up but connects to a remote
Dogtag to check if it's up. It uses 8443 or 8080, which might be
blocked. In my test setup I have both ports blocked so ipa-ca-install
never succeeds.

Christian




signature.asc
Description: OpenPGP digital signature
-- 
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 031] RedHatCAService should wait for local Dogtag instance

[Petr Spacek]

On 1.7.2016 10:42, Christian Heimes wrote:
> RedHatCAService.wait_until_running() uses dogtag.ca_status() to make a
> HTTP(s) request to Dogtag in order to check if /ca/admin/ca/getStatus
> returns OK. The ca_status() function defaults to api.env.ca_host as
> host.
> 
> On a replica without CA ca_host is a remote host (e.g. master's
> FQDN). ipa-ca-install waits for master:8080 instead of replica:8080,
> which might be blocked by a firewall.
> 
> https://fedorahosted.org/freeipa/ticket/6016

Interesting. How it happens that replica without CA is calling RedHatCAService?

Also, why replica should be waiting for CA if it is not installed?

I'm confused.

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

[David Kupka]

Hello Thierry!

Thanks for looking into it. I will try to answer your questions and 
comments inline.


On 01/07/16 10:26, thierry bordaz wrote:

Hi David,

The patch looks good but being not familiar with that code, my comments
may be absolutely wrong

In ipadb_get_pwd_expiration, if it is not 'self' we set '*export=mod_time'.
If for some reason 'mod_time==0', it has now a specific meaning 'not
expiring' . Does it match the comment '* not 'self', so reset */'


mod_time should be timestamp of the modification operation. So mod_time 
== 0 should happen only when the change was performed in the very 
beginning of 70's.




In ipadb_entry_to_mods, it deletes krbPasswordExpiration. But just
before it adds in the mods krbPasswordExpiration=0 or
krbPasswordExpiration=entry->pw_expiration
Could we skip those mods if entry->pw_expiration==0 or expire_time==0 ?


That is exactly what I thought. But in that part of code I have no 
chance to check if the attribute is present in the entry or not. Also I 
can't catch and ignore the resulting error when deleting nonexistent 
attribute. Here only LDAPMod structures are filled but the execution is 
done in an other part of code.
So I choose the easy path and always set the attribute and delete it 
right after if necessary.




In ipapwd_SetPassword, ipapwd_post_modadd, same remark as above.

Something that I am not sure is what is the expected relation between
passwordexpirationtime and krbPasswordExpiration


I'm not sure either. I think we don't use passwordexpirationtime attribute.



thanks
thierry

On 06/30/2016 09:34 PM, David Kupka wrote:

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

--
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 031] RedHatCAService should wait for local Dogtag instance

[Christian Heimes]

RedHatCAService.wait_until_running() uses dogtag.ca_status() to make a
HTTP(s) request to Dogtag in order to check if /ca/admin/ca/getStatus
returns OK. The ca_status() function defaults to api.env.ca_host as
host.

On a replica without CA ca_host is a remote host (e.g. master's
FQDN). ipa-ca-install waits for master:8080 instead of replica:8080,
which might be blocked by a firewall.

https://fedorahosted.org/freeipa/ticket/6016
From 134f639aadad1b63e8715ec05fa06b53a3f12e74 Mon Sep 17 00:00:00 2001
From: Christian Heimes 
Date: Fri, 1 Jul 2016 10:21:06 +0200
Subject: [PATCH] RedHatCAService should wait for local Dogtag instance

RedHatCAService.wait_until_running() uses dogtag.ca_status() to make a
HTTP(s) request to Dogtag in order to check if /ca/admin/ca/getStatus
returns OK. The ca_status() function defaults to api.env.ca_host as
host.

On a replica without CA ca_host is a remote host (e.g. master's
FQDN). ipa-ca-install waits for master:8080 instead of replica:8080,
which might be blocked by a firewall.

https://fedorahosted.org/freeipa/ticket/6016
---
 ipaplatform/redhat/services.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ipaplatform/redhat/services.py b/ipaplatform/redhat/services.py
index 849737059d54df5af47ae288ef97b933d9e869fe..24325347c7d9183e2ecdd8d00bfa52729463fea3 100644
--- a/ipaplatform/redhat/services.py
+++ b/ipaplatform/redhat/services.py
@@ -199,7 +199,8 @@ class RedHatCAService(RedHatService):
 op_timeout = time.time() + timeout
 while time.time() < op_timeout:
 try:
-status = dogtag.ca_status()
+# check status of CA instance on this host, not remote ca_host
+status = dogtag.ca_status(api.env.host)
 except Exception as e:
 status = 'check interrupted due to error: %s' % e
 root_logger.debug('The CA status is: %s' % status)
-- 
2.7.4



signature.asc
Description: OpenPGP digital signature
-- 
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 558] Allow disabling requireing preauth by default for Service Principal Names

[Petr Vobornik]

On 03/08/2016 06:02 PM, Martin Babinsky wrote:
> On 03/08/2016 05:50 PM, Simo Sorce wrote:
>> On Tue, 2016-03-08 at 17:20 +0100, Martin Babinsky wrote:
>>> On 03/08/2016 05:00 PM, Simo Sorce wrote:
 On Tue, 2016-03-08 at 16:51 +0100, Martin Babinsky wrote:
> On 03/08/2016 04:49 PM, Simo Sorce wrote:
>> On Fri, 2015-12-04 at 14:23 +0100, Martin Babinsky wrote:
>>> On 12/01/2015 10:08 PM, Simo Sorce wrote:
 On Tue, 2015-12-01 at 15:59 +0100, Martin Babinsky wrote:
> On 11/30/2015 07:42 PM, Simo Sorce wrote:
>> On Wed, 2015-11-25 at 10:33 +0100, Martin Babinsky wrote:
>>> On 11/24/2015 10:20 PM, Simo Sorce wrote:
 This addresses #3860, giving admins the option to not
 require preauth
 for Hosts and services.

 I did not add this option by default, although it does
 reduce the load
 on the KDC as well as speed up TGT acquisition for service
 principal
 accounts that acquire TGTs.

 Tested and working as expected (SPNs are not returned
 PREAUTH_NEEDED
 error while normal users are).

 HTH,
 Simo.



>>> Hi Simo,
>>>
>>> I was not able to apply the patch on current master branch:
>>>
>>> """
>>> git am
>>> ../review/ssorce/3860/freeipa-simo-558-1-Allow-admins-to-disable-preauth-for-SPNs.patch
>>>
>>> -3
>>>
>>> Applying: Allow admins to disable preauth for SPNs.
>>> error: invalid object 100644
>>> a6b4d4349a9ac6de453d9ad3c679ec32add4e43b
>>> for 'ipalib/plugins/config.py'
>>> fatal: git-write-tree: error building trees
>>> Repository lacks necessary blobs to fall back on 3-way merge.
>>> Cannot fall back to three-way merge.
>>> Patch failed at 0001 Allow admins to disable preauth for SPNs.
>>> """
>>>
>>> It seems that I nedd to apply some of your other patches
>>> first (which one?)
>>
>> Sorry did not see this question earlier, it requires 556 and
>> 557, I just
>> bumped that thread.
>>
>> Simo.
>>
> It seems that I need something else, patch 556-2 applies
> cleanly, but
> patch 557-3 fails with http://fpaste.org/296230/89819431/ on
> both master
> and 4-2 branch.
>

 Rebased 556,557 in their thread, and here is the rebase for 558
 on top
 of them.

 Simo.

>>>
>>> ACK. I'm afraid that this patch and 556, 557 will require another
>>> round
>>> of rebase before pushing, though.
>>
>> Rebased on top of master (not on 556/557) per Petr's request.
>>
>> Simo.
>>
>>
>
> NACK, if you do API changes please increment API version in VERSION.

 Why wasn't this a problem in the previous ACK ?

 Simo.

>>>
>>> Probably because I missed it, sorry.
>>>
>>
>> Fixed.
>>
>> Simo.
>>
> 
> Thanks, ACK.
> 

was this pushed?

-- 
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 0146] Fix internal errors in host-add and other commands caused by DNS resolutio

[Martin Basti]

On 01.07.2016 09:05, Petr Spacek wrote:

On 30.6.2016 21:23, Petr Spacek wrote:

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.


My bad, I forgot to attach cleanup patch 147 which is prerequisite for 146.
(Sorry for the numbering.)


ACK

master:
* ce1f9ca51bd91ed66233c1bac7eb05fac9c855c7 Remove unused is_local(), 
interface, and defaultnet from CheckedIPAddress
* 5e78b54d7c532bec0ee5a4ce3f1b6d6c94d17c51 Fix internal errors in 
host-add and other commands caused by DNS resolution


I will review 4.3 later

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

[Pavel Vomacka]

Hi David,

I did a functional review, and everything works well, so functional-ACK. 
But I did not do the code review.


On 07/01/2016 10:26 AM, thierry bordaz wrote:

Hi David,

The patch looks good but being not familiar with that code, my 
comments may be absolutely wrong


In ipadb_get_pwd_expiration, if it is not 'self' we set 
'*export=mod_time'.
If for some reason 'mod_time==0', it has now a specific meaning 'not 
expiring' . Does it match the comment '* not 'self', so reset */'


In ipadb_entry_to_mods, it deletes krbPasswordExpiration. But just 
before it adds in the mods krbPasswordExpiration=0 or 
krbPasswordExpiration=entry->pw_expiration

Could we skip those mods if entry->pw_expiration==0 or expire_time==0 ?

In ipapwd_SetPassword, ipapwd_post_modadd, same remark as above.

Something that I am not sure is what is the expected relation between 
passwordexpirationtime and krbPasswordExpiration


thanks
thierry

On 06/30/2016 09:34 PM, David Kupka wrote:

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






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

[thierry bordaz]

Hi David,

The patch looks good but being not familiar with that code, my comments 
may be absolutely wrong


In ipadb_get_pwd_expiration, if it is not 'self' we set '*export=mod_time'.
If for some reason 'mod_time==0', it has now a specific meaning 'not 
expiring' . Does it match the comment '* not 'self', so reset */'


In ipadb_entry_to_mods, it deletes krbPasswordExpiration. But just 
before it adds in the mods krbPasswordExpiration=0 or 
krbPasswordExpiration=entry->pw_expiration

Could we skip those mods if entry->pw_expiration==0 or expire_time==0 ?

In ipapwd_SetPassword, ipapwd_post_modadd, same remark as above.

Something that I am not sure is what is the expected relation between 
passwordexpirationtime and krbPasswordExpiration


thanks
thierry

On 06/30/2016 09:34 PM, David Kupka wrote:

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




-- 
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] 0156 extdom: add certificate request

[Martin Basti]

On 24.06.2016 20:41, Lukas Slebodnik wrote:

On (24/06/16 21:09), Alexander Bokovoy wrote:

On Fri, 24 Jun 2016, Lukas Slebodnik wrote:

ah sorry, since 1.14.0 is not release yet we use 1.13.9x to track the
alpha and beta releases and still have incrementing version numbers. So,
it might be better to use '>= 1.13.90' in the spec file instead of
'1.14.0'.

+1, At this point '>= 1.13.90' should be safe.

-1
I vote for official release.
I cannot see a reason why this patch should be pushed immediately.
1.13.90 is just a sssd convention for alpha release and it can be confusing for
others whyt there isn't tarball for 1.13.90

It would allow git master to be built against sssd git master without
additional problems. It is consistency here that I'm after.


It allows it even now and without this patch.
I'm sorry I miss a logic here.

No,

That's not true.

You wrote: "It would allow git master to be built"
against sssd git master"
 ^^
One more time. This patch will not change that
because you can build freeipa git master
against "sssd git master" even now.

It's not my problem that freeipa requires git master of other project.
But it does not mean that you need to officialy requires
some weird version "1.13.90". It is really confusing.


it does not prevent you from running with the code that does not
have needed support. 1.13.4 has no extdom certificate request support
so you would need to make sure you are actually installing the correct
SSSD packages manually while changing the version to 1.13.90 would make
clear we demand a specific functionality.

Building freeipa and installing are two different things.
If you need to install freeipa git master then you need to use
extra copr anyway because:
A) you cannot install freeipa master in rawhide because there isn't
pki-core-10.3.3-1.fc25
b) you cannot install freeipa master on fedora 24 due lots of missing
dependencies (including libsss_nss_idmap-1.14.0)

If one would like to install freeipa git master rpms without copr
then he/she will not able to do it on fedora 24 without new libsss_nss_idmap
because dnf will not be able to find dependency
   "libsss_nss_idmap.so.0(SSS_NSS_IDMAP_0.2.0)(64bit)"


It is a very small thing, of
course, but helpful to those who have to deal with rebases/updates of
their distribution packages and have not been following freeipa-devel@
list in detail. At the very least the inability to find 1.13.90 in a
regular place would cause question being asked.


It's helpful but confusing. That's the reason why we should avoid using
1.13.90. I doubt that anyone will try to use alpha version of sssd or freeipa
on other distributions (debian, ubuntu) especially if they do not work reliably
on fedora. So there's no reason for a rush.

LS


Pushed to master: a635135ba3caa6359c38f305d7982925ef3de50b

Version with dependency on 1.13.90 was pushed, we can increase it when 
SSSD 1.14.0 will be released



From 514cf22027cc8784256595d57b42bea23b1a2a77 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Wed, 22 Jun 2016 10:49:39 +0200
Subject: [PATCH] Bump SSSD version in requires

This is required by commit aa734da49440c5d12c0f8d4566505adaeef254e8 for
function sss_nss_getnamebycert()

https://fedorahosted.org/freeipa/ticket/4955
---
 daemons/configure.ac | 2 +-
 freeipa.spec.in  | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/daemons/configure.ac b/daemons/configure.ac
index 2906def285a0f6ad9553fc07cbc59f7a7f7fd426..94d66d813728fe4e32f9e3c0eef920d8e2395d8f 100644
--- a/daemons/configure.ac
+++ b/daemons/configure.ac
@@ -253,7 +253,7 @@ dnl -- dirsrv is needed for the extdom unit tests --
 PKG_CHECK_MODULES([DIRSRV], [dirsrv  >= 1.3.0])
 dnl -- sss_idmap is needed by the extdom exop --
 PKG_CHECK_MODULES([SSSIDMAP], [sss_idmap])
-PKG_CHECK_MODULES([SSSNSSIDMAP], [sss_nss_idmap])
+PKG_CHECK_MODULES([SSSNSSIDMAP], [sss_nss_idmap >= 1.13.90])
 
 dnl ---
 dnl - Check for systemd unit directory
diff --git a/freeipa.spec.in b/freeipa.spec.in
index b04f819a9ceafe9506e0d5a073ae408fe898..71c24fd7c06a84ee1535516afa4fd89ebc0831ff 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -85,7 +85,7 @@ BuildRequires:  python-pyasn1 >= 0.0.9a
 BuildRequires:  python-qrcode-core >= 5.0.0
 BuildRequires:  python-dns >= 1.11.1
 BuildRequires:  libsss_idmap-devel
-BuildRequires:  libsss_nss_idmap-devel >= 1.12.2
+BuildRequires:  libsss_nss_idmap-devel >= 1.14.0
 BuildRequires:  java-headless
 BuildRequires:  rhino
 BuildRequires:  libverto-devel
@@ -327,7 +327,7 @@ Requires: pam_krb5
 Requires: curl
 Requires: libcurl >= 7.21.7-2
 Requires: xmlrpc-c >= 1.27.4
-Requires: sssd >= 1.13.3-5
+Requires: sssd >= 1.14.0
 Requires: python-sssdconfig
 Requires: certmonger >= 0.78
 Requires: nss-tools
-- 
2.5.5

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

Re: [Freeipa-devel] [PATCH 0109] schema: Perform the check for schema update when, force_schema_check is True

[Jan Cholasta]

On 1.7.2016 10:03, David Kupka wrote:

On 01/07/16 07:59, David Kupka wrote:

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


Offline NACK from Honza, attaching updated patch.


Works for me, ACK.

Pushed to master: cea1f33606e85ac83a7bda66fbef318e47412531

--
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 0109] schema: Perform the check for schema update when, force_schema_check is True

[David Kupka]

On 01/07/16 07:59, David Kupka wrote:

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


Offline NACK from Honza, attaching updated patch.

--
David Kupka
From 3d991e41e9e215c154994948e7d5360f82ea2e29 Mon Sep 17 00:00:00 2001
From: David Kupka 
Date: Fri, 1 Jul 2016 07:50:08 +0200
Subject: [PATCH] schema: Perform the check for schema update when
 force_schema_check is True

https://fedorahosted.org/freeipa/ticket/4739
---
 ipaclient/remote_plugins/schema.py | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/ipaclient/remote_plugins/schema.py b/ipaclient/remote_plugins/schema.py
index a54d4eb777d3fa3d0a1dd6223eca2efeb8db4fbd..89aaeac516b9ee5afa0dd9b6d853d4ed1cf3b801 100644
--- a/ipaclient/remote_plugins/schema.py
+++ b/ipaclient/remote_plugins/schema.py
@@ -496,8 +496,13 @@ class Schema(object):
 logger.warning('Failed to load server properties: {}'
''.format(e))
 
-if no_info or exp < time.time() or not Schema._in_cache(fp):
+force_check = ((not getattr(self, '_schema_checked', False)) and
+   self._api.env.force_schema_check)
+
+if (force_check or
+no_info or exp < time.time() or not Schema._in_cache(fp)):
 (fp, exp) = self._get_schema()
+self._schema_checked = True
 _ensure_dir_created(SERVERS_DIR)
 try:
 with self._open_server_info(self._api.env.server, 'w') as sc:
-- 
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] 0086 Add --ca option to cert-status

[Jan Cholasta]

On 1.7.2016 08:57, Jan Cholasta wrote:

On 1.7.2016 06:54, Jan Cholasta wrote:

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?


To speed things up, I have updated your patch with this, see the
attachment.

If the change looks good to you, we can push the patch.


Your original patch works for me, ACK. My change can be pushed under the 
one-liner rule, so pushing them combined in the modified patch.


Pushed to master: 4844eaec197690e21c6cf44743df7f456d0e185d







(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

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

[Petr Vobornik]

On 07/01/2016 09:04 AM, Pavel Vomacka wrote:
> 
> 
> On 06/30/2016 05:27 PM, Petr Vobornik wrote:
>> 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
> fixed for on_error_add as well.
>>
>> 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);`
>>
> fixed
>> Patch 0070: LGTM
>>
>> Patch 0071: LGTM
>>
>> Patch 0072: LGTM if the change of krbprincipalname to krbcanonicalname
>> is good.
>>
>>
> Updated patches attached.
> 

Focusing on element was returned to patch 68 as discussed offline.

ACK for all.

master:
* df56fd3371bd20a2ce8f5d0097e05437b7827e29 Change error handling in
custom_command_multivalued_widget
* 2232a5bb09b3e99d10598ab64d0bf5d8ef006df4 Set default confirmation
button label to 'Remove'
* 4bc2e3164fbc4fdbbd4ecd1d26001a5d4671dd94 Add widgets for kerberos aliases
* 2da3090a9716bc47e9cf29329ac9bdb734376cb6 Add widget for kerberos
aliases to user page
* 62c4e15d16cf1b29d4a23db146c774ba01bf5935 Add widget for kerberos
aliases to hosts page
* 2ec59b7f232d9119d32d7a5574efba8965904ee8 Add widget for kerberos
aliases to 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] pwpolicy: Do not expire passwords when maxlife is set to 0 (infinity).

[David Kupka]

On 30/06/16 21:34, David Kupka wrote:

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


Updated patch attached.

--
David Kupka
From af5e8516cf743544f529c2cd234af91e5251380e 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  | 22 --
 daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c |  4 
 ipaserver/plugins/pwpolicy.py |  2 +-
 5 files changed, 33 insertions(+), 12 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..0bb50fc319e2b2520d36534d369ad42f95c80c8e 100644
--- 

Re: [Freeipa-devel] [WIP] Kerberos principal aliases pt. 2

[Martin Basti]

On 01.07.2016 09:25, Martin Babinsky wrote:

On 06/30/2016 11:17 PM, David Kupka wrote:

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


Thanks, David.

here are the reviewed patches rebased on the most current master. If 
no one objects I suggest to push them.





master:
* de6abc7af2dac7994b0fff4396115320d1a9a54d ipapython module for Kerberos 
principal manipulation and parsing
* e6fc8f84d3ad5fc4c030ad592a3d743c02393439 Test suite for 
`ipapython/kerberos.py`
* 974eb7b5efd20ad2195b0ad578637ab31f4c1df4 ipalib: introduce Principal 
parameter
* c2af032c0333f7e210c54369159d1d9f5e3fec74 Migrate management framework 
plugins to use Principal parameter
* d1517482b5e9508780087ec48be63a5bb531fed9 Add ACI for admins to modify 
principal attributes
* 7e803aa4625869ef6a8e78a09cd99270c4cc77e5 replace an ACI relying on 
presence of deprecated objectclass
* 750a392fe22aa8ddcb21077e8c24b96d36ecf20c Allow for commands that use 
positional parameters to add/remove attributes
* a28d312796839e3413c98ee37d34ccc892e85357 Make framework consider 
krbcanonicalname as service primary key
* e6ff83e3610d553f6ff98e3adbfbe3c6984b2f17 Provide API for management of 
host, service, and user principal aliases
* acf2234ebc8609a35a8f45598d5d817cbdbff121 Unify display of principal 
names/aliases across entities


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

Patches:
client: add support for pre-schema servers
client: do not crash when overriding remote command as method

brings compatibility with old servers into thin client (making it not so 
thin after all :-). I've tested it against 4.2 and 4.3 servers and the 
important parts work. There are some minor issues (e.g. dynamic defaults 
not working) but they can be addressed later, 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 0108] schema: Decrease schema TTL to one hour

[Martin Basti]

On 01.07.2016 09:08, Petr Spacek wrote:

On 1.7.2016 07:58, David Kupka wrote:

https://fedorahosted.org/freeipa/ticket/4739
--
David Kupka

freeipa-dkupka-0108.0-schema-Decrease-schema-TTL-to-one-hour.patch


 From 796fd4291dd17128e7bdfecf2d14ae7b151987f5 Mon Sep 17 00:00:00 2001
From: David Kupka 
Date: Fri, 1 Jul 2016 07:34:43 +0200
Subject: [PATCH] schema: Decrease schema TTL to one hour

Since checking schema is relatively cheap operation (one round-trip with
almost no data) we can do it offten to ensure schema will fetched by
client ASAP after it was updated on server.

https://fedorahosted.org/freeipa/ticket/4739
---
  ipaserver/plugins/schema.py | 5 -
  1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/ipaserver/plugins/schema.py b/ipaserver/plugins/schema.py
index 
c7aa5f36c37d39a5131ca8ad915cb6e61bb748ec..a82b357899a483fd3b3dc9f7407bd26a4c03aada
 100644
--- a/ipaserver/plugins/schema.py
+++ b/ipaserver/plugins/schema.py
@@ -21,7 +21,10 @@ from ipapython.version import API_VERSION
  
  # Schema TTL sent to clients in response to schema call.

  # Number of seconds before client should check for schema update.
-SCHEMA_TTL = 7*24*3600  # default: 7 days
+# This should be long enough to not slow down regular work or skripts
+# but also short enough to ensure schema will be retvieved soon after
+# it was updated
+SCHEMA_TTL = 3600  # default: 1 hour
  
  __doc__ = _("""

  API Schema
-- 2.7.4

ACK


Pushed to master: e5635f7ef423c7b203004a0cbf625360d351a78e

--
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 0108] schema: Decrease schema TTL to one hour

[Petr Spacek]

On 1.7.2016 07:58, David Kupka wrote:
> https://fedorahosted.org/freeipa/ticket/4739
> -- 
> David Kupka
> 
> freeipa-dkupka-0108.0-schema-Decrease-schema-TTL-to-one-hour.patch
> 
> 
> From 796fd4291dd17128e7bdfecf2d14ae7b151987f5 Mon Sep 17 00:00:00 2001
> From: David Kupka 
> Date: Fri, 1 Jul 2016 07:34:43 +0200
> Subject: [PATCH] schema: Decrease schema TTL to one hour
> 
> Since checking schema is relatively cheap operation (one round-trip with
> almost no data) we can do it offten to ensure schema will fetched by
> client ASAP after it was updated on server.
> 
> https://fedorahosted.org/freeipa/ticket/4739
> ---
>  ipaserver/plugins/schema.py | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/ipaserver/plugins/schema.py b/ipaserver/plugins/schema.py
> index 
> c7aa5f36c37d39a5131ca8ad915cb6e61bb748ec..a82b357899a483fd3b3dc9f7407bd26a4c03aada
>  100644
> --- a/ipaserver/plugins/schema.py
> +++ b/ipaserver/plugins/schema.py
> @@ -21,7 +21,10 @@ from ipapython.version import API_VERSION
>  
>  # Schema TTL sent to clients in response to schema call.
>  # Number of seconds before client should check for schema update.
> -SCHEMA_TTL = 7*24*3600  # default: 7 days
> +# This should be long enough to not slow down regular work or skripts
> +# but also short enough to ensure schema will be retvieved soon after
> +# it was updated
> +SCHEMA_TTL = 3600  # default: 1 hour
>  
>  __doc__ = _("""
>  API Schema
> -- 2.7.4

ACK

-- 
Petr^2 Spacek

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

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

[Martin Basti]

On 30.06.2016 19:42, Martin Babinsky wrote:

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.


master:
* 4ce0258c235c985e07a19d291bd699720d9ef1bf Add option --no-log for 
ipa-replica-conncheck script
* 08fcc7e25af54379eec87f4e22f8cd26db7ffbb0 Do not log to file in remote 
conncheck side


--
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 0146] Fix internal errors in host-add and other commands caused by DNS resolutio

[Petr Spacek]

On 30.6.2016 21:23, Petr Spacek wrote:
> 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.


My bad, I forgot to attach cleanup patch 147 which is prerequisite for 146.
(Sorry for the numbering.)

-- 
Petr^2 Spacek
From 8fd4b5a37c4144328ae5a56b6fd7f8dd21d556ae Mon Sep 17 00:00:00 2001
From: Petr Spacek 
Date: Thu, 30 Jun 2016 13:57:52 +0200
Subject: [PATCH] Remove unused is_local(), interface, and defaultnet from
 CheckedIPAddress

All these were unused so I'm removing them to keep the code clean and
easier to read. At this point it is clear that only difference between
netaddr.IPAddress and CheckedIPAddress is prefixlen attribute.
---
 ipapython/ipautil.py | 9 -
 1 file changed, 9 deletions(-)

diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index c7e20c5102efaa006c10d4c3af849bc259da43e7..8506bf2d58a35cb128e44cee34c3fb62d7a0c5e4 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -87,13 +87,10 @@ class CheckedIPAddress(netaddr.IPAddress):
 if isinstance(addr, CheckedIPAddress):
 super(CheckedIPAddress, self).__init__(addr, flags=self.netaddr_ip_flags)
 self.prefixlen = addr.prefixlen
-self.defaultnet = addr.defaultnet
-self.interface = addr.interface
 return
 
 net = None
 iface = None
-defnet = False
 
 if isinstance(addr, netaddr.IPNetwork):
 net = addr
@@ -161,7 +158,6 @@ class CheckedIPAddress(netaddr.IPAddress):
 raise ValueError('No network interface matches the provided IP address and netmask')
 
 if net is None:
-defnet = True
 if addr.version == 4:
 net = netaddr.IPNetwork(netaddr.cidr_abbrev_to_verbose(str(addr)))
 elif addr.version == 6:
@@ -174,11 +170,6 @@ class CheckedIPAddress(netaddr.IPAddress):
 
 super(CheckedIPAddress, self).__init__(addr, flags=self.netaddr_ip_flags)
 self.prefixlen = net.prefixlen
-self.defaultnet = defnet
-self.interface = iface
-
-def is_local(self):
-return self.interface is not None
 
 def valid_ip(addr):
 return netaddr.valid_ipv4(addr) or netaddr.valid_ipv6(addr)
-- 
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] 0067-72: webui for kerberos aliases

[Pavel Vomacka]

On 06/30/2016 05:27 PM, Petr Vobornik wrote:

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

fixed for on_error_add as well.


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);`


fixed

Patch 0070: LGTM

Patch 0071: LGTM

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



Updated patches attached.

--
Pavel^3 Vomacka

From 27d3d1b23a58de9fb8133ea7ad7102dc8f4118df 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 in
 custom_command_multivalued_widget

The custom_command_multivalued_widget now handles remove and add commands errors
correctly and shows error message.

Part of: https://fedorahosted.org/freeipa/ticket/5381

add_error
---
 install/ui/src/freeipa/widget.js | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/install/ui/src/freeipa/widget.js b/install/ui/src/freeipa/widget.js
index 0972efadb46ac7b5811f4e072121a448618fe434..e3c2dc555df9dbebdea62e19ce445cae4d60c8f7 100644
--- a/install/ui/src/freeipa/widget.js
+++ b/install/ui/src/freeipa/widget.js
@@ -1533,8 +1533,11 @@ IPA.custom_command_multivalued_widget = function(spec) {
 /**
  * Called on error of add command. Override point.
  */
-that.on_error_add = function(data) {
-that.adder_dialog.focus_first_element();
+that.on_error_add = function(xhr, text_status, error_thrown) {
+if (error_thrown.message) {
+var msg = error_thrown.message;
+IPA.notify(msg, 'error');
+}
 };
 
 /**
@@ -1548,8 +1551,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 f52a0e5561497e0a86baa149eb7aa949f7b04c07 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   | 108 +
 install/ui/test/data/ipa_init.json |   6 +++
 ipaserver/plugins/internal.py  |   6 +++
 3 files changed, 120 insertions(+)

diff --git a/install/ui/src/freeipa/widget.js b/install/ui/src/freeipa/widget.js
index d6c1672ad614d13642fe4c9e3dcef55d458bcd7d..c365a8c4f83a039f0e9b2d454e2d9ab1d8fc0772 100644
--- a/install/ui/src/freeipa/widget.js
+++ b/install/ui/src/freeipa/widget.js
@@ -1796,6 +1796,110 @@ IPA.custom_command_multivalued_widget = function(spec) {
 return that;
 };
 
+/**
+ * Multivalued widget which is used for working with kerberos principal aliases.
+ *
+ * @class
+ * @extends IPA.custom_command_multivalued_widget
+ */
+IPA.krb_principal_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();
+   

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

[Martin Basti]

On 30.06.2016 10:24, Florence Blanc-Renaud wrote:

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



ACK

Pushed to master: d9ae9ee1b5397765ba7f184c7647bd36708ca0e8

-- 
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] 961 webui: prevent infinite reload for users with krbbprincipal alias set

[Martin Basti]

On 30.06.2016 19:54, Martin Babinsky wrote:

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.


Pushed to master: 88f7154f7fcb1ca86dcbeeaca3c220ed4b88d55f

--
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] 0070..0071 Fix replica installation from IPA v4.2

[Martin Basti]

On 01.07.2016 08:42, Fraser Tweedale wrote:

On Fri, Jul 01, 2016 at 08:36:29AM +0200, Stanislav Laznicka wrote:

On 06/17/2016 08:59 AM, Fraser Tweedale wrote:

The attached patches fix
https://fedorahosted.org/freeipa/ticket/5963

Thanks Milan for reporting.

Cheers,
Fraser


Tried this patch on 4.4 with domain level set to 0 and it does fix the issue
for me so ACK for 4.4.

Not sure if this is going to be backported but if so, both patches will need
modifications for 4.2 and the latter patch will require modifications for
4.3 for them to apply.


Thanks for testing!

It is only needed for 4.4.

Cheers,
Fraser


master:
* 0334693cfc56bc2788ea3b4f3cea9547c9c00340 Split CA replica installation 
steps for domain level 0
* 3ac3882631564cd774114e61e607fffdbd667eee Fix migration from 
pre-lightweight CAs master


--
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] 0086 Add --ca option to cert-status

[Jan Cholasta]

On 1.7.2016 06:54, Jan Cholasta wrote:

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?


To speed things up, I have updated your patch with this, see the attachment.

If the change looks good to you, we can push the patch.





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


OK.



Thanks,
Fraser







--
Jan Cholasta
From 7352084f62c1aba097e6a3a34c4835861cc27d6c 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 | 16 
 3 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/API.txt b/API.txt
index c01692e..9f9456d 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 23ceecc..212b7d7 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 817bdc2..86db6ce 100644
--- a/ipaserver/plugins/cert.py
+++ b/ipaserver/plugins/cert.py
@@ -638,17 +638,17 @@ 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
+# but check if the specified CA exists.
+self.api.Command.ca_show(kw['cacn'])
+
 return dict(
 result=self.Backend.ra.check_request_status(str(request_id)),
 value=pkey_to_value(request_id, kw),
-- 
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] Fix minor typo

[Martin Basti]

On 30.06.2016 18:56, Yuri Chornoivan wrote:

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



ACK

Pushed to master: f5eb71f75e5e205674ed8df1e4c6324602fbe733

-- 
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] Karma Request for Dogtag 10.2.6 on Fedora 23

[Matthew Harmsen]

The following bug has been addressed in Fedora 23:

 * Bugzilla Bug #1323400 - freeipa fails to start correctly after
   pki-core update on upgraded system
   

Please provide Karma for the following Fedora 23 build located in Bodhi at:

 * https://bodhi.fedoraproject.org/updates/FEDORA-2016-188c172b10
   pki-core-10.2.6-20.fc23

Thanks,
-- Matt

-- 
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] 0070..0071 Fix replica installation from IPA v4.2

[Fraser Tweedale]

On Fri, Jul 01, 2016 at 08:36:29AM +0200, Stanislav Laznicka wrote:
> On 06/17/2016 08:59 AM, Fraser Tweedale wrote:
> > The attached patches fix
> > https://fedorahosted.org/freeipa/ticket/5963
> > 
> > Thanks Milan for reporting.
> > 
> > Cheers,
> > Fraser
> > 
> Tried this patch on 4.4 with domain level set to 0 and it does fix the issue
> for me so ACK for 4.4.
> 
> Not sure if this is going to be backported but if so, both patches will need
> modifications for 4.2 and the latter patch will require modifications for
> 4.3 for them to apply.
>
Thanks for testing!

It is only needed for 4.4.

Cheers,
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] 0070..0071 Fix replica installation from IPA v4.2

[Stanislav Laznicka]

On 06/17/2016 08:59 AM, Fraser Tweedale wrote:

The attached patches fix
https://fedorahosted.org/freeipa/ticket/5963

Thanks Milan for reporting.

Cheers,
Fraser

Tried this patch on 4.4 with domain level set to 0 and it does fix the 
issue for me so ACK for 4.4.


Not sure if this is going to be backported but if so, both patches will 
need modifications for 4.2 and the latter patch will require 
modifications for 4.3 for them to apply.


--
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 0109] schema: Perform the check for schema update when, force_schema_check is True

[David Kupka]

https://fedorahosted.org/freeipa/ticket/4739
--
David Kupka
From 58685f92e8d4c1817f95a7b4042ce0fa4c95a704 Mon Sep 17 00:00:00 2001
From: David Kupka 
Date: Fri, 1 Jul 2016 07:50:08 +0200
Subject: [PATCH] schema: Perform the check for schema update when
 force_schema_check is True

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

diff --git a/ipaclient/remote_plugins/schema.py b/ipaclient/remote_plugins/schema.py
index a54d4eb777d3fa3d0a1dd6223eca2efeb8db4fbd..78ca84484595ce28da12bc81561003894d7c8db3 100644
--- a/ipaclient/remote_plugins/schema.py
+++ b/ipaclient/remote_plugins/schema.py
@@ -496,7 +496,8 @@ class Schema(object):
 logger.warning('Failed to load server properties: {}'
''.format(e))
 
-if no_info or exp < time.time() or not Schema._in_cache(fp):
+if (self._api.env.force_schema_check or
+no_info or exp < time.time() or not Schema._in_cache(fp)):
 (fp, exp) = self._get_schema()
 _ensure_dir_created(SERVERS_DIR)
 try:
-- 
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 0108] schema: Decrease schema TTL to one hour

[David Kupka]

https://fedorahosted.org/freeipa/ticket/4739
--
David Kupka
From 796fd4291dd17128e7bdfecf2d14ae7b151987f5 Mon Sep 17 00:00:00 2001
From: David Kupka 
Date: Fri, 1 Jul 2016 07:34:43 +0200
Subject: [PATCH] schema: Decrease schema TTL to one hour

Since checking schema is relatively cheap operation (one round-trip with
almost no data) we can do it offten to ensure schema will fetched by
client ASAP after it was updated on server.

https://fedorahosted.org/freeipa/ticket/4739
---
 ipaserver/plugins/schema.py | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/ipaserver/plugins/schema.py b/ipaserver/plugins/schema.py
index c7aa5f36c37d39a5131ca8ad915cb6e61bb748ec..a82b357899a483fd3b3dc9f7407bd26a4c03aada 100644
--- a/ipaserver/plugins/schema.py
+++ b/ipaserver/plugins/schema.py
@@ -21,7 +21,10 @@ from ipapython.version import API_VERSION
 
 # Schema TTL sent to clients in response to schema call.
 # Number of seconds before client should check for schema update.
-SCHEMA_TTL = 7*24*3600  # default: 7 days
+# This should be long enough to not slow down regular work or skripts
+# but also short enough to ensure schema will be retvieved soon after
+# it was updated
+SCHEMA_TTL = 3600  # default: 1 hour
 
 __doc__ = _("""
 API Schema
-- 
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