Re: [Freeipa-devel] [PATCH] 441 Consolidate .gitignore entries

2013-12-10 Thread Petr Viktorin

On 12/09/2013 05:43 PM, Martin Kosek wrote:

Clean up the .gitignore file:
- Remove no longer used .gitignore entries, like .bzr files
- Do not repeat autotools generated files over and over again
- Whitelist existent Makefiles in the repository
- Better separate the .gitignore entries

===

When porting Jan's patches downstream, I had hard time merging changes to
/Makefile in the repository as it was stated in .gitignore which made git (and
me) suffer. I fixed that by whitelisting this one.

Unfortunately, when I saw other entries in .gitignore I could not resist and
refactored the file to make it (hopefully) simpler and easier to maintain.

Martin



Thanks! ACK, pushed to master: 1e0405880fb1855563cb9b58a39213e1d3b4a2c6

--
Petr³

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 211 Fix internal error in the user-status command

2013-12-10 Thread Petr Viktorin

On 12/05/2013 02:45 PM, Jan Cholasta wrote:

Hi,

the attached patch fixes https://fedorahosted.org/freeipa/ticket/4066.

Honza


Patch looks good, ACK.

I've added a small regression test for this, does it look OK?

--
Petr³

From bb333401aa551f84a16192d2d42de20bd5633f84 Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Tue, 10 Dec 2013 12:16:25 +0100
Subject: [PATCH] Regression test for user_status crash

https://fedorahosted.org/freeipa/ticket/4066
---
 ipatests/test_xmlrpc/test_user_plugin.py | 25 +
 1 file changed, 25 insertions(+)

diff --git a/ipatests/test_xmlrpc/test_user_plugin.py b/ipatests/test_xmlrpc/test_user_plugin.py
index 2f07e1495ff31cac7ea4db2eb42453e065aec418..14a4b501d49537eb1ddcc6c593ad4b75c15da6da 100644
--- a/ipatests/test_xmlrpc/test_user_plugin.py
+++ b/ipatests/test_xmlrpc/test_user_plugin.py
@@ -23,6 +23,8 @@
 Test the `ipalib/plugins/user.py` module.
 
 
+import re
+
 from ipalib import api, errors
 from ipatests.test_xmlrpc import objectclasses
 from ipatests.util import assert_equal, assert_not_equal
@@ -45,6 +47,9 @@
 sshpubkey = u'ssh-rsa B3NzaC1yc2EDAQABAAABAQDGAX3xAeLeaJggwTqMjxNwa6XHBUAikXPGMzEpVrlLDCZtv00djsFTBi38PkgxBJVkgRWMrcBsr/35lq7P6w8KGIwA8GI48Z0qBS2NBMJ2u9WQ2hjLN6GdMlo77O0uJY3251p12pCVIS/bHRSq8kHO2No8g7KA9fGGcagPfQH+ee3t7HUkpbQkFTmbPPN++r3V8oVUk5LxbryB3UIIVzNmcSIn3JrXynlvui4MixvrtX6zx+O/bBo68o8/eZD26QrahVbA09fivrn/4h3TM019Eu/c2jOdckfU3cHUV/3Tno5d6JicibyaoDDK7S/yjdn5jhaz8MSEayQvFkZkiF0L public key test'
 sshpubkeyfp = u'13:67:6B:BF:4E:A2:05:8E:AE:25:8B:A1:31:DE:6F:1B public key test (ssh-rsa)'
 
+# Date in ISO format (2013-12-10T12:00:00)
+isodate_re = re.compile('^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z$')
+
 
 def get_user_result(uid, givenname, sn, operation='show', omit=[],
 **overrides):
@@ -1449,4 +1454,24 @@ class test_user(Declarative):
 ),
 ),
 
+dict(
+desc='Query status of %s' % user1,
+command=('user_status', [user1], {}),
+expected=dict(
+count=1,
+result=[
+dict(
+dn=get_user_dn(user1),
+krblastfailedauth=[u'N/A'],
+krblastsuccessfulauth=[u'N/A'],
+krbloginfailedcount=u'0',
+now=isodate_re.match,
+server=api.env.host,
+),
+],
+summary=u'Account disabled: False',
+truncated=False,
+),
+),
+
 ]
-- 
1.8.3.1

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 0333 test_webui: Allow False values in configuration for no_ca, no_dns, has_trusts

2013-12-10 Thread Petr Vobornik

On 6.12.2013 16:52, Petr Viktorin wrote:

Hello,
As I'm consolidating test job configuration, I learned that the line
 no_dns: False
