[Freeipa-devel] [PATCH] 42 Search for users in all the naming contexts present on the directory server

2011-08-23 Thread Jan Cholasta
This patch fixes ipa-kpasswd in cases where we have more than one naming 
context in the directory server.


https://fedorahosted.org/freeipa/ticket/1655
https://fedorahosted.org/freeipa/ticket/1656

Honza

--
Jan Cholasta
From aeb3b9c214a6ec82a5a9e5bbb0651b9cc3d9effb Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Tue, 23 Aug 2011 10:53:22 +0200
Subject: [PATCH] Search for users in all the naming contexts present on the
 directory server.

ticket 1655, 1656
---
 daemons/ipa-kpasswd/ipa_kpasswd.c |   39 +++-
 1 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/daemons/ipa-kpasswd/ipa_kpasswd.c b/daemons/ipa-kpasswd/ipa_kpasswd.c
index acec3db..f973e42 100644
--- a/daemons/ipa-kpasswd/ipa_kpasswd.c
+++ b/daemons/ipa-kpasswd/ipa_kpasswd.c
@@ -322,7 +322,6 @@ int ldap_pwd_change(char *client_name, char *realm_name, krb5_data pwd, char **e
 	char hostname[1024];
 	char *uri;
 	struct berval **ncvals;
-	char *ldap_base = NULL;
 	char *filter;
 	char *attrs[] = {krbprincipalname, NULL};
 	char *root_attrs[] = {namingContexts, NULL};
@@ -340,6 +339,7 @@ int ldap_pwd_change(char *client_name, char *realm_name, krb5_data pwd, char **e
 	int ret, rc;
 	int fd;
 	int kpwd_err = KRB5_KPASSWD_HARDERROR;
+	int i;
 
 	tmp_file = strdup(TMP_TEMPLATE);
 	if (!tmp_file) {
@@ -410,7 +410,6 @@ int ldap_pwd_change(char *client_name, char *realm_name, krb5_data pwd, char **e
 	}
 
 	/* find base dn */
-	/* TODO: address the case where we have multiple naming contexts */
 	tv.tv_sec = 10;
 	tv.tv_usec = 0;
 
@@ -433,10 +432,8 @@ int ldap_pwd_change(char *client_name, char *realm_name, krb5_data pwd, char **e
 		goto done;
 	}
 
-	ldap_base = strdup(ncvals[0]-bv_val);
-
-	ldap_value_free_len(ncvals);
 	ldap_msgfree(res);
+	res = NULL;
 
 	/* find user dn */
 	ret = asprintf(filter, krbPrincipalName=%s, client_name);
@@ -448,8 +445,26 @@ int ldap_pwd_change(char *client_name, char *realm_name, krb5_data pwd, char **e
 	tv.tv_sec = 10;
 	tv.tv_usec = 0; 
 
-	ret = ldap_search_ext_s(ld, ldap_base, LDAP_SCOPE_SUBTREE,
-filter, attrs, 1, NULL, NULL, tv, 0, res);
+	for (i = 0; !userdn  ncvals[i]; i++) {
+		ret = ldap_search_ext_s(ld, ncvals[i]-bv_val,
+	LDAP_SCOPE_SUBTREE, filter, attrs, 1,
+	NULL, NULL, tv, 0, res);
+
+		if (ret != LDAP_SUCCESS) {
+			break;
+		}
+
+		/* for now just use the first result we get */
+		entry = ldap_first_entry(ld, res);
+		if (entry) {
+			userdn = ldap_get_dn(ld, entry);
+		}
+
+		ldap_msgfree(res);
+		res = NULL;
+	}
+
+	ldap_value_free_len(ncvals);
 
 	if (ret != LDAP_SUCCESS) {
 		syslog(LOG_ERR, Search for %s failed with error %d,
@@ -460,14 +475,9 @@ int ldap_pwd_change(char *client_name, char *realm_name, krb5_data pwd, char **e
 		}
 		goto done;
 	}
-	free(filter);
-
-	/* for now just use the first result we get */
-	entry = ldap_first_entry(ld, res);
-	userdn = ldap_get_dn(ld, entry);
 
-	ldap_msgfree(res);
-	res = NULL;
+	free(filter);
+	filter = NULL;
 
 	if (!userdn) {
 		syslog(LOG_ERR, No userdn, can't change password!);
@@ -651,6 +661,7 @@ done:
 	if (control) ber_bvfree(control);
 	free(exterr1);
 	free(exterr2);
+	free(filter);
 	free(userdn);
 	if (ld) ldap_unbind_ext(ld, NULL, NULL);
 	if (tmp_file) {
-- 
1.7.6

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

Re: [Freeipa-devel] [PATCH] 005 Show error in serial association

2011-08-23 Thread Petr Vobornik

On 08/22/2011 07:05 PM, Endi Sukma Dewata wrote:

On 8/22/2011 10:06 AM, Petr Vobornik wrote:

'Failed' moved to command. On 'failed' success is transformed to error -
can be change behaviour of serial associator in some commands
(previously some commands were executed even after 'failed' of
previous). It isn't probably big issue because they fail probably from
the same reason (consequent commands would fail to).


This will be addressed in ticket #1688.


- 'failed' message is extended by related object (so user would know for
which command in the batch it is related to).


Just to be consistent, I think we should format the message like this:
primary key: error message.


OK

- there is still the problem ('no modifications to be performed') I
wrote about on Aug 18. I think it should be fixed before commiting this
path.


This will be addressed in ticket #1692.

One more issue, a command could return multiple error messages in each
failure, but right now the get_failed() only reads the first message in
each failure. Try adding several users into a group, but remove some of
them just before adding it, only the first missing user is reported. I
think the code in ipa.js:395-401 should iterate through all messages.


Reworked.


I'm thinking that we should show only notification dialog (like in batch 
error for 'failed' commands. The reason is that part of the command can 
be successfully executed, so offering retry is wrong because it may 
cause other error (try it in the example above). Second reason is that 
if we want to show all errors we have to concatenate error messages with 
some separator (quite easy, current implementation) or to pass array of 
error messages  to error dialog  like in batch error (needs to add 
suppor for it in command).


I'm thinking about dialog with this content:

dialog-titleOperation Error/dialog-title

p
Some parts of operation failed. Completed: $completed.
/p
show-hide-link /
ul
li $key: $message /li
li $key2: $message2 /li
/ul
 ok-button

The 'Completed' part would be shown only if present.

Other problem in error dialog is that there are used untranslated 
strings. We should modify it to use translated and as fallback (like in 
init method) untranslated.


--
Petr Vobornik
From 7baa345b59ef61e6539f745bfea5a701cfcff549 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Wed, 17 Aug 2011 15:06:41 +0200
Subject: [PATCH] Show error in adding associations

https://fedorahosted.org/freeipa/ticket/1628
---
 install/ui/ipa.js|  112 +++--
 install/ui/test/association_tests.js |4 +-
 2 files changed, 93 insertions(+), 23 deletions(-)

diff --git a/install/ui/ipa.js b/install/ui/ipa.js
index 9a252f1e50fdaf544cb872fe0dbed9377e791559..c6c8ef2fef9d99642e3695d08811d27587f9fa55 100644
--- a/install/ui/ipa.js
+++ b/install/ui/ipa.js
@@ -331,6 +331,7 @@ IPA.command = function(spec) {
 }
 
 function success_handler(data, text_status, xhr) {
+var failed;
 
 if (!data) {
 // error_handler() calls IPA.hide_activity_icon()
@@ -347,7 +348,18 @@ IPA.command = function(spec) {
 message: data.error.message,
 data: data
 });
+} else if ((failed = that.get_failed(that, data.result, text_status, xhr)) 
+!failed.is_empty()) {
+var message = failed.errors[0].message;
+for(var i = 1; i  failed.errors.length; i++) {
+message += '\n' + failed.errors[i].message;
+}
 
+error_handler.call(this, xhr, text_status,  /* error_thrown */ {
+name: failed.errors[0].name,
+message: message,
+data: data
+});
 } else if (that.on_success) {
 IPA.hide_activity_icon();
 //custom success handling, maintaining AJAX call's context
@@ -379,6 +391,27 @@ IPA.command = function(spec) {
 $.ajax(request);
 };
 
+that.get_failed = function(command, result, text_status, xhr) {
+var errors = IPA.error_list();
+if(result  result.failed) {
+for(var association in result.failed) {
+for(var member_name in result.failed[association]) {
+var member = result.failed[association][member_name];
+for(var i = 0; i  member.length; i++) {
+if(member[i].length  1) {
+var name = 'IPA Error';
+var message = member[i][1];
+if(member[i][0])
+message = member[i][0] + ': ' + message;
+errors.add(command, name, message, text_status);
+}
+}
+}
+}
+}
+return 

Re: [Freeipa-devel] [PATCH] 41 Verify that the external CA certificate files are correct

2011-08-23 Thread Jan Cholasta

On 18.8.2011 17:47, Rob Crittenden wrote:

Jan Cholasta wrote:

On 17.8.2011 10:27, Jan Cholasta wrote:

Verify that --external_cert_file and --external_ca_file are both
readable, valid PEM files and that their subject/issuer is correct.

Also fixes ipalib.x509.load_certificate_from_file.

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

Honza



Patch attached.


nack, but this is very close.

If the CA is a chain the signing check may fail if the first cert isn't
the one that signed the CSR. You need to check all CA certs in the file.

rob


Fixed.

Honza

--
Jan Cholasta
From fe6cb906e43b51bbfb23d85f5502eb71f1ad8f3a Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Wed, 17 Aug 2011 10:19:37 +0200
Subject: [PATCH] Verify that the external CA certificate files are correct.

ticket 1572
---
 install/tools/ipa-server-install |   43 -
 ipalib/x509.py   |   20 -
 2 files changed, 56 insertions(+), 7 deletions(-)

diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install
index 189bb20..05caad8 100755
--- a/install/tools/ipa-server-install
+++ b/install/tools/ipa-server-install
@@ -39,6 +39,7 @@ import traceback
 from ConfigParser import RawConfigParser
 import random
 import tempfile
+import nss.error
 
 from ipaserver.install import dsinstance
 from ipaserver.install import krbinstance
@@ -59,6 +60,7 @@ from ipalib import api, errors, util
 from ipalib.parameters import IA5Str
 from ipapython.config import IPAOptionParser
 from ipalib.dn import DN
+from ipalib.x509 import load_certificate_from_file, load_certificate_chain_from_file
 
 pw_name = None
 uninstalling = False
@@ -567,18 +569,47 @@ def main():
 # already having done the first stage of the CA install.
 print CA is not installed yet. To install with an external CA is a two-stage process.\nFirst run the installer with --external-ca.
 sys.exit(1)
-if not ipautil.file_exists(options.external_cert_file):
-print %s does not exist % options.external_cert_file
-sys.exit(1)
-if not ipautil.file_exists(options.external_ca_file):
-print %s does not exist % options.external_ca_file
-sys.exit(1)
 
 # This will override any settings passed in on the cmdline
 if ipautil.file_exists(ANSWER_CACHE):
 dm_password = read_password(Directory Manager, confirm=False)
 options._update_loose(read_cache(dm_password))
 
+if options.external_cert_file:
+try:
+extcert = load_certificate_from_file(options.external_cert_file)
+except IOError, e:
+print Can't load the PKCS#10 certificate: %s. % str(e)
+sys.exit(1)
+except nss.error.NSPRError:
+print '%s' is not a valid PEM-encoded certificate. % options.external_cert_file
+sys.exit(1)
+
+if DN(unicode(extcert.subject)) != DN(('CN','Certificate Authority'), options.subject):
+print Subject of the PKCS#10 certificate is not correct.
+sys.exit(1)
+
+try:
+extchain = load_certificate_chain_from_file(options.external_ca_file)
+except IOError, e:
+print Can't load the external CA chain: %s. % str(e)
+sys.exit(1)
+except nss.error.NSPRError:
+print '%s' is not a valid PEM-encoded certificate chain. % options.external_ca_file
+sys.exit(1)
+
+certdict = dict((unicode(cert.subject), cert) for cert in extchain)
+if unicode(extcert.issuer) not in certdict:
+print The PKCS#10 certificate is not signed by the external CA.
+sys.exit(1)
+
+cert = extcert
+while unicode(cert.issuer) in certdict and cert.issuer != cert.subject:
+cert = certdict[unicode(cert.issuer)]
+if cert.issuer != cert.subject:
+print The external CA chain is incomplete.
+sys.exit(1)
+
 print ==
 print This program will set up the FreeIPA Server.
 print 
diff --git a/ipalib/x509.py b/ipalib/x509.py
index 23f337e..04e1b94 100644
--- a/ipalib/x509.py
+++ b/ipalib/x509.py
@@ -34,6 +34,7 @@
 import os
 import sys
 import base64
+import re
 import nss.nss as nss
 from nss.error import NSPRError
 from ipapython import ipautil
@@ -45,6 +46,8 @@ from ipalib import errors
 PEM = 0
 DER = 1
 
+PEM_REGEX = re.compile(r'(?=-BEGIN CERTIFICATE-).*?(?=-END CERTIFICATE-)', re.DOTALL)
+
 def valid_issuer(issuer, realm):
 return issuer in ('CN=%s Certificate Authority' % realm,
   'CN=Certificate Authority,O=%s' % realm,)
@@ -89,6 +92,21 @@ def load_certificate(data, datatype=PEM, dbdir=None):
 
 return nss.Certificate(buffer(data))
 
+def load_certificate_chain_from_file(filename, dbdir=None):
+
+Load a certificate chain from a 

[Freeipa-devel] [PATCH] 43 Add subscription-manager dependency for RHEL

2011-08-23 Thread Jan Cholasta

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

Honza

--
Jan Cholasta
From ff23fde28907611ed68cc2a13a89990d5be80cce Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Tue, 23 Aug 2011 14:15:27 +0200
Subject: [PATCH] Add subscription-manager dependency for RHEL.

ticket 1664
---
 freeipa.spec.in |6 ++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 87fd0b6..8a624e0 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -224,6 +224,9 @@ Requires: python-netaddr = 0.7.5-3
 Requires: python-netaddr
 %endif
 Requires: libipa_hbac-python
+%if 0%{?rhel}
+Requires: subscription-manager
+%endif
 
 Obsoletes: ipa-python = 1.0
 
@@ -535,6 +538,9 @@ fi
 %ghost %attr(0644,root,apache) %config(noreplace) %{_sysconfdir}/ipa/default.conf
 
 %changelog
+* Tue Aug 23 2011 Jan Cholasta jchol...@redhat.com - 2.1.0-1
+- Add subscription-manager dependency for RHEL.
+
 * Thu Aug 11 2011 Martin Kosek mko...@redhat.com - 2.0.90-12
 - Set min nvr of 389-ds-base to 1.2.9.6 for fix in BZ 725743,
   723937, and 725542
-- 
1.7.6

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

Re: [Freeipa-devel] [PATCH] 41 Verify that the external CA certificate files are correct

2011-08-23 Thread Rob Crittenden

Jan Cholasta wrote:

On 18.8.2011 17:47, Rob Crittenden wrote:

Jan Cholasta wrote:

On 17.8.2011 10:27, Jan Cholasta wrote:

Verify that --external_cert_file and --external_ca_file are both
readable, valid PEM files and that their subject/issuer is correct.

Also fixes ipalib.x509.load_certificate_from_file.

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

Honza



Patch attached.


nack, but this is very close.

If the CA is a chain the signing check may fail if the first cert isn't
the one that signed the CSR. You need to check all CA certs in the file.

rob


Fixed.

Honza



Nice, I really like the way you import the cert chain.

One more small request. When a failure occurs can you print more detail 
on why? For example, we mandate that the subject of the CA cert be 
CN=Certificate Authority,subject_base. Can you include what we expect 
if this fails? Similarly when reviewing the cert chain display can you 
show what CA is missing?


rob

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


Re: [Freeipa-devel] [PATCH] 251 Updated add and delete association dialog titles.

2011-08-23 Thread Petr Vobornik

On 08/22/2011 08:50 PM, Endi Sukma Dewata wrote:

The association table widget and facet have been modified to accept
titles for the add and delete dialogs. The table and facet definitions
have been modified to specify the appropriate titles.

Some unused code have been removed.

Ticket #1629



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

ACK

--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 42 Search for users in all the naming contexts present on the directory server

2011-08-23 Thread Rob Crittenden

Jan Cholasta wrote:

This patch fixes ipa-kpasswd in cases where we have more than one naming
context in the directory server.

https://fedorahosted.org/freeipa/ticket/1655
https://fedorahosted.org/freeipa/ticket/1656

Honza


Ack, pushed to master and ipa-2-1. I tested by enabling the retro 
changelog plugin which adds another namingcontext.


rob

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


Re: [Freeipa-devel] [PATCH] 43 Add subscription-manager dependency for RHEL

2011-08-23 Thread Rob Crittenden

Jan Cholasta wrote:

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

Honza


nack, this requires only needs to be on freeipa-server.

rob

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


Re: [Freeipa-devel] [PATCH] 251 Updated add and delete association dialog titles.

2011-08-23 Thread Endi Sukma Dewata

On 8/23/2011 8:56 AM, Petr Vobornik wrote:

On 08/22/2011 08:50 PM, Endi Sukma Dewata wrote:

The association table widget and facet have been modified to accept
titles for the add and delete dialogs. The table and facet definitions
have been modified to specify the appropriate titles.

Some unused code have been removed.

Ticket #1629

ACK


Pushed to master and ipa-2-1.

--
Endi S. Dewata

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


Re: [Freeipa-devel] [PATCH] 252 Removed unnecessary HBAC/sudo rule category modification.

2011-08-23 Thread Petr Vobornik

On 08/22/2011 11:37 PM, Endi Sukma Dewata wrote:

Since the Add/Delete links in the association table are disabled when
the category is set to 'all', it's no longer necessary to check the
category before showing the add/delete dialogs and modify the category
before adding entries. Thus, the IPA.rule_association_table_widget is
no longer needed.

Ticket #1692



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

ACK

--
Petr Vobornik

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


[Freeipa-devel] [PATCH] 39 Improve sudorule documentation

2011-08-23 Thread JR Aquino
https://fedorahosted.org/freeipa/ticket/1657
Added brief explanations for the various Sudo components in the top level doc. 
Added doc entries for RunAs User and RunAs Group.



freeipa-jraquino-0039-Improve-sudorule-documentation.patch
Description: freeipa-jraquino-0039-Improve-sudorule-documentation.patch
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 39 Improve sudorule documentation

2011-08-23 Thread Rob Crittenden

JR Aquino wrote:

https://fedorahosted.org/freeipa/ticket/1657
Added brief explanations for the various Sudo components in the top level doc.
Added doc entries for RunAs User and RunAs Group.


Deon suggested a minor grammar correction and I modified the commit 
message a little.


ack, pushed to master and ipa-2-1

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


Re: [Freeipa-devel] [PATCH] 43 Add subscription-manager dependency for RHEL

2011-08-23 Thread Rob Crittenden

Jan Cholasta wrote:

On 23.8.2011 16:22, Rob Crittenden wrote:

Jan Cholasta wrote:

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

Honza


nack, this requires only needs to be on freeipa-server.

rob


OK, I thought I'd put it in the package where entitle.py is located.

Updated patch attached.

Honza



ack, pushed to master and ipa-2-1

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


Re: [Freeipa-devel] [PATCH] 41 Verify that the external CA certificate files are correct

2011-08-23 Thread Rob Crittenden

Jan Cholasta wrote:

On 23.8.2011 15:36, Rob Crittenden wrote:

Jan Cholasta wrote:

On 18.8.2011 17:47, Rob Crittenden wrote:

Jan Cholasta wrote:

On 17.8.2011 10:27, Jan Cholasta wrote:

Verify that --external_cert_file and --external_ca_file are both
readable, valid PEM files and that their subject/issuer is correct.

Also fixes ipalib.x509.load_certificate_from_file.

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

Honza



Patch attached.


nack, but this is very close.

If the CA is a chain the signing check may fail if the first cert isn't
the one that signed the CSR. You need to check all CA certs in the
file.

rob


Fixed.

Honza



Nice, I really like the way you import the cert chain.

One more small request. When a failure occurs can you print more detail
on why? For example, we mandate that the subject of the CA cert be
CN=Certificate Authority,subject_base. Can you include what we expect
if this fails? Similarly when reviewing the cert chain display can you
show what CA is missing?

rob


Updated patch attached.

Honza



ack, pushed to master and ipa-2-1

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


Re: [Freeipa-devel] [PATCH] 005 Show error in serial association

2011-08-23 Thread Endi Sukma Dewata

On 8/23/2011 6:34 AM, Petr Vobornik wrote:

On 08/22/2011 07:05 PM, Endi Sukma Dewata wrote:

One more issue, a command could return multiple error messages in each
failure, but right now the get_failed() only reads the first message in
each failure. Try adding several users into a group, but remove some of
them just before adding it, only the first missing user is reported. I
think the code in ipa.js:395-401 should iterate through all messages.


Reworked.

I'm thinking that we should show only notification dialog (like in batch
error for 'failed' commands. The reason is that part of the command can
be successfully executed, so offering retry is wrong because it may
cause other error (try it in the example above). Second reason is that
if we want to show all errors we have to concatenate error messages with
some separator (quite easy, current implementation) or to pass array of
error messages to error dialog like in batch error (needs to add suppor
for it in command).


Please take a look at the attached patch. This should be applied on top 
of your patch. It does several things:


1. As mentioned over IRC, we should be treating these partial failure as 
a success (the command does return a success). This way it's not going 
to show the Retry button.


2. Instead of concatenating the messages, they are now added into the 
error list. This way they will appear nicely as a list.


3. If the error dialog appears, it will wait until you click OK before 
continuing.


4. Since some of the membership operations are done using serial 
associator you might get multiple dialogs, but this should be gone once 
we fix #1688.


Please feel free to merge this patch into yours if you want to make 
further modifications. Or we can push both patches if you think it's 
good enough.


I can think of some more improvements, but it can be done separately.


I'm thinking about dialog with this content:

dialog-titleOperation Error/dialog-title

p
Some parts of operation failed. Completed: $completed.
/p
show-hide-link /
ul
li $key: $message /li
li $key2: $message2 /li
/ul
ok-button

The 'Completed' part would be shown only if present.


I'm not sure if we need to show the completed operations because we 
should be able to infer that from the command we're trying to execute 
and the error message we're getting. No error means it's completed.


Maybe we should try to provide a better error message, e.g. Some 
failures occurred when removing users from group editors. Also, we 
might want to change the 'Operations Error' title because it's actually 
a success. How about 'Operation Completed'? This can be done separately.


If you think showing the completed operations would be useful please 
file a ticket and we'll discuss it. We might be able to show the 
completed operations under 'Show details'.



Other problem in error dialog is that there are used untranslated
strings. We should modify it to use translated and as fallback (like in
init method) untranslated.


Let's put the locations of any untranslated messages we find into this 
ticket: https://fedorahosted.org/freeipa/ticket/1701


--
Endi S. Dewata
From fd441b13c995e81301cccd08d410980015853632 Mon Sep 17 00:00:00 2001
From: Endi S. Dewata edew...@redhat.com
Date: Tue, 23 Aug 2011 11:27:41 -0500
Subject: [PATCH] Fixed command partial failure handling.

When a command returns a partial failure it should be treated as a
success but the failures should still be displayed.

Ticket #1628
---
 install/ui/ipa.js |   86 
 1 files changed, 46 insertions(+), 40 deletions(-)

diff --git a/install/ui/ipa.js b/install/ui/ipa.js
index c6c8ef2fef9d99642e3695d08811d27587f9fa55..decf93f34bf012b25fc9cef259f77f39b84f0fc8 100644
--- a/install/ui/ipa.js
+++ b/install/ui/ipa.js
@@ -242,6 +242,9 @@ IPA.command = function(spec) {
 
 that.retry = typeof spec.retry == 'undefined' ? true : spec.retry;
 
+that.error_message = spec.error_message || (IPA.messages.dialogs ?
+IPA.messages.dialogs.batch_error_message : 'Some operations failed.');
+
 that.get_command = function() {
 return (that.entity ? that.entity+'_' : '') + that.method;
 };
@@ -331,7 +334,6 @@ IPA.command = function(spec) {
 }
 
 function success_handler(data, text_status, xhr) {
-var failed;
 
 if (!data) {
 // error_handler() calls IPA.hide_activity_icon()
@@ -348,22 +350,37 @@ IPA.command = function(spec) {
 message: data.error.message,
 data: data
 });
-} else if ((failed = that.get_failed(that, data.result, text_status, xhr)) 
-!failed.is_empty()) {
-var message = failed.errors[0].message;
-for(var i = 1; i  failed.errors.length; i++) {
-message += '\n' + failed.errors[i].message;
-}
 
-error_handler.call(this, xhr, 

Re: [Freeipa-devel] [PATCH] 34 Create FreeIPA CLI Plugin for the 389 Auto Membership plugin

2011-08-23 Thread Rob Crittenden

JR Aquino wrote:


On Aug 19, 2011, at 2:16 AM, Martin Kosek wrote:


Hi JR,

I get to your plugin again. You can see my findings below.

On Tue, 2011-08-09 at 22:41 +, JR Aquino wrote:
...

Ok New Patch attached.

I believe this addresses the above.

1. Requires(pre): 389-ds-base= 1.2.9.5-1


1) Please, remove the change to FreeIPA spec, its no longer needed since
we shipped version 2.1 and it already requires sufficient 389-ds-base
version.


Done.





2. replica-automember.ldif added for dsinstance to install during replica 
installs:
+dn: cn=Auto Membership Plugin,cn=plugins,cn=config
+changetype: modify
+add: nsslapd-pluginConfigArea
+nsslapd-pluginConfigArea: cn=automember,cn=etc,$SUFFIX


2) OK. I would do it a bit different - have one LDIF for
nsslapd-pluginConfigArea setting and second for creating the base
automember structure. Master would then use both LDIFs and a replica
both of them. We would then be without duplicates in LDIF. But your way
acceptable.


Please allow the 2 ldif's in as they are.

I tried to split them to leverage cn=config change in common, however, I 
encountered a 389 ds bug.
I will be opening a bug with Nathan in BZ to address the bug.  If you feel 
strongly, we can either:

A: Accept the two LDIFs as is and revisit after a newer version of 389 ds is 
available.
B: Wait until 389 ds addresses the bug and make the minor modification you 
suggested above.





3. autoMemberScope is now set for each:
groups: cn=users,cn=accounts,$SUFFIX
hostgroups: cn=computers,cn=accounts,$SUFFIX


OK



4. Corrected examples
Set the default target group:
ipa automember-default-group-set --default-group=webservers hostgroup
ipa automember-default-group-set --default-group=ipausers group

Set the default target group:
ipa automember-default-group-remove hostgroup
ipa automember-default-group-remove group

Show the default target group:
ipa automember-default-group-show hostgroup
ipa automember-default-group-show group

5. Corrected examples
Add a condition to the rule:
   ipa automember-add-condition --key=fqdn --type=hostgroup 
--inclusive-regex=^web[1-9+]\.example\.com webservers


3) Please fix the regex to ^web[1-9]+\.example\.com. I think its just a
mistake - right now for example a host web11.example.com does not match.


Fixed




   ipa automember-add-condition --key=manager --type=group 
--inclusive-regex=^mscott admins



4) I think you wanted to use devel rule instead of non-existent admins
automember rule.



You are correct, this has been fixed.


Add an exclusive condition to the rule to prevent auto asignment:
   ipa automember-add-condition --key=fqdn --type=hostgroup 
--exclusive-regex=^web5\.example\.com webservers

Remove a condition from the rule:
   ipa automember-remove-condition --key=fqdn --type=hostgroup 
--inclusive-regex=^www[1-9+]\.example\.com webservers


5) The same as in 3)


Fixed





6. Correct bug for adding duplicate conditions. Included test for it in the 
test suite.



OK. Here are my additional findings:

6) There some more example commands in doc which are not complete and
require some user typing:

Display a automember rule:
ipa automember-show webservers

Delete an automember rule:
ipa automember-del webservers

Grouping type option is missing


Fixed.  Added the appropriate flags in the examples



7) I get internal error when running examples from the automember doc:
# ipa automember-add --type=group devel
-
Added automember rule devel
-
  Automember Rule: devel
# ipa automember-add-condition --key=manager --type=group 
--inclusive-regex=^mscott admins
ipa: ERROR: an internal error has occurred


Fixed.




That's all. The plugin gets better with every version, I think we may
soon be ready for pushing - when all of the issues are resolved.

Martin



Please let me know how it looks now.



Looks lots better, just a couple of nits:

* The default-group api has type as an arg and everywhere else it is 
--type, can we make it consistent? We can argue about this with Martin 
tomorrow if you'd like.


* The tests focus mainly on bucket allocation, it also needs to test 
adding/removing conditions and rules. I wonder if there should actually 
be two test suites, one to test the basics of the plugin and one to make 
sure it operates properly when creating entries.


* Can you document in the ldifs and the installer why there are separate 
ones for master and replicas (for dsinstance.py I think you can just say 
# see ldifs for details).


rob

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


Re: [Freeipa-devel] [PATCH] 0283-enable-proxy-for-dogtag

2011-08-23 Thread Adam Young
NACK.  Replicate uses the install code, which grabs the local 
constants.  Need to extend it to use the local constants for a base 
install, but the remote constants for the replica installs.



On 08/19/2011 01:57 PM, Dmitri Pal wrote:

On 08/19/2011 01:19 PM, Adam Young wrote:
The complete solution for this patch requires changes in Dogtag that 
Ade Lee is working on right now.  In order to test, I have provided a 
couple of files that I have been using:



1.  Apply patch, build and install IPA rpms, run ipaserver-install as 
per usual.

2.  Move the dogtag.conf file into /etc/httpd/conf.d directorys
3.  Run the proxy_dogtag.py script   to modify the Dogtag instance to 
accept AJP connections from httpd so httpd can act as a proxy

4. Restart IPA


To test:

1. add a host.
2.  Generate a csr: 
http://freeipa.org/page/Certificate_Authority#Request_a_certificate

3.  request a certificate for the newly added host.
4.  Optionally, Revoke the certificate for the host




Please do not forget to test the proxy test when replica does not have 
the CA installed and has to forward the request to the one that has.





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



--
Thank you,
Dmitri Pal

Sr. Engineering Manager IPA project,
Red Hat Inc.


---
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/




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


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

[Freeipa-devel] [PATCH] 254 Fixed default map type in automount map adder dialog.

2011-08-23 Thread Endi Sukma Dewata

The adder dialog for automount map has been modified to select the
direct map by default.

Ticket #1698

--
Endi S. Dewata
From 8ec68b3e841e7cb3beb3bed4a82726acc9d0d530 Mon Sep 17 00:00:00 2001
From: Endi S. Dewata edew...@redhat.com
Date: Tue, 23 Aug 2011 20:45:06 -0500
Subject: [PATCH] Fixed default map type in automount map adder dialog.

The adder dialog for automount map has been modified to select the
direct map by default.

Ticket #1698
---
 install/ui/automount.js |  125 ++
 install/ui/dialog.js|1 +
 install/ui/widget.js|1 +
 3 files changed, 62 insertions(+), 65 deletions(-)

diff --git a/install/ui/automount.js b/install/ui/automount.js
index dc0ca4e67d3c741da6bf83bd1c9696e862f865d6..eeff8a048bbf0069949e75dbcc8a17b6b635bf47 100644
--- a/install/ui/automount.js
+++ b/install/ui/automount.js
@@ -86,32 +86,39 @@ IPA.entity_factories.automountmap = function() {
 }).
 adder_dialog({
 factory: IPA.automountmap_adder_dialog,
-fields:[{factory:IPA.method_radio_widget,
- name: 'method',
- undo: false,
- label: IPA.messages.objects.automountmap.map_type,
- options: [
- { value: 'add',
-   label: IPA.messages.objects.automountmap.direct },
- { value: 'add_indirect',
-   label: IPA.messages.objects.automountmap.indirect }
- ]
-},
-'automountmapname','description',
-{
-name: 'key',
-label: IPA.get_method_option(
-'automountmap_add_indirect', 'key').label,
-conditional: true,
-undo: false
-},
-{
-name: 'parentmap',
-label: IPA.get_method_option(
-'automountmap_add_indirect', 'parentmap').label,
-conditional: true,
-undo: false
-}]
+fields: [
+{
+factory: IPA.radio_widget,
+name: 'method',
+undo: false,
+label: IPA.messages.objects.automountmap.map_type,
+options: [
+{
+value: 'add',
+label: IPA.messages.objects.automountmap.direct
+},
+{
+value: 'add_indirect',
+label: IPA.messages.objects.automountmap.indirect
+}
+]
+},
+'automountmapname',
+'description',
+{
+name: 'key',
+label: IPA.get_method_option(
+'automountmap_add_indirect', 'key').label,
+conditional: true,
+undo: false
+},
+{
+name: 'parentmap',
+label: IPA.get_method_option(
+'automountmap_add_indirect', 'parentmap').label,
+conditional: true,
+undo: false
+}]
 }).
 build();
 };
@@ -196,13 +203,35 @@ IPA.automount_key_column = function(spec){
 };
 
 
-IPA.automountmap_adder_dialog = function(spec){
+IPA.automountmap_adder_dialog = function(spec) {
+
 var that = IPA.add_dialog(spec);
 
-that.super_create = that.create;
-that.create = function(container) {
-that.super_create(container);
-that.disable_conditional_fields();
+that.create = function() {
+that.dialog_create();
+
+var method_field = that.get_field('method');
+
+var direct_input = $('input[value=add]', method_field.container);
+direct_input.change(function() {
+that.disable_conditional_fields();
+});
+
+var indirect_input = $('input[value=add_indirect]', method_field.container);
+indirect_input.change(function() {
+that.enable_conditional_fields();
+});
+
+direct_input.click();
+};
+
+that.reset = function() {
+that.dialog_reset();
+
+var method_field = that.get_field('method');
+
+var direct_input = $('input[value=add]', method_field.container);
+direct_input.click();
 };
 
 return that;
@@ -223,37 +252,3 @@ IPA.get_option_values = function(){
 });
 return values;
 };
-
-IPA.method_radio_widget = function(spec){
-var direct = true;
-
-var that = IPA.radio_widget(spec);
-
-that.radio_create = that.create;
-
-that.create = function(container) {
-