[Freeipa-devel] [PATCH] 297 Fixed enroll labels.

2011-10-25 Thread Endi Sukma Dewata

Labels using the word enroll (except for host enrollment) have
been modified to use more relevant words.

The IPA.add_dialog has been renamed into IPA.entity_adder_dialog
for clarity.

Ticket #1642

--
Endi S. Dewata
From ae1a5d9d7c1ed811848453c080b316aa3710975c Mon Sep 17 00:00:00 2001
From: Endi S. Dewata edew...@redhat.com
Date: Mon, 24 Oct 2011 19:20:14 -0500
Subject: [PATCH] Fixed enroll labels.

Labels using the word enroll (except for host enrollment) have
been modified to use more relevant words.

The IPA.add_dialog has been renamed into IPA.entity_adder_dialog
for clarity.

Ticket #1642
---
 install/ui/add.js  |2 +-
 install/ui/association.js  |6 +++---
 install/ui/automount.js|2 +-
 install/ui/dialog.js   |2 +-
 install/ui/dns.js  |2 +-
 install/ui/entity.js   |2 +-
 install/ui/group.js|2 +-
 install/ui/host.js |2 +-
 install/ui/service.js  |6 +++---
 install/ui/test/data/ipa_init.json |9 -
 ipalib/plugins/internal.py |9 -
 11 files changed, 21 insertions(+), 23 deletions(-)

diff --git a/install/ui/add.js b/install/ui/add.js
index fd99b02c5eb77acc99484995a6fb0c65598128b7..640604d08692077c2c5cfd67db4858506204f2a5 100644
--- a/install/ui/add.js
+++ b/install/ui/add.js
@@ -23,7 +23,7 @@
 
 /* REQUIRES: ipa.js */
 