in WebUI test config has the same effect as `no_dns: True`. This is
confusing, and it complicates generating the config.

Patch is untested because I don't have the WebUI test environment set up
locally. Petr, could you check if it works?



Tested, ACK
--
Petr Vobornik

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


[Freeipa-devel] [PATCHES] 213-224 Use old entry state in LDAP mods

2013-12-10 Thread Jan Cholasta

Hi,

the attached patches fix https://fedorahosted.org/freeipa/ticket/3488.

Honza

--
Jan Cholasta
From 7fa3ca5c581b54bfb3d8b6c904d33bde0c3845da Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Tue, 10 Dec 2013 11:09:56 +0100
Subject: [PATCH 01/12] Rename LDAPEntry method commit to reset_modlist.

https://fedorahosted.org/freeipa/ticket/3488
---
 ipapython/ipaldap.py| 2 +-
 ipaserver/install/ldapupdate.py | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py
index 39d0edb..c468c6a 100644
--- a/ipapython/ipaldap.py
+++ b/ipapython/ipaldap.py
@@ -740,7 +740,7 @@ class LDAPEntry(collections.MutableMapping):
 
 return result
 
-def commit(self):
+def reset_modlist(self):
 
 Make the current state of the entry a new reference point for change
 tracking.
diff --git a/ipaserver/install/ldapupdate.py b/ipaserver/install/ldapupdate.py
index 87bd0c8..0c44a85 100644
--- a/ipaserver/install/ldapupdate.py
+++ b/ipaserver/install/ldapupdate.py
@@ -281,7 +281,7 @@ class LDAPUpdate:
 
 def _entry_to_entity(self, ent):
 entry = ent.copy()
-entry.commit()
+entry.reset_modlist()
 return entry
 
 def _combine_updates(self, all_updates, update):
-- 
1.8.4.2

From 323209be795748a4dda2994c8cdf324912c25213 Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Tue, 10 Dec 2013 11:13:48 +0100
Subject: [PATCH 02/12] Use old entry state in LDAPClient.update_entry.

https://fedorahosted.org/freeipa/ticket/3488
---
 ipapython/ipaldap.py | 33 -
 1 file changed, 12 insertions(+), 21 deletions(-)

diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py
index c468c6a..92528f4 100644
--- a/ipapython/ipaldap.py
+++ b/ipapython/ipaldap.py
@@ -440,6 +440,7 @@ class IPASimpleLDAPObject(object):
 
 for attr, original_values in original_attrs.items():
 ipa_entry.raw[attr] = original_values
+ipa_entry.reset_modlist()
 
 ipa_result.append(ipa_entry)
 
@@ -1530,16 +1531,6 @@ class LDAPClient(object):
 raise errors.LimitsExceeded()
 return entry[0]
 
-def _get_dn_and_attrs(self, entry_or_dn, entry_attrs):
-Helper for legacy calling style for {add,update}_entry
-
-if entry_attrs is None:
-return entry_or_dn.dn, entry_or_dn
-else:
-assert isinstance(entry_or_dn, DN)
-entry_attrs = self.make_entry(entry_or_dn, entry_attrs)
-return entry_or_dn, entry_attrs
-
 def add_entry(self, entry, entry_attrs=None):
 Create a new entry.
 
@@ -1548,13 +1539,14 @@ class LDAPClient(object):
 The legacy two-argument variant is:
 add_entry(dn, entry_attrs)
 
-dn, attrs = self._get_dn_and_attrs(entry, entry_attrs)
+if entry_attrs is not None:
+entry = self.make_entry(entry, entry_attrs)
 
 # remove all [] values (python-ldap hates 'em)
-attrs = dict((k, v) for k, v in attrs.raw.iteritems() if v)
+attrs = dict((k, v) for k, v in entry.raw.iteritems() if v)
 
 with self.error_handler():
-self.conn.add_s(dn, attrs.items())
+self.conn.add_s(entry.dn, attrs.items())
 
 def update_entry_rdn(self, dn, new_rdn, del_old=True):
 
@@ -1576,20 +1568,17 @@ class LDAPClient(object):
 def _generate_modlist(self, dn, entry_attrs):
 assert isinstance(dn, DN)
 
-# get original entry
-dn, entry_attrs_old = self.get_entry(dn, entry_attrs.keys())
-
 # generate modlist
 # for multi value attributes: no MOD_REPLACE to handle simultaneous
 # updates better
 # for single value attribute: always MOD_REPLACE
 modlist = []
 for (k, v) in entry_attrs.raw.iteritems():
