Re: [Freeipa-devel] [PATCH] 974 minimum selinux-policy for F-17

2012-03-09 Thread Martin Kosek
On Tue, 2012-03-06 at 16:18 -0500, Rob Crittenden wrote:
 Rob Crittenden wrote:
  Update the minimum selinux-policy for F-17. This will enable
  ipa_memcached to run in Enforcing mode. Still waiting on this to be
  backported to at least F-16.
 
  You have to manually enable the boolean.
 
  rob
 
 
 F-16 package is in updates-testing, patch updated.
 
 rob


ACK. New selinux-policy looks OK. I was able to install IPA server with
enforcing. Of course, httpd_manage_ipa need to be ON.

Martin

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


[Freeipa-devel] [PATCH] 235 Fix migration plugin compat check

2012-03-09 Thread Martin Kosek
Ticket #2274 implements a check for compat plugin and warns user if
it is enabled. However, there are 2 issues connected with the plugin:
1) The check is performed against the remote (migrated) LDAP server
   and not the local LDAP server, which does not make much sense
2) When the compat plugin is missing in cn=plugins,cn=config, it
   raises an error and thus breaks the migration
This patch fixes both issues.

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

From 4da35b447a197776ba889936d6579ccdd95938eb Mon Sep 17 00:00:00 2001
From: Martin Kosek mko...@redhat.com
Date: Fri, 9 Mar 2012 12:36:03 +0100
Subject: [PATCH] Fix migration plugin compat check

Ticket #2274 implements a check for compat plugin and warns user if
it is enabled. However, there are 2 issues connected with the plugin:
1) The check is performed against the remote (migrated) LDAP server
   and not the local LDAP server, which does not make much sense
2) When the compat plugin is missing in cn=plugins,cn=config, it
   raises an error and thus breaks the migration
This patch fixes both issues.

https://fedorahosted.org/freeipa/ticket/2508
---
 ipalib/plugins/migration.py |   10 +++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/ipalib/plugins/migration.py b/ipalib/plugins/migration.py