-IPA.add_dialog = function (spec) {
+IPA.entity_adder_dialog = function(spec) {
 
 spec = spec || {};
 
diff --git a/install/ui/association.js b/install/ui/association.js
index ebb6e421ff3b8538116471de240b1f972e08e6bf..d3b66132d5043b0dfe60b8847896e9f27f676059 100644
--- a/install/ui/association.js
+++ b/install/ui/association.js
@@ -889,7 +889,7 @@ IPA.association_facet = function (spec) {
 
 that.add_button = IPA.action_button({
 name: 'add',
-label: IPA.messages.buttons.enroll,
+label: IPA.messages.buttons.add,
 icon: 'add-icon',
 click: function() {
 if (!that.add_button.hasClass('action-button-disabled')) {
@@ -923,7 +923,7 @@ IPA.association_facet = function (spec) {
 }).appendTo(span);
 
 $('label/', {
-text: IPA.messages.association.direct_enrollment,
+text: IPA.messages.association.direct_membership,
 'for': direct_id
 }).appendTo(span);
 
@@ -944,7 +944,7 @@ IPA.association_facet = function (spec) {
 }).appendTo(span);
 
 $('label/', {
-text: IPA.messages.association.indirect_enrollment,
+text: IPA.messages.association.indirect_membership,
 'for': indirect_id
 }).appendTo(span);
 }
diff --git a/install/ui/automount.js b/install/ui/automount.js
index 6b740d8e6d4abc1d0e402f5a1a3b79e8a4a77420..c17e55fe9408b3787a8f3bdb22cb73d7f8c1f498 100644
--- a/install/ui/automount.js
+++ b/install/ui/automount.js
@@ -221,7 +221,7 @@ IPA.automount_key_column = function(spec) {
 
 IPA.automountmap_adder_dialog = function(spec) {
 
-var that = IPA.add_dialog(spec);
+var that = IPA.entity_adder_dialog(spec);
 
 that.create = function() {
 that.add_dialog_create();
diff --git a/install/ui/dialog.js b/install/ui/dialog.js
index b55cce715d9ba16b9062378603b4210301012872..41b35fb42120a015fcdeb56c10e80cf638456bd8 100644
--- a/install/ui/dialog.js
+++ b/install/ui/dialog.js
@@ -492,7 +492,7 @@ IPA.adder_dialog = function(spec) {
 
 var add_button = that.create_button({
 name: 'add',
-label: IPA.messages.buttons.enroll,
+label: IPA.messages.buttons.add,
 click: function() {
 if (!add_button.is_enabled()) return;
 that.execute();
diff --git a/install/ui/dns.js b/install/ui/dns.js
index 2b98f0dfec4c8f70770123930ea06081ff47633a..a1c4bbf68c294aae2563845b65ac952ff7e32d33 100644
--- a/install/ui/dns.js
+++ b/install/ui/dns.js
@@ -383,7 +383,7 @@ IPA.dnszone_adder_dialog = function(spec) {
 
 spec = spec || {};
 
-var that = IPA.add_dialog(spec);
+var that = IPA.entity_adder_dialog(spec);
 
 that.create = function() {
 that.add_dialog_create();
diff --git a/install/ui/entity.js b/install/ui/entity.js
index c82f4a8df727c1a3dd75b133d2b005867e0015ab..704b5c43b2ce7ef9dcc7221f5a73e4bf5df4bf15 100644
--- a/install/ui/entity.js
+++ b/install/ui/entity.js
@@ -914,7 +914,7 @@ IPA.entity_builder = function(){
 };
 
 that.adder_dialog = function(spec) {
-spec.factory = spec.factory || IPA.add_dialog;
+spec.factory = spec.factory || IPA.entity_adder_dialog;
 spec.name = spec.name || 'add';
 
 if (!spec.title) {
diff --git a/install/ui/group.js b/install/ui/group.js
index a63a7800a0b668152188bfccfd372e8548e484fd..8c18c14a252f10c763bfed332b14d0248ecb1c25 100644
--- a/install/ui/group.js

Re: [Freeipa-devel] [PATCH] 028 Code cleanup of HBAC, Sudo rules

2011-10-25 Thread Petr Vobornik
Comments in text. New patch not supplied yet - some topics may require 
further discussion. Most of the comments should be part of the 'Nesting 
widgets' thread.


On 10/24/2011 06:29 PM, Endi Sukma Dewata wrote:

On 10/24/2011 3:29 AM, Petr Vobornik wrote:

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


Some issues:

1. The IPA.hbacrule_details_facet uses IPA.sudo.enable_widget(). This
makes HBAC unnecessarily dependent on sudo. The enable_widget should be
moved to widget.js or rule.js which is shared between HBAC and sudo.


 * enable_widget moved to widget.js
 * rule_association_table_widget and rule_association_adder_dialog 
moved to rule.js



2. The set_facet() is added to widget. I don't think we want to make
widget dependent on facet. The facet so far is only used by
IPA.sudo.enable_widget. In IPA.sudo.options_section the facet object is
passed as a parameter in spec.
Are you saying that dependency on facet in ALL widgets is bad and only 
in a few is good, or in any is bad?


I think it doesn't matter if all is dependant when one is dependant.

Anyway currently all association table widgets are dependant 
(association.js:386).


Btw similar topic could be: How to get current entity's pkey?'. 
Dependancy on facet.get_primary_key() or 
IPA.nav.get_state(that.entity.name+'-pkey') seems wrong too.




3. When updating sudo rule, in the original code the rule is
enabled/disabled after all the modifications are done. In the new code
it's reversed. I'm not sure the importance of the execution order for
this particular situation, but suppose it is, is there a way to maintain
the original order?


I was thinking about a command or widget priority. The 'mod' command 
would have some default priority or it would get it from facet. The 
commands would be sorted by priority before adding them to batch. This 
way we can set exact order in facet update.


The order which is implemented now is reflecting the order in HBAC 
details facet - there were errors when 'mod' was executed before 
clearing associations.



4. The IPA.header_widget is not really a widget, it's actually part of
layout. In the current implementation a widget always corresponds to an
attribute, so it will be loaded, saved, etc. Since there is no attribute
name matching the header name currently this is not a problem, but it
can pollute the namespace.


This is part of widget nesting topic. In this manner composite_widget 
isn't a proper widget too (it can correspond to several or none attribute).


Purely html rendering widgets can be separated from attribute widgets, 
but it would be nice to have them because they can provide easier form 
composing. Without them it would be required to override the create 
method (in facet or composite_widget/section) which is sometimes better 
but often it creates only code duplication with little added logic.




5. In details facet update(), instead of storing the callbacks in
update_custom_on_win and update_custom_on_fail and requiring the
subclasses to call them, it might be better to call them from the
update() itself:

var command = IPA.command({
...
on_success: function(data, text_status, xhr) {
that.on_update_success(data, text_status, xhr)
if (on_win) on_win.call(this, data, text_status, xhr);
},
on_error: function(xhr, text_status, error_thrown) {
that.on_update_error(xhr, text_status, error_thrown);
if (on_fail) on_fail.call(
this, xhr, text_status, error_thrown);
}
});


There are two types of success/error handlers:
1) the default ones in details facet
2) the ones supplied to update(win, fail) method.

In my implementation I have separated 1) from update method so if 
default behaviour isn't suitable these methods can be overridden without 
overriding whole update method. Overriding isn't required. This is IMHO 
good (less code duplication).





6. Can IPA.batch_details_facet be combined into IPA.details_facet? I
think the base details facet should support both regular mod and batch.


The only point where they overlap is the update method. It's possible to 
add a logic to switch between command creations - maybe based on 
attribute (could be defined in spec). But should we do that? Isn't this 
the reason for inheriting the class?




7. In sudo details page, the As Whom category is missing the RunAs
User category radio buttons.


Fixed


8. The IPA.sudo.rule_details_runas_widget uses composite widget instead
of section. They are right now functionally identical, but I suppose the
composite widget is an abstract class and the section will later contain
code specific to section.


Part of widget nesting topic. This was briefly discussed with Adam (in 
that topic). The important question is what is a section? I understand 
section as top level container (in facet) which contains widgets (even 
composites) and semantically separates parts of the facet (with headers 
and section folding...). From this point of view, composite_widget isn't 
an abstract class, but it isn't used used anywhere else 

Re: [Freeipa-devel] Keytab for talking to PKI CA from IPA

2011-10-25 Thread Rob Crittenden

Adam Young wrote:

When setting up replication, it should not be necessary to cache any
passwords, anywhere, until the replication agreemsnts are set up, and
then, all caching should be using known secure mechanisms.

The two main repositories we care about are the Directory Server
instances managed by IPA and the Certificate Authority. Currently, these
are not in the same Dir Srv isntance (although they could be) due to the
fact that we expect to replicate them seperately. Specifically, we
expect to have more IPA instances than CA instances, and we will not
have a CA instance without a co-located IPA instance.


Not really. I created two instances to provide logical separation and 
prevent us from having to deal with multiple naming contexts. There was 
some serious investigation over the summer to see if we could 
consolidate into a single instance. It was determined to be too much too 
late in the cycle.



During normal operations, the IPA instance should not need to talk to
the directory server instance of the CA. All communication between IPA
and the CA should happen via the HTTPS secured via Certificates issued
by the CA.


Right, this has never been in question AFAIK.


Once the replication process starts, the file generated by ipa-prepare
replicate should not need an passwords. Instead, when the replicated
server is installed, the user performing the install should get a ticket
as an administrative user. All authentication for the replication should
be based on that ticket.

The very first step is to install an new Directory server for IPA. For
this, the replication process can generate a single use password and use
it as the Directory server password for the local instance. Next, the
ticket for the adminiustrative user should be used to download a keytab
for the Directory Server to use when communicating back to the original
IPA directory server.With these keytab in place, the replicated IPA DS
should be able to talk to the original IPA DS and establish the
replication agreement. At this point, the single use password should be
disabled.


I think you mean the first step is to create a 389-ds instance for the 
CA, right?


Why not simply pre-create the keytab and ship it in the prepared file?

There is no need to disable the random DM password. Once the 
installation is complete it will be unknown so effectively disabled.



Once the IPA Directory server has been replicated, we can either use the
original keytab or download a second keytab to establish a replication
agreement between the replicated CA directory server and the original CA
directory server. The process would look the same as setting up the
keytab for the IPA directory server.

Why don't we do this now?


Because of permission issues writing the relevant entries to cn=config.

rob

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


[Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib

2011-10-25 Thread Ondrej Hamada

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

Lazy initialization of ipalib plugins is used under all contexts, not 
only when context = cli. Every loaded plugin is pre-finalized - a flag 
is set, which means, that the plugin needs to be finalized. Then every 
call of plugin's __gettattr__ checks the flag and finalizes the plugin 
if necessary. The code was implemented by jcholast. Time reduction of 
commands execution is quite markable:


patch [s] |   normal [s]|   command
---
1.468  |   2.287   |   ipa user-add jsmith --firt=john 
--last=smith

1.658  |   2.235   |   ipa user-del jsmith
1.624  |   2.204   |   ipa dnsrecord-find example.com

--
Regards,

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

diff --git a/ipalib/plugable.py b/ipalib/plugable.py
index b0e4156..2aed1cd 100644
--- a/ipalib/plugable.py
+++ b/ipalib/plugable.py
@@ -173,6 +173,7 @@ class Plugin(ReadOnly):
 
 
 label = None
+__try_finalize = False
 
 def __init__(self):
 self.__api = None
@@ -208,6 +209,16 @@ class Plugin(ReadOnly):
 )
 )
 
+def __getattribute__(self, name):
+if name.startswith('_Plugin__') or name.startswith('_ReadOnly__'):
+return object.__getattribute__(self, name)
+if self.__try_finalize:
+self.__try_finalize = False
+self.finalize()
+if not is_production_mode(self.__api):
+assert islocked(self) is True
+return object.__getattribute__(self, name)
+
 def __get_api(self):
 
 Return `API` instance passed to `finalize()`.
@@ -217,6 +228,9 @@ class Plugin(ReadOnly):
 return self.__api
 api = property(__get_api)
 
+def prefinalize(self):
+self.__try_finalize = True
+
 def finalize(self):
 
 
@@ -638,9 +652,7 @@ class API(DictProxy):
 assert p.instance.api is self
 
 for p in plugins.itervalues():
-p.instance.finalize()
-if not production_mode:
-assert islocked(p.instance) is True
+p.instance.prefinalize()
 object.__setattr__(self, '_API__finalized', True)
 tuple(PluginInfo(p) for p in plugins.itervalues())
 object.__setattr__(self, 'plugins',
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib

2011-10-25 Thread Martin Kosek
On Tue, 2011-10-25 at 15:29 +0200, Ondrej Hamada wrote:
 https://fedorahosted.org/freeipa/ticket/1336
 
 Lazy initialization of ipalib plugins is used under all contexts, not 
 only when context = cli. Every loaded plugin is pre-finalized - a flag 
 is set, which means, that the plugin needs to be finalized. Then every 
 call of plugin's __gettattr__ checks the flag and finalizes the plugin 
 if necessary. The code was implemented by jcholast. Time reduction of 
 commands execution is quite markable:
 
 patch [s] |   normal [s]|   command
 ---
 1.468  |   2.287   |   ipa user-add jsmith --firt=john 
 --last=smith
 1.658  |   2.235   |   ipa user-del jsmith
 1.624  |   2.204   |   ipa dnsrecord-find example.com
 

Thanks for submitting the patch. Ondra, just please provide the patch in
proper format (exported via command `git format-patch -M -C --patience
--full-index -1' which I sent you earlier).

Martin


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


[Freeipa-devel] [PATCH] 155 Fix ipa-managed-entries bind procedure

2011-10-25 Thread Martin Kosek
Make sure that when Directory Manager password is entered,
we directly do a simple bind instead of trying binding via GSSAPI.
Also capture ldap.INVALID_CREDENTIALS exception and provide nice
error message than crash.

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

From 332f96ea1e4c77d429adaad858a459138c0bfb9d Mon Sep 17 00:00:00 2001
From: Martin Kosek mko...@redhat.com
Date: Tue, 25 Oct 2011 15:34:45 +0200
Subject: [PATCH] Fix ipa-managed-entries bind procedure

Make sure that when Directory Manager password is entered,
we directly do a simple bind instead of trying binding via GSSAPI.
Also capture ldap.INVALID_CREDENTIALS exception and provide nice
error message than crash.

https://fedorahosted.org/freeipa/ticket/1927
---
 install/tools/ipa-managed-entries |   20 +---
 1 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/install/tools/ipa-managed-entries b/install/tools/ipa-managed-entries
index 16f0a956cd2b1398dc3385d3f2254cb56cf23c09..649ef80017d7db57ab1efac859c5eb12450168db 100755
--- a/install/tools/ipa-managed-entries
+++ b/install/tools/ipa-managed-entries
@@ -106,15 +106,21 @@ def main():
 try:
 filter = '(objectClass=extensibleObject)'
 conn = ipaldap.IPAdmin(host, 636, cacert=CACERT)
-conn.do_sasl_gssapi_bind()
-except ldap.LOCAL_ERROR:
+
 if options.dirman_password:
-dirman_password = options.dirman_password
+conn.do_simple_bind(bindpw=options.dirman_password)
 else:
-dirman_password = get_dirman_password()
-if dirman_password is None:
-sys.exit(\nDirectory Manager password required)
-conn.do_simple_bind(bindpw=dirman_password)
+conn.do_sasl_gssapi_bind()
+except ldap.LOCAL_ERROR:
+dirman_password = get_dirman_password()
+if dirman_password is None:
+sys.exit(\nDirectory Manager password required)
+try:
+conn.do_simple_bind(bindpw=dirman_password)
+except ldap.INVALID_CREDENTIALS:
+sys.exit(Invalid credentials)
+except ldap.INVALID_CREDENTIALS:
+sys.exit(Invalid credentials)
 except errors.ExecutionError, lde:
 sys.exit(An error occurred while connecting to the server.\n%s\n %
 str(lde))
-- 
1.7.6.4

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

Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib

2011-10-25 Thread Ondrej Hamada

On 10/25/2011 04:01 PM, Martin Kosek wrote:

On Tue, 2011-10-25 at 15:29 +0200, Ondrej Hamada wrote:

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

Lazy initialization of ipalib plugins is used under all contexts, not
only when context = cli. Every loaded plugin is pre-finalized - a flag
is set, which means, that the plugin needs to be finalized. Then every
call of plugin's __gettattr__ checks the flag and finalizes the plugin
if necessary. The code was implemented by jcholast. Time reduction of
commands execution is quite markable:

patch [s] |   normal [s]|   command
---
1.468  |   2.287   |   ipa user-add jsmith --firt=john
--last=smith
1.658  |   2.235   |   ipa user-del jsmith
1.624  |   2.204   |   ipa dnsrecord-find example.com


Thanks for submitting the patch. Ondra, just please provide the patch in
proper format (exported via command `git format-patch -M -C --patience
--full-index -1' which I sent you earlier).

Martin



Sorry, correct version attached

--
Regards,

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

From 798d8f8a624f8350974e54c328c1c58c06944b26 Mon Sep 17 00:00:00 2001
From: Ondrej Hamada oham...@redhat.com
Date: Tue, 25 Oct 2011 16:20:44 +0200
Subject: [PATCH] lazy initialization patch

---
 ipalib/plugable.py |   18 +++---
 1 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/ipalib/plugable.py b/ipalib/plugable.py
index b0e415656e0428eb164c35a2862fcfbf50883381..2aed1cdcda18840728558ed53435ab10ae28e802 100644
--- a/ipalib/plugable.py
+++ b/ipalib/plugable.py
@@ -173,6 +173,7 @@ class Plugin(ReadOnly):
 
 
 label = None
+__try_finalize = False
 
 def __init__(self):
 self.__api = None
@@ -208,6 +209,16 @@ class Plugin(ReadOnly):
 )
 )
 
+def __getattribute__(self, name):
+if name.startswith('_Plugin__') or name.startswith('_ReadOnly__'):
+return object.__getattribute__(self, name)
+if self.__try_finalize:
+self.__try_finalize = False
+self.finalize()
+if not is_production_mode(self.__api):
+assert islocked(self) is True
+return object.__getattribute__(self, name)
+
 def __get_api(self):
 
 Return `API` instance passed to `finalize()`.
@@ -217,6 +228,9 @@ class Plugin(ReadOnly):
 return self.__api
 api = property(__get_api)
 
+def prefinalize(self):
+self.__try_finalize = True
+
 def finalize(self):
 
 
@@ -638,9 +652,7 @@ class API(DictProxy):
 assert p.instance.api is self
 
 for p in plugins.itervalues():
-p.instance.finalize()
-if not production_mode:
-assert islocked(p.instance) is True
+p.instance.prefinalize()
 object.__setattr__(self, '_API__finalized', True)
 tuple(PluginInfo(p) for p in plugins.itervalues())
 object.__setattr__(self, 'plugins',
-- 
1.7.6.4

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

Re: [Freeipa-devel] [PATCH] 295 Fixed inconsistent required/optional attributes.

2011-10-25 Thread Endi Sukma Dewata

On 10/25/2011 9:20 AM, Petr Vobornik wrote:

ACK


Pushed to master.


Also proposing minor visual enhancement (see attached patch).


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] 0030 Quote worker option

2011-10-25 Thread Alexander Bokovoy
https://fedorahosted.org/freeipa/ticket/2023

-- 
/ Alexander Bokovoy
From 29eb102e9319eff837d71e4da6ad45796f3e7868 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy aboko...@redhat.com
Date: Tue, 25 Oct 2011 18:41:32 +0300
Subject: [PATCH] Quote multiple workers option

https://fedorahosted.org/freeipa/ticket/2023
---
 ipaserver/install/krbinstance.py |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/ipaserver/install/krbinstance.py b/ipaserver/install/krbinstance.py
index 
b4f0a5446be0dfb1621e256b65ac34906df7351e..ce70c231dfb7e7b6b59c0496721cced0d09f1604
 100644
--- a/ipaserver/install/krbinstance.py
+++ b/ipaserver/install/krbinstance.py
@@ -365,7 +365,7 @@ class KrbInstance(service.Service):
 replacevars = {'KRB5REALM':self.realm}
 appendvars = {}
 if workers and cpus  1:
-appendvars = {'KRB5KDC_ARGS': -w %s % str(cpus)}
+appendvars = {'KRB5KDC_ARGS': '-w %s' % str(cpus)}
 ipautil.backup_config_and_replace_variables(self.fstore, 
/etc/sysconfig/krb5kdc,
 replacevars=replacevars,
 appendvars=appendvars)
-- 
1.7.7

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

Re: [Freeipa-devel] [PATCH] 0030 Quote worker option

2011-10-25 Thread Martin Kosek
On Tue, 2011-10-25 at 18:44 +0300, Alexander Bokovoy wrote:
 https://fedorahosted.org/freeipa/ticket/2023
 

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

Martin

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


Re: [Freeipa-devel] [PATCH] 028 Code cleanup of HBAC, Sudo rules

2011-10-25 Thread Endi Sukma Dewata

On 10/25/2011 7:49 AM, Petr Vobornik wrote:

2. The set_facet() is added to widget. I don't think we want to make
widget dependent on facet. The facet so far is only used by
IPA.sudo.enable_widget. In IPA.sudo.options_section the facet object is
passed as a parameter in spec.



Are you saying that dependency on facet in ALL widgets is bad and only
in a few is good, or in any is bad?


In general less dependency is better. More dependency will limit its 
usage. This is inline with the goal to make purely HTML widget.


But if a widget is meant to be used only with a facet then it's 
certainly ok to make it depends on facet. However, the rest should not 
become unnecessarily dependent on facet too.



I think it doesn't matter if all is dependant when one is dependant.

Anyway currently all association table widgets are dependant
(association.js:386).


Let's use this as an example. The association table explicitly checks if 
the facet is dirty and asks the user whether to update/reset/cancel. 
Suppose we want to use this inside a dialog, it will still check whether 
the current facet is dirty instead of the dialog itself, which may not 
be what we want.



Btw similar topic could be: How to get current entity's pkey?'.
Dependancy on facet.get_primary_key() or
IPA.nav.get_state(that.entity.name+'-pkey') seems wrong too.


Yes. Ideally we should have a path (REST-style URL) instead of the 
current parameter-based URL. We should get the pkey from that path.



3. When updating sudo rule, in the original code the rule is
enabled/disabled after all the modifications are done. In the new code
it's reversed. I'm not sure the importance of the execution order for
this particular situation, but suppose it is, is there a way to maintain
the original order?


I was thinking about a command or widget priority. The 'mod' command
would have some default priority or it would get it from facet. The
commands would be sorted by priority before adding them to batch. This
way we can set exact order in facet update.


It's possible, but managing priority could be problematic if they are 
spread out, because we have to know all existing priorities before we 
can add a new one. The original code uses an ordered list of commands.



The order which is implemented now is reflecting the order in HBAC
details facet - there were errors when 'mod' was executed before
clearing associations.


Right, so for certain things the order is important.


4. The IPA.header_widget is not really a widget, it's actually part of
layout. In the current implementation a widget always corresponds to an
attribute, so it will be loaded, saved, etc. Since there is no attribute
name matching the header name currently this is not a problem, but it
can pollute the namespace.


This is part of widget nesting topic. In this manner composite_widget
isn't a proper widget too (it can correspond to several or none attribute).

Purely html rendering widgets can be separated from attribute widgets,
but it would be nice to have them because they can provide easier form
composing. Without them it would be required to override the create
method (in facet or composite_widget/section) which is sometimes better
but often it creates only code duplication with little added logic.


One other solution is to split widgets into non-visual fields and purely 
HTML components. Then in the facet we use separate lists for the fields 
and HTML components. The fields will have a reference to the 
corresponding HTML components. There can be more HTML components than 
fields. But this will require a significant restructuring.



5. In details facet update(), instead of storing the callbacks in
update_custom_on_win and update_custom_on_fail and requiring the
subclasses to call them, it might be better to call them from the
update() itself:

var command = IPA.command({
...
on_success: function(data, text_status, xhr) {
that.on_update_success(data, text_status, xhr)
if (on_win) on_win.call(this, data, text_status, xhr);
},
on_error: function(xhr, text_status, error_thrown) {
that.on_update_error(xhr, text_status, error_thrown);
if (on_fail) on_fail.call(
this, xhr, text_status, error_thrown);
}
});


There are two types of success/error handlers:
1) the default ones in details facet
2) the ones supplied to update(win, fail) method.

In my implementation I have separated 1) from update method so if
default behaviour isn't suitable these methods can be overridden without
overriding whole update method. Overriding isn't required. This is IMHO
good (less code duplication).


There is still code duplication, the subclass that overrides the 
on_update_success/error (#1) will still have to call 
update_custom_on_win/error (#2) manually. In the changes that I proposed 
you don't have to do that because #2 are called inside update().



6. Can IPA.batch_details_facet be combined into IPA.details_facet? I
think the base details facet should support both regular mod and batch.


The only point 

Re: [Freeipa-devel] [PATCH] 136 Fix ipa-managed-entries password option long form

2011-10-25 Thread Rob Crittenden

Martin Kosek wrote:

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


ACK

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


Re: [Freeipa-devel] [PATCH] 151 Add --zonemgr validator

2011-10-25 Thread Rob Crittenden

Martin Kosek wrote:

On Mon, 2011-10-24 at 17:08 +0200, Martin Kosek wrote:

On Mon, 2011-10-24 at 09:02 -0400, Rob Crittenden wrote:

Martin Kosek wrote:

On Fri, 2011-10-21 at 11:31 -0400, Rob Crittenden wrote:

Martin Kosek wrote:

On Fri, 2011-10-14 at 14:11 -0400, Rob Crittenden wrote:

Martin Kosek wrote:

Do at least a basic validation of DNS zone manager mail address.

Do not require '@' to be in the mail address as it is not used
in common DNS zone configuration (in bind for example) and people
may be used to configure it that way. '@' is always removed by the
installer before the DNS zone is created.

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


There is already a zonemgr_callback defined for this option, can the
verify_zonemgr call be either integrated or called from that?

rob



Right. Please, try this one. I also added a parser error when more than
one '@' is in the checked value.

Martin


A couple of things:

In the block where you are counting @ why not add an :

else:
   raise ValueError('address is not fully qualified')

rather than looking for '.' in the result? I think it will be clearer
that way. I wonder if the error should contain an example as well, are
people going to know what a fully-qualified means?

The regex is very strict for e-mail addresses, perhaps too much so. It
doesn't allow upper-case characters, periods or _, both of which are
allowed in login names. A common e-mail format is first.last@domain.

rob


I reorganized the validator a little and let people enter _ in the
domain name. I also added a small explanation of what we mean by
fully-qualified.

Since we have the zonemgr validator available, why not use it for the
DNS plugin too? I enhanced the plugin to use this validator too. Please,
see attached patch.

Martin


NACK, a client might not have the server sub-package installed so the
import of bindinstance will fail.

I think that moving the validator into dns.py as a central location
should work though.

Otherwise looks good.

rob


Thanks. I didn't realize that user can have freeipa-admintools without
freeipa-server installed.

I placed the validator to ipalib/util.py as we cannot place it to the
dns plugin directly as all our plugins assume that we have initialized
api.

Updated patch attached.

Martin


And now also with API.txt change. This patch must be cursed or
something.

No need to bump API version, just the validator has been added.

Martin


ACK

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


[Freeipa-devel] [PATCH] 298 Fixed host Enrolled column.

2011-10-25 Thread Endi Sukma Dewata

The Enrolled column in the host search page has been added back
to show the host enrollment status based on has_keytab attribute.

Ticket #2020

--
Endi S. Dewata
From 05c85f889e1f58716fd6368451bdc8c9a22afb33 Mon Sep 17 00:00:00 2001
From: Endi S. Dewata edew...@redhat.com
Date: Tue, 25 Oct 2011 14:25:31 -0500
Subject: [PATCH] Fixed host Enrolled column.

The Enrolled column in the host search page has been added back
to show the host enrollment status based on has_keytab attribute.

Ticket #2020
---
 install/ui/host.js  |6 +++-
 install/ui/test/data/host_find.json |   59 --
 2 files changed, 40 insertions(+), 25 deletions(-)

diff --git a/install/ui/host.js b/install/ui/host.js
index b50287291568c95830b275cdfa8dc3332677e2c2..4c0ce6ed0e461a38a565c1450cd483098b0c2dc7 100644
--- a/install/ui/host.js
+++ b/install/ui/host.js
@@ -31,7 +31,11 @@ IPA.entity_factories.host = function () {
 search_facet({
 columns: [
 'fqdn',
-'description'
+'description',
+{
+name: 'has_keytab',
+label: IPA.messages.objects.host.enrolled
+}
 ]
 }).
 details_facet({
diff --git a/install/ui/test/data/host_find.json b/install/ui/test/data/host_find.json
index 48b1fcb893f8f94f50a369dcd16775297a449e64..34ca71a2ee48e9b947cafed67b4cbdc9b7460d63 100644
--- a/install/ui/test/data/host_find.json
+++ b/install/ui/test/data/host_find.json
@@ -1,6 +1,6 @@
 {
 error: null,
-id: 0,
+id: null,
 result: {
 count: 2,
 result: [
@@ -8,46 +8,44 @@
 cn: [
 dev.example.com
 ],
-dn: fqdn=dev.example.com,cn=computers,cn=accounts,dc=dev,dc=example,dc=com,
+dn: fqdn=dev.example.com,cn=computers,cn=accounts,dc=example,dc=com,
+enrolledby_user: [
+admin
+],
 fqdn: [
 dev.example.com
 ],
+has_keytab: true,
+has_password: false,
 ipauniqueid: [
-fc6a6d5a-f388-11df-9c01-00163e72f2d9
+d2c1f41c-ff41-11e0-bde8-525400e135d8
 ],
 krbextradata: [
 {
-__base64__: AAL+5+VMYWRtaW4vYWRtaW5AREVWLkVYQU1QTEUuQ09NAA==
-},
-{
-__base64__: AAgBAA==
+__base64__: AAIIEqdOaG9zdC9kZXYuZXhhbXBsZS5jb21ASURNLkxBQi5CT1MuUkVESEFULkNPTQA=
 }
 ],
 krblastpwdchange: [
-20101119025910Z
-],
-krbpasswordexpiration: [
-1970010100Z
+20111025194616Z
 ],
 krbprincipalname: [
-host/dev.example@dev.example.com
+host/dev.example@example.com
 ],
-krbticketflags: [
-0
+managedby_host: [
+dev.example.com
 ],
-managedby: [
-fqdn=dev.example.com,cn=computers,cn=accounts,dc=dev,dc=example,dc=com
+managing_host: [
+dev.example.com
 ],
 objectclass: [
-top,
 ipaobject,
 nshost,
 ipahost,
-ipaservice,
 pkiuser,
+ipaservice,
 krbprincipalaux,
 krbprincipal,
-krbticketpolicyaux
+top
 ],
 serverhostname: [
 dev
@@ -57,18 +55,31 @@
 cn: [
 test.example.com
 ],
-dn: fqdn=test.example.com,cn=computers,cn=accounts,dc=dev,dc=example,dc=com,
+dn: fqdn=test.example.com,cn=computers,cn=accounts,dc=example,dc=com,
+enrolledby_user: [
+admin
+],
 fqdn: [
 test.example.com
 ],
+has_keytab: false,
+has_password: false,
 ipauniqueid: [
-ac28dca0-f3b5-11df-879f-00163e72f2d9
+d5d1400e-ff41-11e0-9a21-525400e135d8
+],
+krbextradata: [
+{
+__base64__: AAIiEqdOaG9zdC90ZXN0LmV4YW1wbGUuY29tQElETS5MQUIuQk9TLlJFREhBVC5DT00A
+}
 ],
 krbprincipalname: [
-host/test.example@dev.example.com
+

Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib

2011-10-25 Thread Rob Crittenden

Ondrej Hamada wrote:

On 10/25/2011 04:01 PM, Martin Kosek wrote:

On Tue, 2011-10-25 at 15:29 +0200, Ondrej Hamada wrote:

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

Lazy initialization of ipalib plugins is used under all contexts, not
only when context = cli. Every loaded plugin is pre-finalized - a flag
is set, which means, that the plugin needs to be finalized. Then every
call of plugin's __gettattr__ checks the flag and finalizes the plugin
if necessary. The code was implemented by jcholast. Time reduction of
commands execution is quite markable:

patch [s] | normal [s] | command
---
1.468 | 2.287 | ipa user-add jsmith --firt=john
--last=smith
1.658 | 2.235 | ipa user-del jsmith
1.624 | 2.204 | ipa dnsrecord-find example.com


Thanks for submitting the patch. Ondra, just please provide the patch in
proper format (exported via command `git format-patch -M -C --patience
--full-index -1' which I sent you earlier).

Martin



Sorry, correct version attached


Wow, this is very impressive, great job Ondra and Honza!

Martin, ACK from me but I'd like a second opinion. The patch is very 
straightforward and clean, just want to make sure we're not missing a 
corner case.


I ran the self-tests and didn't see any problem there.

Before pushing please add the ticket # to the commit.

rob

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