Re: [Freeipa-devel] [PATCH] 802 add message summary to sudorule
On Wed, 2011-06-15 at 17:29 +, JR Aquino wrote: On Jun 14, 2011, at 6:36 PM, Rob Crittenden wrote: Some of the sudorule commands were missing a message summary. ticket https://fedorahosted.org/freeipa/ticket/1255 rob freeipa-rcrit-802-sudo.patch___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel NACK error: patch failed: ipalib/plugins/sudorule.py:189 error: ipalib/plugins/sudorule.py: patch does not apply Appears to perhaps be off by 1 line number. You might have to rebase. I already ack-ed and pushed this patch to master, ipa-2-0. It applied to the branches without any problem. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 083 Improve IP address handling in IPA option parser
Implements a way to pass match_local and parse_netmask parameters to IP option checker. Now, there is just one common option type ip with new optional attributes ip_local and ip_netmask which can be used to pass IP address validation parameters. https://fedorahosted.org/freeipa/ticket/1333 From 8c6dd974cd734109e06c9cd1ca45bf8b09310793 Mon Sep 17 00:00:00 2001 From: Martin Kosek mko...@redhat.com Date: Thu, 16 Jun 2011 10:47:11 +0200 Subject: [PATCH] Improve IP address handling in IPA option parser Implements a way to pass match_local and parse_netmask parameters to IP option checker. Now, there is just one common option type ip with new optional attributes ip_local and ip_netmask which can be used to pass IP address validation parameters. https://fedorahosted.org/freeipa/ticket/1333 --- install/tools/ipa-dns-install |4 ++-- install/tools/ipa-replica-install |2 +- install/tools/ipa-replica-prepare |3 ++- install/tools/ipa-server-install |5 +++-- ipapython/config.py | 11 +++ 5 files changed, 15 insertions(+), 10 deletions(-) diff --git a/install/tools/ipa-dns-install b/install/tools/ipa-dns-install index 39998ac47b02244aaec1d09a14980db25ec636f4..b5295b5c7567c3d559ad808d1752d79c1d915573 100755 --- a/install/tools/ipa-dns-install +++ b/install/tools/ipa-dns-install @@ -38,9 +38,9 @@ def parse_options(): parser.add_option(-d, --debug, dest=debug, action=store_true, default=False, help=print debugging information) parser.add_option(--ip-address, dest=ip_address, - type=ipnet, help=Master Server IP Address) + type=ip, ip_netmask=True, ip_local=True, help=Master Server IP Address) parser.add_option(--forwarder, dest=forwarders, action=append, - type=ipaddr, help=Add a DNS forwarder) + type=ip, help=Add a DNS forwarder) parser.add_option(--no-forwarders, dest=no_forwarders, action=store_true, default=False, help=Do not add any DNS forwarders, use root servers instead) parser.add_option(--no-reverse, dest=no_reverse, diff --git a/install/tools/ipa-replica-install b/install/tools/ipa-replica-install index f91ac51a60f848f57e5820d609ef6c6356ed5246..c39d992de8c42a1d1e1e641e541aacb705946d40 100755 --- a/install/tools/ipa-replica-install +++ b/install/tools/ipa-replica-install @@ -64,7 +64,7 @@ def parse_options(): parser.add_option(--setup-dns, dest=setup_dns, action=store_true, default=False, help=configure bind with our zone) parser.add_option(--forwarder, dest=forwarders, action=append, - type=ipaddr, help=Add a DNS forwarder) + type=ip, help=Add a DNS forwarder) parser.add_option(--no-forwarders, dest=no_forwarders, action=store_true, default=False, help=Do not add any DNS forwarders, use root servers instead) parser.add_option(--no-reverse, dest=no_reverse, action=store_true, diff --git a/install/tools/ipa-replica-prepare b/install/tools/ipa-replica-prepare index 8117bfcddc32a826869973485718c63875df76e1..97dd96a19befe3740bfde5fa8e1ab062c875c583 100755 --- a/install/tools/ipa-replica-prepare +++ b/install/tools/ipa-replica-prepare @@ -54,7 +54,8 @@ def parse_options(): parser.add_option(-p, --password, dest=password, help=Directory Manager (existing master) password) parser.add_option(--ip-address, dest=ip_address, - type=ipnet, help=Add A and PTR records of the future replica) + type=ip, ip_netmask=True, + help=Add A and PTR records of the future replica) parser.add_option(--ca, dest=ca_file, default=/root/cacert.p12, help=Location of CA PKCS#12 file, default /root/cacert.p12) parser.add_option(--no-pkinit, dest=setup_pkinit, action=store_false, diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install index 8fb13a3a76819b7b34b0b7196409d950c3737e6d..886d391a26664faedb8fda084f4dd90ed5540e90 100755 --- a/install/tools/ipa-server-install +++ b/install/tools/ipa-server-install @@ -100,11 +100,12 @@ def parse_options(): help=File containing PKCS#10 of the external CA chain) parser.add_option(--hostname, dest=host_name, help=fully qualified name of server) parser.add_option(--ip-address, dest=ip_address, - type=ipnet, help=Master Server IP Address) + type=ip, ip_netmask=True, ip_local=True, + help=Master Server IP Address) parser.add_option(--setup-dns, dest=setup_dns, action=store_true, default=False, help=configure bind with our zone) parser.add_option(--forwarder, dest=forwarders, action=append, - type=ipaddr, help=Add a DNS forwarder) + type=ip, help=Add a DNS forwarder)
Re: [Freeipa-devel] [PATCH] 799 The IP address provided to ipa-server-install must be local
On Wed, 2011-06-15 at 14:29 -0400, Rob Crittenden wrote: Rob Crittenden wrote: Martin Kosek wrote: On Tue, 2011-06-14 at 08:56 -0400, Rob Crittenden wrote: Martin Kosek wrote: On Mon, 2011-06-13 at 16:41 -0400, Rob Crittenden wrote: Compare the configured interfaces with the supplied IP address and optional netmask to determine if the interface is available. Note the subtle change when comparing addresses. We have two object types, IPNetwork and IPAddress. We should only compare addresses when we don't have an IPNetwork otherwise we can end up comparing an address to an object with a netmask and get a bad result. https://fedorahosted.org/freeipa/ticket/1175 NACK. 1) This breaks ipa-replica-prepare: # ipa-replica-prepare vm-046.idm.lab.bos.redhat.com --ip-address=10.16.78.46 Usage: ipa-replica-prepare [options] FQDN (e.g. replica.example.com) ipa-replica-prepare: error: option --ip-address: invalid IP address 10.16.78.46: No network interface matches the provided IP address and netmask Actually, this is not your fault, we just don't use IP address checking in IPAOptionParser correctly. --ip-address option in ipa-replica-prepare has type ipnet which is validated by the CheckedIPAddress. As match_local defaults to True, your new exception is raised. Ok, but is 10.16.78.46 a configured network interface? It is an IP address of new replica, i.e. its not a local network interface address. As I written, the problem is in a type of --ip-address option in ipa-replica-prepare. You can check Honza's mail for implementation hint. Ah, prepare. I tested with an existing replica file... Well, I wonder if an easier fix would be to set match_local=False by default and specifically ask to match_local when we want. Updated patch attached. rob I think this is still not right. When you let match_local default to False, --ip-address option in ipa-server-install is checked with match_local=False and thus the check required by BZ isn't made. Please check my patch 083 I sent this morning. It makes sure that IP address validation with CheckedIPAddress is run with correct parameters (i.e. match_local, parse_netmask). You may want to build your patch on top of this one. Should we be so strict and raise an exception when the IP address does not match any local interface? Maybe a warning would be enough. ipa-server-install will fail anyway few steps later in a scenario described in BZ. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 799 The IP address provided to ipa-server-install must be local
On 15.6.2011 20:29, Rob Crittenden wrote: Rob Crittenden wrote: Martin Kosek wrote: On Tue, 2011-06-14 at 08:56 -0400, Rob Crittenden wrote: Martin Kosek wrote: On Mon, 2011-06-13 at 16:41 -0400, Rob Crittenden wrote: Compare the configured interfaces with the supplied IP address and optional netmask to determine if the interface is available. Note the subtle change when comparing addresses. We have two object types, IPNetwork and IPAddress. We should only compare addresses when we don't have an IPNetwork otherwise we can end up comparing an address to an object with a netmask and get a bad result. https://fedorahosted.org/freeipa/ticket/1175 NACK. 1) This breaks ipa-replica-prepare: # ipa-replica-prepare vm-046.idm.lab.bos.redhat.com --ip-address=10.16.78.46 Usage: ipa-replica-prepare [options] FQDN (e.g. replica.example.com) ipa-replica-prepare: error: option --ip-address: invalid IP address 10.16.78.46: No network interface matches the provided IP address and netmask Actually, this is not your fault, we just don't use IP address checking in IPAOptionParser correctly. --ip-address option in ipa-replica-prepare has type ipnet which is validated by the CheckedIPAddress. As match_local defaults to True, your new exception is raised. Ok, but is 10.16.78.46 a configured network interface? It is an IP address of new replica, i.e. its not a local network interface address. As I written, the problem is in a type of --ip-address option in ipa-replica-prepare. You can check Honza's mail for implementation hint. Ah, prepare. I tested with an existing replica file... Well, I wonder if an easier fix would be to set match_local=False by default and specifically ask to match_local when we want. Updated patch attached. parse_ip_address and verify_ip_address still have match_local=True as default - it probably should be changed for the sake of consistency. The check for local IP address in parse_ip_address should be removed, it's not needed anymore, because you check it in CheckedIPAddress. rob Martin I think we need 2 new option types for IPAOptionParser such as iplocal and ipnetlocal which would be used for --ip-address option in ipa-server-install or ipa-dns-install and which would use match_local=True. Current types ip and ipnet should use match_local=False. 2) CheckedIPAddress functionality (i.e. this fix) is neither in ipa-2-0 stable branch nor in RHEL 6.1. But this should be OK since it is targeted for RHEL 6.2. Right, I wasn't planning on pushing this to 2.0. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 22 Improve IP address handling in the host-add command
On 14.6.2011 20:54, Simo Sorce wrote:
On Tue, 2011-06-14 at 14:26 -0400, Rob Crittenden wrote:
Jan Cholasta wrote:
This patch enables the user to specify netmasks in the --ip-address
option of host-add. They're used for proper DNS reverse zone and PTR
record creation. Also the IP addresses are more strictly checked (just
like in the install scripts).
https://fedorahosted.org/freeipa/ticket/1234
Do we want a reverse zone created automatically when a host is added? I
think a warning that the reverse zone doesn't exist may be adequate.
A warning is preferable as we may not be controlling that reverse zone.
Simo.
Updated patch attached. NonFatalError is raised when the reverse zone is
not found.
Honza
--
Jan Cholasta
From 003e0e71197f5b0f663c9cdc3d55c6f8a71ad1bd Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Tue, 14 Jun 2011 16:31:36 +0200
Subject: [PATCH] Improve IP address handling in the host-add command.
IP addresses are more strictly checked. Netmasks can be specified
and are used in DNS PTR record creation. DNS reverse zones are
automatically created if needed.
ticket 1234
---
ipalib/plugins/host.py | 51 +--
1 files changed, 36 insertions(+), 15 deletions(-)
diff --git a/ipalib/plugins/host.py b/ipalib/plugins/host.py
index a602df4..178d526 100644
--- a/ipalib/plugins/host.py
+++ b/ipalib/plugins/host.py
@@ -89,7 +89,7 @@ from ipalib.plugins.dns import dns_container_exists, _record_types
from ipalib.plugins.dns import add_forward_record
from ipalib import _, ngettext
from ipalib import x509
-from ipapython.ipautil import ipa_generate_password
+from ipapython.ipautil import ipa_generate_password, CheckedIPAddress
from ipalib.request import context
import base64
import nss.nss as nss
@@ -115,17 +115,30 @@ def is_forward_record(zone, str_address):
return result['count'] 0
-def get_reverse_zone(ipaddr):
+def get_reverse_zone(ipaddr, prefixlen=None):
ip = netaddr.IPAddress(ipaddr)
revdns = unicode(ip.reverse_dns)
-revzone = u''
+if prefixlen is None:
+revzone = u''
-result = api.Command['dnszone_find']()['result']
-for zone in result:
-zonename = zone['idnsname'][0]
-if revdns.endswith(zonename) and len(zonename) len(revzone):
-revzone = zonename
+result = api.Command['dnszone_find']()['result']
+for zone in result:
+zonename = zone['idnsname'][0]
+if revdns.endswith(zonename) and len(zonename) len(revzone):
+revzone = zonename
+else:
+if ip.version == 4:
+pos = 4 - prefixlen / 8
+elif ip.version == 6:
+pos = 32 - prefixlen / 4
+items = ip.reverse_dns.split('.')
+revzone = u'.'.join(items[pos:])
+
+try:
+api.Command['dnszone_show'](revzone)
+except errors.NotFound:
+revzone = u''
if len(revzone) == 0:
raise errors.NotFound(
@@ -188,7 +201,9 @@ def validate_ipaddr(ugettext, ipaddr):
Verify that we have either an IPv4 or IPv6 address.
-if not util.validate_ipaddr(ipaddr):
+try:
+ip = CheckedIPAddress(ipaddr, match_local=False)
+except:
return _('invalid IP address')
return None
@@ -340,17 +355,21 @@ class host_add(LDAPCreate):
raise errors.NotFound(
reason=_('DNS zone %(zone)s not found') % dict(zone=domain)
)
+ip = CheckedIPAddress(options['ip_address'], match_local=False)
if not options.get('no_reverse', False):
try:
+prefixlen = None
+if not ip.defaultnet:
+prefixlen = ip.prefixlen
# we prefer lookup of the IP through the reverse zone
-revzone, revname = get_reverse_zone(options['ip_address'])
+revzone, revname = get_reverse_zone(ip, prefixlen)
reverse = api.Command['dnsrecord_find'](revzone, idnsname=revname)
if reverse['count'] 0:
raise errors.DuplicateEntry(message=u'This IP address is already assigned.')
except errors.NotFound:
pass
else:
-if is_forward_record(domain, options['ip_address']):
+if is_forward_record(domain, unicode(ip)):
raise errors.DuplicateEntry(message=u'This IP address is already assigned.')
if not options.get('force', False) and not 'ip_address' in options:
util.validate_host_dns(self.log, keys[-1])
@@ -388,15 +407,17 @@ class host_add(LDAPCreate):
parts = keys[-1].split('.')
domain = unicode('.'.join(parts[1:]))
-add_forward_record(domain, parts[0], options['ip_address'])
+ip =
Re: [Freeipa-devel] [PATCH] 779 Require an imported certificate's issuer to match our issuer
On 14.6.2011 15:16, Rob Crittenden wrote: Jan Cholasta wrote: On 6.6.2011 21:25, Rob Crittenden wrote: Jan Cholasta wrote: On 26.4.2011 22:52, Rob Crittenden wrote: The goal is to not import foreign certificates. This caused a bunch of tests to fail because we had a hardcoded server certificate. Instead a developer will need to run make-testcert to create a server certificate generated by the local CA to test against. ticket 1134 rob NACK The certificate isn't verified in host-add. I suspect that certificates signed by an intermediate CA (i.e. when the certificate chain length 2) are considered invalid. Is that the desired behavior? That will work as long as the issuer is the IPA CA. I see that if we are given a service cert issued by another CA in the chain things could go badly. I'm not sure this is something to really worry about though. I guess it's not. But I'd like a second opinion on that. We really only want to support those certs we issue otherwise things like revocation get tricky, because we can't manage things we don't issue. make-testcert fails with: Traceback (most recent call last): File ./make-testcert, line 126, in module sys.exit(makecert(reqdir)) File ./make-testcert, line 105, in makecert add=True) File ./make-testcert, line 66, in run result = self.execute(method, *args, **options) File /home/jcholast/freeipa/ipalib/backend.py, line 142, in execute raise error #pylint: disable=E0702 ipalib.errors.CommandError: unknown command 'cert_request' This is probably an error on my part (tried running in on both my machine without IPA installed and on VM with IPA installed with no luck), but nonetheless it should be fixed to fail gracefully so that the tests in make test have a chance to run. Similarly, the tests which use the test certificate created by make-testcert should be skipped if the certificate isn't available. You need to take the certificate databases from a self-signed install and copy them to ~/.ipa/alias/ in order to do certificate testing. There is documentation on how to do this in tests/test_xmlrpc/test_cert.py I think this should be mandatory as certificates are a main feature of v2. No matter what I do, I'm still getting the unknown command error. Can you describe the steps needed to make make-testcert successfully run? BTW, it would be nice if make test printed an informational message when the requirements to run the tests aren't met instead of failing with some random error. You need enable_ra = True in ~/.ipa/default.conf. What I tend to do is copy /etc/ipa/default.conf from my underlying install to ~/.ipa and comment out the xmlrpc_uri. This is now caught by the script. rob These tests fail: test_host[19]: service_mod: Update u'HTTP/testhost1.idm.lab.bos.redhat@idm.lab.bos.redhat.com' ... FAIL test_host[20]: service_show: Retrieve u'HTTP/testhost1.idm.lab.bos.redhat@idm.lab.bos.redhat.com' to verify update ... FAIL because they expect the CN to be puma.greyoak.com. I'm not sure if this issue is in the scope of this patch - if it's not, then ACK. Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 799 The IP address provided to ipa-server-install must be local
Jan Cholasta wrote: On 15.6.2011 20:29, Rob Crittenden wrote: Rob Crittenden wrote: Martin Kosek wrote: On Tue, 2011-06-14 at 08:56 -0400, Rob Crittenden wrote: Martin Kosek wrote: On Mon, 2011-06-13 at 16:41 -0400, Rob Crittenden wrote: Compare the configured interfaces with the supplied IP address and optional netmask to determine if the interface is available. Note the subtle change when comparing addresses. We have two object types, IPNetwork and IPAddress. We should only compare addresses when we don't have an IPNetwork otherwise we can end up comparing an address to an object with a netmask and get a bad result. https://fedorahosted.org/freeipa/ticket/1175 NACK. 1) This breaks ipa-replica-prepare: # ipa-replica-prepare vm-046.idm.lab.bos.redhat.com --ip-address=10.16.78.46 Usage: ipa-replica-prepare [options] FQDN (e.g. replica.example.com) ipa-replica-prepare: error: option --ip-address: invalid IP address 10.16.78.46: No network interface matches the provided IP address and netmask Actually, this is not your fault, we just don't use IP address checking in IPAOptionParser correctly. --ip-address option in ipa-replica-prepare has type ipnet which is validated by the CheckedIPAddress. As match_local defaults to True, your new exception is raised. Ok, but is 10.16.78.46 a configured network interface? It is an IP address of new replica, i.e. its not a local network interface address. As I written, the problem is in a type of --ip-address option in ipa-replica-prepare. You can check Honza's mail for implementation hint. Ah, prepare. I tested with an existing replica file... Well, I wonder if an easier fix would be to set match_local=False by default and specifically ask to match_local when we want. Updated patch attached. parse_ip_address and verify_ip_address still have match_local=True as default - it probably should be changed for the sake of consistency. parse_ip_address is only used by ipa-replica-install and in that case we do want to enforce match_local, so True is fine. Similarly verify_ip_address are run on the local machine, we want enforcement. The check for local IP address in parse_ip_address should be removed, it's not needed anymore, because you check it in CheckedIPAddress. rob Martin I think we need 2 new option types for IPAOptionParser such as iplocal and ipnetlocal which would be used for --ip-address option in ipa-server-install or ipa-dns-install and which would use match_local=True. Current types ip and ipnet should use match_local=False. 2) CheckedIPAddress functionality (i.e. this fix) is neither in ipa-2-0 stable branch nor in RHEL 6.1. But this should be OK since it is targeted for RHEL 6.2. Right, I wasn't planning on pushing this to 2.0. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Honza ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 799 The IP address provided to ipa-server-install must be local
Martin Kosek wrote: On Wed, 2011-06-15 at 14:29 -0400, Rob Crittenden wrote: Rob Crittenden wrote: Martin Kosek wrote: On Tue, 2011-06-14 at 08:56 -0400, Rob Crittenden wrote: Martin Kosek wrote: On Mon, 2011-06-13 at 16:41 -0400, Rob Crittenden wrote: Compare the configured interfaces with the supplied IP address and optional netmask to determine if the interface is available. Note the subtle change when comparing addresses. We have two object types, IPNetwork and IPAddress. We should only compare addresses when we don't have an IPNetwork otherwise we can end up comparing an address to an object with a netmask and get a bad result. https://fedorahosted.org/freeipa/ticket/1175 NACK. 1) This breaks ipa-replica-prepare: # ipa-replica-prepare vm-046.idm.lab.bos.redhat.com --ip-address=10.16.78.46 Usage: ipa-replica-prepare [options] FQDN (e.g. replica.example.com) ipa-replica-prepare: error: option --ip-address: invalid IP address 10.16.78.46: No network interface matches the provided IP address and netmask Actually, this is not your fault, we just don't use IP address checking in IPAOptionParser correctly. --ip-address option in ipa-replica-prepare has type ipnet which is validated by the CheckedIPAddress. As match_local defaults to True, your new exception is raised. Ok, but is 10.16.78.46 a configured network interface? It is an IP address of new replica, i.e. its not a local network interface address. As I written, the problem is in a type of --ip-address option in ipa-replica-prepare. You can check Honza's mail for implementation hint. Ah, prepare. I tested with an existing replica file... Well, I wonder if an easier fix would be to set match_local=False by default and specifically ask to match_local when we want. Updated patch attached. rob I think this is still not right. When you let match_local default to False, --ip-address option in ipa-server-install is checked with match_local=False and thus the check required by BZ isn't made. Yes but it is checked again later. Try it, enforcement happens. Please check my patch 083 I sent this morning. It makes sure that IP address validation with CheckedIPAddress is run with correct parameters (i.e. match_local, parse_netmask). You may want to build your patch on top of this one. Should we be so strict and raise an exception when the IP address does not match any local interface? Maybe a warning would be enough. ipa-server-install will fail anyway few steps later in a scenario described in BZ. We should fail as soon as possible. By doing this before installation starts they don't have to uninstall. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 779 Require an imported certificate's issuer to match our issuer
Jan Cholasta wrote: On 14.6.2011 15:16, Rob Crittenden wrote: Jan Cholasta wrote: On 6.6.2011 21:25, Rob Crittenden wrote: Jan Cholasta wrote: On 26.4.2011 22:52, Rob Crittenden wrote: The goal is to not import foreign certificates. This caused a bunch of tests to fail because we had a hardcoded server certificate. Instead a developer will need to run make-testcert to create a server certificate generated by the local CA to test against. ticket 1134 rob NACK The certificate isn't verified in host-add. I suspect that certificates signed by an intermediate CA (i.e. when the certificate chain length 2) are considered invalid. Is that the desired behavior? That will work as long as the issuer is the IPA CA. I see that if we are given a service cert issued by another CA in the chain things could go badly. I'm not sure this is something to really worry about though. I guess it's not. But I'd like a second opinion on that. We really only want to support those certs we issue otherwise things like revocation get tricky, because we can't manage things we don't issue. make-testcert fails with: Traceback (most recent call last): File ./make-testcert, line 126, in module sys.exit(makecert(reqdir)) File ./make-testcert, line 105, in makecert add=True) File ./make-testcert, line 66, in run result = self.execute(method, *args, **options) File /home/jcholast/freeipa/ipalib/backend.py, line 142, in execute raise error #pylint: disable=E0702 ipalib.errors.CommandError: unknown command 'cert_request' This is probably an error on my part (tried running in on both my machine without IPA installed and on VM with IPA installed with no luck), but nonetheless it should be fixed to fail gracefully so that the tests in make test have a chance to run. Similarly, the tests which use the test certificate created by make-testcert should be skipped if the certificate isn't available. You need to take the certificate databases from a self-signed install and copy them to ~/.ipa/alias/ in order to do certificate testing. There is documentation on how to do this in tests/test_xmlrpc/test_cert.py I think this should be mandatory as certificates are a main feature of v2. No matter what I do, I'm still getting the unknown command error. Can you describe the steps needed to make make-testcert successfully run? BTW, it would be nice if make test printed an informational message when the requirements to run the tests aren't met instead of failing with some random error. You need enable_ra = True in ~/.ipa/default.conf. What I tend to do is copy /etc/ipa/default.conf from my underlying install to ~/.ipa and comment out the xmlrpc_uri. This is now caught by the script. rob These tests fail: test_host[19]: service_mod: Update u'HTTP/testhost1.idm.lab.bos.redhat@idm.lab.bos.redhat.com' ... FAIL test_host[20]: service_show: Retrieve u'HTTP/testhost1.idm.lab.bos.redhat@idm.lab.bos.redhat.com' to verify update ... FAIL because they expect the CN to be puma.greyoak.com. I'm not sure if this issue is in the scope of this patch - if it's not, then ACK. I'll fix them up. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 779 Require an imported certificate's issuer to match our issuer
Rob Crittenden wrote: Jan Cholasta wrote: On 14.6.2011 15:16, Rob Crittenden wrote: Jan Cholasta wrote: On 6.6.2011 21:25, Rob Crittenden wrote: Jan Cholasta wrote: On 26.4.2011 22:52, Rob Crittenden wrote: The goal is to not import foreign certificates. This caused a bunch of tests to fail because we had a hardcoded server certificate. Instead a developer will need to run make-testcert to create a server certificate generated by the local CA to test against. ticket 1134 rob NACK The certificate isn't verified in host-add. I suspect that certificates signed by an intermediate CA (i.e. when the certificate chain length 2) are considered invalid. Is that the desired behavior? That will work as long as the issuer is the IPA CA. I see that if we are given a service cert issued by another CA in the chain things could go badly. I'm not sure this is something to really worry about though. I guess it's not. But I'd like a second opinion on that. We really only want to support those certs we issue otherwise things like revocation get tricky, because we can't manage things we don't issue. make-testcert fails with: Traceback (most recent call last): File ./make-testcert, line 126, in module sys.exit(makecert(reqdir)) File ./make-testcert, line 105, in makecert add=True) File ./make-testcert, line 66, in run result = self.execute(method, *args, **options) File /home/jcholast/freeipa/ipalib/backend.py, line 142, in execute raise error #pylint: disable=E0702 ipalib.errors.CommandError: unknown command 'cert_request' This is probably an error on my part (tried running in on both my machine without IPA installed and on VM with IPA installed with no luck), but nonetheless it should be fixed to fail gracefully so that the tests in make test have a chance to run. Similarly, the tests which use the test certificate created by make-testcert should be skipped if the certificate isn't available. You need to take the certificate databases from a self-signed install and copy them to ~/.ipa/alias/ in order to do certificate testing. There is documentation on how to do this in tests/test_xmlrpc/test_cert.py I think this should be mandatory as certificates are a main feature of v2. No matter what I do, I'm still getting the unknown command error. Can you describe the steps needed to make make-testcert successfully run? BTW, it would be nice if make test printed an informational message when the requirements to run the tests aren't met instead of failing with some random error. You need enable_ra = True in ~/.ipa/default.conf. What I tend to do is copy /etc/ipa/default.conf from my underlying install to ~/.ipa and comment out the xmlrpc_uri. This is now caught by the script. rob These tests fail: test_host[19]: service_mod: Update u'HTTP/testhost1.idm.lab.bos.redhat@idm.lab.bos.redhat.com' ... FAIL test_host[20]: service_show: Retrieve u'HTTP/testhost1.idm.lab.bos.redhat@idm.lab.bos.redhat.com' to verify update ... FAIL because they expect the CN to be puma.greyoak.com. I'm not sure if this issue is in the scope of this patch - if it's not, then ACK. I'll fix them up. attached From 97ff5d4ab7573180d3051ebed0dcee4282046b2d Mon Sep 17 00:00:00 2001 From: Rob Crittenden rcrit...@redhat.com Date: Tue, 26 Apr 2011 16:45:19 -0400 Subject: [PATCH] Require an imported certificate's issuer to match our issuer. The goal is to not import foreign certificates. This caused a bunch of tests to fail because we had a hardcoded server certificate. Instead a developer will need to run make-testcert to create a server certificate generated by the local CA to test against. ticket 1134 --- Makefile |1 + ipalib/plugins/host.py |7 ++ ipalib/plugins/service.py| 27 ++- make-testcert| 135 ++ tests/test_xmlrpc/test_host_plugin.py| 41 + tests/test_xmlrpc/test_service_plugin.py | 48 +++ tests/test_xmlrpc/xmlrpc_test.py |6 ++ 7 files changed, 228 insertions(+), 37 deletions(-) create mode 100755 make-testcert diff --git a/Makefile b/Makefile index d12bb43..3e21ef4 100644 --- a/Makefile +++ b/Makefile @@ -82,6 +82,7 @@ lint: test: $(MAKE) -C install/po test_lang + ./make-testcert ./make-test release-update: diff --git a/ipalib/plugins/host.py b/ipalib/plugins/host.py index 29f659f..5d6a23f 100644 --- a/ipalib/plugins/host.py +++ b/ipalib/plugins/host.py @@ -85,6 +85,7 @@ from ipalib.plugins.service import normalize_certificate from ipalib.plugins.service import set_certificate_attrs from ipalib.plugins.service import make_pem, check_writable_file from ipalib.plugins.service import write_certificate +from ipalib.plugins.service import verify_cert_subject from ipalib.plugins.dns import dns_container_exists, _record_types from ipalib.plugins.dns import add_forward_record from ipalib import _, ngettext @@ -400,6
[Freeipa-devel] [PATCH] 181 Fixed self-service links.
In self-service mode the user's association facets have been modified
such that the entries are not linked since the only available entity
is the user entity.
A 'link' parameter has been added to IPA.association_facet and
IPA.column to control whether to link the entries. The link_handler()
method can be used to define how to handle the link.
Ticket #1072
--
Endi S. Dewata
From f9809612e0ecd55e80f4f1785790eb8024a9a916 Mon Sep 17 00:00:00 2001
From: Endi S. Dewata edew...@redhat.com
Date: Mon, 13 Jun 2011 23:18:57 -0500
Subject: [PATCH] Fixed self-service links.
In self-service mode the user's association facets have been modified
such that the entries are not linked since the only available entity
is the user entity.
A 'link' parameter has been added to IPA.association_facet and
IPA.column to control whether to link the entries. The link_handler()
method can be used to define how to handle the link.
Ticket #1072
---
install/ui/add.js |2 +-
install/ui/association.js | 79
install/ui/dialog.js |2 +
install/ui/entity.js | 29 +++-
install/ui/group.js |2 +-
install/ui/hbac.js| 21 +---
install/ui/host.js| 22 +---
install/ui/navigation.js |2 +
install/ui/search.js | 28 +++
install/ui/service.js | 22 +---
install/ui/sudo.js|4 +-
install/ui/user.js| 21
install/ui/webui.js |7 +++-
install/ui/widget.js | 34 +--
14 files changed, 98 insertions(+), 177 deletions(-)
diff --git a/install/ui/add.js b/install/ui/add.js
index 33df62abcaef6e5ec30e397b10d73ea5d8b478ff..cde6d335b42c7bcd895d079ab7d753752e89b2ba 100644
--- a/install/ui/add.js
+++ b/install/ui/add.js
@@ -83,7 +83,7 @@ IPA.add_dialog = function (spec) {
pkey = pkey[0];
}
-IPA.nav.show_page(that.entity_name, 'details', pkey);
+IPA.nav.show_page(that.entity_name, 'default', pkey);
}
);
});
diff --git a/install/ui/association.js b/install/ui/association.js
index 2115e0fe15d1a02a6e5e929081ff6bf52df591a2..8030631b4fee45dfec6bc77b26f0114259665fd4 100644
--- a/install/ui/association.js
+++ b/install/ui/association.js
@@ -277,25 +277,6 @@ IPA.association_config = function (spec) {
return that;
};
-
-IPA.association_pkey_setup = function (container, record) {
-var other_entity = this.entity_name;
-container.empty();
-var value = record[this.name];
-value = value ? value.toString() : '';
-$('a/', {
-'href': '#'+value,
-'html': value,
-'click': function (value) {
-return function() {
-IPA.nav.show_page(other_entity, 'default', value);
-return false;
-};
-}(value)
-}).appendTo(container);
-};
-
-
IPA.association_table_widget = function (spec) {
spec = spec || {};
@@ -325,14 +306,6 @@ IPA.association_table_widget = function (spec) {
return column;
};
-that.super_create_column = that.create_column;
-
-that.create_column = function(spec){
-if (spec.link_entity){
-spec.setup = IPA.association_pkey_setup;
-}
-return that.super_create_column(spec);
-};
/*this is duplicated in the facet... should be unified*/
var i;
if (spec.columns){
@@ -363,6 +336,13 @@ IPA.association_table_widget = function (spec) {
for (var i=0; icolumns.length; i++) {
column = columns[i];
column.entity_name = that.other_entity;
+
+if (column.link) {
+column.link_handler = function(value) {
+IPA.nav.show_page(that.other_entity, 'default', value);
+return false;
+};
+}
}
var adder_columns = that.adder_columns.values;
@@ -698,6 +678,7 @@ IPA.association_facet = function (spec) {
that.facet_group = spec.facet_group;
that.read_only = spec.read_only;
+that.link = spec.link === undefined ? true : spec.link;
that.associator = spec.associator || IPA.bulk_associator;
that.add_method = spec.add_method || 'add_member';
@@ -718,9 +699,6 @@ IPA.association_facet = function (spec) {
that.create_column = function(spec) {
var column = IPA.column(spec);
-if (spec.link_entity){
-column.setup = IPA.association_pkey_setup;
-}
that.add_column(column);
return column;
};
@@ -775,39 +753,26 @@ IPA.association_facet = function (spec) {
});
var columns = that.columns.values;
-if (columns.length) {
-that.table.set_columns(columns);
-
-} else {
-
-column = that.table.create_column({
-name: that.table.name,
-label:
Re: [Freeipa-devel] [PATCH] 779 Require an imported certificate's issuer to match our issuer
On 16.6.2011 15:12, Rob Crittenden wrote: Rob Crittenden wrote: Jan Cholasta wrote: On 14.6.2011 15:16, Rob Crittenden wrote: Jan Cholasta wrote: On 6.6.2011 21:25, Rob Crittenden wrote: Jan Cholasta wrote: On 26.4.2011 22:52, Rob Crittenden wrote: The goal is to not import foreign certificates. This caused a bunch of tests to fail because we had a hardcoded server certificate. Instead a developer will need to run make-testcert to create a server certificate generated by the local CA to test against. ticket 1134 rob NACK The certificate isn't verified in host-add. I suspect that certificates signed by an intermediate CA (i.e. when the certificate chain length 2) are considered invalid. Is that the desired behavior? That will work as long as the issuer is the IPA CA. I see that if we are given a service cert issued by another CA in the chain things could go badly. I'm not sure this is something to really worry about though. I guess it's not. But I'd like a second opinion on that. We really only want to support those certs we issue otherwise things like revocation get tricky, because we can't manage things we don't issue. make-testcert fails with: Traceback (most recent call last): File ./make-testcert, line 126, in module sys.exit(makecert(reqdir)) File ./make-testcert, line 105, in makecert add=True) File ./make-testcert, line 66, in run result = self.execute(method, *args, **options) File /home/jcholast/freeipa/ipalib/backend.py, line 142, in execute raise error #pylint: disable=E0702 ipalib.errors.CommandError: unknown command 'cert_request' This is probably an error on my part (tried running in on both my machine without IPA installed and on VM with IPA installed with no luck), but nonetheless it should be fixed to fail gracefully so that the tests in make test have a chance to run. Similarly, the tests which use the test certificate created by make-testcert should be skipped if the certificate isn't available. You need to take the certificate databases from a self-signed install and copy them to ~/.ipa/alias/ in order to do certificate testing. There is documentation on how to do this in tests/test_xmlrpc/test_cert.py I think this should be mandatory as certificates are a main feature of v2. No matter what I do, I'm still getting the unknown command error. Can you describe the steps needed to make make-testcert successfully run? BTW, it would be nice if make test printed an informational message when the requirements to run the tests aren't met instead of failing with some random error. You need enable_ra = True in ~/.ipa/default.conf. What I tend to do is copy /etc/ipa/default.conf from my underlying install to ~/.ipa and comment out the xmlrpc_uri. This is now caught by the script. rob These tests fail: test_host[19]: service_mod: Update u'HTTP/testhost1.idm.lab.bos.redhat@idm.lab.bos.redhat.com' ... FAIL test_host[20]: service_show: Retrieve u'HTTP/testhost1.idm.lab.bos.redhat@idm.lab.bos.redhat.com' to verify update ... FAIL because they expect the CN to be puma.greyoak.com. I'm not sure if this issue is in the scope of this patch - if it's not, then ACK. I'll fix them up. attached ACK Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 29 Raise DuplicateEntry Error when adding a duplicate sudo option
JR Aquino wrote: On Jun 15, 2011, at 8:03 AM, Rob Crittenden wrote: A minor issue and a question. The minor issue is you changed a couple of options from optional to mandatory, which is fine, but we need to bump up the minor version in VERSION (older clients otherwise could not send the string and blow things up). Is there a rule of thumb or document that details when this is appropriate? The question is, should we raise EmptyModList() when removing an option that doesn't exist or NotFound(reason=_())? I think the second might be more explanatory but might be harder for handle in scripts (how would you distinguish between entry not found and option not found)? rob As per IRC conversation: Added new Exception: AttrValueNotFound Incremented minor version in VERSION Adjusted API 1276 (Raise AttrValueNotFound when trying to remove a non-existent option from Sudo rule) 1277 (Raise DuplicateEntry Error when adding a duplicate sudo option) 1308 (Make sudooption a required option for sudorule_remove_option) This is very close, found a couple more issues: I don't think I was very clear in what to update in VERSION, you want it to look like this: diff --git a/VERSION b/VERSION index 6cbf732..e31f0d0 100644 --- a/VERSION +++ b/VERSION @@ -79,4 +79,4 @@ IPA_DATA_VERSION=2010061412 # # IPA_API_VERSION_MAJOR=2 -IPA_API_VERSION_MINOR=5 +IPA_API_VERSION_MINOR=6 Two tests are failing. One is failing because externalhost is returned as a tuple (rather than not at all). The second because sudorule_remove_option has changed the type of data being returned. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 179 Fixed paging for indirect members.
On 06/15/2011 06:10 PM, Endi Sukma Dewata wrote: Since ticket #1273 has been fixed, the indirect members can be shown using the regular association facet which supports paging. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ACK. Pushed to master ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 804 slight perf improvement
This patch adds the production mode test to a few more places in the
code. The speed increase is slight, a few hundred ms in my tests, but
every little bit helps.
ticket 1023
rob
From 3eae1ef4f31a4ec5d1f9e16b2c9bc06f8ea41cf8 Mon Sep 17 00:00:00 2001
From: Rob Crittenden rcrit...@redhat.com
Date: Thu, 16 Jun 2011 11:31:41 -0400
Subject: [PATCH] Slight performance improvement by not doing some checking in production mode
These changes save a few hundred ms but every little bit helps.
ticket 1023
---
ipalib/plugable.py | 18 --
1 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/ipalib/plugable.py b/ipalib/plugable.py
index 48ff919..56546bb 100644
--- a/ipalib/plugable.py
+++ b/ipalib/plugable.py
@@ -74,7 +74,8 @@ class SetProxy(ReadOnly):
if type(s) not in allowed:
raise TypeError('%r not in %r' % (type(s), allowed))
self.__s = s
-lock(self)
+if not is_production_mode(self):
+lock(self)
def __len__(self):
@@ -293,9 +294,11 @@ class Registrar(DictProxy):
def __base_iter(self):
for (base, sub_d) in self.__allowed.iteritems():
-assert inspect.isclass(base)
+if not is_production_mode(self):
+assert inspect.isclass(base)
name = base.__name__
-assert not hasattr(self, name)
+if not is_production_mode(self):
+assert not hasattr(self, name)
setattr(self, name, MagicDict(sub_d))
yield (name, base)
@@ -308,7 +311,8 @@ class Registrar(DictProxy):
:param klass: The plugin class to find bases for.
-assert inspect.isclass(klass)
+if not is_production_mode(self):
+assert inspect.isclass(klass)
found = False
for (base, sub_d) in self.__allowed.iteritems():
if issubclass(klass, base):
@@ -599,7 +603,8 @@ class API(DictProxy):
self.module = str(p.klass.__module__)
self.plugin = '%s.%s' % (self.module, self.name)
self.bases = tuple(b.__name__ for b in p.bases)
-lock(self)
+if not is_production_mode(self):
+lock(self)
plugins = {}
def plugin_iter(base, subclasses):
@@ -608,7 +613,8 @@ class API(DictProxy):
if klass not in plugins:
plugins[klass] = PluginInstance(klass)
p = plugins[klass]
-assert base not in p.bases
+if not is_production_mode(self):
+assert base not in p.bases
p.bases.append(base)
yield p.instance
--
1.7.4
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 180 Renamed associate.js to association.js.
On 06/15/2011 06:24 PM, Endi Sukma Dewata wrote: ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ACK. Pushed to master ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 181 Fixed self-service links.
On 06/16/2011 09:55 AM, Endi Sukma Dewata wrote: In self-service mode the user's association facets have been modified such that the entries are not linked since the only available entity is the user entity. A 'link' parameter has been added to IPA.association_facet and IPA.column to control whether to link the entries. The link_handler() method can be used to define how to handle the link. Ticket #1072 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ACK. Pushed to master ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 799 The IP address provided to ipa-server-install must be local
On Thu, 2011-06-16 at 09:07 -0400, Rob Crittenden wrote: I think this is still not right. When you let match_local default to False, --ip-address option in ipa-server-install is checked with match_local=False and thus the check required by BZ isn't made. Yes but it is checked again later. Try it, enforcement happens. Yes. Please check my patch 083 I sent this morning. It makes sure that IP address validation with CheckedIPAddress is run with correct parameters (i.e. match_local, parse_netmask). You may want to build your patch on top of this one. Should we be so strict and raise an exception when the IP address does not match any local interface? Maybe a warning would be enough. ipa-server-install will fail anyway few steps later in a scenario described in BZ. We should fail as soon as possible. By doing this before installation starts they don't have to uninstall. rob In fact, if we apply your patch on top of my patch 083 it works just fine and --ip-address is checked against network interfaces in option parsing phase. So ACK from me if it is applied on top of my patch 083 (not reviewed yet). Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 0238-test-for-dirty
From 76786348d9b0cf8fc5b0760a1ce4053fdf60c73e Mon Sep 17 00:00:00 2001
From: Adam Young ayo...@redhat.com
Date: Thu, 16 Jun 2011 12:44:47 -0400
Subject: [PATCH] test for dirty
instead of always setting dirty, we do the original test, and then set the flag and show the link.
https://fedorahosted.org/freeipa/ticket/1337
---
install/ui/widget.js | 48 +++-
1 files changed, 47 insertions(+), 1 deletions(-)
diff --git a/install/ui/widget.js b/install/ui/widget.js
index 3e31b4c4a5a62be189e2eb4dd0bb6f4581caca79..4dc2d5f81b68fb3637f3b7c0032b3f3b2c946e39 100644
--- a/install/ui/widget.js
+++ b/install/ui/widget.js
@@ -141,6 +141,52 @@ IPA.widget = function(spec) {
}
}
};
+/**
+ * This function compares the original values and the
+ * values entered in the UI. If the values have changed
+ * it will return true.
+ */
+that.test_dirty = function(){
+
+if (that.read_only) {
+return false;
+}
+
+var values = that.save();
+
+if (!values) { // ignore null values
+return false;
+}
+
+if (!that.values) {
+
+if (values instanceof Array) {
+
+if ((values.length === 0) ||
+(values.length === 1)
+(values[0] === '')) {
+return false;
+}
+}
+
+return true;
+}
+
+if (values.length != that.values.length) {
+return true;
+}
+
+values.sort();
+that.values.sort();
+
+for (var i=0; ivalues.length; i++) {
+if (values[i] != that.values[i]) {
+return true;
+}
+}
+
+return false;
+};
that.create = function(container) {
that.container = container;
@@ -324,7 +370,7 @@ IPA.text_widget = function(spec) {
var input = $('input[name='+that.name+']', that.container);
input.keyup(function() {
-that.set_dirty(true);
+that.set_dirty(that.test_dirty());
that.validate();
});
--
1.7.5.2
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0238-test-for-dirty
On 6/16/2011 11:46 AM, Adam Young wrote: ACK and pushed to master. -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 29 Raise DuplicateEntry Error when adding a duplicate sudo option
On Jun 16, 2011, at 8:01 AM, Rob Crittenden wrote: JR Aquino wrote: On Jun 15, 2011, at 8:03 AM, Rob Crittenden wrote: A minor issue and a question. The minor issue is you changed a couple of options from optional to mandatory, which is fine, but we need to bump up the minor version in VERSION (older clients otherwise could not send the string and blow things up). Is there a rule of thumb or document that details when this is appropriate? The question is, should we raise EmptyModList() when removing an option that doesn't exist or NotFound(reason=_())? I think the second might be more explanatory but might be harder for handle in scripts (how would you distinguish between entry not found and option not found)? rob As per IRC conversation: Added new Exception: AttrValueNotFound Incremented minor version in VERSION Adjusted API 1276 (Raise AttrValueNotFound when trying to remove a non-existent option from Sudo rule) 1277 (Raise DuplicateEntry Error when adding a duplicate sudo option) 1308 (Make sudooption a required option for sudorule_remove_option) This is very close, found a couple more issues: I don't think I was very clear in what to update in VERSION, you want it to look like this: diff --git a/VERSION b/VERSION index 6cbf732..e31f0d0 100644 --- a/VERSION +++ b/VERSION @@ -79,4 +79,4 @@ IPA_DATA_VERSION=2010061412 # # IPA_API_VERSION_MAJOR=2 -IPA_API_VERSION_MINOR=5 +IPA_API_VERSION_MINOR=6 Two tests are failing. One is failing because externalhost is returned as a tuple (rather than not at all). The second because sudorule_remove_option has changed the type of data being returned. rob Ok, the VERSION issue is resolved, and the ipasudoopt test issue is solved. I have created: https://fedorahosted.org/freeipa/ticket/1339 to address the externalhost tuple as it is separate from the sudo options effort. binL1EGmvO8nL.bin Description: freeipa-jraquino-0029-Raise-DuplicateEntry-Error-when-adding-a-duplicate.patch ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 0239-test-dirty-multivalue
From 4c22666ff142eaf86658b8df8f73797c371e2b5d Mon Sep 17 00:00:00 2001
From: Adam Young ayo...@redhat.com
Date: Thu, 16 Jun 2011 15:24:48 -0400
Subject: [PATCH] test dirty multivalue test the multivalue widgets for
changes before showing the undo link.
https://fedorahosted.org/freeipa/ticket/1337
---
install/ui/widget.js | 23 ++-
1 files changed, 22 insertions(+), 1 deletions(-)
diff --git a/install/ui/widget.js b/install/ui/widget.js
index 445b949db61cddae4fb1b1f5a424bcd7a24f27e1..14a40a577bf23fca32e5218b68aa141e6e7099cf 100644
--- a/install/ui/widget.js
+++ b/install/ui/widget.js
@@ -434,6 +434,27 @@ IPA.multivalued_text_widget = function(spec) {
}
};
+that.super_test_dirty = that.test_dirty;
+
+that.test_dirty = function(index){
+if (index === undefined) {
+return that.super_test_dirty();
+}
+var row = that.get_row(index);
+var return_value = false;
+
+$('input[name='+that.name+']', row).each(function() {
+var input = $(this);
+if (input.is('.strikethrough')) return_value = true;
+var value = input.val();
+
+if (value !== that.values[index]){
+return_value = true;
+}
+});
+return return_value;
+};
+
that.set_dirty = function(dirty, index) {
that.widget_set_dirty(dirty);
if (that.undo) {
@@ -589,7 +610,7 @@ IPA.multivalued_text_widget = function(spec) {
var index = that.row_index(row);
// uncross removed value
input.removeClass('strikethrough');
-that.set_dirty(true, index);
+that.set_dirty( that.test_dirty(index), index);
if (that.undo) {
if (index that.values.length) {
remove_link.css('display', 'inline');
--
1.7.5.2
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] 32 Don't add empty tuple to entry_attrs['externalhost']
https://fedorahosted.org/freeipa/ticket/1339 binniSici8OHk.bin Description: freeipa-jraquino-0032-Dont-add-empty-tuple-to-entry_attrs-externalhost.patch ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH]0240-test-dirty-onchange
From 472d1828957795b60df0d409ba29b1755fb921d5 Mon Sep 17 00:00:00 2001
From: Adam Young ayo...@redhat.com
Date: Thu, 16 Jun 2011 15:37:27 -0400
Subject: [PATCH] test dirty onchange
instead of blindly setting dirty, check if the filed has a different value than it originally did.
https://fedorahosted.org/freeipa/ticket/1337
---
install/ui/aci.js|6 +++---
install/ui/widget.js | 12 ++--
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/install/ui/aci.js b/install/ui/aci.js
index 70fe6d9258b6a495bec0a73ce6f22d7b6f5fdb56..077cbebdc00fa8da759ad9aeaea1ee6e236582a1 100644
--- a/install/ui/aci.js
+++ b/install/ui/aci.js
@@ -234,7 +234,7 @@ IPA.attributes_widget = function(spec) {
click: function(){
$('.aci-attribute', that.table).
attr('checked', $(this).attr('checked'));
-that.set_dirty(true);
+that.set_dirty(that.test_dirty());
}
})
})).append($('th/', {
@@ -298,7 +298,7 @@ IPA.attributes_widget = function(spec) {
value: value,
'class': 'aci-attribute',
click: function() {
-that.set_dirty(true);
+that.set_dirty(that.test_dirty());
}
}));
td = $('td/').appendTo(aci_tr);
@@ -335,7 +335,7 @@ IPA.attributes_widget = function(spec) {
value: value,
'class': 'aci-attribute',
change: function() {
-that.set_dirty(true);
+that.set_dirty(that.test_dirty());
}
}));
diff --git a/install/ui/widget.js b/install/ui/widget.js
index 14a40a577bf23fca32e5218b68aa141e6e7099cf..80906a2f5981fe69d2d32c605dc7fe3c40d87c80 100644
--- a/install/ui/widget.js
+++ b/install/ui/widget.js
@@ -728,7 +728,7 @@ IPA.checkbox_widget = function (spec) {
var input = $('input[name='+that.name+']', that.container);
input.change(function() {
-that.set_dirty(true);
+that.set_dirty(that.test_dirty());
});
var undo = that.get_undo();
@@ -802,7 +802,7 @@ IPA.checkboxes_widget = function (spec) {
var input = $('input[name='+that.name+']', that.container);
input.change(function() {
-that.set_dirty(true);
+that.set_dirty(that.test_dirty());
});
var undo = that.get_undo();
@@ -880,7 +880,7 @@ IPA.radio_widget = function(spec) {
var input = $('input[name='+that.name+']', that.container);
input.change(function() {
-that.set_dirty(true);
+that.set_dirty(that.test_dirty());
});
var undo = that.get_undo();
@@ -957,7 +957,7 @@ IPA.select_widget = function(spec) {
that.select = $('select[name='+that.name+']', that.container);
that.select.change(function() {
-that.set_dirty(true);
+that.set_dirty(that.test_dirty());
});
var undo = that.get_undo();
@@ -1575,7 +1575,7 @@ IPA.entity_select_widget = function(spec) {
that.entity_select = $('select/', {
id: that.name + '-entity-select',
change: function(){
-that.set_dirty(true);
+that.set_dirty(that.test_dirty());
}
}).appendTo(container);
@@ -1586,7 +1586,7 @@ IPA.entity_select_widget = function(spec) {
style: 'display: none;',
keyup: function(){
populate_select();
-that.set_dirty(true);
+that.set_dirty(that.test_dirty());
}
}).appendTo(container);
--
1.7.5.2
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0239-test-dirty-multivalue
On 6/16/2011 2:26 PM, Adam Young wrote: ACK and pushed to master. -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH]0240-test-dirty-onchange
On 6/16/2011 2:39 PM, Adam Young wrote: ACK and pushed to master. -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 182 Merged direct and indirect association facets
The direct and indirect associations are now displayed in the same
facet. The type of association to be displayed can be selected
using radio buttons.
Ticket #1338
--
Endi S. Dewata
From 8e667ca1c43fc2b766027d94b48bfc7b3b00a095 Mon Sep 17 00:00:00 2001
From: Endi S. Dewata edew...@redhat.com
Date: Thu, 16 Jun 2011 16:35:13 -0500
Subject: [PATCH] Merged direct and indirect association facets
The direct and indirect associations are now displayed in the same
facet. The type of association to be displayed can be selected
using radio buttons.
Ticket #1338
---
install/ui/association.js | 62 ++--
install/ui/details.js |4 +-
install/ui/entity.js | 36 ++---
install/ui/ipa.css|2 +-
4 files changed, 93 insertions(+), 11 deletions(-)
diff --git a/install/ui/association.js b/install/ui/association.js
index 8030631b4fee45dfec6bc77b26f0114259665fd4..3c847c59c7b1050b0f4fa59dad4b89a19ac64650 100644
--- a/install/ui/association.js
+++ b/install/ui/association.js
@@ -674,7 +674,11 @@ IPA.association_facet = function (spec) {
var that = IPA.facet(spec);
that.attribute_member = spec.attribute_member;
+that.indirect_attribute_member = spec.indirect_attribute_member;
+
that.other_entity = spec.other_entity;
+
+that.association_type = 'direct';
that.facet_group = spec.facet_group;
that.read_only = spec.read_only;
@@ -821,6 +825,48 @@ IPA.association_facet = function (spec) {
}
}).appendTo(that.controls);
}
+
+if (that.indirect_attribute_member) {
+var span = $('span/', {
+'class': 'right-aligned-controls'
+}).appendTo(that.controls);
+
+span.append('Show Results ');
+
+that.direct_radio = $('input/', {
+type: 'radio',
+name: 'type',
+value: 'direct',
+click: function() {
+that.association_type = $(this).val();
+that.refresh();
+return true;
+}
+}).appendTo(span);
+
+span.append(' Direct Enrollment ');
+
+that.indirect_radio = $('input/', {
+type: 'radio',
+name: 'type',
+value: 'indirect',
+click: function() {
+that.association_type = $(this).val();
+that.refresh();
+return true;
+}
+}).appendTo(span);
+
+span.append(' Indirect Enrollment');
+}
+};
+
+that.get_attribute_name = function() {
+if (that.association_type == 'direct') {
+return that.attribute_member+'_'+that.other_entity;
+} else {
+return that.indirect_attribute_member+'_'+that.other_entity;
+}
};
that.create_content = function(container) {
@@ -948,7 +994,7 @@ IPA.association_facet = function (spec) {
that.table.current_page_input.val(that.table.current_page);
that.table.total_pages_span.text(that.table.total_pages);
-var pkeys = that.record[that.name];
+var pkeys = that.record[that.get_attribute_name()];
if (!pkeys || !pkeys.length) {
that.table.empty();
that.table.summary.text('No entries.');
@@ -1003,7 +1049,7 @@ IPA.association_facet = function (spec) {
if (!length) return;
var batch = IPA.batch_command({
-'name': that.entity_name+'_'+that.name,
+'name': that.entity_name+'_'+that.get_attribute_name(),
'on_success': on_success,
'on_error': on_error
});
@@ -1026,12 +1072,22 @@ IPA.association_facet = function (spec) {
that.refresh = function() {
+if (that.association_type == 'direct') {
+if (that.direct_radio) that.direct_radio.attr('checked', true);
+if (that.add_button) that.add_button.css('display', 'inline');
+if (that.remove_button) that.remove_button.css('display', 'inline');
+} else {
+if (that.indirect_radio) that.indirect_radio.attr('checked', true);
+if (that.add_button) that.add_button.css('display', 'none');
+if (that.remove_button) that.remove_button.css('display', 'none');
+}
+
function on_success(data, text_status, xhr) {
that.record = data.result.result;
that.table.current_page = 1;
-var pkeys = that.record[that.name];
+var pkeys = that.record[that.get_attribute_name()];
if (pkeys) {
that.table.total_pages =
Math.ceil(pkeys.length / that.table.page_length);
diff --git a/install/ui/details.js b/install/ui/details.js
index f5a3e4d80fec40da6e8a55704461a12d46647a0e..d4a013ad9c18404271d38d137dd8a78ca8c2de75 100644
---
Re: [Freeipa-devel] [PATCH] 182 Merged direct and indirect association facets
On 06/16/2011 07:25 PM, Endi Sukma Dewata wrote: The direct and indirect associations are now displayed in the same facet. The type of association to be displayed can be selected using radio buttons. Ticket #1338 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ACK. Pushed to master ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