index a3baf97fe32cc67603047d0ba89bc38939a88a17..7adddb5aa03b6543babcc2d1458d48c3cff1a506 100644
--- a/ipalib/plugins/migration.py
+++ b/ipalib/plugins/migration.py
@@ -670,9 +670,13 @@ can use their Kerberos accounts.''')
 
 #check whether the compat plugin is enabled
 if not options.get('compat'):
-(dn,check_compat) = ds_ldap.get_entry(_compat_dn, normalize=False)
-if check_compat is not None and check_compat.get('nsslapd-pluginenabled', [''])[0].lower() == 'on':
-return dict(result={},failed={},enabled=True, compat=False)
+try:
+(dn,check_compat) = ldap.get_entry(_compat_dn, normalize=False)
+if check_compat is not None and \
+check_compat.get('nsslapd-pluginenabled', [''])[0].lower() == 'on':
+return dict(result={},failed={},enabled=True, compat=False)
+except errors.NotFound:
+pass
 
 if not ds_base_dn:
 # retrieve base DN from remote LDAP server
-- 
1.7.7.6

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

[Freeipa-devel] [PATCH] 0023 Don't crash when searching with empty relationship options

2012-03-09 Thread Petr Viktorin

See commit message.

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

--
Petr³
From 074afddae80c7f1851f54324e746ed6a01b878f2 Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Fri, 9 Mar 2012 04:45:15 -0500
Subject: [PATCH] Don't crash when searching with empty relationship options

Empty sequences (and sequences of empty strings) are normalized
to None, but the member filter code expected a list.
This patch extends a test for missing options to also catch
false values.
The functional change is from `if param_name in options:` to
`if options.get(param_name):`; the rest of the patch is code
de-duplication and tests.

These are CSV params with csv_skipspace set, so on the CLI, empty
set is given as a string with just spaces and commas (including
the empty string).

https://fedorahosted.org/freeipa/ticket/2479
---
 ipalib/plugins/baseldap.py|   36 --
 tests/test_xmlrpc/test_netgroup_plugin.py |  108 +
 2 files changed, 122 insertions(+), 22 deletions(-)

diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index c0f25479a1460cec9b46db7f10da837d07103887..184480915f2b869ff54863e99ae4047ca0e6701f 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -1744,28 +1744,20 @@ class LDAPSearch(BaseLDAPCommand, crud.Search):
 relationship = self.obj.relationships.get(
 attr, ['member', '', 'no_']
 )
-param_name = '%s%s' % (relationship[1], to_cli(ldap_obj_name))
-if param_name in options:
-dns = []
-for pkey in options[param_name]:
-dns.append(ldap_obj.get_dn(pkey))
-flt = ldap.make_filter_from_attr(
-attr, dns, ldap.MATCH_ALL
-)
-filter = ldap.combine_filters(
-(filter, flt), ldap.MATCH_ALL
-)
-param_name = '%s%s' % (relationship[2], to_cli(ldap_obj_name))
-if param_name in options:
-dns = []
-for pkey in options[param_name]:
-dns.append(ldap_obj.get_dn(pkey))
-flt = ldap.make_filter_from_attr(
-attr, dns, ldap.MATCH_NONE
-)
-filter = ldap.combine_filters(
-(filter, flt), ldap.MATCH_ALL
-)
+# Handle positive (MATCH_ALL) and negative (MATCH_NONE)
+# searches similarly
+param_prefixes = relationship[1:]  # e.g. ('in_', 'not_in_')
+rules = ldap.MATCH_ALL, ldap.MATCH_NONE
+for param_prefix, rule in zip(param_prefixes, rules):
+param_name = '%s%s' % (param_prefix, to_cli(ldap_obj_name))
+if options.get(param_name):
+dns = []
+for pkey in options[param_name]:
+dns.append(ldap_obj.get_dn(pkey))
+flt = ldap.make_filter_from_attr(attr, dns, rule)
+filter = ldap.combine_filters(
+(filter, flt), ldap.MATCH_ALL
+)
 return filter
 
 has_output_params = global_output_params
diff --git a/tests/test_xmlrpc/test_netgroup_plugin.py b/tests/test_xmlrpc/test_netgroup_plugin.py
index 1c6b94bd20027bc3c7e550322ae27a5a05bfe426..c40b01ad623f1566ce98f60e1254b43d539752bb 100644
--- a/tests/test_xmlrpc/test_netgroup_plugin.py
+++ b/tests/test_xmlrpc/test_netgroup_plugin.py
@@ -397,6 +397,43 @@ class test_netgroup(Declarative):
 
 
 dict(
+desc='Search for netgroups using no_user',
+command=('netgroup_find', [], dict(no_user=user1)),
+expected=dict(
+count=2,
+truncated=False,
+summary=u'2 netgroups matched',
+result=[
+{
+'dn': fuzzy_netgroupdn,
+'cn': [netgroup2],
+'description': [u'Test netgroup 2'],
+'nisdomainname': [u'%s' % api.env.domain],
+},
+{
+'dn': fuzzy_netgroupdn,
+'memberhost_host': (host1,),
+'memberhost_hostgroup': (hostgroup1,),
+'cn': [netgroup1],
+'description': [u'Test netgroup 1'],
+'nisdomainname': [u'%s' % api.env.domain],
+},
+],
+),
+),
+
+dict(
+desc=Check %r doesn't match when searching for %s % (netgroup1, user1),
+command=('netgroup_find', [], dict(user=user1)),
+expected=dict(
+count=0,
+

[Freeipa-devel] [PATCH] 107 Content is no more overwritten by error message

2012-03-09 Thread Petr Vobornik
When an error which caused calling of report_error occur, the content of 
a facet got replaced by error message. There was no way how to force the 
facet to recreate its content and the facet became unusable.


This patch creates a container for an error message. On error, 
report_error writes its content to error container, content container is 
hidden and error container is shown. Older comment in a code suggested 
to move the error message to facet's footer. A message in a footer could 
be missed by the user and on top of that a footer is sometimes used by 
various facet and we would have to solve the same problem again.


From experience the cause of an error is usually a missing pkey in a 
path. Therefore error information suggests user to navigate to top 
level. It causes to load default facets with default values so errors in 
navigation state shouldn't happen.


Facet content is displayed back on facet_show. If user tries to display 
same object as before facet's need_update() would return false, 
therefore need_update was modified to always return true if error is 
displayed.


Also it may be possible to display facet content on refresh success. I 
tried to avoid that because it would require putting show_content calls 
to various success handlers or load methods. It would add one more thing 
developer needs to think of when overriding refresh or load method.


Reproduction:
 1) display any nested entity - ie DNS record
 2) delete its parent pkey from path - dnszone-pkey=example.com
 3) reload the page with this path

https://fedorahosted.org/freeipa/ticket/2449
--
Petr Vobornik
From b77ddffb4134e485cb2887ae627660570fa43efd Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Fri, 9 Mar 2012 13:31:08 +0100
Subject: [PATCH] Content is no more overwritten by error message

When an error which caused calling of report_error occurt, the content of a facet got replaced by error message. There was no way how to force the facet to recreate its content and the facet became unusable.

This patch creates a containter for an error message. On error,  report_error writes its content to error container, content container is hidden and error container is shown. Older comment in a code suggested to move the error message to facet's footer. A message in a footer could be missed by the user and on top of that a footer is sometimes used by various facet and we would have to solve the same problem again.

From experience the cause of an error is usually a missing pkey in a path. Therefore error information suggests user to navigate to top level. It causes to load default facets with default values so errors in navigation state shouldn't happen.

Facet content is displayed back on facet_show. If user tries to display same object as before facet's need_update() would return false, therefore need_update was modified to always return true if error is displayed.

Also it may be possible to display facet content on refresh success. I tried to avoid that because it would require putting show_content calls to various success handlers or load methods. It would add one more thing developer needs to think of when overriding refresh or load method.

Reproduction:
 1) display any nested entity - ie DNS record
 2) delete its parent pkey from path - dnszone-pkey=example.com
 3) reload the page with this path

https://fedorahosted.org/freeipa/ticket/2449
---
 install/ui/association.js  |2 +
 install/ui/details.js  |4 ++-
 install/ui/facet.js|   38 +++
 install/ui/ipa.css |   12 +++
 install/ui/test/data/ipa_init.json |5 
 ipalib/plugins/internal.py |5 
 6 files changed, 60 insertions(+), 6 deletions(-)

diff --git a/install/ui/association.js b/install/ui/association.js
index b170b39d231ef6ce22161a199b039390faca0a34..87f2fd4ab27b1aa250e9f506fa5160fbeddc4576 100644
--- a/install/ui/association.js
+++ b/install/ui/association.js
@@ -1063,6 +1063,8 @@ IPA.association_facet = function (spec) {
 var page = parseInt(IPA.nav.get_state(that.entity.name+'-page'), 10) || 1;
 if (that.table.current_page !== page) return true;
 
+if (that.error_displayed()) return true;
+
 return false;
 };
 
diff --git a/install/ui/details.js b/install/ui/details.js
index 8d5dcff685da99b73ecad4a35bbf3f02dc620a67..ee57875a0e49dc08f1c8d6a1fb78c1e9e307b67a 100644
--- a/install/ui/details.js
+++ b/install/ui/details.js
@@ -419,7 +419,9 @@ IPA.details_facet = function(spec) {
 that.needs_update = function() {
 if (that._needs_update !== undefined) return that._needs_update;
 var pkey = IPA.nav.get_state(that.entity.name+'-pkey');
-return pkey !== that.pkey;
+var needs_update = that.error_displayed();
+needs_update = needs_update || pkey !== that.pkey;
+return needs_update;
 };
 
 that.field_dirty_changed = function(dirty) {
diff 

[Freeipa-devel] [PATCH] 17 More exception handlers in ipa-client-install

2012-03-09 Thread Ondrej Hamada

https://fedorahosted.org/freeipa/ticket/2415
https://fedorahosted.org/freeipa/ticket/1995

Added exception handler to certutil operation of adding CA to the
default NSS database. If operation fails, installation is aborted and
changes are rolled back. #2415

If obtaining host TGT fails, the installation is aborted and changes are
rolled back. #1995

--
Regards,

Ondrej Hamada
FreeIPA team
jabber: oh...@jabbim.cz
IRC: ohamada

From e3e556d68f4f04df5ca948341d6b8c0384df47b6 Mon Sep 17 00:00:00 2001
From: Ondrej Hamada oham...@redhat.com
Date: Fri, 9 Mar 2012 13:04:23 +0100
Subject: [PATCH] More exception handlers in ipa-client-install

Added exception handler to certutil operation of adding CA to the
default NSS database. If operation fails, installation is aborted and
changes are rolled back.

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

If obtaining host TGT fails, the installation is aborted and changes are
rolled back.

https://fedorahosted.org/freeipa/ticket/1995
---
 ipa-client/ipa-install/ipa-client-install |9 -
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index 22c6a925672b1e5e769bf09eaf49e48988bbea41..604283ae4da3ac2e668d9475a77f7053d5bc0ab2 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -1337,7 +1337,11 @@ def install(options, env, fstore, statestore):
 print Configured /etc/sssd/sssd.conf
 
 # Add the CA to the default NSS database and trust it
-run([/usr/bin/certutil, -A, -d, /etc/pki/nssdb, -n, IPA CA, -t, CT,C,C, -a, -i, /etc/ipa/ca.crt])
+try:
+run([/usr/bin/certutil, -A, -d, /etc/pki/nssdb, -n, IPA CA, -t, CT,C,C, -a, -i, /etc/ipa/ca.crt])
+except CalledProcessError, e:
+print sys.stderr, Failed to add CA to the default NSS database.
+return CLIENT_INSTALL_ERROR
 
 # If on master assume kerberos is already configured properly.
 if not options.on_master:
@@ -1354,6 +1358,9 @@ def install(options, env, fstore, statestore):
 api.Backend.xmlclient.connect()
 except CalledProcessError, e:
 print sys.stderr, Failed to obtain host TGT.
+# fail to obtain ticket makes it impossible to login and bind from sssd to LDAP,
+# abort installation and rollback changes
+return CLIENT_INSTALL_ERROR
 
 if not options.on_master:
 client_dns(cli_server, hostname, options.dns_updates)
-- 
1.7.6.5

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

Re: [Freeipa-devel] [PATCH] 983 add subject key identifier

2012-03-09 Thread Martin Kosek
On Wed, 2012-03-07 at 17:49 -0500, Rob Crittenden wrote:
 Add subject key identifier to the dogtag server cert profile.
 
 This will add it on upgrades too and any new certs issued will have a 
 subject key identifier set.
 
 If the user has customized the profile themselves then this won't be 
 applied.
 
 rob

NACK

I found few issues with the patch:

1) There is an extraneous pdb statement:
+import pdb; pdb.set_trace()

2) A name of config file should be put to some variable once and not
created every time again in enable_subject_key_identifier. It would be
much more readable and less error prone:
+installutils.set_directive('/var/lib/%
s/profiles/ca/caIPAserviceCert.cfg' % PKI_INSTANCE_NAME,
'policyset.serverCertSet.list', '1,2,3,4,5,6,7,8,10', quotes=False,
separator='=')
+installutils.set_directive('/var/lib/%
s/profiles/ca/caIPAserviceCert.cfg' % PKI_INSTANCE_NAME,
'policyset.serverCertSet.10.constraint.class_id', 'noConstraintImpl',
quotes=False, separator='=')
...

3) We do not handle gracefully missing config file. This is what happens
when replica without CA is upgraded:
# rpm -Uvh --force /home/mkosek/dist-review/rpms/freeipa-*
Preparing...### [100%]
   1:freeipa-python ### [ 17%]
   2:freeipa-client ### [ 33%]
   3:freeipa-admintools ### [ 50%]
   4:freeipa-server ### [ 67%]
Upgraded /etc/httpd/conf.d/ipa-pki-proxy.conf to version 1
Traceback (most recent call last):
  File /usr/sbin/ipa-upgradeconfig, line 301, in module
sys.exit(main())
  File /usr/sbin/ipa-upgradeconfig, line 297, in main
upgrade_ipa_profile(krbctx.default_realm)
  File /usr/sbin/ipa-upgradeconfig, line 243, in upgrade_ipa_profile
if ca.enable_subject_key_identifier():
  File /usr/lib/python2.7/site-packages/ipaserver/install/cainstance.py, line 
1079, in enable_subject_key_identifier
setlist = 
installutils.get_directive('/var/lib/%s/profiles/ca/caIPAserviceCert.cfg' % 
PKI_INSTANCE_NAME, 'policyset.serverCertSet.list', separator='=')
  File /usr/lib/python2.7/site-packages/ipaserver/install/installutils.py, 
line 429, in get_directive
fd = open(filename, r)
IOError: [Errno 2] No such file or directory: 
'/var/lib/pki-ca/profiles/ca/caIPAserviceCert.cfg'
   5:freeipa-server-selinux ### [ 83%]
   6:freeipa-debuginfo  ### [100%]

 1. Martin

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


Re: [Freeipa-devel] [PATCH] 17 More exception handlers in ipa-client-install

2012-03-09 Thread Martin Kosek
On Fri, 2012-03-09 at 14:18 +0100, Ondrej Hamada wrote:
 https://fedorahosted.org/freeipa/ticket/2415
 https://fedorahosted.org/freeipa/ticket/1995
 
 Added exception handler to certutil operation of adding CA to the
 default NSS database. If operation fails, installation is aborted and
 changes are rolled back. #2415
 
 If obtaining host TGT fails, the installation is aborted and changes are
 rolled back. #1995

ACK. Pushed to master, ipa-2-2.

Martin

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


Re: [Freeipa-devel] [PATCH] 16 Netgroup nisdomain and hosts validation

2012-03-09 Thread Martin Kosek
On Thu, 2012-03-08 at 14:52 +0100, Ondrej Hamada wrote:
 Netgroup nisdomain and hosts validation
 
 nisdomain validation:
 Added pattern to the 'nisdomain' parameter to validate the specified
 nisdomain name. According to most common use cases the same patter as
 for netgroup should fit. Unit-tests added.
 
 https://fedorahosted.org/freeipa/ticket/2447
 
 hosts validation:
 Added precallback to netgroup_add_member. It validates the specified
 hostnames and raises ValidationError exception for invalid hostnames.
 Unit-test added.
 
 https://fedorahosted.org/freeipa/ticket/2448


I checked the host validation part and it could be improved. Issue
described in #2447 (you have switched the ticket IDs) affects all
objects that allow external hosts, users, ..., i.e. those who call
add_external_post_callback in their post_callback.

Should we fix all of these when we deal with this issue? Otherwise user
could do something like this:
# ipa sudorule-add-user foo --users=a+b
  Rule name: foo
  Enabled: TRUE
  External User: a+b

We could create a similar function called add_external_pre_callback()
and pass it attribute name and validating function (which would be
common with the linked object). It would then do the validation for all
these affected objects consistently and without redundant code.

I didn't liked much the implemented pre_callback anyway

+def pre_callback(self, ldap, dn, found, not_found, *keys,
**options):
+# validate entered hostnames
+if 'host' in options:
+invalid_hostnames=[]
+for hostname in options['host']:
+try:
+validate_hostname(hostname, False)
+except ValueError:
+invalid_hostnames.append(hostname)
+if invalid_hostnames:
+raise errors.ValidationError(name='host',
error='hostnames:\%s\ contain invalid characters' %
','.join(invalid_hostnames))
+return dn

I would rather raise the ValidationError with the first invalid hostname
and tell what's wrong (function validate_hostname tells it to you). If
you go with the proposed approach, you wouldn't have to deal with
formatting error messages, you would just raise the one returned by the
validator shared with the linked LDAP object (hostname, user, ...).

Martin

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


[Freeipa-devel] [PATCH] 0024 Fix expected exception checking in tests

2012-03-09 Thread Petr Viktorin

Can you spot the bug in this test code?

try:
do_invalid_operation()
except ExpectedError:
pass


Our test suite had several of those.
Nose provides nice tools, `raises` (a decorator) and `assert_raises` (a 
context manager) that make checking expected exceptions a lot easier and 
less error-prone. This patch makes our tests use them.


If you didn't catch it, the error is that the test will pass when no 
exception is raised. Some of our tests handled that by adding an `else: 
assert False`, or an `assert False` at the end of the try block.
For consistency, the patch switches these correct ones to 
raises/assert_raises as well.


I've also uncovered and fixed a few test bugs that were hidden by this.

--
Petr³
From 378576d6252f6b304671d13b12276beffd7dd12d Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Fri, 9 Mar 2012 09:41:16 -0500
Subject: [PATCH] Use nose tools to check for exceptions

Some of our tests checked for exceptions using an error-prone
try block: they allowed the expected exception to pass, but sometimes
forgot an else block, so the test passed when an exception wasn't
thrown.

This changes the tests to use the appropriate nose tools (raises,
assert_raises).
For consistency, tests that had a correct else block are also changed.

Also fix some test problems that were hidden by the above:
- in some sudorule and HBAC tests, change the *_add_user argument name
  from `users` to `user`
- don't remove HBAC testing data while it was still used
---
 tests/test_install/test_updates.py |   15 ++---
 tests/test_xmlrpc/test_automount_plugin.py |   82 --
 tests/test_xmlrpc/test_cert.py |7 +-
 tests/test_xmlrpc/test_hbac_plugin.py  |  103 
 tests/test_xmlrpc/test_passwd_plugin.py|9 +--
 tests/test_xmlrpc/test_sudorule_plugin.py  |   74 
 6 files changed, 103 insertions(+), 187 deletions(-)

diff --git a/tests/test_install/test_updates.py b/tests/test_install/test_updates.py
index eb376f19113e534563d02eae51a798f7b9d9c773..8b9ac28eb68535d3820feea6d47639d32df7be56 100644
--- a/tests/test_install/test_updates.py
+++ b/tests/test_install/test_updates.py
@@ -24,7 +24,8 @@ import os
 import sys
 import ldap
 import nose
-from tests.util import raises, PluginTester
+from nose.tools import raises
+from tests.util import PluginTester
 from tests.data import unicode_str
 from ipalib import api
 from ipalib import errors
@@ -182,20 +183,16 @@ class test_update(object):
 
 assert(modified == True)
 
+@raises(BadSyntax)
 def test_8_badsyntax(self):
 
 Test the updater with an unknown keyword
 
-try:
-modified = self.updater.update([self.testdir + 8_badsyntax.update])
-except BadSyntax:
-pass
+modified = self.updater.update([self.testdir + 8_badsyntax.update])
 
+@raises(BadSyntax)
 def test_9_badsyntax(self):
 
 Test the updater with an incomplete line
 
-try:
-modified = self.updater.update([self.testdir + 9_badsyntax.update])
-except BadSyntax:
-pass
+modified = self.updater.update([self.testdir + 9_badsyntax.update])
diff --git a/tests/test_xmlrpc/test_automount_plugin.py b/tests/test_xmlrpc/test_automount_plugin.py
index cfea61270fca423fe29e00747b507d170e05d255..6abc44f4d38a4fe85f160c5dd37e45f709ae5f42 100644
--- a/tests/test_xmlrpc/test_automount_plugin.py
+++ b/tests/test_xmlrpc/test_automount_plugin.py
@@ -22,6 +22,8 @@ Test the `ipalib/plugins/automount.py' module.
 
 
 import sys
+from nose.tools import raises, assert_raises  # pylint: disable=E0611
+
 from xmlrpc_test import XMLRPC_test, assert_attr_equal
 from ipalib import api
 from ipalib import errors
@@ -81,12 +83,8 @@ class test_automount(XMLRPC_test):
 
 Test adding a duplicate key using `xmlrpc.automountkey_add` method.
 
-try:
+with assert_raises(errors.DuplicateEntry):
 api.Command['automountkey_add'](self.locname, self.mapname, **self.key_kw)
-except errors.DuplicateEntry:
-pass
-else:
-assert False
 
 def test_5_automountmap_show(self):
 
@@ -153,12 +151,8 @@ class test_automount(XMLRPC_test):
 assert_attr_equal(res, 'failed', '')
 
 # Verify that it is gone
-try:
+with assert_raises(errors.NotFound):
 api.Command['automountkey_show'](self.locname, self.mapname, **delkey_kw)
-except errors.NotFound:
-pass
-else:
-assert False
 
 def test_c_automountlocation_del(self):
 
@@ -169,12 +163,8 @@ class test_automount(XMLRPC_test):
 assert_attr_equal(res, 'failed', '')
 
 # Verify that it is gone
-try:
+with assert_raises(errors.NotFound):
 api.Command['automountlocation_show'](self.locname)
-except 

[Freeipa-devel] Git post-commit hook

2012-03-09 Thread Petr Viktorin

Let me share my post-commit hook with you.

Save this as .git/hooks/post-commit, chmod +x it, and each time you make 
a commit, it will run pylint and a PEP8 check on all .py files you've 
changed.


It takes a long time, but it runs after the commit is made, so you can 
quite safely kill it or switch to a different terminal and continue working.


It's also quite noisy -- IPA doesn't really conform to its style guide, 
mainly because there are lots of long lines. I always like to check 
around the changes I made for little style fixes to sneak into my patch.


yum install python-pep8 for the style checks.

Improvements welcome!

--
Petr³
#!/bin/sh
#
# Post-commit hook for FreeIPA

if git rev-parse --verify HEAD~ /dev/null 21
then
against=HEAD~
else
# Initial commit: diff against an empty tree object
against=4b825dc642cb6eb9a060e54bf8d69288fbee4904
fi

FILES=`git diff --cached --name-only $against | grep '\.py$'`
echo $FILES
if [ -n $FILES ]; then
echo Commit `git rev-parse --verify HEAD`: Linting...
./make-lint $FILES
pep8 $FILES -r
fi
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

[Freeipa-devel] [PATCH] 107 Fixed evaluating checkbox dirty status

2012-03-09 Thread Petr Vobornik

Problem:
When value in checkbox is modified twice in a row (so it is at its 
original value) an 'undo' button is still visible even when it shouldn't be.


Cause:
IPA server sends boolean values as 'TRUE' or 'FALSE' (strings). 
Checkbox_widget converts them to JavaScript? boolean (true, false). Save 
method in checkbox_widget is returning array with a boolean. So 
test_dirty method always evaluates to dirty because 'FALSE' != false.


This patch is fixing the problem.

Note (future enhancements):
As we were talking before about making fields less dependent on widget 
types. The dependency comes from the fact that dirty evaluation is in 
field. I plan to move the core to widget. I'm thinking about something 
like:

Widget would have:
  * input value parser - ie for parsing strings to booleans - configurable
  * original value (parsed)
  * inner state (inner_save())
  * is_dirty() - compare inner state with original value
  * output formatter - can make boolean back to strings (just example) 
- configurable

  * save() would return array of formatted values

Field:
  * load(record) would pick values from record as now
  * is_dirty - needed for facets. Would be: dirty = 'one of associated 
widgets is dirty'
  * save() - merge associated widgets values - usually only on array 
from one widget


Maybe input and output formatters should be in field.

https://fedorahosted.org/freeipa/ticket/2494
--
Petr Vobornik
From 2b800fb59a7d04be9a67a5216e20168bec6aae8e Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Fri, 9 Mar 2012 15:52:05 +0100
Subject: [PATCH] Fixed evaluating checkbox dirty status

Problem:
When value in checkbox is modified twice in a row (so it is at its original value) an 'undo' button is still visible even when it shouldn't be.

Cause:
IPA server sends boolean values as 'TRUE' or 'FALSE' (strings). Checkbox_widget converts them to JavaScript? boolean (true, false). Save method in checkbox_widget is returning array with a boolean. So test_dirty method always evaluates to dirty because 'FALSE' != false.

This patch is fixing the problem.

https://fedorahosted.org/freeipa/ticket/2494
---
 install/ui/field.js |   24 +---
 install/ui/group.js |2 +-
 install/ui/test/widget_tests.js |4 +++-
 install/ui/widget.js|   12 +++-
 4 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/install/ui/field.js b/install/ui/field.js
index 5f073705d665b1b1af72e6e21686af545e4f55ab..162ec81ba96a740a6267d4a5f3241279eb69f131 100644
--- a/install/ui/field.js
+++ b/install/ui/field.js
@@ -487,6 +487,23 @@ IPA.checkbox_field = function(spec) {
 var that = IPA.field(spec);
 
 that.checked = spec.checked || false;
+that.boolean_formatter = IPA.boolean_formatter();
+
+that.load = function(record) {
+
+that.record = record;
+
+that.values = that.get_value(record, that.param);
+
+var value = that.boolean_formatter.parse(that.values);
+if (value === '') value = that.widget.checked; //default value
+
+that.values = [value];
+
+that.load_writable(record);
+
+that.reset();
+};
 
 that.widgets_created = function() {
 
@@ -510,13 +527,6 @@ IPA.checkboxes_field = function(spec) {
 
 var that = IPA.field(spec);
 
-that.checkbox_load = that.load;
-/*
-// a checkbox will always have a value, so it's never required
-that.is_required = function() {
-return false;
-};
-*/
 return that;
 };
 
diff --git a/install/ui/group.js b/install/ui/group.js
index 93b2fe0fff62579fab7bb4f3e8b54c44099f37de..5ad9ba1f1d887c1297573a90c209ace00e3f3a86 100644
--- a/install/ui/group.js
+++ b/install/ui/group.js
@@ -140,7 +140,7 @@ IPA.group_nonposix_checkbox_widget = function (spec) {
 };
 
 IPA.widget_factories['nonposix_checkbox'] = IPA.group_nonposix_checkbox_widget;
-IPA.field_factories['nonposix_checkbox'] = IPA.checkbox_fields;
+IPA.field_factories['nonposix_checkbox'] = IPA.checkbox_field;
 
 IPA.group_adder_dialog = function(spec) {
 
diff --git a/install/ui/test/widget_tests.js b/install/ui/test/widget_tests.js
index 951f943a10d8e560b10a73b658972a29147107d7..489572c2c21077216f65dc47ac84919b85e71176 100644
--- a/install/ui/test/widget_tests.js
+++ b/install/ui/test/widget_tests.js
@@ -223,7 +223,9 @@ test(Testing checkbox widget., function() {
 spec = {name:'title'};
 base_widget_test('test_value');
 
-var mock_record = { 'title': 'TRUE' };
+//Changing mock record from 'TRUE' to true. Value normalization is field's
+//job. Checkbox should work with booleans values.
+ var mock_record = { 'title': [true] };
 
 widget.update(mock_record.title);
 same(widget.save(),[true], Checkbox is set);
diff --git a/install/ui/widget.js b/install/ui/widget.js
index f906d165c6202b3e2b54b9299cfcafed9bccb0e4..83f07594a1eea19bcd18e770ebc3bf20c7b9a214 100644
--- a/install/ui/widget.js
+++ b/install/ui/widget.js
@@ -632,20 +632,14 @@ 

Re: [Freeipa-devel] IPAv2 on SL6.2 using NIS fails with Failed password error

2012-03-09 Thread Dmitri Pal
On 03/08/2012 07:49 PM, Joshua Dotson wrote:
 Well

 I think I can now answer my own question.

 The following is
 from: http://fedoraproject.org/wiki/QA:Testcase_freeipav2_nis

 Password Hashes
 You may notice that password hashes are not available, even when
 you attempt to retrieve entries as root. As this is the default
 behavior, a prospective client system would need to also be
 configured to use either Kerberos or LDAP to check user passwords.

 I'm sorry for the spam.. :-)... And also, my inconsistent hosts and
 IP's below are the result of a failed obfuscation, rather than actual
 inconsistencies in my config.

 Cheers and thanks for FreeIPA!


Joshua is this just test of waters or you actually plan to use NIS on 6.2?
It seams odd as 6.2 has much more superior solution (SSSD configured
with ipa-client) then NIS.
NIS support is mostly for legacy systems that can't do the LDAP.

As far as I understand underlying DS can also be configured to create
weak hashes needed for NIS but it is not recommended. But this is
something that gurus should confirm.


 -Joshua

 P.S. I guess I'll go some other route to authenticate these ancient
 Ubuntu 9.04 boxes to IPA. lol


 On Thu, Mar 8, 2012 at 7:29 PM, freeipa-devel-requ...@redhat.com
 mailto:freeipa-devel-requ...@redhat.com wrote:

 Send Freeipa-devel mailing list submissions to
freeipa-devel@redhat.com mailto:freeipa-devel@redhat.com

 To subscribe or unsubscribe via the World Wide Web, visit
https://www.redhat.com/mailman/listinfo/freeipa-devel
 or, via email, send a message with subject or body 'help' to
freeipa-devel-requ...@redhat.com
 mailto:freeipa-devel-requ...@redhat.com

 You can reach the person managing the list at
freeipa-devel-ow...@redhat.com
 mailto:freeipa-devel-ow...@redhat.com

 When replying, please edit your Subject line so it is more specific
 than Re: Contents of Freeipa-devel digest...


 Today's Topics:

   1. IPAv2 on SL6.2 using NIS fails with Failed   password error
  (Joshua Dotson)


 --

 Message: 1
 Date: Thu, 8 Mar 2012 19:29:10 -0500
 From: Joshua Dotson j...@knoesis.org mailto:j...@knoesis.org
 To: freeipa-devel@redhat.com mailto:freeipa-devel@redhat.com
 Subject: [Freeipa-devel] IPAv2 on SL6.2 using NIS fails with Failed
password error
 Message-ID:
  
  canlzmlhi99zk986f4mh0pcykrrhx3wgdk7crw+34q3eofbm...@mail.gmail.com
 
 mailto:canlzmlhi99zk986f4mh0pcykrrhx3wgdk7crw%2b34q3eofbm...@mail.gmail.com
 Content-Type: text/plain; charset=iso-8859-1

 Hi All,

 I'm having a problem with my IPA installs; I can't seem to get the
 NIS mode
 to work.  I tried it with and without 'Migration Mode' enabled.

 I bind to it and 'getent passwd' and 'getent group' just fine, but
 when I
 type my password (post initial kinit password change) in for ssh,
 I get
 permission denied and the following in my client-side
 /var/log/secure log:

 Mar  8 18:15:07 bastion sshd[18480]: Failed password for bob from
 192.168.5.68 port 50788 ssh2
 Mar  8 18:15:22 bastion sshd[18480]: Failed password for bob from
 192.168.5.68 port 50788 ssh2
 Mar  8 18:46:13 bastion sshd[18556]: pam_unix(sshd:auth):
 authentication
 failure; logname= uid=0 euid=0 tty=ssh ruser= rhost=192.168.6.68
  user=bob
 Mar  8 18:46:16 bastion sshd[18556]: Failed password for bob from
 192.168.5.68 port 50839 ssh2

 On the server, I can find no error on the server side, matching the
 timestamp of when I attempt login from a third host to the bastion
 host
 (see below).

 Am I mistaken that IPAv2 provides backwards compatible NIS, without
 client-side SSSD, KRB5 and the like?  Am I missing a service or
 something?

 Thanks very much!  Please excuse the long email.  Perhaps I'm too
 eager.
 lol  :-)

 -Joshua.

 BACKGROUND INFO FOLLOWS=

 Here are the details of my install, which is my fourth IPA
 install, so far.
  As a side note, however, I've not been able to get the NIS mode
 working,
 yet.


   - 2 nearly identical KVM's to test this. (1 for server and 1 for NIS
   client)
   - x86_64
   - ext4 over LVM over qcow2 over NFSv3
   - using virtio
   - Scientific Linux 6.2 minimal install from GUI of Install DVD
   - all available yum updates applied
   - iptables off
   - ipv4 only
   - added self FQDN to both /etc/hosts files
   - NetworkManager off in favor of network
   - static public IP's
   - Used the following commands to install my IPA server:

 # yum -y install \
ipa-server \
bind \
bind-dyndb-ldap

 # ipa-server-install \
  -a 'admin_pass_example' \
  --hostname=ipa.example.com