-if not v and k in entry_attrs_old:
+if not v and k in entry_attrs.orig_data:
 modlist.append((ldap.MOD_DELETE, k, None))
 else:
 v = set(v)
-old_v = set(entry_attrs_old.raw.get(k, []))
+old_v = set(entry_attrs.orig_data.raw.get(k, []))
 
 adds = list(v.difference(old_v))
 rems = list(old_v.difference(v))
@@ -1629,16 +1618,18 @@ class LDAPClient(object):
 The legacy two-argument variant is:
 update_entry(dn, entry_attrs)
 
-dn, attrs = self._get_dn_and_attrs(entry, entry_attrs)
+if entry_attrs is not None:
+entry = self.get_entry(entry, entry_attrs.keys())
+entry.update(entry_attrs)
 
 # generate modlist
-modlist = self._generate_modlist(dn, attrs)
+modlist = self._generate_modlist(entry.dn, entry)
 if not modlist:
 raise errors.EmptyModlist()
 
 # pass arguments to python-ldap
 with 

Re: [Freeipa-devel] [PATCH] 211 Fix internal error in the user-status command

2013-12-10 Thread Jan Cholasta

On 10.12.2013 12:18, Petr Viktorin wrote:

On 12/05/2013 02:45 PM, Jan Cholasta wrote:

Hi,

the attached patch fixes https://fedorahosted.org/freeipa/ticket/4066.

Honza


Patch looks good, ACK.

I've added a small regression test for this, does it look OK?


Thanks, it looks OK except I don't see dn in result and I would rename 
isodate_re to generalizedtime_re.


--
Jan Cholasta

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


[Freeipa-devel] [PATCH] 0335 ipa-replica-install: Move check for existing host before DNS resolution check

2013-12-10 Thread Petr Viktorin

See commit message  ticket for details.

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

--
Petr³
From 0c159673b1df2b31ce693398536ff31ebf4bb53a Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Tue, 10 Dec 2013 13:00:16 +0100
Subject: [PATCH] ipa-replica-install: Move check for existing host before DNS
 resolution check

The checks for existing host and existing replication agreement
set a flag that caused an exit() if any of them failed.

Between these checks there was an unrelated check, DNS resolution.
If the host and DNS checks both failed, this made it look like
the DNS check was the cause of failed install. Especially if the user
ignored the DNS check in unattended mode, the output was confusing.

Remove the flag and fail directly.
Do the replication agreement check first; fixing this with
ipa-replica-manage del will also remove the host entry.

Also, use the logger for error messages so they appear in the log
file as well as on the console.

https://fedorahosted.org/freeipa/ticket/3889
---
 install/tools/ipa-replica-install | 41 ---
 1 file changed, 25 insertions(+), 16 deletions(-)

diff --git a/install/tools/ipa-replica-install b/install/tools/ipa-replica-install
index 0e7aefef48d47fefa290607e0604c014d9469fdd..462526bb456c6b8f80812cd061db26f590c8059d 100755
--- a/install/tools/ipa-replica-install
+++ b/install/tools/ipa-replica-install
@@ -606,14 +606,34 @@ def main():
  tls_cacertfile=CACERT)
 replman = ReplicationManager(config.realm_name, config.master_host_name,
  config.dirman_password)
-found = False
+
+# Check that we don't already have a replication agreement
+try:
+(agreement_cn, agreement_dn) = replman.agreement_dn(host)
+entry = conn.get_entry(agreement_dn, ['*'])
+except errors.NotFound:
+pass
+else:
+root_logger.info('Error: A replication agreement for this host '
+'already exists.')
+print ('A replication agreement for this host already exists. '
+   'It needs to be removed.')
+print Run this on the master that generated the info file:
+print %% ipa-replica-manage del %s --force % host
+exit(3)
+
+# Check pre-existing host entry
 try:
 entry = conn.find_entries(u'fqdn=%s' % host, ['fqdn'], DN(api.env.container_host, api.env.basedn))
-print The host %s already exists on the master server.\nYou should remove it before proceeding: % host
+except errors.NotFound:
+pass
+else:
+root_logger.info(
+'Error: Host %s already exists on the master server.' % host)
+print 'The host %s already exists on the master server.' % host
+print You should remove it before proceeding:
 print %% ipa host-del %s % host
-found = True
-except errors.NotFound:
-pass
+exit(3)
 
 # If remote host has DNS, check forward/reverse resolution
 with temporary_ldap2_connection(
@@ -633,17 +653,6 @@ def main():
 root_logger.debug('No IPA DNS servers, '
 'skipping forward/reverse resolution check')
 
-# Check that we don't already have a replication agreement
-try:
-(agreement_cn, agreement_dn) = replman.agreement_dn(host)
-entry = conn.get_entry(agreement_dn, ['*'])
-print A replication agreement for this host already exists. It needs to be removed. Run this on the master that generated the info file:
-print %% ipa-replica-manage del %s --force % host
-found = True
-except errors.NotFound:
-pass
-if found:
-sys.exit(3)
 except errors.ACIError:
 sys.exit(\nThe password provided is incorrect for LDAP server %s % config.master_host_name)
 except errors.LDAPError:
-- 
1.8.3.1

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 211 Fix internal error in the user-status command

2013-12-10 Thread Petr Viktorin

On 12/10/2013 02:15 PM, Jan Cholasta wrote:

On 10.12.2013 12:18, Petr Viktorin wrote:

On 12/05/2013 02:45 PM, Jan Cholasta wrote:

Hi,

the attached patch fixes https://fedorahosted.org/freeipa/ticket/4066.

Honza


Patch looks good, ACK.

I've added a small regression test for this, does it look OK?


Thanks, it looks OK except I don't see dn in result and I would rename
isodate_re to generalizedtime_re.


Your patch adds dn.

user_status without --raw will report time in ISO 8601 
(%Y-%m-%dT%H:%M:%SZ). GeneralizedTime would be %Y%m%d%H%M%SZ.


--
Petr³

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 211 Fix internal error in the user-status command

2013-12-10 Thread Jan Cholasta

On 10.12.2013 15:18, Petr Viktorin wrote:

On 12/10/2013 02:15 PM, Jan Cholasta wrote:

On 10.12.2013 12:18, Petr Viktorin wrote:

On 12/05/2013 02:45 PM, Jan Cholasta wrote:

Hi,

the attached patch fixes
https://fedorahosted.org/freeipa/ticket/4066.

Honza


Patch looks good, ACK.

I've added a small regression test for this, does it look OK?


Thanks, it looks OK except I don't see dn in result and I would rename
isodate_re to generalizedtime_re.


Your patch adds dn.


Oh, right.



user_status without --raw will report time in ISO 8601
(%Y-%m-%dT%H:%M:%SZ). GeneralizedTime would be %Y%m%d%H%M%SZ.



Also right.

Sorry for the fuss then, ACK.

--
Jan Cholasta

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 211 Fix internal error in the user-status command

2013-12-10 Thread Petr Viktorin

On 12/10/2013 03:23 PM, Jan Cholasta wrote:

On 10.12.2013 15:18, Petr Viktorin wrote:

On 12/10/2013 02:15 PM, Jan Cholasta wrote:

On 10.12.2013 12:18, Petr Viktorin wrote:

On 12/05/2013 02:45 PM, Jan Cholasta wrote:

Hi,

the attached patch fixes
https://fedorahosted.org/freeipa/ticket/4066.

Honza


Patch looks good, ACK.

I've added a small regression test for this, does it look OK?


Thanks, it looks OK except I don't see dn in result and I would rename
isodate_re to generalizedtime_re.


Your patch adds dn.


Oh, right.



user_status without --raw will report time in ISO 8601
(%Y-%m-%dT%H:%M:%SZ). GeneralizedTime would be %Y%m%d%H%M%SZ.



Also right.

Sorry for the fuss then, ACK.



Thanks, pushed both to master: b6563984154e577cdf430f8f74f15f912ac0ee12

--
Petr³

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 0333 test_webui: Allow False values in configuration for no_ca, no_dns, has_trusts

2013-12-10 Thread Petr Viktorin

On 12/10/2013 02:00 PM, Petr Vobornik wrote:

On 6.12.2013 16:52, Petr Viktorin wrote:

Hello,
As I'm consolidating test job configuration, I learned that the line
 no_dns: False
in WebUI test config has the same effect as `no_dns: True`. This is
confusing, and it complicates generating the config.

Patch is untested because I don't have the WebUI test environment set up
locally. Petr, could you check if it works?



Tested, ACK


Thanks, pushed to
master: f2ee8a74032708717f8b370de5b1acc86d4d74cc
ipa-3-3: 5640049f7aa132ab73099db856b352c3413489bb

--
Petr³

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


[Freeipa-devel] [PATCH] 0336 rpcserver: Consolidate __call__ in xmlclient and jsonclient_kerb

2013-12-10 Thread Petr Viktorin

See commit message  ticket.

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

--
Petr³
From f2eaff6f9965331d83a2213161100ae21cd9a26a Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Tue, 10 Dec 2013 17:36:32 +0100
Subject: [PATCH] rpcserver: Consolidate __call__ in xmlclient and
 jsonclient_kerb

The two classes had very similar __call__ methods, but the JSON
server lacked error handling.

Create a common class for the __call__ method.

https://fedorahosted.org/freeipa/ticket/4069
---
 ipaserver/rpcserver.py | 110 -
 1 file changed, 45 insertions(+), 65 deletions(-)

diff --git a/ipaserver/rpcserver.py b/ipaserver/rpcserver.py
index a37d3cd0f25eed8b19e4980fa30a496c261ebde4..992b89583b920355b9137db0f760378624141f5e 100644
--- a/ipaserver/rpcserver.py
+++ b/ipaserver/rpcserver.py
@@ -596,7 +596,50 @@ def finalize_kerberos_acquisition(self, who, ccache_name, environ, start_respons
 return ['']
 
 
-class xmlserver(WSGIExecutioner, HTTP_Status, KerberosSession):
+class KerberosWSGIExecutioner(WSGIExecutioner, HTTP_Status, KerberosSession):
+Base class for xmlserver and jsonserver_kerb
+
+
+def _on_finalize(self):
+super(KerberosWSGIExecutioner, self)._on_finalize()
+self.kerb_session_on_finalize()
+
+def __call__(self, environ, start_response):
+self.debug('KerberosWSGIExecutioner.__call__:')
+user_ccache=environ.get('KRB5CCNAME')
+
+headers = [('Content-Type', '%s; charset=utf-8' % self.content_type)]
+
+if user_ccache is None:
+
+status = HTTP_STATUS_SERVER_ERROR
+response_headers = [('Content-Type', 'text/html; charset=utf-8')]
+
+self.log.error(
+'%s: %s', status,
+'KerberosWSGIExecutioner.__call__: '
+'KRB5CCNAME not defined in HTTP request environment')
+
+return self.marshal(None, CCacheError())
+try:
+self.create_context(ccache=user_ccache)
+response = super(KerberosWSGIExecutioner, self).__call__(
+environ, start_response)
+session_data = getattr(context, 'session_data', None)
+if (session_data is None and self.env.context != 'lite'):
+self.finalize_kerberos_acquisition(
+'xmlserver', user_ccache, environ, start_response, headers)
+except PublicError, e:
+status = HTTP_STATUS_SUCCESS
+response = status
+start_response(status, headers)
+return self.marshal(None, e)
+finally:
+destroy_context()
+return response
+
+
+class xmlserver(KerberosWSGIExecutioner):
 
 Execution backend plugin for XML-RPC server.
 
@@ -606,41 +649,6 @@ class xmlserver(WSGIExecutioner, HTTP_Status, KerberosSession):
 content_type = 'text/xml'
 key = '/xml'
 
-def _on_finalize(self):
-self.__system = {
-'system.listMethods': self.listMethods,
-'system.methodSignature': self.methodSignature,
-'system.methodHelp': self.methodHelp,
-}
-super(xmlserver, self)._on_finalize()
-self.kerb_session_on_finalize()
-
-def __call__(self, environ, start_response):
-'''
-'''
-
-self.debug('WSGI xmlserver.__call__:')
-user_ccache=environ.get('KRB5CCNAME')
-headers = [('Content-Type', 'text/xml; charset=utf-8')]
-if user_ccache is None:
-self.internal_error(environ, start_response,
-'xmlserver.__call__: KRB5CCNAME not defined in HTTP request environment')
-return self.marshal(None, CCacheError())
-try:
-self.create_context(ccache=user_ccache)
-response = super(xmlserver, self).__call__(environ, start_response)
-if getattr(context, 'session_data', None) is None and \
-  self.env.context != 'lite':
-self.finalize_kerberos_acquisition('xmlserver', user_ccache, environ, start_response, headers)
-except PublicError, e:
-status = HTTP_STATUS_SUCCESS
-response = status
-start_response(status, headers)
-return self.marshal(None, e)
-finally:
-destroy_context()
-return response
-
 def listMethods(self, *params):
 return tuple(name.decode('UTF-8') for name in self.Command)
 
@@ -769,41 +777,13 @@ def __call__(self, environ, start_response):
 return response
 
 
-class jsonserver_kerb(jsonserver, KerberosSession):
+class jsonserver_kerb(jsonserver, KerberosWSGIExecutioner):
 
 JSON RPC server protected with kerberos auth.
 
 
 key = '/json'
 
-def _on_finalize(self):
-super(jsonserver_kerb, self)._on_finalize()
-self.kerb_session_on_finalize()
-
-def __call__(self, environ, start_response):
-'''
-'''
-
-